From ac6b618bef8193bf347c0ccf6a885832f0977a56 Mon Sep 17 00:00:00 2001 From: Kyle Mendell Date: Thu, 8 Oct 2026 19:41:50 -0500 Subject: [PATCH] fix(updater): recreate containers per docker and compose spec --- updater/apply.go | 336 +++------- updater/apply_test.go | 12 +- updater/check.go | 41 +- updater/compose_groups.go | 36 +- updater/compose_images.go | 119 ++-- updater/compose_images_test.go | 24 +- updater/compose_updater.go | 62 +- updater/config.go | 19 +- updater/container.go | 619 ++++++++---------- updater/container_test.go | 125 ++-- updater/default_auth.go | 100 +-- updater/defaults.go | 57 +- updater/defaults_test.go | 2 +- updater/errors.go | 28 - updater/go.mod | 6 +- updater/go.sum | 4 + updater/hooks.go | 24 +- updater/internal/compose/compose.go | 30 +- updater/internal/compose/compose_test.go | 13 +- updater/internal/deps/sorter.go | 31 +- updater/internal/digestcheck/digestcheck.go | 96 +-- updater/internal/match/matching.go | 103 +-- updater/internal/registryhost/registryhost.go | 28 - .../registryhost/registryhost_test.go | 25 - updater/labelpolicy.go | 11 +- updater/labels/labels.go | 32 +- updater/memory_store_test.go | 2 +- updater/pkg/utils/tagpolicy/tagpolicy.go | 21 +- updater/ports.go | 5 +- updater/refs/reference.go | 44 +- updater/registry/registry_http.go | 91 +-- updater/registry/registry_http_test.go | 32 +- updater/registry/tags.go | 9 +- updater/registry/tags_test.go | 48 +- updater/restart.go | 487 +++++--------- updater/restart_helpers.go | 89 +-- updater/restart_test.go | 18 +- updater/results.go | 32 +- updater/status.go | 75 +-- updater/tag_defaults.go | 8 +- updater/targeted.go | 342 ++++------ updater/targeted_test.go | 102 ++- updater/types.go | 22 +- updater/types/updates.go | 10 +- updater/updater.go | 91 +-- 45 files changed, 1301 insertions(+), 2210 deletions(-) delete mode 100644 updater/internal/registryhost/registryhost.go delete mode 100644 updater/internal/registryhost/registryhost_test.go diff --git a/updater/apply.go b/updater/apply.go index 6fb0dd5..fac1c55 100644 --- a/updater/apply.go +++ b/updater/apply.go @@ -4,10 +4,12 @@ import ( "context" "fmt" "io" + "slices" "strings" "github.com/moby/moby/client" "go.getarcane.app/docker/compat" + kit "go.getarcane.app/kit/pkg" "go.getarcane.app/updater/internal/digestcheck" "go.getarcane.app/updater/internal/match" @@ -22,20 +24,16 @@ func (s *Service) ApplyPending(ctx context.Context, opts Options) (out *Result, if s.config.PendingStore == nil { return nil, ErrPendingStoreRequired } - records, err := s.config.PendingStore.PendingImageUpdates(ctx) if err != nil { return nil, fmt.Errorf("query pending image updates: %w", err) } + if slices.ContainsFunc(records, func(record ImageUpdateRecord) bool { return record.IsTagUpdate() || record.ContainerID != "" }) { + return out, s.applyTargetedRecords(ctx, records, opts, out) + } if len(records) == 0 { return out, nil } - - for _, record := range records { - if record.IsTagUpdate() || record.ContainerID != "" { - return out, s.applyTargetedRecordsInternal(ctx, records, opts, out) - } - } usedImages, err := s.usedImages(ctx) if err != nil { return nil, fmt.Errorf("collect used images: %w", err) @@ -44,249 +42,131 @@ func (s *Service) ApplyPending(ctx context.Context, opts Options) (out *Result, return out, nil } - var planDockerClient *client.Client - if dockerClient, dockerErr := s.dockerClient(ctx); dockerErr == nil { - planDockerClient = dockerClient + dockerClient, dockerErr := s.dockerClient(ctx) + digestChecker := digestcheck.NewChecker(dockerClient, s.config.RegistryDigestResolver) + var plans []updatePlan + for _, record := range records { + oldRef := record.ImageRef() + oldNorm := refs.NormalizeImageUpdateRef(oldRef) + if !record.NeedsUpdate() || oldRef == "" { + continue + } + if oldNorm == "" { + s.logger.DebugContext(ctx, "skipping invalid pending image reference", "imageRef", oldRef) + continue + } + if _, used := usedImages[oldNorm]; used { + oldIDs, _ := digestChecker.GetImageIDsForRef(ctx, oldRef) + plans = append(plans, updatePlan{record: record, oldRef: oldRef, newRef: record.NewImageRef(), oldIDs: match.AppendImageUpdateRecordIDToOldIDs(oldIDs, record.ID)}) + } } - plans := s.buildUpdatePlans(ctx, records, usedImages, digestcheck.NewChecker(planDockerClient, nil)) if len(plans) == 0 { return out, nil } - - var dockerClient *client.Client - if !opts.DryRun || s.config.RegistryDigestResolver != nil { - dockerClient, err = s.dockerClient(ctx) - if err != nil && !opts.DryRun { - return nil, fmt.Errorf("docker connect: %w", err) - } + if dockerErr != nil && !opts.DryRun { + return nil, dockerErr } - digestChecker := digestcheck.NewChecker(dockerClient, s.config.RegistryDigestResolver) + oldIDToNewRef := map[string]string{} oldRefToNewRef := map[string]string{} - for i := range plans { plan := &plans[i] - item := s.executeUpdatePlan(ctx, digestChecker, plan, opts) - out.Checked++ - switch item.Status { - case StatusSkipped: - out.Skipped++ - case StatusFailed: - out.Failed++ - case StatusUpdated: - out.Updated++ + item := ResourceResult{ + ResourceID: plan.oldRef, ResourceType: ResourceTypeImage, ResourceName: plan.oldRef, + Status: StatusSkipped, OldImage: plan.oldRef, NewImage: plan.newRef, + } + if !opts.DryRun { + item.Status, item.Error = s.applyUpdatePlan(ctx, digestChecker, plan, opts.Force) + item.UpdateApplied, item.UpdateAvailable = item.Status == StatusUpdated, item.Status == StatusUpdated + } + s.appendRecordedResult(ctx, out, item) + if item.Status == StatusUpdated { for _, oldID := range plan.oldIDs { oldIDToNewRef[oldID] = plan.newRef } oldRefToNewRef[plan.oldRef] = plan.newRef - case StatusChecked, StatusRestarted, StatusUpToDate, StatusUpdateAvailable: - // Already counted by the unconditional out.Checked++ above. } - out.Items = append(out.Items, item) - _ = s.recordResult(ctx, item) - } - - if restartErr := s.restartAfterApply(ctx, out, plans, oldIDToNewRef, oldRefToNewRef); restartErr != nil { - return out, restartErr } - s.clearAppliedPlanRecords(ctx, plans) - return out, nil -} - -// executeUpdatePlan applies a single pending-update plan: it honors -// dry-run, pulls the new reference unless it is already current locally, and -// reports whether any old image ID is still stale. It marks plan.pulled once -// the target image is present. -func (s *Service) executeUpdatePlan(ctx context.Context, digestChecker *digestcheck.Checker, plan *updatePlan, opts Options) ResourceResult { - item := ResourceResult{ - ResourceID: plan.oldRef, - ResourceType: ResourceTypeImage, - ResourceName: plan.oldRef, - Status: StatusChecked, - OldImage: plan.oldRef, - NewImage: plan.newRef, - } - - if opts.DryRun { - item.Status = StatusSkipped - return item - } - upToDateMessage := "image digest unchanged after pull" - if !opts.Force && planImageUpToDate(ctx, digestChecker, *plan) { - upToDateMessage = "image already up to date" - } else { - if s.config.ImagePuller == nil { - item.Status = StatusFailed - item.Error = ErrImagePullerRequired.Error() - return item + // Containers still on a replaced image are restarted; a failed restart keeps its plan's record. + var restarted []ResourceResult + if len(oldIDToNewRef) > 0 || len(oldRefToNewRef) > 0 { + restarted, err = s.RestartContainersUsingOldImages(ctx, oldIDToNewRef, oldRefToNewRef) + for _, item := range restarted { + s.appendRecordedResult(ctx, out, item) } - if err := s.config.ImagePuller.PullImage(ctx, plan.newRef, io.Discard); err != nil { - item.Status = StatusFailed - item.Error = fmt.Sprintf("pull failed: %v", err) - return item + if err != nil { + return out, err } } - plan.pulled = true - - if !opts.Force { - if targetIDs, targetErr := digestChecker.GetImageIDsForRef(ctx, plan.newRef); targetErr == nil { - stale, matched := staleImageIDs(plan.oldIDs, targetIDs) - if len(stale) == 0 && matched { - item.Status = StatusUpToDate - item.Error = upToDateMessage - return item + for _, plan := range plans { + newRef := refs.NormalizeImageUpdateRef(plan.newRef) + restartFailed := slices.ContainsFunc(restarted, func(item ResourceResult) bool { + return item.Status == StatusFailed && newRef != "" && refs.NormalizeImageUpdateRef(item.NewImage) == newRef + }) + switch { + case !plan.pulled: + case restartFailed: + s.logger.WarnContext(ctx, "keeping image update record after restart failure", "imageRef", plan.oldRef, "newRef", plan.newRef) + default: + if clearErr := s.config.PendingStore.ClearImageUpdateRecord(ctx, plan.record); clearErr != nil { + s.logger.WarnContext(ctx, "failed to clear image update record", "imageRef", plan.oldRef, "error", clearErr) } - plan.oldIDs = stale - } - } - - item.Status = StatusUpdated - item.UpdateApplied = true - item.UpdateAvailable = true - return item -} - -// restartAfterApply restarts containers still running the replaced -// images and folds their results into out, flagging plans whose restart failed -// so their pending records are kept. -func (s *Service) restartAfterApply(ctx context.Context, out *Result, plans []updatePlan, oldIDToNewRef, oldRefToNewRef map[string]string) error { - if len(oldIDToNewRef) == 0 && len(oldRefToNewRef) == 0 { - return nil - } - containerResults, restartErr := s.RestartContainersUsingOldImages(ctx, oldIDToNewRef, oldRefToNewRef) - markRestartFailedPlans(plans, containerResults) - for _, item := range containerResults { - s.applyResultCount(out, item) - out.Items = append(out.Items, item) - _ = s.recordResult(ctx, item) - } - return restartErr -} - -func (s *Service) clearAppliedPlanRecords(ctx context.Context, plans []updatePlan) { - for i := range plans { - if !plans[i].pulled { - continue - } - if plans[i].restartFailed { - s.logger.WarnContext(ctx, "keeping image update record after restart failure", "imageRef", plans[i].oldRef, "newRef", plans[i].newRef) - continue - } - if err := s.config.PendingStore.ClearImageUpdateRecord(ctx, plans[i].record); err != nil { - s.logger.WarnContext(ctx, "failed to clear image update record", "imageRef", plans[i].oldRef, "error", err) - } - } -} - -func markRestartFailedPlans(plans []updatePlan, results []ResourceResult) { - failedRefs := restartFailedImageRefs(results) - if len(failedRefs) == 0 { - return - } - - for i := range plans { - if !plans[i].pulled { - continue - } - newRef := refs.NormalizeImageUpdateRef(plans[i].newRef) - if _, failed := failedRefs[newRef]; failed { - plans[i].restartFailed = true } } + return out, nil } -func restartFailedImageRefs(results []ResourceResult) map[string]struct{} { - failedRefs := map[string]struct{}{} - for _, result := range results { - if result.Status != StatusFailed { - continue +// applyUpdatePlan pulls a plan's new reference unless it is already current, then reports whether any old +// image ID is still stale. It marks plan.pulled once the target image is present. +func (s *Service) applyUpdatePlan(ctx context.Context, digestChecker *digestcheck.Checker, plan *updatePlan, force bool) (ResourceStatus, string) { + upToDate := false + if !force { + imageRef := refs.NormalizeImageUpdateRef(plan.newRef) + if knownDigest := strings.TrimSpace(kit.FromPtr(plan.record.LatestDigest)); plan.record.IsDigestUpdate() && knownDigest != "" { + check := digestChecker.CheckImageMatchesKnownDigest(ctx, imageRef, knownDigest) + upToDate = check.Error == nil && !check.NeedsUpdate + } else { + check := digestChecker.CheckImageNeedsUpdate(ctx, imageRef) + upToDate = check.CheckedViaAPI && check.Error == nil && !check.NeedsUpdate + } + } + upToDateMessage := kit.Ternary(upToDate, "image already up to date", "image digest unchanged after pull") + if !upToDate { + if s.config.ImagePuller == nil { + return StatusFailed, ErrImagePullerRequired.Error() } - if newRef := refs.NormalizeImageUpdateRef(result.NewImage); newRef != "" { - failedRefs[newRef] = struct{}{} + if err := s.config.ImagePuller.PullImage(ctx, plan.newRef, io.Discard); err != nil { + return StatusFailed, fmt.Sprintf("pull failed: %v", err) } } - return failedRefs -} + plan.pulled = true -func planImageUpToDate(ctx context.Context, checker *digestcheck.Checker, plan updatePlan) bool { - imageRef := refs.NormalizeImageUpdateRef(plan.newRef) - if plan.record.IsDigestUpdate() && plan.record.LatestDigest != nil { - if knownDigest := strings.TrimSpace(*plan.record.LatestDigest); knownDigest != "" { - check := checker.CheckImageMatchesKnownDigest(ctx, imageRef, knownDigest) - return check.Error == nil && !check.NeedsUpdate + if targetIDs, targetErr := digestChecker.GetImageIDsForRef(ctx, plan.newRef); targetErr == nil && !force { + matched := slices.ContainsFunc(plan.oldIDs, func(id string) bool { return slices.Contains(targetIDs, id) }) + plan.oldIDs = slices.DeleteFunc(plan.oldIDs, func(id string) bool { return id == "" || slices.Contains(targetIDs, id) }) + if len(plan.oldIDs) == 0 && matched { + return StatusUpToDate, upToDateMessage } } - - check := checker.CheckImageNeedsUpdate(ctx, imageRef) - return check.CheckedViaAPI && check.Error == nil && !check.NeedsUpdate + return StatusUpdated, "" } func (s *Service) dockerClient(ctx context.Context) (*client.Client, error) { if s.config.DockerClientProvider == nil { - return nil, ErrDockerClientProviderRequired + return nil, fmt.Errorf("docker connect: %w", ErrDockerClientProviderRequired) } dockerClient, err := s.config.DockerClientProvider.DockerClient(ctx) - if err != nil { - return nil, err + if err == nil && dockerClient == nil { + err = ErrNilDockerClient } - if dockerClient == nil { - return nil, ErrNilDockerClient + if err != nil { + return nil, fmt.Errorf("docker connect: %w", err) } return dockerClient, nil } -func (s *Service) buildUpdatePlans(ctx context.Context, records []ImageUpdateRecord, usedImages map[string]struct{}, digestChecker *digestcheck.Checker) []updatePlan { - var plans []updatePlan - for _, record := range records { - if !record.NeedsUpdate() { - continue - } - oldRef := record.ImageRef() - if oldRef == "" { - continue - } - oldNorm := refs.NormalizeImageUpdateRef(oldRef) - if oldNorm == "" { - s.logger.DebugContext(ctx, "skipping invalid pending image reference", "imageRef", oldRef) - continue - } - if _, ok := usedImages[oldNorm]; !ok { - continue - } - - newRef := record.NewImageRef() - var oldIDs []string - if digestChecker != nil { - oldIDs, _ = digestChecker.GetImageIDsForRef(ctx, oldRef) - } - oldIDs = match.AppendImageUpdateRecordIDToOldIDs(oldIDs, record.ID) - plans = append(plans, updatePlan{record: record, oldRef: oldRef, newRef: newRef, oldIDs: oldIDs}) - } - return plans -} - -func staleImageIDs(oldIDs, targetIDs []string) ([]string, bool) { - current := make(map[string]struct{}, len(targetIDs)) - for _, id := range targetIDs { - current[strings.TrimSpace(id)] = struct{}{} - } - var stale []string - matched := false - for _, id := range oldIDs { - id = strings.TrimSpace(id) - if id == "" { - continue - } - if _, ok := current[id]; ok { - matched = true - continue - } - stale = append(stale, id) - } - return stale, matched -} - +// usedImages returns the normalized references of images that running, eligible containers use. func (s *Service) usedImages(ctx context.Context) (map[string]struct{}, error) { if s.config.UsedImageCollector != nil { return s.config.UsedImageCollector.UsedImages(ctx) @@ -295,9 +175,7 @@ func (s *Service) usedImages(ctx context.Context) (map[string]struct{}, error) { if err != nil { return nil, err } - - out := map[string]struct{}{} - excludedContainers, err := s.excludedContainerSet(ctx) + excluded, err := s.excludedContainerSet(ctx) if err != nil { return nil, err } @@ -305,8 +183,12 @@ func (s *Service) usedImages(ctx context.Context) (map[string]struct{}, error) { if err != nil { return nil, err } + + policy := s.config.LabelPolicy + out := map[string]struct{}{} for _, summary := range listResult.Items { - if shouldSkipSummary(summary, excludedContainers, "", s.config.LabelPolicy) { + if isExcluded(excluded, summary.ID, summary.Names...) || policy.IsUpdateDisabled(summary.Labels) || + (policy.IsSwarmTask(summary.Labels) && !policy.IsSelfUpdateTarget(summary.Labels)) { continue } if imageRef := refs.NormalizeImageUpdateRef(summary.Image); imageRef != "" { @@ -314,33 +196,33 @@ func (s *Service) usedImages(ctx context.Context) (map[string]struct{}, error) { continue } inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, summary.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { + if inspectErr != nil || inspectResult.Container.Config == nil || policy.IsUpdateDisabled(inspectResult.Container.Config.Labels) { continue } - if inspectResult.Container.Config != nil && s.config.LabelPolicy.IsUpdateDisabled(inspectResult.Container.Config.Labels) { - continue + candidates := []string{inspectResult.Container.Config.Image} + if imageInspect, imageErr := dockerClient.ImageInspect(ctx, inspectResult.Container.Image); imageErr == nil { + candidates = append(candidates, imageInspect.RepoTags...) } - for _, tag := range normalizedTagsForContainer(ctx, dockerClient, inspectResult.Container) { - out[tag] = struct{}{} + for _, candidate := range candidates { + if normalized := refs.NormalizeImageUpdateRef(candidate); normalized != "" { + out[normalized] = struct{}{} + } } } return out, nil } func (s *Service) excludedContainerSet(ctx context.Context) (map[string]bool, error) { + excluded := map[string]bool{} if s.config.Settings == nil { - return nil, nil + return excluded, nil } - excluded, err := s.config.Settings.ExcludedContainers(ctx) + names, err := s.config.Settings.ExcludedContainers(ctx) if err != nil { return nil, err } - out := make(map[string]bool, len(excluded)) - for _, name := range excluded { - name = strings.TrimSpace(name) - if name != "" { - out[name] = true - } + for _, name := range kit.TrimNonEmpty(names) { + excluded[name] = true } - return out, nil + return excluded, nil } diff --git a/updater/apply_test.go b/updater/apply_test.go index 01375c1..7892da5 100644 --- a/updater/apply_test.go +++ b/updater/apply_test.go @@ -94,7 +94,7 @@ func TestApplyPendingSkipsUnchangedPulledImage(t *testing.T) { if err != nil { t.Fatalf("ApplyPending() error = %v", err) } - if got.Checked != 1 || got.Updated != 0 || got.Skipped != 0 || got.Failed != 0 { + if got.Checked != 1 || got.Updated != 0 || got.Skipped != 1 || got.Failed != 0 { t.Fatalf("ApplyPending() counts = checked:%d updated:%d skipped:%d failed:%d", got.Checked, got.Updated, got.Skipped, got.Failed) } if len(got.Items) != 1 || got.Items[0].Status != StatusUpToDate { @@ -264,7 +264,7 @@ func TestApplyPendingReportsUpToDateWhenKnownDigestMatchesAnyLocalRepoDigest(t * if err != nil { t.Fatalf("ApplyPending() error = %v", err) } - if got.Checked != 1 || got.Updated != 0 || got.Skipped != 0 || got.Failed != 0 { + if got.Checked != 1 || got.Updated != 0 || got.Skipped != 1 || got.Failed != 0 { t.Fatalf("ApplyPending() counts = checked:%d updated:%d skipped:%d failed:%d", got.Checked, got.Updated, got.Skipped, got.Failed) } if len(got.Items) != 1 || got.Items[0].Status != StatusUpToDate { @@ -302,7 +302,7 @@ func TestApplyPendingRestartsStaleContainersWhenImageAlreadyPulled(t *testing.T) {ID: "app-id", Names: []string{"/app"}, Image: "app:1", ImageID: "sha256:old-app", State: "running"}, }) case r.Method == http.MethodGet && path == "/containers/app-id/json": - writeDockerJSON(t, w, container.InspectResponse{ID: "app-id", Name: "/app", Image: "sha256:old-app", Config: &container.Config{Image: "app:1"}}) + writeDockerJSON(t, w, container.InspectResponse{State: &container.State{Running: true}, ID: "app-id", Name: "/app", Image: "sha256:old-app", Config: &container.Config{Image: "app:1"}}) case r.Method == http.MethodPost && strings.HasSuffix(path, "/stop"): w.WriteHeader(http.StatusOK) case r.Method == http.MethodDelete && strings.HasPrefix(path, "/containers/"): @@ -376,8 +376,8 @@ func TestApplyPendingReusesDockerClientWhileBuildingPlans(t *testing.T) { _, _ = service.ApplyPending(t.Context(), Options{}) - if provider.calls != 2 { - t.Fatalf("DockerClient calls = %d, want 2 total calls independent of record count", provider.calls) + if provider.calls != 1 { + t.Fatalf("DockerClient calls = %d, want 1 call independent of record count", provider.calls) } } @@ -407,7 +407,7 @@ func TestApplyPendingKeepsPulledRecordWhenRestartFails(t *testing.T) { {ID: "app-id", Names: []string{"/app"}, Image: "app:1", ImageID: "sha256:old-app", State: "running"}, }) case r.Method == http.MethodGet && path == "/containers/app-id/json": - writeDockerJSON(t, w, container.InspectResponse{ID: "app-id", Name: "/app", Image: "sha256:old-app", Config: &container.Config{Image: "app:1"}}) + writeDockerJSON(t, w, container.InspectResponse{State: &container.State{Running: true}, ID: "app-id", Name: "/app", Image: "sha256:old-app", Config: &container.Config{Image: "app:1"}}) case r.Method == http.MethodPost && strings.HasSuffix(path, "/stop"): w.WriteHeader(http.StatusOK) case r.Method == http.MethodDelete && strings.HasPrefix(path, "/containers/"): diff --git a/updater/check.go b/updater/check.go index b5f6e67..77d6941 100644 --- a/updater/check.go +++ b/updater/check.go @@ -4,7 +4,7 @@ import ( "context" "errors" "fmt" - "strings" + "slices" "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" @@ -20,7 +20,7 @@ import ( // CheckImageUpdate discovers an image update without pulling or changing state. // CurrentDigest should describe the image in use, not a separately cached tag. func (s *Service) CheckImageUpdate(ctx context.Context, request types.CheckRequest) (types.CheckResult, error) { - result, err := s.selectImageTagInternal(ctx, request.ImageRef, request.Policy) + result, err := s.selectImageTag(ctx, request.ImageRef, request.Policy) if err != nil || result.Reason != "" || result.UpdateAvailable { return result, err } @@ -57,7 +57,7 @@ func (s *Service) CheckImageUpdate(ctx context.Context, request types.CheckReque return result, nil } -func (s *Service) selectImageTagInternal(ctx context.Context, imageRef string, policy types.Policy) (types.CheckResult, error) { +func (s *Service) selectImageTag(ctx context.Context, imageRef string, policy types.Policy) (types.CheckResult, error) { result := types.CheckResult{CurrentRef: imageRef, TargetRef: imageRef} if err := ctx.Err(); err != nil { return result, err @@ -121,7 +121,7 @@ func (s *Service) CheckContainerUpdate(ctx context.Context, containerID string) if result.ContainerID == "" { result.ContainerID = containerID } - reason, err := s.containerEligibilityInternal(ctx, inspect, enforceSettingsExclusions) + reason, err := s.containerEligibility(ctx, inspect, enforceSettingsExclusions) if err != nil { return result, err } @@ -129,14 +129,12 @@ func (s *Service) CheckContainerUpdate(ctx context.Context, containerID string) result.Reason = reason return result, nil } - imageRef := inspect.Config.Image - result, err = s.selectImageTagInternal(ctx, imageRef, s.config.LabelPolicy.TagPolicy(inspect.Config.Labels)) + result, err = s.selectImageTag(ctx, inspect.Config.Image, s.config.LabelPolicy.TagPolicy(inspect.Config.Labels)) result.ContainerID = inspect.ID if err != nil || result.Reason != "" || result.UpdateAvailable { return result, err } - // Compare the running image, even if the configured tag is already cached at - // a newer digest in the daemon. + // Compare the running image, even if the configured tag is cached at a newer digest. image, err := dockerClient.ImageInspect(ctx, inspect.Image) if err != nil { return result, err @@ -152,25 +150,22 @@ func (s *Service) CheckContainerUpdate(ctx context.Context, containerID string) if err != nil { return result, err } - result.CurrentDigest = image.ID + localDigests := digestcheck.RepoDigests(image.RepoDigests, result.CurrentRef) result.UpdateType = string(UpdateTypeDigest) - result.UpdateAvailable = true - for _, repoDigest := range image.RepoDigests { - if local, ok := digest.FromReferenceSuffix(repoDigest); ok { - result.CurrentDigest = local - if local == result.TargetDigest { - result.UpdateAvailable = false - break - } - } + result.UpdateAvailable = !slices.Contains(localDigests, result.TargetDigest) + result.CurrentDigest = image.ID + switch { + case !result.UpdateAvailable: + result.CurrentDigest = result.TargetDigest + case len(localDigests) > 0: + result.CurrentDigest = localDigests[0] } return result, nil } -// containerEligibilityInternal reports why a container cannot be updated, or -// "" when it can. The label, Swarm, and configuration checks always apply; -// policy decides whether the settings exclusion list is consulted too. -func (s *Service) containerEligibilityInternal(ctx context.Context, inspect container.InspectResponse, policy exclusionPolicy) (string, error) { +// containerEligibility reports why a container cannot be updated, or "" when it can; policy decides +// whether the settings exclusions apply on top of the label, Swarm, and config checks. +func (s *Service) containerEligibility(ctx context.Context, inspect container.InspectResponse, policy exclusionPolicy) (string, error) { if inspect.Config == nil { return "container config unavailable", nil } @@ -188,7 +183,7 @@ func (s *Service) containerEligibilityInternal(ctx context.Context, inspect cont if err != nil { return "", err } - if excluded[inspect.ID] || excluded[strings.TrimPrefix(inspect.Name, "/")] { + if isExcluded(excluded, inspect.ID, inspect.Name) { return "container excluded by settings", nil } return "", nil diff --git a/updater/compose_groups.go b/updater/compose_groups.go index 1ce0a8f..0ff1065 100644 --- a/updater/compose_groups.go +++ b/updater/compose_groups.go @@ -15,9 +15,9 @@ type composeGroup struct { err error projectName string services []string - seen map[string]struct{} } +// buildComposeGroups groups planned Compose containers by resolved project, flagging conflicting service targets. func (s *Service) buildComposeGroups(ctx context.Context, sorted []deps.ContainerWithDeps, plansByName map[string]*restartPlan) map[string]composeGroup { groups := map[string]composeGroup{} if s.config.ProjectUpdater == nil { @@ -26,15 +26,10 @@ func (s *Service) buildComposeGroups(ctx context.Context, sorted []deps.Containe for _, candidate := range sorted { plan := plansByName[candidate.Name] - if plan == nil || plan.newRef == "" || plan.inspect == nil || plan.inspect.Config == nil { + if plan == nil || plan.newRef == "" || plan.inspect == nil || plan.inspect.Config == nil || s.config.LabelPolicy.IsSelfUpdateTarget(plan.inspect.Config.Labels) { continue } - labels := plan.inspect.Config.Labels - if s.config.LabelPolicy.IsSelfUpdateTarget(labels) { - continue - } - projectName := compose.ProjectLabel(labels) - serviceName := compose.ServiceLabel(labels) + projectName, serviceName := compose.ProjectLabel(plan.inspect.Config.Labels), compose.ServiceLabel(plan.inspect.Config.Labels) if projectName == "" || serviceName == "" { continue } @@ -44,35 +39,20 @@ func (s *Service) buildComposeGroups(ctx context.Context, sorted []deps.Containe } group := groups[project.ID] group.projectName = projectName - if group.seen == nil { - group.seen = map[string]struct{}{} - } if group.images == nil { group.images = map[string]types.ServiceImageChange{} } change := types.ServiceImageChange{ExpectedRef: plan.inspect.Config.Image, TargetRef: plan.newRef} - if previous, ok := group.images[serviceName]; ok && previous != change { + previous, seen := group.images[serviceName] + if seen && previous != change { group.err = fmt.Errorf("conflicting targets for compose service %s/%s", projectName, serviceName) } - group.images[serviceName] = change - group.tagChanges = group.tagChanges || isComposeTagChangeInternal(plan) - if _, seen := group.seen[serviceName]; !seen { + if !seen { group.services = append(group.services, serviceName) - group.seen[serviceName] = struct{}{} } + group.images[serviceName] = change + group.tagChanges = group.tagChanges || isComposeTagChange(plan.inspect, plan.newRef) groups[project.ID] = group } return groups } - -func composeProjectID(projectName string, groups map[string]composeGroup) string { - if projectName == "" { - return "" - } - for projectID, group := range groups { - if group.projectName == projectName { - return projectID - } - } - return "" -} diff --git a/updater/compose_images.go b/updater/compose_images.go index 8d6f65b..66b98b8 100644 --- a/updater/compose_images.go +++ b/updater/compose_images.go @@ -15,102 +15,79 @@ import ( updatetypes "go.getarcane.app/updater/types" ) -func (s *Service) preflightComposeImageInternal(ctx context.Context, cnt container.Summary, inspect container.InspectResponse, newRef string) error { +// preflightComposeImage resolves the Compose project a tag change to newRef must be persisted +// through, and returns "" when the change needs none. +func (s *Service) preflightComposeImage(ctx context.Context, containerID string, inspect container.InspectResponse, newRef string) (string, error) { labels := labelsFromInspect(inspect) - if s.isSelfUpdateCandidate(cnt.ID, labels) { - return nil - } - if inspect.Config != nil && refs.NormalizeImageUpdateRef(inspect.Config.Image) == refs.NormalizeImageUpdateRef(newRef) { - return nil + if s.isSelfUpdateCandidate(containerID, labels) || !isComposeTagChange(&inspect, newRef) { + return "", nil } projectName, serviceName := compose.ProjectLabel(labels), compose.ServiceLabel(labels) - if projectName == "" && serviceName == "" { - projectName, serviceName = compose.ProjectLabel(cnt.Labels), compose.ServiceLabel(cnt.Labels) - } - if projectName == "" && serviceName == "" { - return nil - } if projectName == "" || serviceName == "" { - return errors.New("compose tag update requires project and service labels") + return "", errors.New("compose tag update requires project and service labels") } if _, ok := s.config.ProjectUpdater.(updatetypes.ProjectImageUpdater); !ok { - return fmt.Errorf("compose project %s requires a ProjectImageUpdater adapter for tag updates", projectName) + return "", fmt.Errorf("compose project %s requires a ProjectImageUpdater adapter for tag updates", projectName) } project, err := s.config.ProjectUpdater.ProjectByComposeName(ctx, projectName) if err != nil { - return fmt.Errorf("resolve compose project %s for tag update: %w", projectName, err) + return "", fmt.Errorf("resolve compose project %s for tag update: %w", projectName, err) } if strings.TrimSpace(project.ID) == "" { - return fmt.Errorf("compose project %s has no project ID for tag update", projectName) + return "", fmt.Errorf("compose project %s has no project ID for tag update", projectName) } - return nil + return project.ID, nil } -func (s *Service) updateComposeImageInternal(ctx context.Context, target container.Summary, inspect container.InspectResponse, newRef string) error { - if err := s.preflightComposeImageInternal(ctx, target, inspect, newRef); err != nil { - return err - } - labels := labelsFromInspect(inspect) - projectName, serviceName := compose.ProjectLabel(labels), compose.ServiceLabel(labels) - if projectName == "" && serviceName == "" { - projectName, serviceName = compose.ProjectLabel(target.Labels), compose.ServiceLabel(target.Labels) - } - adapter, ok := s.config.ProjectUpdater.(updatetypes.ProjectImageUpdater) - if !ok || projectName == "" || serviceName == "" || inspect.Config == nil { - return errors.New("compose tag update requires an image updater and complete container configuration") - } - project, err := s.config.ProjectUpdater.ProjectByComposeName(ctx, projectName) - if err != nil { - return fmt.Errorf("resolve compose project %s for tag update: %w", projectName, err) - } - if strings.TrimSpace(project.ID) == "" { - return fmt.Errorf("compose project %s has no project ID for tag update", projectName) - } - opCtx, cancel := s.opCtx(ctx) - defer cancel() - err = adapter.UpdateServiceImages(opCtx, project.ID, map[string]updatetypes.ServiceImageChange{ - serviceName: {ExpectedRef: inspect.Config.Image, TargetRef: newRef}, - }) - if err != nil { - return fmt.Errorf("update compose service %s/%s image: %w", projectName, serviceName, err) - } - dockerClient, err := s.dockerClient(ctx) - if err != nil { - return err - } - return verifyComposeTargetInternal(ctx, dockerClient, projectName, serviceName, newRef) +// isComposeTagChange reports whether recreating a Compose container as newRef changes its configured image, +// which only a ProjectImageUpdater can persist. +func isComposeTagChange(inspect *container.InspectResponse, newRef string) bool { + return inspect != nil && inspect.Config != nil && + refs.NormalizeImageUpdateRef(inspect.Config.Image) != refs.NormalizeImageUpdateRef(newRef) && + (compose.ProjectLabel(inspect.Config.Labels) != "" || compose.ServiceLabel(inspect.Config.Labels) != "") } -func verifyComposeTargetInternal(ctx context.Context, dockerClient *client.Client, projectName, serviceName, newRef string) error { - normalizedRef := refs.NormalizeImageUpdateRef(newRef) - if dockerClient == nil || strings.TrimSpace(projectName) == "" || strings.TrimSpace(serviceName) == "" || normalizedRef == "" { - return errors.New("verify compose target requires a Docker client, project, service, and valid target reference") - } - image, err := dockerClient.ImageInspect(ctx, newRef) - if err != nil { - return fmt.Errorf("verify compose target: inspect target image: %w", err) +// verifyComposeService checks that a service's running containers run targetRef or, without a target, +// no longer run oldImageID. +func verifyComposeService(ctx context.Context, dockerClient *client.Client, projectName, serviceName, oldImageID, targetRef string) error { + if targetRef == "" && oldImageID == "" { + return nil } - if strings.TrimSpace(image.ID) == "" { - return errors.New("verify compose target: target image has no image ID") + targetID := "" + if targetRef != "" { + image, err := dockerClient.ImageInspect(ctx, targetRef) + if err != nil { + return fmt.Errorf("verify compose service: inspect target image: %w", err) + } + if targetID = strings.TrimSpace(image.ID); targetID == "" { + return errors.New("verify compose service: target image has no image ID") + } } - filters := make(client.Filters) - filters = filters.Add("label", compose.ProjectLabelKey+"="+projectName) - filters = filters.Add("label", compose.ServiceLabelKey+"="+serviceName) - containers, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{All: false, Filters: filters}) + filters := make(client.Filters).Add("label", compose.ProjectLabelKey+"="+projectName, compose.ServiceLabelKey+"="+serviceName, compose.ServiceContainerFilter) + containers, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{Filters: filters}) if err != nil { - return fmt.Errorf("verify compose target: list service containers: %w", err) + return fmt.Errorf("verify compose service: list service containers: %w", err) } if len(containers.Items) == 0 { return fmt.Errorf("compose service %s/%s has no running container after update", projectName, serviceName) } for _, cnt := range containers.Items { - inspected, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cnt.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { - return fmt.Errorf("verify compose target: inspect container %s: %w", cnt.ID, inspectErr) + currentImageID := strings.TrimSpace(cnt.ImageID) + var current container.InspectResponse + if targetID != "" || currentImageID == "" { + inspected, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cnt.ID, client.ContainerInspectOptions{}) + if inspectErr != nil { + return fmt.Errorf("verify compose service: inspect container %s: %w", cnt.ID, inspectErr) + } + current = inspected.Container + currentImageID = strings.TrimSpace(current.Image) } - current := inspected.Container - if current.Config == nil || refs.NormalizeImageUpdateRef(current.Config.Image) != normalizedRef || current.Image != image.ID || current.State == nil || !current.State.Running { - return fmt.Errorf("compose service %s/%s container %s does not run target %s (%s)", projectName, serviceName, cnt.ID, newRef, image.ID) + switch { + case targetID == "" && currentImageID == oldImageID: + return fmt.Errorf("compose service %s/%s still running old image %s after update", projectName, serviceName, oldImageID) + case targetID != "" && (current.Config == nil || refs.NormalizeImageUpdateRef(current.Config.Image) != refs.NormalizeImageUpdateRef(targetRef) || + currentImageID != targetID || current.State == nil || !current.State.Running): + return fmt.Errorf("compose service %s/%s container %s does not run target %s (%s)", projectName, serviceName, cnt.ID, targetRef, targetID) } } return nil diff --git a/updater/compose_images_test.go b/updater/compose_images_test.go index 2230d25..6b5ac7f 100644 --- a/updater/compose_images_test.go +++ b/updater/compose_images_test.go @@ -14,19 +14,19 @@ import ( updatetypes "go.getarcane.app/updater/types" ) -type recordingProjectImagesInternal struct { +type recordingProjectImages struct { fakeProjectUpdater changes map[string]updatetypes.ServiceImageChange projectID string imageErr error } -func (f *recordingProjectImagesInternal) UpdateServiceImages(_ context.Context, projectID string, changes map[string]updatetypes.ServiceImageChange) error { +func (f *recordingProjectImages) UpdateServiceImages(_ context.Context, projectID string, changes map[string]updatetypes.ServiceImageChange) error { f.projectID, f.changes = projectID, changes return f.imageErr } -func TestPreflightComposeImageInternal(t *testing.T) { +func TestPreflightComposeImage(t *testing.T) { for _, tt := range []struct { name string adapter ProjectUpdater @@ -40,19 +40,19 @@ func TestPreflightComposeImageInternal(t *testing.T) { {name: "missing service", labels: map[string]string{compose.ProjectLabelKey: "app"}, newRef: "app:2", wantError: true}, {name: "self update exempt", labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}, selfID: "self", newRef: "app:2"}, {name: "digest exempt", labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}, newRef: "docker.io/library/app:1"}, - {name: "unresolved", labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}, adapter: &recordingProjectImagesInternal{}, newRef: "app:2", wantError: true}, + {name: "unresolved", labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}, adapter: &recordingProjectImages{}, newRef: "app:2", wantError: true}, { name: "empty project ID", labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}, - adapter: &recordingProjectImagesInternal{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {}}}}, newRef: "app:2", wantError: true, + adapter: &recordingProjectImages{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {}}}}, newRef: "app:2", wantError: true, }, { name: "supported", labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}, - adapter: &recordingProjectImagesInternal{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}}, newRef: "app:2", + adapter: &recordingProjectImages{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}}, newRef: "app:2", }, } { t.Run(tt.name, func(t *testing.T) { service := newService(Config{ProjectUpdater: tt.adapter, SelfContainerID: tt.selfID}) - err := service.preflightComposeImageInternal(t.Context(), container.Summary{ID: "self"}, container.InspectResponse{Config: &container.Config{Image: "app:1", Labels: tt.labels}}, tt.newRef) + _, err := service.preflightComposeImage(t.Context(), "self", container.InspectResponse{Config: &container.Config{Image: "app:1", Labels: tt.labels}}, tt.newRef) if (err != nil) != tt.wantError { t.Fatalf("error = %v, wantError %v", err, tt.wantError) } @@ -60,14 +60,14 @@ func TestPreflightComposeImageInternal(t *testing.T) { } } -func TestUpdateComposeImageInternal(t *testing.T) { +func TestUpdateComposeImage(t *testing.T) { for _, failed := range []bool{false, true} { name := "persists and verifies" if failed { name = "adapter failure" } t.Run(name, func(t *testing.T) { - adapter := &recordingProjectImagesInternal{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}} + adapter := &recordingProjectImages{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}} if failed { adapter.imageErr = errors.New("persist failed") } @@ -89,7 +89,7 @@ func TestUpdateComposeImageInternal(t *testing.T) { Image: "sha256:same", Config: &container.Config{Image: "app:1", Labels: map[string]string{compose.ProjectLabelKey: "app", compose.ServiceLabelKey: "web"}}, } - err := service.updateComposeImageInternal(t.Context(), container.Summary{ID: "old"}, inspect, "app:2") + err := service.updateComposeOrStandalone(t.Context(), container.Summary{ID: "old"}, inspect, "app:2") if (err != nil) != failed { t.Fatalf("error = %v", err) } @@ -100,7 +100,7 @@ func TestUpdateComposeImageInternal(t *testing.T) { } } -func TestVerifyComposeTargetInternal(t *testing.T) { +func TestVerifyComposeService(t *testing.T) { for _, tt := range []struct { name, configuredRef, imageID string running, empty, wantError bool @@ -134,7 +134,7 @@ func TestVerifyComposeTargetInternal(t *testing.T) { http.NotFound(w, r) } }) - err := verifyComposeTargetInternal(t.Context(), dockerClient, "app", "web", "app:2") + err := verifyComposeService(t.Context(), dockerClient, "app", "web", "", "app:2") if (err != nil) != tt.wantError { t.Fatalf("error = %v, wantError %v", err, tt.wantError) } diff --git a/updater/compose_updater.go b/updater/compose_updater.go index ebae548..e5a5cac 100644 --- a/updater/compose_updater.go +++ b/updater/compose_updater.go @@ -1,9 +1,11 @@ package updater import ( + "cmp" "context" "errors" "fmt" + "os" "os/exec" "strings" @@ -21,14 +23,13 @@ type dockerComposeProjectMetadata struct { projectName string workingDir string configFiles []string + envFiles []string + dockerHost string } // NewDockerComposeProjectUpdater returns a Docker Compose CLI project updater. func NewDockerComposeProjectUpdater(provider DockerClientProvider) ProjectUpdater { - if provider == nil { - provider = NewDockerClientProvider() - } - return dockerComposeProjectUpdater{dockerClientProvider: provider} + return dockerComposeProjectUpdater{dockerClientProvider: cmp.Or[DockerClientProvider](provider, NewDockerClientProvider())} } func (u dockerComposeProjectUpdater) ProjectByComposeName(ctx context.Context, composeName string) (ComposeProject, error) { @@ -39,6 +40,8 @@ func (u dockerComposeProjectUpdater) ProjectByComposeName(ctx context.Context, c return ComposeProject{ID: metadata.projectName, Name: metadata.projectName}, nil } +// UpdateServices recreates services with the project directory, config and env files Compose recorded, +// against the same engine the provider uses. func (u dockerComposeProjectUpdater) UpdateServices(ctx context.Context, projectID string, services []string) error { metadata, err := u.resolveProjectMetadata(ctx, projectID) if err != nil { @@ -50,23 +53,36 @@ func (u dockerComposeProjectUpdater) UpdateServices(ctx context.Context, project } args := []string{"compose", "-p", metadata.projectName} + if metadata.workingDir != "" { + args = append(args, "--project-directory", metadata.workingDir) + } for _, configFile := range metadata.configFiles { args = append(args, "-f", configFile) } + for _, envFile := range metadata.envFiles { + args = append(args, "--env-file", envFile) + } args = append(args, "up", "-d", "--no-deps", "--force-recreate") args = append(args, services...) cmd := exec.CommandContext(ctx, "docker", args...) - if metadata.workingDir != "" { - cmd.Dir = metadata.workingDir + cmd.Dir = metadata.workingDir + if metadata.dockerHost != "" { + cmd.Env = append(os.Environ(), "DOCKER_HOST="+metadata.dockerHost) } output, err := cmd.CombinedOutput() if err != nil { - return fmt.Errorf("docker compose update failed: %w: %s", err, truncateComposeOutput(strings.TrimSpace(string(output)))) + const maxComposeErrorOutput = 4096 + message := strings.TrimSpace(string(output)) + if len(message) > maxComposeErrorOutput { + message = message[:maxComposeErrorOutput] + " (truncated)" + } + return fmt.Errorf("docker compose update failed: %w: %s", err, message) } return nil } +// resolveProjectMetadata reads a project's Compose labels from one of its service containers. func (u dockerComposeProjectUpdater) resolveProjectMetadata(ctx context.Context, composeName string) (dockerComposeProjectMetadata, error) { composeName = strings.TrimSpace(composeName) if composeName == "" { @@ -78,8 +94,7 @@ func (u dockerComposeProjectUpdater) resolveProjectMetadata(ctx context.Context, return dockerComposeProjectMetadata{}, fmt.Errorf("docker connect: %w", err) } - filters := make(client.Filters) - filters = filters.Add("label", compose.ProjectLabelKey+"="+composeName) + filters := make(client.Filters).Add("label", compose.ProjectLabelKey+"="+composeName, compose.ServiceContainerFilter) containers, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{All: true, Filters: filters}) if err != nil { return dockerComposeProjectMetadata{}, fmt.Errorf("list compose containers: %w", err) @@ -87,25 +102,12 @@ func (u dockerComposeProjectUpdater) resolveProjectMetadata(ctx context.Context, if len(containers.Items) == 0 { return dockerComposeProjectMetadata{}, fmt.Errorf("compose project not found: %s", composeName) } - - for _, summary := range containers.Items { - if compose.ProjectLabel(summary.Labels) != composeName { - continue - } - return dockerComposeProjectMetadata{ - projectName: composeName, - workingDir: compose.WorkingDirLabel(summary.Labels), - configFiles: compose.ConfigFilesLabel(summary.Labels), - }, nil - } - return dockerComposeProjectMetadata{}, fmt.Errorf("compose project not found: %s", composeName) -} - -func truncateComposeOutput(output string) string { - const maxComposeErrorOutput = 4096 - output = strings.TrimSpace(output) - if len(output) <= maxComposeErrorOutput { - return output - } - return output[:maxComposeErrorOutput] + " (truncated)" + labels := containers.Items[0].Labels + return dockerComposeProjectMetadata{ + projectName: composeName, + workingDir: strings.TrimSpace(labels[compose.WorkingDirLabelKey]), + configFiles: kit.TrimNonEmpty(strings.Split(labels[compose.ConfigFilesLabelKey], ",")), + envFiles: kit.TrimNonEmpty(strings.Split(labels[compose.EnvironmentFileLabelKey], ",")), + dockerHost: dockerClient.DaemonHost(), + }, nil } diff --git a/updater/config.go b/updater/config.go index 268b100..36b17aa 100644 --- a/updater/config.go +++ b/updater/config.go @@ -7,13 +7,11 @@ import ( "go.getarcane.app/updater/types" ) -// Config configures a Service. Every field is optional: New fills in a -// Docker-backed default for each port that has one, and leaves the rest nil — -// a nil optional port simply disables the behavior it provides. +// Config configures a Service. Every field is optional: New fills in a Docker-backed default for +// each port that has one, and a nil optional port disables the behavior it provides. type Config struct { - // DockerClientProvider supplies the Docker client every operation uses. - // Defaults to NewDockerClientProvider(), which reads the standard Docker - // environment variables. + // DockerClientProvider supplies the Docker client every operation uses. Defaults to + // NewDockerClientProvider(), which reads the standard Docker environment variables. DockerClientProvider DockerClientProvider // ImagePuller pulls images. Defaults to NewImagePuller(DockerClientProvider). ImagePuller ImagePuller @@ -26,8 +24,7 @@ type Config struct { // RegistryTagLister discovers version tags. Defaults to Docker-config credentials. RegistryTagLister types.RegistryTagLister // ProjectUpdater updates Docker Compose services. Defaults to - // NewDockerComposeProjectUpdater(DockerClientProvider), which shells out to - // the docker compose CLI. + // NewDockerComposeProjectUpdater(DockerClientProvider), which runs the docker compose CLI. ProjectUpdater ProjectUpdater // LabelPolicy decides which containers the updater may touch. Each nil func // is filled in from DefaultLabelPolicy. @@ -48,10 +45,8 @@ type Config struct { // images by listing running containers. UsedImageCollector UsedImageCollector - // SelfContainerID is the ID (or ID prefix) of the container the host - // application itself runs in. When set, that container is always routed - // through the SelfUpdater even if its labels do not mark it as a - // self-update target. Optional. + // SelfContainerID is the ID (or ID prefix) of the container the host application runs in. + // When set, that container always goes through the SelfUpdater, whatever its labels. SelfContainerID string // OperationTimeout optionally bounds individual Docker mutation operations // and compose project updates. Zero leaves caller context deadlines unchanged. diff --git a/updater/container.go b/updater/container.go index 4ccb169..ff35e8b 100644 --- a/updater/container.go +++ b/updater/container.go @@ -1,22 +1,31 @@ package updater import ( + "cmp" "context" "errors" "fmt" "io" "maps" + "path" "slices" "strings" + cerrdefs "github.com/containerd/errdefs" + "github.com/containerd/platforms" + dockerspec "github.com/moby/docker-image-spec/specs-go/v1" "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/image" + "github.com/moby/moby/api/types/mount" "github.com/moby/moby/api/types/network" "github.com/moby/moby/client" "go.getarcane.app/docker/compat" + kit "go.getarcane.app/kit/pkg" "go.getarcane.app/updater/internal/compose" "go.getarcane.app/updater/internal/digestcheck" "go.getarcane.app/updater/refs" + updatetypes "go.getarcane.app/updater/types" ) // UpdateContainer updates a single Docker container by pulling its latest image and recreating it. @@ -26,11 +35,9 @@ func (s *Service) UpdateContainer(ctx context.Context, containerID string, opts dockerClient, err := s.dockerClient(ctx) if err != nil { - return nil, fmt.Errorf("docker connect: %w", err) + return nil, err } - - filters := make(client.Filters) - filters = filters.Add("id", strings.TrimSpace(containerID)) + filters := make(client.Filters).Add("id", strings.TrimSpace(containerID)) containerList, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{All: true, Filters: filters}) if err != nil { return nil, fmt.Errorf("list containers: %w", err) @@ -43,153 +50,103 @@ func (s *Service) UpdateContainer(ctx context.Context, containerID string, opts name := containerSummaryName(target) inspectResult, err := compat.ContainerInspectWithCompatibility(ctx, dockerClient, target.ID, client.ContainerInspectOptions{}) if err != nil { - item := failedContainerResult(target.ID, name, fmt.Sprintf("inspect failed: %v", err)) - out.Items = append(out.Items, item) - out.Failed++ + appendResult(out, failedContainerResult(target.ID, name, fmt.Sprintf("inspect failed: %v", err))) return out, nil } inspect := inspectResult.Container labels := labelsFromInspect(inspect) + defer s.BeginContainerUpdate(target.ID)() + defer s.BeginProjectUpdate(compose.ProjectLabel(labels))() - endContainerStatus := s.BeginContainerUpdate(target.ID) - defer endContainerStatus() - endProjectStatus := s.BeginProjectUpdate(compose.ProjectLabel(labels)) - defer endProjectStatus() - - reason, eligibilityErr := s.containerEligibilityInternal(ctx, inspect, opts.targetExclusionPolicyInternal()) - if eligibilityErr != nil { - return out, eligibilityErr + reason, err := s.containerEligibility(ctx, inspect, exclusionPolicy(!opts.IgnoreSettingsExclusions)) + if err != nil { + return out, err } - if reason != "" { - out.Items = append(out.Items, skippedContainerResult(target.ID, name, reason)) - out.Checked = 1 - out.Skipped++ - return out, nil + if reason == "" && (refs.IsDigestPinnedReference(inspect.Config.Image) || refs.IsImageIDLikeReference(inspect.Config.Image)) { + reason = "immutable image reference" } - configImageRef := "" - if inspect.Config != nil { - configImageRef = strings.TrimSpace(inspect.Config.Image) - } - if refs.IsDigestPinnedReference(configImageRef) || refs.IsImageIDLikeReference(configImageRef) { - out.Items = append(out.Items, skippedContainerResult(target.ID, name, "immutable image reference")) - out.Checked = 1 - out.Skipped++ + if reason != "" { + appendResult(out, skippedContainerResult(target.ID, name, reason)) return out, nil } - imageRef := refs.PullableImageRef(target.Image, configImageRef, nil) + imageRef := refs.PullableImageRef(target.Image, inspect.Config.Image, nil) if imageRef == "" && inspect.Image != "" { if imageInspect, inspectErr := dockerClient.ImageInspect(ctx, inspect.Image); inspectErr == nil { - imageRef = refs.PullableImageRef(target.Image, configImageRef, imageInspect.RepoTags) + imageRef = refs.PullableImageRef(target.Image, inspect.Config.Image, imageInspect.RepoTags) } } normalizedRef := refs.NormalizeImageUpdateRef(imageRef) if normalizedRef == "" { - item := skippedContainerResult(target.ID, name, "unable to resolve a pullable image reference for container") - out.Items = append(out.Items, item) - out.Skipped++ + appendResult(out, skippedContainerResult(target.ID, name, "unable to resolve a pullable image reference for container")) return out, nil } - selection, selectionErr := s.selectImageTagInternal(ctx, normalizedRef, s.config.LabelPolicy.TagPolicy(labels)) + selection, selectionErr := s.selectImageTag(ctx, normalizedRef, s.config.LabelPolicy.TagPolicy(labels)) if selectionErr != nil { - out.Items = append(out.Items, failedContainerResult(target.ID, name, selectionErr.Error())) - out.Failed++ - out.Checked = 1 + appendResult(out, failedContainerResult(target.ID, name, selectionErr.Error())) return out, nil } tagChanged := selection.UpdateAvailable && selection.UpdateType == string(UpdateTypeTag) normalizedRef = selection.TargetRef - if preflightErr := s.preflightComposeImageInternal(ctx, target, inspect, normalizedRef); preflightErr != nil { - out.Items = append(out.Items, failedContainerResult(target.ID, name, preflightErr.Error())) - out.Failed++ - out.Checked = 1 + if _, preflightErr := s.preflightComposeImage(ctx, target.ID, inspect, normalizedRef); preflightErr != nil { + appendResult(out, failedContainerResult(target.ID, name, preflightErr.Error())) return out, nil } if opts.DryRun { - item := ResourceResult{ + appendResult(out, ResourceResult{ ResourceID: target.ID, ResourceName: name, ResourceType: ResourceTypeContainer, Status: StatusSkipped, UpdateAvailable: selection.UpdateAvailable, OldImage: imageRef, NewImage: normalizedRef, - } - out.Items = append(out.Items, item) - out.Checked = 1 - out.Skipped++ + }) return out, nil } if s.config.ImagePuller == nil { - item := failedContainerResult(target.ID, name, ErrImagePullerRequired.Error()) - out.Items = append(out.Items, item) - out.Failed++ + appendResult(out, failedContainerResult(target.ID, name, ErrImagePullerRequired.Error())) return out, nil } if pullErr := s.config.ImagePuller.PullImage(ctx, normalizedRef, io.Discard); pullErr != nil { - item := failedContainerResult(target.ID, name, fmt.Sprintf("pull failed: %v", pullErr)) - out.Items = append(out.Items, item) - out.Failed++ + appendResult(out, failedContainerResult(target.ID, name, fmt.Sprintf("pull failed: %v", pullErr))) return out, nil } changed, compareErr := digestcheck.NewChecker(dockerClient, nil).CompareWithPulled(ctx, inspect.Image, normalizedRef) if compareErr != nil && tagChanged { - out.Items = append(out.Items, failedContainerResult(target.ID, name, compareErr.Error())) - out.Checked = 1 - out.Failed++ + appendResult(out, failedContainerResult(target.ID, name, compareErr.Error())) return out, nil } if compareErr == nil && !changed && !opts.Force && !tagChanged { - item := skippedContainerResult(target.ID, name, "image digest unchanged after pull") - out.Items = append(out.Items, item) - out.Checked = 1 - out.Skipped++ - s.clearPendingRecordInternal(ctx, target.ID, normalizedRef) + appendResult(out, skippedContainerResult(target.ID, name, "image digest unchanged after pull")) + s.clearPendingRecord(ctx, target.ID, normalizedRef) return out, nil } - if s.isSelfUpdateCandidate(target.ID, labels) { - if selfUpdateErr := s.triggerSelfUpdate(ctx, target.ID, name, normalizedRef, labels); selfUpdateErr != nil { - item := failedContainerResult(target.ID, name, selfUpdateErr.Error()) - out.Items = append(out.Items, item) - out.Failed++ - return out, nil - } - item := updatedContainerResult(target.ID, name, inspect.Image, normalizedRef) - out.Items = append(out.Items, item) - out.Checked = 1 - out.Updated++ - s.clearPendingRecordInternal(ctx, target.ID, normalizedRef) + selfUpdate := s.isSelfUpdateCandidate(target.ID, labels) + var updateErr error + if selfUpdate { + updateErr = s.triggerSelfUpdate(ctx, target.ID, name, normalizedRef, labels) + } else { + updateErr = s.updateComposeOrStandalone(ctx, target, inspect, normalizedRef) + } + if updateErr != nil { + appendResult(out, failedContainerResult(target.ID, name, updateErr.Error())) return out, nil } - - if updateErr := s.updateComposeOrStandalone(ctx, target, inspect, normalizedRef); updateErr != nil { - item := failedContainerResult(target.ID, name, updateErr.Error()) - out.Items = append(out.Items, item) - out.Failed++ - } else { - item := updatedContainerResult(target.ID, name, inspect.Image, normalizedRef) - out.Items = append(out.Items, item) - out.Updated++ + appendResult(out, ResourceResult{ + ResourceID: target.ID, ResourceName: name, ResourceType: ResourceTypeContainer, Status: StatusUpdated, + UpdateAvailable: true, UpdateApplied: true, OldImage: inspect.Image, NewImage: normalizedRef, + }) + if !selfUpdate { _ = s.notify(ctx, target.ID, name, imageRef, inspect.Image, normalizedRef) - s.clearPendingRecordInternal(ctx, target.ID, normalizedRef) } - out.Checked = 1 + s.clearPendingRecord(ctx, target.ID, normalizedRef) return out, nil } -// clearPendingRecord clears the pending update record for an applied -// image ref through the same PendingStore seam ApplyPending uses — a -// single-container update must not leave its "update available" record -// behind, or the update keeps counting as pending. Only records whose -// NewImageRef matches the applied ref are cleared: a tag-update record -// (e.g. nginx:1.27 -> 1.28) stays pending when only the old tag was -// re-pulled. The records are looked up -// and cleared as stored (ID and all) rather than reconstructed, because -// stores key by ID when one is present. -func (s *Service) clearPendingRecordInternal(ctx context.Context, containerID, imageRef string) { - if s.config.PendingStore == nil { - return - } +// clearPendingRecord clears the stored records an applied update satisfied, matching NewImageRef so a +// re-pulled old tag leaves its tag update pending. Records are cleared as stored, since stores key by ID. +func (s *Service) clearPendingRecord(ctx context.Context, containerID, imageRef string) { normalized := refs.NormalizeImageUpdateRef(imageRef) - if normalized == "" { + if s.config.PendingStore == nil || normalized == "" { return } records, err := s.config.PendingStore.PendingImageUpdates(ctx) @@ -198,74 +155,104 @@ func (s *Service) clearPendingRecordInternal(ctx context.Context, containerID, i return } for _, record := range records { - if record.ContainerID != "" && record.ContainerID != containerID { + if (record.ContainerID != "" && record.ContainerID != containerID) || + (record.IsTagUpdate() && record.ContainerID == "") || + refs.NormalizeImageUpdateRef(record.NewImageRef()) != normalized { continue } - if record.IsTagUpdate() && record.ContainerID == "" { - continue - } - if refs.NormalizeImageUpdateRef(record.NewImageRef()) != normalized { - continue - } - err = s.config.PendingStore.ClearImageUpdateRecord(ctx, record) - if err != nil { + if err = s.config.PendingStore.ClearImageUpdateRecord(ctx, record); err != nil { s.logger.WarnContext(ctx, "failed to clear applied update record", "imageRef", imageRef, "error", err) } } } -// updateStandaloneContainer recreates a non-compose container with newRef. -func (s *Service) updateStandaloneContainer(ctx context.Context, cnt container.Summary, inspect container.InspectResponse, newRef string) error { +// updateComposeOrStandalone applies newRef through the container's Compose project when it resolves, +// and recreates the container standalone otherwise. +func (s *Service) updateComposeOrStandalone(ctx context.Context, target container.Summary, inspect container.InspectResponse, newRef string) error { labels := labelsFromInspect(inspect) - if err := s.validateStandaloneContainerUpdate(labels); err != nil { - return err + if s.isSelfUpdateCandidate(target.ID, labels) { + return ErrSelfUpdateContainer } - - dockerClient, err := s.dockerClient(ctx) - if err != nil { - return fmt.Errorf("docker connect: %w", err) + projectName, serviceName := compose.ProjectLabel(labels), compose.ServiceLabel(labels) + if isComposeTagChange(&inspect, newRef) { + projectID, err := s.preflightComposeImage(ctx, target.ID, inspect, newRef) + if err != nil { + return err + } + adapter, _ := s.config.ProjectUpdater.(updatetypes.ProjectImageUpdater) + opCtx, cancel := s.opCtx(ctx) + err = adapter.UpdateServiceImages(opCtx, projectID, map[string]updatetypes.ServiceImageChange{ + serviceName: {ExpectedRef: inspect.Config.Image, TargetRef: newRef}, + }) + cancel() + if err != nil { + return fmt.Errorf("update compose service %s/%s image: %w", projectName, serviceName, err) + } + dockerClient, err := s.dockerClient(ctx) + if err != nil { + return err + } + return verifyComposeService(ctx, dockerClient, projectName, serviceName, "", newRef) + } + if projectName != "" && serviceName != "" && s.config.ProjectUpdater != nil { + project, err := s.config.ProjectUpdater.ProjectByComposeName(ctx, projectName) + if err == nil { + // Never fall back once Compose owns the update, or a standalone recreate could clobber a partial up. + opCtx, cancel := s.opCtx(ctx) + defer cancel() + return s.config.ProjectUpdater.UpdateServices(opCtx, project.ID, []string{serviceName}) + } + s.logger.WarnContext(ctx, "compose project not resolved; falling back to standalone container update", + "container", containerSummaryName(target), + "project", projectName, + "service", serviceName, + "error", err, + ) } - err = s.stopAndRemoveStandaloneContainer(ctx, dockerClient, cnt, inspect) + dockerClient, err := s.dockerClient(ctx) if err != nil { return err } - return s.createStartOrRollback(ctx, dockerClient, cnt, inspect, newRef) + if stopErr := s.stopAndRemoveStandaloneContainer(ctx, dockerClient, target, inspect); stopErr != nil { + return stopErr + } + return s.createStartOrRollback(ctx, dockerClient, target, inspect, newRef) } -// createStartOrRollback recreates the container as newRef, spelled like its -// configured image so a short Compose or run reference is not qualified. +// createStartOrRollback recreates the container as newRef, spelled like its configured image, and +// recreates it from its old image ID when that fails. func (s *Service) createStartOrRollback(ctx context.Context, dockerClient *client.Client, cnt container.Summary, inspect container.InspectResponse, newRef string) error { if inspect.Config != nil { newRef = refs.PreserveConfiguredRef(inspect.Config.Image, newRef) } + name := containerSummaryName(cnt) createdID, err := s.createAndStartStandaloneContainer(ctx, dockerClient, cnt, inspect, newRef) if err == nil { return nil } - s.removeFailedCreatedContainer(ctx, dockerClient, createdID, containerSummaryName(cnt)) - if rollbackErr := s.rollbackStandaloneContainer(ctx, dockerClient, cnt, inspect); rollbackErr != nil { + s.removeFailedCreatedContainer(ctx, dockerClient, createdID, name) + + rollbackID, rollbackErr := "", errors.New("old image ID unavailable") + if strings.TrimSpace(inspect.Image) != "" { + rollbackID, rollbackErr = s.createAndStartStandaloneContainer(ctx, dockerClient, cnt, inspect, inspect.Image) + } + if rollbackErr != nil { + s.removeFailedCreatedContainer(ctx, dockerClient, rollbackID, name) return fmt.Errorf("%w; rollback failed: %w", err, rollbackErr) } + _ = s.recordEvent(ctx, "container_rollback", rollbackID, name, map[string]any{ + "action": "updater_rollback", + "oldContainerID": cnt.ID, + "rollbackImage": inspect.Image, + }) return fmt.Errorf("%w; rollback succeeded", err) } -func (s *Service) validateStandaloneContainerUpdate(labels map[string]string) error { - if s.config.LabelPolicy.IsSelfUpdateTarget(labels) { - return ErrSelfUpdateContainer - } - return nil -} - func (s *Service) stopAndRemoveStandaloneContainer(ctx context.Context, dockerClient *client.Client, cnt container.Summary, inspect container.InspectResponse) error { - labels := labelsFromInspect(inspect) name := containerSummaryName(cnt) - stopOptions := client.ContainerStopOptions{} - if signal := s.config.LabelPolicy.StopSignal(labels); signal != "" { - stopOptions.Signal = signal - } stopCtx, cancelStop := s.opCtx(ctx) - _, err := dockerClient.ContainerStop(stopCtx, cnt.ID, stopOptions) + _, err := dockerClient.ContainerStop(stopCtx, cnt.ID, client.ContainerStopOptions{Signal: s.config.LabelPolicy.StopSignal(labelsFromInspect(inspect))}) cancelStop() if err != nil { return fmt.Errorf("stop: %w", err) @@ -273,39 +260,78 @@ func (s *Service) stopAndRemoveStandaloneContainer(ctx context.Context, dockerCl _ = s.recordEvent(ctx, "container_stop", cnt.ID, name, map[string]any{"action": "updater_stop"}) removeCtx, cancelRemove := s.opCtx(ctx) - _, err = dockerClient.ContainerRemove(removeCtx, cnt.ID, client.ContainerRemoveOptions{}) - cancelRemove() - if err != nil { + defer cancelRemove() + if inspect.HostConfig != nil && inspect.HostConfig.AutoRemove { + // The daemon removes --rm containers once stopped; wait so the name is free for the recreate. + wait := dockerClient.ContainerWait(removeCtx, cnt.ID, client.ContainerWaitOptions{Condition: container.WaitConditionRemoved}) + select { + case <-wait.Result: + case err = <-wait.Error: + } + } else { + _, err = dockerClient.ContainerRemove(removeCtx, cnt.ID, client.ContainerRemoveOptions{}) + } + if err != nil && !cerrdefs.IsNotFound(err) { return fmt.Errorf("remove: %w", err) } _ = s.recordEvent(ctx, "container_delete", cnt.ID, name, map[string]any{"action": "updater_delete"}) return nil } -func (s *Service) createAndStartStandaloneContainer(ctx context.Context, dockerClient *client.Client, cnt container.Summary, inspect container.InspectResponse, newRef string) (string, error) { +// createAndStartStandaloneContainer recreates inspect's container from imageRef, starting it only if the old one ran. +func (s *Service) createAndStartStandaloneContainer(ctx context.Context, dockerClient *client.Client, cnt container.Summary, inspect container.InspectResponse, imageRef string) (string, error) { name := containerSummaryName(cnt) - cfg := inspect.Config - if cfg == nil { - cfg = &container.Config{} - } - cfg = cloneContainerConfig(cfg) - cfg.Image = newRef - - imageLabels := map[string]string(nil) - imageID := "" - imageInspect, imageInspectErr := dockerClient.ImageInspect(ctx, newRef) - if imageInspectErr != nil { - s.logger.WarnContext(ctx, "could not inspect target image labels; preserving container OCI overrides", - "image", newRef, - "error", imageInspectErr, - ) - } else { - imageID = imageInspect.ID - if imageInspect.Config != nil { - imageLabels = imageInspect.Config.Labels + oldImage, err := dockerClient.ImageInspect(ctx, inspect.Image) + if err != nil { + s.logger.WarnContext(ctx, "could not inspect previous image; keeping its inherited container config", "image", inspect.Image, "error", err) + } + // Drop what the container inherited from its old image so the daemon merges in the new image's + // defaults, as on a fresh create; an unknown old image strips nothing. + imageConfig := cmp.Or(oldImage.Config, &dockerspec.DockerOCIImageConfig{}) + portBindings := kit.FromPtr(inspect.HostConfig).PortBindings + cfg := new(kit.FromPtr(inspect.Config)) + cfg.Image = imageRef + cfg.Hostname = kit.Ternary(cfg.Hostname == inspect.ID[:min(12, len(inspect.ID))], "", cfg.Hostname) + cfg.Env = slices.DeleteFunc(slices.Clone(cfg.Env), func(env string) bool { return slices.Contains(imageConfig.Env, env) }) + cfg.Labels = maps.Clone(cfg.Labels) + maps.DeleteFunc(cfg.Labels, func(key, value string) bool { + imageValue, ok := imageConfig.Labels[key] + return ok && imageValue == value + }) + cfg.Volumes = maps.Clone(cfg.Volumes) + maps.DeleteFunc(cfg.Volumes, func(target string, _ struct{}) bool { + _, ok := imageConfig.Volumes[target] + return ok + }) + cfg.ExposedPorts = maps.Clone(cfg.ExposedPorts) + maps.DeleteFunc(cfg.ExposedPorts, func(port network.Port, _ struct{}) bool { + _, exposed := imageConfig.ExposedPorts[port.String()] + _, bound := portBindings[port] + return exposed && !bound + }) + cfg.Entrypoint, cfg.Cmd = slices.Clone(cfg.Entrypoint), slices.Clone(cfg.Cmd) + // The daemon only inherits the image's Cmd when Entrypoint is inherited too. + if slices.Equal(cfg.Entrypoint, imageConfig.Entrypoint) { + cfg.Entrypoint = nil + if slices.Equal(cfg.Cmd, imageConfig.Cmd) { + cfg.Cmd, cfg.ArgsEscaped = nil, false } } - cfg.Labels = refreshRecreatedImageLabelsInternal(cfg.Labels, imageLabels, imageID) + cfg.WorkingDir = kit.Ternary(cfg.WorkingDir == imageConfig.WorkingDir, "", cfg.WorkingDir) + cfg.User = kit.Ternary(cfg.User == imageConfig.User, "", cfg.User) + cfg.StopSignal = kit.Ternary(cfg.StopSignal == imageConfig.StopSignal, "", cfg.StopSignal) + cfg.Shell = kit.Ternary(slices.Equal(cfg.Shell, imageConfig.Shell), nil, cfg.Shell) + cfg.OnBuild = kit.Ternary(slices.Equal(cfg.OnBuild, imageConfig.OnBuild), nil, cfg.OnBuild) + if cfg.Healthcheck != nil && imageConfig.Healthcheck != nil { + health, imageHealth := *cfg.Healthcheck, imageConfig.Healthcheck + health.Test = kit.Ternary(slices.Equal(health.Test, imageHealth.Test), nil, health.Test) + health.Interval = kit.Ternary(health.Interval == imageHealth.Interval, 0, health.Interval) + health.Timeout = kit.Ternary(health.Timeout == imageHealth.Timeout, 0, health.Timeout) + health.StartPeriod = kit.Ternary(health.StartPeriod == imageHealth.StartPeriod, 0, health.StartPeriod) + health.StartInterval = kit.Ternary(health.StartInterval == imageHealth.StartInterval, 0, health.StartInterval) + health.Retries = kit.Ternary(health.Retries == imageHealth.Retries, 0, health.Retries) + cfg.Healthcheck = &health + } hostConfig, _, _, err := compat.PrepareRecreateHostConfigForEngine(ctx, dockerClient, inspect.HostConfig) if err != nil { @@ -314,63 +340,122 @@ func (s *Service) createAndStartStandaloneContainer(ctx context.Context, dockerC var networkMode container.NetworkMode if hostConfig != nil { networkMode = hostConfig.NetworkMode + pinVolumeMounts(hostConfig, inspect.Mounts) } if networkMode.IsHost() || networkMode.IsContainer() { - cfg.Hostname = "" - cfg.Domainname = "" + cfg.Hostname, cfg.Domainname = "", "" } if networkMode.IsContainer() { cfg.ExposedPorts = nil - if hostConfig != nil { - hostConfig.PortBindings = nil - hostConfig.PublishAllPorts = false - } + hostConfig.PortBindings, hostConfig.PublishAllPorts = nil, false } apiVersion := compat.DetectDockerAPIVersion(ctx, dockerClient) - networkingConfig := buildRecreateNetworkingConfig(networkMode, inspect.NetworkSettings, apiVersion) - containerName := strings.TrimPrefix(inspect.Name, "/") + var inspectOptions []client.ImageInspectOption + if compat.IsDockerAPIVersionAtLeast(apiVersion, "1.48") { + inspectOptions = append(inspectOptions, client.ImageInspectWithManifests(true)) + } + newImage, err := dockerClient.ImageInspect(ctx, imageRef, inspectOptions...) + if err != nil { + s.logger.WarnContext(ctx, "could not inspect target image; keeping its Compose image label", "image", imageRef, "error", err) + } + if _, ok := cfg.Labels[compose.ImageLabelKey]; ok && err == nil { + // Mirror Compose: the host platform's available image manifest, else the only available one, else the image ID. + available := slices.DeleteFunc(slices.Clone(newImage.Manifests), func(m image.ManifestSummary) bool { return m.Kind != image.ManifestKindImage || !m.Available }) + matcher := platforms.Default() + i := slices.IndexFunc(available, func(m image.ManifestSummary) bool { return m.ImageData != nil && matcher.Match(m.ImageData.Platform) }) + if i < 0 && len(available) == 1 { + i = 0 + } + cfg.Labels[compose.ImageLabelKey] = newImage.ID + if i >= 0 { + cfg.Labels[compose.ImageLabelKey] = available[i].ID + } + } + + var networkingConfig *network.NetworkingConfig + if !networkMode.IsContainer() && inspect.NetworkSettings != nil { + shortID := inspect.ID[:min(12, len(inspect.ID))] + endpoints := make(map[string]*network.EndpointSettings, len(inspect.NetworkSettings.Networks)) + for networkName, endpoint := range inspect.NetworkSettings.Networks { + endpoint = cmp.Or(endpoint, &network.EndpointSettings{}) + // Pre-1.44 inspect lists the old short ID as an alias; carrying it over would accumulate them. + endpoints[networkName] = &network.EndpointSettings{ + IPAMConfig: endpoint.IPAMConfig, + Links: endpoint.Links, + Aliases: slices.DeleteFunc(slices.Clone(endpoint.Aliases), func(alias string) bool { return alias == shortID }), + DriverOpts: endpoint.DriverOpts, + GwPriority: endpoint.GwPriority, + MacAddress: endpoint.MacAddress, + } + } + if sanitized := compat.SanitizeContainerCreateEndpointSettingsForDockerAPI(endpoints, apiVersion); len(sanitized) > 0 { + networkingConfig = &network.NetworkingConfig{EndpointsConfig: sanitized} + } + } + createCtx, cancelCreate := s.opCtx(ctx) resp, err := compat.ContainerCreateWithCompatibilityForAPIVersion(createCtx, dockerClient, client.ContainerCreateOptions{ Config: cfg, HostConfig: hostConfig, NetworkingConfig: networkingConfig, - Name: containerName, + Name: strings.TrimPrefix(inspect.Name, "/"), }, apiVersion) cancelCreate() if err != nil { return resp.ID, fmt.Errorf("create: %w", err) } - _ = s.recordEvent(ctx, "container_create", resp.ID, name, map[string]any{"action": "updater_create", "newImageID": resp.ID}) + _ = s.recordEvent(ctx, "container_create", resp.ID, name, map[string]any{"action": "updater_create", "newImageID": newImage.ID}) - startCtx, cancelStart := s.opCtx(ctx) - _, err = dockerClient.ContainerStart(startCtx, resp.ID, client.ContainerStartOptions{}) - cancelStart() - if err != nil { - running, inspectErr := containerRunning(ctx, dockerClient, resp.ID) - if inspectErr != nil { - return resp.ID, fmt.Errorf("start: %w; inspect after start error: %w", err, inspectErr) - } - if !running { - return resp.ID, fmt.Errorf("start: %w", err) + if inspect.State != nil && (inspect.State.Running || inspect.State.Restarting) { + startCtx, cancelStart := s.opCtx(ctx) + _, err = dockerClient.ContainerStart(startCtx, resp.ID, client.ContainerStartOptions{}) + cancelStart() + if err != nil { + inspected, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, resp.ID, client.ContainerInspectOptions{}) + switch { + case inspectErr != nil: + return resp.ID, fmt.Errorf("start: %w; inspect after start error: %w", err, inspectErr) + case inspected.Container.State == nil || !inspected.Container.State.Running: + return resp.ID, fmt.Errorf("start: %w", err) + } + s.logger.WarnContext(ctx, "container start returned error but inspect reports running", "containerId", resp.ID, "containerName", name, "error", err) } - s.logger.WarnContext(ctx, "container start returned error but inspect reports running", "containerId", resp.ID, "containerName", name, "error", err) + _ = s.recordEvent(ctx, "container_start", resp.ID, name, map[string]any{"action": "updater_start"}) } - _ = s.recordEvent(ctx, "container_start", resp.ID, name, map[string]any{"action": "updater_start"}) - return resp.ID, s.recordEvent(ctx, "container_update", resp.ID, name, map[string]any{"oldContainerID": cnt.ID, "newContainerID": resp.ID, "newImage": newRef}) + return resp.ID, s.recordEvent(ctx, "container_update", resp.ID, name, map[string]any{"oldContainerID": cnt.ID, "newContainerID": resp.ID, "newImage": imageRef}) } -func containerRunning(ctx context.Context, dockerClient *client.Client, containerID string) (bool, error) { - inspectResult, err := compat.ContainerInspectWithCompatibility(ctx, dockerClient, containerID, client.ContainerInspectOptions{}) - if err != nil { - return false, err +// pinVolumeMounts reattaches every volume the old container mounted by name, so anonymous and +// image-declared volumes keep their data instead of being recreated empty. +func pinVolumeMounts(hostConfig *container.HostConfig, mountPoints []container.MountPoint) { + hostConfig.Binds, hostConfig.Mounts = slices.Clone(hostConfig.Binds), slices.Clone(hostConfig.Mounts) + for _, mountPoint := range mountPoints { + if mountPoint.Type != mount.TypeVolume || mountPoint.Name == "" { + continue + } + destination := path.Clean(mountPoint.Destination) + if i := slices.IndexFunc(hostConfig.Mounts, func(m mount.Mount) bool { return path.Clean(m.Target) == destination }); i >= 0 { + if hostConfig.Mounts[i].Type == mount.TypeVolume && hostConfig.Mounts[i].Source == "" { + hostConfig.Mounts[i].Source = mountPoint.Name + } + continue + } + // Short syntax is "/dest" for an anonymous volume, else "source:/dest[:options]". + i := slices.IndexFunc(hostConfig.Binds, func(bind string) bool { + parts := strings.SplitN(bind, ":", 3) + return path.Clean(parts[min(1, len(parts)-1)]) == destination + }) + switch { + case i < 0: + hostConfig.Mounts = append(hostConfig.Mounts, mount.Mount{Type: mount.TypeVolume, Source: mountPoint.Name, Target: mountPoint.Destination, ReadOnly: !mountPoint.RW}) + case !strings.Contains(hostConfig.Binds[i], ":"): + hostConfig.Binds[i] = mountPoint.Name + ":" + hostConfig.Binds[i] + } } - state := inspectResult.Container.State - return state != nil && state.Running, nil } func (s *Service) removeFailedCreatedContainer(ctx context.Context, dockerClient *client.Client, containerID, containerName string) { - containerID = strings.TrimSpace(containerID) if containerID == "" { return } @@ -384,156 +469,26 @@ func (s *Service) removeFailedCreatedContainer(ctx context.Context, dockerClient _ = s.recordEvent(ctx, "container_cleanup", containerID, containerName, map[string]any{"action": "updater_cleanup_failed_create"}) } -func (s *Service) rollbackStandaloneContainer(ctx context.Context, dockerClient *client.Client, cnt container.Summary, inspect container.InspectResponse) error { - oldImageID := strings.TrimSpace(inspect.Image) - if oldImageID == "" { - return errors.New("old image ID unavailable") - } - rollbackID, err := s.createAndStartStandaloneContainer(ctx, dockerClient, cnt, inspect, oldImageID) - if err != nil { - s.removeFailedCreatedContainer(ctx, dockerClient, rollbackID, containerSummaryName(cnt)) - return err - } - _ = s.recordEvent(ctx, "container_rollback", rollbackID, containerSummaryName(cnt), map[string]any{ - "action": "updater_rollback", - "oldContainerID": cnt.ID, - "rollbackImage": oldImageID, - }) - return nil -} - -func (s *Service) updateComposeOrStandalone(ctx context.Context, target container.Summary, inspect container.InspectResponse, normalizedRef string) error { - labels := labelsFromInspect(inspect) - if inspect.Config != nil && - refs.NormalizeImageUpdateRef(inspect.Config.Image) != refs.NormalizeImageUpdateRef(normalizedRef) && - (compose.ProjectLabel(labels) != "" || compose.ServiceLabel(labels) != "") { - return s.updateComposeImageInternal(ctx, target, inspect, normalizedRef) - } - projectName := compose.ProjectLabel(labels) - serviceName := compose.ServiceLabel(labels) - if projectName != "" && serviceName != "" && s.config.ProjectUpdater != nil { - project, err := s.config.ProjectUpdater.ProjectByComposeName(ctx, projectName) - if err == nil { - // The compose path was chosen; a failure here must surface instead - // of falling back, so a partial compose up is never clobbered by a - // standalone recreate. - return s.config.ProjectUpdater.UpdateServices(ctx, project.ID, []string{serviceName}) - } - s.logger.WarnContext(ctx, "compose project not resolved; falling back to standalone container update", - "container", containerSummaryName(target), - "project", projectName, - "service", serviceName, - "error", err, - ) - } - return s.updateStandaloneContainer(ctx, target, inspect, normalizedRef) -} - -func normalizedTagsForContainer(ctx context.Context, dockerClient *client.Client, inspect container.InspectResponse) []string { - seen := map[string]struct{}{} - if dockerClient != nil { - if imageInspect, err := dockerClient.ImageInspect(ctx, inspect.Image); err == nil { - for _, tag := range imageInspect.RepoTags { - if normalized := refs.NormalizeImageUpdateRef(tag); normalized != "" { - seen[normalized] = struct{}{} - } - } - } - } - if inspect.Config != nil { - if normalized := refs.NormalizeImageUpdateRef(inspect.Config.Image); normalized != "" { - seen[normalized] = struct{}{} - } - } - return slices.Collect(maps.Keys(seen)) -} - -func buildRecreateNetworkingConfig(networkMode container.NetworkMode, settings *container.NetworkSettings, apiVersion string) *network.NetworkingConfig { - if networkMode.IsContainer() || settings == nil || len(settings.Networks) == 0 { - return nil +// pinNetworkContainer rewrites a container: network mode to the owner's name, which survives its recreate. +func pinNetworkContainer(inspect *container.InspectResponse, containers []container.Summary) { + if inspect.HostConfig == nil || !inspect.HostConfig.NetworkMode.IsContainer() { + return } - - rawEndpointsConfig := make(map[string]*network.EndpointSettings, len(settings.Networks)) - for networkName, endpoint := range settings.Networks { - if endpoint == nil { - rawEndpointsConfig[networkName] = &network.EndpointSettings{} - continue - } - rawEndpointsConfig[networkName] = &network.EndpointSettings{ - IPAMConfig: endpoint.IPAMConfig.Copy(), - Links: slices.Clone(endpoint.Links), - Aliases: slices.Clone(endpoint.Aliases), - DriverOpts: maps.Clone(endpoint.DriverOpts), - GwPriority: endpoint.GwPriority, - MacAddress: endpoint.MacAddress, - } + ref := inspect.HostConfig.NetworkMode.ConnectedContainer() + if ref == "" || slices.ContainsFunc(containers, func(c container.Summary) bool { return containerSummaryName(c) == ref }) { + return } - - sanitized := compat.SanitizeContainerCreateEndpointSettingsForDockerAPI(rawEndpointsConfig, apiVersion) - if len(sanitized) == 0 { - return nil + if i := slices.IndexFunc(containers, func(c container.Summary) bool { return strings.HasPrefix(c.ID, ref) }); i >= 0 { + inspect.HostConfig.NetworkMode = container.NetworkMode("container:" + containerSummaryName(containers[i])) } - return &network.NetworkingConfig{EndpointsConfig: sanitized} } -// containerSummaryName returns a displayable name for a container: -// Docker's first name, or a short ID when the container has none. +// containerSummaryName returns the container's own name, skipping legacy link aliases such as +// "web/db", or its short ID when it has none. func containerSummaryName(cnt container.Summary) string { - if len(cnt.Names) > 0 { - if name := strings.TrimPrefix(cnt.Names[0], "/"); name != "" { - return name - } - } - if len(cnt.ID) >= 12 { - return cnt.ID[:12] + name := "" + if i := slices.IndexFunc(cnt.Names, func(n string) bool { return !strings.Contains(strings.TrimPrefix(n, "/"), "/") }); i >= 0 { + name = strings.TrimPrefix(cnt.Names[i], "/") } - return cnt.ID -} - -func cloneContainerConfig(config *container.Config) *container.Config { - if config == nil { - return nil - } - copied := new(*config) - copied.Env = slices.Clone(config.Env) - copied.Cmd = slices.Clone(config.Cmd) - copied.Entrypoint = slices.Clone(config.Entrypoint) - copied.Labels = maps.Clone(config.Labels) - return copied -} - -func refreshRecreatedImageLabelsInternal(containerLabels, imageLabels map[string]string, imageID string) map[string]string { - refreshed := maps.Clone(containerLabels) - - // Docker does not expose whether an effective container label was inherited - // or explicitly set, so existing values must win. - for key, value := range imageLabels { - if !strings.HasPrefix(strings.ToLower(key), "org.opencontainers.image.") { - continue - } - preserved := false - for existingKey := range refreshed { - if strings.EqualFold(existingKey, key) { - preserved = true - break - } - } - if preserved { - continue - } - if refreshed == nil { - refreshed = make(map[string]string) - } - refreshed[key] = value - } - - if refreshed != nil && imageID != "" { - for key := range refreshed { - if strings.EqualFold(key, "com.docker.compose.image") { - refreshed[key] = imageID - } - } - } - - return refreshed + return cmp.Or(name, cnt.ID[:min(12, len(cnt.ID))]) } diff --git a/updater/container_test.go b/updater/container_test.go index 880ba0e..91433cf 100644 --- a/updater/container_test.go +++ b/updater/container_test.go @@ -57,7 +57,7 @@ func TestClearPendingRecord(t *testing.T) { store := &fakePendingStore{records: []ImageUpdateRecord{tt.record}} service := newServiceForTest(t, Config{PendingStore: store}) - service.clearPendingRecordInternal(t.Context(), "container", tt.appliedRef) + service.clearPendingRecord(t.Context(), "container", tt.appliedRef) if cleared := len(store.cleared) > 0; cleared != tt.wantCleared { t.Fatalf("cleared = %v (%v), want %v", cleared, store.cleared, tt.wantCleared) @@ -66,25 +66,6 @@ func TestClearPendingRecord(t *testing.T) { } } -func TestRefreshRecreatedImageLabelsInternalPreservesEqualValueOverride(t *testing.T) { - containerLabels := map[string]string{ - "org.opencontainers.image.version": "v2.6.0-next.30", - } - targetImageLabels := map[string]string{ - "org.opencontainers.image.version": "v2.7.0-next.17", - "org.opencontainers.image.title": "Arcane", - } - - got := refreshRecreatedImageLabelsInternal(containerLabels, targetImageLabels, "") - - if got["org.opencontainers.image.version"] != "v2.6.0-next.30" { - t.Fatalf("OCI version = %q, want equal-value container override", got["org.opencontainers.image.version"]) - } - if got["org.opencontainers.image.title"] != "Arcane" { - t.Fatalf("OCI title = %q, want target image value for missing label", got["org.opencontainers.image.title"]) - } -} - func TestUpdateStandaloneContainerRollsBackAndRemovesDanglingCreateOnStartFailure(t *testing.T) { var operations []string var createdImages []string @@ -155,16 +136,15 @@ func TestUpdateStandaloneContainerRollsBackAndRemovesDanglingCreateOnStartFailur EventRecorder: recorder, }) - err := service.updateStandaloneContainer(t.Context(), + err := service.updateComposeOrStandalone(t.Context(), container.Summary{ID: "old-id", Names: []string{"/app"}}, - container.InspectResponse{ID: "old-id", Name: "/app", Image: "sha256:old-image", Config: &container.Config{ + container.InspectResponse{State: &container.State{Running: true}, ID: "old-id", Name: "/app", Image: "sha256:old-image", Config: &container.Config{ Image: "app:1", Labels: map[string]string{ "org.opencontainers.image.version": "v2.6.0-next.30", "org.opencontainers.image.revision": "old-revision", "org.opencontainers.image.source": "https://container.example/override", "com.docker.compose.image": "sha256:old-image", - "com.docker.compose.project": "arcane", "com.example.custom": "keep", }, }}, @@ -172,7 +152,7 @@ func TestUpdateStandaloneContainerRollsBackAndRemovesDanglingCreateOnStartFailur ) if err == nil { - t.Fatal("updateStandaloneContainer() error = nil, want start failure with rollback outcome") + t.Fatal("updateComposeOrStandalone() error = nil, want start failure with rollback outcome") } if !strings.Contains(err.Error(), "rollback succeeded") { t.Fatalf("error = %q, want rollback succeeded detail", err.Error()) @@ -180,36 +160,23 @@ func TestUpdateStandaloneContainerRollsBackAndRemovesDanglingCreateOnStartFailur if len(createdImages) != 2 || createdImages[0] != "app:2" || createdImages[1] != "sha256:old-image" { t.Fatalf("created images = %#v, want new ref then old image ID", createdImages) } - if got := createdLabels[0]["org.opencontainers.image.version"]; got != "v2.6.0-next.30" { - t.Fatalf("new container OCI version = %q, want equal-value container override", got) - } - if got := createdLabels[0]["org.opencontainers.image.revision"]; got != "old-revision" { - t.Fatalf("new container OCI revision = %q, want equal-value container override", got) - } - if got := createdLabels[0]["org.opencontainers.image.source"]; got != "https://container.example/override" { - t.Fatalf("new container OCI source = %q, want container override", got) - } - if got := createdLabels[0]["org.opencontainers.image.title"]; got != "new-title" { - t.Fatalf("new container OCI title = %q, want target image value for missing label", got) + // Labels equal to the old image's were inherited, so they are dropped for the daemon to merge in the new image's. + for i, phase := range []string{"new", "rollback"} { + for _, key := range []string{"org.opencontainers.image.version", "org.opencontainers.image.revision", "org.opencontainers.image.title"} { + if got, ok := createdLabels[i][key]; ok { + t.Fatalf("%s container %s = %q, want inherited label left to the image", phase, key, got) + } + } + if got := createdLabels[i]["org.opencontainers.image.source"]; got != "https://container.example/override" { + t.Fatalf("%s container OCI source = %q, want container override", phase, got) + } + if got := createdLabels[i]["com.example.custom"]; got != "keep" { + t.Fatalf("%s container custom label = %q, want keep", phase, got) + } } if got := createdLabels[0]["com.docker.compose.image"]; got != "sha256:new-image" { t.Fatalf("new container Compose image = %q, want sha256:new-image", got) } - if got := createdLabels[0]["com.docker.compose.project"]; got != "arcane" { - t.Fatalf("new container Compose project = %q, want arcane", got) - } - if got := createdLabels[0]["com.example.custom"]; got != "keep" { - t.Fatalf("new container custom label = %q, want keep", got) - } - if got := createdLabels[1]["org.opencontainers.image.version"]; got != "v2.6.0-next.30" { - t.Fatalf("rollback container OCI version = %q, want v2.6.0-next.30", got) - } - if got := createdLabels[1]["org.opencontainers.image.source"]; got != "https://container.example/override" { - t.Fatalf("rollback container OCI source = %q, want container override", got) - } - if got := createdLabels[1]["org.opencontainers.image.title"]; got != "old-title" { - t.Fatalf("rollback container OCI title = %q, want previous image value for missing label", got) - } if got := createdLabels[1]["com.docker.compose.image"]; got != "sha256:old-image" { t.Fatalf("rollback container Compose image = %q, want sha256:old-image", got) } @@ -307,9 +274,10 @@ func TestUpdateStandaloneContainerRemovesCreatedContainerWhenExtraNetworkConnect OperationTimeout: 10 * time.Millisecond, }) - err := service.updateStandaloneContainer(t.Context(), + err := service.updateComposeOrStandalone(t.Context(), container.Summary{ID: "old-id", Names: []string{"/app"}}, container.InspectResponse{ + State: &container.State{Running: true}, ID: "old-id", Name: "/app", Image: "sha256:old-image", @@ -323,7 +291,7 @@ func TestUpdateStandaloneContainerRemovesCreatedContainerWhenExtraNetworkConnect ) if err == nil { - t.Fatal("updateStandaloneContainer() error = nil, want network timeout with rollback outcome") + t.Fatal("updateComposeOrStandalone() error = nil, want network timeout with rollback outcome") } if !strings.Contains(err.Error(), "rollback succeeded") { t.Fatalf("error = %q, want rollback succeeded detail", err.Error()) @@ -376,13 +344,13 @@ func TestUpdateStandaloneContainerTreatsAmbiguousStartErrorAsSuccessWhenInspectR DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, }) - err := service.updateStandaloneContainer(t.Context(), + err := service.updateComposeOrStandalone(t.Context(), container.Summary{ID: "old-id", Names: []string{"/app"}}, - container.InspectResponse{ID: "old-id", Name: "/app", Image: "sha256:old-image", Config: &container.Config{Image: "app:1"}}, + container.InspectResponse{State: &container.State{Running: true}, ID: "old-id", Name: "/app", Image: "sha256:old-image", Config: &container.Config{Image: "app:1"}}, "app:2", ) if err != nil { - t.Fatalf("updateStandaloneContainer() error = %v, want nil after inspect confirms running", err) + t.Fatalf("updateComposeOrStandalone() error = %v, want nil after inspect confirms running", err) } assertOperationsInOrder(t, operations, []string{ "start:new-id", @@ -443,14 +411,14 @@ func TestUpdateStandaloneContainerRollsBackAmbiguousStartErrorWhenInspectNotRunn DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, }) - err := service.updateStandaloneContainer(t.Context(), + err := service.updateComposeOrStandalone(t.Context(), container.Summary{ID: "old-id", Names: []string{"/app"}}, - container.InspectResponse{ID: "old-id", Name: "/app", Image: "sha256:old-image", Config: &container.Config{Image: "app:1"}}, + container.InspectResponse{State: &container.State{Running: true}, ID: "old-id", Name: "/app", Image: "sha256:old-image", Config: &container.Config{Image: "app:1"}}, "app:2", ) if err == nil { - t.Fatal("updateStandaloneContainer() error = nil, want ambiguous start failure with rollback outcome") + t.Fatal("updateComposeOrStandalone() error = nil, want ambiguous start failure with rollback outcome") } if !strings.Contains(err.Error(), "rollback succeeded") { t.Fatalf("error = %q, want rollback succeeded detail", err.Error()) @@ -488,9 +456,9 @@ func TestServiceFallsBackToStandaloneWhenComposeProjectUnresolved(t *testing.T) } } -const tagSelectionContainerIDInternal, tagSelectionContainerNameInternal = "app", "web" +const tagSelectionContainerID, tagSelectionContainerName = "app", "web" -type tagSelectionScenarioInternal struct { +type tagSelectionScenario struct { name string dry, force, self bool current, constraint string @@ -506,9 +474,9 @@ type tagSelectionScenarioInternal struct { wantSkipped bool } -func TestUpdateContainerTagSelectionInternal(t *testing.T) { - const containerID, containerName = tagSelectionContainerIDInternal, tagSelectionContainerNameInternal - for _, scenario := range []tagSelectionScenarioInternal{ +func TestUpdateContainerTagSelection(t *testing.T) { + const containerID, containerName = tagSelectionContainerID, tagSelectionContainerName + for _, scenario := range []tagSelectionScenario{ {name: "same digest still changes standalone reference"}, {name: "dry run selects target", dry: true}, {name: "self receives new tag", self: true}, @@ -534,21 +502,21 @@ func TestUpdateContainerTagSelectionInternal(t *testing.T) { {name: "override keeps unchanged digest", excluded: containerName, override: true, unchangedDigest: true}, } { t.Run(scenario.name, func(t *testing.T) { - runTagSelectionScenarioInternal(t, scenario) + runTagSelectionScenario(t, scenario) }) } } -// tagSelectionDockerInternal serves a single container to the updater and +// tagSelectionDocker serves a single container to the updater and // records the images it recreates and every mutating request. -type tagSelectionDockerInternal struct { +type tagSelectionDocker struct { current string values map[string]string created []string mutations int } -func newTagSelectionDockerInternal(scenario tagSelectionScenarioInternal) *tagSelectionDockerInternal { +func newTagSelectionDocker(scenario tagSelectionScenario) *tagSelectionDocker { current := scenario.current if current == "" { current = cmp.Or(scenario.repository, "app") + ":1.0.0" @@ -567,12 +535,12 @@ func newTagSelectionDockerInternal(scenario tagSelectionScenarioInternal) *tagSe values["com.docker.compose.project"] = "project" values["com.docker.compose.service"] = "web" } - return &tagSelectionDockerInternal{current: current, values: values} + return &tagSelectionDocker{current: current, values: values} } -func (d *tagSelectionDockerInternal) handlerInternal(t *testing.T) http.HandlerFunc { +func (d *tagSelectionDocker) handler(t *testing.T) http.HandlerFunc { t.Helper() - const containerID, containerName = tagSelectionContainerIDInternal, tagSelectionContainerNameInternal + const containerID, containerName = tagSelectionContainerID, tagSelectionContainerName return func(w http.ResponseWriter, r *http.Request) { path := dockerAPIPath(r.URL.Path) if r.Method != http.MethodGet { @@ -583,7 +551,8 @@ func (d *tagSelectionDockerInternal) handlerInternal(t *testing.T) http.HandlerF writeDockerJSON(t, w, []container.Summary{{ID: containerID, Names: []string{"/" + containerName}, Image: d.current, ImageID: "same", Labels: d.values}}) case path == "/containers/"+containerID+"/json": writeDockerJSON(t, w, container.InspectResponse{ - ID: containerID, Name: "/" + containerName, Image: "same", + State: &container.State{Running: true}, + ID: containerID, Name: "/" + containerName, Image: "same", Config: &container.Config{Image: d.current, Labels: d.values}, HostConfig: &container.HostConfig{}, }) case strings.HasPrefix(path, "/images/"): @@ -607,10 +576,10 @@ func (d *tagSelectionDockerInternal) handlerInternal(t *testing.T) http.HandlerF } } -func runTagSelectionScenarioInternal(t *testing.T, scenario tagSelectionScenarioInternal) { +func runTagSelectionScenario(t *testing.T, scenario tagSelectionScenario) { t.Helper() - docker := newTagSelectionDockerInternal(scenario) - dockerClient := newDockerClientForHandler(t, docker.handlerInternal(t)) + docker := newTagSelectionDocker(scenario) + dockerClient := newDockerClientForHandler(t, docker.handler(t)) puller := &fakePuller{} lister := &testTagLister{tags: []string{"1.1.0", "2.0.0"}} self := &fakeSelfUpdater{} @@ -620,10 +589,10 @@ func runTagSelectionScenarioInternal(t *testing.T, scenario tagSelectionScenario } cfg := Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, RegistryTagLister: lister, ImagePuller: puller, SelfUpdater: self, Settings: settings} if scenario.self { - cfg.SelfContainerID = tagSelectionContainerIDInternal + cfg.SelfContainerID = tagSelectionContainerID } service := newServiceForTest(t, cfg) - result, err := service.UpdateContainer(t.Context(), tagSelectionContainerIDInternal, Options{DryRun: scenario.dry, Force: scenario.force, IgnoreSettingsExclusions: scenario.override}) + result, err := service.UpdateContainer(t.Context(), tagSelectionContainerID, Options{DryRun: scenario.dry, Force: scenario.force, IgnoreSettingsExclusions: scenario.override}) if err != nil { t.Fatal(err) } @@ -648,10 +617,10 @@ func runTagSelectionScenarioInternal(t *testing.T, scenario tagSelectionScenario } return } - assertTagSelectionUpdatedInternal(t, scenario, result, docker, puller, self) + assertTagSelectionUpdated(t, scenario, result, docker, puller, self) } -func assertTagSelectionUpdatedInternal(t *testing.T, scenario tagSelectionScenarioInternal, result *Result, docker *tagSelectionDockerInternal, puller *fakePuller, self *fakeSelfUpdater) { +func assertTagSelectionUpdated(t *testing.T, scenario tagSelectionScenario, result *Result, docker *tagSelectionDocker, puller *fakePuller, self *fakeSelfUpdater) { t.Helper() wantCreated := cmp.Or(scenario.repository, "app") + ":1.1.0" if scenario.constraint != "" { diff --git a/updater/default_auth.go b/updater/default_auth.go index c4ff2a0..d00ea9e 100644 --- a/updater/default_auth.go +++ b/updater/default_auth.go @@ -8,89 +8,30 @@ import ( "strings" dockerCliConfig "github.com/docker/cli/cli/config" - dockerCliConfigTypes "github.com/docker/cli/cli/config/types" - "github.com/google/go-containerregistry/pkg/authn" - dockerauthconfig "github.com/moby/moby/api/pkg/authconfig" dockerregistry "github.com/moby/moby/api/types/registry" - "github.com/moby/moby/client" - kitregistry "go.getarcane.app/kit/pkg/registry" - "go.getarcane.app/updater/internal/registryhost" + "go.getarcane.app/updater/refs" ) -func defaultImagePullOptions(ctx context.Context, imageRef string) (client.ImagePullOptions, error) { - authConfig, ok, err := defaultDockerConfigRegistryAuthConfig(ctx, imageRef) - if err != nil { - return client.ImagePullOptions{}, err - } - if !ok { - logAnonymousRegistryFallback(ctx, imageRef, "no credentials found", nil) - return client.ImagePullOptions{}, nil - } - - encoded, err := dockerauthconfig.Encode(authConfig) - if err != nil { - return client.ImagePullOptions{}, fmt.Errorf("encode registry auth: %w", err) - } - return client.ImagePullOptions{ - RegistryAuth: encoded, - // Retry anonymously when the registry rejects the credentials. - PrivilegeFunc: func(context.Context) (string, error) { return "", nil }, - }, nil -} - -func defaultDigestCredentials(ctx context.Context, imageRef string) (*authn.AuthConfig, error) { - authConfig, ok, err := defaultDockerConfigRegistryAuthConfig(ctx, imageRef) - if err != nil { - return nil, err - } - if !ok { - logAnonymousRegistryFallback(ctx, imageRef, "no credentials found", nil) - return nil, nil - } - - credential := &authn.AuthConfig{ - Username: authConfig.Username, - Password: authConfig.Password, - IdentityToken: authConfig.IdentityToken, - RegistryToken: authConfig.RegistryToken, - } - if (credential.Username == "" || credential.Password == "") && credential.IdentityToken == "" && credential.RegistryToken == "" { - logAnonymousRegistryFallback(ctx, imageRef, "no usable username/password or token", nil) - return nil, nil - } - return credential, nil -} - +// defaultDockerConfigRegistryAuthConfig loads usable Docker config credentials for imageRef's registry, +// reporting false when the request should go out anonymously. func defaultDockerConfigRegistryAuthConfig(ctx context.Context, imageRef string) (dockerregistry.AuthConfig, bool, error) { - server, err := registryhost.AuthAddress(imageRef) + parsed, err := refs.NormalizeReference(imageRef) if err != nil { return dockerregistry.AuthConfig{}, false, fmt.Errorf("get registry address: %w", err) } - if kitregistry.Normalize(server) == "docker.io" { - server = "docker.io" - } - - configDir := strings.TrimSpace(os.Getenv(dockerCliConfig.EnvOverrideConfigDir)) - configFile, err := dockerCliConfig.Load(configDir) + configFile, err := dockerCliConfig.Load(strings.TrimSpace(os.Getenv(dockerCliConfig.EnvOverrideConfigDir))) if err != nil { return dockerregistry.AuthConfig{}, false, fmt.Errorf("load Docker config: %w", err) } - authConfig, err := configFile.GetAuthConfig(server) - if err == nil { - if defaultDockerConfigAuthEmpty(authConfig) { - logAnonymousRegistryFallback(ctx, imageRef, "empty credentials", nil) - return dockerregistry.AuthConfig{}, false, nil - } - return dockerRegistryAuthConfigFromDockerConfig(authConfig), true, nil + authConfig, err := configFile.GetAuthConfig(parsed.RegistryHost) + if err != nil { + slog.DebugContext(ctx, "registry credentials unavailable; proceeding anonymously", "imageRef", imageRef, "reason", "credential lookup failed", "error", err) + return dockerregistry.AuthConfig{}, false, nil } - logAnonymousRegistryFallback(ctx, imageRef, "credential lookup failed", err) - return dockerregistry.AuthConfig{}, false, nil -} - -func dockerRegistryAuthConfigFromDockerConfig(authConfig dockerCliConfigTypes.AuthConfig) dockerregistry.AuthConfig { - return dockerregistry.AuthConfig{ + // docker/cli decodes "auth" into Username and Password when it loads the config. + credential := dockerregistry.AuthConfig{ Username: strings.TrimSpace(authConfig.Username), Password: strings.TrimSpace(authConfig.Password), Auth: authConfig.Auth, @@ -98,20 +39,9 @@ func dockerRegistryAuthConfigFromDockerConfig(authConfig dockerCliConfigTypes.Au IdentityToken: strings.TrimSpace(authConfig.IdentityToken), RegistryToken: strings.TrimSpace(authConfig.RegistryToken), } -} - -func defaultDockerConfigAuthEmpty(authConfig dockerCliConfigTypes.AuthConfig) bool { - return strings.TrimSpace(authConfig.Username) == "" && - strings.TrimSpace(authConfig.Password) == "" && - strings.TrimSpace(authConfig.Auth) == "" && - strings.TrimSpace(authConfig.IdentityToken) == "" && - strings.TrimSpace(authConfig.RegistryToken) == "" -} - -func logAnonymousRegistryFallback(ctx context.Context, imageRef, reason string, err error) { - args := []any{"imageRef", imageRef, "reason", reason} - if err != nil { - args = append(args, "error", err) + if (credential.Username == "" || credential.Password == "") && credential.IdentityToken == "" && credential.RegistryToken == "" { + slog.DebugContext(ctx, "registry credentials unavailable; proceeding anonymously", "imageRef", imageRef, "reason", "no usable username/password or token") + return dockerregistry.AuthConfig{}, false, nil } - slog.DebugContext(ctx, "registry credentials unavailable; proceeding anonymously", args...) + return credential, true, nil } diff --git a/updater/defaults.go b/updater/defaults.go index ee99f54..4de0bde 100644 --- a/updater/defaults.go +++ b/updater/defaults.go @@ -1,6 +1,7 @@ package updater import ( + "cmp" "context" json "encoding/json/v2" "errors" @@ -9,15 +10,16 @@ import ( "net/http" "sync/atomic" + "github.com/google/go-containerregistry/pkg/authn" + dockerauthconfig "github.com/moby/moby/api/pkg/authconfig" "github.com/moby/moby/client" "go.getarcane.app/updater/refs" "go.getarcane.app/updater/registry" ) -// DockerClient is the built-in DockerClientProvider. It lazily opens one -// Docker client, verifies it with a ping on every hand-out, and reconnects -// after the daemon goes away. Construct one with NewDockerClientProvider. +// DockerClient is the built-in DockerClientProvider: it lazily opens one client, pings it on every +// hand-out, and reconnects after the daemon goes away. Construct one with NewDockerClientProvider. type DockerClient struct { options []client.Opt client atomic.Pointer[client.Client] @@ -31,29 +33,20 @@ type defaultRegistryDigestResolver struct { httpClient *http.Client } -// NewDockerClientProvider returns a Docker client provider that reads the -// local Docker environment, plus any options given. The caller owns the result -// and should Close it when done — unless it was left to New to build, in which -// case Service.Close handles it. +// NewDockerClientProvider returns a provider for the local Docker environment plus options. The caller +// owns it and should Close it, unless New built it, in which case Service.Close does. func NewDockerClientProvider(options ...client.Opt) *DockerClient { return &DockerClient{options: append([]client.Opt{client.FromEnv}, options...)} } // NewImagePuller returns an image puller backed by Docker's ImagePull API. func NewImagePuller(provider DockerClientProvider) ImagePuller { - if provider == nil { - provider = NewDockerClientProvider() - } - return defaultImagePuller{dockerClientProvider: provider} + return defaultImagePuller{dockerClientProvider: cmp.Or[DockerClientProvider](provider, NewDockerClientProvider())} } // NewRegistryDigestResolver returns a registry HTTP digest resolver. func NewRegistryDigestResolver() RegistryDigestResolver { - return newRegistryDigestResolver(nil) -} - -func newRegistryDigestResolver(httpClient *http.Client) RegistryDigestResolver { - return defaultRegistryDigestResolver{httpClient: httpClient} + return defaultRegistryDigestResolver{} } // DockerClient returns a live Docker client, opening or reopening one as @@ -64,13 +57,11 @@ func (p *DockerClient) DockerClient(ctx context.Context) (*client.Client, error) } if dockerClient := p.client.Load(); dockerClient != nil { if _, err := dockerClient.Ping(ctx, client.PingOptions{}); err != nil { + var closeErr error if p.client.CompareAndSwap(dockerClient, nil) { - closeErr := dockerClient.Close() - if closeErr != nil { - return nil, fmt.Errorf("ping docker daemon: %w", errors.Join(err, closeErr)) - } + closeErr = dockerClient.Close() } - return nil, fmt.Errorf("ping docker daemon: %w", err) + return nil, fmt.Errorf("ping docker daemon: %w", errors.Join(err, closeErr)) } return dockerClient, nil } @@ -79,12 +70,8 @@ func (p *DockerClient) DockerClient(ctx context.Context) (*client.Client, error) if err != nil { return nil, err } - _, err = dockerClient.Ping(ctx, client.PingOptions{}) - if err != nil { - if closeErr := dockerClient.Close(); closeErr != nil { - return nil, fmt.Errorf("ping docker daemon: %w", errors.Join(err, closeErr)) - } - return nil, fmt.Errorf("ping docker daemon: %w", err) + if _, err = dockerClient.Ping(ctx, client.PingOptions{}); err != nil { + return nil, fmt.Errorf("ping docker daemon: %w", errors.Join(err, dockerClient.Close())) } if p.client.CompareAndSwap(nil, dockerClient) { return dockerClient, nil @@ -117,10 +104,18 @@ func (p defaultImagePuller) PullImage(ctx context.Context, imageRef string, prog if err != nil { return fmt.Errorf("docker connect: %w", err) } - pullOptions, err := defaultImagePullOptions(ctx, imageRef) + authConfig, ok, err := defaultDockerConfigRegistryAuthConfig(ctx, imageRef) if err != nil { return fmt.Errorf("registry auth: %w", err) } + var pullOptions client.ImagePullOptions + if ok { + if pullOptions.RegistryAuth, err = dockerauthconfig.Encode(authConfig); err != nil { + return fmt.Errorf("registry auth: encode registry auth: %w", err) + } + // Retry anonymously when the registry rejects the credentials. + pullOptions.PrivilegeFunc = func(context.Context) (string, error) { return "", nil } + } resp, err := dockerClient.ImagePull(ctx, imageRef, pullOptions) if err != nil { return err @@ -151,9 +146,13 @@ func (r defaultRegistryDigestResolver) ImageDigest(ctx context.Context, imageRef if err != nil { return "", err } - credential, err := defaultDigestCredentials(ctx, imageRef) + authConfig, ok, err := defaultDockerConfigRegistryAuthConfig(ctx, imageRef) if err != nil { return "", fmt.Errorf("registry auth: %w", err) } + var credential *authn.AuthConfig + if ok { + credential = &authn.AuthConfig{Username: authConfig.Username, Password: authConfig.Password, IdentityToken: authConfig.IdentityToken, RegistryToken: authConfig.RegistryToken} + } return registry.FetchDigest(ctx, parsed.RegistryHost, parsed.Repository, parsed.Tag, credential, r.httpClient) } diff --git a/updater/defaults_test.go b/updater/defaults_test.go index b08aedb..0cb1935 100644 --- a/updater/defaults_test.go +++ b/updater/defaults_test.go @@ -27,7 +27,7 @@ func TestDefaultRegistryDigestResolverFetchesDigest(t *testing.T) { if err != nil { t.Fatal(err) } - resolver := newRegistryDigestResolver(server.Client()) + resolver := defaultRegistryDigestResolver{httpClient: server.Client()} got, err := resolver.ImageDigest(t.Context(), serverURL.Host+"/owner/app:1.0") if err != nil { diff --git a/updater/errors.go b/updater/errors.go index b3d56bb..1f4db49 100644 --- a/updater/errors.go +++ b/updater/errors.go @@ -46,31 +46,3 @@ func (e *ConfigError) Error() string { func (e *ConfigError) Unwrap() error { return e.Reason } - -// validate reports the first problem that would leave the Service unable to -// run. It is called after defaults are applied, so it catches only what a -// default cannot supply. -// -// The three port checks below are defensive: applyConfigDefaults fills each of -// them with a constructor that never returns nil, so New cannot reach them -// today. They exist so that a future default which can fail is caught here -// rather than at the first Docker call. The sentinels they carry are returned -// for real from the call sites that need those ports — ErrPendingStoreRequired -// from ApplyPending, ErrDockerClientProviderRequired from dockerClient — which -// a caller can still reach by constructing a Service around a nil port -// directly. -func (c Config) validate() error { - if c.DockerClientProvider == nil { - return &ConfigError{Field: "DockerClientProvider", Reason: ErrDockerClientProviderRequired} - } - if c.ImagePuller == nil { - return &ConfigError{Field: "ImagePuller", Reason: ErrImagePullerRequired} - } - if c.PendingStore == nil { - return &ConfigError{Field: "PendingStore", Reason: ErrPendingStoreRequired} - } - if c.OperationTimeout < 0 { - return &ConfigError{Field: "OperationTimeout", Reason: errors.New("must not be negative")} - } - return nil -} diff --git a/updater/go.mod b/updater/go.mod index f568fef..53204f2 100644 --- a/updater/go.mod +++ b/updater/go.mod @@ -6,9 +6,12 @@ replace go.getarcane.app/kit => ../ require ( github.com/Masterminds/semver/v3 v3.5.0 + github.com/containerd/errdefs v1.0.0 + github.com/containerd/platforms v1.0.0-rc.5 github.com/distribution/reference v0.6.0 github.com/docker/cli v29.8.2+incompatible github.com/google/go-containerregistry v0.22.1 + github.com/moby/docker-image-spec v1.3.1 github.com/moby/moby/api v1.56.1 github.com/moby/moby/client v0.6.1 github.com/opencontainers/go-digest v1.0.0 @@ -22,8 +25,8 @@ require ( github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/containerd/errdefs v1.0.0 // indirect github.com/containerd/errdefs/pkg v0.3.0 // indirect + github.com/containerd/log v0.1.0 // indirect github.com/docker/docker-credential-helpers v0.9.9 // indirect github.com/docker/go-connections v0.8.1 // indirect github.com/docker/go-units v0.5.0 // indirect @@ -32,7 +35,6 @@ require ( github.com/go-logr/stdr v1.2.2 // indirect github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect github.com/klauspost/compress v1.20.0 // indirect - github.com/moby/docker-image-spec v1.3.1 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/opencontainers/image-spec v1.1.1 // indirect github.com/prometheus/client_golang v1.24.1 // indirect diff --git a/updater/go.sum b/updater/go.sum index 4f0dd6a..7e5d1f0 100644 --- a/updater/go.sum +++ b/updater/go.sum @@ -12,6 +12,10 @@ github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG github.com/containerd/errdefs v1.0.0/go.mod h1:+YBYIdtsnF4Iw6nWZhJcqGSg/dwvV7tyJ/kCkyJ2k+M= github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151Xdx3ZPPE= github.com/containerd/errdefs/pkg v0.3.0/go.mod h1:NJw6s9HwNuRhnjJhM7pylWwMyAkmCQvQ4GpJHEqRLVk= +github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= +github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= +github.com/containerd/platforms v1.0.0-rc.5 h1:vXd569rDrz8LeMXzAnBsy6LADV5YtsD8oyaRarxdmSU= +github.com/containerd/platforms v1.0.0-rc.5/go.mod h1:lKlMXyLybmBedS/JJm11uDofzI8L2v0J2ZbYvNsbq1A= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= github.com/docker/cli v29.8.2+incompatible h1:2zgdFuoFst2T80oS42vhKGxkd0JRo305eIEKTsxR7pQ= diff --git a/updater/hooks.go b/updater/hooks.go index a9752e6..cfff1dd 100644 --- a/updater/hooks.go +++ b/updater/hooks.go @@ -4,16 +4,15 @@ import ( "context" "maps" "strings" + + kit "go.getarcane.app/kit/pkg" ) func (s *Service) triggerSelfUpdate(ctx context.Context, containerID, containerName, newImageRef string, labels map[string]string) error { if s.config.SelfUpdater == nil { return ErrSelfUpdaterRequired } - instanceType := "server" - if s.config.LabelPolicy.IsAgent(labels) { - instanceType = "agent" - } + instanceType := kit.Ternary(s.config.LabelPolicy.IsAgent(labels), "agent", "server") _ = s.recordEvent(ctx, "self_update_trigger", containerID, containerName, map[string]any{ "instanceType": instanceType, "newImage": newImageRef, @@ -27,22 +26,14 @@ func (s *Service) triggerSelfUpdate(ctx context.Context, containerID, containerN }) } -// isSelfUpdateCandidate reports whether a container must be handled by -// the host SelfUpdater, either by label policy or because it is the container -// the host application itself runs in. +// isSelfUpdateCandidate reports whether the host SelfUpdater must handle a container, by label policy +// or because the host application runs in it. func (s *Service) isSelfUpdateCandidate(containerID string, labels map[string]string) bool { if s.config.LabelPolicy.IsSelfUpdateTarget(labels) { return true } selfID := strings.TrimSpace(s.config.SelfContainerID) - return selfID != "" && (strings.HasPrefix(containerID, selfID) || strings.HasPrefix(selfID, containerID)) -} - -func (s *Service) recordResult(ctx context.Context, result ResourceResult) error { - if s.config.RunRecorder == nil { - return nil - } - return s.config.RunRecorder.RecordUpdateRun(ctx, result) + return selfID != "" && containerID != "" && (strings.HasPrefix(containerID, selfID) || strings.HasPrefix(selfID, containerID)) } func (s *Service) notify(ctx context.Context, containerID, containerName, imageRef, oldImage, newImage string) error { @@ -58,8 +49,7 @@ func (s *Service) notify(ctx context.Context, containerID, containerName, imageR }) } -// recordEvent records a container-scoped update event; every event the -// updater emits today concerns a container, so the resource type is fixed. +// recordEvent records a container-scoped update event; every event the updater emits concerns a container. func (s *Service) recordEvent(ctx context.Context, phase, resourceID, resourceName string, metadata map[string]any) error { if s.config.EventRecorder == nil { return nil diff --git a/updater/internal/compose/compose.go b/updater/internal/compose/compose.go index 5dbe69d..b65ae0a 100644 --- a/updater/internal/compose/compose.go +++ b/updater/internal/compose/compose.go @@ -2,21 +2,23 @@ // group containers into projects and services. package compose -import ( - "strings" - - kit "go.getarcane.app/kit/pkg" -) +import "strings" const ( // ProjectLabelKey is Docker Compose's project label key. ProjectLabelKey = "com.docker.compose.project" // ServiceLabelKey is Docker Compose's service label key. ServiceLabelKey = "com.docker.compose.service" - // WorkingDirLabelKey is Docker Compose's project working directory label key. + // WorkingDirLabelKey is Docker Compose's project directory label key. WorkingDirLabelKey = "com.docker.compose.project.working_dir" - // ConfigFilesLabelKey is Docker Compose's project config files label key. + // ConfigFilesLabelKey is Docker Compose's comma-separated config files label key. ConfigFilesLabelKey = "com.docker.compose.project.config_files" + // EnvironmentFileLabelKey is Docker Compose's comma-separated --env-file label key. + EnvironmentFileLabelKey = "com.docker.compose.project.environment_file" + // ImageLabelKey is the image content digest Compose compares on up. + ImageLabelKey = "com.docker.compose.image" + // ServiceContainerFilter excludes `docker compose run` one-off containers, as Compose does. + ServiceContainerFilter = "com.docker.compose.oneoff=False" ) // ProjectLabel returns the trimmed Docker Compose project label. @@ -28,17 +30,3 @@ func ProjectLabel(labels map[string]string) string { func ServiceLabel(labels map[string]string) string { return strings.TrimSpace(labels[ServiceLabelKey]) } - -// WorkingDirLabel returns the trimmed Docker Compose project working directory label. -func WorkingDirLabel(labels map[string]string) string { - return strings.TrimSpace(labels[WorkingDirLabelKey]) -} - -// ConfigFilesLabel returns Docker Compose project config file labels. -func ConfigFilesLabel(labels map[string]string) []string { - raw := strings.TrimSpace(labels[ConfigFilesLabelKey]) - if raw == "" { - return nil - } - return kit.TrimNonEmpty(strings.Split(raw, ",")) -} diff --git a/updater/internal/compose/compose_test.go b/updater/internal/compose/compose_test.go index 9a2d14e..741a07f 100644 --- a/updater/internal/compose/compose_test.go +++ b/updater/internal/compose/compose_test.go @@ -4,10 +4,8 @@ import "testing" func TestComposeLabels(t *testing.T) { labels := map[string]string{ - ProjectLabelKey: " app ", - ServiceLabelKey: "web", - WorkingDirLabelKey: " /srv/app ", - ConfigFilesLabelKey: " compose.yaml, compose.override.yaml ,, ", + ProjectLabelKey: " app ", + ServiceLabelKey: "web", } if got := ProjectLabel(labels); got != "app" { t.Fatalf("ProjectLabel() = %q, want app", got) @@ -15,11 +13,4 @@ func TestComposeLabels(t *testing.T) { if got := ServiceLabel(labels); got != "web" { t.Fatalf("ServiceLabel() = %q, want web", got) } - if got := WorkingDirLabel(labels); got != "/srv/app" { - t.Fatalf("WorkingDirLabel() = %q, want /srv/app", got) - } - files := ConfigFilesLabel(labels) - if len(files) != 2 || files[0] != "compose.yaml" || files[1] != "compose.override.yaml" { - t.Fatalf("ConfigFilesLabel() = %#v, want two config files", files) - } } diff --git a/updater/internal/deps/sorter.go b/updater/internal/deps/sorter.go index 1f0df64..1fd2eca 100644 --- a/updater/internal/deps/sorter.go +++ b/updater/internal/deps/sorter.go @@ -59,9 +59,8 @@ func (s *ContainerSorter) Sort() ([]ContainerWithDeps, error) { return s.sorted, nil } -// ExtractContainerDeps extracts dependency information from a container inspect -// response. name is the caller's display name for the container, which the -// returned value is keyed by. +// ExtractContainerDeps extracts dependency information from a container +// inspect response, keyed by the caller's display name for the container. func ExtractContainerDeps(ctx context.Context, name string, cnt container.Summary, inspect container.InspectResponse) ContainerWithDeps { c := ContainerWithDeps{ Container: cnt, @@ -74,24 +73,13 @@ func ExtractContainerDeps(ctx context.Context, name string, cnt container.Summar linkName, _, _ := strings.Cut(link, ":") c.Links = append(c.Links, strings.TrimPrefix(linkName, "/")) } - } - - if inspect.Config != nil && inspect.Config.Labels != nil { - if deps, ok := inspect.Config.Labels[labels.LabelDependsOn]; ok { - for dep := range strings.SplitSeq(deps, ",") { - dep = strings.TrimSpace(dep) - if dep != "" { - c.DependsOn = append(c.DependsOn, dep) - } - } + if inspect.HostConfig.NetworkMode.IsContainer() { + c.NetworkDeps = append(c.NetworkDeps, inspect.HostConfig.NetworkMode.ConnectedContainer()) } } - - if inspect.HostConfig != nil { - networkMode := inspect.HostConfig.NetworkMode - if networkMode.IsContainer() { - containerRef := strings.TrimPrefix(string(networkMode), "container:") - c.NetworkDeps = append(c.NetworkDeps, containerRef) + if inspect.Config != nil { + if deps, ok := inspect.Config.Labels[labels.LabelDependsOn]; ok { + c.DependsOn = kit.TrimNonEmpty(strings.Split(deps, ",")) } } @@ -99,9 +87,8 @@ func ExtractContainerDeps(ctx context.Context, name string, cnt container.Summar return c } -// UpdateImplicitRestart adds every container that depends on one already in -// markedForRestart, and returns the names it newly marked. Callers repeat the -// call until it returns nothing to reach the transitive closure. +// UpdateImplicitRestart marks every container depending on a marked one and +// returns the newly marked names; repeat until it returns none. func UpdateImplicitRestart(containers []ContainerWithDeps, markedForRestart map[string]bool) []string { var implicitRestarts []string for _, c := range containers { diff --git a/updater/internal/digestcheck/digestcheck.go b/updater/internal/digestcheck/digestcheck.go index 86ae962..351dc4c 100644 --- a/updater/internal/digestcheck/digestcheck.go +++ b/updater/internal/digestcheck/digestcheck.go @@ -1,6 +1,5 @@ -// Package digestcheck compares the image digests Docker holds locally with the -// digests a registry reports, so the updater can tell a real update from a -// no-op re-pull. +// Package digestcheck compares local image digests with registry digests, so the updater can tell a +// real update from a no-op re-pull. package digestcheck import ( @@ -9,6 +8,7 @@ import ( "fmt" "log/slog" "math" + "slices" "strings" "github.com/moby/moby/client" @@ -18,9 +18,8 @@ import ( "go.getarcane.app/updater/refs" ) -// Resolver resolves a remote image digest without pulling. It is declared -// structurally so the root package's public resolver interface satisfies it -// without this package importing the root. +// Resolver resolves a remote image digest without pulling. It is structural so +// the root package's public resolver satisfies it without an import cycle. type Resolver interface { ImageDigest(ctx context.Context, imageRef string) (string, error) } @@ -40,12 +39,31 @@ type CheckResult struct { CheckedViaAPI bool } -// NewChecker creates a digest checker. Both arguments are optional; the checks -// that need them report an error instead of panicking when they are absent. +// NewChecker creates a digest checker. Both arguments are optional; checks that +// need a missing one report an error. func NewChecker(dockerClient *client.Client, digestResolver Resolver) *Checker { return &Checker{dockerClient: dockerClient, digestResolver: digestResolver} } +// RepoDigests returns the normalized digests in repoDigests that belong to +// imageRef's repository, ignoring digests the image carries for other repositories. +func RepoDigests(repoDigests []string, imageRef string) []string { + target, err := refs.NormalizeReference(imageRef) + if err != nil { + return nil + } + var out []string + for _, repoDigest := range repoDigests { + name, _, _ := strings.Cut(repoDigest, "@") + repo, repoErr := refs.NormalizeReference(name) + normalized, ok := digest.FromReferenceSuffix(repoDigest) + if repoErr == nil && ok && repo.RegistryHost == target.RegistryHost && repo.Repository == target.Repository { + out = append(out, normalized) + } + } + return out +} + // CheckImageNeedsUpdate compares local and remote digests for an image. func (c *Checker) CheckImageNeedsUpdate(ctx context.Context, imageRef string) CheckResult { result := CheckResult{} @@ -61,13 +79,22 @@ func (c *Checker) CheckImageNeedsUpdate(ctx context.Context, imageRef string) Ch slog.DebugContext(ctx, "CheckImageNeedsUpdate: checking image", "imageRef", imageRef, "normalizedRef", refs.NormalizeImageUpdateRef(imageRef)) - localDigest, err := c.localDigest(ctx, imageRef) + inspect, err := c.dockerClient.ImageInspect(ctx, imageRef) if err != nil { result.NeedsUpdate = true - result.Error = err + result.Error = fmt.Errorf("image not found locally: %w", err) + return result + } + localDigests := RepoDigests(inspect.RepoDigests, imageRef) + if len(localDigests) == 0 && inspect.ID != "" { + localDigests = []string{inspect.ID} + } + if len(localDigests) == 0 { + result.NeedsUpdate = true + result.Error = errors.New("no digest available for image") return result } - result.LocalDigest = localDigest + result.LocalDigest = localDigests[0] if c.digestResolver == nil { result.Error = errors.New("remote digest resolver unavailable") @@ -82,7 +109,7 @@ func (c *Checker) CheckImageNeedsUpdate(ctx context.Context, imageRef string) Ch result.RemoteDigest = remoteDigest result.CheckedViaAPI = true - result.NeedsUpdate = localDigest != remoteDigest + result.NeedsUpdate = !slices.Contains(localDigests, remoteDigest) return result } @@ -110,19 +137,14 @@ func (c *Checker) CheckImageMatchesKnownDigest(ctx context.Context, imageRef, kn return result } - for _, repoDigest := range inspect.RepoDigests { - localDigest, ok := digest.FromReferenceSuffix(repoDigest) - if !ok { - continue - } - result.LocalDigest = localDigest - if localDigest == normalizedDigest { - return result - } + localDigests := RepoDigests(inspect.RepoDigests, imageRef) + if len(localDigests) == 0 { + localDigests = []string{strings.TrimSpace(inspect.ID)} } - - if result.LocalDigest == "" && strings.TrimSpace(inspect.ID) != "" { - result.LocalDigest = strings.TrimSpace(inspect.ID) + result.LocalDigest = localDigests[0] + if slices.Contains(localDigests, normalizedDigest) { + result.LocalDigest = normalizedDigest + return result } result.NeedsUpdate = true return result @@ -159,11 +181,8 @@ func (c *Checker) GetImageIDsForRef(ctx context.Context, ref string) ([]string, normalizedRef := refs.NormalizeImageUpdateRef(ref) var ids []string for _, img := range imageList.Items { - for _, tag := range img.RepoTags { - if refs.NormalizeImageUpdateRef(tag) == normalizedRef { - ids = append(ids, img.ID) - break - } + if slices.ContainsFunc(img.RepoTags, func(tag string) bool { return refs.NormalizeImageUpdateRef(tag) == normalizedRef }) { + ids = append(ids, img.ID) } } return ids, nil @@ -179,8 +198,7 @@ type RefIDCache struct { func NewRefIDCache(checker *Checker) *RefIDCache { return &RefIDCache{ checker: checker, - // This cache is a per-scan snapshot; eviction could make repeated - // lookups for one ref observe different Docker image state. + // Never evict: repeated lookups for one ref must see one Docker snapshot. ids: hot.NewHotCache[string, []string](hot.LRU, math.MaxInt). WithoutLocking(). Build(), @@ -197,19 +215,3 @@ func (c *RefIDCache) IDsForRef(ctx context.Context, ref string) []string { c.ids.Set(ref, ids) return ids } - -func (c *Checker) localDigest(ctx context.Context, imageRef string) (string, error) { - inspect, err := c.dockerClient.ImageInspect(ctx, imageRef) - if err != nil { - return "", fmt.Errorf("image not found locally: %w", err) - } - for _, repoDigest := range inspect.RepoDigests { - if normalized, ok := digest.FromReferenceSuffix(repoDigest); ok { - return normalized, nil - } - } - if inspect.ID != "" { - return inspect.ID, nil - } - return "", errors.New("no digest available for image") -} diff --git a/updater/internal/match/matching.go b/updater/internal/match/matching.go index 57bab1a..d1eee6c 100644 --- a/updater/internal/match/matching.go +++ b/updater/internal/match/matching.go @@ -1,14 +1,11 @@ package match import ( - "context" - "fmt" + "cmp" "slices" "strings" "github.com/moby/moby/api/types/container" - "github.com/moby/moby/client" - "go.getarcane.app/docker/compat" "go.getarcane.app/updater/digest" "go.getarcane.app/updater/internal/compose" @@ -18,10 +15,7 @@ import ( // AppendImageUpdateRecordIDToOldIDs includes SHA-like update record IDs in the old-image match set. func AppendImageUpdateRecordIDToOldIDs(oldIDs []string, recordID string) []string { recordID = strings.TrimSpace(recordID) - if !refs.IsImageIDLikeReference(recordID) { - return oldIDs - } - if slices.Contains(oldIDs, recordID) { + if !refs.IsImageIDLikeReference(recordID) || slices.Contains(oldIDs, recordID) { return oldIDs } return append(oldIDs, recordID) @@ -29,27 +23,28 @@ func AppendImageUpdateRecordIDToOldIDs(oldIDs []string, recordID string) []strin // ResolveContainerImageMatch finds the new image reference for a running container. func ResolveContainerImageMatch(c container.Summary, inspect *container.InspectResponse, oldIDToNewRef, updatedNorm map[string]string) (newRef, match string) { - if c.ImageID != "" { - if nr, ok := oldIDToNewRef[c.ImageID]; ok { - return nr, c.ImageID - } + if nr, ok := oldIDToNewRef[c.ImageID]; ok && c.ImageID != "" { + return nr, c.ImageID } if inspect != nil && inspect.Image != "" { if nr, ok := oldIDToNewRef[inspect.Image]; ok { return nr, inspect.Image } } - if newRef, match = resolveImageRefMatch(c.Image, updatedNorm); newRef != "" { - return newRef, match - } - if inspect != nil && inspect.Config != nil { - if newRef, match = resolveImageRefMatch(inspect.Config.Image, updatedNorm); newRef != "" { - return newRef, match + candidates := []string{c.Image} + if inspect != nil { + if inspect.Config != nil { + candidates = append(candidates, inspect.Config.Image) } + candidates = append(candidates, inspect.Image) } - if inspect != nil { - if newRef, match = resolveImageRefMatch(inspect.Image, updatedNorm); newRef != "" { - return newRef, match + for _, imageRef := range candidates { + if refs.IsImageIDLikeReference(imageRef) { + continue + } + norm := refs.NormalizeImageUpdateRef(imageRef) + if nr, ok := updatedNorm[norm]; ok && norm != "" { + return nr, norm } } return "", "" @@ -57,11 +52,10 @@ func ResolveContainerImageMatch(c container.Summary, inspect *container.InspectR // ShouldInspectUnmatchedContainerForImageMatch reports whether inspect may recover a tag match. func ShouldInspectUnmatchedContainerForImageMatch(c container.Summary) bool { - imageRef := strings.TrimSpace(c.Image) - if imageRef == "" || refs.IsImageIDLikeReference(imageRef) { + if strings.TrimSpace(c.Image) == "" || refs.IsImageIDLikeReference(c.Image) { return true } - if _, isDigestRef := digest.FromReferenceSuffix(imageRef); !isDigestRef { + if _, isDigestRef := digest.FromReferenceSuffix(c.Image); !isDigestRef { return false } return compose.ProjectLabel(c.Labels) != "" || compose.ServiceLabel(c.Labels) != "" @@ -69,63 +63,8 @@ func ShouldInspectUnmatchedContainerForImageMatch(c container.Summary) bool { // CurrentContainerImageID returns the best available image ID for a container. func CurrentContainerImageID(c container.Summary, inspect *container.InspectResponse) string { - if imageID := strings.TrimSpace(c.ImageID); imageID != "" { - return imageID - } - if inspect != nil { - return strings.TrimSpace(inspect.Image) - } - return "" -} - -// VerifyComposeServiceUpdatedImage verifies that a compose service no longer runs oldImageID. -func VerifyComposeServiceUpdatedImage(ctx context.Context, dockerClient *client.Client, projectName, serviceName, oldImageID string) error { - projectName = strings.TrimSpace(projectName) - serviceName = strings.TrimSpace(serviceName) - oldImageID = strings.TrimSpace(oldImageID) - if dockerClient == nil || projectName == "" || serviceName == "" || oldImageID == "" { - return nil - } - - filters := make(client.Filters) - filters = filters.Add("label", compose.ProjectLabelKey+"="+projectName) - filters = filters.Add("label", compose.ServiceLabelKey+"="+serviceName) - - listResult, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{All: false, Filters: filters}) - if err != nil { - return fmt.Errorf("verify compose service image: list containers: %w", err) - } - if len(listResult.Items) == 0 { - return fmt.Errorf("compose service %s/%s has no running container after update", projectName, serviceName) - } - - for _, c := range listResult.Items { - currentImageID := strings.TrimSpace(c.ImageID) - if currentImageID == "" { - inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, c.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { - return fmt.Errorf("verify compose service image: inspect container %s: %w", c.ID, inspectErr) - } - currentImageID = strings.TrimSpace(inspectResult.Container.Image) - } - if currentImageID == oldImageID { - return fmt.Errorf("compose service %s/%s still running old image %s after update", projectName, serviceName, oldImageID) - } + if inspect == nil { + return strings.TrimSpace(c.ImageID) } - return nil -} - -func resolveImageRefMatch(imageRef string, updatedNorm map[string]string) (newRef, match string) { - imageRef = strings.TrimSpace(imageRef) - if imageRef == "" || refs.IsImageIDLikeReference(imageRef) { - return "", "" - } - norm := refs.NormalizeImageUpdateRef(imageRef) - if norm == "" { - return "", "" - } - if nr, ok := updatedNorm[norm]; ok { - return nr, norm - } - return "", "" + return cmp.Or(strings.TrimSpace(c.ImageID), strings.TrimSpace(inspect.Image)) } diff --git a/updater/internal/registryhost/registryhost.go b/updater/internal/registryhost/registryhost.go deleted file mode 100644 index fc26fc4..0000000 --- a/updater/internal/registryhost/registryhost.go +++ /dev/null @@ -1,28 +0,0 @@ -// Package registryhost resolves the registry hosts the Docker daemon expects -// for authentication. Canonicalizing hosts for equality checks lives in -// go.getarcane.app/kit/pkg/registry. -package registryhost - -import ( - "fmt" - - ref "github.com/distribution/reference" -) - -const ( - defaultRegistryDomain = "docker.io" - defaultRegistryHost = "registry-1.docker.io" -) - -// AuthAddress returns the Docker daemon auth address for an image reference. -func AuthAddress(imageRef string) (string, error) { - named, err := ref.ParseNormalizedNamed(imageRef) - if err != nil { - return "", fmt.Errorf("parse image reference: %w", err) - } - addr := ref.Domain(named) - if addr == defaultRegistryDomain { - return defaultRegistryHost, nil - } - return addr, nil -} diff --git a/updater/internal/registryhost/registryhost_test.go b/updater/internal/registryhost/registryhost_test.go deleted file mode 100644 index 002539d..0000000 --- a/updater/internal/registryhost/registryhost_test.go +++ /dev/null @@ -1,25 +0,0 @@ -package registryhost - -import "testing" - -func TestAuthAddress(t *testing.T) { - got, err := AuthAddress("nginx:1.27") - if err != nil { - t.Fatalf("AuthAddress() error = %v", err) - } - if got != "registry-1.docker.io" { - t.Fatalf("AuthAddress() = %q, want registry-1.docker.io", got) - } - - got, err = AuthAddress("ghcr.io/getarcaneapp/arcane:v2") - if err != nil { - t.Fatalf("AuthAddress() custom error = %v", err) - } - if got != "ghcr.io" { - t.Fatalf("AuthAddress() custom = %q, want ghcr.io", got) - } - - if _, err = AuthAddress("not a reference"); err == nil { - t.Fatal("AuthAddress() error = nil, want parse failure") - } -} diff --git a/updater/labelpolicy.go b/updater/labelpolicy.go index 2aae383..87ed8b3 100644 --- a/updater/labelpolicy.go +++ b/updater/labelpolicy.go @@ -7,10 +7,8 @@ import ( "go.getarcane.app/updater/types" ) -// LabelPolicy decides, from a container's labels, whether the updater may touch -// it, whether the host application must update it itself, and how to stop it. -// Every field is optional; a nil func answers false (or "" for StopSignalFunc), -// and New fills the nil ones in from DefaultLabelPolicy. +// LabelPolicy decides from a container's labels whether the updater may touch it, whether the host must +// update it itself, and how to stop it. New fills nil funcs from DefaultLabelPolicy. type LabelPolicy struct { TagPolicyFunc func(map[string]string) types.Policy IsUpdateDisabledFunc func(map[string]string) bool @@ -41,9 +39,8 @@ func DefaultLabelPolicy() LabelPolicy { } } -// mergeLabelPolicyDefaults fills every nil func in policy from -// DefaultLabelPolicy. The merge is per field, so a caller that overrides one -// behavior keeps the defaults for the rest. +// mergeLabelPolicyDefaults fills every nil func in policy from DefaultLabelPolicy, field by field, so +// overriding one behavior keeps the defaults for the rest. func mergeLabelPolicyDefaults(policy LabelPolicy) LabelPolicy { defaults := DefaultLabelPolicy() if policy.TagPolicyFunc == nil { diff --git a/updater/labels/labels.go b/updater/labels/labels.go index 1f88648..d19b875 100644 --- a/updater/labels/labels.go +++ b/updater/labels/labels.go @@ -1,7 +1,5 @@ -// Package labels reads the container labels that tell the updater what a -// container is: whether it opts out of updates, whether it is an Arcane server -// or agent that must update itself, whether it belongs to Docker Swarm, and how -// it wants to be stopped. +// Package labels reads the container labels that tell the updater whether a container opts out, is an +// Arcane server or agent that updates itself, belongs to Swarm, and how it wants to be stopped. package labels import ( @@ -45,18 +43,19 @@ func IsArcaneServerContainer(labels map[string]string) bool { return (hasTruthyLabel(labels, LabelArcane) || hasTruthyLabel(labels, LabelArcaneLegacyServer)) && !IsArcaneAgentContainer(labels) } -// ShouldDisableArcaneServerRedeploy reports whether redeploy or edit should be blocked -// because the container is the running Arcane server or agent. +// ShouldDisableArcaneServerRedeploy reports whether redeploy or edit should be +// blocked because the container is the running Arcane server or agent. func ShouldDisableArcaneServerRedeploy(labels map[string]string, containerID, currentContainerID string, currentErr error) bool { if !IsArcaneContainer(labels) { return false } - if currentErr != nil || strings.TrimSpace(currentContainerID) == "" { + current := strings.TrimSpace(currentContainerID) + if currentErr != nil || current == "" { return true } - - return containerIDsMatch(containerID, currentContainerID) + containerID = strings.TrimSpace(containerID) + return containerID != "" && (strings.HasPrefix(containerID, current) || strings.HasPrefix(current, containerID)) } // IsArcaneAgentContainer reports whether labels identify an Arcane agent. @@ -75,7 +74,9 @@ func IsUpdateDisabled(labels map[string]string) bool { // IsSwarmTask reports whether labels identify a Docker Swarm task. func IsSwarmTask(labels map[string]string) bool { - return hasNonEmptyLabel(labels, LabelSwarmServiceID) || hasNonEmptyLabel(labels, LabelSwarmServiceName) + serviceID, _ := lookupLabel(labels, LabelSwarmServiceID) + serviceName, _ := lookupLabel(labels, LabelSwarmServiceName) + return strings.TrimSpace(serviceID) != "" || strings.TrimSpace(serviceName) != "" } // StopSignal returns a custom stop signal from labels. @@ -93,11 +94,6 @@ func hasTruthyLabel(labels map[string]string, target string) bool { return ok && truthy } -func hasNonEmptyLabel(labels map[string]string, target string) bool { - value, ok := lookupLabel(labels, target) - return ok && strings.TrimSpace(value) != "" -} - func lookupLabel(labels map[string]string, target string) (string, bool) { for key, value := range labels { if strings.EqualFold(key, target) { @@ -106,9 +102,3 @@ func lookupLabel(labels map[string]string, target string) (string, bool) { } return "", false } - -func containerIDsMatch(a, b string) bool { - a = strings.TrimSpace(a) - b = strings.TrimSpace(b) - return a != "" && b != "" && (strings.HasPrefix(a, b) || strings.HasPrefix(b, a)) -} diff --git a/updater/memory_store_test.go b/updater/memory_store_test.go index 30deb89..4d35f36 100644 --- a/updater/memory_store_test.go +++ b/updater/memory_store_test.go @@ -53,7 +53,7 @@ func TestMemoryPendingStoreRespectsCanceledContext(t *testing.T) { } } -func TestMemoryPendingStoreScopesSharedImageInternal(t *testing.T) { +func TestMemoryPendingStoreScopesSharedImage(t *testing.T) { latest := "1.1.0" first := ImageUpdateRecord{ID: "shared", ContainerID: "first", Repository: "app", Tag: "1.0.0", LatestVersion: &latest, HasUpdate: true, UpdateType: UpdateTypeTag} second := first diff --git a/updater/pkg/utils/tagpolicy/tagpolicy.go b/updater/pkg/utils/tagpolicy/tagpolicy.go index 5358b84..711dbcf 100644 --- a/updater/pkg/utils/tagpolicy/tagpolicy.go +++ b/updater/pkg/utils/tagpolicy/tagpolicy.go @@ -13,9 +13,8 @@ import ( "go.getarcane.app/updater/types" ) -// Resolve selects a strategy from the configured reference and validates tag policies. -// An undeclared strategy follows the digest unless a constraint or tag pattern is set; -// auto follows stable complete semantic versions and uses digests for other tags. +// Resolve validates policy and picks its strategy: digest unless a constraint or tag pattern is set, +// while auto follows stable complete semantic versions and uses digests for other tags. func Resolve(imageRef string, policy types.Policy) (types.Policy, error) { if refs.IsDigestPinnedReference(imageRef) || refs.IsImageIDLikeReference(imageRef) { policy.Strategy = "digest" @@ -40,7 +39,7 @@ func Resolve(imageRef string, policy types.Policy) (types.Policy, error) { return policy, nil } if strategy == "auto" && !hasRules { - version, parseErr := parseInternal(parsed.Tag, nil) + version, parseErr := parse(parsed.Tag, nil) if parseErr != nil || version.Prerelease() != "" { policy.Strategy = "digest" return policy, nil //nolint:nilerr // Non-version tags intentionally select digest updates in auto mode. @@ -57,11 +56,11 @@ func Resolve(imageRef string, policy types.Policy) (types.Policy, error) { // Select returns the highest eligible newer tag, or current when none qualifies. // Equal candidate versions are resolved using the lexically smallest tag. func Select(current string, tags []string, policy types.Policy) (string, error) { - pattern, constraint, err := compileInternal(policy) + pattern, constraint, err := compile(policy) if err != nil { return "", err } - currentVersion, err := parseInternal(current, pattern) + currentVersion, err := parse(current, pattern) if err != nil { return "", fmt.Errorf("invalid current tag %q: %w", current, err) } @@ -71,7 +70,7 @@ func Select(current string, tags []string, policy types.Policy) (string, error) selected := current best := currentVersion for _, tag := range tags { - version, parseErr := parseInternal(tag, pattern) + version, parseErr := parse(tag, pattern) if parseErr != nil || !version.GreaterThan(currentVersion) { continue } @@ -91,18 +90,18 @@ func Select(current string, tags []string, policy types.Policy) (string, error) // Version extracts and parses a tag's complete semantic version. func Version(tag string, policy types.Policy) (string, error) { - pattern, _, err := compileInternal(policy) + pattern, _, err := compile(policy) if err != nil { return "", err } - version, err := parseInternal(tag, pattern) + version, err := parse(tag, pattern) if err != nil { return "", err } return version.String(), nil } -func compileInternal(policy types.Policy) (*regexp.Regexp, *semver.Constraints, error) { +func compile(policy types.Policy) (*regexp.Regexp, *semver.Constraints, error) { var pattern *regexp.Regexp if policy.TagPattern != "" { var err error @@ -131,7 +130,7 @@ func compileInternal(policy types.Policy) (*regexp.Regexp, *semver.Constraints, return pattern, constraint, nil } -func parseInternal(tag string, pattern *regexp.Regexp) (*semver.Version, error) { +func parse(tag string, pattern *regexp.Regexp) (*semver.Version, error) { value := tag if pattern != nil { matches := pattern.FindStringSubmatch(tag) diff --git a/updater/ports.go b/updater/ports.go index efe1995..10c6ee7 100644 --- a/updater/ports.go +++ b/updater/ports.go @@ -7,9 +7,8 @@ import ( "github.com/moby/moby/client" ) -// DockerClientProvider provides Docker clients. The provider owns the returned -// client's lifecycle; Service never closes returned clients. Providers may -// return the same client across calls. +// DockerClientProvider provides Docker clients. The provider owns their lifecycle, so Service never +// closes them, and it may return the same client across calls. type DockerClientProvider interface { DockerClient(ctx context.Context) (*client.Client, error) } diff --git a/updater/refs/reference.go b/updater/refs/reference.go index fb24924..2be9262 100644 --- a/updater/refs/reference.go +++ b/updater/refs/reference.go @@ -1,6 +1,5 @@ -// Package refs normalizes Docker image references so that the many spellings of -// one image — "nginx", "nginx:latest", "docker.io/library/nginx:latest" — compare -// equal, and identifies the references that cannot meaningfully be updated. +// Package refs normalizes image references so every spelling of one image ("nginx", "nginx:latest", +// "docker.io/library/nginx:latest") compares equal, and flags references that cannot be updated. package refs import ( @@ -60,9 +59,8 @@ func NormalizeImageUpdateRef(imageRef string) string { return parts.NormalizedRef } -// NormalizeImageUpdateRefMapKeys returns a copy of refToValue keyed by the -// normalized form of each reference; entries whose keys fail to normalize are -// dropped. +// NormalizeImageUpdateRefMapKeys returns refToValue keyed by normalized reference, dropping keys that +// fail to normalize. func NormalizeImageUpdateRefMapKeys(refToValue map[string]string) map[string]string { out := make(map[string]string, len(refToValue)) for imageRef, value := range refToValue { @@ -73,11 +71,8 @@ func NormalizeImageUpdateRefMapKeys(refToValue map[string]string) map[string]str return out } -// PreserveConfiguredRef returns targetRef spelled like configuredRef. An -// equivalent target returns configuredRef itself; a target in the same -// repository keeps the configured repository and takes the target's tag. -// Image IDs, digest-pinned targets, and other repositories return targetRef -// unchanged. +// PreserveConfiguredRef returns targetRef spelled like configuredRef: the configured repository with the +// target's tag. Image IDs, digest-pinned targets, and other repositories come back unchanged. func PreserveConfiguredRef(configuredRef, targetRef string) string { configuredRef = strings.TrimSpace(configuredRef) targetRef = strings.TrimSpace(targetRef) @@ -129,29 +124,14 @@ func IsDigestPinnedReference(imageRef string) bool { return err == nil } -// PullableImageRef chooses the best reference to pull for a container from what -// Docker reports about it: the image in its config, the image on its list -// summary, then its image's repo tags. Image IDs and digest-pinned references -// are rejected because re-pulling them can never produce a newer image, so "" -// means the container has no updatable reference. +// PullableImageRef picks the reference to pull for a container: its config +// image, its summary image, then its repo tags. Image IDs and digests never update, so "" means none. func PullableImageRef(summaryImage, inspectConfigImage string, repoTags []string) string { - if image := strings.TrimSpace(inspectConfigImage); isMutablePullableRef(image) { - return image - } - if image := strings.TrimSpace(summaryImage); isMutablePullableRef(image) { - return image - } - for _, tag := range repoTags { - trimmed := strings.TrimSpace(tag) - if trimmed == ":" || !isMutablePullableRef(trimmed) { - continue + for _, imageRef := range append([]string{inspectConfigImage, summaryImage}, repoTags...) { + imageRef = strings.TrimSpace(imageRef) + if imageRef != "" && imageRef != ":" && !IsImageIDLikeReference(imageRef) && !IsDigestPinnedReference(imageRef) { + return imageRef } - return trimmed } return "" } - -func isMutablePullableRef(imageRef string) bool { - imageRef = strings.TrimSpace(imageRef) - return imageRef != "" && !IsImageIDLikeReference(imageRef) && !IsDigestPinnedReference(imageRef) -} diff --git a/updater/registry/registry_http.go b/updater/registry/registry_http.go index c9f89be..8a93dbc 100644 --- a/updater/registry/registry_http.go +++ b/updater/registry/registry_http.go @@ -3,6 +3,7 @@ package registry import ( + "cmp" "context" "errors" "fmt" @@ -68,12 +69,15 @@ func FetchRegistryRateLimit(ctx context.Context, registryHost, repository, tag s requestCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - ref, err := manifestReferenceInternal(registryHost, repository, tag) + repo, err := parseRepository(registryHost, repository) if err != nil { return nil, err } - repo := ref.Context() - authorized, err := transport.NewWithContext(requestCtx, repo.Registry, authenticatorInternal(credential), baseTransportInternal(httpClient), []string{repo.Scope(transport.PullScope)}) + ref, err := name.NewTag(repo.Name() + ":" + strings.TrimSpace(tag)) + if err != nil { + return nil, fmt.Errorf("parse image tag: %w", err) + } + authorized, err := transport.NewWithContext(requestCtx, repo.Registry, authenticator(credential), baseTransport(httpClient), []string{repo.Scope(transport.PullScope)}) if err != nil { return nil, fmt.Errorf("authorize registry: %w", err) } @@ -99,7 +103,26 @@ func FetchRegistryRateLimit(ctx context.Context, registryHost, repository, tag s if err != nil { return nil, err } - return extractRateLimitFromHeaders(resp.Header) + + info := &RateLimitInfo{} + if limit, window := parseRateLimitHeader(resp.Header.Get("Ratelimit-Limit")); limit != nil { + info.Limit = limit + info.WindowSeconds = window + info.Source = registryRateLimitHeaderSource + } + if remaining, window := parseRateLimitHeader(resp.Header.Get("Ratelimit-Remaining")); remaining != nil { + info.Remaining = remaining + info.WindowSeconds = cmp.Or(info.WindowSeconds, window) + info.Source = registryRateLimitHeaderSource + } + if used, usedErr := strconv.Atoi(strings.TrimSpace(resp.Header.Get("Docker-Ratelimit-Used"))); usedErr == nil { + info.Used = &used + info.Source = cmp.Or(info.Source, "docker") + } + if info.Limit == nil && info.Remaining == nil && info.Used == nil { + return nil, errors.New("no registry rate limit headers found") + } + return info, nil } // FetchDigest fetches the manifest digest for a registry image reference. @@ -107,11 +130,15 @@ func FetchDigest(ctx context.Context, registryHost, repository, tag string, cred requestCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - ref, err := manifestReferenceInternal(registryHost, repository, tag) + repo, err := parseRepository(registryHost, repository) if err != nil { return "", err } - options := remoteOptionsInternal(requestCtx, credential, httpClient) + ref, err := name.NewTag(repo.Name() + ":" + strings.TrimSpace(tag)) + if err != nil { + return "", fmt.Errorf("parse image tag: %w", err) + } + options := []remote.Option{remote.WithContext(requestCtx), remote.WithAuth(authenticator(credential)), remote.WithTransport(baseTransport(httpClient))} // HEAD does not count as a pull. Fall back to GET only when the registry // answered without a digest header, since GET computes it from the manifest. @@ -129,7 +156,7 @@ func FetchDigest(ctx context.Context, registryHost, repository, tag string, cred return desc.Digest.String(), nil } -func repositoryInternal(registryHost, repository string) (name.Repository, error) { +func parseRepository(registryHost, repository string) (name.Repository, error) { registryHost = kitregistry.Normalize(registryHost) if registryHost == "docker.io" { registryHost = defaultRegistryHost @@ -141,66 +168,20 @@ func repositoryInternal(registryHost, repository string) (name.Repository, error return repo, nil } -func manifestReferenceInternal(registryHost, repository, tag string) (name.Tag, error) { - repo, err := repositoryInternal(registryHost, repository) - if err != nil { - return name.Tag{}, err - } - ref, err := name.NewTag(repo.Name() + ":" + strings.TrimSpace(tag)) - if err != nil { - return name.Tag{}, fmt.Errorf("parse image tag: %w", err) - } - return ref, nil -} - -func remoteOptionsInternal(ctx context.Context, credential *authn.AuthConfig, httpClient *http.Client) []remote.Option { - return []remote.Option{ - remote.WithContext(ctx), - remote.WithAuth(authenticatorInternal(credential)), - remote.WithTransport(baseTransportInternal(httpClient)), - } -} - -func baseTransportInternal(httpClient *http.Client) http.RoundTripper { +func baseTransport(httpClient *http.Client) http.RoundTripper { if httpClient != nil && httpClient.Transport != nil { return httpClient.Transport } return http.DefaultTransport } -func authenticatorInternal(credential *authn.AuthConfig) authn.Authenticator { +func authenticator(credential *authn.AuthConfig) authn.Authenticator { if credential == nil || *credential == (authn.AuthConfig{}) { return authn.Anonymous } return authn.FromConfig(*credential) } -func extractRateLimitFromHeaders(header http.Header) (*RateLimitInfo, error) { - info := &RateLimitInfo{} - if limit, window := parseRateLimitHeader(header.Get("Ratelimit-Limit")); limit != nil { - info.Limit = limit - info.WindowSeconds = window - info.Source = registryRateLimitHeaderSource - } - if remaining, window := parseRateLimitHeader(header.Get("Ratelimit-Remaining")); remaining != nil { - info.Remaining = remaining - if info.WindowSeconds == nil { - info.WindowSeconds = window - } - info.Source = registryRateLimitHeaderSource - } - if used, err := strconv.Atoi(strings.TrimSpace(header.Get("Docker-Ratelimit-Used"))); err == nil { - info.Used = &used - if info.Source == "" { - info.Source = "docker" - } - } - if info.Limit == nil && info.Remaining == nil && info.Used == nil { - return nil, errors.New("no registry rate limit headers found") - } - return info, nil -} - func parseRateLimitHeader(value string) (*int, *int) { value = strings.TrimSpace(value) if value == "" { diff --git a/updater/registry/registry_http_test.go b/updater/registry/registry_http_test.go index 4de5179..81ebf7a 100644 --- a/updater/registry/registry_http_test.go +++ b/updater/registry/registry_http_test.go @@ -43,17 +43,17 @@ func TestIsFallbackEligibleDaemonError(t *testing.T) { const testDigest = "sha256:1111111111111111111111111111111111111111111111111111111111111111" -// fakeRegistryInternal serves registry.test with bearer auth from auth.test and +// fakeRegistry serves registry.test with bearer auth from auth.test and // passes authorized requests to handle. -func fakeRegistryInternal(t *testing.T, realm string, checkToken func(*http.Request), handle func(*http.Request) *http.Response) *http.Client { +func fakeRegistry(t *testing.T, realm string, checkToken func(*http.Request), handle func(*http.Request) *http.Response) *http.Client { t.Helper() return &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { if r.URL.Host == "auth.test" { checkToken(r) - return tagsResponseInternal(r, http.StatusOK, `{"token":"registry-token"}`), nil + return tagsResponse(r, http.StatusOK, `{"token":"registry-token"}`), nil } if r.Header.Get("Authorization") != "Bearer registry-token" { - resp := tagsResponseInternal(r, http.StatusUnauthorized, "") + resp := tagsResponse(r, http.StatusUnauthorized, "") resp.Header.Set("WWW-Authenticate", `Bearer realm="`+realm+`",service="registry.test"`) return resp, nil } @@ -61,8 +61,8 @@ func fakeRegistryInternal(t *testing.T, realm string, checkToken func(*http.Requ })} } -func manifestResponseInternal(r *http.Request, body, digest string) *http.Response { - resp := tagsResponseInternal(r, http.StatusOK, body) +func manifestResponse(r *http.Request, body, digest string) *http.Response { + resp := tagsResponse(r, http.StatusOK, body) if r.Method == http.MethodHead { resp.Body = http.NoBody } @@ -100,7 +100,7 @@ func TestFetchDigestUsesHeadWithTokenAuth(t *testing.T) { }}, } { t.Run(tt.name, func(t *testing.T) { - client := fakeRegistryInternal(t, "https://auth.test/token", func(r *http.Request) { + client := fakeRegistry(t, "https://auth.test/token", func(r *http.Request) { if r.URL.Query().Get("scope") != "repository:team/app:pull" && r.FormValue("scope") != "repository:team/app:pull" { t.Errorf("token request missing pull scope: %s", r.URL) } @@ -114,7 +114,7 @@ func TestFetchDigestUsesHeadWithTokenAuth(t *testing.T) { t.Errorf("Accept = %q, want %s", r.Header.Get("Accept"), mediaType) } } - return manifestResponseInternal(r, "{}", testDigest) + return manifestResponse(r, "{}", testDigest) }) digest, err := FetchDigest(t.Context(), "registry.test", "team/app", "1.2.3", tt.credential, client) if err != nil || digest != testDigest { @@ -127,8 +127,8 @@ func TestFetchDigestUsesHeadWithTokenAuth(t *testing.T) { func TestFetchDigestFallsBackToGetWithoutDigestHeader(t *testing.T) { body := `{"schemaVersion":2}` sum := sha256.Sum256([]byte(body)) - client := fakeRegistryInternal(t, "https://auth.test/token", func(*http.Request) {}, func(r *http.Request) *http.Response { - return manifestResponseInternal(r, body, "") + client := fakeRegistry(t, "https://auth.test/token", func(*http.Request) {}, func(r *http.Request) *http.Response { + return manifestResponse(r, body, "") }) digest, err := FetchDigest(t.Context(), "registry.test", "team/app", "1.2.3", nil, client) if want := "sha256:" + hex.EncodeToString(sum[:]); err != nil || digest != want { @@ -137,11 +137,11 @@ func TestFetchDigestFallsBackToGetWithoutDigestHeader(t *testing.T) { } func TestFetchDigestDoesNotFallBackOnRegistryError(t *testing.T) { - client := fakeRegistryInternal(t, "https://auth.test/token", func(*http.Request) {}, func(r *http.Request) *http.Response { + client := fakeRegistry(t, "https://auth.test/token", func(*http.Request) {}, func(r *http.Request) *http.Response { if r.Method != http.MethodHead { t.Errorf("unexpected %s after registry error", r.Method) } - return tagsResponseInternal(r, http.StatusNotFound, "") + return tagsResponse(r, http.StatusNotFound, "") }) if _, err := FetchDigest(t.Context(), "registry.test", "team/app", "1.2.3", nil, client); err == nil { t.Fatal("FetchDigest returned nil error") @@ -149,10 +149,10 @@ func TestFetchDigestDoesNotFallBackOnRegistryError(t *testing.T) { } func TestFetchDigestRejectsNonHTTPSAuthRealm(t *testing.T) { - client := fakeRegistryInternal(t, "http://auth.test/token", func(*http.Request) { + client := fakeRegistry(t, "http://auth.test/token", func(*http.Request) { t.Error("token requested from non-HTTPS realm") }, func(r *http.Request) *http.Response { - return manifestResponseInternal(r, "{}", testDigest) + return manifestResponse(r, "{}", testDigest) }) if _, err := FetchDigest(t.Context(), "registry.test", "team/app", "1.2.3", nil, client); err == nil { t.Fatal("FetchDigest returned nil error") @@ -161,7 +161,7 @@ func TestFetchDigestRejectsNonHTTPSAuthRealm(t *testing.T) { func TestFetchRegistryRateLimitUsesHead(t *testing.T) { for _, credential := range []*authn.AuthConfig{nil, {Username: "user", Password: "secret"}} { - client := fakeRegistryInternal(t, "https://auth.test/token", func(r *http.Request) { + client := fakeRegistry(t, "https://auth.test/token", func(r *http.Request) { if user, _, _ := r.BasicAuth(); credential != nil && user != "user" { t.Errorf("token user = %q, want user", user) } @@ -169,7 +169,7 @@ func TestFetchRegistryRateLimitUsesHead(t *testing.T) { if r.Method != http.MethodHead { t.Errorf("manifest method = %s, want HEAD", r.Method) } - resp := manifestResponseInternal(r, "{}", testDigest) + resp := manifestResponse(r, "{}", testDigest) resp.Header.Set("RateLimit-Limit", "100;w=21600") resp.Header.Set("RateLimit-Remaining", "90;w=21600") return resp diff --git a/updater/registry/tags.go b/updater/registry/tags.go index 4eb1069..f3e7e4a 100644 --- a/updater/registry/tags.go +++ b/updater/registry/tags.go @@ -15,9 +15,8 @@ const ( defaultTagsFetchTimeout = 120 * time.Second ) -// FetchTags lists all repository tags, including paginated results. A partial -// listing is never returned on failure. The caller's deadline bounds the whole -// walk; without one the lookup falls back to a 120-second limit. +// FetchTags lists every repository tag across pages, never returning a partial listing. The caller's +// deadline bounds the walk, defaulting to 120 seconds. func FetchTags( ctx context.Context, registryHost, repository string, @@ -30,11 +29,11 @@ func FetchTags( defer cancel() } - repo, err := repositoryInternal(registryHost, repository) + repo, err := parseRepository(registryHost, repository) if err != nil { return nil, err } - tags, err := remote.List(repo, append(remoteOptionsInternal(ctx, credential, httpClient), remote.WithPageSize(tagsPageSize))...) + tags, err := remote.List(repo, remote.WithContext(ctx), remote.WithAuth(authenticator(credential)), remote.WithTransport(baseTransport(httpClient)), remote.WithPageSize(tagsPageSize)) if err != nil { return nil, fmt.Errorf("list registry tags: %w", err) } diff --git a/updater/registry/tags_test.go b/updater/registry/tags_test.go index 3fa4ef0..fab2b92 100644 --- a/updater/registry/tags_test.go +++ b/updater/registry/tags_test.go @@ -14,7 +14,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" ) -func tagsResponseInternal(r *http.Request, status int, body string) *http.Response { +func tagsResponse(r *http.Request, status int, body string) *http.Response { return &http.Response{StatusCode: status, Body: io.NopCloser(strings.NewReader(body)), Header: http.Header{}, Request: r} } @@ -32,22 +32,22 @@ func TestFetchTagsPaginationAndAuthentication(t *testing.T) { if user != "user" || password != "password" || r.URL.Query().Get("scope") != "repository:team/app:pull" { t.Error("token request did not preserve credentials and scope") } - return tagsResponseInternal(r, http.StatusOK, `{"token":"private-token"}`), nil + return tagsResponse(r, http.StatusOK, `{"token":"private-token"}`), nil } if r.Header.Get("Authorization") != "Bearer private-token" { - resp := tagsResponseInternal(r, http.StatusUnauthorized, "") + resp := tagsResponse(r, http.StatusUnauthorized, "") resp.Header.Set("WWW-Authenticate", `Bearer realm="https://registry.test/token",service="registry"`) return resp, nil } if r.URL.Path == "/v2/" { - return tagsResponseInternal(r, http.StatusOK, "{}"), nil + return tagsResponse(r, http.StatusOK, "{}"), nil } pageCalls++ if r.URL.Query().Get("n") != "1000" { t.Errorf("page %q requested without explicit page size: %s", r.URL.Query().Get("last"), r.URL.RawQuery) } page := pages[r.URL.Query().Get("last")] - resp := tagsResponseInternal(r, http.StatusOK, page.body) + resp := tagsResponse(r, http.StatusOK, page.body) if page.link != "" { resp.Header.Set("Link", page.link) } @@ -68,23 +68,23 @@ func TestFetchTagsRequestsChallengeScope(t *testing.T) { client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { if r.URL.Path == "/oauth2/token" { if user, password, _ := r.BasicAuth(); user != "user" || password != "password" { - return tagsResponseInternal(r, http.StatusUnauthorized, ""), nil + return tagsResponse(r, http.StatusUnauthorized, ""), nil } if slices.Contains(r.URL.Query()["scope"], metadataScope) { - return tagsResponseInternal(r, http.StatusOK, `{"access_token":"metadata-token"}`), nil + return tagsResponse(r, http.StatusOK, `{"access_token":"metadata-token"}`), nil } - return tagsResponseInternal(r, http.StatusOK, `{"access_token":"pull-token"}`), nil + return tagsResponse(r, http.StatusOK, `{"access_token":"pull-token"}`), nil } challenge := `Bearer realm="https://registry.test/oauth2/token",service="registry"` switch { case r.URL.Path == "/v2/" && r.Header.Get("Authorization") != "": - return tagsResponseInternal(r, http.StatusOK, "{}"), nil + return tagsResponse(r, http.StatusOK, "{}"), nil case r.URL.Path != "/v2/" && r.Header.Get("Authorization") == "Bearer metadata-token": - return tagsResponseInternal(r, http.StatusOK, `{"name":"team/app","tags":["1.1.1-1"]}`), nil + return tagsResponse(r, http.StatusOK, `{"name":"team/app","tags":["1.1.1-1"]}`), nil case r.URL.Path != "/v2/": challenge += `,scope="` + metadataScope + `"` } - resp := tagsResponseInternal(r, http.StatusUnauthorized, "") + resp := tagsResponse(r, http.StatusUnauthorized, "") resp.Header.Set("WWW-Authenticate", challenge) return resp, nil })} @@ -108,13 +108,13 @@ func TestFetchTagsFailures(t *testing.T) { t.Run(tt.name, func(t *testing.T) { client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { if r.URL.Path == "/v2/" { - return tagsResponseInternal(r, http.StatusOK, "{}"), nil + return tagsResponse(r, http.StatusOK, "{}"), nil } status := tt.status if status == 0 { status = http.StatusOK } - resp := tagsResponseInternal(r, status, tt.body) + resp := tagsResponse(r, status, tt.body) if tt.link != "" { resp.Header.Set("Link", tt.link) } @@ -135,12 +135,12 @@ func TestFetchTagsDockerHubAndEmpty(t *testing.T) { t.Errorf("unexpected URL: %s", r.URL) } if r.URL.Path == "/v2/" { - return tagsResponseInternal(r, http.StatusOK, "{}"), nil + return tagsResponse(r, http.StatusOK, "{}"), nil } if r.URL.Path != "/v2/library/alpine/tags/list" { t.Errorf("unexpected URL: %s", r.URL) } - return tagsResponseInternal(r, http.StatusOK, body), nil + return tagsResponse(r, http.StatusOK, body), nil })} tags, err := FetchTags(t.Context(), "docker.io", "library/alpine", nil, client) if err != nil || len(tags) != 0 { @@ -153,11 +153,11 @@ func TestFetchTagsDiscardsPartialListing(t *testing.T) { client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { switch { case r.URL.Path == "/v2/": - return tagsResponseInternal(r, http.StatusOK, "{}"), nil + return tagsResponse(r, http.StatusOK, "{}"), nil case r.URL.Query().Has("last"): - return tagsResponseInternal(r, http.StatusNotFound, ""), nil + return tagsResponse(r, http.StatusNotFound, ""), nil } - resp := tagsResponseInternal(r, http.StatusOK, `{"tags":["1.0.0"]}`) + resp := tagsResponse(r, http.StatusOK, `{"tags":["1.0.0"]}`) resp.Header.Set("Link", `; rel="next"`) return resp, nil })} @@ -197,7 +197,7 @@ func TestFetchTagsCancellation(t *testing.T) { if err := r.Context().Err(); err != nil { return nil, err } - resp := tagsResponseInternal(r, http.StatusOK, `{"tags":["1.0.0"]}`) + resp := tagsResponse(r, http.StatusOK, `{"tags":["1.0.0"]}`) if r.URL.Path != "/v2/" && !r.URL.Query().Has("last") { resp.Header.Set("Link", `; rel="next"`) } @@ -211,7 +211,7 @@ func TestFetchTagsCancellation(t *testing.T) { } } -func TestFetchTagsHonorsCallerDeadlineInternal(t *testing.T) { +func TestFetchTagsHonorsCallerDeadline(t *testing.T) { for _, tt := range []struct { name string timeout time.Duration @@ -235,7 +235,7 @@ func TestFetchTagsHonorsCallerDeadlineInternal(t *testing.T) { if remaining := time.Until(deadline); remaining < tt.min || remaining > tt.max { t.Fatalf("request deadline %s remaining, want between %s and %s", remaining, tt.min, tt.max) } - return tagsResponseInternal(r, http.StatusOK, `{"tags":["1.0.0"]}`), nil + return tagsResponse(r, http.StatusOK, `{"tags":["1.0.0"]}`), nil })} if _, err := FetchTags(ctx, "registry.test", "team/app", nil, client); err != nil { t.Fatal(err) @@ -253,11 +253,11 @@ func TestFetchTagsFollowsRedirectsWithoutForwardingCredentials(t *testing.T) { if r.Header.Get("Authorization") != "" { t.Error("credentials forwarded to redirect target") } - return tagsResponseInternal(r, http.StatusOK, `{"name":"mirror/team/app","tags":["1.0.0","1.1.0"]}`), nil + return tagsResponse(r, http.StatusOK, `{"name":"mirror/team/app","tags":["1.0.0","1.1.0"]}`), nil case r.URL.Path == "/v2/": - return tagsResponseInternal(r, http.StatusOK, "{}"), nil + return tagsResponse(r, http.StatusOK, "{}"), nil } - resp := tagsResponseInternal(r, http.StatusTemporaryRedirect, "") + resp := tagsResponse(r, http.StatusTemporaryRedirect, "") resp.Header.Set("Location", "https://mirror.test/v2/mirror/team/app/tags/list") return resp, nil })} diff --git a/updater/restart.go b/updater/restart.go index a5673c4..55ade73 100644 --- a/updater/restart.go +++ b/updater/restart.go @@ -1,6 +1,7 @@ package updater import ( + "cmp" "context" "errors" "fmt" @@ -9,6 +10,7 @@ import ( "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "go.getarcane.app/docker/compat" + kit "go.getarcane.app/kit/pkg" "go.getarcane.app/updater/internal/compose" "go.getarcane.app/updater/internal/deps" @@ -18,145 +20,83 @@ import ( "go.getarcane.app/updater/types" ) -// RestartContainersUsingOldImages restarts running containers matching old image -// IDs or refs. If dependency sorting detects a cycle, containers are restarted -// in discovery order to preserve historical best-effort behavior. +// restartScan holds the per-container plans, the restart-marked set, and every eligible container with its dependencies. +type restartScan struct { + plansByName map[string]*restartPlan + markedForRestart map[string]bool + containers []deps.ContainerWithDeps +} + +// RestartContainersUsingOldImages restarts running containers matching old image IDs or refs, plus +// their dependents. A dependency cycle falls back to discovery order. func (s *Service) RestartContainersUsingOldImages(ctx context.Context, oldIDToNewRef, oldRefToNewRef map[string]string) ([]ResourceResult, error) { dockerClient, err := s.dockerClient(ctx) if err != nil { - return nil, fmt.Errorf("docker connect: %w", err) + return nil, err } - listResult, err := dockerClient.ContainerList(ctx, client.ContainerListOptions{All: false}) if err != nil { return nil, fmt.Errorf("list containers: %w", err) } - - excludedContainers, err := s.excludedContainerSet(ctx) + excluded, err := s.excludedContainerSet(ctx) if err != nil { return nil, err } - - scan := s.scanRestartCandidates(ctx, dockerClient, restartScanInput{ - containers: listResult.Items, - excludedContainers: excludedContainers, - dockerProxyName: dockerProxyContainerName(dockerClient.DaemonHost()), - oldIDToNewRef: oldIDToNewRef, - updatedNorm: refs.NormalizeImageUpdateRefMapKeys(oldRefToNewRef), - }) - s.resolveRestartDependencies(ctx, dockerClient, scan) - propagateImplicitRestarts(scan) - sorted := s.sortRestartCandidates(ctx, scan) - return s.executeRestartPlans(ctx, dockerClient, sorted, scan.plansByName) -} - -type restartScanInput struct { - containers []container.Summary - excludedContainers map[string]bool - dockerProxyName string - oldIDToNewRef map[string]string - updatedNorm map[string]string -} - -// restartScan holds the discovery state shared by the restart phases: the -// per-container plans, the restart-marked set, and every eligible container -// with its dependency info. -type restartScan struct { - plansByName map[string]*restartPlan - markedForRestart map[string]bool - containers []deps.ContainerWithDeps -} - -// scanRestartCandidates builds a restart plan for every eligible -// running container, marking those whose image matches an applied update. -func (s *Service) scanRestartCandidates(ctx context.Context, dockerClient *client.Client, in restartScanInput) *restartScan { - scan := &restartScan{ - plansByName: map[string]*restartPlan{}, - markedForRestart: map[string]bool{}, - containers: make([]deps.ContainerWithDeps, 0, len(in.containers)), + if proxy := dockerProxyContainerName(dockerClient.DaemonHost()); proxy != "" { + excluded[proxy] = true } - targetImageIDs := digestcheck.NewRefIDCache(digestcheck.NewChecker(dockerClient, nil)) - for _, summary := range in.containers { - if shouldSkipSummary(summary, in.excludedContainers, in.dockerProxyName, s.config.LabelPolicy) { + updatedNorm := refs.NormalizeImageUpdateRefMapKeys(oldRefToNewRef) + targetImageIDs := digestcheck.NewRefIDCache(digestcheck.NewChecker(dockerClient, nil)) + policy := s.config.LabelPolicy + scan := &restartScan{plansByName: map[string]*restartPlan{}, markedForRestart: map[string]bool{}} + for _, summary := range listResult.Items { + if isExcluded(excluded, summary.ID, summary.Names...) || policy.IsUpdateDisabled(summary.Labels) || + (policy.IsSwarmTask(summary.Labels) && !policy.IsSelfUpdateTarget(summary.Labels)) { continue } - if summary.Labels == nil { - summary.Labels = map[string]string{} - } - name := containerSummaryName(summary) scan.containers = append(scan.containers, deps.ContainerWithDeps{Container: summary, Name: name}) - inspected, newRef, matchValue := s.matchContainerImage(ctx, dockerClient, summary, in.oldIDToNewRef, in.updatedNorm) - if newRef != "" && containerOnTargetImage(ctx, targetImageIDs, summary, inspected, newRef) { + // Inspect only when the summary alone cannot settle the match. + var inspected *container.InspectResponse + newRef, matchValue := match.ResolveContainerImageMatch(summary, nil, oldIDToNewRef, updatedNorm) + if newRef == "" && match.ShouldInspectUnmatchedContainerForImageMatch(summary) { + if inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, summary.ID, client.ContainerInspectOptions{}); inspectErr == nil { + inspected = &inspectResult.Container + newRef, matchValue = match.ResolveContainerImageMatch(summary, inspected, oldIDToNewRef, updatedNorm) + } + } + currentImageID := match.CurrentContainerImageID(summary, inspected) + if newRef != "" && currentImageID != "" && slices.Contains(targetImageIDs.IDsForRef(ctx, newRef), currentImageID) { newRef = "" } - - plan := &restartPlan{cnt: summary, inspect: inspected, newRef: newRef, match: matchValue, explicit: newRef != ""} - scan.plansByName[name] = plan - if plan.explicit { + scan.plansByName[name] = &restartPlan{cnt: summary, inspect: inspected, newRef: newRef, match: matchValue} + if newRef != "" { scan.markedForRestart[name] = true } } - return scan -} - -// matchContainerImage resolves the updated image ref for a container, -// falling back to an inspect-based match when the summary alone is inconclusive. -func (s *Service) matchContainerImage( - ctx context.Context, - dockerClient *client.Client, - summary container.Summary, - oldIDToNewRef, updatedNorm map[string]string, -) (*container.InspectResponse, string, string) { - newRef, matchValue := match.ResolveContainerImageMatch(summary, nil, oldIDToNewRef, updatedNorm) - if newRef != "" || !match.ShouldInspectUnmatchedContainerForImageMatch(summary) { - return nil, newRef, matchValue - } - inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, summary.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { - return nil, newRef, matchValue - } - inspected := &inspectResult.Container - newRef, matchValue = match.ResolveContainerImageMatch(summary, inspected, oldIDToNewRef, updatedNorm) - return inspected, newRef, matchValue -} - -// containerOnTargetImage reports whether the container already runs -// one of the image IDs the updated reference resolves to. -func containerOnTargetImage(ctx context.Context, targetImageIDs *digestcheck.RefIDCache, summary container.Summary, inspected *container.InspectResponse, newRef string) bool { - currentImageID := match.CurrentContainerImageID(summary, inspected) - return currentImageID != "" && slices.Contains(targetImageIDs.IDsForRef(ctx, newRef), currentImageID) -} -// resolveRestartDependencies fills in dependency info (and inspect -// data, where missing) for every scanned container once at least one restart -// is planned. -func (s *Service) resolveRestartDependencies(ctx context.Context, dockerClient *client.Client, scan *restartScan) { if len(scan.markedForRestart) == 0 { - return + return nil, nil } - for i := range scan.containers { - cwd := scan.containers[i] - if plan, ok := scan.plansByName[cwd.Name]; ok && plan.inspect != nil { - scan.containers[i] = deps.ExtractContainerDeps(ctx, cwd.Name, cwd.Container, *plan.inspect) - continue - } - inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cwd.Container.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { - continue - } - inspect := inspectResult.Container - scan.containers[i] = deps.ExtractContainerDeps(ctx, cwd.Name, cwd.Container, inspect) - if plan, ok := scan.plansByName[scan.containers[i].Name]; ok { - plan.inspect = &inspect + for i, scanned := range scan.containers { + plan := scan.plansByName[scanned.Name] + if plan.inspect == nil { + inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, scanned.Container.ID, client.ContainerInspectOptions{}) + if inspectErr != nil { + continue + } + plan.inspect = &inspectResult.Container } + pinNetworkContainer(plan.inspect, listResult.Items) + scan.containers[i] = deps.ExtractContainerDeps(ctx, scanned.Name, scanned.Container, *plan.inspect) } + propagateImplicitRestarts(scan) + return s.executeRestartPlans(ctx, dockerClient, s.sortRestartCandidates(ctx, scan), scan.plansByName) } -// propagateImplicitRestarts marks dependents of restarting containers -// for restart until the set stops growing. +// propagateImplicitRestarts marks dependents of restarting containers until the set stops growing. func propagateImplicitRestarts(scan *restartScan) { for { added := deps.UpdateImplicitRestart(scan.containers, scan.markedForRestart) @@ -164,174 +104,129 @@ func propagateImplicitRestarts(scan *restartScan) { return } for _, name := range added { - if plan, ok := scan.plansByName[name]; ok && plan.newRef == "" { - plan.newRef = fallbackImageForPlan(plan) - plan.match = "dependency_restart" - plan.implicit = true + plan, ok := scan.plansByName[name] + if !ok || plan.newRef != "" { + continue + } + configImage := "" + if plan.inspect != nil && plan.inspect.Config != nil { + configImage = plan.inspect.Config.Image } + plan.newRef, plan.match, plan.implicit = cmp.Or(configImage, plan.cnt.Image), "dependency_restart", true } } } -// sortRestartCandidates orders restart-marked containers by dependency -// (falling back to discovery order on cycles) with self-update targets last. +// sortRestartCandidates orders restart-marked containers by dependency (discovery order on cycles), then +// moves self-update targets last with agents before the server hosting this process. func (s *Service) sortRestartCandidates(ctx context.Context, scan *restartScan) []deps.ContainerWithDeps { - candidates := make([]deps.ContainerWithDeps, 0, len(scan.containers)) - for _, cd := range scan.containers { - if scan.markedForRestart[cd.Name] { - candidates = append(candidates, cd) - } - } + candidates := slices.DeleteFunc(slices.Clone(scan.containers), func(cd deps.ContainerWithDeps) bool { return !scan.markedForRestart[cd.Name] }) sorted, sortErr := deps.NewContainerSorter(candidates).Sort() if sortErr != nil { s.logger.WarnContext(ctx, "container dependency sort failed; restarting in discovery order", "error", sortErr) sorted = candidates } - return orderSelfUpdateLast(sorted, scan.plansByName, s.config.LabelPolicy) -} - -// restartRun accumulates the results and deferred work of a restart pass. -type restartRun struct { - composeGroups map[string]composeGroup - processedProjects map[string]bool - projectResults map[string]error - standaloneCandidates []deps.ContainerWithDeps - standaloneIndexes map[string]int - selfUpdateCandidates []selfUpdatePlan - selfUpdateIndexes map[string]int - results []ResourceResult + rank := func(candidate deps.ContainerWithDeps) int { + labels := candidate.Container.Labels + if plan := scan.plansByName[candidate.Name]; plan != nil && plan.inspect != nil && plan.inspect.Config != nil { + labels = plan.inspect.Config.Labels + } + return kit.Ternary(s.config.LabelPolicy.IsAgent(labels), 1, kit.Ternary(s.config.LabelPolicy.IsServer(labels), 2, 0)) + } + slices.SortStableFunc(sorted, func(a, b deps.ContainerWithDeps) int { return cmp.Compare(rank(a), rank(b)) }) + return sorted } -// executeRestartPlans routes each sorted candidate to the compose, -// self-update, or standalone path and returns the merged results. +// executeRestartPlans routes each sorted candidate through Compose, the standalone recreate, or, last +// of all because it may stop this process, the self-updater. func (s *Service) executeRestartPlans(ctx context.Context, dockerClient *client.Client, sorted []deps.ContainerWithDeps, plansByName map[string]*restartPlan) ([]ResourceResult, error) { - run := &restartRun{ - composeGroups: s.buildComposeGroups(ctx, sorted, plansByName), - processedProjects: map[string]bool{}, - projectResults: map[string]error{}, - standaloneIndexes: map[string]int{}, - selfUpdateIndexes: map[string]int{}, - } + composeGroups := s.buildComposeGroups(ctx, sorted, plansByName) + projectErrs := map[string]error{} + var results []ResourceResult + var standalone []deps.ContainerWithDeps + standaloneIndexes := map[string]int{} + var selfUpdateIndexes []int for _, candidate := range sorted { plan := plansByName[candidate.Name] if plan == nil { continue } - s.dispatchRestartCandidate(ctx, dockerClient, run, candidate, plan) - } - - if len(run.standaloneCandidates) > 0 { - standaloneResults := s.updateStandaloneRestartCandidates(ctx, dockerClient, run.standaloneCandidates, plansByName) - for _, result := range standaloneResults { - if index, ok := run.standaloneIndexes[result.ResourceName]; ok { - run.results[index] = result + if plan.inspect == nil { + inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, plan.cnt.ID, client.ContainerInspectOptions{}) + if inspectErr != nil { + results = append(results, failedContainerResult(plan.cnt.ID, candidate.Name, fmt.Sprintf("inspect failed: %v", inspectErr))) + continue } + plan.inspect = new(inspectResult.Container) } - } - - s.triggerDeferredSelfUpdates(ctx, run) - return run.results, nil -} - -// dispatchRestartCandidate records the candidate's result and either -// applies a compose service update immediately or queues the container for the -// standalone or self-update phase. -func (s *Service) dispatchRestartCandidate(ctx context.Context, dockerClient *client.Client, run *restartRun, candidate deps.ContainerWithDeps, plan *restartPlan) { - if plan.inspect == nil { - inspectResult, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, plan.cnt.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { - run.results = append(run.results, failedContainerResult(plan.cnt.ID, candidate.Name, fmt.Sprintf("inspect failed: %v", inspectErr))) - return + res := standaloneRestartResult(candidate, plan) + if plan.newRef == "" { + res.Status, res.Error = StatusSkipped, "no matching updated image" + results = append(results, res) + continue } - plan.inspect = new(inspectResult.Container) - } - res := standaloneRestartResult(candidate, plan) - if plan.newRef == "" { - res.Status = StatusSkipped - res.Error = "no matching updated image" - run.results = append(run.results, res) - return + labels := labelsFromInspect(*plan.inspect) + projectName := compose.ProjectLabel(labels) + endContainerStatus := s.BeginContainerUpdate(plan.cnt.ID) + endProjectStatus := s.BeginProjectUpdate(projectName) + projectID := "" + for id, group := range composeGroups { + if projectName != "" && group.projectName == projectName { + projectID = id + break + } + } + selfUpdate := s.isSelfUpdateCandidate(plan.cnt.ID, labels) + switch { + case selfUpdate: + selfUpdateIndexes = append(selfUpdateIndexes, len(results)) + case isComposeTagChange(plan.inspect, plan.newRef) && projectID == "": + res.Status, res.Error = StatusFailed, "compose tag update project could not be resolved" + case projectID != "" && compose.ServiceLabel(labels) != "": + res = s.applyComposeServiceUpdate(ctx, dockerClient, res, plan, projectID, composeGroups[projectID], projectErrs) + default: + standaloneIndexes[candidate.Name] = len(results) + standalone = append(standalone, candidate) + } + endProjectStatus() + endContainerStatus() + results = append(results, res) } - labels := labelsFromInspect(*plan.inspect) - endContainerStatus := s.BeginContainerUpdate(plan.cnt.ID) - defer endContainerStatus() - endProjectStatus := s.BeginProjectUpdate(compose.ProjectLabel(labels)) - defer endProjectStatus() - - projectName := compose.ProjectLabel(labels) - serviceName := compose.ServiceLabel(labels) - projectID := composeProjectID(projectName, run.composeGroups) - selfUpdate := s.isSelfUpdateCandidate(plan.cnt.ID, labels) - - switch { - case isComposeTagChangeInternal(plan) && projectID == "" && !selfUpdate: - res.Status = StatusFailed - res.Error = "compose tag update project could not be resolved" - case projectID != "" && serviceName != "" && !selfUpdate: - res = s.applyComposeServiceUpdate(ctx, dockerClient, res, plan, projectID, run) - case selfUpdate: - // Defer the actual trigger until every other container has been - // recreated: the self-updater may stop this process, so it must be - // the last action of the run. - run.selfUpdateIndexes[candidate.Name] = len(run.results) - run.selfUpdateCandidates = append(run.selfUpdateCandidates, selfUpdatePlan{ - containerID: plan.cnt.ID, - name: candidate.Name, - newRef: plan.newRef, - labels: labels, - }) - default: - if err := s.validateStandaloneContainerUpdate(labels); err != nil { - res.Status = StatusFailed - res.Error = err.Error() - break + if len(standalone) > 0 { + for name, result := range s.updateStandaloneRestartCandidates(ctx, dockerClient, standalone, plansByName) { + results[standaloneIndexes[name]] = result } - run.standaloneIndexes[candidate.Name] = len(run.results) - run.standaloneCandidates = append(run.standaloneCandidates, candidate) } - run.results = append(run.results, res) -} -// triggerDeferredSelfUpdates triggers queued self-updates last; -// candidates arrive sorted agents-first so the server (which hosts this -// process) is the final one handled. -func (s *Service) triggerDeferredSelfUpdates(ctx context.Context, run *restartRun) { - for _, target := range run.selfUpdateCandidates { - index, ok := run.selfUpdateIndexes[target.name] - if !ok { - continue - } - res := run.results[index] - endContainerStatus := s.BeginContainerUpdate(target.containerID) - if err := s.triggerSelfUpdate(ctx, target.containerID, target.name, target.newRef, target.labels); err != nil { - res.Status = StatusFailed - res.Error = err.Error() + // Self-updates arrive sorted agents first, so the server hosting this process goes last. + for _, index := range selfUpdateIndexes { + res := results[index] + plan := plansByName[res.ResourceName] + endContainerStatus := s.BeginContainerUpdate(plan.cnt.ID) + if err := s.triggerSelfUpdate(ctx, plan.cnt.ID, res.ResourceName, plan.newRef, labelsFromInspect(*plan.inspect)); err != nil { + res.Status, res.Error = StatusFailed, err.Error() } else { - res.Status = StatusUpdated - res.UpdateAvailable = true - res.UpdateApplied = true + res.Status, res.UpdateAvailable, res.UpdateApplied = StatusUpdated, true, true } endContainerStatus() - run.results[index] = res + results[index] = res } + return results, nil } -type selfUpdatePlan struct { - containerID string - name string - newRef string - labels map[string]string -} - -func (s *Service) updateStandaloneRestartCandidates(ctx context.Context, dockerClient *client.Client, candidates []deps.ContainerWithDeps, plansByName map[string]*restartPlan) []ResourceResult { +// updateStandaloneRestartCandidates stops dependents before what they depend on, then recreates in dependency order. +func (s *Service) updateStandaloneRestartCandidates( + ctx context.Context, + dockerClient *client.Client, + candidates []deps.ContainerWithDeps, + plansByName map[string]*restartPlan, +) map[string]ResourceResult { endStatus := make([]func(), 0, len(candidates)) for _, candidate := range candidates { - if plan := plansByName[candidate.Name]; plan != nil { - endStatus = append(endStatus, s.BeginContainerUpdate(plan.cnt.ID)) - } + endStatus = append(endStatus, s.BeginContainerUpdate(plansByName[candidate.Name].cnt.ID)) } defer func() { for _, end := range slices.Backward(endStatus) { @@ -339,113 +234,79 @@ func (s *Service) updateStandaloneRestartCandidates(ctx context.Context, dockerC } }() - resultsByName := map[string]ResourceResult{} + results := make(map[string]ResourceResult, len(candidates)) for _, candidate := range slices.Backward(candidates) { plan := plansByName[candidate.Name] - if plan == nil || plan.inspect == nil { - continue - } result := standaloneRestartResult(candidate, plan) if err := s.stopAndRemoveStandaloneContainer(ctx, dockerClient, plan.cnt, *plan.inspect); err != nil { - result.Status = StatusFailed - result.Error = err.Error() + result.Status, result.Error = StatusFailed, err.Error() } - resultsByName[candidate.Name] = result + results[candidate.Name] = result } for _, candidate := range candidates { - plan := plansByName[candidate.Name] - if plan == nil || plan.inspect == nil { - continue - } - result, ok := resultsByName[candidate.Name] - if !ok { - result = standaloneRestartResult(candidate, plan) - } + plan, result := plansByName[candidate.Name], results[candidate.Name] if result.Status == StatusFailed { - resultsByName[candidate.Name] = result continue } - if err := s.createStartOrRollback(ctx, dockerClient, plan.cnt, *plan.inspect, plan.newRef); err != nil { - result.Status = StatusFailed - result.Error = err.Error() - resultsByName[candidate.Name] = result + result.Status, result.Error = StatusFailed, err.Error() + results[candidate.Name] = result continue } - - result.UpdateApplied = true - if plan.implicit { - result.Status = StatusRestarted - } else { - result.Status = StatusUpdated - result.UpdateAvailable = true + result.Status = kit.Ternary(plan.implicit, StatusRestarted, StatusUpdated) + result.UpdateAvailable, result.UpdateApplied = !plan.implicit, true + if !plan.implicit { _ = s.notify(ctx, plan.cnt.ID, candidate.Name, plan.newRef, plan.match, refs.NormalizeImageUpdateRef(plan.newRef)) } - resultsByName[candidate.Name] = result - } - - out := make([]ResourceResult, 0, len(candidates)) - for _, candidate := range candidates { - if result, ok := resultsByName[candidate.Name]; ok { - out = append(out, result) - } + results[candidate.Name] = result } - return out + return results } +// applyComposeServiceUpdate updates the candidate's Compose project once per run and verifies the service. func (s *Service) applyComposeServiceUpdate( ctx context.Context, dockerClient *client.Client, res ResourceResult, plan *restartPlan, projectID string, - run *restartRun, + group composeGroup, + projectErrs map[string]error, ) ResourceResult { - labels := labelsFromInspect(*plan.inspect) - projectName := compose.ProjectLabel(labels) - serviceName := compose.ServiceLabel(labels) - if !run.processedProjects[projectID] { - group := run.composeGroups[projectID] + projectErr, processed := projectErrs[projectID] + if !processed { opCtx, cancel := s.opCtx(ctx) - projectErr := group.err - if projectErr == nil { - if group.tagChanges { - adapter, ok := s.config.ProjectUpdater.(types.ProjectImageUpdater) - if !ok { - projectErr = errors.New("compose tag updates require a ProjectImageUpdater adapter") - } else { - projectErr = adapter.UpdateServiceImages(opCtx, projectID, group.images) - } - } else { - projectErr = s.config.ProjectUpdater.UpdateServices(opCtx, projectID, group.services) - } + adapter, ok := s.config.ProjectUpdater.(types.ProjectImageUpdater) + switch { + case group.err != nil: + projectErr = group.err + case group.tagChanges && !ok: + projectErr = errors.New("compose tag updates require a ProjectImageUpdater adapter") + case group.tagChanges: + projectErr = adapter.UpdateServiceImages(opCtx, projectID, group.images) + default: + projectErr = s.config.ProjectUpdater.UpdateServices(opCtx, projectID, group.services) } cancel() - run.processedProjects[projectID] = true - if projectErr != nil { - run.projectResults[projectID] = projectErr - } + projectErrs[projectID] = projectErr } - projectErr := run.projectResults[projectID] - var verifyErr error - if run.composeGroups[projectID].tagChanges { + labels := labelsFromInspect(*plan.inspect) + projectName, serviceName := compose.ProjectLabel(labels), compose.ServiceLabel(labels) + oldImageID, targetRef := match.CurrentContainerImageID(plan.cnt, plan.inspect), "" + if group.tagChanges { if projectErr != nil { - res.Status = StatusFailed - res.Error = projectErr.Error() + res.Status, res.Error = StatusFailed, projectErr.Error() return res } - verifyErr = verifyComposeTargetInternal(ctx, dockerClient, projectName, serviceName, plan.newRef) - } else { - verifyErr = match.VerifyComposeServiceUpdatedImage(ctx, dockerClient, projectName, serviceName, match.CurrentContainerImageID(plan.cnt, plan.inspect)) + oldImageID, targetRef = "", plan.newRef } - if verifyErr != nil { + if verifyErr := verifyComposeService(ctx, dockerClient, projectName, serviceName, oldImageID, targetRef); verifyErr != nil { res.Status = StatusFailed + res.Error = fmt.Sprintf("service update verification failed: %v", verifyErr) if projectErr != nil { res.Error = fmt.Sprintf("project-level update failed: %v; service update verification failed: %v", projectErr, verifyErr) - } else { - res.Error = fmt.Sprintf("service update verification failed: %v", verifyErr) } return res } @@ -453,12 +314,8 @@ func (s *Service) applyComposeServiceUpdate( if projectErr != nil { s.logger.WarnContext(ctx, "service updated despite project-level compose error", "projectId", projectID, "projectName", projectName, "serviceName", serviceName, "error", projectErr) } - res.Status = StatusUpdated - if plan.implicit { - res.Status = StatusRestarted - } - res.UpdateAvailable = !plan.implicit - res.UpdateApplied = true + res.Status = kit.Ternary(plan.implicit, StatusRestarted, StatusUpdated) + res.UpdateAvailable, res.UpdateApplied = !plan.implicit, true _ = s.notify(ctx, plan.cnt.ID, res.ResourceName, plan.newRef, plan.match, refs.NormalizeImageUpdateRef(plan.newRef)) return res } diff --git a/updater/restart_helpers.go b/updater/restart_helpers.go index 1f9d2e0..2a6377f 100644 --- a/updater/restart_helpers.go +++ b/updater/restart_helpers.go @@ -1,98 +1,25 @@ package updater import ( - "maps" "net" "net/url" + "slices" "strings" - - "github.com/moby/moby/api/types/container" - - "go.getarcane.app/updater/internal/deps" ) -func shouldSkipSummary(summary container.Summary, excludedContainers map[string]bool, dockerProxyName string, policy LabelPolicy) bool { - for _, name := range summary.Names { - cleanName := strings.TrimPrefix(name, "/") - if excludedContainers[cleanName] || cleanName == dockerProxyName { - return true - } - } - if policy.IsUpdateDisabled(summary.Labels) { - return true - } - return policy.IsSwarmTask(summary.Labels) && !policy.IsSelfUpdateTarget(summary.Labels) -} - -func fallbackImageForPlan(plan *restartPlan) string { - if plan == nil { - return "" - } - if plan.inspect != nil && plan.inspect.Config != nil && strings.TrimSpace(plan.inspect.Config.Image) != "" { - return strings.TrimSpace(plan.inspect.Config.Image) - } - return strings.TrimSpace(plan.cnt.Image) -} - -func orderSelfUpdateLast(sorted []deps.ContainerWithDeps, plansByName map[string]*restartPlan, policy LabelPolicy) []deps.ContainerWithDeps { - if len(sorted) < 2 { - return sorted - } - - normalCandidates := make([]deps.ContainerWithDeps, 0, len(sorted)) - agentCandidates := make([]deps.ContainerWithDeps, 0, len(sorted)) - serverCandidates := make([]deps.ContainerWithDeps, 0, len(sorted)) - for _, candidate := range sorted { - labels := restartCandidateLabels(candidate, plansByName[candidate.Name]) - switch { - case policy.IsAgent(labels): - agentCandidates = append(agentCandidates, candidate) - case policy.IsServer(labels): - serverCandidates = append(serverCandidates, candidate) - default: - normalCandidates = append(normalCandidates, candidate) - } - } - ordered := make([]deps.ContainerWithDeps, 0, len(sorted)) - ordered = append(ordered, normalCandidates...) - ordered = append(ordered, agentCandidates...) - ordered = append(ordered, serverCandidates...) - return ordered -} - -func restartCandidateLabels(candidate deps.ContainerWithDeps, plan *restartPlan) map[string]string { - labels := map[string]string{} - if len(candidate.Container.Labels) > 0 { - maps.Copy(labels, candidate.Container.Labels) - } - if candidate.Inspect.Config != nil && len(candidate.Inspect.Config.Labels) > 0 { - maps.Copy(labels, candidate.Inspect.Config.Labels) - } - if plan != nil { - if len(plan.cnt.Labels) > 0 { - maps.Copy(labels, plan.cnt.Labels) - } - if plan.inspect != nil && plan.inspect.Config != nil && len(plan.inspect.Config.Labels) > 0 { - maps.Copy(labels, plan.inspect.Config.Labels) - } - } - if len(labels) == 0 { - return nil - } - return labels +// isExcluded reports whether the settings exclusions name a container by ID or by any of its names. +func isExcluded(excluded map[string]bool, id string, names ...string) bool { + return excluded[id] || slices.ContainsFunc(names, func(name string) bool { return excluded[strings.TrimPrefix(name, "/")] }) } +// dockerProxyContainerName returns the container name a tcp DOCKER_HOST points at, or "" for anything else. func dockerProxyContainerName(dockerHost string) string { - dockerHost = strings.TrimSpace(dockerHost) - if dockerHost == "" { - return "" - } - u, err := url.Parse(dockerHost) - if err != nil || strings.ToLower(u.Scheme) != "tcp" { + u, err := url.Parse(strings.TrimSpace(dockerHost)) + if err != nil || !strings.EqualFold(u.Scheme, "tcp") { return "" } host := u.Hostname() - if host == "" || host == "localhost" || strings.Contains(host, ".") || net.ParseIP(host) != nil { + if host == "localhost" || strings.Contains(host, ".") || net.ParseIP(host) != nil { return "" } return host diff --git a/updater/restart_test.go b/updater/restart_test.go index 89d70ad..79f08de 100644 --- a/updater/restart_test.go +++ b/updater/restart_test.go @@ -28,6 +28,7 @@ func TestRestartContainersUsingOldImagesRestartsDependenciesInWatchtowerOrder(t }) case r.Method == http.MethodGet && path == "/containers/db-id/json": writeDockerJSON(t, w, container.InspectResponse{ + State: &container.State{Running: true}, ID: "db-id", Name: "/db", Image: "sha256:old-db", @@ -37,6 +38,7 @@ func TestRestartContainersUsingOldImagesRestartsDependenciesInWatchtowerOrder(t }) case r.Method == http.MethodGet && path == "/containers/web-id/json": writeDockerJSON(t, w, container.InspectResponse{ + State: &container.State{Running: true}, ID: "web-id", Name: "/web", Image: "sha256:web", @@ -106,9 +108,15 @@ func TestRestartContainersUsingOldImagesLogsCycleFallback(t *testing.T) { {ID: "b-id", Names: []string{"/b"}, Image: "b:1", ImageID: "sha256:old-b", Labels: map[string]string{labels.LabelDependsOn: "a"}, State: "running"}, }) case r.Method == http.MethodGet && path == "/containers/a-id/json": - writeDockerJSON(t, w, container.InspectResponse{ID: "a-id", Name: "/a", Image: "sha256:old-a", Config: &container.Config{Image: "a:1", Labels: map[string]string{labels.LabelDependsOn: "b"}}}) + writeDockerJSON(t, w, container.InspectResponse{ + ID: "a-id", Name: "/a", Image: "sha256:old-a", State: &container.State{Running: true}, + Config: &container.Config{Image: "a:1", Labels: map[string]string{labels.LabelDependsOn: "b"}}, + }) case r.Method == http.MethodGet && path == "/containers/b-id/json": - writeDockerJSON(t, w, container.InspectResponse{ID: "b-id", Name: "/b", Image: "sha256:old-b", Config: &container.Config{Image: "b:1", Labels: map[string]string{labels.LabelDependsOn: "a"}}}) + writeDockerJSON(t, w, container.InspectResponse{ + ID: "b-id", Name: "/b", Image: "sha256:old-b", State: &container.State{Running: true}, + Config: &container.Config{Image: "b:1", Labels: map[string]string{labels.LabelDependsOn: "a"}}, + }) case r.Method == http.MethodGet && path == "/images/a:2/json": writeDockerJSON(t, w, image.InspectResponse{ID: "sha256:new-a"}) case r.Method == http.MethodGet && path == "/images/b:2/json": @@ -177,6 +185,7 @@ func TestRestartContainersUsingOldImagesRoutesLegacyArcaneServerThroughSelfUpdat }) case r.Method == http.MethodGet && path == "/containers/arcane-id/json": writeDockerJSON(t, w, container.InspectResponse{ + State: &container.State{Running: true}, ID: "arcane-id", Name: "/arcane", Image: "sha256:old-arcane", @@ -246,6 +255,7 @@ func TestRestartContainersUsingOldImagesSelfContainerIDFiresAfterStandalone(t *t }) case r.Method == http.MethodGet && path == "/containers/app-id/json": writeDockerJSON(t, w, container.InspectResponse{ + State: &container.State{Running: true}, ID: "app-id", Name: "/app", Image: "sha256:old-app", @@ -253,6 +263,7 @@ func TestRestartContainersUsingOldImagesSelfContainerIDFiresAfterStandalone(t *t }) case r.Method == http.MethodGet && path == "/containers/self-id/json": writeDockerJSON(t, w, container.InspectResponse{ + State: &container.State{Running: true}, ID: "self-id", Name: "/arcane", Image: "sha256:old-arcane", @@ -345,6 +356,7 @@ func TestRestartContainersUsingOldImagesVerifiesComposeServiceAfterProjectError( }) case r.Method == http.MethodGet && path == "/containers/web-old/json": writeDockerJSON(t, w, container.InspectResponse{ + State: &container.State{Running: true}, ID: "web-old", Name: "/web", Image: "sha256:old-app", @@ -390,7 +402,7 @@ func TestRestartContainersUsingOldImagesOperationTimeoutCancelsSlowStop(t *testi {ID: "app-id", Names: []string{"/app"}, Image: "app:1", ImageID: "sha256:old-app", State: "running"}, }) case r.Method == http.MethodGet && path == "/containers/app-id/json": - writeDockerJSON(t, w, container.InspectResponse{ID: "app-id", Name: "/app", Image: "sha256:old-app", Config: &container.Config{Image: "app:1"}}) + writeDockerJSON(t, w, container.InspectResponse{State: &container.State{Running: true}, ID: "app-id", Name: "/app", Image: "sha256:old-app", Config: &container.Config{Image: "app:1"}}) case r.Method == http.MethodGet && path == "/images/app:2/json": writeDockerJSON(t, w, image.InspectResponse{ID: "sha256:new-app"}) case r.Method == http.MethodPost && path == "/containers/app-id/stop": diff --git a/updater/results.go b/updater/results.go index d618ad2..a680828 100644 --- a/updater/results.go +++ b/updater/results.go @@ -1,6 +1,7 @@ package updater import ( + "context" "time" "github.com/moby/moby/api/types/container" @@ -17,18 +18,28 @@ func newTimedResult() (*Result, func(*error)) { } } -func (s *Service) applyResultCount(out *Result, item ResourceResult) { +// appendResult counts item into out and appends it: every item counts as checked, and up-to-date ones as skipped too. +func appendResult(out *Result, item ResourceResult) { + out.Checked++ switch item.Status { case StatusUpdated: out.Updated++ case StatusRestarted: out.Restarted++ - case StatusSkipped: + case StatusSkipped, StatusUpToDate: out.Skipped++ case StatusFailed: out.Failed++ - case StatusChecked, StatusUpToDate, StatusUpdateAvailable: - out.Checked++ + case StatusChecked, StatusUpdateAvailable: + } + out.Items = append(out.Items, item) +} + +// appendRecordedResult appends item to out and reports it to the RunRecorder. +func (s *Service) appendRecordedResult(ctx context.Context, out *Result, item ResourceResult) { + appendResult(out, item) + if s.config.RunRecorder != nil { + _ = s.config.RunRecorder.RecordUpdateRun(ctx, item) } } @@ -40,19 +51,6 @@ func skippedContainerResult(id, name, message string) ResourceResult { return ResourceResult{ResourceID: id, ResourceName: name, ResourceType: ResourceTypeContainer, Status: StatusSkipped, Error: message} } -func updatedContainerResult(id, name, oldImage, newImage string) ResourceResult { - return ResourceResult{ - ResourceID: id, - ResourceName: name, - ResourceType: ResourceTypeContainer, - Status: StatusUpdated, - UpdateAvailable: true, - UpdateApplied: true, - OldImage: oldImage, - NewImage: newImage, - } -} - func labelsFromInspect(inspect container.InspectResponse) map[string]string { if inspect.Config == nil || len(inspect.Config.Labels) == 0 { return nil diff --git a/updater/status.go b/updater/status.go index 6cc3f0c..0381df3 100644 --- a/updater/status.go +++ b/updater/status.go @@ -4,13 +4,14 @@ import ( "slices" "strings" "sync/atomic" + + kit "go.getarcane.app/kit/pkg" ) // Status returns a point-in-time updater status snapshot. func (s *Service) Status() Status { - containerIDs := statusSnapshot(&s.updatingContainers) - projectIDs := statusSnapshot(&s.updatingProjects) - + containerIDs := append([]string{}, kit.FromPtr(s.updatingContainers.Load())...) + projectIDs := append([]string{}, kit.FromPtr(s.updatingProjects.Load())...) return Status{ UpdatingContainers: len(containerIDs), UpdatingProjects: len(projectIDs), @@ -21,69 +22,39 @@ func (s *Service) Status() Status { // BeginContainerUpdate marks a container as updating and returns a completion callback. func (s *Service) BeginContainerUpdate(containerID string) func() { - return s.beginStatusUpdate(containerID, &s.updatingContainers) + containerID = strings.TrimSpace(containerID) + if containerID == "" { + return func() {} + } + updateStatusSnapshot(&s.updatingContainers, containerID, true) + return func() { updateStatusSnapshot(&s.updatingContainers, containerID, false) } } // BeginProjectUpdate marks a project as updating and returns a completion callback. func (s *Service) BeginProjectUpdate(projectID string) func() { - return s.beginStatusUpdate(projectID, &s.updatingProjects) -} - -func (s *Service) beginStatusUpdate(id string, active *atomic.Pointer[[]string]) func() { - id = strings.TrimSpace(id) - if id == "" { + projectID = strings.TrimSpace(projectID) + if projectID == "" { return func() {} } - - updateStatusSnapshot(active, id, true) - return func() { - updateStatusSnapshot(active, id, false) - } -} - -func statusSnapshot(active *atomic.Pointer[[]string]) []string { - if active == nil { - return []string{} - } - ids := active.Load() - if ids == nil || len(*ids) == 0 { - return []string{} - } - return slices.Clone(*ids) + updateStatusSnapshot(&s.updatingProjects, projectID, true) + return func() { updateStatusSnapshot(&s.updatingProjects, projectID, false) } } +// updateStatusSnapshot adds or removes id in the sorted snapshot active points at, retrying lost CAS races. func updateStatusSnapshot(active *atomic.Pointer[[]string], id string, add bool) { - if active == nil { - return - } for { currentPtr := active.Load() - var current []string - if currentPtr != nil { - current = *currentPtr - } - + current := kit.FromPtr(currentPtr) index, found := slices.BinarySearch(current, id) - if add { - if found { - return - } - next := make([]string, 0, len(current)+1) - next = append(next, current[:index]...) - next = append(next, id) - next = append(next, current[index:]...) - if active.CompareAndSwap(currentPtr, &next) { - return - } - continue - } - - if !found { + if found == add { return } - next := make([]string, 0, len(current)-1) - next = append(next, current[:index]...) - next = append(next, current[index+1:]...) + next := slices.Clone(current) + if add { + next = slices.Insert(next, index, id) + } else { + next = slices.Delete(next, index, index+1) + } if active.CompareAndSwap(currentPtr, &next) { return } diff --git a/updater/tag_defaults.go b/updater/tag_defaults.go index 0292017..5415f04 100644 --- a/updater/tag_defaults.go +++ b/updater/tag_defaults.go @@ -5,6 +5,8 @@ import ( "fmt" "net/http" + "github.com/google/go-containerregistry/pkg/authn" + "go.getarcane.app/updater/refs" "go.getarcane.app/updater/registry" "go.getarcane.app/updater/types" @@ -20,9 +22,13 @@ func (l defaultRegistryTagLister) ListTags(ctx context.Context, imageRef string) if err != nil { return nil, err } - credential, err := defaultDigestCredentials(ctx, imageRef) + authConfig, ok, err := defaultDockerConfigRegistryAuthConfig(ctx, imageRef) if err != nil { return nil, fmt.Errorf("registry auth: %w", err) } + var credential *authn.AuthConfig + if ok { + credential = &authn.AuthConfig{Username: authConfig.Username, Password: authConfig.Password, IdentityToken: authConfig.IdentityToken, RegistryToken: authConfig.RegistryToken} + } return registry.FetchTags(ctx, parsed.RegistryHost, parsed.Repository, credential, l.httpClient) } diff --git a/updater/targeted.go b/updater/targeted.go index 4fb3f13..6c3710f 100644 --- a/updater/targeted.go +++ b/updater/targeted.go @@ -5,12 +5,12 @@ import ( "errors" "fmt" "io" + "slices" "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "go.getarcane.app/docker/compat" - "go.getarcane.app/updater/internal/compose" "go.getarcane.app/updater/internal/deps" "go.getarcane.app/updater/pkg/utils/tagpolicy" "go.getarcane.app/updater/refs" @@ -18,11 +18,13 @@ import ( type targetedRecord struct { record ImageUpdateRecord - targets map[string]bool + targets []string failed bool } -func (s *Service) applyTargetedRecordsInternal(ctx context.Context, records []ImageUpdateRecord, opts Options, out *Result) error { +// applyTargetedRecords applies container-scoped and tag records by recreating exactly the containers they +// target, and clears a record only once every one of its targets succeeded. +func (s *Service) applyTargetedRecords(ctx context.Context, records []ImageUpdateRecord, opts Options, out *Result) error { dockerClient, err := s.dockerClient(ctx) if err != nil { return err @@ -31,44 +33,133 @@ func (s *Service) applyTargetedRecordsInternal(ctx context.Context, records []Im if err != nil { return err } + proxy := dockerProxyContainerName(dockerClient.DaemonHost()) scan := &restartScan{plansByName: map[string]*restartPlan{}, markedForRestart: map[string]bool{}} - states, err := s.collectTargetRecordsInternal(ctx, dockerClient, listed.Items, records, scan, out) - if err != nil { - return err + states := make([]targetedRecord, 0, len(records)) + for _, record := range records { + if !record.NeedsUpdate() { + continue + } + state, collectErr := s.collectTargetRecord(ctx, dockerClient, listed.Items, proxy, record, scan, out) + if collectErr != nil { + return collectErr + } + states = append(states, state) } // All Compose conflicts and adapter capabilities must be checked before pulls. - err = s.validateComposeGroupsInternal(ctx, s.sortRestartCandidates(ctx, scan), scan.plansByName) - if err != nil { - return err + for _, group := range s.buildComposeGroups(ctx, s.sortRestartCandidates(ctx, scan), scan.plansByName) { + if group.err != nil { + return group.err + } } - results := s.pullTargetPlansInternal(ctx, dockerClient, scan, opts) - if !opts.DryRun { - s.addTargetDependenciesInternal(ctx, dockerClient, listed.Items, scan) - sorted := s.sortRestartCandidates(ctx, scan) - restarted, restartErr := s.executeRestartPlans(ctx, dockerClient, sorted, scan.plansByName) + + results := s.pullTargetPlans(ctx, dockerClient, scan, opts) + if !opts.DryRun && len(scan.markedForRestart) > 0 { + for _, cnt := range listed.Items { + name := containerSummaryName(cnt) + if _, planned := scan.plansByName[name]; planned || cnt.State != container.StateRunning || name == proxy { + continue + } + inspected, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cnt.ID, client.ContainerInspectOptions{}) + if inspectErr != nil { + continue + } + if reason, eligibilityErr := s.containerEligibility(ctx, inspected.Container, enforceSettingsExclusions); eligibilityErr != nil || reason != "" { + continue + } + pinNetworkContainer(&inspected.Container, listed.Items) + scan.plansByName[name] = &restartPlan{cnt: cnt, inspect: &inspected.Container} + scan.containers = append(scan.containers, deps.ExtractContainerDeps(ctx, name, cnt, inspected.Container)) + } + propagateImplicitRestarts(scan) + restarted, restartErr := s.executeRestartPlans(ctx, dockerClient, s.sortRestartCandidates(ctx, scan), scan.plansByName) if restartErr != nil { return restartErr } results = append(results, restarted...) } - successful := map[string]bool{} - failed := map[string]bool{} + + successful, failed := map[string]bool{}, map[string]bool{} for _, result := range results { - s.appendTargetResultInternal(ctx, out, result) - if result.Status == StatusFailed { - failed[result.ResourceID] = true - } + s.appendRecordedResult(ctx, out, result) + failed[result.ResourceID] = failed[result.ResourceID] || result.Status == StatusFailed successful[result.ResourceID] = !failed[result.ResourceID] && (successful[result.ResourceID] || result.Status == StatusUpdated || result.Status == StatusUpToDate) } - if !opts.DryRun { - return s.clearTargetedRecordsInternal(ctx, states, successful) + for _, state := range states { + if opts.DryRun || state.failed || slices.ContainsFunc(state.targets, func(id string) bool { return !successful[id] }) { + continue + } + if err = s.config.PendingStore.ClearImageUpdateRecord(ctx, state.record); err != nil { + return fmt.Errorf("clear applied pending record: %w", err) + } } - return nil } -func (s *Service) targetRecordPlanInternal(ctx context.Context, cnt container.Summary, inspect container.InspectResponse, record ImageUpdateRecord) (*restartPlan, string, error) { - reason, err := s.containerEligibilityInternal(ctx, inspect, enforceSettingsExclusions) +// collectTargetRecord plans every running container record targets and reports the ones it must reject. +func (s *Service) collectTargetRecord( + ctx context.Context, + dockerClient *client.Client, + containers []container.Summary, + proxy string, + record ImageUpdateRecord, + scan *restartScan, + out *Result, +) (targetedRecord, error) { + state := targetedRecord{record: record} + for _, cnt := range containers { + if cnt.State != container.StateRunning || (record.ContainerID != "" && record.ContainerID != cnt.ID) { + continue + } + name := containerSummaryName(cnt) + inspected, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cnt.ID, client.ContainerInspectOptions{}) + if inspectErr != nil { + // An unscoped record cannot safely be cleared after an incomplete scan. + state.failed = true + s.appendRecordedResult(ctx, out, failedContainerResult(cnt.ID, name, inspectErr.Error())) + continue + } + inspect := inspected.Container + if record.ContainerID == "" && (inspect.Config == nil || refs.NormalizeImageUpdateRef(inspect.Config.Image) != refs.NormalizeImageUpdateRef(record.ImageRef())) { + continue + } + if proxy != "" && name == proxy { + state.failed = true + s.appendRecordedResult(ctx, out, skippedContainerResult(cnt.ID, proxy, "Docker proxy excluded from pending updates")) + continue + } + state.targets = append(state.targets, cnt.ID) + plan, reason, planErr := s.targetRecordPlan(ctx, cnt, inspect, record) + if planErr != nil || reason != "" { + state.failed = true + if reason != "" { + s.appendRecordedResult(ctx, out, skippedContainerResult(cnt.ID, name, reason)) + } else { + s.appendRecordedResult(ctx, out, failedContainerResult(cnt.ID, name, planErr.Error())) + } + continue + } + if previous := scan.plansByName[name]; previous != nil { + if previous.newRef != plan.newRef { + return state, fmt.Errorf("conflicting image targets for container %s", cnt.ID) + } + continue + } + pinNetworkContainer(plan.inspect, containers) + scan.plansByName[name] = plan + scan.containers = append(scan.containers, deps.ExtractContainerDeps(ctx, name, cnt, *plan.inspect)) + scan.markedForRestart[name] = true + } + if len(state.targets) == 0 { + state.failed = true + s.appendRecordedResult(ctx, out, skippedContainerResult(record.ContainerID, record.ImageRef(), "pending update has no matching container")) + } + return state, nil +} + +// targetRecordPlan validates that record still applies to the container under its current policy. +func (s *Service) targetRecordPlan(ctx context.Context, cnt container.Summary, inspect container.InspectResponse, record ImageUpdateRecord) (*restartPlan, string, error) { + reason, err := s.containerEligibility(ctx, inspect, enforceSettingsExclusions) if err != nil || reason != "" { return nil, reason, err } @@ -80,17 +171,13 @@ func (s *Service) targetRecordPlanInternal(ctx context.Context, cnt container.Su if oldRef == "" || newRef == "" { return nil, "", errors.New("invalid pending image reference") } - current = refs.NormalizeImageUpdateRef(current) - if current != oldRef && current != newRef { + if current = refs.NormalizeImageUpdateRef(current); current != oldRef && current != newRef { return nil, "", errors.New("container image changed since update check") } if record.IsTagUpdate() { - old, parseErr := refs.NormalizeReference(oldRef) - if parseErr != nil { - return nil, "", parseErr - } - next, parseErr := refs.NormalizeReference(newRef) - if parseErr != nil { + old, oldErr := refs.NormalizeReference(oldRef) + next, nextErr := refs.NormalizeReference(newRef) + if parseErr := errors.Join(oldErr, nextErr); parseErr != nil { return nil, "", parseErr } policy, policyErr := tagpolicy.Resolve(oldRef, s.config.LabelPolicy.TagPolicy(inspect.Config.Labels)) @@ -108,48 +195,43 @@ func (s *Service) targetRecordPlanInternal(ctx context.Context, cnt container.Su return nil, "", errors.New("pending tag is not a newer allowed version") } } - err = s.preflightComposeImageInternal(ctx, cnt, inspect, newRef) - if err != nil { + if _, err = s.preflightComposeImage(ctx, cnt.ID, inspect, newRef); err != nil { return nil, "", err } - return &restartPlan{cnt: cnt, inspect: &inspect, newRef: newRef, match: oldRef, explicit: true}, "", nil + return &restartPlan{cnt: cnt, inspect: &inspect, newRef: newRef, match: oldRef}, "", nil } -func (s *Service) pullTargetPlansInternal(ctx context.Context, dockerClient *client.Client, scan *restartScan, opts Options) []ResourceResult { +// pullTargetPlans pulls each planned target once, in scan order for deterministic outcomes, and unplans +// failed and no-op targets so they do not seed dependency restarts. +func (s *Service) pullTargetPlans(ctx context.Context, dockerClient *client.Client, scan *restartScan, opts Options) []ResourceResult { pulled := map[string]error{} var results []ResourceResult - // Use scan order, rather than map order, for deterministic pulls and outcomes. for _, candidate := range scan.containers { plan := scan.plansByName[candidate.Name] result := standaloneRestartResult(candidate, plan) if opts.DryRun { - result.Status = StatusSkipped - result.UpdateAvailable = true + result.Status, result.UpdateAvailable = StatusSkipped, true results = append(results, result) delete(scan.markedForRestart, candidate.Name) continue } err, seen := pulled[plan.newRef] if !seen { - if s.config.ImagePuller == nil { - err = ErrImagePullerRequired - } else { + err = ErrImagePullerRequired + if s.config.ImagePuller != nil { err = s.config.ImagePuller.PullImage(ctx, plan.newRef, io.Discard) } pulled[plan.newRef] = err } if err != nil { - result.Status = StatusFailed - result.Error = fmt.Sprintf("pull target image: %v", err) + result.Status, result.Error = StatusFailed, fmt.Sprintf("pull target image: %v", err) } else { image, inspectErr := dockerClient.ImageInspect(ctx, plan.newRef) switch { case inspectErr != nil: - result.Status = StatusFailed - result.Error = fmt.Sprintf("inspect pulled image: %v", inspectErr) + result.Status, result.Error = StatusFailed, fmt.Sprintf("inspect pulled image: %v", inspectErr) case image.ID == "": - result.Status = StatusFailed - result.Error = "pulled image has no image ID" + result.Status, result.Error = StatusFailed, "pulled image has no image ID" case !opts.Force && image.ID == plan.inspect.Image && refs.NormalizeImageUpdateRef(plan.inspect.Config.Image) == plan.newRef: result.Status = StatusUpToDate default: @@ -158,169 +240,7 @@ func (s *Service) pullTargetPlansInternal(ctx context.Context, dockerClient *cli } results = append(results, result) delete(scan.markedForRestart, candidate.Name) - // Failed/no-op targets must not seed dependency propagation. plan.newRef = "" } return results } - -func (s *Service) addTargetDependenciesInternal(ctx context.Context, dockerClient *client.Client, containers []container.Summary, scan *restartScan) { - if len(scan.markedForRestart) == 0 { - return - } - for _, cnt := range containers { - name := containerSummaryName(cnt) - if _, ok := scan.plansByName[name]; ok { - continue - } - if cnt.State != container.StateRunning || containerSummaryName(cnt) == dockerProxyContainerName(dockerClient.DaemonHost()) { - continue - } - inspected, err := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cnt.ID, client.ContainerInspectOptions{}) - if err != nil { - continue - } - reason, err := s.containerEligibilityInternal(ctx, inspected.Container, enforceSettingsExclusions) - if err != nil || reason != "" { - continue - } - scan.plansByName[name] = &restartPlan{cnt: cnt, inspect: &inspected.Container} - scan.containers = append(scan.containers, deps.ExtractContainerDeps(ctx, name, cnt, inspected.Container)) - } - propagateImplicitRestarts(scan) -} - -func (s *Service) appendTargetResultInternal(ctx context.Context, out *Result, result ResourceResult) { - if result.Status != StatusUpToDate && result.Status != StatusChecked && result.Status != StatusUpdateAvailable { - out.Checked++ - } - s.applyResultCount(out, result) - if result.Status == StatusUpToDate { - out.Skipped++ - } - out.Items = append(out.Items, result) - _ = s.recordResult(ctx, result) -} - -func (s *Service) validateComposeGroupsInternal(ctx context.Context, sorted []deps.ContainerWithDeps, plans map[string]*restartPlan) error { - groups := s.buildComposeGroups(ctx, sorted, plans) - for _, group := range groups { - if group.err != nil { - return group.err - } - } - return nil -} - -// isComposeTagChangeInternal includes implicit restarts in tag-aware groups but -// only requires image persistence when the configured image reference changes. -func isComposeTagChangeInternal(plan *restartPlan) bool { - return plan.inspect != nil && plan.inspect.Config != nil && - refs.NormalizeImageUpdateRef(plan.inspect.Config.Image) != refs.NormalizeImageUpdateRef(plan.newRef) && - (compose.ProjectLabel(plan.inspect.Config.Labels) != "" || compose.ServiceLabel(plan.inspect.Config.Labels) != "") -} - -func (s *Service) collectTargetRecordsInternal( - ctx context.Context, - dockerClient *client.Client, - containers []container.Summary, - records []ImageUpdateRecord, - scan *restartScan, - out *Result, -) ([]targetedRecord, error) { - states := make([]targetedRecord, 0, len(records)) - for _, record := range records { - if !record.NeedsUpdate() { - continue - } - state, err := s.collectTargetRecordInternal(ctx, dockerClient, containers, record, scan, out) - if err != nil { - return nil, err - } - states = append(states, state) - } - - return states, nil -} - -func (s *Service) collectTargetRecordInternal( - ctx context.Context, - dockerClient *client.Client, - containers []container.Summary, - record ImageUpdateRecord, - scan *restartScan, - out *Result, -) (targetedRecord, error) { - state := targetedRecord{record: record, targets: map[string]bool{}} - for _, cnt := range containers { - if cnt.State != container.StateRunning { - continue - } - if record.ContainerID != "" && record.ContainerID != cnt.ID { - continue - } - if proxy := dockerProxyContainerName(dockerClient.DaemonHost()); proxy != "" && containerSummaryName(cnt) == proxy { - state.failed = true - s.appendTargetResultInternal(ctx, out, skippedContainerResult(cnt.ID, proxy, "Docker proxy excluded from pending updates")) - continue - } - inspected, inspectErr := compat.ContainerInspectWithCompatibility(ctx, dockerClient, cnt.ID, client.ContainerInspectOptions{}) - if inspectErr != nil { - // An unscoped record cannot safely be cleared after an incomplete scan. - state.failed = true - s.appendTargetResultInternal(ctx, out, failedContainerResult(cnt.ID, containerSummaryName(cnt), inspectErr.Error())) - continue - } - inspect := inspected.Container - if record.ContainerID == "" && (inspect.Config == nil || refs.NormalizeImageUpdateRef(inspect.Config.Image) != refs.NormalizeImageUpdateRef(record.ImageRef())) { - continue - } - state.targets[cnt.ID] = true - plan, reason, planErr := s.targetRecordPlanInternal(ctx, cnt, inspect, record) - if planErr != nil || reason != "" { - state.failed = true - if reason != "" { - s.appendTargetResultInternal(ctx, out, skippedContainerResult(cnt.ID, containerSummaryName(cnt), reason)) - } else { - s.appendTargetResultInternal(ctx, out, failedContainerResult(cnt.ID, containerSummaryName(cnt), planErr.Error())) - } - continue - } - name := containerSummaryName(cnt) - if previous := scan.plansByName[name]; previous != nil { - if previous.newRef != plan.newRef { - return state, fmt.Errorf("conflicting image targets for container %s", cnt.ID) - } - continue - } - scan.plansByName[name] = plan - scan.containers = append(scan.containers, deps.ExtractContainerDeps(ctx, name, cnt, inspect)) - scan.markedForRestart[name] = true - } - if len(state.targets) == 0 { - state.failed = true - s.appendTargetResultInternal(ctx, out, skippedContainerResult(record.ContainerID, record.ImageRef(), "pending update has no matching container")) - } - - return state, nil -} - -func (s *Service) clearTargetedRecordsInternal(ctx context.Context, states []targetedRecord, successful map[string]bool) error { - for _, state := range states { - if state.failed { - continue - } - all := true - for id := range state.targets { - if !successful[id] { - all = false - } - } - if all { - if err := s.config.PendingStore.ClearImageUpdateRecord(ctx, state.record); err != nil { - return fmt.Errorf("clear applied pending record: %w", err) - } - } - } - return nil -} diff --git a/updater/targeted_test.go b/updater/targeted_test.go index ffbb37a..5741dda 100644 --- a/updater/targeted_test.go +++ b/updater/targeted_test.go @@ -21,25 +21,25 @@ import ( updatetypes "go.getarcane.app/updater/types" ) -type targetedDockerInternal struct { +type targetedDocker struct { mu sync.Mutex containers []container.InspectResponse created map[string]string mutations int } -func newTargetedContainerInternal(id, constraint string) container.InspectResponse { +func newTargetedContainer(id, constraint string) container.InspectResponse { return container.InspectResponse{ ID: id, Name: "/" + id, Image: "sha256:shared", State: &container.State{Running: true}, Config: &container.Config{Image: "app:1.0.0", Labels: map[string]string{labels.LabelUpdateStrategy: "tag", labels.LabelUpdateConstraint: constraint}}, } } -func targetedPendingInternal(id, target string) ImageUpdateRecord { +func targetedPending(id, target string) ImageUpdateRecord { return ImageUpdateRecord{ID: "sha256:shared", ContainerID: id, Repository: "app", Tag: "1.0.0", LatestVersion: &target, HasUpdate: true, UpdateType: UpdateTypeTag} } -func (f *targetedDockerInternal) handlerInternal(t *testing.T) http.HandlerFunc { +func (f *targetedDocker) handler(t *testing.T) http.HandlerFunc { t.Helper() return func(w http.ResponseWriter, r *http.Request) { f.mu.Lock() @@ -92,7 +92,7 @@ func (f *targetedDockerInternal) handlerInternal(t *testing.T) http.HandlerFunc } } -func TestApplyPendingTargetedSharedImageInternal(t *testing.T) { +func TestApplyPendingTargetedSharedImage(t *testing.T) { for _, tt := range []struct { name, secondTarget, secondConstraint string wantPulls int @@ -103,13 +103,13 @@ func TestApplyPendingTargetedSharedImageInternal(t *testing.T) { {name: "dry run", secondTarget: "2.1.0", secondConstraint: "2.x", dryRun: true}, } { t.Run(tt.name, func(t *testing.T) { - fixture := &targetedDockerInternal{ - containers: []container.InspectResponse{newTargetedContainerInternal("one", "1.x"), newTargetedContainerInternal("two", tt.secondConstraint)}, + fixture := &targetedDocker{ + containers: []container.InspectResponse{newTargetedContainer("one", "1.x"), newTargetedContainer("two", tt.secondConstraint)}, created: map[string]string{}, } fixture.containers[1].Config.Image = "docker.io/library/app:1.0.0" - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0"), targetedPendingInternal("two", tt.secondTarget)) + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + store := NewMemoryPendingStore(targetedPending("one", "1.2.0"), targetedPending("two", tt.secondTarget)) puller := &fakePuller{} service := newService(Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, ImagePuller: puller}) result, err := service.ApplyPending(t.Context(), Options{DryRun: tt.dryRun}) @@ -138,7 +138,7 @@ func TestApplyPendingTargetedSharedImageInternal(t *testing.T) { } } -func TestApplyPendingTargetedRetainsRejectedInternal(t *testing.T) { +func TestApplyPendingTargetedRetainsRejected(t *testing.T) { for _, tt := range []struct { name string change func(*container.InspectResponse) @@ -159,13 +159,13 @@ func TestApplyPendingTargetedRetainsRejectedInternal(t *testing.T) { {name: "pull failure", pullError: true, wantPulls: 1}, } { t.Run(tt.name, func(t *testing.T) { - cnt := newTargetedContainerInternal("one", "1.x") + cnt := newTargetedContainer("one", "1.x") if tt.change != nil { tt.change(&cnt) } - fixture := &targetedDockerInternal{containers: []container.InspectResponse{cnt}, created: map[string]string{}} - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0")) + fixture := &targetedDocker{containers: []container.InspectResponse{cnt}, created: map[string]string{}} + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + store := NewMemoryPendingStore(targetedPending("one", "1.2.0")) puller := &fakePuller{} if tt.pullError { puller.err = errors.New("registry unavailable") @@ -192,15 +192,15 @@ func TestApplyPendingTargetedRetainsRejectedInternal(t *testing.T) { } } -type targetedComposeAdapterInternal struct { +type targetedComposeAdapter struct { fakeProjectUpdater - fixture *targetedDockerInternal + fixture *targetedDocker calls int changes map[string]updatetypes.ServiceImageChange fail bool } -func (f *targetedComposeAdapterInternal) UpdateServiceImages(_ context.Context, projectID string, changes map[string]updatetypes.ServiceImageChange) error { +func (f *targetedComposeAdapter) UpdateServiceImages(_ context.Context, projectID string, changes map[string]updatetypes.ServiceImageChange) error { f.calls++ f.changes = changes if f.fail { @@ -217,7 +217,7 @@ func (f *targetedComposeAdapterInternal) UpdateServiceImages(_ context.Context, return nil } -func TestApplyPendingTargetedComposeGroupsInternal(t *testing.T) { +func TestApplyPendingTargetedComposeGroups(t *testing.T) { for _, tt := range []struct { name string conflict, fail bool @@ -225,7 +225,7 @@ func TestApplyPendingTargetedComposeGroupsInternal(t *testing.T) { {name: "group service changes"}, {name: "conflicting service targets", conflict: true}, {name: "adapter failure retained", fail: true}, } { t.Run(tt.name, func(t *testing.T) { - fixture := &targetedDockerInternal{containers: []container.InspectResponse{newTargetedContainerInternal("one", "1.x"), newTargetedContainerInternal("two", "1.x")}, created: map[string]string{}} + fixture := &targetedDocker{containers: []container.InspectResponse{newTargetedContainer("one", "1.x"), newTargetedContainer("two", "1.x")}, created: map[string]string{}} for i := range fixture.containers { fixture.containers[i].Config.Labels[compose.ProjectLabelKey] = "app" fixture.containers[i].Config.Labels[compose.ServiceLabelKey] = fixture.containers[i].ID @@ -233,9 +233,9 @@ func TestApplyPendingTargetedComposeGroupsInternal(t *testing.T) { if tt.conflict { fixture.containers[1].Config.Labels[compose.ServiceLabelKey] = "one" } - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - adapter := &targetedComposeAdapterInternal{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}, fixture: fixture, fail: tt.fail} - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0"), targetedPendingInternal("two", "1.3.0")) + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + adapter := &targetedComposeAdapter{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}, fixture: fixture, fail: tt.fail} + store := NewMemoryPendingStore(targetedPending("one", "1.2.0"), targetedPending("two", "1.3.0")) puller := &fakePuller{} service := newService(Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, ImagePuller: puller, ProjectUpdater: adapter}) result, err := service.ApplyPending(t.Context(), Options{}) @@ -268,13 +268,13 @@ func TestApplyPendingTargetedComposeGroupsInternal(t *testing.T) { } } -func TestApplyPendingTargetedScopedClearingInternal(t *testing.T) { - fixture := &targetedDockerInternal{containers: []container.InspectResponse{newTargetedContainerInternal("one", "1.x"), newTargetedContainerInternal("two", "<1.1.0")}, created: map[string]string{}} +func TestApplyPendingTargetedScopedClearing(t *testing.T) { + fixture := &targetedDocker{containers: []container.InspectResponse{newTargetedContainer("one", "1.x"), newTargetedContainer("two", "<1.1.0")}, created: map[string]string{}} // One record is already applied. Its successful verification must not clear // the rejected record for a different container using the same image ID. fixture.containers[0].Config.Image = "app:1.2.0" - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0"), targetedPendingInternal("two", "1.2.0"), targetedPendingInternal("missing", "1.2.0")) + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + store := NewMemoryPendingStore(targetedPending("one", "1.2.0"), targetedPending("two", "1.2.0"), targetedPending("missing", "1.2.0")) puller := &fakePuller{} service := newService(Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, ImagePuller: puller}) result, err := service.ApplyPending(t.Context(), Options{}) @@ -300,29 +300,29 @@ func TestApplyPendingTargetedScopedClearingInternal(t *testing.T) { } } -type selectiveTargetPullerInternal struct{ failedRef string } +type selectiveTargetPuller struct{ failedRef string } -func (p selectiveTargetPullerInternal) PullImage(_ context.Context, ref string, _ io.Writer) error { +func (p selectiveTargetPuller) PullImage(_ context.Context, ref string, _ io.Writer) error { if ref == p.failedRef { return errors.New("target unavailable") } return nil } -func TestApplyPendingTargetedFailedComposeDependencyRetainedInternal(t *testing.T) { - fixture := &targetedDockerInternal{containers: []container.InspectResponse{newTargetedContainerInternal("one", "1.x"), newTargetedContainerInternal("two", "1.x")}, created: map[string]string{}} +func TestApplyPendingTargetedFailedComposeDependencyRetained(t *testing.T) { + fixture := &targetedDocker{containers: []container.InspectResponse{newTargetedContainer("one", "1.x"), newTargetedContainer("two", "1.x")}, created: map[string]string{}} for i := range fixture.containers { fixture.containers[i].Config.Labels[compose.ProjectLabelKey] = "app" fixture.containers[i].Config.Labels[compose.ServiceLabelKey] = fixture.containers[i].ID } fixture.containers[1].Config.Labels[labels.LabelDependsOn] = "one" - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - adapter := &targetedComposeAdapterInternal{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}, fixture: fixture} - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0"), targetedPendingInternal("two", "1.3.0")) + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + adapter := &targetedComposeAdapter{fakeProjectUpdater: fakeProjectUpdater{projects: map[string]ComposeProject{"app": {ID: "project-app"}}}, fixture: fixture} + store := NewMemoryPendingStore(targetedPending("one", "1.2.0"), targetedPending("two", "1.3.0")) service := newService(Config{ DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, - ImagePuller: selectiveTargetPullerInternal{failedRef: "docker.io/library/app:1.3.0"}, + ImagePuller: selectiveTargetPuller{failedRef: "docker.io/library/app:1.3.0"}, ProjectUpdater: adapter, }) result, err := service.ApplyPending(t.Context(), Options{}) @@ -351,14 +351,12 @@ func TestApplyPendingTargetedFailedComposeDependencyRetainedInternal(t *testing. } } -// Settings exclusions, stopped containers and the Docker proxy are all left -// alone by a pending batch, even when the batch is forced: the -// IgnoreSettingsExclusions override is reserved for an explicitly requested -// UpdateContainer target. -func TestApplyPendingTargetedExcludedContainersInternal(t *testing.T) { +// A pending batch, even forced, leaves settings exclusions, stopped containers and the Docker proxy +// alone; IgnoreSettingsExclusions only applies to an explicit UpdateContainer target. +func TestApplyPendingTargetedExcludedContainers(t *testing.T) { for _, name := range []string{"stopped", "docker-proxy", "settings-excluded"} { t.Run(name, func(t *testing.T) { - cnt := newTargetedContainerInternal(name, "1.x") + cnt := newTargetedContainer(name, "1.x") if name == "stopped" { cnt.State.Running = false } @@ -366,8 +364,8 @@ func TestApplyPendingTargetedExcludedContainersInternal(t *testing.T) { if name == "settings-excluded" { settings.excluded = []string{name} } - fixture := &targetedDockerInternal{containers: []container.InspectResponse{cnt}, created: map[string]string{}} - server := httptest.NewServer(fixture.handlerInternal(t)) + fixture := &targetedDocker{containers: []container.InspectResponse{cnt}, created: map[string]string{}} + server := httptest.NewServer(fixture.handler(t)) defer server.Close() dockerClient, err := client.New(client.WithHost("tcp://docker-proxy:2375"), client.WithAPIVersion("1.41"), client.WithDialContext(func(ctx context.Context, network, _ string) (net.Conn, error) { return (&net.Dialer{}).DialContext(ctx, network, server.Listener.Addr().String()) @@ -380,7 +378,7 @@ func TestApplyPendingTargetedExcludedContainersInternal(t *testing.T) { t.Error(closeErr) } }() - store := NewMemoryPendingStore(targetedPendingInternal(name, "1.2.0")) + store := NewMemoryPendingStore(targetedPending(name, "1.2.0")) puller := &fakePuller{} service := newService(Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, ImagePuller: puller, Settings: settings}) result, err := service.ApplyPending(t.Context(), Options{Force: true, IgnoreSettingsExclusions: true}) @@ -400,12 +398,12 @@ func TestApplyPendingTargetedExcludedContainersInternal(t *testing.T) { } } -func TestApplyPendingTargetedSatisfiedDependencyClearedInternal(t *testing.T) { - fixture := &targetedDockerInternal{containers: []container.InspectResponse{newTargetedContainerInternal("one", "1.x"), newTargetedContainerInternal("two", "1.x")}, created: map[string]string{}} +func TestApplyPendingTargetedSatisfiedDependencyCleared(t *testing.T) { + fixture := &targetedDocker{containers: []container.InspectResponse{newTargetedContainer("one", "1.x"), newTargetedContainer("two", "1.x")}, created: map[string]string{}} fixture.containers[1].Config.Image = "app:1.3.0" fixture.containers[1].Config.Labels[labels.LabelDependsOn] = "one" - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0"), targetedPendingInternal("two", "1.3.0")) + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + store := NewMemoryPendingStore(targetedPending("one", "1.2.0"), targetedPending("two", "1.3.0")) service := newService(Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, ImagePuller: &fakePuller{}}) result, err := service.ApplyPending(t.Context(), Options{}) if err != nil { @@ -427,17 +425,17 @@ func TestApplyPendingTargetedSatisfiedDependencyClearedInternal(t *testing.T) { } } -func TestApplyPendingAutomaticTagPolicyInternal(t *testing.T) { +func TestApplyPendingAutomaticTagPolicy(t *testing.T) { for _, strategy := range []string{"", "auto", "digest"} { t.Run(strategy, func(t *testing.T) { - cnt := newTargetedContainerInternal("one", "") + cnt := newTargetedContainer("one", "") cnt.Config.Labels = map[string]string{} if strategy != "" { cnt.Config.Labels[labels.LabelUpdateStrategy] = strategy } - fixture := &targetedDockerInternal{containers: []container.InspectResponse{cnt}, created: map[string]string{}} - dockerClient := newDockerClientForHandler(t, fixture.handlerInternal(t)) - store := NewMemoryPendingStore(targetedPendingInternal("one", "1.2.0")) + fixture := &targetedDocker{containers: []container.InspectResponse{cnt}, created: map[string]string{}} + dockerClient := newDockerClientForHandler(t, fixture.handler(t)) + store := NewMemoryPendingStore(targetedPending("one", "1.2.0")) puller := &fakePuller{} service := newService(Config{DockerClientProvider: &fakeDockerClientProvider{client: dockerClient}, PendingStore: store, ImagePuller: puller}) result, err := service.ApplyPending(t.Context(), Options{}) diff --git a/updater/types.go b/updater/types.go index f19a72f..9b6e81c 100644 --- a/updater/types.go +++ b/updater/types.go @@ -13,17 +13,12 @@ type Options struct { // DryRun reports what would be updated without pulling or recreating // anything. DryRun bool `json:"dryRun,omitempty"` - // IgnoreSettingsExclusions lets UpdateContainer update its explicitly - // requested container even when the SettingsProvider excludes it from - // automatic updates. It applies only to that target: ApplyPending and the - // restart and dependency passes keep honoring exclusions, and a container - // that is disabled by label, a Swarm task, or on an immutable reference - // stays ineligible. The exclusion itself is never modified. + // IgnoreSettingsExclusions lets UpdateContainer update its requested container despite a settings + // exclusion. Batch and dependency passes still honor exclusions, and label, Swarm, and immutability checks still apply. IgnoreSettingsExclusions bool `json:"ignoreSettingsExclusions,omitempty"` } -// exclusionPolicy tells containerEligibilityInternal whether to consult the -// SettingsProvider's exclusion list. +// exclusionPolicy tells containerEligibility whether to consult the SettingsProvider's exclusions. type exclusionPolicy bool const ( @@ -31,12 +26,6 @@ const ( ignoreSettingsExclusions exclusionPolicy = false ) -// targetExclusionPolicyInternal is the policy for the container a caller -// explicitly asked UpdateContainer to update. -func (o Options) targetExclusionPolicyInternal() exclusionPolicy { - return exclusionPolicy(!o.IgnoreSettingsExclusions) -} - // ResourceResult represents the result of an update operation on one resource. type ResourceResult struct { ResourceID string `json:"resourceId"` @@ -149,9 +138,8 @@ type SelfUpdateTarget struct { // NewImageRef is the resolved image reference the self-updater should // upgrade the container to. Empty when the updater could not resolve one. NewImageRef string - // PullImageRef is an immutable reference the host froze the update to. - // The container must still be recreated as NewImageRef. Empty when the - // host pulls NewImageRef directly. + // PullImageRef is an immutable reference the host froze the update to; the container is still + // recreated as NewImageRef. Empty when the host pulls NewImageRef directly. PullImageRef string } diff --git a/updater/types/updates.go b/updater/types/updates.go index 31b45fd..a63cadf 100644 --- a/updater/types/updates.go +++ b/updater/types/updates.go @@ -3,9 +3,8 @@ package types import "context" -// Policy controls image updates. The zero policy follows the current tag's digest. -// A constraint or tag pattern without a strategy, or strategy auto, selects tag -// updates for stable complete semantic versions. +// Policy controls image updates. The zero policy follows the current tag's digest; a constraint or +// tag pattern, or strategy auto, selects tag updates for stable complete semantic versions. type Policy struct { Strategy string `json:"strategy,omitempty"` Constraint string `json:"constraint,omitempty"` @@ -47,9 +46,8 @@ type ServiceImageChange struct { TargetRef string `json:"targetRef"` } -// ProjectImageUpdater persists service image changes and recreates those services. -// Returning success guarantees persistence as well as recreation. Implement this -// optional interface on the configured ProjectUpdater to support tag changes. +// ProjectImageUpdater persists service image changes and recreates those services; success guarantees both. +// Implement it on the configured ProjectUpdater to support tag changes. type ProjectImageUpdater interface { UpdateServiceImages(ctx context.Context, projectID string, changes map[string]ServiceImageChange) error } diff --git a/updater/updater.go b/updater/updater.go index b408b11..102f4ba 100644 --- a/updater/updater.go +++ b/updater/updater.go @@ -1,22 +1,22 @@ package updater import ( + "cmp" "context" + "errors" "log/slog" "sync/atomic" "github.com/moby/moby/api/types/container" ) -// Service coordinates Docker image updates, container recreation, and the host -// adapters configured through Config. A Service is safe for concurrent use. +// Service coordinates Docker image updates, container recreation, and the host adapters configured +// through Config. A Service is safe for concurrent use. type Service struct { config Config logger *slog.Logger - // ownedDockerClient is the default provider this Service constructed, and - // is therefore the only one Close may shut down. A caller-supplied provider - // belongs to the caller. + // ownedDockerClient is the default provider New built, the only one Close may shut down. ownedDockerClient *DockerClient updatingContainers atomic.Pointer[[]string] @@ -24,12 +24,11 @@ type Service struct { } type updatePlan struct { - record ImageUpdateRecord - oldRef string - newRef string - oldIDs []string - pulled bool - restartFailed bool + record ImageUpdateRecord + oldRef string + newRef string + oldIDs []string + pulled bool } type restartPlan struct { @@ -37,50 +36,43 @@ type restartPlan struct { inspect *container.InspectResponse newRef string match string - explicit bool implicit bool } -// New constructs an updater service. The zero Config is valid and yields a -// service backed entirely by the local Docker environment: -// -// service, err := updater.New(updater.Config{}) -// -// New fills in a default for every port that has one, merges each nil -// LabelPolicy func from DefaultLabelPolicy, and then validates the result. It -// returns a ConfigError when the configuration cannot produce a usable service. +// New constructs an updater service; the zero Config is valid and uses the local Docker environment. +// Nil ports get their Docker-backed defaults and nil LabelPolicy funcs come from DefaultLabelPolicy. func New(config Config) (*Service, error) { - ownedDockerClient := applyConfigDefaults(&config) - if err := config.validate(); err != nil { - return nil, err + if config.OperationTimeout < 0 { + return nil, &ConfigError{Field: "OperationTimeout", Reason: errors.New("must not be negative")} } + var owned *DockerClient + if config.DockerClientProvider == nil { + owned = NewDockerClientProvider() + config.DockerClientProvider = owned + } + config.ImagePuller = cmp.Or(config.ImagePuller, NewImagePuller(config.DockerClientProvider)) + config.PendingStore = cmp.Or(config.PendingStore, NewMemoryPendingStore()) + config.RegistryDigestResolver = cmp.Or(config.RegistryDigestResolver, NewRegistryDigestResolver()) + config.RegistryTagLister = cmp.Or(config.RegistryTagLister, NewRegistryTagLister()) + config.ProjectUpdater = cmp.Or(config.ProjectUpdater, NewDockerComposeProjectUpdater(config.DockerClientProvider)) service := newService(config) - service.ownedDockerClient = ownedDockerClient + service.ownedDockerClient = owned return service, nil } -// newService assembles a Service from config as given, without filling -// in any port defaults. New applies the defaults first; tests use this directly -// to run against their own fakes with the remaining ports left nil. +// newService assembles a Service from config without port defaults, so tests can leave ports nil. func newService(config Config) *Service { config.LabelPolicy = mergeLabelPolicyDefaults(config.LabelPolicy) - logger := config.Logger - if logger == nil { - logger = slog.Default() - } service := &Service{ config: config, - logger: logger, + logger: cmp.Or(config.Logger, slog.Default()), } service.updatingContainers.Store(&[]string{}) service.updatingProjects.Store(&[]string{}) return service } -// Close releases the Docker client New created for this Service. It is a no-op -// when the caller supplied their own DockerClientProvider, because that -// provider's lifecycle belongs to the caller. Close is safe to call more than -// once. +// Close releases the Docker client New created; a caller-supplied provider is left to its owner. Close is idempotent. func (s *Service) Close() error { if s == nil || s.ownedDockerClient == nil { return nil @@ -88,33 +80,6 @@ func (s *Service) Close() error { return s.ownedDockerClient.Close() } -// applyConfigDefaults fills in the Docker-backed default for every port -// config leaves nil. It returns the Docker client it created, if any, so the -// Service knows which client it owns. -func applyConfigDefaults(config *Config) *DockerClient { - var owned *DockerClient - if config.DockerClientProvider == nil { - owned = NewDockerClientProvider() - config.DockerClientProvider = owned - } - if config.ImagePuller == nil { - config.ImagePuller = NewImagePuller(config.DockerClientProvider) - } - if config.PendingStore == nil { - config.PendingStore = NewMemoryPendingStore() - } - if config.RegistryDigestResolver == nil { - config.RegistryDigestResolver = NewRegistryDigestResolver() - } - if config.RegistryTagLister == nil { - config.RegistryTagLister = NewRegistryTagLister() - } - if config.ProjectUpdater == nil { - config.ProjectUpdater = NewDockerComposeProjectUpdater(config.DockerClientProvider) - } - return owned -} - func (s *Service) opCtx(ctx context.Context) (context.Context, context.CancelFunc) { if s == nil || s.config.OperationTimeout <= 0 { return ctx, func() {}