fix(provider): clean up stale relay and guard start/restart against it

A provider that stops publishing endpoints on a later `up` left its
previously created relay running: deployRelay was only evaluated when
endpoints were non-empty, so a zero-endpoint up fell through with no
cleanup. The generic reconciler is no help either — it returns early
for any service.Provider != nil, before the recreate logic that would
otherwise catch this. runPlugin now removes any existing relay when a
provider publishes no endpoint on an up, whether it never did or
stopped doing so since a previous run.

Separately, startServiceContainer and restartContainer had no
relay-awareness at all, unlike exec/cp which already refuse to target
one via checkRelayTarget. A relay is a shell-less, FROM-scratch binary
standing in for the service on the network: it has no filesystem or
process for secrets/configs injection or PreStart/PostStart/PreStop
hooks to act on. Nothing in the compose-spec schema forbids declaring
secrets, configs or hooks on a provider: service, so this was reachable
in practice, not just in theory. Both now skip those steps for a relay
container while still starting/restarting the container itself
normally.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Signed-off-by: Nicolas De Loof <[email protected]>

pkg/compose/plugins.go

@@ -131,6 +131,14 @@ func (s *composeService) runPlugin(ctx context.Context, project *types.Project,
 	if deployRelay {
 		return s.ensureServiceRelay(ctx, project, service, variables.endpoints, networkKeys)
 	}
+	if command == "up" {
+		// The provider published no endpoint on this run — whether it never
+		// did, or stopped doing so since a previous up deployed a relay for
+		// it. Either way there is nothing to route to, so any relay left
+		// over from an earlier run must go: routing to whatever upstream it
+		// still holds would be silently wrong instead of just absent.
+		return s.removeServiceRelay(ctx, project.Name, service.Name)
+	}
 	return nil
 }
 

pkg/compose/relay.go

@@ -257,6 +257,27 @@ func (s *composeService) waitRelayRemoved(ctx context.Context, projectName, serv
 	}
 }
 
+// removeServiceRelay removes a service's relay container, if any. Called
+// whenever an up finds the provider publishing no endpoint — whether it
+// never did, or a relay from an earlier up is now stale — so a relay that
+// still routes to an upstream the provider no longer serves doesn't linger.
+func (s *composeService) removeServiceRelay(ctx context.Context, projectName, serviceName string) error {
+	existing, err := s.findRelayContainer(ctx, projectName, serviceName)
+	if err != nil {
+		return err
+	}
+	if existing == nil {
+		return nil
+	}
+	eventID := "Relay " + getCanonicalContainerName(*existing)
+	s.events.On(removingEvent(eventID))
+	if _, err := s.apiClient().ContainerRemove(ctx, existing.ID, client.ContainerRemoveOptions{Force: true}); err != nil {
+		return fmt.Errorf("remove stale relay for service %s: %w", serviceName, err)
+	}
+	s.events.On(removedEvent(eventID))
+	return nil
+}
+
 // findRelayContainer returns the service's relay container, if any.
 func (s *composeService) findRelayContainer(ctx context.Context, projectName, serviceName string) (*container.Summary, error) {
 	f := projectFilter(projectName)
@@ -372,11 +393,20 @@ func (s *composeService) pullRelayImage(ctx context.Context) error {
 	return nil
 }
 
+// isRelayContainer reports whether a container is a service's network relay
+// rather than a regular instance of it: a static scratch-based binary with
+// no shell or service process, so anything expecting one to be there —
+// secrets/configs injection, lifecycle hooks, process-level commands — must
+// skip it.
+func isRelayContainer(ctr container.Summary) bool {
+	return ctr.Labels[api.RelayLabel] != ""
+}
+
 // checkRelayTarget refuses process-level operations on a relay container: it
 // stands in for the provider's resource on the network, but there is no
 // service process in it to act on.
 func checkRelayTarget(target container.Summary, serviceName, operation string) error {
-	if target.Labels[api.RelayLabel] == "" {
+	if !isRelayContainer(target) {
 		return nil
 	}
 	return fmt.Errorf("service %q is managed by a provider: its container is a network relay and does not support %s", serviceName, operation)

pkg/compose/relay_test.go

@@ -254,3 +254,38 @@ func TestCheckRelayTarget(t *testing.T) {
 	regular := container.Summary{Labels: map[string]string{api.ServiceLabel: "db"}}
 	assert.NilError(t, checkRelayTarget(regular, "db", "exec"))
 }
+
+// A provider publishing no endpoint on this up -- whether it never did, or a
+// relay from an earlier up is now stale -- must not leave a relay routing to
+// an upstream the provider no longer serves.
+func TestRemoveServiceRelayRemovesExisting(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	apiMock, cli := prepareMocks(mockCtrl)
+	tested, err := NewComposeService(cli)
+	assert.NilError(t, err)
+	svc := tested.(*composeService)
+
+	apiMock.EXPECT().ContainerList(gomock.Any(), gomock.Any()).Return(client.ContainerListResult{
+		Items: []container.Summary{{ID: "relay-1", Names: []string{"/p-db-1"}}},
+	}, nil)
+	apiMock.EXPECT().ContainerRemove(gomock.Any(), "relay-1", client.ContainerRemoveOptions{Force: true}).
+		Return(client.ContainerRemoveResult{}, nil)
+
+	assert.NilError(t, svc.removeServiceRelay(t.Context(), "p", "db"))
+}
+
+// No relay ever existed for the service: nothing to do, and nothing calls
+// ContainerRemove (mockCtrl.Finish would fail an unexpected call anyway).
+func TestRemoveServiceRelayNoopWhenNoneExists(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	apiMock, cli := prepareMocks(mockCtrl)
+	tested, err := NewComposeService(cli)
+	assert.NilError(t, err)
+	svc := tested.(*composeService)
+
+	apiMock.EXPECT().ContainerList(gomock.Any(), gomock.Any()).Return(client.ContainerListResult{}, nil)
+
+	assert.NilError(t, svc.removeServiceRelay(t.Context(), "p", "db"))
+}

pkg/compose/restart.go

@@ -105,12 +105,16 @@ func (s *composeService) prepareRestartProject(ctx context.Context, containers C
 }
 
 // restartContainer restarts a container, running its pre_stop and post_start
-// hooks around the restart
+// hooks around the restart — except for a relay, a shell-less scratch binary
+// standing in for the service: it has no process for a hook to act on.
 func (s *composeService) restartContainer(ctx context.Context, def types.ServiceConfig, ctr container.Summary, options api.RestartOptions) error {
-	for _, hook := range def.PreStop {
-		err := s.runHook(ctx, ctr, def, hook, nil)
-		if err != nil {
-			return err
+	relay := isRelayContainer(ctr)
+	if !relay {
+		for _, hook := range def.PreStop {
+			err := s.runHook(ctx, ctr, def, hook, nil)
+			if err != nil {
+				return err
+			}
 		}
 	}
 	eventName := getContainerProgressName(ctr)
@@ -122,10 +126,12 @@ func (s *composeService) restartContainer(ctx context.Context, def types.Service
 		return err
 	}
 	s.events.On(newEvent(eventName, api.Done, api.StatusStarted))
-	for _, hook := range def.PostStart {
-		err := s.runHook(ctx, ctr, def, hook, nil)
-		if err != nil {
-			return err
+	if !relay {
+		for _, hook := range def.PostStart {
+			err := s.runHook(ctx, ctr, def, hook, nil)
+			if err != nil {
+				return err
+			}
 		}
 	}
 	return nil

pkg/compose/restart_test.go

@@ -150,3 +150,25 @@ func TestRestartContainer_Order(t *testing.T) {
 		"Container prj-web-1: Started",
 	})
 }
+
+// A relay stands in for the service on the network but is a shell-less
+// scratch binary: pre_stop/post_start have no process inside it to act on.
+// No ExecCreate expectation is set: per newStartTestService, gomock fails
+// the test if either hook still runs.
+func TestRestartContainerSkipsHooksForRelay(t *testing.T) {
+	svc, apiClient, _ := newStartTestService(t)
+
+	service := types.ServiceConfig{
+		Name:      "db",
+		PreStop:   []types.ServiceHook{{Command: types.ShellCommand{"quiesce"}}},
+		PostStart: []types.ServiceHook{{Command: types.ShellCommand{"warmup"}}},
+	}
+	relay := serviceContainer("db", 1, container.StateRunning)
+	relay.Labels[api.RelayLabel] = "abc123"
+
+	apiClient.EXPECT().ContainerRestart(gomock.Any(), relay.ID, gomock.Any()).
+		Return(client.ContainerRestartResult{}, nil)
+
+	err := svc.restartContainer(t.Context(), service, relay, api.RestartOptions{})
+	assert.NilError(t, err)
+}

pkg/compose/service_containers.go

@@ -617,11 +617,19 @@ func (s *composeService) startService(ctx context.Context,
 }
 
 func (s *composeService) startServiceContainer(ctx context.Context, project *types.Project, service types.ServiceConfig, ctr container.Summary, listener api.ContainerEventListener) error {
-	if err := s.injectSecrets(ctx, project, service, ctr.ID); err != nil {
-		return err
-	}
-	if err := s.injectConfigs(ctx, project, service, ctr.ID); err != nil {
-		return err
+	// A relay stands in for the service on the network but is a static,
+	// shell-less scratch binary: secrets/configs injection and lifecycle
+	// hooks have no filesystem or process to act on inside it. The compose
+	// spec doesn't forbid declaring them on a provider: service, so this
+	// must be checked, not assumed unreachable.
+	relay := isRelayContainer(ctr)
+	if !relay {
+		if err := s.injectSecrets(ctx, project, service, ctr.ID); err != nil {
+			return err
+		}
+		if err := s.injectConfigs(ctx, project, service, ctr.ID); err != nil {
+			return err
+		}
 	}
 
 	eventName := getContainerProgressName(ctr)
@@ -633,9 +641,11 @@ func (s *composeService) startServiceContainer(ctx context.Context, project *typ
 		return err
 	}
 
-	for _, hook := range service.PostStart {
-		if err := s.runHook(ctx, ctr, service, hook, listener); err != nil {
-			return err
+	if !relay {
+		for _, hook := range service.PostStart {
+			if err := s.runHook(ctx, ctr, service, hook, listener); err != nil {
+				return err
+			}
 		}
 	}
 

pkg/compose/service_containers_test.go

@@ -776,6 +776,31 @@ func TestRuntimeAPIVersionRetriesOnTransientError(t *testing.T) {
 	assert.Equal(t, version, "1.44")
 }
 
+// A relay stands in for the service on the network but is a shell-less
+// scratch binary: post_start has no process inside it to act on. The
+// compose spec doesn't forbid declaring hooks on a provider: service, so
+// this must be guarded explicitly rather than assumed unreachable. No
+// ExecCreate expectation is set: per newStartTestService, gomock fails the
+// test if the hook still runs.
+func TestStartServiceContainerSkipsHooksForRelay(t *testing.T) {
+	svc, apiClient, _ := newStartTestService(t)
+
+	service := types.ServiceConfig{
+		Name: "db",
+		PostStart: []types.ServiceHook{
+			{Command: types.ShellCommand{"echo", "hi"}},
+		},
+	}
+	relay := serviceContainer("db", 1, container.StateCreated)
+	relay.Labels[api.RelayLabel] = "abc123"
+
+	apiClient.EXPECT().ContainerStart(gomock.Any(), relay.ID, gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+
+	err := svc.startServiceContainer(t.Context(), &types.Project{}, service, relay, nil)
+	assert.NilError(t, err)
+}
+
 // TestWaitDependencyDeadline locks the timeout semantics of the dependency
 // wait: an expired deadline surfaces as "timeout waiting for dependencies",
 // while a plain user cancellation is not a wait failure.