fix: provider migration condemns the service's stale replicas - #14264
Merged
Merged
Conversation
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
ndeloof
force-pushed
the
provider-migration-cleanup
branch
from
September 25, 2026 13:37
5021fbc to
85ce614
Compare
glours
previously approved these changes
Sep 25, 2026
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
A service migrated from regular replicas to a provider left its old containers standing: not converged (a provider service has no replicas to converge), not orphaned (the service is still in the model). The first up after the migration then failed creating the relay, because the leftover replica still held the service's canonical container name the relay takes over. And even without the name conflict (replicas 2..n of a scaled service), stale replicas kept the service's network alias, competing with the relay on DNS and serving outdated traffic. The reconciler now plans a stop+remove for every observed non-relay container of a provider service, and the RunProvider node depends on those removals: the canonical name is free by the time ensureServiceRelay creates the relay. The relay itself (RelayLabel) is the provider service's legitimate container and is left alone — converged by ensureServiceRelay, not by the plan. The e2e scenario migrates a deployed service to a provider and locks the replacement (Recreated, relay reachable at the compose-native address); the Scenario DSL gains FromFile — a local mirror of FromRemote — to switch the model between steps. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ndeloof
force-pushed
the
provider-migration-cleanup
branch
from
September 25, 2026 13:48
85ce614 to
4d9d8e9
Compare
glours
approved these changes
Sep 25, 2026
ndeloof
enabled auto-merge (rebase)
September 25, 2026 13:53
docker-agent
left a comment
docker-agent
left a comment
Contributor
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does, in one sentence: migrating an already-deployed service to a provider now works — the first
upafter the migration replaces the service's old containers with the provider's relay instead of failing on a name conflict.Context
Take a deployed compose application and migrate one of its services to a provider (
provider: type: …). On the nextup, the old service container is in a blind spot: the reconciler deliberately leaves provider services' containers alone (that rule protects the relay), and the container is not an orphan either — its service is still in the model. Nobody removes it. The provider then runs, publishes its endpoints, and compose fails deploying the relay:The relay takes over the service's canonical container name by design — so label-driven commands keep treating it as the service — and the leftover replica still holds that name. The conflict is only the visible half of the problem: replicas 2..n of a scaled service would not collide by name, but they keep the service's network alias, competing with the relay on DNS and serving outdated traffic.
What the PR brings
The reconciler treats the migration as what it is: a replacement. For a provider-backed service, every observed container that is not the relay is condemned by the plan — stop, then remove — and the
RunProvidernode depends on those removals, so the canonical name is guaranteed free by the time the relay is created. The relay itself (identified by itsRelayLabel) stays untouched: its convergence belongs toensureServiceRelay, unchanged.Guardrails:
RunProvideralone); a standing up-to-date relay is left alone (locked by a dedicated unit test).up— asserting the container is replaced (Recreated) and the relay serves at the compose-native address. Without the fix, that scenario fails exactly on the reported conflict.FromFile, a local mirror ofFromRemote, so a scenario can switch its compose model between steps — the natural shape for migration tests.