From f7a1f421483a37eb0afac663ccf0849993100b90 Mon Sep 17 00:00:00 2001 From: Apple FCU Fleet Date: Fri, 4 Sep 2026 20:02:18 -0400 Subject: [PATCH 1/2] feat(proxmox): support fixed lease IDs Implements replay-safe fixed Proxmox lease acquisition with durable VMID binding, exact identity validation, conflict detection, and terminal tombstones.\n\nCloses https://github.com/openclaw/crabbox/issues/1847 --- docs/features/identifiers.md | 38 +- docs/providers/proxmox.md | 18 +- internal/cli/claim.go | 3 + internal/cli/claim_test.go | 6 + internal/cli/proxmox.go | 20 +- internal/cli/proxmox_test.go | 37 ++ internal/providers/proxmox/backend.go | 431 ++++++++++++++++++ .../providers/proxmox/backend_doctor_test.go | 16 + internal/providers/proxmox/fixed_test.go | 306 +++++++++++++ 9 files changed, 857 insertions(+), 18 deletions(-) create mode 100644 internal/providers/proxmox/fixed_test.go diff --git a/docs/features/identifiers.md b/docs/features/identifiers.md index f32d60b7e..8f82ff2fe 100644 --- a/docs/features/identifiers.md +++ b/docs/features/identifiers.md @@ -57,15 +57,15 @@ private token and generation never appear in public lease records. Fixed-ID to own replay, and caller cancellation never releases them. Automation may instead supply the canonical ID with `warmup --lease-id`. For -direct AWS, direct Machine0, direct local-container, and managed coordinator -leases, that ID is an immutable create identity: an identical semantic replay -returns the same lease, while intent drift returns `lease_id_conflict`. External -providers also accept requested IDs when their protocol explicitly advertises -idempotent lease identity support. The coordinator durably stores a versioned -normalized request hash. Direct AWS durably stores the intent and current -resolved EC2 attempt in the normal lease claim before `RunInstances`, then uses -a deterministic regional/zonal client token. No path uses the slug to decide -replay ownership. +direct AWS, direct Machine0, direct local-container, direct Proxmox, and managed +coordinator leases, that ID is an immutable create identity: an identical +semantic replay returns the same lease, while intent drift returns +`lease_id_conflict`. External providers also accept requested IDs when their +protocol explicitly advertises idempotent lease identity support. The +coordinator durably stores a versioned normalized request hash. Direct AWS +durably stores the intent and current resolved EC2 attempt in the normal lease +claim before `RunInstances`, then uses a deterministic regional/zonal client +token. No path uses the slug to decide replay ownership. Direct Machine0 binds the intent to its deterministic VM name before creation; the durable attempt binds the first visible match to its Machine0 resource ID, @@ -81,6 +81,15 @@ missing acquired container fails closed instead of starting another container. Its fixed claims use the downgrade-safe `local-container-fixed-v1` marker, so older clients cannot mistake them for ordinary local-container claims. +Direct Proxmox selects a free VMID through the cluster allocator, then durably +binds that exact VMID, the normalized intent, source node, and cluster scope +before submitting the template clone with an explicit `newid`. Replay inspects +that VMID and requires matching lease labels, intent fingerprint, provider +scope, and native `vmgenid`; it never derives a VMID from the lease ID or adopts +by slug. Missing or ambiguous post-submit state retains the attempt and cannot +issue another clone. Its fixed claims use the downgrade-safe +`proxmox-fixed-v1` marker. + After the direct AWS launch attempt is durable, Crabbox never submits that attempt again. An ambiguous replay with no visible tagged instance fails closed; a later replay can adopt the one instance after inventory converges only when @@ -89,11 +98,12 @@ match the persisted attempt exactly. Fixed AWS claims use the downgrade-safe local discriminator `aws-fixed-v1`; current clients map it to runtime AWS, while older clients skip/refuse it. -Fixed IDs are single-use operation identities. Direct AWS, Machine0, and -local-container keep a compact terminal claim tombstone after successful -destroy release or exact missing-resource cleanup. Tombstones contain only the -ID, slug, provider scope, versioned intent hash, timestamps, and terminal -state; automatic provider cleanup never prunes them. +Fixed IDs are single-use operation identities. Direct AWS, Machine0, +local-container, and Proxmox keep a terminal claim tombstone after successful +destroy release or exact missing-resource cleanup. Proxmox retains the selected +VMID and, when observed, its native generation identity so release +reconciliation remains exact. Automatic provider cleanup never prunes fixed-ID +tombstones. There is no time-based reuse window. Explicitly deleting local Crabbox claim state forfeits this replay protection, so automation must instead mint a new operation ID. diff --git a/docs/providers/proxmox.md b/docs/providers/proxmox.md index cef87d300..2d3273b1f 100644 --- a/docs/providers/proxmox.md +++ b/docs/providers/proxmox.md @@ -16,7 +16,9 @@ then drives the normal SSH sync/run/release path. The provider is direct-only: it talks to the Proxmox API straight from the CLI. The Crabbox coordinator (broker) does not provision or broker Proxmox capacity, so brokered shared-team leases are not available here. Proxmox supports the -`ssh`, `crabbox-sync`, and `cleanup` features on `target=linux` only. +`ssh`, `crabbox-sync`, and `cleanup` features on `target=linux` only. Direct +Proxmox also supports caller-supplied fixed lease IDs with +`warmup --lease-id cbx_<12 lowercase hex>`. ## When to use @@ -378,6 +380,20 @@ only to discover candidates; they never authorize deletion by themselves. Failed acquisition cleanup removes the per-lease SSH key only after confirming the VM is absent across the cluster. +For an ordinary acquire, step 2 remains a per-create `/cluster/nextid` lookup. +For `warmup --lease-id`, Crabbox instead persists the normalized create intent, +selected VMID, source node, and cluster scope in the fixed lease claim before +submitting the clone, then passes that exact VMID as the clone API's `newid`. +An identical replay inspects the persisted VMID and adopts only the VM whose +lease labels, intent fingerprint, cluster scope, VMID, and native `vmgenid` +match. Slugs are never replay authority. A changed intent, copied labels, +different VMID or generation, duplicate match, or unresolved post-submit +attempt returns `lease_id_conflict` without issuing another clone. + +Successful fixed-ID release, including authoritative confirmation that the +selected VMID is absent, retains a terminal local tombstone. The fixed lease ID +is single-use and cannot allocate another VM after release. + ### Automatic cleanup ownership Cleanup requires exactly one local claim matching the provider, configured API diff --git a/internal/cli/claim.go b/internal/cli/claim.go index d1f0a83d3..449105e30 100644 --- a/internal/cli/claim.go +++ b/internal/cli/claim.go @@ -82,6 +82,7 @@ type FixedCreateIntent struct { const FixedAWSClaimProvider = "aws-fixed-v1" const FixedMachine0ClaimProvider = "machine0-fixed-v1" const FixedLocalContainerClaimProvider = "local-container-fixed-v1" +const FixedProxmoxClaimProvider = "proxmox-fixed-v1" const maxLocalClaimInventoryFileBytes int64 = 1 * 1024 * 1024 @@ -1180,6 +1181,8 @@ func canonicalClaimProvider(provider string) string { return "machine0" case FixedLocalContainerClaimProvider: return "local-container" + case FixedProxmoxClaimProvider: + return "proxmox" case "exec-provider": return "external" } diff --git a/internal/cli/claim_test.go b/internal/cli/claim_test.go index 77c7ebed0..5f9e4896f 100644 --- a/internal/cli/claim_test.go +++ b/internal/cli/claim_test.go @@ -94,6 +94,12 @@ func TestFixedLocalContainerClaimProviderCanonicalizesWithoutOverwritingMarker(t } } +func TestFixedProxmoxClaimProviderCanonicalizes(t *testing.T) { + if got := canonicalClaimProvider(FixedProxmoxClaimProvider); got != "proxmox" { + t.Fatalf("fixed Proxmox marker canonicalized to %q", got) + } +} + func TestClaimEndpointReservationDeadlineStartsAfterClaimLockAcquired(t *testing.T) { t.Setenv("XDG_STATE_HOME", t.TempDir()) const leaseID = "cbx_reservation_lock" diff --git a/internal/cli/proxmox.go b/internal/cli/proxmox.go index 3cb41f334..cfd830290 100644 --- a/internal/cli/proxmox.go +++ b/internal/cli/proxmox.go @@ -838,6 +838,10 @@ func (c *ProxmoxClient) nextID(ctx context.Context) (int, error) { } } +func (c *ProxmoxClient) NextVMID(ctx context.Context) (int, error) { + return c.nextID(ctx) +} + type proxmoxVM struct { VMID int `json:"vmid"` Name string `json:"name"` @@ -998,15 +1002,22 @@ func (c *ProxmoxClient) VMExistsInCluster(ctx context.Context, id string) (bool, } func (c *ProxmoxClient) CreateServer(ctx context.Context, cfg Config, publicKey, leaseID, slug string, keep bool) (Server, error) { + vmid, err := c.nextID(ctx) + if err != nil { + return Server{}, err + } + return c.CreateServerWithVMID(ctx, cfg, publicKey, leaseID, slug, keep, vmid, nil) +} + +func (c *ProxmoxClient) CreateServerWithVMID(ctx context.Context, cfg Config, publicKey, leaseID, slug string, keep bool, vmid int, extraLabels map[string]string) (Server, error) { if cfg.TargetOS != targetLinux { return Server{}, exit(2, "proxmox provider currently supports target=linux only") } if cfg.Proxmox.TemplateID <= 0 { return Server{}, exit(3, "proxmox templateId is required (set proxmox.templateId or CRABBOX_PROXMOX_TEMPLATE_ID)") } - vmid, err := c.nextID(ctx) - if err != nil { - return Server{}, err + if vmid <= 0 { + return Server{}, exit(2, "proxmox VMID must be positive") } name := leaseProviderName(leaseID, slug) full := "1" @@ -1041,6 +1052,9 @@ func (c *ProxmoxClient) CreateServer(ctx context.Context, cfg Config, publicKey, now := time.Now().UTC() labels := directLeaseLabels(cfg, leaseID, slug, "proxmox", "", keep, now) + for key, value := range extraLabels { + labels[key] = value + } labels["node"] = cfg.Proxmox.Node labels["template_id"] = strconv.Itoa(cfg.Proxmox.TemplateID) description := proxmoxDescription(labels) diff --git a/internal/cli/proxmox_test.go b/internal/cli/proxmox_test.go index 2999b17e3..fade539e2 100644 --- a/internal/cli/proxmox_test.go +++ b/internal/cli/proxmox_test.go @@ -1421,6 +1421,43 @@ func TestProxmoxCreateServerCleansUpCloneOnConfigFailure(t *testing.T) { } } +func TestProxmoxCreateServerWithVMIDSkipsNextIDAndUsesExplicitCloneTarget(t *testing.T) { + nextIDCalls := 0 + var clone url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/api2/json/cluster/nextid": + nextIDCalls++ + t.Fatal("explicit VMID create queried /cluster/nextid") + case r.Method == http.MethodPost && r.URL.Path == "/api2/json/nodes/pve1/qemu/9000/clone": + clone = readForm(t, r) + http.Error(w, "stop after clone request", http.StatusInternalServerError) + default: + t.Fatalf("%s %s", r.Method, r.URL.String()) + } + })) + defer server.Close() + + cfg := baseConfig() + cfg.Provider = "proxmox" + cfg.Proxmox.APIURL = server.URL + cfg.Proxmox.TokenID = "runner@pve!crabbox" + cfg.Proxmox.TokenSecret = "secret" + cfg.Proxmox.Node = "pve1" + cfg.Proxmox.TemplateID = 9000 + client, err := NewProxmoxClient(cfg) + if err != nil { + t.Fatal(err) + } + _, err = client.CreateServerWithVMID(context.Background(), cfg, "ssh-ed25519 AAAA test", "cbx_123456abcdef", "blue-crab", false, 417, nil) + if err == nil { + t.Fatal("expected fixture clone failure") + } + if nextIDCalls != 0 || clone.Get("newid") != "417" { + t.Fatalf("nextIDCalls=%d clone=%v", nextIDCalls, clone) + } +} + func testProxmoxClient(t *testing.T, serverURL string) *ProxmoxClient { t.Helper() cfg := baseConfig() diff --git a/internal/providers/proxmox/backend.go b/internal/providers/proxmox/backend.go index 84f1ed468..7b5c159c5 100644 --- a/internal/providers/proxmox/backend.go +++ b/internal/providers/proxmox/backend.go @@ -2,9 +2,12 @@ package proxmox import ( "context" + "crypto/sha256" + "encoding/json" "errors" "fmt" "io" + "maps" "strconv" "strings" "time" @@ -37,6 +40,8 @@ type proxmoxClient interface { ListCrabboxServers(context.Context) ([]Server, error) ListCrabboxServersCluster(context.Context) ([]Server, error) CreateServer(context.Context, Config, string, string, string, bool) (Server, error) + NextVMID(context.Context) (int, error) + CreateServerWithVMID(context.Context, Config, string, string, string, bool, int, map[string]string) (Server, error) GetServer(context.Context, string) (Server, error) GetServerOnNode(context.Context, string, string) (Server, error) VMExistsInCluster(context.Context, string) (bool, error) @@ -58,12 +63,287 @@ func NewLeaseBackend(spec ProviderSpec, cfg Config, rt Runtime) Backend { return &leaseBackend{DirectSSHBackend: shared.DirectSSHBackend{SpecValue: spec, Cfg: cfg, RT: rt, StoredLeaseKeys: true}} } +func (b *leaseBackend) SupportsRequestedLeaseID() bool { return true } + func (b *leaseBackend) Acquire(ctx context.Context, req AcquireRequest) (LeaseTarget, error) { + if strings.TrimSpace(req.RequestedLeaseID) != "" { + return b.acquireFixed(ctx, req) + } return shared.AcquireAttemptsRetry(b.RT, req.Keep, func() (LeaseTarget, error) { return b.acquireOnce(ctx, req.Keep, req.RequestedSlug) }) } +const fixedProxmoxCreateIntentVersion = 1 + +var fixedProxmoxLeaseKind = core.FixedLeaseKind{ + ClaimProvider: core.FixedProxmoxClaimProvider, + IntentVersion: fixedProxmoxCreateIntentVersion, + Label: "Proxmox", + TerminalIdentityLabels: []string{ + "crabbox", "provider", "lease", "slug", "provider_key", + "fixed_intent_sha256", "node", "template_id", + }, +} + +type fixedProxmoxCreateIntent struct { + ProviderScope string `json:"providerScope"` + Node string `json:"node"` + TemplateID int `json:"templateId"` + Storage string `json:"storage,omitempty"` + Pool string `json:"pool,omitempty"` + Bridge string `json:"bridge,omitempty"` + User string `json:"user"` + WorkRoot string `json:"workRoot"` + FullClone bool `json:"fullClone"` + ServerType string `json:"serverType"` + TargetOS string `json:"targetOS"` + RequestedSlug string `json:"requestedSlug,omitempty"` + Keep bool `json:"keep"` + TTLNanoseconds int64 `json:"ttlNanoseconds"` + IdleNanos int64 `json:"idleNanoseconds"` + SSHPublicKey string `json:"sshPublicKey"` +} + +func fixedProxmoxFingerprint(cfg Config, req AcquireRequest, providerScope, publicKey string) (string, error) { + data, err := json.Marshal(fixedProxmoxCreateIntent{ + ProviderScope: providerScope, Node: strings.TrimSpace(cfg.Proxmox.Node), + TemplateID: cfg.Proxmox.TemplateID, Storage: strings.TrimSpace(cfg.Proxmox.Storage), + Pool: strings.TrimSpace(cfg.Proxmox.Pool), Bridge: strings.TrimSpace(cfg.Proxmox.Bridge), + User: strings.TrimSpace(cfg.SSHUser), WorkRoot: strings.TrimSpace(cfg.WorkRoot), + FullClone: cfg.Proxmox.FullClone, ServerType: strings.TrimSpace(cfg.ServerType), + TargetOS: strings.TrimSpace(cfg.TargetOS), RequestedSlug: core.NormalizeLeaseSlug(req.RequestedSlug), + Keep: req.Keep, TTLNanoseconds: cfg.TTL.Nanoseconds(), IdleNanos: cfg.IdleTimeout.Nanoseconds(), + SSHPublicKey: strings.TrimSpace(publicKey), + }) + if err != nil { + return "", fmt.Errorf("fingerprint fixed Proxmox create intent: %w", err) + } + return fmt.Sprintf("%x", sha256.Sum256(data)), nil +} + +func (b *leaseBackend) acquireFixed(ctx context.Context, req AcquireRequest) (LeaseTarget, error) { + if b.Cfg.Proxmox.TemplateID <= 0 { + return LeaseTarget{}, exit(3, "proxmox templateId is required (set proxmox.templateId or CRABBOX_PROXMOX_TEMPLATE_ID)") + } + leaseID := strings.TrimSpace(req.RequestedLeaseID) + cfg := b.Cfg + cfg.ServerType = proxmoxServerTypeForConfig(cfg) + providerScope := strings.TrimSpace(core.ProviderClaimScope("proxmox", cfg)) + if providerScope == "" { + return LeaseTarget{}, exit(2, "Proxmox cluster scope is unavailable; refusing fixed lease creation") + } + client, err := newClient(cfg) + if err != nil { + return LeaseTarget{}, err + } + var publicKey, fingerprint string + freshClaim := false + acquired, err := core.AcquireFixedLease(core.FixedAcquireOptions{ + Kind: fixedProxmoxLeaseKind, LeaseID: leaseID, CheckpointID: req.RequestedCheckpointID, + RepoRoot: req.Repo.Root, Reclaim: req.Reclaim, TargetOS: cfg.TargetOS, + WindowsMode: cfg.WindowsMode, TTL: cfg.TTL, IdleTimeout: cfg.IdleTimeout, + }, func(ctx context.Context, _ *core.LeaseClaim, exists bool) (core.FixedLeaseBinding, error) { + freshClaim = !exists + keyPath, key, err := ensureTestboxKeyForConfig(cfg, leaseID) + if err != nil { + return core.FixedLeaseBinding{}, err + } + cfg.SSHKey, publicKey = keyPath, key + cfg.ProviderKey = providerKeyForLease(leaseID) + fingerprint, err = fixedProxmoxFingerprint(cfg, req, providerScope, publicKey) + if err != nil { + return core.FixedLeaseBinding{}, err + } + binding := core.FixedLeaseBinding{ProviderScope: providerScope, Fingerprint: fingerprint} + if exists { + return binding, nil + } + servers, err := client.ListCrabboxServersCluster(ctx) + if err != nil { + return core.FixedLeaseBinding{}, err + } + binding.Slug, err = allocateDirectLeaseSlug(leaseID, req.RequestedSlug, servers) + return binding, err + }, func(ctx context.Context, claim *core.LeaseClaim, intent *core.FixedCreateIntent, persist func() error) (LeaseTarget, error) { + if claim.ProviderScope != providerScope || intent.ProviderScope != providerScope { + return LeaseTarget{}, exit(4, "lease_id_conflict: fixed Proxmox lease %s provider scope changed", leaseID) + } + server, found, err := b.findFixedProxmoxServer(ctx, client, *claim) + if err != nil { + return LeaseTarget{}, err + } + attemptVMID, attemptNode, attemptErr := fixedProxmoxAttempt(*claim) + if attemptErr != nil { + return LeaseTarget{}, attemptErr + } + if !found { + if attemptVMID != 0 { + exists, err := client.VMExistsInCluster(ctx, strconv.Itoa(attemptVMID)) + if err != nil { + return LeaseTarget{}, fmt.Errorf("reconcile fixed Proxmox VMID %d: %w", attemptVMID, err) + } + if exists { + return LeaseTarget{}, exit(4, "lease_id_conflict: fixed Proxmox VMID %d exists without matching lease identity", attemptVMID) + } + return LeaseTarget{}, exit(4, "lease_id_conflict: fixed Proxmox lease %s has an unresolved clone attempt; retain its claim for recovery", leaseID) + } + if !freshClaim { + return LeaseTarget{}, exit(4, "lease_id_conflict: fixed Proxmox lease %s has no provably unsubmitted attempt; retain its claim", leaseID) + } + attemptVMID, err = client.NextVMID(ctx) + if err != nil { + return LeaseTarget{}, err + } + if attemptVMID <= 0 { + return LeaseTarget{}, exit(4, "lease_id_conflict: Proxmox selected invalid VMID %d", attemptVMID) + } + attemptNode = strings.TrimSpace(cfg.Proxmox.Node) + intent.Attempt = map[string]string{ + "vmid": strconv.Itoa(attemptVMID), "node": attemptNode, + } + labels := fixedProxmoxIdentityLabels(cfg, leaseID, intent.Slug, fingerprint, attemptNode) + claim.CloudID = strconv.Itoa(attemptVMID) + claim.CloudNumericID = int64(attemptVMID) + claim.Labels = maps.Clone(labels) + if err := persist(); err != nil { + return LeaseTarget{}, err + } + fmt.Fprintf(b.RT.Stderr, "provisioning provider=proxmox lease=%s slug=%s node=%s template=%d vmid=%d keep=%v fixed=true\n", + leaseID, intent.Slug, cfg.Proxmox.Node, cfg.Proxmox.TemplateID, attemptVMID, req.Keep) + server, err = client.CreateServerWithVMID(ctx, cfg, publicKey, leaseID, intent.Slug, req.Keep, attemptVMID, labels) + if err != nil { + recovered, recoveredFound, reconcileErr := b.findFixedProxmoxServer(ctx, client, *claim) + if reconcileErr != nil { + return LeaseTarget{}, errors.Join(err, reconcileErr) + } + if !recoveredFound { + return LeaseTarget{}, err + } + server = recovered + } + } + if err := validateFixedProxmoxServer(server, *claim, attemptVMID, attemptNode); err != nil { + return LeaseTarget{}, err + } + if claim.CloudImmutableID == "" { + claim.CloudNumericID = server.ID + claim.CloudImmutableID = server.ImmutableID + claim.Labels = maps.Clone(server.Labels) + if err := persist(); err != nil { + return LeaseTarget{}, err + } + } else if claim.CloudID != server.CloudID || claim.CloudImmutableID != server.ImmutableID { + return LeaseTarget{}, exit(4, "lease_id_conflict: fixed Proxmox lease %s resource identity changed", leaseID) + } + target := sshTargetFromConfig(cfg, server.PublicNet.IPv4.IP) + if err := waitForSSHReadyFunc(ctx, &target, b.RT.Stderr, "bootstrap", bootstrapWaitTimeout(cfg)); err != nil { + return LeaseTarget{}, err + } + server.Labels = maps.Clone(server.Labels) + server.Labels["state"] = "ready" + if err := client.SetLabelsOnNode(ctx, server.HostID, server.CloudID, server.Labels); err != nil { + return LeaseTarget{}, fmt.Errorf("persist Proxmox fixed lease labels: %w", err) + } + return LeaseTarget{Server: server, SSH: target, LeaseID: leaseID}, nil + }, ctx) + if err != nil { + return LeaseTarget{}, err + } + if req.OnAcquired != nil { + if err := req.OnAcquired(acquired); err != nil { + return LeaseTarget{}, fmt.Errorf("acknowledge fixed Proxmox acquisition: %w", err) + } + } + return acquired, nil +} + +func fixedProxmoxIdentityLabels(cfg Config, leaseID, slug, fingerprint, node string) map[string]string { + return map[string]string{ + "crabbox": "true", + "provider": "proxmox", + "lease": leaseID, + "slug": slug, + "provider_key": core.ProviderKeyForLease(leaseID), + "fixed_intent_sha256": fingerprint, + "node": node, + "template_id": strconv.Itoa(cfg.Proxmox.TemplateID), + } +} + +func fixedProxmoxAttempt(claim core.LeaseClaim) (int, string, error) { + intent := claim.FixedCreateIntent + if !fixedProxmoxLeaseKind.IsFixedClaim(claim) || intent.Version != fixedProxmoxCreateIntentVersion || + intent.Fingerprint == "" || intent.Slug != claim.Slug || intent.ProviderScope == "" || + (intent.State != "prepared" && intent.State != "acquired") || len(intent.FailedAttempts) != 0 { + return 0, "", exit(4, "lease_id_conflict: invalid fixed Proxmox create intent for lease %s", claim.LeaseID) + } + if len(intent.Attempt) == 0 { + if claim.CloudID != "" || claim.CloudImmutableID != "" || len(claim.Labels) != 0 { + return 0, "", exit(4, "lease_id_conflict: fixed Proxmox lease %s has no durable clone attempt", claim.LeaseID) + } + return 0, "", nil + } + vmid, err := strconv.Atoi(intent.Attempt["vmid"]) + node := strings.TrimSpace(intent.Attempt["node"]) + if err != nil || vmid <= 0 || strconv.Itoa(vmid) != intent.Attempt["vmid"] || node == "" || len(intent.Attempt) != 2 { + return 0, "", exit(4, "lease_id_conflict: invalid fixed Proxmox clone attempt for lease %s", claim.LeaseID) + } + if claim.CloudID != strconv.Itoa(vmid) || claim.CloudNumericID != int64(vmid) || + claim.ProviderScope != intent.ProviderScope || + claim.Labels["crabbox"] != "true" || claim.Labels["provider"] != "proxmox" || + claim.Labels["lease"] != claim.LeaseID || claim.Labels["slug"] != claim.Slug || + claim.Labels["provider_key"] != core.ProviderKeyForLease(claim.LeaseID) || + claim.Labels["fixed_intent_sha256"] != intent.Fingerprint || + claim.Labels["node"] != node || claim.Labels["template_id"] == "" || + intent.State == "acquired" && claim.CloudImmutableID == "" { + return 0, "", exit(4, "lease_id_conflict: fixed Proxmox lease %s durable VM identity is inconsistent", claim.LeaseID) + } + return vmid, node, nil +} + +func (b *leaseBackend) findFixedProxmoxServer(ctx context.Context, client proxmoxClient, claim core.LeaseClaim) (Server, bool, error) { + servers, err := client.ListCrabboxServersCluster(ctx) + if err != nil { + return Server{}, false, err + } + var found []Server + for _, server := range servers { + if strings.TrimSpace(server.Labels["lease"]) == claim.LeaseID { + found = append(found, server) + } + } + if len(found) > 1 { + return Server{}, false, exit(4, "lease_id_conflict: multiple Proxmox VMs match fixed lease %s", claim.LeaseID) + } + if len(found) == 1 { + return found[0], true, nil + } + return Server{}, false, nil +} + +func validateFixedProxmoxServer(server Server, claim core.LeaseClaim, vmid int, attemptNode string) error { + intent := claim.FixedCreateIntent + if vmid <= 0 || server.CloudID != strconv.Itoa(vmid) || server.ID != 0 && server.ID != int64(vmid) || + server.Provider != "proxmox" || server.HostID == "" || server.ImmutableID == "" || + server.Labels["crabbox"] != "true" || server.Labels["provider"] != "proxmox" || + server.Labels["lease"] != claim.LeaseID || core.NormalizeLeaseSlug(server.Labels["slug"]) != intent.Slug || + server.Labels["provider_key"] != core.ProviderKeyForLease(claim.LeaseID) || + server.Labels["fixed_intent_sha256"] != intent.Fingerprint || + server.Labels["node"] != attemptNode || + server.Labels["template_id"] != claim.Labels["template_id"] { + return exit(4, "lease_id_conflict: Proxmox VM for lease %s does not match its durable fixed identity", claim.LeaseID) + } + if claim.CloudID != "" && server.CloudID != claim.CloudID || + claim.CloudImmutableID != "" && server.ImmutableID != claim.CloudImmutableID { + return exit(4, "lease_id_conflict: Proxmox VM for lease %s does not match its bound VMID and vmgenid", claim.LeaseID) + } + if attemptNode == "" { + return exit(4, "lease_id_conflict: fixed Proxmox lease %s has no durable source node", claim.LeaseID) + } + return nil +} + func (b *leaseBackend) acquireOnce(ctx context.Context, keep bool, requestedSlug string) (LeaseTarget, error) { if b.Cfg.Proxmox.TemplateID <= 0 { return LeaseTarget{}, exit(3, "proxmox templateId is required (set proxmox.templateId or CRABBOX_PROXMOX_TEMPLATE_ID)") @@ -354,6 +634,31 @@ func (b *leaseBackend) Doctor(ctx context.Context, _ core.DoctorRequest) (core.D } func (b *leaseBackend) ReleaseLease(ctx context.Context, req ReleaseLeaseRequest) error { + _, err := b.ReleaseLeaseWithOutcome(ctx, req) + return err +} + +func (b *leaseBackend) ReleaseLeaseWithOutcome(ctx context.Context, req ReleaseLeaseRequest) (core.ReleaseLeaseOutcome, error) { + leaseID := strings.TrimSpace(req.Lease.LeaseID) + if leaseID == "" { + leaseID = proxmoxClaimLabelLeaseID(req.Lease.Server) + } + claim, exists, err := core.ReadLeaseClaimWithPresence(leaseID) + if err != nil { + return core.ReleaseLeaseOutcome{}, err + } + if exists && fixedProxmoxLeaseKind.IsFixedClaim(claim) { + if label := proxmoxClaimLabelLeaseID(req.Lease.Server); label != "" && label != leaseID { + return core.ReleaseLeaseOutcome{}, exit(4, "lease_id_conflict: fixed Proxmox release lease label %s does not match %s", label, leaseID) + } + err := b.releaseFixed(ctx, req, false) + return core.ReleaseLeaseOutcome{Terminal: err == nil}, err + } + err = b.releaseOrdinary(ctx, req) + return core.ReleaseLeaseOutcome{Terminal: err == nil}, err +} + +func (b *leaseBackend) releaseOrdinary(ctx context.Context, req ReleaseLeaseRequest) error { client, err := newClient(b.Cfg) if err != nil { return err @@ -388,6 +693,110 @@ func (b *leaseBackend) ReleaseLease(ctx context.Context, req ReleaseLeaseRequest return removeCleanupLeaseResidue(ctx, client, deleted, remaining, b.Cfg, b.RT.Stderr) } +func (b *leaseBackend) releaseFixed(ctx context.Context, req ReleaseLeaseRequest, requireCleanupEligible bool) error { + leaseID := strings.TrimSpace(req.Lease.LeaseID) + if leaseID == "" { + leaseID = proxmoxClaimLabelLeaseID(req.Lease.Server) + } + client, err := newClient(b.Cfg) + if err != nil { + return err + } + return core.WithDurableLeaseClaimLock(leaseID, func(claim *core.LeaseClaim, exists bool, persist func() error) error { + if !exists || !fixedProxmoxLeaseKind.IsFixedClaim(*claim) { + return exit(4, "lease_id_conflict: fixed Proxmox lease %s has no durable ownership claim", leaseID) + } + if claim.FixedCreateIntent.State == "released" { + return fixedProxmoxLeaseKind.ValidateTerminalClaim(*claim, core.LeaseClaim{}, leaseID, validateFixedProxmoxTerminalClaim) + } + if strings.TrimSpace(core.ProviderClaimScope("proxmox", b.Cfg)) != claim.FixedCreateIntent.ProviderScope || + claim.ProviderScope != claim.FixedCreateIntent.ProviderScope { + return exit(4, "lease_id_conflict: fixed Proxmox lease %s provider scope changed before release", leaseID) + } + vmid, node, err := fixedProxmoxAttempt(*claim) + if err != nil { + return err + } + if vmid == 0 { + return exit(4, "lease_id_conflict: fixed Proxmox lease %s has no durable clone attempt", leaseID) + } + server, found, err := b.findFixedProxmoxServer(ctx, client, *claim) + if err != nil { + return err + } + if found { + if err := validateFixedProxmoxServer(server, *claim, vmid, node); err != nil { + return err + } + if claim.CloudImmutableID == "" { + claim.CloudNumericID, claim.CloudImmutableID = server.ID, server.ImmutableID + claim.Labels = maps.Clone(server.Labels) + if err := persist(); err != nil { + return err + } + } + check := func(live Server) error { + if err := validateFixedProxmoxServer(live, *claim, vmid, node); err != nil { + return err + } + if requireCleanupEligible { + if eligible, reason := core.ShouldCleanupServer(live, time.Now().UTC()); !eligible { + return fmt.Errorf("Proxmox VM %s no longer eligible: %s", live.CloudID, reason) + } + } + return nil + } + if err := client.DeleteServerOnNodeChecked(ctx, server.HostID, server.CloudID, check); err != nil { + return err + } + } + remaining, err := client.ListCrabboxServersCluster(ctx) + if err != nil { + return fmt.Errorf("verify fixed Proxmox release inventory: %w", err) + } + for _, candidate := range remaining { + if candidate.CloudID == strconv.Itoa(vmid) || candidate.Labels["lease"] == leaseID { + return exit(4, "lease_id_conflict: fixed Proxmox lease %s still has a surviving VM", leaseID) + } + } + present, err := client.VMExistsInCluster(ctx, strconv.Itoa(vmid)) + if err != nil { + return fmt.Errorf("verify fixed Proxmox VMID %d absence: %w", vmid, err) + } + if present { + return exit(4, "lease_id_conflict: fixed Proxmox VMID %d still exists after release", vmid) + } + if len(claim.Labels) == 0 { + claim.Labels = fixedProxmoxIdentityLabels(b.Cfg, claim.LeaseID, claim.Slug, claim.FixedCreateIntent.Fingerprint, node) + } + *claim = fixedProxmoxLeaseKind.TerminalClaim(*claim, time.Now().UTC()) + return persist() + }) +} + +func validateFixedProxmoxTerminalClaim(claim core.LeaseClaim) error { + if claim.CloudID == "" || claim.CloudID != strconv.FormatInt(claim.CloudNumericID, 10) || + claim.Labels["lease"] != claim.LeaseID || claim.Labels["provider"] != "proxmox" || + claim.Labels["fixed_intent_sha256"] != claim.FixedCreateIntent.Fingerprint { + return exit(4, "lease_id_conflict: fixed Proxmox lease %s has an invalid terminal VM identity", claim.LeaseID) + } + return nil +} + +func (b *leaseBackend) RetainLeaseClaimAfterRelease(lease LeaseTarget) bool { + retained, err := b.retainLeaseClaimAfterRelease(lease, core.LeaseClaim{}) + return retained || err != nil +} + +func (b *leaseBackend) RetainLeaseClaimAfterReleaseWithClaim(lease LeaseTarget, previous core.LeaseClaim) (bool, error) { + return b.retainLeaseClaimAfterRelease(lease, previous) +} + +func (b *leaseBackend) retainLeaseClaimAfterRelease(lease LeaseTarget, previous core.LeaseClaim) (bool, error) { + fixedEvidence := strings.TrimSpace(lease.Server.Labels["fixed_intent_sha256"]) != "" + return fixedProxmoxLeaseKind.RetainClaimAfterRelease(lease.LeaseID, previous, fixedEvidence, validateFixedProxmoxTerminalClaim, nil) +} + func (b *leaseBackend) backfillReleaseClaimScope(leaseID, cloudID string, server Server) error { if leaseID == "" || proxmoxClaimLabelLeaseID(server) != leaseID { return nil @@ -442,6 +851,28 @@ func (b *leaseBackend) Cleanup(ctx context.Context, req CleanupRequest) error { return err } for _, server := range servers { + var fixedClaim core.LeaseClaim + for _, claim := range claims { + if claim.LeaseID == proxmoxClaimLabelLeaseID(server) && fixedProxmoxLeaseKind.IsFixedClaim(claim) { + fixedClaim = claim + break + } + } + if fixedClaim.LeaseID != "" { + if eligible, reason := core.ShouldCleanupServer(server, time.Now().UTC()); !eligible { + fmt.Fprintf(b.RT.Stderr, "skip server id=%s name=%s reason=%s\n", server.DisplayID(), server.Name, reason) + continue + } + if req.DryRun { + fmt.Fprintf(b.RT.Stderr, "would delete server id=%s name=%s\n", server.DisplayID(), server.Name) + continue + } + if err := b.releaseFixed(ctx, ReleaseLeaseRequest{Lease: LeaseTarget{LeaseID: fixedClaim.LeaseID, Server: server}}, true); err != nil { + return err + } + fmt.Fprintf(b.RT.Stderr, "delete server id=%s name=%s fixed=true key_retained=true\n", server.DisplayID(), server.Name) + continue + } claim, binding, err := b.cleanupClaim(server, servers, claims) if err != nil { fmt.Fprintf(b.RT.Stderr, "skip server id=%s name=%s reason=%v\n", server.DisplayID(), server.Name, err) diff --git a/internal/providers/proxmox/backend_doctor_test.go b/internal/providers/proxmox/backend_doctor_test.go index 7c7729d73..4f1be38bf 100644 --- a/internal/providers/proxmox/backend_doctor_test.go +++ b/internal/providers/proxmox/backend_doctor_test.go @@ -73,6 +73,14 @@ func (c *fakeProxmoxDoctorClient) CreateServer(_ context.Context, _ Config, _ st return Server{}, nil } +func (c *fakeProxmoxDoctorClient) NextVMID(context.Context) (int, error) { + return 101, nil +} + +func (c *fakeProxmoxDoctorClient) CreateServerWithVMID(ctx context.Context, cfg Config, publicKey, leaseID, slug string, keep bool, _ int, _ map[string]string) (Server, error) { + return c.CreateServer(ctx, cfg, publicKey, leaseID, slug, keep) +} + func (c *fakeProxmoxDoctorClient) GetServer(_ context.Context, id string) (Server, error) { c.getCalls++ if c.getCallsByID == nil { @@ -266,6 +274,14 @@ func TestProxmoxTouchUsesMigratedVMNode(t *testing.T) { } } +func TestProxmoxAdvertisesRequestedLeaseIDSupport(t *testing.T) { + backend := NewLeaseBackend(Provider{}.Spec(), Config{}, Runtime{}) + fixed, ok := backend.(core.IdempotentLeaseIDBackend) + if !ok || !fixed.SupportsRequestedLeaseID() { + t.Fatalf("backend=%T fixed=%t, want requested lease ID support", backend, ok) + } +} + func TestProxmoxAcquireRejectsMissingTemplateBeforeClientWork(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) t.Setenv("XDG_STATE_HOME", t.TempDir()) diff --git a/internal/providers/proxmox/fixed_test.go b/internal/providers/proxmox/fixed_test.go new file mode 100644 index 000000000..bf9941264 --- /dev/null +++ b/internal/providers/proxmox/fixed_test.go @@ -0,0 +1,306 @@ +package proxmox + +import ( + "context" + "errors" + "io" + "maps" + "reflect" + "strings" + "testing" + "time" + + core "github.com/openclaw/crabbox/internal/cli" +) + +const fixedTestGeneration = "8be39656-32b0-4c47-b68c-0a9e1d3ef901" + +type fixedProxmoxClient struct { + *fakeProxmoxDoctorClient + nextVMID int + nextCalls int + fixedCreates int + beforeClone func(int, map[string]string) error + fixedCreateErr error +} + +func (c *fixedProxmoxClient) NextVMID(context.Context) (int, error) { + c.nextCalls++ + return c.nextVMID, nil +} + +func (c *fixedProxmoxClient) CreateServerWithVMID(_ context.Context, _ Config, _ string, leaseID, slug string, _ bool, vmid int, labels map[string]string) (Server, error) { + c.fixedCreates++ + if c.beforeClone != nil { + if err := c.beforeClone(vmid, labels); err != nil { + return Server{}, err + } + } + if c.fixedCreateErr != nil { + return Server{}, c.fixedCreateErr + } + server := Server{ + Provider: "proxmox", + CloudID: "417", + HostID: "pve1", + ImmutableID: fixedTestGeneration, + ID: 417, + Name: "crabbox-" + slug, + Labels: maps.Clone(labels), + } + server.Labels["lease"] = leaseID + server.Labels["slug"] = slug + server.Labels["provider"] = "proxmox" + server.Labels["crabbox"] = "true" + server.PublicNet.IPv4.IP = "192.0.2.17" + c.servers = []Server{server} + return server, nil +} + +func fixedProxmoxFixture(t *testing.T) (*leaseBackend, *fixedProxmoxClient, AcquireRequest) { + t.Helper() + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + t.Setenv("XDG_STATE_HOME", t.TempDir()) + cfg := Config{ + Provider: "proxmox", + TargetOS: "linux", + SSHUser: "crabbox", + WorkRoot: "/work/crabbox", + IdleTimeout: 30 * time.Minute, + Proxmox: core.ProxmoxConfig{ + APIURL: "https://pve.example.test:8006", + Node: "pve1", + TemplateID: 9400, + Storage: "local-lvm", + Bridge: "vmbr0", + User: "crabbox", + WorkRoot: "/work/crabbox", + FullClone: true, + TokenID: "runner@pve!crabbox", + TokenSecret: "secret", + }, + } + client := &fixedProxmoxClient{fakeProxmoxDoctorClient: &fakeProxmoxDoctorClient{}, nextVMID: 417} + previousClient := newClient + newClient = func(Config) (proxmoxClient, error) { return client, nil } + t.Cleanup(func() { newClient = previousClient }) + previousWait := waitForSSHReadyFunc + waitForSSHReadyFunc = func(context.Context, *SSHTarget, io.Writer, string, time.Duration) error { return nil } + t.Cleanup(func() { waitForSSHReadyFunc = previousWait }) + backend := NewLeaseBackend(Provider{}.Spec(), cfg, Runtime{Stdout: io.Discard, Stderr: io.Discard}).(*leaseBackend) + req := AcquireRequest{RequestedLeaseID: "cbx_123456abcdef", RequestedSlug: "fixed-proxmox", Repo: core.Repo{Root: t.TempDir()}} + return backend, client, req +} + +func readFixedProxmoxClaim(t *testing.T, leaseID string) core.LeaseClaim { + t.Helper() + claim, exists, err := core.ReadLeaseClaimWithPresence(leaseID) + if err != nil || !exists { + t.Fatalf("claim exists=%t err=%v", exists, err) + } + return claim +} + +func TestProxmoxFixedAcquirePersistsVMIDBeforeCloneAndReplaysExactVM(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + client.beforeClone = func(vmid int, labels map[string]string) error { + claim := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if claim.FixedCreateIntent == nil || claim.FixedCreateIntent.State != "prepared" || + claim.FixedCreateIntent.Attempt["vmid"] != "417" || + claim.FixedCreateIntent.Attempt["node"] != "pve1" || + claim.CloudID != "417" || claim.CloudNumericID != 417 || + claim.FixedCreateIntent.Fingerprint == "" || + claim.Labels["fixed_intent_sha256"] != claim.FixedCreateIntent.Fingerprint || + labels["fixed_intent_sha256"] != claim.FixedCreateIntent.Fingerprint || + vmid != 417 { + t.Fatalf("clone preceded durable intent/VMID: vmid=%d labels=%v claim=%+v", vmid, labels, claim) + } + return nil + } + first, err := backend.Acquire(context.Background(), req) + if err != nil { + t.Fatal(err) + } + replayed, err := backend.Acquire(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if client.nextCalls != 1 || client.fixedCreates != 1 || first.Server.CloudID != "417" || replayed.Server.CloudID != first.Server.CloudID || replayed.Server.ImmutableID != first.Server.ImmutableID { + t.Fatalf("next=%d clones=%d first=%+v replay=%+v", client.nextCalls, client.fixedCreates, first.Server, replayed.Server) + } +} + +func TestProxmoxFixedAcquireIntentDriftConflicts(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + if _, err := backend.Acquire(context.Background(), req); err != nil { + t.Fatal(err) + } + req.Keep = true + if _, err := backend.Acquire(context.Background(), req); err == nil || !strings.Contains(err.Error(), "lease_id_conflict") { + t.Fatalf("intent drift err=%v", err) + } + if client.fixedCreates != 1 { + t.Fatalf("clones=%d, want 1", client.fixedCreates) + } +} + +func TestProxmoxFixedAcquireRejectsIdentityMismatch(t *testing.T) { + for _, tc := range []struct { + name string + mutate func(*core.LeaseClaim, *Server) + }{ + {"lease label", func(_ *core.LeaseClaim, server *Server) { server.Labels["lease"] = "cbx_aaaaaaaaaaaa" }}, + {"provider label", func(_ *core.LeaseClaim, server *Server) { server.Labels["provider"] = "other" }}, + {"source node label", func(_ *core.LeaseClaim, server *Server) { server.Labels["node"] = "pve2" }}, + {"VMID", func(_ *core.LeaseClaim, server *Server) { server.CloudID = "418" }}, + {"vmgenid", func(_ *core.LeaseClaim, server *Server) { server.ImmutableID = replacementGeneration }}, + {"provider scope", func(claim *core.LeaseClaim, _ *Server) { claim.ProviderScope += "/other" }}, + {"claim lease label", func(claim *core.LeaseClaim, _ *Server) { claim.Labels["lease"] = "cbx_aaaaaaaaaaaa" }}, + } { + t.Run(tc.name, func(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + if _, err := backend.Acquire(context.Background(), req); err != nil { + t.Fatal(err) + } + before := readFixedProxmoxClaim(t, req.RequestedLeaseID) + claim := before + claim.Labels = maps.Clone(before.Labels) + server := client.servers[0] + server.Labels = maps.Clone(server.Labels) + tc.mutate(&claim, &server) + if !reflect.DeepEqual(claim, before) { + if err := core.ReplaceLeaseClaimIfUnchanged(before.LeaseID, before, claim); err != nil { + t.Fatal(err) + } + } + client.servers = []Server{server} + if _, err := backend.Acquire(context.Background(), req); err == nil || !strings.Contains(err.Error(), "lease_id_conflict") { + t.Fatalf("mismatch accepted: %v", err) + } + if client.fixedCreates != 1 { + t.Fatalf("mismatch issued clone count=%d", client.fixedCreates) + } + }) + } +} + +func TestProxmoxFixedAcquireRetainsUncertainAttemptWithoutSecondClone(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + client.fixedCreateErr = errors.New("clone response lost") + if _, err := backend.Acquire(context.Background(), req); err == nil { + t.Fatal("expected first ambiguous clone failure") + } + before := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if before.FixedCreateIntent.Attempt["vmid"] != "417" || before.FixedCreateIntent.State != "prepared" { + t.Fatalf("uncertain attempt not retained: %+v", before) + } + if _, err := backend.Acquire(context.Background(), req); err == nil || !strings.Contains(err.Error(), "unresolved") { + t.Fatalf("replay err=%v", err) + } + after := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if client.nextCalls != 1 || client.fixedCreates != 1 || !reflect.DeepEqual(before, after) { + t.Fatalf("next=%d clones=%d before=%+v after=%+v", client.nextCalls, client.fixedCreates, before, after) + } +} + +func TestProxmoxFixedAcquireNeverAdoptsBySlug(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + client.fixedCreateErr = errors.New("clone response lost") + if _, err := backend.Acquire(context.Background(), req); err == nil { + t.Fatal("expected first ambiguous clone failure") + } + unrelated := Server{ + Provider: "proxmox", CloudID: "417", HostID: "pve1", ImmutableID: fixedTestGeneration, + ID: 417, Name: "crabbox-" + req.RequestedSlug, + Labels: map[string]string{ + "crabbox": "true", "provider": "proxmox", "lease": "cbx_aaaaaaaaaaaa", + "slug": req.RequestedSlug, "provider_key": core.ProviderKeyForLease("cbx_aaaaaaaaaaaa"), + }, + } + client.servers = []Server{unrelated} + if _, err := backend.Acquire(context.Background(), req); err == nil || !strings.Contains(err.Error(), "lease_id_conflict") { + t.Fatalf("slug-only VM adopted: %v", err) + } + if client.fixedCreates != 1 { + t.Fatalf("slug-only replay issued clone count=%d", client.fixedCreates) + } +} + +func TestProxmoxFixedConfirmedAbsentAttemptLeavesTerminalTombstone(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + client.fixedCreateErr = errors.New("clone response lost") + if _, err := backend.Acquire(context.Background(), req); err == nil { + t.Fatal("expected first ambiguous clone failure") + } + previous := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if err := backend.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: LeaseTarget{LeaseID: req.RequestedLeaseID}}); err != nil { + t.Fatal(err) + } + claim := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if claim.FixedCreateIntent.State != "released" || claim.CloudID != "417" { + t.Fatalf("confirmed absence did not retain terminal VMID: %+v", claim) + } + if _, err := backend.Acquire(context.Background(), req); err == nil { + t.Fatal("confirmed-absent fixed ID was reusable") + } + if retained, err := backend.RetainLeaseClaimAfterReleaseWithClaim(LeaseTarget{LeaseID: req.RequestedLeaseID}, previous); err != nil || !retained { + t.Fatalf("terminal claim retained=%t err=%v", retained, err) + } +} + +func TestProxmoxFixedReleaseLeavesTerminalTombstone(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + lease, err := backend.Acquire(context.Background(), req) + if err != nil { + t.Fatal(err) + } + previous := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if err := backend.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err != nil { + t.Fatal(err) + } + claim := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if claim.Provider != core.FixedProxmoxClaimProvider || claim.FixedCreateIntent == nil || claim.FixedCreateIntent.State != "released" { + t.Fatalf("invalid terminal claim: %+v", claim) + } + if _, err := backend.Acquire(context.Background(), req); err == nil || !strings.Contains(err.Error(), "terminal") { + t.Fatalf("terminal replay err=%v", err) + } + if client.fixedCreates != 1 || client.deleteCalls != 1 { + t.Fatalf("clones=%d deletes=%d", client.fixedCreates, client.deleteCalls) + } + if retained, err := backend.RetainLeaseClaimAfterReleaseWithClaim(lease, previous); err != nil || !retained { + t.Fatalf("terminal claim retained=%t err=%v", retained, err) + } +} + +func TestProxmoxFixedReleaseRejectsMismatchedLeaseLabel(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + lease, err := backend.Acquire(context.Background(), req) + if err != nil { + t.Fatal(err) + } + lease.Server.Labels = maps.Clone(lease.Server.Labels) + lease.Server.Labels["lease"] = "cbx_aaaaaaaaaaaa" + before := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if err := backend.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: lease}); err == nil { + t.Fatal("mismatched release label was accepted") + } + if client.deleteCalls != 0 || !reflect.DeepEqual(before, readFixedProxmoxClaim(t, req.RequestedLeaseID)) { + t.Fatal("mismatched release label changed resource or claim") + } +} + +func TestProxmoxFixedCleanupLeavesTerminalTombstone(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + if _, err := backend.Acquire(context.Background(), req); err != nil { + t.Fatal(err) + } + client.servers[0].Labels["state"] = "failed" + if err := backend.Cleanup(context.Background(), CleanupRequest{}); err != nil { + t.Fatal(err) + } + claim := readFixedProxmoxClaim(t, req.RequestedLeaseID) + if claim.FixedCreateIntent.State != "released" || client.deleteCalls != 1 { + t.Fatalf("cleanup claim=%+v deletes=%d", claim, client.deleteCalls) + } +} From 25340a91781fe7e0c726dd07236823ae0568238b Mon Sep 17 00:00:00 2001 From: Apple FCU Fleet Date: Fri, 4 Sep 2026 21:50:40 -0400 Subject: [PATCH 2/2] fix(proxmox): converge unclaimed fixed release on proven absence Release-only resolution of an exact fixed Proxmox lease ID failed with exit 4 "lease/server not found" whenever Crabbox held no claim, leaving callers stuck retrying destruction of a VM that no longer exists. A completed cluster-wide inventory that shows no VM carrying the lease identity now converges as confirmed absence, so release is idempotent. Unreadable inventory, reused or conflicting VM identity, an unresolved cluster scope, and non-exact identifiers still fail closed, and no tombstone is fabricated for an ID that has no durable create intent. Refs https://github.com/openclaw/crabbox/issues/1847 Co-authored-by: Cursor --- internal/providers/proxmox/backend.go | 39 ++++++++++++- internal/providers/proxmox/fixed_test.go | 74 ++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/internal/providers/proxmox/backend.go b/internal/providers/proxmox/backend.go index 7b5c159c5..3c7c1c60d 100644 --- a/internal/providers/proxmox/backend.go +++ b/internal/providers/proxmox/backend.go @@ -477,12 +477,12 @@ func (b *leaseBackend) Resolve(ctx context.Context, req ResolveRequest) (LeaseTa return target, nil } if req.ReleaseOnly { - return b.releaseTargetFromClaim(ctx, client, req.ID) + return b.releaseTargetFromClaim(ctx, client, req.ID, servers) } return LeaseTarget{}, exit(4, "lease/server not found: %s", req.ID) } -func (b *leaseBackend) releaseTargetFromClaim(ctx context.Context, client proxmoxClient, id string) (LeaseTarget, error) { +func (b *leaseBackend) releaseTargetFromClaim(ctx context.Context, client proxmoxClient, id string, inventory []Server) (LeaseTarget, error) { var ( claim core.LeaseClaim ok bool @@ -500,7 +500,10 @@ func (b *leaseBackend) releaseTargetFromClaim(ctx context.Context, client proxmo if err != nil { return LeaseTarget{}, err } - if !ok || claim.LeaseID == "" || !core.LeaseClaimMatchesIdentifier(claim, id) { + if !ok { + return b.confirmedAbsentReleaseTarget(id, inventory) + } + if claim.LeaseID == "" || !core.LeaseClaimMatchesIdentifier(claim, id) { return LeaseTarget{}, exit(4, "lease/server not found: %s", id) } cloudID := strings.TrimSpace(claim.CloudID) @@ -561,6 +564,36 @@ func (b *leaseBackend) releaseTargetFromClaim(ctx context.Context, client proxmo }, nil } +// confirmedAbsentReleaseTarget converges an exact lease ID that Crabbox does +// not claim into an idempotent release, so a caller holding a fixed ID is not +// stuck retrying destruction of a VM that provably does not exist. The caller +// supplies a completed cluster-wide inventory; a failed inventory read never +// reaches here, so absence always rests on a full cluster read. No tombstone +// is written because an unclaimed ID has no durable intent to bind one to. +func (b *leaseBackend) confirmedAbsentReleaseTarget(id string, inventory []Server) (LeaseTarget, error) { + if !core.IsCanonicalLeaseID(id) { + return LeaseTarget{}, exit(4, "lease/server not found: %s", id) + } + if strings.TrimSpace(core.ProviderClaimScope("proxmox", b.Cfg)) == "" { + return LeaseTarget{}, exit(2, "refusing to confirm absence of unclaimed Proxmox lease %s without a resolved cluster scope", id) + } + providerKey := core.ProviderKeyForLease(id) + for _, server := range inventory { + if proxmoxClaimLabelLeaseID(server) == id || strings.TrimSpace(server.Labels["provider_key"]) == providerKey { + return LeaseTarget{}, exit(4, "lease_id_conflict: unclaimed Proxmox lease %s still owns VM %s", id, server.DisplayID()) + } + } + fmt.Fprintf(b.RT.Stderr, "release provider=proxmox lease=%s reason=cluster_absence_confirmed\n", id) + return LeaseTarget{ + LeaseID: id, + Server: Server{ + Provider: "proxmox", + HostID: proxmoxReleaseAbsentMarker, + Labels: map[string]string{"lease": id, "provider": "proxmox"}, + }, + }, nil +} + func (b *leaseBackend) resolveNumericClaim(cloudID string) (core.LeaseClaim, bool, error) { claims, err := core.ListLeaseClaims() if err != nil { diff --git a/internal/providers/proxmox/fixed_test.go b/internal/providers/proxmox/fixed_test.go index bf9941264..e0e7fa856 100644 --- a/internal/providers/proxmox/fixed_test.go +++ b/internal/providers/proxmox/fixed_test.go @@ -248,6 +248,80 @@ func TestProxmoxFixedConfirmedAbsentAttemptLeavesTerminalTombstone(t *testing.T) } } +func TestProxmoxFixedReleaseOnlyConvergesOnProvenClusterAbsenceWithoutClaim(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + target, err := backend.Resolve(context.Background(), ResolveRequest{ID: req.RequestedLeaseID, ReleaseOnly: true}) + if err != nil { + t.Fatalf("release-only resolve: %v", err) + } + if target.LeaseID != req.RequestedLeaseID { + t.Fatalf("target=%#v, want confirmed-absent target for %s", target, req.RequestedLeaseID) + } + if err := backend.ReleaseLease(context.Background(), ReleaseLeaseRequest{Lease: target}); err != nil { + t.Fatalf("release: %v", err) + } + if client.deleteCalls != 0 || client.mutated { + t.Fatalf("deletes=%d mutated=%t, want no provider mutation", client.deleteCalls, client.mutated) + } + if _, exists, err := core.ReadLeaseClaimWithPresence(req.RequestedLeaseID); err != nil || exists { + t.Fatalf("claim exists=%t err=%v, want no fabricated claim", exists, err) + } +} + +func TestProxmoxFixedReleaseOnlyAbsenceFailsClosed(t *testing.T) { + for _, tc := range []struct { + name string + id string + mutate func(*leaseBackend, *fixedProxmoxClient) + want string + }{ + { + name: "unreadable inventory", + mutate: func(_ *leaseBackend, client *fixedProxmoxClient) { + client.clusterListErr = errors.New("cluster inventory unavailable") + }, + want: "cluster inventory unavailable", + }, + { + name: "reused provider key", + mutate: func(_ *leaseBackend, client *fixedProxmoxClient) { + client.servers = []Server{{ + Provider: "proxmox", CloudID: "417", HostID: "pve1", ID: 417, + Labels: map[string]string{ + "crabbox": "true", "provider": "proxmox", "lease": "cbx_aaaaaaaaaaaa", + "slug": "other", "provider_key": core.ProviderKeyForLease("cbx_123456abcdef"), + }, + }} + }, + want: "lease_id_conflict", + }, + { + name: "unresolved cluster scope", + mutate: func(backend *leaseBackend, _ *fixedProxmoxClient) { backend.Cfg.Proxmox.Node = "" }, + want: "cluster scope", + }, + { + name: "non-canonical identifier", + id: "fixed-proxmox-lookalike", + want: "lease/server not found", + }, + } { + t.Run(tc.name, func(t *testing.T) { + backend, client, req := fixedProxmoxFixture(t) + if tc.mutate != nil { + tc.mutate(backend, client) + } + _, err := backend.Resolve(context.Background(), ResolveRequest{ID: core.Blank(tc.id, req.RequestedLeaseID), ReleaseOnly: true}) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("resolve err=%v, want %q", err, tc.want) + } + if client.deleteCalls != 0 || client.mutated { + t.Fatalf("deletes=%d mutated=%t, want no provider mutation", client.deleteCalls, client.mutated) + } + }) + } +} + func TestProxmoxFixedReleaseLeavesTerminalTombstone(t *testing.T) { backend, client, req := fixedProxmoxFixture(t) lease, err := backend.Acquire(context.Background(), req)