refactor: simplify and speed up plan output formatting by removing name resolver dependency

pkg/client/deploy/deploy.go

@@ -45,7 +45,7 @@ type ServicePlan struct {
 }
 
 // Format renders the service plan as a styled block with a spec diff and nested container operations.
-func (sp *ServicePlan) Format(resolver operation.NameResolver) string {
+func (sp *ServicePlan) Format() string {
 	// Determine service-level operation type and extract the old spec from container operations.
 	// Assume replace operations precede remove operations (rolling strategy) so the first replace operation
 	// (if exists) determines the old spec for the diff. Otherwise, fallback to the first remove operation.
@@ -156,7 +156,7 @@ func (sp *ServicePlan) Format(resolver operation.NameResolver) string {
 		if i == opsCount-1 {
 			connector = tui.Faint.Render("  ╰──")
 		}
-		out.WriteString(connector + " " + op.Format(resolver))
+		out.WriteString(connector + " " + op.Format())
 		out.WriteString("\n")
 	}
 

pkg/client/deploy/operation/container.go

@@ -17,6 +17,8 @@ type RunContainerOperation struct {
 	ServiceID string
 	Spec      api.ServiceSpec
 	MachineID string
+	// MachineName is used for formatting the operation as part of the deployment plan.
+	MachineName string
 	// SkipHealthMonitor skips the monitoring period and health checks after starting a container.
 	SkipHealthMonitor bool
 }
@@ -43,13 +45,12 @@ func (o *RunContainerOperation) Execute(ctx context.Context, cli Client) error {
 	return nil
 }
 
-func (o *RunContainerOperation) Format(resolver NameResolver) string {
-	machineName := resolver.MachineName(o.MachineID)
+func (o *RunContainerOperation) Format() string {
 	return tui.BoldGreen.Render("+") + "   " +
 		tui.Faint.Render("run container") + " " +
 		o.Spec.Name + " " +
 		tui.Faint.Render("on") + " " +
-		machineName
+		o.MachineName
 }
 
 func (o *RunContainerOperation) String() string {
@@ -59,9 +60,11 @@ func (o *RunContainerOperation) String() string {
 
 // StopContainerOperation stops a container on a specific machine.
 type StopContainerOperation struct {
-	ServiceID       string
-	ContainerID     string
-	MachineID       string
+	ServiceID   string
+	ContainerID string
+	MachineID   string
+	// MachineName is used for formatting the operation as part of the deployment plan.
+	MachineName     string
 	StopGracePeriod *time.Duration
 }
 
@@ -72,8 +75,7 @@ func (o *StopContainerOperation) Execute(ctx context.Context, cli Client) error
 	return nil
 }
 
-func (o *StopContainerOperation) Format(resolver NameResolver) string {
-	machineName := resolver.MachineName(o.MachineID)
+func (o *StopContainerOperation) Format() string {
 	// TODO: pass service name to format the display name consistently with other operations.
 	displayName := stringid.TruncateID(o.ContainerID)
 
@@ -81,7 +83,7 @@ func (o *StopContainerOperation) Format(resolver NameResolver) string {
 		tui.Faint.Render("stop container") + " " +
 		displayName + " " +
 		tui.Faint.Render("on") + " " +
-		machineName
+		o.MachineName
 }
 
 func (o *StopContainerOperation) String() string {
@@ -91,7 +93,9 @@ func (o *StopContainerOperation) String() string {
 
 // RemoveContainerOperation stops and removes a container from a specific machine.
 type RemoveContainerOperation struct {
-	MachineID       string
+	MachineID string
+	// MachineName is used for formatting the operation as part of the deployment plan.
+	MachineName     string
 	Container       api.ServiceContainer
 	StopGracePeriod *time.Duration
 }
@@ -112,15 +116,14 @@ func (o *RemoveContainerOperation) Execute(ctx context.Context, cli Client) erro
 	return nil
 }
 
-func (o *RemoveContainerOperation) Format(resolver NameResolver) string {
-	machineName := resolver.MachineName(o.MachineID)
+func (o *RemoveContainerOperation) Format() string {
 	displayName := o.Container.ServiceSpec.Name + tui.Faint.Render("/") + o.Container.ShortID()
 
 	return tui.BoldRed.Render("-") + "   " +
 		tui.Faint.Render("remove container") + " " +
 		displayName + " " +
 		tui.Faint.Render("on") + " " +
-		machineName
+		o.MachineName
 }
 
 func (o *RemoveContainerOperation) String() string {
@@ -132,9 +135,11 @@ func (o *RemoveContainerOperation) String() string {
 // For start-first: starts new container, then removes old container.
 // For stop-first: stops old container, starts new container, then removes old container.
 type ReplaceContainerOperation struct {
-	ServiceID    string
-	Spec         api.ServiceSpec
-	MachineID    string
+	ServiceID string
+	Spec      api.ServiceSpec
+	MachineID string
+	// MachineName is used for formatting the operation as part of the deployment plan.
+	MachineName  string
 	OldContainer api.ServiceContainer
 	// Order specifies the update order: "start-first" or "stop-first".
 	Order string
@@ -223,8 +228,7 @@ func (o *ReplaceContainerOperation) Execute(ctx context.Context, cli Client) err
 	return nil
 }
 
-func (o *ReplaceContainerOperation) Format(resolver NameResolver) string {
-	machineName := resolver.MachineName(o.MachineID)
+func (o *ReplaceContainerOperation) Format() string {
 	displayName := o.Spec.Name + tui.Faint.Render("/") + o.OldContainer.ShortID()
 
 	if o.Order == api.UpdateOrderStopFirst {
@@ -232,14 +236,14 @@ func (o *ReplaceContainerOperation) Format(resolver NameResolver) string {
 			tui.Faint.Render("replace container") + " " +
 			displayName + " " +
 			tui.Faint.Render("on") + " " +
-			machineName + " " +
+			o.MachineName + " " +
 			tui.Yellow.Render("(stop-first)")
 	}
 	return tui.BoldGreen.Render("+") + tui.Green.Render("/") + tui.BoldGreen.Render("-") + " " +
 		tui.Faint.Render("replace container") + " " +
 		displayName + " " +
 		tui.Faint.Render("on") + " " +
-		machineName
+		o.MachineName
 }
 
 func (o *ReplaceContainerOperation) String() string {

pkg/client/deploy/operation/operation.go

@@ -12,8 +12,7 @@ type Operation interface {
 	// Execute performs the operation using the provided client.
 	Execute(ctx context.Context, cli Client) error
 	// Format returns a human-readable representation of the operation.
-	// TODO: get rid of the resolver and assign the required names for formatting in the operation itself.
-	Format(resolver NameResolver) string
+	Format() string
 	String() string
 }
 
@@ -22,9 +21,3 @@ type Client interface {
 	api.ContainerClient
 	api.VolumeClient
 }
-
-// NameResolver resolves machine and container IDs to their names.
-type NameResolver interface {
-	MachineName(machineID string) string
-	ContainerName(containerID string) string
-}

pkg/client/deploy/operation/sequence.go

@@ -20,10 +20,10 @@ func (o *SequenceOperation) Execute(ctx context.Context, cli Client) error {
 	return nil
 }
 
-func (o *SequenceOperation) Format(resolver NameResolver) string {
+func (o *SequenceOperation) Format() string {
 	lines := make([]string, len(o.Operations))
 	for i, op := range o.Operations {
-		lines[i] = op.Format(resolver)
+		lines[i] = op.Format()
 	}
 
 	return strings.Join(lines, "\n")

pkg/client/deploy/operation/volume.go

@@ -13,7 +13,7 @@ import (
 type CreateVolumeOperation struct {
 	VolumeSpec api.VolumeSpec
 	MachineID  string
-	// MachineName is used for formatting the operation output only.
+	// MachineName is used for formatting the operation as part of the deployment plan.
 	MachineName string
 }
 
@@ -40,7 +40,7 @@ func (o *CreateVolumeOperation) Execute(ctx context.Context, cli Client) error {
 	return nil
 }
 
-func (o *CreateVolumeOperation) Format(_ NameResolver) string {
+func (o *CreateVolumeOperation) Format() string {
 	return fmt.Sprintf("%s create volume %s %s %s",
 		tui.BoldGreen.Render("+"),
 		tui.NameStyle.Render(o.VolumeSpec.DockerVolumeName()),

pkg/client/deploy/strategy.go

@@ -67,6 +67,12 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec)
 		return plan, err
 	}
 
+	// Build a machine ID to name map from the cluster state to resolve IDs for operations.
+	machineNames := make(map[string]string, len(s.state.Machines))
+	for _, m := range s.state.Machines {
+		machineNames[m.Info.Id] = m.Info.Name
+	}
+
 	sched := scheduler.NewServiceScheduler(s.state, spec)
 	// TODO: return a detailed report on required constraints and which ones are satisfied?
 	availableMachines, err := sched.EligibleMachines()
@@ -151,6 +157,7 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec)
 				ServiceID:         plan.ServiceID,
 				Spec:              spec,
 				MachineID:         m.Id,
+				MachineName:       m.Name,
 				SkipHealthMonitor: s.SkipHealthMonitor,
 			})
 			continue
@@ -172,6 +179,7 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec)
 			ServiceID:         plan.ServiceID,
 			Spec:              spec,
 			MachineID:         m.Id,
+			MachineName:       m.Name,
 			OldContainer:      ctr,
 			Order:             order,
 			SkipHealthMonitor: s.SkipHealthMonitor,
@@ -184,6 +192,7 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec)
 		for _, c := range containers {
 			plan.Operations = append(plan.Operations, &operation.RemoveContainerOperation{
 				MachineID:       mid,
+				MachineName:     machineNames[mid],
 				Container:       c,
 				StopGracePeriod: spec.StopGracePeriod,
 			})
@@ -204,6 +213,12 @@ func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Se
 		return plan, err
 	}
 
+	// Build a machine ID to name map from the cluster state to resolve IDs for operations.
+	machineNames := make(map[string]string, len(s.state.Machines))
+	for _, m := range s.state.Machines {
+		machineNames[m.Info.Id] = m.Info.Name
+	}
+
 	// Map machineID to service containers on that machine. For the global mode, there should be at most one
 	// container per machine but we use a slice to handle multiple containers that may exist due to a bug
 	// or interruption in the previous deployment.
@@ -223,7 +238,13 @@ func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Se
 	for _, m := range availableMachines {
 		containers := containersOnMachine[m.Info.Id]
 		ops, err := reconcileGlobalContainer(
-			containers, spec, plan.ServiceID, m.Info.Id, s.ForceRecreate, s.SkipHealthMonitor)
+			containers,
+			spec,
+			plan.ServiceID,
+			m.Info,
+			s.ForceRecreate,
+			s.SkipHealthMonitor,
+		)
 		if err != nil {
 			return plan, err
 		}
@@ -237,6 +258,7 @@ func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Se
 		for _, c := range containers {
 			plan.Operations = append(plan.Operations, &operation.RemoveContainerOperation{
 				MachineID:       c.MachineID,
+				MachineName:     machineNames[c.MachineID],
 				Container:       c.Container,
 				StopGracePeriod: spec.StopGracePeriod,
 			})
@@ -250,7 +272,10 @@ func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Se
 // It ensures exactly one container with the desired spec is running on the machine by creating a new container and
 // removing old ones. If there is a host port conflict, it stops the old container before starting a new one.
 func reconcileGlobalContainer(
-	containers []api.MachineServiceContainer, spec api.ServiceSpec, serviceID, machineID string,
+	containers []api.MachineServiceContainer,
+	spec api.ServiceSpec,
+	serviceID string,
+	machine *pb.MachineInfo,
 	forceRecreate, skipHealthCheck bool,
 ) ([]operation.Operation, error) {
 	var ops []operation.Operation
@@ -260,7 +285,8 @@ func reconcileGlobalContainer(
 		ops = append(ops, &operation.RunContainerOperation{
 			ServiceID:         serviceID,
 			Spec:              spec,
-			MachineID:         machineID,
+			MachineID:         machine.Id,
+			MachineName:       machine.Name,
 			SkipHealthMonitor: skipHealthCheck,
 		})
 		return ops, nil
@@ -290,6 +316,7 @@ func reconcileGlobalContainer(
 				}
 				ops = append(ops, &operation.RemoveContainerOperation{
 					MachineID:       old.MachineID,
+					MachineName:     machine.Name,
 					Container:       old.Container,
 					StopGracePeriod: spec.StopGracePeriod,
 				})
@@ -325,7 +352,8 @@ func reconcileGlobalContainer(
 				ops = append(ops, &operation.StopContainerOperation{
 					ServiceID:       serviceID,
 					ContainerID:     c.Container.ID,
-					MachineID:       machineID,
+					MachineID:       machine.Id,
+					MachineName:     machine.Name,
 					StopGracePeriod: spec.StopGracePeriod,
 				})
 			}
@@ -336,7 +364,8 @@ func reconcileGlobalContainer(
 		ops = append(ops, &operation.ReplaceContainerOperation{
 			ServiceID:         serviceID,
 			Spec:              spec,
-			MachineID:         machineID,
+			MachineID:         machine.Id,
+			MachineName:       machine.Name,
 			OldContainer:      containerToReplace.Container,
 			Order:             order,
 			SkipHealthMonitor: skipHealthCheck,
@@ -350,6 +379,7 @@ func reconcileGlobalContainer(
 			}
 			ops = append(ops, &operation.RemoveContainerOperation{
 				MachineID:       c.MachineID,
+				MachineName:     machine.Name,
 				Container:       c.Container,
 				StopGracePeriod: spec.StopGracePeriod,
 			})
@@ -359,12 +389,14 @@ func reconcileGlobalContainer(
 		ops = append(ops, &operation.RunContainerOperation{
 			ServiceID:         serviceID,
 			Spec:              spec,
-			MachineID:         machineID,
+			MachineID:         machine.Id,
+			MachineName:       machine.Name,
 			SkipHealthMonitor: skipHealthCheck,
 		})
 		for _, c := range containers {
 			ops = append(ops, &operation.RemoveContainerOperation{
 				MachineID:       c.MachineID,
+				MachineName:     machine.Name,
 				Container:       c.Container,
 				StopGracePeriod: spec.StopGracePeriod,
 			})

pkg/client/deploy/strategy_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/psviderski/uncloud/internal/machine/api/pb"
 	"github.com/psviderski/uncloud/pkg/api"
 	"github.com/psviderski/uncloud/pkg/client/deploy/operation"
 	"github.com/stretchr/testify/assert"
@@ -315,8 +316,9 @@ func TestReconcileGlobalContainer(t *testing.T) {
 			},
 			expectedOps: []operation.Operation{
 				&operation.RunContainerOperation{
-					ServiceID: "service-1",
-					MachineID: "machine-1",
+					ServiceID:   "service-1",
+					MachineID:   "machine-1",
+					MachineName: "machine-1",
 				},
 			},
 		},
@@ -335,6 +337,7 @@ func TestReconcileGlobalContainer(t *testing.T) {
 				&operation.ReplaceContainerOperation{
 					ServiceID:    "service-1",
 					MachineID:    "machine-1",
+					MachineName:  "machine-1",
 					OldContainer: container1,
 					Order:        api.UpdateOrderStopFirst,
 				},
@@ -358,16 +361,19 @@ func TestReconcileGlobalContainer(t *testing.T) {
 					ServiceID:   "service-1",
 					ContainerID: "container-2",
 					MachineID:   "machine-1",
+					MachineName: "machine-1",
 				},
 				&operation.ReplaceContainerOperation{
 					ServiceID:    "service-1",
 					MachineID:    "machine-1",
+					MachineName:  "machine-1",
 					OldContainer: container1,
 					Order:        api.UpdateOrderStopFirst,
 				},
 				&operation.RemoveContainerOperation{
-					MachineID: "machine-1",
-					Container: container2WithPort9090,
+					MachineID:   "machine-1",
+					MachineName: "machine-1",
+					Container:   container2WithPort9090,
 				},
 			},
 		},
@@ -388,12 +394,14 @@ func TestReconcileGlobalContainer(t *testing.T) {
 				&operation.ReplaceContainerOperation{
 					ServiceID:    "service-1",
 					MachineID:    "machine-1",
+					MachineName:  "machine-1",
 					OldContainer: container1,
 					Order:        api.UpdateOrderStopFirst,
 				},
 				&operation.RemoveContainerOperation{
-					MachineID: "machine-1",
-					Container: container2WithPort3000,
+					MachineID:   "machine-1",
+					MachineName: "machine-1",
+					Container:   container2WithPort3000,
 				},
 			},
 		},
@@ -402,7 +410,12 @@ func TestReconcileGlobalContainer(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ops, err := reconcileGlobalContainer(
-				tt.containers, tt.spec, "service-1", "machine-1", tt.forceRecreate, false,
+				tt.containers,
+				tt.spec,
+				"service-1",
+				&pb.MachineInfo{Id: "machine-1", Name: "machine-1"},
+				tt.forceRecreate,
+				false,
 			)
 			assert.NoError(t, err)
 			assertOperationsEqual(t, tt.expectedOps, ops)