Skip to content

chore(compose): comments state behavior, not history #14130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
glours merged 1 commit into from
Aug 27, 2026
+39 −24
Merged

chore(compose): comments state behavior, not history #14130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/compose/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
return err
}

// Emit "Running" events for containers that are already up-to-date,
// matching the previous convergence behavior for progress display.
// Emit "Running" events for containers that are already up-to-date, so
// the progress display accounts for containers the plan will not touch.
emitRunningEvents(project, observed, plan, s.events)

return s.executePlan(ctx, project, observed, plan)
Expand Down Expand Up @@ -1461,8 +1461,8 @@ func (s *composeService) createNetwork(ctx context.Context, n *types.NetworkConf
if _, err := s.apiClient().NetworkCreate(ctx, n.Name, networkCreateOptions); err != nil {
// A concurrent `docker compose up|run` may have created the same network
// between the observed-state snapshot and now. Treat the resulting
// conflict as success rather than failing hard, mirroring the retry the
// previous ensureNetwork performed.
// conflict as success rather than failing hard: the network we wanted
// exists.
if errdefs.IsConflict(err) {
s.events.On(createdEvent(networkEventName))
return nil
Expand Down
10 changes: 6 additions & 4 deletions pkg/compose/observed_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,9 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
}

// --- Containers ---
// Use oneOffInclude to detect orphaned one-off containers (matching the
// previous behavior of create() which used oneOffInclude + isOrphaned).
// One-off (run) containers are included in the listing on purpose: exited
// one-offs are classified as orphans below, so `--remove-orphans` can
// clean them up.
raw, err := s.getContainers(ctx, project.Name, oneOffInclude, true)
if err != nil {
return nil, err
Expand Down Expand Up @@ -365,8 +366,9 @@ func (s *ObservedState) setResolvedVolumes(volumes map[string]string) {
}

// emitRunningEvents emits "Running" progress events for containers that are already
// running and have no operations planned for them. This matches the previous behavior
// where convergence.ensureService emitted runningEvent for up-to-date containers.
// running and have no operations planned for them, so the progress display
// accounts for every container of the project — including the up-to-date ones
// the plan deliberately leaves alone.
//
// Iterates project.Services (not observed.Containers) so that containers of
// disabled services (e.g. dependencies untouched by `compose run --no-deps`)
Expand Down
45 changes: 29 additions & 16 deletions pkg/compose/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ func (r *reconciler) resolveObserved() {
// Divergence is detected by comparing NetworkHash(desired) with the config-hash
// persisted on the live network (observed.ConfigHash). A network with no
// recorded hash (e.g. created by an older Compose or manually) is left
// untouched, matching the previous ensureNetwork behavior.
// untouched: without a recorded hash there is no reliable way to tell
// configuration drift apart from deliberate manual setup.
//
// A rename (observed.Name != desired.Name) also diverges the hash — NetworkHash
// includes the name — and is handled by the same recreation path: the old
Expand Down Expand Up @@ -242,7 +243,8 @@ func (r *reconciler) planCreateNetwork(key string, networkConfig *types.NetworkC
// Attached containers must be disconnected before the network can be removed
// (Docker refuses to remove a network with active endpoints) and are reconnected
// to the fresh network afterwards — they keep their identity and are not
// recreated, matching the previous ensureNetwork/removeDivergedNetwork behavior.
// recreated: a network definition change alone is not a reason to lose
// container state.
//
// Stops are deduplicated through stoppedByPlan so a container attached to several
// diverged networks (or later recreated by reconcileContainers) is stopped once.
Expand Down Expand Up @@ -345,8 +347,9 @@ func (r *reconciler) planRecreateNetworks(keys []string) {
//
// Divergence is detected by comparing VolumeHash(desired) with the config-hash
// persisted on the live volume (observed.ConfigHash). A volume with no recorded
// hash (e.g. created by an older Compose) is left untouched, matching the
// previous ensureVolume behavior.
// hash (e.g. created by an older Compose) is left untouched: without a
// recorded hash there is no reliable way to tell configuration drift apart
// from deliberate manual setup, and volumes carry data.
func (r *reconciler) reconcileVolumes() error {
var diverged []string
for _, key := range sortedKeys(r.project.Volumes) {
Expand All @@ -368,16 +371,16 @@ func (r *reconciler) reconcileVolumes() error {
}
if observed.Name != desired.Name {
// The volume was renamed: the live volume matched by label carries a
// different name, i.e. a distinct Docker resource. Match the
// historical additive behavior — create the new volume and leave the
// old one (and its data) untouched — instead of prompting to delete
// data under a name that does not exist yet.
// different name, i.e. a distinct Docker resource. A rename is
// additive — create the new volume and leave the old one (and its
// data) untouched — instead of prompting to delete data under a
// name that does not exist yet.
r.planCreateVolume(key, &desired, "renamed")
// Rewrite the observed name to the desired one so reconcileContainers
// detects the mount mismatch and migrates existing containers onto
// the new volume within the same up (as the pre-reconcile ensureVolume
// path did), and so later runs match deterministically on the new
// name rather than split-braining between the two.
// the new volume within the same up, and so later runs match
// deterministically on the new name rather than split-braining
// between the two.
observed.Name = desired.Name
r.resolvedVolumes[key] = observed
continue
Expand Down Expand Up @@ -676,8 +679,9 @@ func (r *reconciler) reconcileService(service types.ServiceConfig) error {
}
parentRecreated := r.parentNamespaceRecreated(service)

// Sort containers: obsolete first, then by number descending, then reverse
// to get the same ordering as the existing convergence code.
// Order containers so that the scale-down below (i >= expected) trims
// obsolete containers and the highest replica numbers first — see
// sortContainers for the resulting ordering.
r.sortContainers(containers, service, expectedHash, parentRecreated, strategy)

// Collect dependency nodes that container creation should depend on
Expand Down Expand Up @@ -716,7 +720,11 @@ func (r *reconciler) reconcileService(service types.ServiceConfig) error {
// Container is up-to-date
switch oc.State {
case container.StateRunning, container.StateCreated, container.StateRestarting, container.StateExited:
// Nothing to do (exited containers are left as-is, matching service_containers.go behavior)
// Nothing to plan. Starting created/exited containers is NOT the
// plan's job: `up` runs a separate start phase afterwards
// (start.go), which lists containers again and starts them in
// dependency order. Exited containers are deliberately left as-is
// here so that phase (or the user) decides.
default:
// Any other state (paused, dead, ...): attempt to (re)start
lastNode = r.plan.addNode(Operation{
Expand Down Expand Up @@ -1009,8 +1017,13 @@ func (r *reconciler) infrastructureDeps(service types.ServiceConfig) []*PlanNode
return deps
}

// sortContainers sorts containers the same way as the start path in service_containers.go:
// obsolete first, then by container number descending, then reversed.
// sortContainers orders the slice so that, read from the front, up-to-date
// containers come first in ascending replica-number order, followed by the
// containers that must be recreated (the comparator sorts obsolete-first and
// number-descending, then the slice is reversed). Scale-down trims the tail
// of this slice (i >= expected in reconcileService), so this ordering is what
// guarantees that obsolete containers and the highest replica numbers are
// removed first while low-numbered healthy replicas survive.
//
// mustRecreate is evaluated once per container before sorting to avoid
// quadratic re-evaluation in the comparator.
Expand Down