Pulling an oci:// resource resolved each layer digest through the
registry manifests endpoint, which answers 500 when the digest points
to a non-manifest blob. containerd v2.3.0+ (pulled in by buildx v0.36
and buildkit v0.32) no longer falls back to the blobs endpoint unless
manifests returned 404, so publish/pull of compose artifacts broke.
Fetch layers directly with the descriptors already listed in the
manifest instead of resolving them again.
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
Follow-up to #13603: scale, watch and shell completion loaded the
project without any tolerance option, so a missing env_file on a
service not involved in the operation aborted the command, while
up/exec/ps already tolerate this since #13156 and #13603.
Mirror the WithServices pattern: load with WithoutEnvironmentResolution
and resolve the environment once the project has been reduced to the
selected services, so targeted services still get their env_file
validated. Completion only needs names and never resolves. This also
aligns the config hash of scale-created containers with up-created
ones.
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
The runServices, runVolumes, runNetworks, runModels and runHash paths
called ProjectOptions.ToProject directly, bypassing the configOptions
wrapper that applies --no-consistency, --no-interpolate, --no-normalize,
--no-path-resolution, --profile filtering and env_file discarding.
Restore the variadic wrapper (mirroring configOptions.ToModel) and route
the five call sites through it.
Regression introduced by b80bb0586 (LoadProject API migration).
Fixes#13974
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
The caller in pkg/compose/cp.go unconditionally defers
res.Content.Close() once the call succeeds. The dry-run client
returned a zero-value result with a nil Content reader, so
`docker compose cp --dry-run <ctr>:<path> <dst>` panicked with a
nil pointer dereference. Return an empty NopCloser instead, matching
the pattern already used for the other stream results in this file.
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
addPreStartHookPulls skipped any hook image already present locally,
regardless of pull policy. As a result a service with pull_policy: always
had its own image force-pulled on every up while its pre_start hook images
were left stale — diverging from both the service image and the `pull`
command path (which already re-pulls hooks under always).
Skip the "already present" shortcut when the parent service is
pull_policy: always, so hook images get the same force-pull treatment.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Address review feedback on pre_start hook image resolution:
1. GetDependentImages now skips a hook image equal to the service image
(resolved via GetImageNameOrDefault), so `config --images` no longer
prints a duplicate line and pullRequiredImages no longer schedules a
redundant pull for it.
2. pullRequiredImages (up/create path) now dedups dependent images by
reference via a `scheduled` set, so several hooks/services sharing the
same missing image don't schedule concurrent redundant pulls. The hook
pass moved to a helper (addPreStartHookPulls) to keep complexity in check.
3. The `pull` command no longer skips hook images under `pull_policy: build`.
A hook image is a registry image that can't be built, so only `never`
justifies skipping it — making `pull` consistent with the `up` path.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
pre_start hooks run as ephemeral init containers with their own image
(ServiceHook.Image), but that image was ignored by image resolution:
`config --images` didn't list it, `pull` didn't fetch it, and `up` failed
at runtime with "No such image" when it wasn't already present locally.
Add a GetDependentImages helper that returns a service's pre_start hook
images, and use it wherever service images are collected/pulled:
getLocalImagesDigests, pullRequiredImages (up path), the pull command, and
`config --images`. Hook images inherit the parent service pull policy.
post_start/pre_stop hooks run via ExecCreate inside the service container
and never use hook.Image, so they are intentionally out of scope.
Digest resolution/locking (--resolve-image-digests / --lock-image-digests)
is not covered: compose-go's WithImagesResolved only resolves service.Image
(needs an upstream change), and the --lock-image-digests override merges
pre_start by concatenation, which would duplicate hooks.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
collectObservedState indexed networks/volumes by compose label into a
single-valued map, so two live resources sharing a label (e.g. a leftover
after a rename) collided and the "winner" depended on the daemon's list
order — a nondeterministic `up` (spurious create/recreate events, possible
churn) on subsequent runs.
Make collection lossless and move the selection into the reconciler:
- ObservedState.Networks/Volumes become map[string][]Observed*: collection
records every label-sharing resource and makes no premature choice.
- selectNetwork/selectVolume deterministically pick the resource matching
the desired name (else the lexicographically smallest), returning the
others as orphans.
- reconcile resolves the observed state once (resolveObserved) into
single-valued resolvedNetworks/resolvedVolumes used everywhere, and warns
about orphans instead of acting on them — they are left untouched because
removing them could drop data or break unrelated workloads.
Adds selection unit tests, a collector aggregation test and a reconcile
conflict test (deterministic no-op + orphan warning across list orders).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
A network rename does not require removing the old network — the new one
has a different name and is created independently. Yet the old removal
could block the whole operation: NetworkRemove fails when non-Compose
containers are still attached, and CreateNetwork depended on it.
Split the rename path from the same-name divergence path:
- Rename: CreateNetwork no longer depends on RemoveNetwork; the container
migration proceeds regardless. RemoveNetwork is marked best-effort and,
if the network is still in use (reported as a conflict), is skipped with
a warning instead of failing. Any other error (transport, Moby API) is
still propagated.
- Same-name divergence keeps the mandatory remove-before-create ordering.
Adds an Operation.BestEffort flag, honored by execRemoveNetwork, plus
reconcile and executor tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Address review findings on the network reconcile migration:
- createNetwork now treats a NetworkCreate conflict as success. A
concurrent `docker compose up|run` can create the same network in the
TOCTOU window between the observed-state snapshot and the create call;
the previous ensureNetwork retried on conflict, the plain create must
not fail hard.
- discoverUnmanagedNetworks/Volumes preserve the config-hash when the live
resource is owned by this project (project label present, key label
absent — e.g. written by an older Compose) so genuine divergence is
still detected. For resources we don't own the hash stays empty and they
are reused untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Treat a network rename (observed.Name != desired.Name) as a recreation
rather than an additive create: the old network is removed, the new one
created, and attached containers are migrated onto it (reconnected), so
they no longer stay on the previous network until recreated for another
reason.
Networks carry no data, so removing the previous network — instead of
leaving it dangling alongside the new one under the same compose label —
is safe and keeps subsequent runs deterministic. This is a marginal
behavior change from previous Compose releases (which created the new
network and left the old attachments in place) in exchange for the more
logical outcome.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Mirror the volume reconciliation work (#13962) for networks: move network
divergence detection and recreation out of the imperative pre-reconcile
path (ensureNetwork/resolveOrCreateNetwork/removeDivergedNetwork) into the
reconciliation plan.
- reconcileNetworks now owns creation of missing networks and, for a
network whose config-hash diverged, an explicit recreation sequence
(no user confirmation: recreating a network is not destructive):
stop containers -> disconnect -> remove network -> create network ->
reconnect containers. Attached containers keep their identity (they are
reconnected, not recreated), matching the previous behavior. If a
container is independently recreated by reconcileContainers, its removal
is ordered after the reconnect so they don't race.
- Renaming a network creates the new one additively and leaves the old one
untouched.
- collectObservedState discovers legacy/unlabeled networks by name and
records them as unmanaged matches (empty config-hash) so the reconciler
reuses them untouched; ownership warnings move to warnUnmanagedNetworks.
checkExternalNetworks keeps external-network validation/resolution.
- execCreateNetwork now issues a plain createNetwork; the imperative
ensureNetwork/resolveOrCreateNetwork/removeDivergedNetwork and the
connect/disconnect helpers are removed.
Adds reconcile, observed-state and executor tests covering network
create/diverge/rename, the entangled diverge+recreate case, legacy
by-name discovery and the ownership warnings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Rely on compose-go WithImagesResolved, which now resolves dependent
images — `type: image` volume sources and pre_start hook images —
with its already-digested guard, per-call memoization and
sibling-service detection (compose-spec/compose-go#894, #899), rather
than duplicating resolution logic CLI-side. The interpolated path gets
this for free; --no-interpolate maps the raw model onto a
pseudo-project keyed by service names to reuse the same resolution,
and --lock-image-digests keeps type:image volumes in its output.
pre_start hooks can't be carried into the lock override (hook lists
are appended on merge), so generating a lock warns that hook images
stay unpinned there.
As a side effect, `compose publish` now fails fast on unresolvable
dependent images.
Fixes#13827
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
The additive rename path created the new volume but kept the old name in
the observed state, so hasVolumeMismatch never fired: existing containers
stayed on the old volume while fresh replicas mounted the new one
(split-brain), and later runs picked a nondeterministic winner between the
two equally labelled volumes.
Rewrite the observed volume name to the desired one after planning the
"renamed" create, so reconcileContainers migrates the existing containers
onto the new volume in the same up — restoring parity with the old
ensureVolume path — while still leaving the old volume and its data intact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Two edge regressions from the switch to a label-scoped observed state,
both reported against the old ensureVolume path:
- A same-named volume created manually or by another project (no compose
label) was invisible to the observed state, so a VolumeCreate was
planned on every up: a hard failure if the driver differed, spurious
Creating/Created events otherwise. collectObservedState now discovers
such volumes by name (pre-label Compose semantics) and records them as
unmanaged matches with an empty config-hash, so the reconciler reuses
them untouched. The ownership warnings move to warnUnmanagedVolumes,
driven off the observed state; checkVolumes shrinks to external-only
validation (checkExternalVolumes).
- Renaming a volume hit the diverged path and, with up -y, deleted the
old volume and its data (VolumeHash includes Name), where it previously
just created the new one. When observed.Name != desired.Name the volume
is now created additively, leaving the old one untouched, with no prompt.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
servicesUsingVolume only matched services mounting the volume directly, so
a service reaching it through volumes_from was not stopped/removed before
RemoveVolume. Docker materializes the inherited mount on the consumer's
container, so its removal would fail with "volume in use". Compute the
transitive volumes_from closure so every container referencing the volume
is removed first. (network_mode/ipc/pid: service:x share namespaces, not
mounts, and are intentionally excluded.)
Also reassign the result of Labels.Add in createVolume: it mutates in
place only when the map is non-nil, so discarding the return would drop
the config-hash label for a volume with no CustomLabels.
Addresses review feedback: documents why observed.Containers is cleared
without touching the observedContainersByService hashing snapshot, and
strengthens the cascade tests to assert the full plan ordering.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Move volume divergence detection and recreation out of the imperative
pre-reconcile path (ensureVolume/removeDivergedVolume) and into the
reconciliation plan, activating the dormant planRecreateVolume seam.
A diverged volume now produces an explicit, forward-only sequence:
stop containers -> remove containers -> remove volume -> create volume
-> create containers. Container re-creation is delegated to
reconcileContainers (affected services are cleared from the observed
snapshot so they are scheduled fresh, gated on the CreateVolume node),
and the recreation cascades to namespace/volume-sharing dependents.
User confirmation (recreate, data will be lost) is consulted while
building the plan via reconciler.prompt; declining leaves the volume
untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
`docker compose -f oci://<insecure-registry>/... up` failed against a
plain-HTTP registry unless --yes was passed:
failed to pull OCI resource "localhost:5000/test:interpolated":
Head "https://localhost:5000/v2/test/manifests/interpolated":
http: server gave HTTP response to HTTPS client
`up` loads the project twice. The first load goes through ToProject,
which built its OCI options from --insecure-registry correctly. Without
--yes, checksForRemoteStack then calls promptForInterpolatedVariables,
which re-loads the project through ToModel to list the interpolation
variables. That second load builds its own resource loaders via
remoteLoaders, and those passed an empty api.OCIOptions{}, dropping the
flag. Since the OCI loader always performs a network resolve, the
re-load spoke HTTPS to a plain-HTTP registry and failed before the
prompt could be shown.
The two construction sites had drifted apart, so rather than patching
the second one, both now share ProjectOptions.ociOptions(). `config`
and `viz` use the same ToModel path and are fixed as well.
Covered by an e2e case in TestPublish, which already runs an insecure
registry and an oci:// round-trip: it publishes a fixture carrying an
interpolation variable so the prompt fires, then runs `up` without
--yes and declines, asserting the re-load does not fail with
"server gave HTTP response to HTTPS client".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Domantas Petrauskas <dom.petrauskas@gmail.com>
With the containerd image store and BuildKit provenance attestations
(the default), a built image is stored as an attested index whose
top-level digest also covers the attestation manifest. That digest
churns on every build even when the runnable content is unchanged,
so compose recreated containers on every `up --build`.
Compare the digest of the "image" kind manifest instead, selected for
the target platform and restricted to locally available manifests, so
it is deterministic and reflects only config + layers. Both the build
and up sides of the staleness check go through the same selection, and
registry-only images keep the Bake-reported digest.
Fixes#13636
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
`docker compose config --no-interpolate <service>` and
`docker compose config --variables <service>` load the raw model
without applying service filtering, so the full model is rendered
regardless of the services passed as arguments. Filtering will not
be supported on these paths, so emit a warning to make sure users
are no longer misled by silently ignored arguments.
Fixes#13614
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
runPreStart executes a service's pre_start hooks sequentially as
ephemeral containers that share the first non-running replica's
volumes via VolumesFrom and attach to the same networks. A non-zero
hook exit gates service start.
per_replica: false is the only currently supported mode; per_replica:
true is rejected up front. The donor replica is the lowest-numbered
one so the choice is deterministic. ContainerWait uses
WaitConditionNextExit, and the wait loop deterministically handles
the daemon's clean-close (nil on Error + exit code on Result) and
transport-error races to avoid spurious hook failures.
The log stream is opened before ContainerStart to avoid racing
AutoRemove on fast-exiting hooks, and runs under a derived context
so a daemon that keeps the connection open cannot deadlock the call.
Hook containers carry project/service/version labels; the two
cleanup paths force-remove the never-started container explicitly
and warn when removal fails.
pre_start runs once per service when no replica is already running
(initial up, force-recreate or spec change), and is skipped on
scale-up so additional replicas don't re-trigger the hooks.
Coverage: 11 unit tests (including scheduler-race stress) with
goroutine-leak verification via goleak, plus 10 E2E tests (success
path, hook failure gating start, build-image inheritance, idempotent
re-up, spec change, force-recreate, mid-sequence failure, ordering,
scale-up, scaled service).
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
The reconciler hashed the raw service config while the executor
hashed the form with network_mode/ipc/pid/volumes_from references
resolved to container IDs. Persisted hash and recomputed hash
never matched, so dependents were recreated on every `up`.
Resolve references against observed containers before hashing,
and cascade recreation to namespace-sharing dependents when a
parent is replaced — otherwise the dependent would keep a stale
"container:<old_id>" reference. Also dedup stops in
planStopDependents via stoppedByPlan: with the cascade restored,
the dependent would otherwise receive two Stop nodes.
Fixes#13878.
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
emitRunningEvents iterated the full ObservedState.Containers map,
which is intentionally broader than the operation scope (it covers
DisabledServices for orphan classification). compose run --no-deps
SERVICE leaves project.Services empty and moves every other service
to DisabledServices, so their running containers were reported as
Running even though this command must not manage them.
Filter the iteration by project.Services, matching the reconciler
scope, and document the contract on the function.
Fixes#13882
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
When a service declares an env var without a value (e.g. `- KEY` or
`KEY:`), MappingWithEquals stores it as a nil *string. The previous
condition `existing != nil && ...` skipped the warning for this case,
allowing silent overwrites. Change to `existing == nil || ...` so the
warning fires for both nil (shell-inherit) and value-mismatch cases.
Add e2e tests for both list-style (`- KEY`) and map-style (`KEY:`)
YAML forms to lock in the behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Yohta Kimura <38206553+rajyan@users.noreply.github.com>
rawsetenv injects provider variables without the service-name prefix, so
a key can collide with a value already set on the dependent service,
whether declared by the user in environment or emitted by another
provider. Log a warning and overwrite on collision, document the
precedence and the non-deterministic ordering between concurrent
providers, and cover the user-environment override with an e2e test.
Signed-off-by: Yohta Kimura <38206553+rajyan@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Providers can now send rawsetenv messages to inject environment
variables into dependent services without the automatic service name
prefix. This enables use cases where applications require exact
variable names that cannot be altered.
Closes#13727
Signed-off-by: Yohta Kimura <38206553+rajyan@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The run parameter was always passed as false at the single call site
and the run==true branch was dead code. Remove it so unparam stops
flagging callers added by PR #13742.
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
This fixture was not a valid JWT; the first 2 elements decode, but the last
one is malformed;
echo 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9' | base64 -d
{"alg":"HS256","typ":"JWT"}⏎
echo 'eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ' | base64 -d
{"sub":"1234567890","name":"John Doe","iat":1516239022⏎
echo 'SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw' | base64 -d
I�J�IHNJ(]�O��lj~�:N�%_�u
,⏎
This causes problems if the JWT parser is strict and rejecting invalid
JWT's.
It was added in 55b5f233c2, and probably copied
from an example, like https://github.com/knottx/JWTCodable#example-jwt-token,
but the last 2 bytes were truncated.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`compose up --build` populates BuildOptions.Deps=true so the initial
startup also builds images for depends_on services. The watch rebuild
path reused the same BuildOptions pointer, only resetting Build.Services
to the watched service. Build.Deps stayed true, so s.build() switched
back to IncludeDependencies and rebuilt the upstream dependency too.
Fix it by working on a local copy of BuildOptions in rebuild() and
explicitly setting Deps=false. Using a local copy also removes the data
race on the shared pointer when concurrent file events fire.
Also fix a related leak in doBuildBake: the loop populating bake
configuration iterates over every service in the project (needed so
additional_contexts: service:xxx references can resolve), but it was
emitting the "Image X Building" progress event and tracking expected
images for services that were not part of serviceToBeBuild. Filter
those side-effects to the actual build set so the watch rebuild log
shows only the watched service.
Adds an e2e test reproducing the bug.
Fixes#13853
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
External reviewer noted that the alreadyStopped branch adds createNode
to removeDeps while the !alreadyStopped branch does not — semantically
correct but fragile, since it relies on the implicit invariant that
stopNode.DependsOn contains createNode in the !alreadyStopped path.
Spell out the invariant in a comment so a future maintainer who edits
the stop → create edge in the normal path knows they must also add
createNode unconditionally in the remove deps.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
Two coverage gaps surfaced by an external review:
- TestReconcileContainers_DependsOnChain: asserts that a service B with
depends_on: [A] produces a CreateContainer for B that depends on A's
last plan node (the serviceNodes mechanism in infrastructureDeps).
This was the only depends_on-via-plan-DAG behavior untested before.
- TestReconcileContainers_DependsOnScaleDown: companion test that
exercises the scale-down → dependent path specifically, verifying
that the previous commit's lastNode-on-scale-down fix actually wires
the dependency through.
- TestOperationTypeString: adds OpRunProvider to the table; all other
OperationType values were already covered.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
Three fixes surfaced by an external code review of the new reconciler:
1. Scale-down now propagates through serviceNodes. When a service is
scaled down (all containers in excess), reconcileService used to
continue without assigning lastNode, leaving r.serviceNodes[svc]
unset. Dependent services then declared no edge on the scale-down
ops and could start before the cleanup finished. Track the
RemoveContainer node as lastNode so depends_on chains pick it up.
2. mustRecreate errors are no longer silently ignored by sortContainers.
The comparator used `obsi, _ := r.mustRecreate(...)`, falling back
to false on any hashing error. Pre-compute obsolescence into a map
keyed by container ID before sorting and propagate the error to
reconcileService.
3. A container is no longer Stopped twice when its network and its
config both diverge. planRecreateNetwork already stops the affected
container as part of the disconnect/remove/recreate dance; the
subsequent planRecreateContainer (triggered via hasNetworkMismatch)
used to add another OpStopContainer against the now-stopped target.
Track stops in r.stoppedByPlan; planRecreateContainer reuses an
existing Stop node when present, and chains its Remove on both that
Stop and the replacement Create.
Two golden tests (TestReconcileNetworks_Diverged*) are updated to
reflect the new, dedupe'd plan shape (one Stop instead of two per
recreated container).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
Three improvements identified in the Principal Engineer pass but
deliberately deferred:
1. Test fidelity. Split executePlan into newPlanExecutor (constructs
the executor seeded from observed state) and (*planExecutor).run
(walks the DAG). Production callers go through executePlan
unchanged. TestExecutePlanRemoveContainerDropsFromCache now uses
newPlanExecutor + run, exercising the same errgroup, done-channel
and group-tracker wiring as production instead of a hand-rolled
loop over executeNode.
2. //nolint:unused chain. The three preserved helpers
(reconciler.prompt, planRecreateVolume, servicesUsingVolume) each
carried a separate "kept for future" comment. Consolidate the
rationale on the reconciler.prompt field doc and point the helper
nolint directives there, so a future cleanup is a single grep.
3. Concurrency test. Add TestExecutePlanConcurrentRemovesCacheCoherence
which builds N independent Stop→Remove chains in one plan; the
errgroup fans them out across goroutines that all hit
containersByService under the mutex. Passes under -race. Failure
would expose a missing or incorrect lock.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
Two related changes to the executor, plus the small cleanups they
attracted in review:
* Rename node consults a planner-set CreateNodeID instead of walking
ancestors. The old execRenameContainer searched node.DependsOn for a
CreateContainer result and fell back to a recursive walk through the
group chain. That worked only by convention; a future op with cross-
node data needs would have to rediscover or copy the pattern. Now the
rename op carries an explicit CreateNodeID int set at plan time and
the executor reads pctx[CreateNodeID].ContainerID directly. The
recursive findCreatedIDInChain is gone.
* Stop re-listing containers on every create. execCreateContainer used
to call getContainersByService(ctx, projectName) — a fresh
ContainerList per create — to resolve service references at execute
time. The executor now holds a live containersByService view seeded
from ObservedState (via observed.containersByService()) and grown as
OpCreateContainer nodes complete, so service references resolve from
memory. On OpRemoveContainer the removed container is dropped from
the view via slices.DeleteFunc, so a dependent's create that resolves
network_mode: service:x against the just-removed container cannot
pick up a stale ID (Containers.sorted() orders by canonical name and
would otherwise return the removed container).
* Defensive slices.Clone of op.Service.VolumesFrom in execCreateContainer.
resolveServiceReferences mutates VolumesFrom in place, and the
shallow struct copy of *op.Service still shares the backing array.
Single-execution-per-node makes it safe today, but the clone removes
the trap for any future parallel-execution mode.
* Operation gains a CreateNodeID int (not a *PlanNode pointer) to avoid
a structural cycle between Operation and PlanNode. OperationType
values are pinned to explicit integers so adding an op in the middle
cannot silently shift the others.
* execRenameContainer carries two checks so a missing CreateNodeID and
an empty produced ID are distinguishable in logs. Both are programmer
invariants (prefixed "internal:").
* containersByServiceFromObserved moved from a package-level helper to
a method on *ObservedState.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
ensureProjectVolumes already prompts the user when a volume's config hash
diverges from the compose file (create.go:1626) and recreates it on confirm.
The reconciler ran after ensureProjectVolumes and prompted again with the
exact same message — so a user who declined the first prompt was asked the
same question a second time.
Drop the prompt + recreate call from reconcileVolumes(). Recreation of
diverged volumes stays owned by ensureProjectVolumes; the reconciler only
plans the creation of missing volumes. If the user declined recreation,
the existing container's mounts still match the existing volume name and
hasVolumeMismatch correctly returns false, so containers are not falsely
flagged as obsolete.
Keep the supporting infrastructure available for future use, when
divergence detection migrates fully into the reconciler:
- reconciler.prompt field
- prompt parameter on reconcile()
- planRecreateVolume function (//nolint:unused)
- servicesUsingVolume function (//nolint:unused)
- noPrompt test helper
The reframed test (TestReconcileVolumes_DivergedIsIgnored) asserts the
new contract: a diverged volume produces no plan operations from the
reconciler.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
- plan.go: pin OperationType constants to explicit values so adding an op
in the middle doesn't shift the others.
- executor.go: remove the meaningless `var _ = getContainerProgressName`
line — same-package functions are always accessible.
- reconcile.go: fix the stale switch-default comment that contradicted
the case clause above it.
- reconcile.go: drop the local `serviceLabel` const that shadowed
`api.ServiceLabel` and use the shared constant.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
- Fix ContainerReplaceLabel detection: use op.Inherited != nil (not
op.Container) as signal for recreate in execCreateContainer
- Use observed network name (not desired) for DisconnectNetwork and
RemoveNetwork operations, in case the name changed
- Use observed volume name (not desired) for RemoveVolume operations
- Update reconciliation.md with 3 new lessons learned (7.8, 7.9, 7.10)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
- Remove the `convergence` struct and `newConvergence` constructor
- Extract `resolveServiceReferences` as a standalone function taking
`map[string]Containers` instead of a method on convergence
- Add `getContainersByService` helper on composeService
- Update run.go and executor.go to use the new standalone function
- Remove dead code: `getObservedState`, `setObservedState`
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
