From 922f506fe5eeb166fb0744a5ca180118cb95f8bf Mon Sep 17 00:00:00 2001 From: claude code agent Date: Mon, 29 Jun 2026 14:29:57 +0300 Subject: [PATCH] =?UTF-8?q?feat(automation):=20guard=20update=E2=86=92roll?= =?UTF-8?q?back=20loop;=20name=20Settings=20types;=20tests=20&=20doc=20fix?= =?UTF-8?q?es=20(F1-F7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: record rolled-back targets per service (endpointID/containerName + remote digest) and skip auto-update during a 24h cooldown unless the remote digest changes — breaks the infinite update→rollback loop on a persistently unhealthy image, without blocking a genuinely new image. F2: unit-test applyContainerUpdate dispatch/payload mapping. F3: settings_update.go comments mention auto-heal AND auto-update. F4: drop stale '(future M4)' TS docs; primitives are frontend-only. F5: replace the anonymous ContainerAutomation settings struct with named types (identical JSON tags). F6: drop parseEnable (duplicate of boolLabel). F7: remove the unused gitService dependency. Co-Authored-By: Claude Opus 4.8 (1M context) --- api/cmd/portainer/main.go | 2 +- api/containerautomation/autoupdate.go | 119 ++++++++++++- api/containerautomation/labels.go | 28 +-- api/containerautomation/rollback.go | 50 +++++- api/containerautomation/rollback_test.go | 51 ++++++ api/containerautomation/service.go | 58 +++++- api/http/handler/settings/settings_update.go | 7 +- api/portainer.go | 46 +++-- .../update/applyContainerUpdate.test.ts | 165 ++++++++++++++++++ .../containers/update/applyContainerUpdate.ts | 9 +- .../update/resolveContainerUpdatePath.ts | 3 +- app/react/docker/containers/update/types.ts | 4 +- 12 files changed, 482 insertions(+), 60 deletions(-) create mode 100644 app/react/docker/containers/update/applyContainerUpdate.test.ts diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go index be5201dd0..91c47c300 100644 --- a/api/cmd/portainer/main.go +++ b/api/cmd/portainer/main.go @@ -579,7 +579,7 @@ func buildServer(flags *portainer.CLIFlags, shutdownCtx context.Context, shutdow } containerService := docker.NewContainerService(dockerClientFactory, dataStore) - containerAutomationService := containerautomation.NewService(shutdownCtx, scheduler, dataStore, dockerClientFactory, containerService, stackDeployer, gitService) + containerAutomationService := containerautomation.NewService(shutdownCtx, scheduler, dataStore, dockerClientFactory, containerService, stackDeployer) containerAutomationService.Start() sslDBSettings, err := dataStore.SSLSettings().Settings() diff --git a/api/containerautomation/autoupdate.go b/api/containerautomation/autoupdate.go index 530f7a92b..cbbd6baba 100644 --- a/api/containerautomation/autoupdate.go +++ b/api/containerautomation/autoupdate.go @@ -2,6 +2,8 @@ package containerautomation import ( "context" + "fmt" + "strings" "time" portainer "github.com/portainer/portainer/api" @@ -82,6 +84,10 @@ func (s *Service) update() error { s.updateEndpoint(endpoint, scope, opts) } + // Drop rolled-back records whose cooldown has fully elapsed (mirrors auto-heal's + // pruneRetries), so the loop-guard map cannot grow unbounded. + s.pruneRolledBack(time.Now()) + return nil } @@ -171,7 +177,7 @@ func (s *Service) updateEndpoint(endpoint *portainer.Endpoint, scope string, opt continue } - candidates = append(candidates, UpdateCandidate{ID: c.ID, ImageID: c.ImageID, Labels: c.Labels}) + candidates = append(candidates, UpdateCandidate{ID: c.ID, Name: containerName(c.Names), ImageID: c.ImageID, Labels: c.Labels}) } // Route and de-duplicate: one redeploy per stack per tick. @@ -231,6 +237,17 @@ func (s *Service) stackLookupForEndpoint(endpointID portainer.EndpointID) func(p func (s *Service) updateStandalone(cli *dockerclient.Client, endpoint *portainer.Endpoint, c UpdateCandidate, opts updateOptions) { endpointID := int(endpoint.ID) + // Update->rollback loop guard: if this container's update was rolled back + // recently and the remote still points at the SAME failed image, skip it until + // the cooldown elapses. A genuinely new upstream image (a changed remote digest) + // is not blocked. + rollbackMapKey := rollbackKey(endpoint.ID, c.Name) + if rec, ok := s.getRolledBack(rollbackMapKey); ok && s.shouldSkipRolledBack(rollbackMapKey, rec) { + log.Info().Str("container_id", c.ID).Str("container", c.Name).Str("image", rec.ref).Int("endpoint_id", endpointID). + Msg("auto-update: skipping update, a recent rollback failed on this image and the remote is unchanged (cooldown)") + return + } + // Capture the pre-update image identity for a possible rollback. The container // list gives us the old image id; an inspect adds the original reference (re-tag // target), whether a usable healthcheck exists, and the healthcheck start_period @@ -307,7 +324,7 @@ func (s *Service) updateStandalone(cli *dockerclient.Client, endpoint *portainer // back and do not emit an event (we never observed a real failure). return case gateRollback: - s.rollback(cli, endpoint, newContainer.ID, oldImageID, originalRef) + s.rollback(cli, endpoint, newContainer.ID, oldImageID, originalRef, c.Name) return case gateHealthy: // Confirmed healthy: fall through to emit "updated" and clean up. @@ -326,6 +343,104 @@ func (s *Service) updateStandalone(cli *dockerclient.Client, endpoint *portainer } } +// containerName returns a container's primary name without the leading slash, or +// "" when none is reported. The name is stable across a recreate (Recreate +// assigns a new container ID but preserves the name), so it keys the rolled-back +// loop-guard map. +func containerName(names []string) string { + if len(names) == 0 { + return "" + } + + return strings.TrimPrefix(names[0], "/") +} + +// rollbackKey identifies a standalone container in the rolled-back map by its +// endpoint and (recreate-stable) name. A recreate assigns a new container ID, so +// the ID cannot key state across an update; the name is preserved. +func rollbackKey(endpointID portainer.EndpointID, name string) string { + return fmt.Sprintf("%d/%s", int(endpointID), name) +} + +// resolveRemoteDigest fetches the current remote image digest for a reference. It +// tells whether a rolled-back container's upstream target is still the same +// failed image (skip) or a new push (retry). +func (s *Service) resolveRemoteDigest(ctx context.Context, ref string) (string, error) { + img, err := images.ParseImage(images.ParseImageOptions{Name: ref}) + if err != nil { + return "", err + } + + dig, err := s.digestClient.RemoteDigest(ctx, img) + if err != nil { + return "", err + } + + return dig.String(), nil +} + +// recordRolledBack stores the failed target after a successful rollback so the +// next poll skips re-pulling the same broken image. The failed remote digest is +// resolved now (the registry is reachable, the image was just pulled); if it +// cannot be resolved the record is still stored with an empty digest and the +// guard skips conservatively until the cooldown elapses. +func (s *Service) recordRolledBack(endpoint *portainer.Endpoint, name, ref string) { + if name == "" { + // Without a stable key we cannot reliably match the container next tick. + log.Debug().Str("image", ref).Int("endpoint_id", int(endpoint.ID)). + Msg("auto-update: rolled-back container has no name, loop guard not recorded") + return + } + + ctx, cancel := context.WithTimeout(s.baseCtx, statusCheckTimeout) + digest, err := s.resolveRemoteDigest(ctx, ref) + cancel() + if err != nil { + log.Debug().Err(err).Str("image", ref).Int("endpoint_id", int(endpoint.ID)). + Msg("auto-update: could not resolve failed remote digest, loop guard will skip conservatively until cooldown") + } + + s.setRolledBack(rollbackKey(endpoint.ID, name), rolledBackTarget{ref: ref, digest: digest, at: time.Now()}) +} + +// shouldSkipRolledBack reports whether a standalone container must be skipped this +// tick to avoid the update->rollback loop, clearing the record once the skip no +// longer applies (cooldown elapsed or a new upstream image). It resolves the +// current remote digest so a genuinely new image is never blocked. +func (s *Service) shouldSkipRolledBack(key string, rec rolledBackTarget) bool { + now := time.Now() + + // Fast paths that avoid a registry call: cooldown elapsed -> clear & proceed; + // no recorded digest -> skip conservatively while the cooldown is open. + if now.Sub(rec.at) >= updateRollbackCooldown { + s.clearRolledBack(key) + return false + } + if rec.digest == "" { + return true + } + + ctx, cancel := context.WithTimeout(s.baseCtx, statusCheckTimeout) + currentDigest, err := s.resolveRemoteDigest(ctx, rec.ref) + cancel() + if err != nil { + // Cannot confirm the upstream target changed: stay conservative and skip to + // avoid re-entering the loop, until the cooldown elapses. + log.Debug().Err(err).Str("image", rec.ref). + Msg("auto-update: cannot resolve remote digest for a rolled-back container, skipping until cooldown") + return true + } + + if decideUpdateSkip(rec, currentDigest, now, updateRollbackCooldown) { + return true + } + + // New upstream image (changed digest): the failed target is gone, clear the + // record and let the update proceed. + s.clearRolledBack(key) + return false +} + // cleanupOldImage attempts a conservative removal of the previous image after a // standalone update. The removal is NOT forced: Docker refuses to delete an // image that still carries tags or is referenced by any container, so this only diff --git a/api/containerautomation/labels.go b/api/containerautomation/labels.go index fcd7fbc2d..dff4544bf 100644 --- a/api/containerautomation/labels.go +++ b/api/containerautomation/labels.go @@ -28,27 +28,6 @@ const ( defaultRetries = 3 ) -// parseEnable resolves the enable label (primary first, alias second). -// It returns the parsed boolean value and whether the label was present at all. -// Invalid values are treated as false but still count as "present". -func parseEnable(labels map[string]string) (enabled bool, present bool) { - raw, ok := labels[labelEnable] - if !ok { - raw, ok = labels[labelEnableAlias] - } - - if !ok { - return false, false - } - - value, err := strconv.ParseBool(raw) - if err != nil { - return false, true - } - - return value, true -} - // InScope reports whether a container is subject to auto-heal given the global // scope and the container's labels. // @@ -56,7 +35,7 @@ func parseEnable(labels map[string]string) (enabled bool, present bool) { // enable label set to false. // - "labeled" (default): only containers with the enable label set to true. func InScope(scope string, labels map[string]string) bool { - enabled, present := parseEnable(labels) + enabled, present := boolLabel(labels, labelEnable, labelEnableAlias) switch scope { case ScopeAll: @@ -179,7 +158,10 @@ func resolveContainerUpdateRouting(labels map[string]string, stackLookup func(pr // UpdateCandidate is an outdated, in-scope container considered for auto-update. type UpdateCandidate struct { - ID string + ID string + // Name is the container's primary name (no leading slash). It is stable across + // a recreate and keys the update->rollback loop guard. + Name string ImageID string Labels map[string]string } diff --git a/api/containerautomation/rollback.go b/api/containerautomation/rollback.go index 97d37ed87..0c6c874f8 100644 --- a/api/containerautomation/rollback.go +++ b/api/containerautomation/rollback.go @@ -36,8 +36,51 @@ const ( // Docker API blip must not trigger a false rollback, so the gate keeps polling // and only gives up once the failures are clearly not transient. maxConsecutiveInspectErrors = 3 + // updateRollbackCooldown is how long a standalone container whose update was + // rolled back is skipped from updating to the SAME failed image again. It + // breaks the update->rollback loop: without it a persistently-unhealthy new + // image would be re-pulled and rolled back on every poll tick. A genuinely new + // upstream image (a changed remote digest) is not blocked; the cooldown only + // suppresses the exact target that just failed. It is generous because a broken + // upstream image is normally fixed by a new push, which lifts the skip at once. + updateRollbackCooldown = 24 * time.Hour ) +// rolledBackTarget records that a standalone container's update to a specific +// remote image was rolled back, so the same target is skipped until the cooldown +// elapses or the upstream digest changes. +type rolledBackTarget struct { + // ref is the container's original image reference (the re-tag target), used to + // re-resolve the current remote digest on later ticks. + ref string + // digest is the remote image digest that failed the health gate. A later tick + // resolving a DIFFERENT digest (a new upstream push) is allowed through; the + // same digest is skipped until the cooldown elapses. Empty when it could not be + // resolved at rollback time, in which case the guard skips conservatively. + digest string + // at is when the rollback happened; the cooldown is measured from it. + at time.Time +} + +// decideUpdateSkip is the pure core of the update->rollback loop guard: given a +// recorded rolled-back target and the freshly-resolved current remote digest, it +// reports whether the standalone update must be skipped this tick. The skip holds +// only while the cooldown is open AND the remote still points at the same failed +// image; once the cooldown elapses the skip is lifted. An unknown recorded digest +// is skipped conservatively (we cannot prove the target changed). Mirrors the +// decideRestart pattern so it is unit-testable without Docker. +func decideUpdateSkip(rec rolledBackTarget, currentDigest string, now time.Time, cooldown time.Duration) bool { + if now.Sub(rec.at) >= cooldown { + return false + } + + if rec.digest == "" { + return true + } + + return currentDigest == rec.digest +} + // rollbackOutcome is the decision produced from a single health sample. type rollbackOutcome int @@ -296,7 +339,7 @@ func (s *Service) gateDeadlineResult() gateResult { // If any step fails the previous image cannot be safely restored, so the // (unhealthy) new container is left running rather than destroyed, and a loud // failure notification is emitted. -func (s *Service) rollback(cli *dockerclient.Client, endpoint *portainer.Endpoint, newContainerID, oldImageID, originalRef string) { +func (s *Service) rollback(cli *dockerclient.Client, endpoint *portainer.Endpoint, newContainerID, oldImageID, originalRef, containerName string) { endpointID := int(endpoint.ID) log.Warn().Str("container_id", newContainerID).Str("image", originalRef).Int("endpoint_id", endpointID). @@ -336,4 +379,9 @@ func (s *Service) rollback(cli *dockerclient.Client, endpoint *portainer.Endpoin Kind: EventRollback, EndpointID: endpointID, ContainerID: newContainerID, Image: originalRef, Message: "rolled back to previous image after failed health check", }) + + // Record the failed target so the next poll does not immediately re-pull the + // same broken image and roll back again (the update->rollback loop). Recorded + // only after a SUCCESSFUL rollback; a changed remote digest later lifts the skip. + s.recordRolledBack(endpoint, containerName, originalRef) } diff --git a/api/containerautomation/rollback_test.go b/api/containerautomation/rollback_test.go index 9a070ce56..d27256471 100644 --- a/api/containerautomation/rollback_test.go +++ b/api/containerautomation/rollback_test.go @@ -226,3 +226,54 @@ func TestParseRollbackTimeout(t *testing.T) { }) } } + +func TestDecideUpdateSkip(t *testing.T) { + now := time.Date(2026, 6, 28, 12, 0, 0, 0, time.UTC) + cooldown := 24 * time.Hour + + tests := []struct { + name string + rec rolledBackTarget + currentDigest string + want bool + }{ + { + name: "same digest within cooldown is skipped", + rec: rolledBackTarget{digest: "sha256:aaa", at: now.Add(-1 * time.Hour)}, + currentDigest: "sha256:aaa", + want: true, + }, + { + name: "new digest within cooldown is not skipped", + rec: rolledBackTarget{digest: "sha256:aaa", at: now.Add(-1 * time.Hour)}, + currentDigest: "sha256:bbb", + want: false, + }, + { + name: "same digest after cooldown is not skipped", + rec: rolledBackTarget{digest: "sha256:aaa", at: now.Add(-25 * time.Hour)}, + currentDigest: "sha256:aaa", + want: false, + }, + { + name: "unknown recorded digest is skipped conservatively within cooldown", + rec: rolledBackTarget{digest: "", at: now.Add(-1 * time.Hour)}, + currentDigest: "sha256:aaa", + want: true, + }, + { + name: "unknown recorded digest after cooldown is not skipped", + rec: rolledBackTarget{digest: "", at: now.Add(-25 * time.Hour)}, + currentDigest: "sha256:aaa", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := decideUpdateSkip(tt.rec, tt.currentDigest, now, cooldown); got != tt.want { + t.Errorf("decideUpdateSkip() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/api/containerautomation/service.go b/api/containerautomation/service.go index 09907029d..a7012ff92 100644 --- a/api/containerautomation/service.go +++ b/api/containerautomation/service.go @@ -47,7 +47,6 @@ type Service struct { digestClient *images.DigestClient containerService *docker.ContainerService stackDeployer deployments.StackDeployer - gitService portainer.GitService // notifier receives automation events (update/rollback/failure/heal). The // default is logNotifier; the field is the seam external senders plug into. @@ -64,14 +63,22 @@ type Service struct { retryMu sync.Mutex retries map[string]retryState + + // rolledBackMu guards rolledBack. + rolledBackMu sync.Mutex + // rolledBack records standalone containers whose update was rolled back, keyed + // by endpoint+name, so the auto-update job does not immediately re-pull the + // same failed image and roll back again on the next tick (the update->rollback + // loop guard, mirroring the auto-heal retries map). + rolledBack map[string]rolledBackTarget } // NewService creates a new container automation service. Call Start to schedule // the jobs according to the persisted settings. baseCtx is the application // shutdown context: it bounds the job operation contexts so a shutdown cancels -// any in-flight heal/update. The stackDeployer, gitService and containerService -// are used by the auto-update job; they may be nil only in tests that do not -// exercise auto-update. +// any in-flight heal/update. The stackDeployer and containerService are used by +// the auto-update job; they may be nil only in tests that do not exercise +// auto-update. func NewService( baseCtx context.Context, scheduler *scheduler.Scheduler, @@ -79,7 +86,6 @@ func NewService( clientFactory *dockerclient.ClientFactory, containerService *docker.ContainerService, stackDeployer deployments.StackDeployer, - gitService portainer.GitService, ) *Service { if baseCtx == nil { baseCtx = context.Background() @@ -93,9 +99,9 @@ func NewService( digestClient: images.NewClientWithRegistry(images.NewRegistryClient(dataStore), clientFactory), containerService: containerService, stackDeployer: stackDeployer, - gitService: gitService, notifier: logNotifier{}, retries: make(map[string]retryState), + rolledBack: make(map[string]rolledBackTarget), } } @@ -234,6 +240,46 @@ func (s *Service) setRetry(containerID string, state retryState) { s.retries[containerID] = state } +// getRolledBack returns the rolled-back target for a key and whether it exists. +func (s *Service) getRolledBack(key string) (rolledBackTarget, bool) { + s.rolledBackMu.Lock() + defer s.rolledBackMu.Unlock() + + rec, ok := s.rolledBack[key] + + return rec, ok +} + +// setRolledBack records a rolled-back target for a key. +func (s *Service) setRolledBack(key string, rec rolledBackTarget) { + s.rolledBackMu.Lock() + defer s.rolledBackMu.Unlock() + + s.rolledBack[key] = rec +} + +// clearRolledBack drops the rolled-back record for a key (cooldown elapsed or a +// new upstream image lifted the skip). +func (s *Service) clearRolledBack(key string) { + s.rolledBackMu.Lock() + defer s.rolledBackMu.Unlock() + + delete(s.rolledBack, key) +} + +// pruneRolledBack drops rolled-back records whose cooldown has fully elapsed, so +// the map cannot grow unbounded. It mirrors pruneRetries. +func (s *Service) pruneRolledBack(now time.Time) { + s.rolledBackMu.Lock() + defer s.rolledBackMu.Unlock() + + for key, rec := range s.rolledBack { + if now.Sub(rec.at) >= updateRollbackCooldown { + delete(s.rolledBack, key) + } + } +} + // pruneRetries drops retry state for containers whose retry window has fully // elapsed since their last restart. A container is kept regardless of whether it // appeared in the current tick: one that briefly leaves the unhealthy filter diff --git a/api/http/handler/settings/settings_update.go b/api/http/handler/settings/settings_update.go index aec63a5ac..693e9837a 100644 --- a/api/http/handler/settings/settings_update.go +++ b/api/http/handler/settings/settings_update.go @@ -68,7 +68,7 @@ type settingsUpdatePayload struct { EdgePortainerURL *string `json:"EdgePortainerURL"` // ForceSecureCookies forces the Secure attribute on auth cookies regardless of the detected scheme ForceSecureCookies *bool `example:"false"` - // Native container automation settings (auto-heal) + // Native container automation settings (auto-heal / auto-update) ContainerAutomation *containerAutomationSettingsPayload } @@ -204,8 +204,9 @@ func (handler *Handler) settingsUpdate(w http.ResponseWriter, r *http.Request) * return response.TxErrorResponse(err) } - // Re-apply container automation settings so the auto-heal job is rescheduled - // (or stopped) with the new interval/scope after a successful save. + // Re-apply container automation settings so the auto-heal and auto-update jobs + // are rescheduled (or stopped) with the new interval/scope after a successful + // save. if handler.ContainerAutomationService != nil { if err := handler.ContainerAutomationService.Reload(); err != nil { log.Warn().Err(err).Msg("unable to reload container automation settings") diff --git a/api/portainer.go b/api/portainer.go index 10dcb9c8d..8d1a6d0ab 100644 --- a/api/portainer.go +++ b/api/portainer.go @@ -1156,6 +1156,33 @@ type ( AsyncMode bool `json:"AsyncMode,omitempty" example:"false"` } + // ContainerAutoHealSettings holds the native auto-heal settings. + ContainerAutoHealSettings struct { + Enabled bool `json:"Enabled"` + CheckInterval string `json:"CheckInterval" example:"30s"` + Scope string `json:"Scope" example:"labeled"` // "labeled" | "all" + } + + // ContainerAutoUpdateSettings holds the native auto-update settings. + ContainerAutoUpdateSettings struct { + Enabled bool `json:"Enabled"` + PollInterval string `json:"PollInterval" example:"6h"` + Scope string `json:"Scope" example:"labeled"` // "labeled" | "all" + Cleanup bool `json:"Cleanup"` // remove dangling old images after a standalone update + // RollbackOnFailure health-gates a standalone update: if the new + // container does not become healthy within RollbackTimeout, it is + // recreated back on the previous image. Standalone-only (M5). + RollbackOnFailure bool `json:"RollbackOnFailure"` + RollbackTimeout string `json:"RollbackTimeout" example:"120s"` + } + + // ContainerAutomationSettings holds native container automation settings + // (auto-heal and auto-update). + ContainerAutomationSettings struct { + AutoHeal ContainerAutoHealSettings `json:"AutoHeal"` + AutoUpdate ContainerAutoUpdateSettings `json:"AutoUpdate"` + } + // Settings represents the application settings Settings struct { // URL to a logo that will be displayed on the login page as well as on top of the sidebar. Will use default Portainer logo when value is empty string @@ -1218,24 +1245,7 @@ type ( ForceSecureCookies bool `json:"ForceSecureCookies" example:"false"` // ContainerAutomation holds native container automation settings. - ContainerAutomation struct { - AutoHeal struct { - Enabled bool `json:"Enabled"` - CheckInterval string `json:"CheckInterval" example:"30s"` - Scope string `json:"Scope" example:"labeled"` // "labeled" | "all" - } `json:"AutoHeal"` - AutoUpdate struct { - Enabled bool `json:"Enabled"` - PollInterval string `json:"PollInterval" example:"6h"` - Scope string `json:"Scope" example:"labeled"` // "labeled" | "all" - Cleanup bool `json:"Cleanup"` // remove dangling old images after a standalone update - // RollbackOnFailure health-gates a standalone update: if the new - // container does not become healthy within RollbackTimeout, it is - // recreated back on the previous image. Standalone-only (M5). - RollbackOnFailure bool `json:"RollbackOnFailure"` - RollbackTimeout string `json:"RollbackTimeout" example:"120s"` - } `json:"AutoUpdate"` - } `json:"ContainerAutomation"` + ContainerAutomation ContainerAutomationSettings `json:"ContainerAutomation"` } // SnapshotJob represents a scheduled job that can create environment(endpoint) snapshots diff --git a/app/react/docker/containers/update/applyContainerUpdate.test.ts b/app/react/docker/containers/update/applyContainerUpdate.test.ts new file mode 100644 index 000000000..e439304a1 --- /dev/null +++ b/app/react/docker/containers/update/applyContainerUpdate.test.ts @@ -0,0 +1,165 @@ +import { vi, describe, it, expect, beforeEach } from 'vitest'; + +import { Stack, StackType } from '@/react/common/stacks/types'; + +import { + applyContainerUpdate, + EXTERNAL_STACK_UPDATE_ERROR, +} from './applyContainerUpdate'; +import { ContainerUpdateContext } from './types'; +import { resolveContainerUpdatePath } from './resolveContainerUpdatePath'; + +// Mock the side-effecting mutations and the file fetch so we assert the dispatch +// and payload mapping without touching the network. +const recreateContainer = vi.fn(); +const updateStack = vi.fn(); +const updateGitStack = vi.fn(); +const getStackFile = vi.fn(); + +vi.mock('../containers.service', () => ({ + recreateContainer: (...args: unknown[]) => recreateContainer(...args), +})); +vi.mock('@/react/docker/stacks/useUpdateStack', () => ({ + updateStack: (...args: unknown[]) => updateStack(...args), +})); +vi.mock('@/react/portainer/gitops/queries/useUpdateGitStack', () => ({ + updateGitStack: (...args: unknown[]) => updateGitStack(...args), +})); +vi.mock('@/react/common/stacks/queries/useStackFile', () => ({ + getStackFile: (...args: unknown[]) => getStackFile(...args), +})); + +// Drive the standalone/stack/external dispatch deterministically; the resolver's +// own routing logic is covered by resolveContainerUpdatePath.test.ts. +vi.mock('./resolveContainerUpdatePath', () => ({ + resolveContainerUpdatePath: vi.fn(), +})); + +const resolveMock = vi.mocked(resolveContainerUpdatePath); + +function buildContext( + overrides: Partial = {} +): ContainerUpdateContext { + return { + id: 'abc123', + name: 'my-container', + image: 'nginx:latest', + environmentId: 3, + nodeName: 'node-1', + ...overrides, + }; +} + +function buildStack(overrides: Partial): Stack { + return { + Id: 7, + Name: 'my-stack', + EndpointId: 3, + Type: StackType.DockerCompose, + Env: [{ name: 'FOO', value: 'bar' }], + Option: { Prune: true }, + ...overrides, + } as Stack; +} + +describe('applyContainerUpdate', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('recreates a standalone container with pull and node, returning "standalone"', async () => { + resolveMock.mockReturnValue({ kind: 'standalone' }); + + const context = buildContext(); + const result = await applyContainerUpdate(context, []); + + expect(result).toBe('standalone'); + expect(recreateContainer).toHaveBeenCalledWith(3, 'abc123', true, { + nodeName: 'node-1', + }); + expect(updateStack).not.toHaveBeenCalled(); + expect(updateGitStack).not.toHaveBeenCalled(); + }); + + it('passes pullImage=false through to the standalone recreate', async () => { + resolveMock.mockReturnValue({ kind: 'standalone' }); + + await applyContainerUpdate(buildContext(), [], { pullImage: false }); + + expect(recreateContainer).toHaveBeenCalledWith(3, 'abc123', false, { + nodeName: 'node-1', + }); + }); + + it('redeploys a git stack via updateGitStack with RepullImageAndRedeploy', async () => { + const stack = buildStack({ + Id: 9, + GitConfig: { URL: 'https://example.com/repo.git' } as Stack['GitConfig'], + }); + resolveMock.mockReturnValue({ + kind: 'stack', + stackId: 9, + isGitStack: true, + }); + + const result = await applyContainerUpdate(buildContext(), [stack]); + + expect(result).toBe('stack'); + expect(updateGitStack).toHaveBeenCalledWith(9, 3, { + RepullImageAndRedeploy: true, + Env: [{ name: 'FOO', value: 'bar' }], + Prune: true, + }); + expect(getStackFile).not.toHaveBeenCalled(); + expect(updateStack).not.toHaveBeenCalled(); + }); + + it('redeploys a file stack via updateStack, preserving its current file content', async () => { + const stack = buildStack({ Id: 7 }); + resolveMock.mockReturnValue({ + kind: 'stack', + stackId: 7, + isGitStack: false, + }); + getStackFile.mockResolvedValue({ StackFileContent: 'version: "3"\n' }); + + const result = await applyContainerUpdate(buildContext(), [stack]); + + expect(result).toBe('stack'); + expect(getStackFile).toHaveBeenCalledWith({ stackId: 7 }); + expect(updateStack).toHaveBeenCalledWith({ + stackId: 7, + environmentId: 3, + payload: { + stackFileContent: 'version: "3"\n', + env: [{ name: 'FOO', value: 'bar' }], + prune: true, + repullImageAndRedeploy: true, + }, + }); + expect(updateGitStack).not.toHaveBeenCalled(); + }); + + it('throws for an externally-managed compose container', async () => { + resolveMock.mockReturnValue({ kind: 'external' }); + + await expect(applyContainerUpdate(buildContext(), [])).rejects.toThrow( + EXTERNAL_STACK_UPDATE_ERROR + ); + expect(recreateContainer).not.toHaveBeenCalled(); + expect(updateStack).not.toHaveBeenCalled(); + expect(updateGitStack).not.toHaveBeenCalled(); + }); + + it('throws rather than bare-recreating when the resolved stack is missing', async () => { + // Guard: resolve routed to a stack, but no stack with that id is present. + resolveMock.mockReturnValue({ kind: 'stack', stackId: 999 }); + + await expect(applyContainerUpdate(buildContext(), [])).rejects.toThrow( + EXTERNAL_STACK_UPDATE_ERROR + ); + expect(recreateContainer).not.toHaveBeenCalled(); + expect(updateStack).not.toHaveBeenCalled(); + expect(updateGitStack).not.toHaveBeenCalled(); + }); +}); diff --git a/app/react/docker/containers/update/applyContainerUpdate.ts b/app/react/docker/containers/update/applyContainerUpdate.ts index 1479360c0..aca3c04d0 100644 --- a/app/react/docker/containers/update/applyContainerUpdate.ts +++ b/app/react/docker/containers/update/applyContainerUpdate.ts @@ -43,9 +43,12 @@ async function redeployStackWithPull(stack: Stack, pullImage: boolean) { /** * Shared "apply an image update" primitive. Decides standalone-vs-stack-vs-external - * and runs the matching mutation. This is the single code path behind the - * "Update now" button, the bulk "Update selected" action and (future M4) the - * auto-update job, guaranteeing manual and automatic updates behave identically. + * and runs the matching mutation. This is the single frontend code path behind the + * "Update now" button and the bulk "Update selected" action, guaranteeing both + * manual flows behave identically. The backend auto-update daemon does NOT call + * this primitive: it is a separate Go implementation + * (resolveContainerUpdateRouting / groupContainersForUpdate in + * api/containerautomation) that mirrors the same routing on the server. * * A stack-managed container is ALWAYS routed through stack redeploy so it stays * part of its stack; an externally-managed compose container is refused rather diff --git a/app/react/docker/containers/update/resolveContainerUpdatePath.ts b/app/react/docker/containers/update/resolveContainerUpdatePath.ts index 29c03425f..77cff88e1 100644 --- a/app/react/docker/containers/update/resolveContainerUpdatePath.ts +++ b/app/react/docker/containers/update/resolveContainerUpdatePath.ts @@ -7,7 +7,8 @@ import { ContainerUpdatePath } from './types'; /** * Decide how a container's image update must be applied, given the list of * Portainer-managed stacks. Pure and side-effect free so it can be unit-tested - * and reused by the details button, the bulk action and the M4 auto-update job. + * and reused by the details button and the bulk action. (The backend auto-update + * daemon mirrors the same routing separately in Go.) * * - No compose project label -> `standalone` (recreate-with-pull). * - Compose project that matches a Portainer Docker Compose `Stack` (same name + diff --git a/app/react/docker/containers/update/types.ts b/app/react/docker/containers/update/types.ts index bea1e988f..0a06b21b6 100644 --- a/app/react/docker/containers/update/types.ts +++ b/app/react/docker/containers/update/types.ts @@ -5,8 +5,8 @@ import { ContainerId } from '../types'; /** * Minimal context needed to apply an image update to a single container, - * independent of whether it comes from the details view, the bulk action or - * (future M4) the auto-update job. + * independent of whether it comes from the details view or the bulk action. + * (The backend auto-update daemon mirrors the same routing separately in Go.) */ export interface ContainerUpdateContext { id: ContainerId;