From 66289f9a22726027519a4c52a0431c995f728ec2 Mon Sep 17 00:00:00 2001 From: Oscar Zhou <100548325+oscarzhou-portainer@users.noreply.github.com> Date: Mon, 3 Aug 2026 09:26:16 +1200 Subject: [PATCH] fix(stacks): reconcile swarm stack status after recovery [BE-13214] (#3317) --- api/cmd/portainer/main.go | 6 ++ api/exec/swarm_stack.go | 32 ++++++++ api/exec/swarm_stack_test.go | 66 +++++++++++++++ api/portainer.go | 1 + api/stacks/deployments/deploy.go | 64 +++++++++++++++ api/stacks/deployments/deploy_test.go | 114 ++++++++++++++++++++++++++ 6 files changed, 283 insertions(+) create mode 100644 api/exec/swarm_stack_test.go diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go index 83162ec813..7e1331df0f 100644 --- a/api/cmd/portainer/main.go +++ b/api/cmd/portainer/main.go @@ -67,6 +67,8 @@ import ( "github.com/rs/zerolog/log" ) +const swarmStackStatusCheckInterval = time.Minute + func initCLI() *portainer.CLIFlags { cliService := cli.Service{} @@ -581,6 +583,10 @@ func buildServer(flags *portainer.CLIFlags, shutdownCtx context.Context, shutdow log.Fatal().Err(err).Msg("failed to start source scheduler") } + sched.StartJobEvery(swarmStackStatusCheckInterval, func() error { + return deployments.ReconcileSwarmStackStatus(shutdownCtx, dataStore, swarmStackManager) + }) + sslDBSettings, err := dataStore.SSLSettings().Settings() if err != nil { log.Fatal().Msg("failed to fetch SSL settings from DB") diff --git a/api/exec/swarm_stack.go b/api/exec/swarm_stack.go index a797ebeaff..d04b1d7b6f 100644 --- a/api/exec/swarm_stack.go +++ b/api/exec/swarm_stack.go @@ -15,6 +15,9 @@ import ( // postDeployFailureCheckTimeout bounds how long Deploy waits for tasks to start or fail after being accepted by Swarm. const postDeployFailureCheckTimeout = 30 * time.Second +// runningStatusProbeTimeout bounds how long IsCurrentlyRunning waits for a single status probe before giving up. +const runningStatusProbeTimeout = 10 * time.Second + // SwarmStackManager represents a service for managing stacks. type SwarmStackManager struct { deployer swarm.Deployer @@ -85,6 +88,35 @@ func (manager *SwarmStackManager) Deploy( return nil } +// CheckRunningStatus probes live Swarm state for up to runningStatusProbeTimeout and reports whether +// the stack is confirmed running. It returns false, not an error, if that can't be confirmed in time. +func (manager *SwarmStackManager) CheckRunningStatus(ctx context.Context, stack *portainer.Stack, endpoint *portainer.Endpoint) (bool, error) { + url, proxy, err := fetchEndpointProxy(manager.proxyManager, endpoint) + if err != nil { + return false, fmt.Errorf("failed to fetch environment proxy: %w", err) + } + + if proxy != nil { + defer proxy.Close() + } + + options := swarm.Options{ + ProjectName: stack.Name, + Host: url, + } + + waitCtx, cancel := context.WithTimeout(ctx, runningStatusProbeTimeout) + defer cancel() + + result := manager.deployer.WaitForStatus(waitCtx, stack.Name, options, libstack.StatusRunning) + if result.Status == libstack.StatusError { + return false, nil + } + + // ErrorMsg is only empty when Running was actually observed, not just assumed on timeout. + return result.ErrorMsg == "", nil +} + // Remove deletes all resources belonging to a Swarm stack. func (manager *SwarmStackManager) Remove( ctx context.Context, diff --git a/api/exec/swarm_stack_test.go b/api/exec/swarm_stack_test.go new file mode 100644 index 0000000000..edcd9f33b9 --- /dev/null +++ b/api/exec/swarm_stack_test.go @@ -0,0 +1,66 @@ +package exec + +import ( + "context" + "testing" + + portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/pkg/libstack" + "github.com/portainer/portainer/pkg/libstack/swarm" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type stubSwarmDeployer struct { + swarm.Deployer + + waitResult libstack.WaitResult +} + +func (s *stubSwarmDeployer) WaitForStatus(context.Context, string, swarm.Options, libstack.Status) libstack.WaitResult { + return s.waitResult +} + +func TestSwarmStackManager_CheckRunningStatus(t *testing.T) { + t.Parallel() + + stack := &portainer.Stack{Name: "my-stack"} + // unix:// URLs skip fetchEndpointProxy's proxy-manager path entirely, so a nil proxyManager is safe here. + endpoint := &portainer.Endpoint{URL: "unix:///var/run/docker.sock"} + + tests := []struct { + name string + waitResult libstack.WaitResult + expectedRunning bool + }{ + { + name: "reports running once Running is confirmed", + waitResult: libstack.WaitResult{Status: libstack.StatusRunning}, + expectedRunning: true, + }, + { + name: "reports not running when an explicit failure is observed", + waitResult: libstack.WaitResult{Status: libstack.StatusError, ErrorMsg: "no such image"}, + expectedRunning: false, + }, + { + name: "reports not running when the probe times out without confirming", + waitResult: libstack.WaitResult{Status: libstack.StatusRunning, ErrorMsg: "failed to wait for status: context deadline exceeded"}, + expectedRunning: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + deployer := &stubSwarmDeployer{waitResult: tt.waitResult} + manager := NewSwarmStackManager(deployer, nil) + + running, err := manager.CheckRunningStatus(t.Context(), stack, endpoint) + require.NoError(t, err) + assert.Equal(t, tt.expectedRunning, running) + }) + } +} diff --git a/api/portainer.go b/api/portainer.go index cf9996a101..8145e1ecf4 100644 --- a/api/portainer.go +++ b/api/portainer.go @@ -2086,6 +2086,7 @@ type ( Deploy(ctx context.Context, stack *Stack, prune bool, pullImage bool, endpoint *Endpoint, registries []Registry) error Remove(ctx context.Context, stack *Stack, endpoint *Endpoint) error NormalizeStackName(name string) string + CheckRunningStatus(ctx context.Context, stack *Stack, endpoint *Endpoint) (bool, error) } ) diff --git a/api/stacks/deployments/deploy.go b/api/stacks/deployments/deploy.go index 5f0d0d1890..7d0528e4b8 100644 --- a/api/stacks/deployments/deploy.go +++ b/api/stacks/deployments/deploy.go @@ -276,6 +276,70 @@ func redeployWhenChangedSecondStage( return nil } +type erroredSwarmStack struct { + stack portainer.Stack + endpoint *portainer.Endpoint +} + +// ReconcileSwarmStackStatus flips errored Swarm stacks back to active once their live services recover. +func ReconcileSwarmStackStatus(ctx context.Context, datastore dataservices.DataStore, swarmStackManager portainer.SwarmStackManager) error { + var erroredStacks []erroredSwarmStack + + if err := datastore.ViewTx(func(tx dataservices.DataStoreTx) error { + stacks, err := tx.Stack().ReadAll(func(stack portainer.Stack) bool { + return stack.Type == portainer.DockerSwarmStack && stack.Status == portainer.StackStatusError + }) + if err != nil { + return err + } + + for _, stack := range stacks { + endpoint, err := tx.Endpoint().Endpoint(stack.EndpointID) + if err != nil { + log.Warn().Err(err).Int("stack_id", int(stack.ID)).Msg("Failed to find the environment for stack status check") + continue + } + + erroredStacks = append(erroredStacks, erroredSwarmStack{stack: stack, endpoint: endpoint}) + } + + return nil + }); err != nil { + return errors.WithMessage(err, "failed to list errored swarm stacks") + } + + for _, es := range erroredStacks { + running, err := swarmStackManager.CheckRunningStatus(ctx, &es.stack, es.endpoint) + if err != nil { + log.Warn().Err(err).Int("stack_id", int(es.stack.ID)).Msg("Failed to check swarm stack status") + continue + } + + if !running { + continue + } + + if err := datastore.UpdateTx(func(tx dataservices.DataStoreTx) error { + current, err := tx.Stack().Read(es.stack.ID) + if err != nil { + return err + } + + if current.Status != portainer.StackStatusError { + return nil + } + + stackutils.UpdateStackStatusFromDeploymentResult(current, nil) + + return tx.Stack().Update(current.ID, current) + }); err != nil { + log.Error().Err(err).Int("stack_id", int(es.stack.ID)).Msg("Failed to update stack status after recovery") + } + } + + return nil +} + func getUserRegistries(datastore dataservices.DataStore, user *portainer.User, endpointID portainer.EndpointID) ([]portainer.Registry, error) { registries, err := datastore.Registry().ReadAll() if err != nil { diff --git a/api/stacks/deployments/deploy_test.go b/api/stacks/deployments/deploy_test.go index 6a0851c330..fd174fac8c 100644 --- a/api/stacks/deployments/deploy_test.go +++ b/api/stacks/deployments/deploy_test.go @@ -388,6 +388,120 @@ func Test_redeployWhenChanged_KubernetesStack_DeployFailure_DoesNotAdvanceArtifa assert.Empty(t, workflows[0].Artifacts[0].Files[0].Hash, "artifact hash should stay at its old value (empty, in this fixture) since the deploy failed - the UI's git banner reads this field") } +type stubSwarmStackManager struct { + portainer.SwarmStackManager + + running map[portainer.StackID]bool + errs map[portainer.StackID]error +} + +func (f *stubSwarmStackManager) CheckRunningStatus(_ context.Context, stack *portainer.Stack, _ *portainer.Endpoint) (bool, error) { + if err, ok := f.errs[stack.ID]; ok { + return false, err + } + + return f.running[stack.ID], nil +} + +func Test_ReconcileSwarmStackStatus(t *testing.T) { + t.Parallel() + + newEndpoint := func(t *testing.T, store dataservices.DataStore, id portainer.EndpointID) { + t.Helper() + err := store.UpdateTx(func(tx dataservices.DataStoreTx) error { + return tx.Endpoint().Create(&portainer.Endpoint{ID: id}) + }) + require.NoError(t, err, "error creating environment") + } + + newSwarmStack := func(t *testing.T, store dataservices.DataStore, id portainer.StackID, endpointID portainer.EndpointID, stackType portainer.StackType, status portainer.StackStatus) *portainer.Stack { + t.Helper() + stack := &portainer.Stack{ + ID: id, + EndpointID: endpointID, + Type: stackType, + Status: status, + } + err := store.UpdateTx(func(tx dataservices.DataStoreTx) error { + return tx.Stack().Create(stack) + }) + require.NoError(t, err, "error creating stack") + + return stack + } + + t.Run("flips a recovered errored swarm stack back to active", func(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, false, true) + newEndpoint(t, store, 1) + stack := newSwarmStack(t, store, 1, 1, portainer.DockerSwarmStack, portainer.StackStatusError) + + manager := &stubSwarmStackManager{running: map[portainer.StackID]bool{1: true}} + + err := ReconcileSwarmStackStatus(t.Context(), store, manager) + require.NoError(t, err) + + result, err := store.Stack().Read(stack.ID) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusActive, result.Status) + require.NotEmpty(t, result.DeploymentStatus) + assert.Equal(t, portainer.StackStatusActive, result.DeploymentStatus[len(result.DeploymentStatus)-1].Status) + }) + + t.Run("leaves a still-failing swarm stack in the error state", func(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, false, true) + newEndpoint(t, store, 1) + stack := newSwarmStack(t, store, 1, 1, portainer.DockerSwarmStack, portainer.StackStatusError) + + manager := &stubSwarmStackManager{running: map[portainer.StackID]bool{1: false}} + + err := ReconcileSwarmStackStatus(t.Context(), store, manager) + require.NoError(t, err) + + result, err := store.Stack().Read(stack.ID) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusError, result.Status) + }) + + t.Run("continues past a stack whose live check errors", func(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, false, true) + newEndpoint(t, store, 1) + stack := newSwarmStack(t, store, 1, 1, portainer.DockerSwarmStack, portainer.StackStatusError) + + manager := &stubSwarmStackManager{errs: map[portainer.StackID]error{1: errors.New("agent unreachable")}} + + err := ReconcileSwarmStackStatus(t.Context(), store, manager) + require.NoError(t, err, "a single stack's check failing should not fail the whole reconcile pass") + + result, err := store.Stack().Read(stack.ID) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusError, result.Status, "status should be left untouched when the check itself failed") + }) + + t.Run("ignores stacks that aren't swarm stacks or aren't in the error state", func(t *testing.T) { + t.Parallel() + _, store := datastore.MustNewTestStore(t, false, true) + newEndpoint(t, store, 1) + composeStack := newSwarmStack(t, store, 1, 1, portainer.DockerComposeStack, portainer.StackStatusError) + activeSwarmStack := newSwarmStack(t, store, 2, 1, portainer.DockerSwarmStack, portainer.StackStatusActive) + + manager := &stubSwarmStackManager{running: map[portainer.StackID]bool{1: true, 2: true}} + + err := ReconcileSwarmStackStatus(t.Context(), store, manager) + require.NoError(t, err) + + result, err := store.Stack().Read(composeStack.ID) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusError, result.Status, "non-swarm stacks must not be touched") + + result, err = store.Stack().Read(activeSwarmStack.ID) + require.NoError(t, err) + assert.Equal(t, portainer.StackStatusActive, result.Status, "stacks that aren't in error must not be touched") + }) +} + func Test_getUserRegistries(t *testing.T) { t.Parallel() _, store := datastore.MustNewTestStore(t, false, true)