From ba224bb5bedd22b1c98ed8ffbbd253aa82e1b2cb Mon Sep 17 00:00:00 2001 From: Nick Date: Mon, 12 Oct 2026 13:02:54 +0700 Subject: [PATCH] fix(deploy): prepare replacement before stop-first cutover (#420) --- pkg/client/deploy/operation/container.go | 16 +- pkg/client/deploy/operation/container_test.go | 155 ++++++++++++++++++ 2 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 pkg/client/deploy/operation/container_test.go diff --git a/pkg/client/deploy/operation/container.go b/pkg/client/deploy/operation/container.go index de651eff..2c177c07 100644 --- a/pkg/client/deploy/operation/container.go +++ b/pkg/client/deploy/operation/container.go @@ -173,6 +173,16 @@ type ReplaceContainerOperation struct { func (o *ReplaceContainerOperation) Execute(ctx context.Context, cli Client) error { stopFirst := o.Order == api.UpdateOrderStopFirst + // Create the new container before stopping the old one. CreateContainer also pulls the image according to the + // service pull policy, so the stop-first downtime only includes stopping the old container and starting and + // monitoring the new one. + resp, err := cli.CreateContainer(ctx, o.ServiceID, o.Spec, o.MachineID) + if err != nil { + return fmt.Errorf("create new container: %w", err) + } + // Override event ID so StartContainer and WaitContainerHealthy update the same progress line as creation. + newCtx := cliprogress.WithEventID(ctx, cliprogress.NewContainerEventID(ctx, resp.Name, o.MachineName)) + wasRunning := false if stopFirst { // Inspect the old container to remember its running state before stopping. @@ -189,12 +199,6 @@ func (o *ReplaceContainerOperation) Execute(ctx context.Context, cli Client) err } } - resp, err := cli.CreateContainer(ctx, o.ServiceID, o.Spec, o.MachineID) - if err != nil { - return fmt.Errorf("create new container: %w", err) - } - // Override event ID so StartContainer and WaitContainerHealthy update the same progress line as creation. - newCtx := cliprogress.WithEventID(ctx, cliprogress.NewContainerEventID(ctx, resp.Name, o.MachineName)) if err = cli.StartContainer(newCtx, o.ServiceID, resp.ID); err != nil { return fmt.Errorf("start new container: %w", err) } diff --git a/pkg/client/deploy/operation/container_test.go b/pkg/client/deploy/operation/container_test.go new file mode 100644 index 00000000..892d86e6 --- /dev/null +++ b/pkg/client/deploy/operation/container_test.go @@ -0,0 +1,155 @@ +package operation + +import ( + "context" + "errors" + "testing" + + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/volume" + "github.com/psviderski/uncloud/pkg/api" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReplaceContainerOperation_StopFirstPreparation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + createErr error + wantErr string + wantCalls []string + }{ + { + name: "prepare before cutover", + wantCalls: []string{ + "create:service", + "inspect:old", + "stop:old", + "start:new", + "wait:new", + "remove:old", + }, + }, + { + name: "create failure does not interrupt old container", + createErr: errors.New("pull failed"), + wantErr: "create new container: pull failed", + wantCalls: []string{"create:service"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cli := &recordingClient{ + oldContainerRunning: true, + createErr: tt.createErr, + } + op := newStopFirstReplaceContainerOperation() + + err := op.Execute(context.Background(), cli) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, tt.wantCalls, cli.calls) + }) + } +} + +func newStopFirstReplaceContainerOperation() *ReplaceContainerOperation { + return &ReplaceContainerOperation{ + ServiceID: "service-id", + Spec: api.ServiceSpec{Name: "service"}, + MachineID: "machine-id", + MachineName: "machine", + OldContainer: api.ServiceContainer{Container: api.Container{InspectResponse: container.InspectResponse{ContainerJSONBase: &container.ContainerJSONBase{ID: "old"}}}}, + Order: api.UpdateOrderStopFirst, + } +} + +type recordingClient struct { + calls []string + oldContainerRunning bool + createErr error +} + +func (c *recordingClient) CreateContainer( + _ context.Context, _ string, spec api.ServiceSpec, _ string, +) (api.CreateContainerResponse, error) { + c.calls = append(c.calls, "create:"+spec.Name) + return api.CreateContainerResponse{ + CreateResponse: container.CreateResponse{ID: "new"}, + Name: "new", + }, c.createErr +} + +func (c *recordingClient) InspectContainer( + _ context.Context, _, containerID string, +) (api.MachineServiceContainer, error) { + c.calls = append(c.calls, "inspect:"+containerID) + return api.MachineServiceContainer{ + Container: api.ServiceContainer{ + Container: api.Container{InspectResponse: container.InspectResponse{ + ContainerJSONBase: &container.ContainerJSONBase{ + ID: containerID, + State: &container.State{Running: c.oldContainerRunning}, + }, + }}, + }, + }, nil +} + +func (c *recordingClient) StartContainer(_ context.Context, _, containerID string) error { + c.calls = append(c.calls, "start:"+containerID) + return nil +} + +func (c *recordingClient) StopContainer( + _ context.Context, _, containerID string, _ container.StopOptions, +) error { + c.calls = append(c.calls, "stop:"+containerID) + return nil +} + +func (c *recordingClient) RemoveContainer( + _ context.Context, _, containerID string, _ container.RemoveOptions, +) error { + c.calls = append(c.calls, "remove:"+containerID) + return nil +} + +func (c *recordingClient) CreatePreDeployHookContainer( + context.Context, string, api.ServiceSpec, string, +) (api.CreateContainerResponse, error) { + panic("unexpected CreatePreDeployHookContainer call") +} + +func (c *recordingClient) ExecContainer(context.Context, string, string, api.ExecOptions) (int, error) { + panic("unexpected ExecContainer call") +} + +func (c *recordingClient) WaitContainerHealthy( + _ context.Context, _, containerID string, _ api.WaitContainerHealthyOptions, +) error { + c.calls = append(c.calls, "wait:"+containerID) + return nil +} + +func (c *recordingClient) CreateVolume( + context.Context, string, volume.CreateOptions, +) (api.MachineVolume, error) { + panic("unexpected CreateVolume call") +} + +func (c *recordingClient) ListVolumes(context.Context, *api.VolumeFilter) ([]api.MachineVolume, error) { + panic("unexpected ListVolumes call") +} + +func (c *recordingClient) RemoveVolume(context.Context, string, string, bool) error { + panic("unexpected RemoveVolume call") +}