Keep provider MSP clients running across upgrades

Every upgrade.sh run recreates the control plane, and a recreated support
container is no longer attached to any client's isolated tenant network.
The health monitor then could not reach the clients, counted three
failures, and stopped each one, expecting the unless-stopped restart policy
to bring it back. Docker never restarts a container stopped through the
API, so every client workspace stayed down until someone started it by
hand. A Traefik recreate also left client routes returning 504.
Reproduced on a v6.4.1 provider bundle with two clients.

The monitor now reattaches Traefik and the control plane to each active
client's network once at startup and on every pass, and recovers an
unhealthy client with a single restart instead of a stop. With the fix on
the same install, a control-plane recreate rejoined both networks within a
second, a Traefik-only recreate had its routes back on the next pass, and
no client was stopped.

docs/release-control/v6/internal/subsystems/cloud-paid.md

@@ -3431,6 +3431,24 @@ Pulse-service-backed capabilities. `providerChained` retains its narrower
 meaning: a Pulse-signed licence is available to embed so release builds can
 verify the lease.
 
+### Provider MSP health monitor keeps clients running across upgrades
+
+The control plane's health monitor restarts an unhealthy client workspace
+with a single Docker restart, never a stop: Docker's `unless-stopped` policy
+does not bring back a container stopped through the API, so a stopped client
+stayed down until someone started it by hand. Before each health check, and
+once as soon as the control plane starts, the monitor also reattaches the
+provider support containers (Traefik and the control plane) to every active
+client's isolated tenant network. Recreating either one, which every
+`upgrade.sh` run does to the control plane, drops those attachments, cutting
+the client's Traefik route and, with the old stop behaviour, taking every
+client offline three minutes later. A client without an isolated network is
+skipped quietly. Regression coverage:
+`TestHealthMonitorReattachesSupportContainersAndRestartsInsteadOfStopping` in
+`internal/cloudcp/health_monitor_test.go` and
+`TestEnsureSupportContainersOnTenantNetworkSkipsMissingNetwork` in
+`internal/cloudcp/docker/manager_test.go`.
+
 ### Provider MSP status reads client health that any caller can observe
 
 `provider-msp status`, which `upgrade.sh` gates on before every provider

docs/release-control/v6/internal/subsystems/deployment-installability.md

@@ -2416,6 +2416,19 @@ artifact-selection behaviour.
 
 ## Current State
 
+### Provider MSP clients survive a support-container recreate
+
+Recreating the control plane or Traefik (every `upgrade.sh` run recreates the
+control plane) used to detach them from each client's isolated network. The
+health monitor then saw every client as failing and stopped it, and nothing
+restarted it, so an upgrade on a v6.4.1 install with clients took all of them
+offline. The monitor now reattaches the support containers on startup and on
+every pass, and restarts rather than stops an unhealthy client. Verified on
+2026-09-23 on a v6.4.1 provider bundle with two clients: a control-plane
+recreate rejoined both client networks within a second, a Traefik-only
+recreate had its client routes back on the next 60-second pass, and neither
+client was stopped.
+
 ### Provider MSP upgrades work once a client exists
 
 `deploy/provider-msp/upgrade.sh` and `run-install-proof.sh` run

internal/cloudcp/docker/manager.go

@@ -863,6 +863,39 @@ func (m *Manager) Stop(ctx context.Context, containerID string) error {
 	return err
 }
 
+// Restart stops and starts a container in one daemon call. The health
+// monitor relies on it to recover an unhealthy tenant: a container stopped
+// through the API is never brought back by the unless-stopped restart
+// policy, so stopping it left the client workspace down for good.
+func (m *Manager) Restart(ctx context.Context, containerID string) error {
+	timeout := 30
+	_, err := m.cli.ContainerRestart(ctx, containerID, client.ContainerRestartOptions{Timeout: &timeout})
+	return err
+}
+
+// EnsureSupportContainersOnTenantNetwork reattaches the provider support
+// containers (Traefik and the control plane) to a tenant's isolated network.
+// They join it when the tenant is created, but recreating either one, as an
+// upgrade or any compose change does, drops the attachment: Traefik then has
+// no route to the client and the control plane cannot reach it. A tenant
+// without an isolated network has nothing to reattach.
+func (m *Manager) EnsureSupportContainersOnTenantNetwork(ctx context.Context, tenantID string) error {
+	if m == nil || m.cli == nil || !m.cfg.IsolateTenantNetworks || strings.TrimSpace(tenantID) == "" {
+		return nil
+	}
+	networkName := m.tenantNetworkName(tenantID)
+	if networkName == "" {
+		return nil
+	}
+	if _, err := m.cli.NetworkInspect(ctx, networkName, client.NetworkInspectOptions{}); err != nil {
+		if errdefs.IsNotFound(err) {
+			return nil
+		}
+		return fmt.Errorf("inspect tenant network %q: %w", networkName, err)
+	}
+	return m.connectSupportContainersToTenantNetwork(ctx, networkName)
+}
+
 // Start starts a stopped tenant container.
 func (m *Manager) Start(ctx context.Context, containerID string) error {
 	_, err := m.cli.ContainerStart(ctx, containerID, client.ContainerStartOptions{})

internal/cloudcp/docker/manager_test.go

@@ -2,6 +2,8 @@ package docker
 
 import (
 	"context"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
@@ -603,3 +605,39 @@ func TestDockerRecordedHealthDecidesWhenTheImageDeclaresOne(t *testing.T) {
 		}
 	}
 }
+
+// A client created before per-client networks has no isolated network to
+// rejoin. Reconciling it must be a quiet no-op, not a warning every minute.
+func TestEnsureSupportContainersOnTenantNetworkSkipsMissingNetwork(t *testing.T) {
+	var mutations []string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Api-Version", "1.47")
+		if strings.HasSuffix(r.URL.Path, "/_ping") {
+			_, _ = w.Write([]byte("OK"))
+			return
+		}
+		if r.Method != http.MethodGet {
+			mutations = append(mutations, r.Method+" "+r.URL.Path)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusNotFound)
+		_, _ = w.Write([]byte(`{"message":"network not found"}`))
+	}))
+	t.Cleanup(srv.Close)
+	t.Setenv("DOCKER_HOST", "tcp://"+strings.TrimPrefix(srv.URL, "http://"))
+	t.Setenv("DOCKER_TLS_VERIFY", "")
+	t.Setenv("DOCKER_CERT_PATH", "")
+
+	mgr, err := NewManager(ManagerConfig{Image: "pulse:test", Network: "pulse-provider-msp", IsolateTenantNetworks: true})
+	if err != nil {
+		t.Fatalf("NewManager: %v", err)
+	}
+	t.Cleanup(func() { _ = mgr.Close() })
+
+	if err := mgr.EnsureSupportContainersOnTenantNetwork(context.Background(), "t-legacy"); err != nil {
+		t.Fatalf("EnsureSupportContainersOnTenantNetwork = %v, want nil for a client without an isolated network", err)
+	}
+	if len(mutations) != 0 {
+		t.Fatalf("reconcile changed the host for a missing network: %v", mutations)
+	}
+}

internal/cloudcp/health_monitor.go

@@ -49,6 +49,12 @@ func (m *Monitor) Run(ctx context.Context) {
 		Bool("restart_on_fail", m.cfg.RestartOnFail).
 		Msg("Health monitor started")
 
+	// Check once straight away. The control plane starts fresh after every
+	// upgrade, and the first pass is what puts the recreated support
+	// containers back on each client's network; waiting a full interval
+	// leaves every client route broken for that long.
+	m.checkAll(ctx)
+
 	ticker := time.NewTicker(m.cfg.Interval)
 	defer ticker.Stop()
 
@@ -64,6 +70,9 @@ func (m *Monitor) Run(ctx context.Context) {
 }
 
 func (m *Monitor) checkAll(ctx context.Context) {
+	if ctx.Err() != nil {
+		return
+	}
 	activeContainerIDs := make(map[string]struct{})
 	tenants, err := m.registry.ListByState(registry.TenantStateActive)
 	if err != nil {
@@ -80,6 +89,16 @@ func (m *Monitor) checkAll(ctx context.Context) {
 		}
 		activeContainerIDs[tenant.ContainerID] = struct{}{}
 
+		// Rejoin Traefik and the control plane to this client's network
+		// first. A recreated support container (every upgrade recreates the
+		// control plane) has lost it, which cuts the client's route and
+		// would make a healthy client look unreachable.
+		if err := m.docker.EnsureSupportContainersOnTenantNetwork(ctx, tenant.ID); err != nil {
+			log.Warn().Err(err).
+				Str("tenant_id", tenant.ID).
+				Msg("Health monitor: could not reattach support containers to tenant network")
+		}
+
 		healthy, err := m.docker.HealthCheck(ctx, tenant.ContainerID)
 		if err != nil {
 			log.Warn().Err(err).
@@ -117,14 +136,16 @@ func (m *Monitor) checkAll(ctx context.Context) {
 				Int("fail_threshold", m.cfg.FailThreshold).
 				Msg("Container unhealthy, attempting restart")
 
-			if err := m.docker.Stop(ctx, tenant.ContainerID); err != nil {
+			// Restart, never Stop: the unless-stopped policy does not bring
+			// back a container stopped through the API, so a stop took the
+			// client workspace down until someone started it by hand.
+			if err := m.docker.Restart(ctx, tenant.ContainerID); err != nil {
 				log.Error().
 					Err(err).
 					Str("tenant_id", tenant.ID).
 					Str("container_id", tenant.ContainerID).
-					Msg("Failed to stop unhealthy container")
+					Msg("Failed to restart unhealthy container")
 			}
-			// Docker restart policy (unless-stopped) will restart the container
 			m.failures[tenant.ContainerID] = 0
 		}
 	}

internal/cloudcp/health_monitor_test.go

@@ -0,0 +1,159 @@
+package cloudcp
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"regexp"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/cloudcp/docker"
+	"github.com/rcourtman/pulse-go-rewrite/internal/cloudcp/registry"
+)
+
+// fakeProviderDaemon answers the few Docker Engine API calls the health
+// monitor makes for one provider-hosted client workspace, and records every
+// mutating call so a test can assert what the monitor did to the host.
+type fakeProviderDaemon struct {
+	mu            sync.Mutex
+	tenantNetwork string
+	attached      map[string]bool // container IDs on the tenant network
+	support       map[string]string
+	health        string
+	calls         []string
+}
+
+var dockerAPIVersionPrefix = regexp.MustCompile(`^/v[0-9.]+`)
+
+func (d *fakeProviderDaemon) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	d.mu.Lock()
+	defer d.mu.Unlock()
+
+	path := dockerAPIVersionPrefix.ReplaceAllString(r.URL.Path, "")
+	w.Header().Set("Api-Version", "1.47")
+	w.Header().Set("Content-Type", "application/json")
+	switch {
+	case path == "/_ping":
+		_, _ = w.Write([]byte("OK"))
+	case r.Method == http.MethodGet && path == "/networks/"+d.tenantNetwork:
+		containers := map[string]any{}
+		for id := range d.attached {
+			containers[id] = map[string]any{"Name": id}
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"Name": d.tenantNetwork, "Id": "net-1", "Containers": containers})
+	case r.Method == http.MethodPost && path == "/networks/"+d.tenantNetwork+"/connect":
+		var body struct{ Container string }
+		_ = json.NewDecoder(r.Body).Decode(&body)
+		d.attached[body.Container] = true
+		d.calls = append(d.calls, "connect "+body.Container)
+		w.WriteHeader(http.StatusOK)
+	case r.Method == http.MethodGet && path == "/containers/json":
+		var items []map[string]any
+		for label, id := range d.support {
+			if strings.Contains(r.URL.Query().Get("filters"), label) {
+				items = append(items, map[string]any{"Id": id, "State": "running"})
+			}
+		}
+		_ = json.NewEncoder(w).Encode(items)
+	case r.Method == http.MethodGet && strings.HasPrefix(path, "/containers/") && strings.HasSuffix(path, "/json"):
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"Id":     strings.TrimSuffix(strings.TrimPrefix(path, "/containers/"), "/json"),
+			"State":  map[string]any{"Status": "running", "Running": true, "Health": map[string]any{"Status": d.health}},
+			"Config": map[string]any{"Labels": map[string]string{}},
+		})
+	case r.Method == http.MethodPost && strings.HasPrefix(path, "/containers/"):
+		d.calls = append(d.calls, strings.TrimPrefix(path, "/containers/"))
+		w.WriteHeader(http.StatusNoContent)
+	default:
+		w.WriteHeader(http.StatusNotFound)
+		_ = json.NewEncoder(w).Encode(map[string]string{"message": "no such object: " + path})
+	}
+}
+
+func (d *fakeProviderDaemon) recorded() []string {
+	d.mu.Lock()
+	defer d.mu.Unlock()
+	return append([]string(nil), d.calls...)
+}
+
+// An upgrade recreates the control plane (and often Traefik), and the new
+// containers are not on any client's isolated network. The monitor used to
+// read that as a failing client and, three minutes later, Stop it; Docker's
+// unless-stopped policy never restarts a container stopped through the API,
+// so every client workspace stayed down. The monitor must put the support
+// containers back on the network, and when a client really is unhealthy it
+// must restart it rather than stop it.
+func TestHealthMonitorReattachesSupportContainersAndRestartsInsteadOfStopping(t *testing.T) {
+	daemon := &fakeProviderDaemon{
+		tenantNetwork: "pulse-provider-msp-tenant-t-acme",
+		attached:      map[string]bool{"tenant-container": true},
+		support: map[string]string{
+			"pulse.provider-msp.role=traefik":       "traefik-recreated",
+			"pulse.provider-msp.role=control-plane": "control-plane-recreated",
+		},
+		health: "healthy",
+	}
+	srv := httptest.NewServer(daemon)
+	t.Cleanup(srv.Close)
+	t.Setenv("DOCKER_HOST", "tcp://"+strings.TrimPrefix(srv.URL, "http://"))
+	t.Setenv("DOCKER_TLS_VERIFY", "")
+	t.Setenv("DOCKER_CERT_PATH", "")
+
+	mgr, err := docker.NewManager(docker.ManagerConfig{
+		Image:                 "pulse:test",
+		Network:               "pulse-provider-msp",
+		IsolateTenantNetworks: true,
+		TenantNetworkPrefix:   "pulse-provider-msp-tenant",
+		BaseDomain:            "msp.example.com",
+	})
+	if err != nil {
+		t.Fatalf("NewManager: %v", err)
+	}
+	t.Cleanup(func() { _ = mgr.Close() })
+
+	reg, err := registry.NewTenantRegistry(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewTenantRegistry: %v", err)
+	}
+	t.Cleanup(func() { _ = reg.Close() })
+	if err := reg.Create(&registry.Tenant{ID: "t-acme", AccountID: "a-1", State: registry.TenantStateActive, ContainerID: "tenant-container"}); err != nil {
+		t.Fatalf("Create tenant: %v", err)
+	}
+
+	monitor := NewMonitor(reg, mgr, MonitorConfig{RestartOnFail: true, FailThreshold: 3})
+	monitor.checkAll(context.Background())
+
+	got := strings.Join(daemon.recorded(), ", ")
+	for _, want := range []string{"connect traefik-recreated", "connect control-plane-recreated"} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("after one pass the daemon saw %q, want %q", got, want)
+		}
+	}
+	tenant, err := reg.Get("t-acme")
+	if err != nil || tenant == nil || !tenant.HealthCheckOK {
+		t.Fatalf("tenant after healthy pass = %+v, err %v; want HealthCheckOK", tenant, err)
+	}
+
+	// Already attached: later passes change nothing on the network.
+	monitor.checkAll(context.Background())
+	if again := daemon.recorded(); len(again) != 2 {
+		t.Fatalf("second pass reconnected attached containers: %v", again)
+	}
+
+	daemon.mu.Lock()
+	daemon.health = "unhealthy"
+	daemon.mu.Unlock()
+	for range 3 {
+		monitor.checkAll(context.Background())
+	}
+	got = strings.Join(daemon.recorded(), ", ")
+	if !strings.Contains(got, "tenant-container/restart") {
+		t.Fatalf("unhealthy client was not restarted; daemon saw %q", got)
+	}
+	if strings.Contains(got, "tenant-container/stop") {
+		t.Fatalf("unhealthy client was stopped, which unless-stopped never undoes; daemon saw %q", got)
+	}
+}