diff --git a/docs/release-control/v6/internal/subsystems/cloud-paid.md b/docs/release-control/v6/internal/subsystems/cloud-paid.md index 3a45d6124e..13158af2f1 100644 --- a/docs/release-control/v6/internal/subsystems/cloud-paid.md +++ b/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 diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index a6f98f12ea..9b664bd7d1 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/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 diff --git a/internal/cloudcp/docker/manager.go b/internal/cloudcp/docker/manager.go index d579159919..0889efe36f 100644 --- a/internal/cloudcp/docker/manager.go +++ b/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{}) diff --git a/internal/cloudcp/docker/manager_test.go b/internal/cloudcp/docker/manager_test.go index 40d4726c2b..8a6f7152e2 100644 --- a/internal/cloudcp/docker/manager_test.go +++ b/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) + } +} diff --git a/internal/cloudcp/health_monitor.go b/internal/cloudcp/health_monitor.go index 157db3da31..a249d1fdc2 100644 --- a/internal/cloudcp/health_monitor.go +++ b/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 } } diff --git a/internal/cloudcp/health_monitor_test.go b/internal/cloudcp/health_monitor_test.go new file mode 100644 index 0000000000..d413597035 --- /dev/null +++ b/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(®istry.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) + } +}