From 6c7ddeb82fc8729d0b764803557dfd359ce69a19 Mon Sep 17 00:00:00 2001 From: Pasha Sviderski Date: Tue, 3 Nov 2026 12:54:57 +1000 Subject: [PATCH] fix(plan): make stop operation formatting consistent and do not count it as removal in summary --- pkg/client/compose/plan.go | 2 +- pkg/client/compose/plan_test.go | 17 +++++++++++++++++ pkg/client/deploy/deploy.go | 2 +- pkg/client/deploy/deploy_test.go | 10 ++++++++++ pkg/client/deploy/strategy.go | 1 + pkg/client/deploy/strategy_test.go | 1 + pkg/client/operation/container.go | 7 ++++--- pkg/client/operation/container_test.go | 13 +++++++++++++ 8 files changed, 48 insertions(+), 5 deletions(-) diff --git a/pkg/client/compose/plan.go b/pkg/client/compose/plan.go index 5e8cbd44..e2c1d622 100644 --- a/pkg/client/compose/plan.go +++ b/pkg/client/compose/plan.go @@ -83,8 +83,8 @@ func (p *Plan) formatSummary() string { machines[o.MachineID] = struct{}{} removeCount++ case *operation.StopContainerOperation: + // Stop operations aren't counted because a later operation in the plan removes the stopped container. machines[o.MachineID] = struct{}{} - removeCount++ } } } diff --git a/pkg/client/compose/plan_test.go b/pkg/client/compose/plan_test.go index 1bfc58ea..133f4796 100644 --- a/pkg/client/compose/plan_test.go +++ b/pkg/client/compose/plan_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/charmbracelet/x/ansi" + "github.com/psviderski/uncloud/pkg/api" "github.com/psviderski/uncloud/pkg/client/deploy" "github.com/psviderski/uncloud/pkg/client/operation" "github.com/stretchr/testify/assert" @@ -28,3 +29,19 @@ func TestPlan_FormatSummary(t *testing.T) { assert.Equal(t, "1 create · 2 update (in place) · 1 remove · across 3 machines", ansi.Strip(plan.formatSummary())) } + +func TestPlan_FormatSummary_StopOperationsNotCounted(t *testing.T) { + t.Parallel() + + svc := &deploy.ServicePlan{} + svc.Operations = []operation.Operation{ + &operation.StopContainerOperation{MachineID: "m-1"}, + &operation.StopContainerOperation{MachineID: "m-1"}, + &operation.ReplaceContainerOperation{MachineID: "m-1", Order: api.UpdateOrderStopFirst}, + &operation.RemoveContainerOperation{MachineID: "m-1"}, + } + plan := &Plan{Services: []*deploy.ServicePlan{svc}} + + // Stop operations aren't counted because a later operation in the plan removes the stopped container. + assert.Equal(t, "1 replace (stop-first) · 1 remove · across 1 machine", ansi.Strip(plan.formatSummary())) +} diff --git a/pkg/client/deploy/deploy.go b/pkg/client/deploy/deploy.go index 30cf2bd2..495787cf 100644 --- a/pkg/client/deploy/deploy.go +++ b/pkg/client/deploy/deploy.go @@ -195,8 +195,8 @@ func (sp *ServicePlan) FormatSummary() string { machines[o.MachineID] = struct{}{} removeCount++ case *operation.StopContainerOperation: + // Stop operations aren't counted because a later operation in the plan removes the stopped container. machines[o.MachineID] = struct{}{} - removeCount++ } } diff --git a/pkg/client/deploy/deploy_test.go b/pkg/client/deploy/deploy_test.go index cb8d8a0c..5bf93d69 100644 --- a/pkg/client/deploy/deploy_test.go +++ b/pkg/client/deploy/deploy_test.go @@ -25,6 +25,16 @@ func TestServicePlan_FormatSummary(t *testing.T) { }, expected: "2 update (in place) · across 2 machines", }, + { + name: "stop operations not counted as they are followed by remove", + ops: []operation.Operation{ + &operation.StopContainerOperation{MachineID: "m-1"}, + &operation.StopContainerOperation{MachineID: "m-1"}, + &operation.ReplaceContainerOperation{MachineID: "m-1", Order: api.UpdateOrderStopFirst}, + &operation.RemoveContainerOperation{MachineID: "m-1"}, + }, + expected: "1 replace (stop-first) · 1 remove · across 1 machine", + }, { name: "all operation types", ops: []operation.Operation{ diff --git a/pkg/client/deploy/strategy.go b/pkg/client/deploy/strategy.go index 063f17de..50324933 100644 --- a/pkg/client/deploy/strategy.go +++ b/pkg/client/deploy/strategy.go @@ -386,6 +386,7 @@ func reconcileGlobalContainer( if err != nil || len(conflictingPorts) > 0 { ops = append(ops, &operation.StopContainerOperation{ ServiceID: serviceID, + ServiceName: c.Container.ServiceSpec.Name, ContainerID: c.Container.ID, MachineID: machine.Id, MachineName: machine.Name, diff --git a/pkg/client/deploy/strategy_test.go b/pkg/client/deploy/strategy_test.go index b8279295..f69c066a 100644 --- a/pkg/client/deploy/strategy_test.go +++ b/pkg/client/deploy/strategy_test.go @@ -363,6 +363,7 @@ func TestReconcileGlobalContainer(t *testing.T) { expectedOps: []operation.Operation{ &operation.StopContainerOperation{ ServiceID: "service-1", + ServiceName: container2WithPort9090.ServiceSpec.Name, ContainerID: "container-2", MachineID: "machine-1", MachineName: "machine-1", diff --git a/pkg/client/operation/container.go b/pkg/client/operation/container.go index 2278231f..e2f7dda3 100644 --- a/pkg/client/operation/container.go +++ b/pkg/client/operation/container.go @@ -82,7 +82,9 @@ func (o *RunContainerOperation) String() string { // StopContainerOperation stops a container on a specific machine. type StopContainerOperation struct { - ServiceID string + ServiceID string + // ServiceName is used for formatting the operation as part of the deployment plan. + ServiceName string ContainerID string MachineID string // MachineName is used for formatting the operation as part of the deployment plan. @@ -98,8 +100,7 @@ func (o *StopContainerOperation) Execute(ctx context.Context, cli Client) error } func (o *StopContainerOperation) Format() string { - // TODO: pass service name to format the display name consistently with other operations. - displayName := stringid.TruncateID(o.ContainerID) + displayName := o.ServiceName + tui.Faint.Render("/") + stringid.TruncateID(o.ContainerID) return tui.BoldRed.Render("-") + " " + tui.Faint.Render("stop container") + " " + diff --git a/pkg/client/operation/container_test.go b/pkg/client/operation/container_test.go index 7da49d74..3181c66c 100644 --- a/pkg/client/operation/container_test.go +++ b/pkg/client/operation/container_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/charmbracelet/x/ansi" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/volume" "github.com/psviderski/uncloud/pkg/api" @@ -12,6 +13,18 @@ import ( "github.com/stretchr/testify/require" ) +func TestStopContainerOperation_Format(t *testing.T) { + t.Parallel() + + op := &StopContainerOperation{ + ServiceName: "app", + ContainerID: "0f6df6593e1b0123456789abcdef", + MachineName: "machine-1", + } + + assert.Equal(t, "- stop container app/0f6df6593e1b on machine-1", ansi.Strip(op.Format())) +} + func TestReplaceContainerOperation_StopFirstPreparation(t *testing.T) { t.Parallel()