From 515e88e4c2651924e567df1cf9d9b464a3584b94 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Thu, 24 Sep 2026 09:45:55 +0100 Subject: [PATCH] Keep the provider MSP install proof out of client slots Setup's summary tells every new provider to run the install proof before their first real client. Its temporary workspaces are created on the provider's own account, so they count against the client limit. On a two-client evaluation that already had a client, the proof created its first workspace, was refused the second, and returned no report, so install-proof's cleanup had nothing to remove and "Provider MSP Proof 01" stayed behind in one of the evaluation's two slots. Found on the walkthrough lab. The proof now checks for room before creating anything and says why it cannot run, and a proof that fails part-way returns the IDs it created so install-proof removes them. On the lab, with 3 of 3 slots in use, install-proof now refuses up front and the client count is unchanged. --- .../provider_msp_install_proof.go | 7 +++ .../provider_msp_install_proof_test.go | 51 +++++++++++++++++++ cmd/pulse-control-plane/provider_msp_proof.go | 47 +++++++++++++++-- .../provider_msp_proof_test.go | 47 +++++++++++++++++ .../subsystems/deployment-installability.md | 19 +++++++ 5 files changed, 168 insertions(+), 3 deletions(-) diff --git a/cmd/pulse-control-plane/provider_msp_install_proof.go b/cmd/pulse-control-plane/provider_msp_install_proof.go index 69a3395540..046c472491 100644 --- a/cmd/pulse-control-plane/provider_msp_install_proof.go +++ b/cmd/pulse-control-plane/provider_msp_install_proof.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "path/filepath" "strings" @@ -265,6 +266,12 @@ func runProviderMSPInstallProofWithDependencies(ctx context.Context, cfg *cloudc proofTenantIDs = providerMSPInstallProofTenantIDs(proof.Workspaces) } if err != nil { + // A proof that fails part-way returns no report, but it may already + // have created workspaces; remove those too. + var partial *providerMSPProofPartialError + if errors.As(err, &partial) && len(proofTenantIDs) == 0 { + proofTenantIDs = partial.CreatedTenantIDs + } addFailure("workspace proof: %v", err) return failAfterProof(err) } diff --git a/cmd/pulse-control-plane/provider_msp_install_proof_test.go b/cmd/pulse-control-plane/provider_msp_install_proof_test.go index 9e1fc07b75..a5a9e95025 100644 --- a/cmd/pulse-control-plane/provider_msp_install_proof_test.go +++ b/cmd/pulse-control-plane/provider_msp_install_proof_test.go @@ -407,3 +407,54 @@ func (f *fakeProviderMSPInstallProofRuntime) CleanupProviderMSPProofTenants(_ co func (f *fakeProviderMSPInstallProofRuntime) Close() { f.closed = true } + +// A proof that fails part-way returns no report, so the workspace list the +// install proof used to clean up from was empty and the workspaces it had +// already created stayed behind in the provider's client slots. +func TestProviderMSPInstallProofCleansUpWorkspacesFromAPartialProof(t *testing.T) { + cfg := testProviderMSPProofConfig(t) + fakeRuntime := &fakeProviderMSPInstallProofRuntime{ + proofErr: &providerMSPProofPartialError{ + err: errors.New("create proof workspace 2: workspace limit reached"), + CreatedTenantIDs: []string{"ws-proof-01"}, + }, + } + + report, err := runProviderMSPInstallProofWithDependencies(context.Background(), cfg, providerMSPInstallProofOptions{ + AccountName: "Example MSP", + OwnerEmail: "owner@example.com", + WorkspacePrefix: "Provider MSP Proof", + WorkspaceCount: 2, + InstallType: "pve", + TargetPath: "/settings/infrastructure?add=linux-host", + BackupOutput: "/tmp/provider-msp-install-proof.tar.gz", + RestoreTargetDataDir: "/tmp/provider-msp-restore-drill", + Cleanup: true, + }, providerMSPInstallProofDependencies{ + Bootstrap: func(context.Context, *cloudcp.CPConfig, cloudcp.ProviderMSPBootstrapOptions) (*cloudcp.ProviderMSPBootstrapResult, error) { + return healthyProviderMSPInstallProofBootstrap(), nil + }, + RunPreflight: func(context.Context, *cloudcp.CPConfig, providerMSPPreflightOptions) (*providerMSPPreflightReport, error) { + return healthyProviderMSPStatusPreflightReport(), nil + }, + RunStatus: func(context.Context, *cloudcp.CPConfig, providerMSPStatusOptions) (*providerMSPStatusReport, error) { + return &providerMSPStatusReport{OK: true}, nil + }, + NewProofRuntime: func(*cloudcp.CPConfig) (providerMSPInstallProofRuntime, error) { + return fakeRuntime, nil + }, + CreateBackup: func(context.Context, *cloudcp.CPConfig, string) (*cloudcp.ProviderMSPBackupCreateResult, error) { + t.Fatal("backup should not run after a failed proof") + return nil, nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "workspace limit reached") { + t.Fatalf("install proof error = %v, want the proof failure", err) + } + if report == nil || report.OK || !report.CleanupOK { + t.Fatalf("report = %#v, want failed with cleanup done", report) + } + if !reflect.DeepEqual(fakeRuntime.cleanupTenantIDs, []string{"ws-proof-01"}) { + t.Fatalf("cleanup tenant ids = %#v, want the workspace the partial proof created", fakeRuntime.cleanupTenantIDs) + } +} diff --git a/cmd/pulse-control-plane/provider_msp_proof.go b/cmd/pulse-control-plane/provider_msp_proof.go index 8c4982645d..3a02cc09c1 100644 --- a/cmd/pulse-control-plane/provider_msp_proof.go +++ b/cmd/pulse-control-plane/provider_msp_proof.go @@ -260,7 +260,26 @@ func (rt *providerMSPProofRuntime) runProviderMSPProof(ctx context.Context, opts return nil, fmt.Errorf("bootstrap provider MSP account: %w", err) } + // The proof creates its workspaces on the provider's own account, so they + // count against the same client limit real clients do. Check for room + // before creating anything: on a two-client evaluation that already has a + // client, the second proof workspace would otherwise be refused after the + // first was created, leaving it behind in one of the evaluation's slots. + if bootstrap.WorkspaceLimit > 0 { + inUse, err := rt.registry.CountActiveByAccountID(bootstrap.AccountID) + if err != nil { + return nil, fmt.Errorf("count client workspaces: %w", err) + } + if free := bootstrap.WorkspaceLimit - inUse; free < opts.WorkspaceCount { + return nil, fmt.Errorf("the proof creates %d temporary client workspaces but this account has %d free (%d of %d in use); run it on a fresh platform, before adding clients", + opts.WorkspaceCount, max(free, 0), inUse, bootstrap.WorkspaceLimit) + } + } + createdTenants := make([]*registry.Tenant, 0, opts.WorkspaceCount) + partial := func(err error) error { + return &providerMSPProofPartialError{err: err, CreatedTenantIDs: providerMSPProofTenantIDs(createdTenants)} + } if opts.Cleanup { defer func() { _ = rt.cleanupProviderMSPProofTenants(context.Background(), createdTenants) @@ -292,13 +311,13 @@ func (rt *providerMSPProofRuntime) runProviderMSPProof(ctx context.Context, opts displayName := fmt.Sprintf("%s %02d", opts.WorkspacePrefix, idx+1) tenant, err := rt.createProviderMSPProofWorkspace(ctx, bootstrap.AccountID, displayName, bootstrap.OwnerEmail) if err != nil { - return nil, fmt.Errorf("create proof workspace %d: %w", idx+1, err) + return nil, partial(fmt.Errorf("create proof workspace %d: %w", idx+1, err)) } createdTenants = append(createdTenants, tenant) workspace, err := rt.proveProviderMSPWorkspace(ctx, tenant, bootstrap.OwnerUserID, opts.InstallType, opts.TargetPath) if err != nil { - return nil, fmt.Errorf("prove workspace %s: %w", tenant.ID, err) + return nil, partial(fmt.Errorf("prove workspace %s: %w", tenant.ID, err)) } if workspace.ContainerID == "" { report.RuntimeContainerVerified = false @@ -327,13 +346,35 @@ func (rt *providerMSPProofRuntime) runProviderMSPProof(ctx context.Context, opts boundaryOK, err := rt.verifyProviderMSPInstallTokenIsolation(report.Workspaces) if err != nil { - return nil, err + return nil, partial(err) } report.InstallTokenBoundaryOK = boundaryOK report.WorkspaceCount = len(report.Workspaces) return report, nil } +// providerMSPProofPartialError is a proof that failed after creating some of +// its workspaces. It carries their IDs so the caller can remove them: without +// that, a failure part-way through left proof workspaces occupying client +// slots, because no report (and so no workspace list) comes back on error. +type providerMSPProofPartialError struct { + err error + CreatedTenantIDs []string +} + +func (e *providerMSPProofPartialError) Error() string { return e.err.Error() } +func (e *providerMSPProofPartialError) Unwrap() error { return e.err } + +func providerMSPProofTenantIDs(tenants []*registry.Tenant) []string { + ids := make([]string, 0, len(tenants)) + for _, tenant := range tenants { + if tenant != nil && strings.TrimSpace(tenant.ID) != "" { + ids = append(ids, tenant.ID) + } + } + return ids +} + func normalizeProviderMSPProofOptions(opts providerMSPProofOptions) (providerMSPProofOptions, error) { opts.AccountName = strings.TrimSpace(opts.AccountName) opts.OwnerEmail = strings.ToLower(strings.TrimSpace(opts.OwnerEmail)) diff --git a/cmd/pulse-control-plane/provider_msp_proof_test.go b/cmd/pulse-control-plane/provider_msp_proof_test.go index d81786bb45..e5192fd2e1 100644 --- a/cmd/pulse-control-plane/provider_msp_proof_test.go +++ b/cmd/pulse-control-plane/provider_msp_proof_test.go @@ -6,12 +6,14 @@ import ( "crypto/rand" "encoding/base64" "encoding/json" + "fmt" "os" "strings" "testing" "time" "github.com/rcourtman/pulse-go-rewrite/internal/cloudcp" + "github.com/rcourtman/pulse-go-rewrite/internal/cloudcp/registry" pkglicensing "github.com/rcourtman/pulse-go-rewrite/pkg/licensing" ) @@ -430,3 +432,48 @@ func writeProviderMSPProofLicenseForTest(t *testing.T, licenseID, email, planVer } return path } + +// The proof's workspaces count against the provider's own client limit. When +// there is no room for all of them it must refuse before creating any: on a +// two-client evaluation with a client already added, it used to create the +// first proof workspace, get refused on the second, and leave the first +// occupying a slot. +func TestProviderMSPProofRefusesWithoutFreeSlotsAndCreatesNothing(t *testing.T) { + t.Setenv("DOCKER_HOST", "unix:///tmp/pulse-provider-msp-proof-missing-docker.sock") + t.Setenv("DOCKER_TLS_VERIFY", "") + t.Setenv("DOCKER_CERT_PATH", "") + + cfg := testProviderMSPProofConfig(t) + rt, err := newProviderMSPProofRuntimeFromConfig(cfg) + if err != nil { + t.Fatalf("newProviderMSPProofRuntimeFromConfig: %v", err) + } + defer rt.close() + + bootstrap, err := cloudcp.BootstrapProviderMSP(context.Background(), cfg, cloudcp.ProviderMSPBootstrapOptions{ + AccountName: "Acme Provider", + OwnerEmail: "owner@example.com", + }) + if err != nil { + t.Fatalf("BootstrapProviderMSP: %v", err) + } + if err := rt.registry.Create(®istry.Tenant{ID: "t-realclient", AccountID: bootstrap.AccountID, DisplayName: "Real Client", State: registry.TenantStateActive}); err != nil { + t.Fatalf("create existing client: %v", err) + } + + _, err = rt.runProviderMSPProof(context.Background(), providerMSPProofOptions{ + AccountName: "Acme Provider", + OwnerEmail: "owner@example.com", + WorkspacePrefix: "Provider MSP Proof", + WorkspaceCount: bootstrap.WorkspaceLimit, + InstallType: "pve", + TargetPath: "/settings/infrastructure?add=linux-host", + }) + want := fmt.Sprintf("has %d free (1 of %d in use)", bootstrap.WorkspaceLimit-1, bootstrap.WorkspaceLimit) + if err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("runProviderMSPProof error = %v, want refusal naming %q", err, want) + } + if count, err := rt.registry.CountActiveByAccountID(bootstrap.AccountID); err != nil || count != 1 { + t.Fatalf("active workspaces after refusal = %d, %v; want only the existing client", count, err) + } +} diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 6bc7bb2a9a..d4159a18ef 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -2423,6 +2423,25 @@ artifact-selection behaviour. ## Current State +### Provider MSP install proof never strands a client slot + +`run-install-proof.sh`, which setup's summary recommends before the first real +client, creates its temporary proof workspaces on the provider's own account, +so they count against the same client limit. It now checks for room before +creating any and refuses with the counts ("the proof creates 2 temporary client +workspaces but this account has 0 free (3 of 3 in use)"). Previously, on a +two-client evaluation that already had a client, it created the first proof +workspace, was refused the second, and returned no report, so its cleanup had +no workspace list and the first proof workspace stayed behind in one of the +evaluation's two slots. A proof that fails part-way now returns the IDs it +created, and `install-proof` removes them. Verified on 2026-09-24 against the +walkthrough lab: refusal before any workspace was created, client count +unchanged. Regression coverage: +`TestProviderMSPProofRefusesWithoutFreeSlotsAndCreatesNothing` in +`cmd/pulse-control-plane/provider_msp_proof_test.go` and +`TestProviderMSPInstallProofCleansUpWorkspacesFromAPartialProof` in +`cmd/pulse-control-plane/provider_msp_install_proof_test.go`. + ### Provider MSP setup points buyers at the portal `deploy/provider-msp/setup.sh` no longer tells an evaluating provider to