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