fix: provider migration condemns the service's stale replicas

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>

pkg/compose/reconcile.go

@@ -720,6 +720,40 @@ func (r *reconciler) reconcileService(service types.ServiceConfig) error {
 	if service.Provider != nil {
 		serviceCopy := service
 		deps := r.infrastructureDeps(service)
+		// A service migrated from replicas to a provider leaves its old
+		// containers behind: not converged (a provider service has no
+		// replicas to converge) and not orphaned (the service is still in
+		// the model). They must go before the provider's relay stands in:
+		// the relay takes over the service's canonical container name and
+		// its network alias, so a leftover replica is both a name conflict
+		// at relay creation and a competing DNS entry serving stale
+		// traffic. The relay itself (RelayLabel) is the provider service's
+		// legitimate container and is left alone — converged by
+		// ensureServiceRelay, not by the plan.
+		for i := range r.observed.Containers[service.Name] {
+			oc := &r.observed.Containers[service.Name][i]
+			if isRelayContainer(oc.Summary) {
+				continue
+			}
+			resID := serviceReplicaID(service.Name, oc.Number)
+			stopNode := r.plan.addNode(Operation{
+				Type:       OpStopContainer,
+				ResourceID: resID,
+				Cause:      "service is now provider-backed",
+				Container:  &oc.Summary,
+				Timeout:    r.options.Timeout,
+			}, "")
+			removeNode := r.plan.addNode(Operation{
+				Type:       OpRemoveContainer,
+				ResourceID: resID,
+				Cause:      "service is now provider-backed",
+				Container:  &oc.Summary,
+			}, "", stopNode)
+			r.removedByPlan[resID] = true
+			// the provider run deploys the relay: the canonical name must
+			// be free by then
+			deps = append(deps, removeNode)
+		}
 		node := r.plan.addNode(Operation{
 			Type:       OpRunProvider,
 			ResourceID: "provider:" + service.Name,

pkg/compose/reconcile_test.go

@@ -183,6 +183,83 @@ func TestReconcileNetworks_Diverged(t *testing.T) {
 `)+"\n")
 }
 
+// A service migrated from regular replicas to a provider must have its old
+// containers condemned by the plan: they are neither converged (a provider
+// service has no replicas) nor orphaned (the service is still in the model),
+// yet they hold the canonical container name the relay takes over and the
+// service's network alias. The relay itself is the provider service's
+// legitimate container and must be left alone.
+func TestReconcileService_ProviderMigrationRemovesStaleReplicas(t *testing.T) {
+	db := types.ServiceConfig{Name: "db", Provider: &types.ServiceProviderConfig{Type: "test"}}
+	project := &types.Project{
+		Name:     "myproject",
+		Services: types.Services{"db": db},
+	}
+	stale := ObservedContainer{
+		ID: "old-db", Number: 1, State: container.StateRunning, ConfigHash: "prehash",
+		Summary: container.Summary{
+			ID: "old-db", State: container.StateRunning,
+			Labels: map[string]string{
+				api.ServiceLabel:         "db",
+				api.ContainerNumberLabel: "1",
+				api.ConfigHashLabel:      "prehash",
+			},
+		},
+	}
+	observed := &ObservedState{
+		ProjectName: "myproject",
+		Containers:  map[string][]ObservedContainer{"db": {stale}},
+		Networks:    map[string][]ObservedNetwork{},
+		Volumes:     map[string][]ObservedVolume{},
+	}
+
+	plan, err := reconcile(t.Context(), project, observed, defaultReconcileOptions(), noPrompt)
+	assert.NilError(t, err)
+
+	// the provider run deploys the relay under the canonical name: it must
+	// depend on the removal that frees it
+	assert.Equal(t, plan.String(), strings.TrimSpace(`
+[] -> #1 service:db:1, StopContainer, service is now provider-backed
+[1] -> #2 service:db:1, RemoveContainer, service is now provider-backed
+[2] -> #3 provider:db, RunProvider, provider service
+`)+"\n")
+}
+
+// An up-to-date relay container observed for a provider service is NOT the
+// plan's to touch: ensureServiceRelay converges it during the provider run.
+func TestReconcileService_ProviderRelayLeftAlone(t *testing.T) {
+	db := types.ServiceConfig{Name: "db", Provider: &types.ServiceProviderConfig{Type: "test"}}
+	project := &types.Project{
+		Name:     "myproject",
+		Services: types.Services{"db": db},
+	}
+	relay := ObservedContainer{
+		ID: "relay-db", Number: 1, State: container.StateRunning, ConfigHash: "relay-identity",
+		Summary: container.Summary{
+			ID: "relay-db", State: container.StateRunning,
+			Labels: map[string]string{
+				api.ServiceLabel:         "db",
+				api.ContainerNumberLabel: "1",
+				api.ConfigHashLabel:      "relay-identity",
+				api.RelayLabel:           "relay-identity",
+			},
+		},
+	}
+	observed := &ObservedState{
+		ProjectName: "myproject",
+		Containers:  map[string][]ObservedContainer{"db": {relay}},
+		Networks:    map[string][]ObservedNetwork{},
+		Volumes:     map[string][]ObservedVolume{},
+	}
+
+	plan, err := reconcile(t.Context(), project, observed, defaultReconcileOptions(), noPrompt)
+	assert.NilError(t, err)
+
+	assert.Equal(t, plan.String(), strings.TrimSpace(`
+[] -> #1 provider:db, RunProvider, provider service
+`)+"\n")
+}
+
 // TestReconcileNetworks_DivergedAlsoRecreatesChangedContainer verifies the
 // entangled case: when a container attached to a diverged network also has its
 // own config changed, it is both reconnected (by the network recreate) and

pkg/e2e/providers_test.go

@@ -107,6 +107,26 @@ func TestProviderRawSetEnvOverridesInheritedEnvMapForm(t *testing.T) {
 			OutputContains("overrides environment variable"))
 }
 
+func TestProviderMigration(t *testing.T) {
+	// Regression: migrating an already-deployed service to a provider left
+	// its replica standing — neither converged (provider services have no
+	// replicas) nor orphaned (the service is still in the model) — and the
+	// relay creation then failed with a name conflict on the canonical
+	// container name the replica still held.
+	relayImage := "compose-relay-e2e"
+	s := providerScenario(t, "migrating a deployed service to a provider must replace its replica with the relay")
+	s.CLI().RunCmd(t, "docker", "build", "-t", relayImage, "../../relay")
+	s.Env("PROVIDER_DEMO_ENDPOINT=1", "COMPOSE_RELAY_IMAGE="+relayImage)
+	s.Step("the service first runs as a regular container",
+		ComposeCmd("up", "-d"),
+		ServiceState("db", "running"))
+	s.FromFile("migrated.yaml")
+	s.Step("up after the migration replaces the replica with the relay, reachable at the compose-native address",
+		ComposeCmd("up"),
+		Recreated("db"),
+		OutputContains("test-1  | hello from provider"))
+}
+
 func TestProviderPublishEndpoint(t *testing.T) {
 	// The example provider stands up a real endpoint on the host and
 	// publishes it; compose deploys a relay under the service's name, so the

pkg/e2e/scenario.go

@@ -268,6 +268,25 @@ func (s *Scenario) FromRemote(source string, rootFlags ...string) *Scenario {
 	return s
 }
 
+// FromFile switches the scenario's compose file to another file of the
+// anchored project directory — for scenarios whose intent is a model
+// migration: steps before the switch ran the initial model, steps after it
+// run the migrated one, against the same project.
+func (s *Scenario) FromFile(name string) *Scenario {
+	s.t.Helper()
+	if s.file == "" {
+		s.t.Fatal("FromFile requires an anchored testdata directory")
+	}
+	file := filepath.Join(filepath.Dir(s.file), name)
+	if _, err := os.Stat(file); err != nil {
+		s.t.Fatalf("FromFile(%q): %v", name, err)
+	}
+	s.remote = ""
+	s.rootArgs = nil
+	s.file = file
+	return s
+}
+
 // Requires skips the scenario unless every requirement is met by the target
 // environment.
 func (s *Scenario) Requires(reqs ...Requirement) *Scenario {

pkg/e2e/testdata/TestProviderMigration/compose.yaml

@@ -0,0 +1,5 @@
+services:
+  db:
+    image: alpine
+    init: true
+    command: sleep infinity

pkg/e2e/testdata/TestProviderMigration/migrated.yaml

@@ -0,0 +1,13 @@
+services:
+  test:
+    image: alpine
+    command: wget -qO- -T 10 http://db
+    depends_on:
+      - db
+  db:
+    provider:
+      type: example-provider
+      options:
+        name: db
+        type: test1
+        size: 1