From 11e58b200d27a24cb32d2521c76dd35a1dc2e4b7 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Wed, 10 Jun 2026 13:37:44 +1000 Subject: [PATCH] fix: persist EKS control-plane VM refs before egress alloc to prevent leak (#338) CreateCluster set the control-plane VM/ENI/spread-group refs into the cluster meta but first persisted them to KV only after allocateClusterEgressIP. A failure between placement and that persist left the live sys.medium CP VMs unrecorded, so neither DeleteCluster nor the FAILED-cluster reclaim could reach them and they leaked, reserving host capacity. Persist the meta immediately after placeControlPlane returns. The reclaim path keys off the persisted ControlPlaneNodes + ControlPlaneSpreadGroup, so this also frees reserved spread capacity on teardown. --- spinifex/handlers/eks/create_cluster_test.go | 40 ++++++++++++++++++++ spinifex/handlers/eks/service_impl.go | 16 ++++++-- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/spinifex/handlers/eks/create_cluster_test.go b/spinifex/handlers/eks/create_cluster_test.go index 5d4be8481..9a9d022b3 100644 --- a/spinifex/handlers/eks/create_cluster_test.go +++ b/spinifex/handlers/eks/create_cluster_test.go @@ -101,6 +101,46 @@ func TestCreateCluster_FailedCreateThenDeleteReclaimsNLB(t *testing.T) { assert.ErrorIs(t, getErr, ErrClusterNotFound) } +// A post-placement failure (egress allocation) must leave the just-launched +// control-plane VM refs persisted in the FAILED meta. Otherwise the live +// sys.medium VMs are unrecorded and neither DeleteCluster nor the FAILED-cluster +// reclaim can reach them — they leak. +func TestCreateCluster_EgressFailureLeavesControlPlaneRecorded(t *testing.T) { + f := newEKSServiceFixture(t) + + // Placement succeeds and launches the CP VM, then egress allocation fails. + f.eip.allocateErr = errors.New("no addresses in hidden pool") + + _, err := f.svc.CreateCluster(createInput("alpha"), testAccountID, "") + require.Error(t, err) + require.NotEmpty(t, f.inst.launchCalls, "control-plane VM was launched before the egress step") + + meta, getErr := GetClusterMeta(f.kv, "alpha") + require.NoError(t, getErr, "meta must remain so the leaked CP VM has an owning record") + assert.Equal(t, ClusterStatusFailed, meta.Status) + assert.NotEmpty(t, meta.ControlPlaneInstanceID, "CP instance ID must be persisted before the egress step") + assert.NotEmpty(t, meta.ControlPlaneNodes, "CP node list must be persisted before the egress step") +} + +// End-to-end: a create that fails at egress, then delete-cluster, must terminate +// the control-plane VM the partial create launched — driven by the CP refs the +// early persist recorded. +func TestCreateCluster_EgressFailedThenDeleteTerminatesControlPlane(t *testing.T) { + f := newEKSServiceFixture(t) + f.eip.allocateErr = errors.New("no addresses in hidden pool") + + _, err := f.svc.CreateCluster(createInput("alpha"), testAccountID, "") + require.Error(t, err) + require.NotEmpty(t, f.inst.launchCalls, "create launched a CP VM") + + _, err = f.svc.DeleteCluster(deleteInput("alpha"), testAccountID) + require.NoError(t, err) + + assert.NotEmpty(t, f.inst.terminateCalls, "DeleteCluster must terminate the CP VM recorded by the failed create") + _, getErr := GetClusterMeta(f.kv, "alpha") + assert.ErrorIs(t, getErr, ErrClusterNotFound) +} + // A live (CREATING/ACTIVE) cluster of the same name blocks create with a // cluster-scoped ResourceInUseException, not the ELBv2 target-group message. func TestCreateCluster_ExistingClusterReturnsResourceInUse(t *testing.T) { diff --git a/spinifex/handlers/eks/service_impl.go b/spinifex/handlers/eks/service_impl.go index 983cfde5d..ec4e74a81 100644 --- a/spinifex/handlers/eks/service_impl.go +++ b/spinifex/handlers/eks/service_impl.go @@ -506,6 +506,16 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, meta.ControlPlaneENIIP = primary.ENIIP meta.ControlPlaneMgmtIP = primary.MgmtIP + // Persist the CP VM + ENI + spread-group refs now, before any further fallible + // step. The VMs are live the moment placeControlPlane returns; without this a + // failure between here and the next PutClusterMeta (e.g. egress allocation) + // leaves them launched but unrecorded, so neither DeleteCluster nor the + // FAILED-cluster reclaim could reach them and the sys.medium VMs leak. + if err := PutClusterMeta(acctKV, meta); err != nil { + s.markFailed(acctKV, name) + return nil, logCreateErr(name, accountID, "persist control-plane ids", err) + } + // Egress: the control-plane VM must pull container images. Allocate a // hidden pool address and wire an egress-only SNAT for its /32 (no DNAT — // the VM stays unreachable inbound; the NLB is its only front door). @@ -523,11 +533,11 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, ExternalIp: egressIP, }) - // Record the VM + ENI + egress IP before registering the target so a failed - // register (or anything after) leaves them recoverable by DeleteCluster. + // Record the egress allocation before registering the target so a failed + // register (or anything after) leaves it recoverable by DeleteCluster. if err := PutClusterMeta(acctKV, meta); err != nil { s.markFailed(acctKV, name) - return nil, logCreateErr(name, accountID, "persist control-plane ids", err) + return nil, logCreateErr(name, accountID, "persist egress allocation", err) } cpENIIPs := make([]string, 0, len(cpNodes))