From b23fa3bf6079bde46baec0b914826143f7d26c03 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Fri, 12 Jun 2026 22:29:24 +1000 Subject: [PATCH] feat(eks): managed control-plane VPC (Set B) + control-plane lifecycle hardening (#356) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(eks): VM-side managed-addon delivery transport Stage CreateAddon manifests in KV host-side; the control-plane VM pulls the staged set through the AWS gateway (signed GET /clusters/{name}/internal-addons/ {accountId}), renders the baked bundles into the K3s auto-deploy dir, and reports per-addon delivery status on a dedicated eks.addon.*.status channel that the cluster reconciler CASes onto AWS-visible AddonStatus (CREATING->ACTIVE). - gateway: ListInternalAddons route + ListStagedAddonManifests NATS action - handlers: addon status reconciler (nextAddonStatus), staging installer - eks-node AMI: eks-gateway-fetch binary + mulga-eks-addon-sync agent - agent renders idempotently (cmp before replace) so K3s auto-deploy does not re-apply every tick and clear the Addon CR .status.gvks readiness signal - agent mirrors per-tick delivery outcomes to the serial console for host-side observability * fix(eks): provision cluster NLB synchronously so the endpoint carries the real front-end IP The cluster NLB front-end IP is assigned by the async LB-VM launch (launchLBVMAsync), but EnsureClusterNLB read it back synchronously right after CreateLoadBalancer returned — losing the race and falling back to the synthetic .elb.spinifex.local DNS name. That name is unresolvable (no Route53) and is not in the apiserver serving-cert SAN, so kubectl against the cluster never connects. Split the elbv2 create: the public CreateLoadBalancer stays async (267.4 — an inline multi-minute VM boot head-of-line-blocks the gateway responder), and a new CreateLoadBalancerSync drives the data-plane launch inline and returns the LB carrying its front-end address. The EKS cluster-create worker runs off the gateway responder, so it uses the sync entry and reads the IP from the return value instead of racing persisted state. No polling. - elbv2: factor provisionLBDataPlane out of launchLBVMAsync; add CreateLoadBalancerSync - eks: ensureClusterLB uses the sync entry, sets FrontendIP from the result, drops the read-back; EnsureClusterNLB fails loud (ErrClusterNLBFrontendIPUnavailable) when no front-end IP is allocated rather than shipping the DNS-name fallback - eks: drop the DNS-name endpoint fallback; the endpoint is always the front-end IP * feat: auto-wire IGW for internet-facing EKS cluster endpoint An internet-facing cluster NLB allocates an external front-end IP and the OVN dnat_and_snat for it, but the external switch + localnet + gateway LRP that make that IP answerable on the wire are built only on IGW attach. The control-plane NLB is a system instance, so ensure an IGW on cluster build and reclaim it on teardown. EnsureClusterIGW (idempotent, reuses a customer IGW untouched) runs before EnsureClusterNLB when the endpoint is public; DeleteClusterIGW (ownership- scoped) removes only the cluster-owned IGW in purgeClusterInfra, before the VPC delete. New IGW dep wired from the daemon. * fix(eks): tear down infra on failed cluster launch to stop LB EIP leak A CreateCluster that fails during its background launch flipped the meta to FAILED but performed no teardown, relying on a later same-name create to reclaim and purge the infra. EKS cluster names are unique per create, so that reclaim never fires: the internet-facing load-balancer VM keeps running with its allocated+associated dnat_and_snat EIP indefinitely. Over repeated create/delete cycles the EIP pool fills with allocations bound to terminated instances, so a later allocation's advertised endpoint and its live OVN NAT diverge onto different pool addresses. - failClusterLaunch now runs a best-effort purgeClusterInfra on the in-memory meta (freshest refs even when the failing step was the meta write itself) before MarkClusterFailed. purgeClusterInfra is idempotent, so a later reclaim or DeleteCluster re-purge is a no-op; the FAILED meta is retained for observability. - Daemon releaseSystemInstanceEIP extracted and called from both TerminateSystemInstance and cleanupFailedSystemInstance, so a launch that fails after AssociateAddress releases the eipService allocation instead of leaking it (MarkFailed's ReleasePublicIP only knows the externalIPAM path). * feat: EKS managed control-plane VPC (AWS Set B) composed from EC2 APIs CreateCluster now auto-builds the managed control-plane VPC under the system account, the spinifex analogue of AWS EKS's hidden managed-account VPC: a public subnet for the internet-facing NLB, private subnet(s) for the k3s control-plane VM(s), and a NAT gateway giving the private CP egress for image pulls. It is composed from the real EC2 VPC-family APIs (CreateVpc/Subnet, IGW, RouteTable/Route, NatGateway), so per-subnet egress gating is wired by the existing topology subscribers off the route-table events those APIs publish — no direct OVN mutation. This resolves the mulga-siv-292 root cause: the internet-facing NLB and the private CP were crammed into one customer subnet with no IGW route, so both were treated as private and dropped by the lr_in_policy 1100 rule, killing the LB-VM return leg and lb-agent heartbeat. The CP only survived via a per-instance 1200 reroute bandaid, now removed — the topology expresses egress instead. Placement is uniform single/multi-node (CP always in the private subnet). The system LB VM heartbeats over the mgmt bridge (SystemGatewayURL) rather than the WAN. Managed-VPC teardown reclaims NAT GW + EIP, route tables, subnets, IGW, and the VPC in dependency order. Customer subnetIds (Set A / worker side) remain consumed-not-created and are deferred to the worker round. * fix(eks): bake cluster-owner account into CP-VM EKS_ACCOUNT_ID The managed control-plane VPC (Set B) launches the K3s control-plane VMs and their ENIs under the system (infra) account, but the on-VM agents — bootstrap publish, state report, token-review, and add-on fetch — must key on the cluster-OWNER (customer) account that owns the cluster meta, namespaces, and AccessEntry KV. Add ClusterAccountID to K3sServerInput and bake it into the VM's EKS_ACCOUNT_ID, decoupling it from the infra AccountID the VM launches under. Without this the VM reported state and fetched add-ons against the system account, so the cluster never observed a healthy control plane (stuck CREATING, null CA) and kubectl/add-on delivery timed out. Also refresh the e2e suite doc comment to describe the Set B topology and the data-plane subtests (kubectl + managed-addon delivery) that now run. * fix(eks): explicitly delete addon objects on unstage K3s does not delete the applied resources when an auto-deploy manifest file is removed from the server manifests dir (documented behaviour). The addon-sync GC pass relied on that, so after DeleteAddon the rendered manifest was dropped but its Namespace/ConfigMap stayed standing — the e2e AddonDelivery GC wait timed out. Copy the manifest aside, drop the original so K3s stops tracking it, then kubectl delete the objects (--wait=false to avoid finalizer stalls). --- cmd/eks-gateway-fetch/main.go | 139 +++++ cmd/eks-gateway-fetch/main_test.go | 44 ++ cmd/eks-gateway-publish/main.go | 6 +- internal/eksgw/client.go | 17 +- .../addons/spinifex-noop/0.1.0/noop.yaml | 20 + scripts/images/eks-node/eks-node-role.sh | 6 + scripts/images/eks-node/manifest.conf | 7 +- .../eks-node/mulga-eks-addon-sync.initd | 30 + .../images/eks-node/mulga-eks-addon-sync.sh | 199 +++++++ scripts/images/eks-node/setup.sh | 5 +- spinifex/daemon/daemon.go | 1 + spinifex/daemon/daemon_handlers_eks.go | 4 + spinifex/daemon/daemon_system_instance.go | 75 ++- .../daemon/daemon_system_instance_test.go | 67 +++ spinifex/daemon/eks_deps.go | 4 + spinifex/gateway/eks.go | 9 + spinifex/gateway/eks/internal_addons.go | 36 ++ spinifex/gateway/eks/internal_publish.go | 3 + spinifex/gateway/eks/internal_publish_test.go | 6 + spinifex/gateway/eks_test.go | 1 + spinifex/handlers/eks/addon_catalog.go | 7 + spinifex/handlers/eks/addon_installer.go | 11 +- .../handlers/eks/addon_status_reconciler.go | 78 +++ .../eks/addon_status_reconciler_test.go | 102 ++++ spinifex/handlers/eks/addons.go | 69 +++ spinifex/handlers/eks/addons_test.go | 45 ++ spinifex/handlers/eks/cluster_reconciler.go | 39 +- spinifex/handlers/eks/cluster_state.go | 34 +- spinifex/handlers/eks/cp_vpc.go | 518 ++++++++++++++++++ spinifex/handlers/eks/cp_vpc_test.go | 384 +++++++++++++ spinifex/handlers/eks/create_cluster_test.go | 82 ++- spinifex/handlers/eks/delete_cluster_test.go | 14 +- spinifex/handlers/eks/igw.go | 141 +++++ spinifex/handlers/eks/igw_test.go | 174 ++++++ spinifex/handlers/eks/k3s_server_vm.go | 13 +- spinifex/handlers/eks/k3s_server_vm_test.go | 11 +- spinifex/handlers/eks/nlb.go | 56 +- spinifex/handlers/eks/nlb_test.go | 53 ++ spinifex/handlers/eks/service.go | 4 + spinifex/handlers/eks/service_impl.go | 228 +++++--- spinifex/handlers/eks/service_nats.go | 4 + spinifex/handlers/eks/state_report.go | 46 ++ spinifex/handlers/elbv2/arn_test.go | 23 +- spinifex/handlers/elbv2/service_impl.go | 129 ++++- .../elbv2/service_impl_lb_network_test.go | 55 ++ tests/e2e/eks/eks_test.go | 73 ++- 46 files changed, 2871 insertions(+), 201 deletions(-) create mode 100644 cmd/eks-gateway-fetch/main.go create mode 100644 cmd/eks-gateway-fetch/main_test.go create mode 100644 scripts/images/eks-node/addons/spinifex-noop/0.1.0/noop.yaml create mode 100644 scripts/images/eks-node/mulga-eks-addon-sync.initd create mode 100644 scripts/images/eks-node/mulga-eks-addon-sync.sh create mode 100644 spinifex/gateway/eks/internal_addons.go create mode 100644 spinifex/handlers/eks/addon_status_reconciler.go create mode 100644 spinifex/handlers/eks/addon_status_reconciler_test.go create mode 100644 spinifex/handlers/eks/cp_vpc.go create mode 100644 spinifex/handlers/eks/cp_vpc_test.go create mode 100644 spinifex/handlers/eks/igw.go create mode 100644 spinifex/handlers/eks/igw_test.go diff --git a/cmd/eks-gateway-fetch/main.go b/cmd/eks-gateway-fetch/main.go new file mode 100644 index 000000000..e84465366 --- /dev/null +++ b/cmd/eks-gateway-fetch/main.go @@ -0,0 +1,139 @@ +// eks-gateway-fetch runs inside the EKS K3s control-plane VM and fetches a +// control-plane resource from the host through the AWS gateway, instead of +// dialing core NATS directly. It is the read-side companion to +// eks-gateway-publish. +// +// It SigV4-signs (service "eks") an HTTPS GET to the gateway, retrying with +// backoff until the gateway returns 2xx or the attempt budget is exhausted (so +// a degraded link surfaces as a non-zero exit), then emits the staged add-ons +// as tab-separated lines the on-VM addon-sync shell agent consumes without a +// JSON parser: +// +// \t\t\t +// +// The configuration values are base64-encoded so embedded tabs/newlines cannot +// break the line framing. +// +// Usage: +// +// eks-gateway-fetch -resource addons # GET /clusters/{cluster}/internal-addons/{accountId} +// +// Flags default to environment variables seeded by cloud-init: +// EKS_GATEWAY_URL, EKS_GATEWAY_CA, EKS_ACCESS_KEY, EKS_SECRET_KEY, EKS_REGION, +// EKS_ACCOUNT_ID, EKS_CLUSTER_NAME. +package main + +import ( + "bufio" + "encoding/base64" + "encoding/json" + "flag" + "fmt" + "io" + "log/slog" + "os" + "time" + + "github.com/mulgadc/spinifex/internal/eksgw" + + _ "github.com/mulgadc/spinifex/internal/fipsboot" +) + +// stagedAddon mirrors the gateway's StagedAddonManifest wire shape; duplicated +// here to keep this tiny VM binary free of the handler package's dependency +// graph. +type stagedAddon struct { + AddonName string `json:"addonName"` + AddonVersion string `json:"addonVersion"` + ServiceAccountRoleArn string `json:"serviceAccountRoleArn"` + ConfigurationValues string `json:"configurationValues"` +} + +type internalAddonsResponse struct { + Addons []stagedAddon `json:"addons"` +} + +const ( + maxAttempts = 30 + retryDelay = 5 * time.Second +) + +func main() { + var ( + gatewayURL string + gatewayCA string + accessKey string + secretKey string + region string + accountID string + clusterName string + resource string + ) + + flag.StringVar(&gatewayURL, "gateway", os.Getenv("EKS_GATEWAY_URL"), "Gateway URL (e.g. https://10.15.8.1:9999)") + flag.StringVar(&gatewayCA, "gateway-ca", os.Getenv("EKS_GATEWAY_CA"), "Path to gateway TLS CA PEM (optional; falls back to system trust)") + flag.StringVar(&accessKey, "access-key", os.Getenv("EKS_ACCESS_KEY"), "AWS access key ID") + flag.StringVar(&secretKey, "secret-key", os.Getenv("EKS_SECRET_KEY"), "AWS secret access key") + flag.StringVar(®ion, "region", os.Getenv("EKS_REGION"), "AWS region for SigV4 signing") + flag.StringVar(&accountID, "account-id", os.Getenv("EKS_ACCOUNT_ID"), "Cluster account ID") + flag.StringVar(&clusterName, "cluster", os.Getenv("EKS_CLUSTER_NAME"), "Cluster name") + flag.StringVar(&resource, "resource", "addons", "Resource to fetch: addons") + flag.Parse() + + switch { + case accountID == "": + fatal("--account-id is required (or set EKS_ACCOUNT_ID)") + case clusterName == "": + fatal("--cluster is required (or set EKS_CLUSTER_NAME)") + case resource != "addons": + fatal("--resource must be addons") + } + + client, err := eksgw.New(gatewayURL, gatewayCA, accessKey, secretKey, region) + if err != nil { + fatal(fmt.Sprintf("build gateway client: %v", err)) + } + path := "/clusters/" + clusterName + "/internal-addons/" + accountID + + var lastErr error + for attempt := 1; attempt <= maxAttempts; attempt++ { + body, err := client.Get(path) + if err != nil { + lastErr = err + slog.Warn("eks-gateway-fetch: attempt failed", "resource", resource, "attempt", attempt, "err", err) + if attempt < maxAttempts { + time.Sleep(retryDelay) + } + continue + } + if werr := emitAddonsTSV(os.Stdout, body); werr != nil { + fatal(fmt.Sprintf("render response: %v", werr)) + } + return + } + fatal(fmt.Sprintf("fetch failed after %d attempts: %v", maxAttempts, lastErr)) +} + +// emitAddonsTSV decodes the internal-addons response and writes one TSV line per +// staged add-on to w. An empty add-on set writes nothing (the agent then GCs +// every locally-rendered manifest), which is the correct steady state for a +// cluster with no managed add-ons. +func emitAddonsTSV(out io.Writer, body []byte) error { + var resp internalAddonsResponse + if err := json.Unmarshal(body, &resp); err != nil { + return fmt.Errorf("unmarshal internal-addons response: %w", err) + } + w := bufio.NewWriter(out) + for _, a := range resp.Addons { + cfg := base64.StdEncoding.EncodeToString([]byte(a.ConfigurationValues)) + if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", a.AddonName, a.AddonVersion, a.ServiceAccountRoleArn, cfg); err != nil { + return err + } + } + return w.Flush() +} + +func fatal(msg string) { + slog.Error("eks-gateway-fetch: " + msg) + os.Exit(1) +} diff --git a/cmd/eks-gateway-fetch/main_test.go b/cmd/eks-gateway-fetch/main_test.go new file mode 100644 index 000000000..e54b8b00f --- /dev/null +++ b/cmd/eks-gateway-fetch/main_test.go @@ -0,0 +1,44 @@ +package main + +import ( + "bytes" + "encoding/base64" + "testing" +) + +func TestEmitAddonsTSV(t *testing.T) { + body := []byte(`{"addons":[ + {"addonName":"spinifex-noop","addonVersion":"0.1.0"}, + {"addonName":"aws-load-balancer-controller","addonVersion":"2.8.1", + "serviceAccountRoleArn":"arn:aws:iam::111122223333:role/alb", + "configurationValues":"{\"replicaCount\":2}"} + ]}`) + + var buf bytes.Buffer + if err := emitAddonsTSV(&buf, body); err != nil { + t.Fatalf("emitAddonsTSV: %v", err) + } + cfg := base64.StdEncoding.EncodeToString([]byte(`{"replicaCount":2}`)) + want := "spinifex-noop\t0.1.0\t\t\n" + + "aws-load-balancer-controller\t2.8.1\tarn:aws:iam::111122223333:role/alb\t" + cfg + "\n" + if buf.String() != want { + t.Fatalf("TSV mismatch:\n got: %q\nwant: %q", buf.String(), want) + } +} + +func TestEmitAddonsTSV_Empty(t *testing.T) { + var buf bytes.Buffer + if err := emitAddonsTSV(&buf, []byte(`{"addons":[]}`)); err != nil { + t.Fatalf("emitAddonsTSV: %v", err) + } + if buf.Len() != 0 { + t.Fatalf("empty addon set must emit nothing, got %q", buf.String()) + } +} + +func TestEmitAddonsTSV_BadJSON(t *testing.T) { + var buf bytes.Buffer + if err := emitAddonsTSV(&buf, []byte(`{not-json`)); err == nil { + t.Fatal("expected error on malformed JSON") + } +} diff --git a/cmd/eks-gateway-publish/main.go b/cmd/eks-gateway-publish/main.go index 5cd33a7b2..edec38773 100644 --- a/cmd/eks-gateway-publish/main.go +++ b/cmd/eks-gateway-publish/main.go @@ -69,7 +69,7 @@ func main() { flag.StringVar(®ion, "region", os.Getenv("EKS_REGION"), "AWS region for SigV4 signing") flag.StringVar(&accountID, "account-id", os.Getenv("EKS_ACCOUNT_ID"), "Cluster account ID") flag.StringVar(&clusterName, "cluster", os.Getenv("EKS_CLUSTER_NAME"), "Cluster name") - flag.StringVar(&channel, "channel", "", "Publish channel: bootstrap|state") + flag.StringVar(&channel, "channel", "", "Publish channel: bootstrap|state|addon") flag.StringVar(&kind, "kind", "", "Bootstrap subject kind (bootstrap channel only)") flag.Parse() @@ -78,8 +78,8 @@ func main() { fatal("--account-id is required (or set EKS_ACCOUNT_ID)") case clusterName == "": fatal("--cluster is required (or set EKS_CLUSTER_NAME)") - case channel != "bootstrap" && channel != "state": - fatal("--channel must be bootstrap or state") + case channel != "bootstrap" && channel != "state" && channel != "addon": + fatal("--channel must be bootstrap, state or addon") case channel == "bootstrap" && kind == "": fatal("--kind is required for the bootstrap channel") } diff --git a/internal/eksgw/client.go b/internal/eksgw/client.go index 11b6845cb..328a1987a 100644 --- a/internal/eksgw/client.go +++ b/internal/eksgw/client.go @@ -74,11 +74,24 @@ func New(baseURL, caPath, accessKey, secretKey, region string) (*Client, error) // Post SigV4-signs and sends body to path, returning the response body on 2xx. // Errors include the gateway status and body. No retry; callers wrap as needed. func (c *Client) Post(path string, body []byte) ([]byte, error) { - req, err := http.NewRequest(http.MethodPost, c.baseURL+path, bytes.NewReader(body)) + return c.do(http.MethodPost, path, body) +} + +// Get SigV4-signs and sends a single GET to path (e.g. +// "/clusters/alpha/internal-addons?accountId=..."). Same contract as Post: 2xx +// body or an error carrying the gateway status. No retry. +func (c *Client) Get(path string) ([]byte, error) { + return c.do(http.MethodGet, path, nil) +} + +func (c *Client) do(method, path string, body []byte) ([]byte, error) { + req, err := http.NewRequest(method, c.baseURL+path, bytes.NewReader(body)) if err != nil { return nil, fmt.Errorf("create request: %w", err) } - req.Header.Set("Content-Type", "application/x-amz-json-1.1") + if body != nil { + req.Header.Set("Content-Type", "application/x-amz-json-1.1") + } sum := sha256.Sum256(body) if err := auth.SignReq(req, c.accessKey, c.secretKey, hex.EncodeToString(sum[:]), "eks", c.region); err != nil { diff --git a/scripts/images/eks-node/addons/spinifex-noop/0.1.0/noop.yaml b/scripts/images/eks-node/addons/spinifex-noop/0.1.0/noop.yaml new file mode 100644 index 000000000..a150f3e32 --- /dev/null +++ b/scripts/images/eks-node/addons/spinifex-noop/0.1.0/noop.yaml @@ -0,0 +1,20 @@ +# spinifex-noop — managed-addon delivery-transport fixture. Two trivial, +# dependency-free objects the addon e2e asserts on to prove the stage → fetch → +# render → auto-deploy → ACTIVE → delete round-trip end-to-end, independent of +# the CSI or load-balancer-controller bundles. Not a real workload. +apiVersion: v1 +kind: Namespace +metadata: + name: spinifex-noop + labels: + app.kubernetes.io/managed-by: spinifex-addon +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: spinifex-noop + namespace: spinifex-noop + labels: + app.kubernetes.io/managed-by: spinifex-addon +data: + marker: "spinifex-noop addon delivered" diff --git a/scripts/images/eks-node/eks-node-role.sh b/scripts/images/eks-node/eks-node-role.sh index f74c82c26..9b314efad 100755 --- a/scripts/images/eks-node/eks-node-role.sh +++ b/scripts/images/eks-node/eks-node-role.sh @@ -55,10 +55,16 @@ case "${ROLE}" in rc-update add k3s default rc-update add k3s-first-boot default rc-update add mulga-eks-state-report default + # Managed-addon delivery runs only on the primary server: it renders + # staged addon bundles into the K3s auto-deploy dir, which a single + # writer owns. HA multi-server addon delivery is tracked separately + # (mulga-siv-231.7); server-join nodes do not run it. + rc-update add mulga-eks-addon-sync default rc-service eks-token-webhook start rc-service k3s start rc-service k3s-first-boot start rc-service mulga-eks-state-report start + rc-service mulga-eks-addon-sync start ;; server-join) log "configuring server-join role" diff --git a/scripts/images/eks-node/manifest.conf b/scripts/images/eks-node/manifest.conf index 317270b3c..5e82f6e35 100644 --- a/scripts/images/eks-node/manifest.conf +++ b/scripts/images/eks-node/manifest.conf @@ -18,9 +18,9 @@ APK_PACKAGES="ca-certificates curl jq iptables ip6tables conntrack-tools cgroup- # eks-gateway-publish relays the bootstrap envelopes + state reports to the AWS # gateway over SigV4 HTTPS (server role only). The k3s binary is downloaded + # SHA256-verified by setup.sh (network in chroot). -BUILD_COMMANDS="CGO_ENABLED=0 GOFIPS140=v1.0.0 go build -o bin/eks-token-webhook ./cmd/eks-token-webhook && CGO_ENABLED=0 GOFIPS140=v1.0.0 go build -o bin/eks-gateway-publish ./cmd/eks-gateway-publish" +BUILD_COMMANDS="CGO_ENABLED=0 GOFIPS140=v1.0.0 go build -o bin/eks-token-webhook ./cmd/eks-token-webhook && CGO_ENABLED=0 GOFIPS140=v1.0.0 go build -o bin/eks-gateway-publish ./cmd/eks-gateway-publish && CGO_ENABLED=0 GOFIPS140=v1.0.0 go build -o bin/eks-gateway-fetch ./cmd/eks-gateway-fetch" -INSTALL_BINARIES="bin/eks-token-webhook:/usr/local/bin/eks-token-webhook bin/eks-gateway-publish:/usr/local/bin/eks-gateway-publish" +INSTALL_BINARIES="bin/eks-token-webhook:/usr/local/bin/eks-token-webhook bin/eks-gateway-publish:/usr/local/bin/eks-gateway-publish bin/eks-gateway-fetch:/usr/local/bin/eks-gateway-fetch" # All role services ship; none but the selector are enabled at bake time. # eks-node-role picks server vs agent at first boot and enables the rest. @@ -34,6 +34,9 @@ scripts/images/eks-node/k3s-first-boot.initd:/etc/init.d/k3s-first-boot \ scripts/images/eks-node/k3s-first-boot.sh:/usr/local/sbin/k3s-first-boot \ scripts/images/eks-node/mulga-eks-state-report.initd:/etc/init.d/mulga-eks-state-report \ scripts/images/eks-node/mulga-eks-state-report.sh:/usr/local/sbin/mulga-eks-state-report \ +scripts/images/eks-node/mulga-eks-addon-sync.initd:/etc/init.d/mulga-eks-addon-sync \ +scripts/images/eks-node/mulga-eks-addon-sync.sh:/usr/local/sbin/mulga-eks-addon-sync \ +scripts/images/eks-node/addons/spinifex-noop/0.1.0/noop.yaml:/usr/share/spinifex-eks/addons/spinifex-noop/0.1.0/noop.yaml \ scripts/images/eks-node/mulga-eks-etcd-snapshot.sh:/etc/periodic/daily/mulga-eks-etcd-snapshot" # Only crond (etcd snapshot) + the role selector at bake. The selector adds the diff --git a/scripts/images/eks-node/mulga-eks-addon-sync.initd b/scripts/images/eks-node/mulga-eks-addon-sync.initd new file mode 100644 index 000000000..bc6de91d1 --- /dev/null +++ b/scripts/images/eks-node/mulga-eks-addon-sync.initd @@ -0,0 +1,30 @@ +#!/sbin/openrc-run +# mulga-eks-addon-sync — server-role service that renders Spinifex managed-addon +# bundles into the K3s auto-deploy dir and reports per-addon delivery status to +# the spinifex control plane every ADDON_SYNC_INTERVAL seconds. The host-side +# daemon stages each addon in KV; the VM pulls the staged set through the AWS +# gateway (it cannot read NATS/KV directly) and renders the baked bundles. +# Enabled on the primary server role by eks-node-role.sh. + +description="Mulga EKS managed-addon delivery agent" +command="/usr/local/sbin/mulga-eks-addon-sync" +command_background="yes" +pidfile="/run/mulga-eks-addon-sync.pid" +output_log="/var/log/mulga-eks-addon-sync.log" +error_log="/var/log/mulga-eks-addon-sync.log" + +# Cadence the sync loop sleeps between pulls. 30s keeps CreateAddon→ACTIVE +# latency low without hammering the gateway. +export ADDON_SYNC_INTERVAL="${ADDON_SYNC_INTERVAL:-30}" + +depend() { + need net + after k3s +} + +start_pre() { + if [ ! -f /etc/spinifex-eks/first-boot.env ]; then + eerror "/etc/spinifex-eks/first-boot.env not present — cloud-init did not seed gateway creds" + return 1 + fi +} diff --git a/scripts/images/eks-node/mulga-eks-addon-sync.sh b/scripts/images/eks-node/mulga-eks-addon-sync.sh new file mode 100644 index 000000000..9a6d2b69f --- /dev/null +++ b/scripts/images/eks-node/mulga-eks-addon-sync.sh @@ -0,0 +1,199 @@ +#!/bin/sh +set -eu + +# mulga-eks-addon-sync — renders Spinifex managed-addon bundles into the K3s +# auto-deploy dir and reports per-addon delivery status back to the spinifex +# control plane. The host-side daemon stages each CreateAddon as a manifest +# descriptor in KV; the VM cannot read KV directly, so this agent pulls the +# staged set through the AWS gateway (eks-gateway-fetch, SigV4) and renders the +# baked bundle for each one. Status flows back via eks-gateway-publish on the +# "addon" channel, where the reconciler CASes the AddonRecord CREATING→ACTIVE. +# +# Pull (host→VM): GET /clusters/{cluster}/internal-addons/{accountId} +# Report (VM→host): eks.addon.{accountID}.{clusterName}.status +# +# Runs server-role only (it owns the single auto-deploy dir); enabled by +# eks-node-role.sh. Loops every ADDON_SYNC_INTERVAL seconds in service mode, +# one-shot otherwise. + +ENVFILE=/etc/spinifex-eks/first-boot.env +[ -f "${ENVFILE}" ] || { logger -t mulga-eks-addon-sync "${ENVFILE} missing — exiting"; exit 0; } +# set -a so the sourced creds are exported to the eks-gateway-{fetch,publish} +# children, which read EKS_ACCOUNT_ID etc. from their environment. +set -a +# shellcheck disable=SC1090 +. "${ENVFILE}" +set +a + +: "${EKS_GATEWAY_URL:?}" +: "${EKS_ACCESS_KEY:?}" +: "${EKS_SECRET_KEY:?}" +: "${EKS_ACCOUNT_ID:?}" +: "${EKS_CLUSTER_NAME:?}" + +KUBECONFIG=${KUBECONFIG:-/etc/rancher/k3s/k3s.yaml} +export KUBECONFIG + +# Baked bundles live at BAKED_DIR///*.yaml (air-gapped, shipped +# in the AMI). Rendered output lands in the K3s auto-deploy dir under a stable +# per-addon filename so GC can find orphans this agent owns. +BAKED_DIR=/usr/share/spinifex-eks/addons +DEPLOY_DIR=/var/lib/rancher/k3s/server/manifests +RENDER_PREFIX=spinifex-addon- + +# log mirrors to syslog (on-VM /var/log) and, best-effort, to the serial +# console — the host captures ttyS0 to a per-instance log, so this is the only +# delivery diagnostics channel reachable from the host (the VM has no +# host-routable address). Keep messages terse; one line per sync outcome. +log() { + logger -t mulga-eks-addon-sync "$*" + echo "[mulga-eks-addon-sync] $*" > /dev/console 2>/dev/null || true +} + +# report PHASE ADDON VERSION [MESSAGE] — publish one addon status report. +report() { + _phase=$1; _addon=$2; _version=$3; _msg=${4:-} + printf '{"addon":"%s","version":"%s","phase":"%s","message":"%s","ts":%s}' \ + "${_addon}" "${_version}" "${_phase}" "${_msg}" "$(date +%s)" \ + | eks-gateway-publish -channel addon 2>&1 | logger -t mulga-eks-addon-sync || true +} + +# render_addon ADDON VERSION ROLE_ARN — render the baked bundle into the +# auto-deploy dir, substituting the IRSA role ARN. Returns non-zero (and reports +# failed) when the baked bundle for ADDON/VERSION is absent. +render_addon() { + _addon=$1; _version=$2; _role=$3 + _src="${BAKED_DIR}/${_addon}/${_version}" + _dst="${DEPLOY_DIR}/${RENDER_PREFIX}${_addon}.yaml" + + if [ ! -d "${_src}" ]; then + log "no baked bundle for ${_addon}/${_version} (looked in ${_src})" + report failed "${_addon}" "${_version}" "no baked bundle for version ${_version}" + return 1 + fi + + _tmp="${_dst}.tmp.$$" + : > "${_tmp}" + for _f in "${_src}"/*.yaml; do + [ -e "${_f}" ] || continue + sed -e "s|{{SERVICE_ACCOUNT_ROLE_ARN}}|${_role}|g" "${_f}" >> "${_tmp}" + printf '\n---\n' >> "${_tmp}" + done + # Idempotent: only replace the auto-deploy file when the rendered content + # actually changed. An unconditional mv bumps the file mtime every tick, + # which K3s' auto-deploy controller treats as a change and re-applies — + # clearing the Addon CR's .status.gvks each time, so the same-tick readiness + # check never observes a settled apply and the add-on stays CREATING forever. + if [ -e "${_dst}" ] && cmp -s "${_tmp}" "${_dst}"; then + rm -f "${_tmp}" + return 0 + fi + mv -f "${_tmp}" "${_dst}" + return 0 +} + +# addon_gvks ADDON — echoes the GVKs K3s' auto-deploy controller applied for the +# rendered manifest, empty until it has applied the objects. A non-empty value +# means delivery succeeded. Addon-agnostic — confirms delivery, not deep per-pod +# health (a later hardening item). +# +# K3s (v1.32.x) records the applied GVKs in the addon.k3s.cattle.io/gvks +# *annotation* (e.g. "/v1, Kind=Namespace;/v1, Kind=ConfigMap"); the Addon CR +# carries no .status subresource. The CR is matched by .spec.source (the +# absolute path of the rendered file) rather than a guessed CR name, since K3s +# derives the name from the path with its own transform. +addon_gvks() { + _want="${DEPLOY_DIR}/${RENDER_PREFIX}$1.yaml" + kubectl -n kube-system get addon -o json 2>/dev/null \ + | jq -r --arg src "${_want}" \ + '.items[] | select(.spec.source==$src) + | .metadata.annotations["addon.k3s.cattle.io/gvks"] // ""' 2>/dev/null \ + | grep -v '^$' | head -1 || true +} + +# diag_addon ADDON — one-shot console dump of the live K3s state for a stuck +# add-on, so a delivery that never reaches ready is diagnosable from the +# host-captured serial console (the VM is otherwise unreachable). Fires once per +# process via a sentinel to avoid spamming every tick. +diag_addon() { + [ -f /tmp/.addon-diag-"$1" ] && return 0 + : > /tmp/.addon-diag-"$1" + { + echo "----- addon-diag ${1} -----" + echo "[deploy dir]"; ls -l "${DEPLOY_DIR}" 2>&1 + echo "[rendered file head]"; head -40 "${DEPLOY_DIR}/${RENDER_PREFIX}${1}.yaml" 2>&1 + echo "[all addon CRs]"; kubectl get addon -A 2>&1 + echo "[addon CR ${RENDER_PREFIX}${1} -o yaml]"; kubectl -n kube-system get addon "${RENDER_PREFIX}${1}" -o yaml 2>&1 + echo "[namespace ${1}]"; kubectl get ns "${1}" 2>&1 + echo "[configmaps in ${1}]"; kubectl -n "${1}" get cm 2>&1 + echo "----- end addon-diag ${1} -----" + } > /dev/console 2>&1 || true +} + +sync_once() { + _staged=$(mktemp) + _ferr=$(mktemp) + if ! eks-gateway-fetch -resource addons > "${_staged}" 2>"${_ferr}"; then + log "fetch staged addons failed — keeping current state: $(tr '\n' ' ' < "${_ferr}")" + rm -f "${_staged}" "${_ferr}" + return 0 + fi + rm -f "${_ferr}" + + # Names currently staged, for the GC pass below. + _names=$(cut -f1 "${_staged}") + _count=$(grep -c . "${_staged}" 2>/dev/null || echo 0) + log "fetch ok: ${_count} staged addon(s)" + + # Render/refresh each staged addon and report its phase. + while IFS=$(printf '\t') read -r _addon _version _role _cfg_b64; do + [ -n "${_addon}" ] || continue + : "${_cfg_b64:-}" # config values reserved for per-addon templating (follow-up) + if render_addon "${_addon}" "${_version}" "${_role}"; then + _gvks=$(addon_gvks "${_addon}") + if [ -n "${_gvks}" ]; then + log "addon ${_addon}/${_version}: applied + ready (gvks=${_gvks})" + report ready "${_addon}" "${_version}" + else + log "addon ${_addon}/${_version}: applied, not ready yet (no K3s Addon CR for ${RENDER_PREFIX}${_addon}.yaml with populated .status.gvks)" + diag_addon "${_addon}" + report applied "${_addon}" "${_version}" + fi + fi + done < "${_staged}" + rm -f "${_staged}" + + # GC: remove rendered manifests this agent owns whose addon is no longer + # staged (DeleteAddon removed its descriptor). K3s does NOT delete the applied + # objects when the manifest file disappears, so delete them explicitly. + for _file in "${DEPLOY_DIR}/${RENDER_PREFIX}"*.yaml; do + [ -e "${_file}" ] || continue + _base=$(basename "${_file}") + _name=${_base#"${RENDER_PREFIX}"} + _name=${_name%.yaml} + if ! printf '%s\n' "${_names}" | grep -qx "${_name}"; then + log "unstaging addon ${_name} (no longer staged)" + # Copy the manifest aside, drop the original so K3s stops tracking it + # (and never re-applies), then delete the objects it rendered. + # --wait=false so namespace finalizers don't stall the sync loop. + _gc=$(mktemp) + cp "${_file}" "${_gc}" + rm -f "${_file}" + kubectl delete -f "${_gc}" --ignore-not-found --wait=false 2>&1 \ + | logger -t mulga-eks-addon-sync || true + rm -f "${_gc}" + fi + done +} + +mkdir -p "${DEPLOY_DIR}" + +if [ -n "${ADDON_SYNC_INTERVAL:-}" ] && [ "${ADDON_SYNC_INTERVAL}" -gt 0 ] 2>/dev/null; then + log "starting: cluster=${EKS_CLUSTER_NAME} account=${EKS_ACCOUNT_ID} interval=${ADDON_SYNC_INTERVAL}s gateway=${EKS_GATEWAY_URL}" + while true; do + sync_once + sleep "${ADDON_SYNC_INTERVAL}" + done +else + sync_once +fi diff --git a/scripts/images/eks-node/setup.sh b/scripts/images/eks-node/setup.sh index 2e0896acb..8a2460e11 100755 --- a/scripts/images/eks-node/setup.sh +++ b/scripts/images/eks-node/setup.sh @@ -43,9 +43,10 @@ ln -sf /usr/local/bin/k3s /usr/local/bin/ctr # Init scripts ship as 0644 from INSTALL_FILES; OpenRC requires 0755. Every # role's services are baked; the selector enables the right ones at first boot. chmod 0755 /etc/init.d/eks-node-role /etc/init.d/k3s /etc/init.d/k3s-agent \ - /etc/init.d/eks-token-webhook /etc/init.d/k3s-first-boot /etc/init.d/mulga-eks-state-report + /etc/init.d/eks-token-webhook /etc/init.d/k3s-first-boot /etc/init.d/mulga-eks-state-report \ + /etc/init.d/mulga-eks-addon-sync chmod 0755 /usr/local/sbin/eks-node-role /usr/local/sbin/k3s-first-boot \ - /usr/local/sbin/mulga-eks-state-report + /usr/local/sbin/mulga-eks-state-report /usr/local/sbin/mulga-eks-addon-sync chmod 0755 /etc/periodic/daily/mulga-eks-etcd-snapshot # K3s server config — skeleton; cloud-init / first-boot fills in the diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index 7f560ae17..083089a9d 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -918,6 +918,7 @@ func (d *Daemon) subscribeAll() error { natsSub{"eks.DeleteAddon", d.handleEKSDeleteAddon, "spinifex-workers"}, natsSub{"eks.DescribeAddon", d.handleEKSDescribeAddon, "spinifex-workers"}, natsSub{"eks.UpdateAddon", d.handleEKSUpdateAddon, "spinifex-workers"}, + natsSub{"eks.ListStagedAddonManifests", d.handleEKSListStagedAddonManifests, "spinifex-workers"}, natsSub{"eks.AssociateIdentityProviderConfig", d.handleEKSAssociateIdentityProviderConfig, "spinifex-workers"}, natsSub{"eks.DescribeIdentityProviderConfig", d.handleEKSDescribeIdentityProviderConfig, "spinifex-workers"}, natsSub{"eks.ListIdentityProviderConfigs", d.handleEKSListIdentityProviderConfigs, "spinifex-workers"}, diff --git a/spinifex/daemon/daemon_handlers_eks.go b/spinifex/daemon/daemon_handlers_eks.go index ada154367..3c98f31a5 100644 --- a/spinifex/daemon/daemon_handlers_eks.go +++ b/spinifex/daemon/daemon_handlers_eks.go @@ -118,6 +118,10 @@ func (d *Daemon) handleEKSUpdateAddon(msg *nats.Msg) { handleNATSRequest(msg, d.eksService.UpdateAddon) } +func (d *Daemon) handleEKSListStagedAddonManifests(msg *nats.Msg) { + handleNATSRequest(msg, d.eksService.ListStagedAddonManifests) +} + // --- OIDC identity-provider configs --- func (d *Daemon) handleEKSAssociateIdentityProviderConfig(msg *nats.Msg) { diff --git a/spinifex/daemon/daemon_system_instance.go b/spinifex/daemon/daemon_system_instance.go index bc4c47485..3fb53dfce 100644 --- a/spinifex/daemon/daemon_system_instance.go +++ b/spinifex/daemon/daemon_system_instance.go @@ -497,32 +497,8 @@ func (d *Daemon) TerminateSystemInstance(instanceID string) error { } // Release EIP through the EIP service for system VMs whose public IP was - // allocated via AllocateAddress (internet-facing ALBs). Clears the fields - // so vm.Manager.Terminate's ReleasePublicIP doesn't double-release the - // same IP via externalIPAM. - if instance.PublicIPAllocID != "" && d.eipService != nil { - eniAccount := instance.AccountID - if instance.PublicIPAssocID != "" { - if _, err := d.eipService.DisassociateAddress(&ec2.DisassociateAddressInput{ - AssociationId: aws.String(instance.PublicIPAssocID), - }, eniAccount); err != nil { - slog.Warn("TerminateSystemInstance: DisassociateAddress failed", "instanceId", instanceID, "associationId", instance.PublicIPAssocID, "err", err) - } - } - if _, err := d.eipService.ReleaseAddress(&ec2.ReleaseAddressInput{ - AllocationId: aws.String(instance.PublicIPAllocID), - }, eniAccount); err != nil { - slog.Warn("TerminateSystemInstance: ReleaseAddress failed", "instanceId", instanceID, "allocationId", instance.PublicIPAllocID, "err", err) - } else { - slog.Info("TerminateSystemInstance: released EIP", "instanceId", instanceID, "ip", instance.PublicIP, "allocationId", instance.PublicIPAllocID) - } - d.vmMgr.UpdateState(instance.ID, func(v *vm.VM) { - v.PublicIP = "" - v.PublicIPPool = "" - v.PublicIPAllocID = "" - v.PublicIPAssocID = "" - }) - } + // allocated via AllocateAddress (internet-facing ALBs). + d.releaseSystemInstanceEIP(instance) if err := d.vmMgr.Terminate(instanceID); err != nil { slog.Error("TerminateSystemInstance: vmMgr.Terminate failed", "instanceId", instanceID, "err", err) @@ -564,12 +540,51 @@ func (d *Daemon) refreshSystemInstanceState(inst *vm.VM) error { return nil } +// releaseSystemInstanceEIP disassociates and releases an eipService-allocated +// EIP (internet-facing system instances) and clears the instance's EIP fields so +// a later externalIPAM release path (vm.Manager.Terminate/MarkFailed +// ReleasePublicIP) does not double-release the same IP. No-op when the instance +// holds no eipService allocation. Best-effort: each step is logged, none fatal. +func (d *Daemon) releaseSystemInstanceEIP(instance *vm.VM) { + if instance == nil || instance.PublicIPAllocID == "" || d.eipService == nil { + return + } + eniAccount := instance.AccountID + if instance.PublicIPAssocID != "" { + if _, err := d.eipService.DisassociateAddress(&ec2.DisassociateAddressInput{ + AssociationId: aws.String(instance.PublicIPAssocID), + }, eniAccount); err != nil { + slog.Warn("releaseSystemInstanceEIP: DisassociateAddress failed", "instanceId", instance.ID, "associationId", instance.PublicIPAssocID, "err", err) + } + } + if _, err := d.eipService.ReleaseAddress(&ec2.ReleaseAddressInput{ + AllocationId: aws.String(instance.PublicIPAllocID), + }, eniAccount); err != nil { + slog.Warn("releaseSystemInstanceEIP: ReleaseAddress failed", "instanceId", instance.ID, "allocationId", instance.PublicIPAllocID, "err", err) + } else { + slog.Info("releaseSystemInstanceEIP: released EIP", "instanceId", instance.ID, "ip", instance.PublicIP, "allocationId", instance.PublicIPAllocID) + } + instance.PublicIP = "" + instance.PublicIPPool = "" + instance.PublicIPAllocID = "" + instance.PublicIPAssocID = "" + d.vmMgr.UpdateState(instance.ID, func(v *vm.VM) { + v.PublicIP = "" + v.PublicIPPool = "" + v.PublicIPAllocID = "" + v.PublicIPAssocID = "" + }) +} + // cleanupFailedSystemInstance handles cleanup when a system instance launch -// fails after partial setup (state added, volumes created, etc). Delegates -// to vm.Manager.MarkFailed which runs the synchronous teardown chain -// (volume unmount/delete, tap cleanup, ENI delete, IP release, resource -// deallocation, state migration to terminated KV). +// fails after partial setup (state added, volumes created, etc). Releases an +// eipService-allocated EIP first (MarkFailed's ReleasePublicIP only knows the +// externalIPAM path, so an associated EIP would otherwise leak), then delegates +// to vm.Manager.MarkFailed which runs the synchronous teardown chain (volume +// unmount/delete, tap cleanup, ENI delete, IP release, resource deallocation, +// state migration to terminated KV). func (d *Daemon) cleanupFailedSystemInstance(instance *vm.VM, _ *ec2.InstanceTypeInfo) { + d.releaseSystemInstanceEIP(instance) d.vmMgr.MarkFailed(instance, "system_instance_launch_failed") } diff --git a/spinifex/daemon/daemon_system_instance_test.go b/spinifex/daemon/daemon_system_instance_test.go index c9e5893d2..3b140ae55 100644 --- a/spinifex/daemon/daemon_system_instance_test.go +++ b/spinifex/daemon/daemon_system_instance_test.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + handlers_ec2_eip "github.com/mulgadc/spinifex/spinifex/handlers/ec2/eip" handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc" "github.com/mulgadc/spinifex/spinifex/handlers/elbv2" "github.com/mulgadc/spinifex/spinifex/tags" @@ -579,3 +580,69 @@ func TestLaunchSystemInstance_NATFailureRollsBackPublicIP(t *testing.T) { t.Fatal("rollback must publish vpc.delete-nat to neutralise a half-committed rule") } } + +// releaseSystemInstanceEIP must release an eipService-allocated address back to +// its pool and clear the instance's EIP fields. The vm.Manager teardown +// (Terminate/MarkFailed → ReleasePublicIP) only knows the externalIPAM path, so +// without this an internet-facing system VM's allocated+associated EIP would +// leak when its instance is torn down (mulga-siv-293). +func TestReleaseSystemInstanceEIP_ReleasesEipServiceAllocation(t *testing.T) { + d := createVPCTestDaemon(t) + + jsNS, err := server.NewServer(&server.Options{ + Host: "127.0.0.1", + Port: -1, + JetStream: true, + StoreDir: t.TempDir(), + NoLog: true, + NoSigs: true, + }) + require.NoError(t, err) + go jsNS.Start() + require.True(t, jsNS.ReadyForConnections(5*time.Second)) + t.Cleanup(func() { jsNS.Shutdown() }) + + jsNC, err := nats.Connect(jsNS.ClientURL()) + require.NoError(t, err) + t.Cleanup(func() { jsNC.Close() }) + + js, err := jsNC.JetStream() + require.NoError(t, err) + ipam, err := handlers_ec2_vpc.NewExternalIPAM(js, []handlers_ec2_vpc.ExternalPoolConfig{ + {Name: "wan-test", RangeStart: "203.0.113.10", RangeEnd: "203.0.113.20", Gateway: "203.0.113.1", PrefixLen: 24}, + }) + require.NoError(t, err) + d.externalIPAM = ipam + + eipSvc, err := handlers_ec2_eip.NewEIPServiceImpl(jsNC, ipam, d.vpcService) + require.NoError(t, err) + d.eipService = eipSvc + + // Allocate an EIP the way an internet-facing LB VM launch does, then attach + // it to a registered system instance. + alloc, err := eipSvc.AllocateAddress(&ec2.AllocateAddressInput{}, testAccountID) + require.NoError(t, err) + allocID := aws.StringValue(alloc.AllocationId) + publicIP := aws.StringValue(alloc.PublicIp) + require.NotEmpty(t, allocID) + require.NotEmpty(t, publicIP) + + inst := &vm.VM{ID: "i-lbvm", AccountID: testAccountID, PublicIP: publicIP, PublicIPAllocID: allocID} + d.vmMgr.Insert(inst) + + rec, err := ipam.GetPoolRecord("wan-test") + require.NoError(t, err) + _, allocated := rec.Allocated[publicIP] + require.True(t, allocated, "EIP is held by the pool before release") + + d.releaseSystemInstanceEIP(inst) + + rec, err = ipam.GetPoolRecord("wan-test") + require.NoError(t, err) + _, stillAllocated := rec.Allocated[publicIP] + assert.False(t, stillAllocated, "release must return the EIP to the pool") + + assert.Empty(t, inst.PublicIP, "instance public IP must be cleared") + assert.Empty(t, inst.PublicIPAllocID, "instance alloc ID must be cleared so externalIPAM teardown does not double-release") + assert.Empty(t, inst.PublicIPAssocID) +} diff --git a/spinifex/daemon/eks_deps.go b/spinifex/daemon/eks_deps.go index 75706a345..6773ff3e5 100644 --- a/spinifex/daemon/eks_deps.go +++ b/spinifex/daemon/eks_deps.go @@ -49,7 +49,11 @@ func (d *Daemon) buildEKSServiceDeps() handlers_eks.EKSServiceDeps { Instance: d, Image: d.imageService, EIP: d.eipService, + IGW: d.igwService, Worker: d, + VPCMgr: d.vpcService, + NATGW: d.natGatewayService, + RouteTable: d.routeTableService, PlacementGroup: handlers_ec2_placementgroup.NewNATSPlacementGroupService(d.natsConn), Scheduler: handlers_eks.NewNATSHostScheduler(d.natsConn), } diff --git a/spinifex/gateway/eks.go b/spinifex/gateway/eks.go index 98493bea4..6447365b6 100644 --- a/spinifex/gateway/eks.go +++ b/spinifex/gateway/eks.go @@ -63,6 +63,15 @@ var eksRoutes = []eksRoute{ func(gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { return gateway_eks.WebhookTokenReview(gw.NATSConn, p[0], b) }}, + // Control-plane VM add-on delivery: the on-VM addon-sync agent GETs the set + // of staged add-on manifests for its cluster (system SigV4 creds) to render + // the baked bundles into the K3s auto-deploy dir. acct (system account) is + // ignored — the cluster account is the {accountId} path segment, since a GET + // carries no body to hold it (cf. PublishInternal). + {"GET", regexp.MustCompile(`^/clusters/([^/]+)/internal-addons/([^/]+)$`), "ListInternalAddons", + func(gw *GatewayConfig, acct, callerARN string, p []string, b []byte) (any, error) { + return gateway_eks.ListInternalAddons(gw.NATSConn, p[0], p[1]) + }}, // Nodegroup {"POST", regexp.MustCompile(`^/clusters/([^/]+)/node-groups$`), "CreateNodegroup", diff --git a/spinifex/gateway/eks/internal_addons.go b/spinifex/gateway/eks/internal_addons.go new file mode 100644 index 000000000..abbee70c3 --- /dev/null +++ b/spinifex/gateway/eks/internal_addons.go @@ -0,0 +1,36 @@ +package gateway_eks + +import ( + "errors" + + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_eks "github.com/mulgadc/spinifex/spinifex/handlers/eks" + "github.com/nats-io/nats.go" +) + +// internalAddonsOutput is the body returned to the on-VM addon-sync agent: the +// set of add-on manifests currently staged for the cluster. +type internalAddonsOutput struct { + Addons []handlers_eks.StagedAddonManifest `json:"addons"` +} + +// ListInternalAddons — GET /clusters/{name}/internal-addons?accountId={acct}. +// Internal control-plane VM route (not an AWS-SDK action): the CP VM holds +// system SigV4 creds, so accountID names the customer cluster account explicitly +// — same carve-out as PublishInternal. Returns every staged add-on manifest so +// the VM can render the baked bundles into the K3s auto-deploy dir and GC the +// locally-rendered manifests for add-ons no longer staged. +func ListInternalAddons(natsConn *nats.Conn, clusterName, accountID string) (*internalAddonsOutput, error) { + if natsConn == nil { + return nil, errors.New(awserrors.ErrorServerInternal) + } + if clusterName == "" || accountID == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + out, err := handlers_eks.NewNATSEKSService(natsConn).ListStagedAddonManifests( + &handlers_eks.ListStagedAddonManifestsInput{ClusterName: clusterName}, accountID) + if err != nil { + return nil, err + } + return &internalAddonsOutput{Addons: out.Manifests}, nil +} diff --git a/spinifex/gateway/eks/internal_publish.go b/spinifex/gateway/eks/internal_publish.go index 360905acb..3dec72ce4 100644 --- a/spinifex/gateway/eks/internal_publish.go +++ b/spinifex/gateway/eks/internal_publish.go @@ -14,6 +14,7 @@ import ( const ( internalChannelBootstrap = "bootstrap" internalChannelState = "state" + internalChannelAddon = "addon" ) // internalPublishRequest is the body POSTed to /clusters/{name}/internal-publish. @@ -68,6 +69,8 @@ func PublishInternal(natsConn *nats.Conn, clusterName string, body []byte) (*pub subject = handlers_eks.BootstrapSubject(req.AccountID, clusterName, req.Kind) case internalChannelState: subject = handlers_eks.StateSubject(req.AccountID, clusterName) + case internalChannelAddon: + subject = handlers_eks.AddonStatusSubject(req.AccountID, clusterName) default: slog.Debug("PublishInternal: unknown channel", "cluster", clusterName, "channel", req.Channel) return nil, errors.New(awserrors.ErrorInvalidParameterValue) diff --git a/spinifex/gateway/eks/internal_publish_test.go b/spinifex/gateway/eks/internal_publish_test.go index 9a8bf592c..d0ef4e6ed 100644 --- a/spinifex/gateway/eks/internal_publish_test.go +++ b/spinifex/gateway/eks/internal_publish_test.go @@ -31,6 +31,12 @@ func TestPublishInternal_RelaysToSubjects(t *testing.T) { subject: handlers_eks.StateSubject("111122223333", "alpha"), payload: `{"healthz":"ok","node_count":1,"ts":42}`, }, + { + name: "addon status report", + body: `{"accountId":"111122223333","channel":"addon","payload":{"addon":"spinifex-noop","version":"0.1.0","phase":"ready","ts":42}}`, + subject: handlers_eks.AddonStatusSubject("111122223333", "alpha"), + payload: `{"addon":"spinifex-noop","version":"0.1.0","phase":"ready","ts":42}`, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/spinifex/gateway/eks_test.go b/spinifex/gateway/eks_test.go index 815f811f3..811e26c28 100644 --- a/spinifex/gateway/eks_test.go +++ b/spinifex/gateway/eks_test.go @@ -101,6 +101,7 @@ func TestLookupEKSAction_CoversAllActions(t *testing.T) { // Internal control-plane broker routes, not AWS-SDK EKS actions. "PublishInternal": false, "WebhookTokenReview": false, + "ListInternalAddons": false, "DescribeCluster": false, "ListClusters": false, "UpdateClusterConfig": false, diff --git a/spinifex/handlers/eks/addon_catalog.go b/spinifex/handlers/eks/addon_catalog.go index 50aa2eb5a..1ec1fc55c 100644 --- a/spinifex/handlers/eks/addon_catalog.go +++ b/spinifex/handlers/eks/addon_catalog.go @@ -23,6 +23,13 @@ var addonCatalog = buildAddonCatalog( newAddonSpec("coredns", false, "Cluster DNS server.", "1.11.1"), + // spinifex-noop is the delivery-transport fixture: a trivial bundle + // (Namespace + ConfigMap) used by the addon e2e to prove stage → render → + // auto-deploy → ACTIVE → delete round-trips end-to-end without depending on + // the CSI or load-balancer-controller manifests. Not a real workload addon. + newAddonSpec("spinifex-noop", false, + "No-op delivery-transport fixture (Namespace + ConfigMap).", + "0.1.0"), ) // newAddonSpec builds a spec with DefaultVersion = versions[0]. Panics on empty versions. diff --git a/spinifex/handlers/eks/addon_installer.go b/spinifex/handlers/eks/addon_installer.go index b75ff78c3..732e2705b 100644 --- a/spinifex/handlers/eks/addon_installer.go +++ b/spinifex/handlers/eks/addon_installer.go @@ -19,9 +19,12 @@ type AddonInstaller interface { Uninstall(accountID, cluster, addon string) error } -// stagedManifest is consumed by the VM-side delivery transport: add-on name, -// version, and operator-supplied config for rendering the Kubernetes objects. -type stagedManifest struct { +// StagedAddonManifest is the artifact the VM-side delivery transport consumes. +// It names the bundled add-on + version and carries the operator-supplied config +// so the VM can render the final Kubernetes objects from the baked manifests. +// It is the wire shape the on-VM addon-sync agent fetches via +// GET /clusters/{name}/internal-addons. +type StagedAddonManifest struct { AddonName string `json:"addonName"` AddonVersion string `json:"addonVersion"` ServiceAccountRoleArn string `json:"serviceAccountRoleArn,omitempty"` @@ -61,7 +64,7 @@ func (i *stagingInstaller) Install(accountID, cluster string, rec *AddonRecord) if err != nil { return err } - manifest := stagedManifest{ + manifest := StagedAddonManifest{ AddonName: rec.AddonName, AddonVersion: rec.AddonVersion, ServiceAccountRoleArn: rec.ServiceAccountRoleArn, diff --git a/spinifex/handlers/eks/addon_status_reconciler.go b/spinifex/handlers/eks/addon_status_reconciler.go new file mode 100644 index 000000000..5dc422cba --- /dev/null +++ b/spinifex/handlers/eks/addon_status_reconciler.go @@ -0,0 +1,78 @@ +package handlers_eks + +import ( + "errors" + "log/slog" + "time" +) + +// applyAddonStatusReport CASes the AddonRecord named by an on-VM addon-sync +// delivery report onto its next AWS-visible status. It is a no-op when the +// report names an unknown add-on (DeleteAddon already removed the record) or +// when the status would not change, so a steady stream of "applied"/"ready" +// reports is cheap and idempotent. +func (r *ClusterReconciler) applyAddonStatusReport(report AddonStatusReport) { + if report.Addon == "" { + return + } + now := time.Now().UTC() + _, err := casUpdateAddon(r.acctKV, r.clusterName, report.Addon, func(rec *AddonRecord) bool { + next, changed := nextAddonStatus(rec.Status, report.Phase) + // Surface the agent's message as the add-on health on failure, and clear + // it once the add-on recovers to ACTIVE. + health := rec.Health + switch report.Phase { + case AddonPhaseFailed: + health = report.Message + case AddonPhaseReady: + health = "" + } + if !changed && health == rec.Health { + return false + } + rec.Status = next + rec.Health = health + rec.ModifiedAt = now + return true + }) + if err != nil { + if errors.Is(err, ErrAddonNotFound) { + // Record gone (DeleteAddon) — the agent will GC the rendered manifest + // on its next sync; nothing to update. + return + } + slog.Warn("ClusterReconciler: addon status CAS failed", + "cluster", r.clusterName, "addon", report.Addon, "phase", report.Phase, "err", err) + } +} + +// nextAddonStatus maps a VM-observed delivery phase onto the next AWS-visible +// AddonStatus given the current status, returning the next status and whether it +// changed. The transitions mirror AWS EKS add-on semantics: +// +// - ready → ACTIVE (the add-on's workloads rolled out). +// - applied → no status change: the manifest landed but pods are not yet +// Ready, so the record stays CREATING/UPDATING until a ready report. +// - failed → DEGRADED if the add-on was healthy (ACTIVE), else CREATE_FAILED +// (it never reached ACTIVE). Already-terminal failure states are sticky. +func nextAddonStatus(cur AddonStatus, phase AddonDeliveryPhase) (AddonStatus, bool) { + switch phase { + case AddonPhaseReady: + if cur == AddonStatusActive { + return cur, false + } + return AddonStatusActive, true + case AddonPhaseFailed: + switch cur { + case AddonStatusActive: + return AddonStatusDegraded, true + case AddonStatusDegraded, AddonStatusCreateFailed: + return cur, false + default: + return AddonStatusCreateFailed, true + } + default: + // AddonPhaseApplied and any unknown phase are informational only. + return cur, false + } +} diff --git a/spinifex/handlers/eks/addon_status_reconciler_test.go b/spinifex/handlers/eks/addon_status_reconciler_test.go new file mode 100644 index 000000000..df6c93c98 --- /dev/null +++ b/spinifex/handlers/eks/addon_status_reconciler_test.go @@ -0,0 +1,102 @@ +package handlers_eks + +import ( + "testing" + "time" + + "github.com/nats-io/nats.go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// acctKVForTest opens the per-account KV bucket the test service is backed by. +func acctKVForTest(t *testing.T, svc *EKSServiceImpl) nats.KeyValue { + t.Helper() + js, err := svc.deps.NATSConn.JetStream() + require.NoError(t, err) + kv, err := GetOrCreateAccountBucket(js, testAccountID) + require.NoError(t, err) + return kv +} + +func TestNextAddonStatus(t *testing.T) { + cases := []struct { + name string + cur AddonStatus + phase AddonDeliveryPhase + wantStatus AddonStatus + wantChanged bool + }{ + {"ready from creating -> active", AddonStatusCreating, AddonPhaseReady, AddonStatusActive, true}, + {"ready from updating -> active", AddonStatusUpdating, AddonPhaseReady, AddonStatusActive, true}, + {"ready when already active -> no-op", AddonStatusActive, AddonPhaseReady, AddonStatusActive, false}, + {"applied is informational -> no-op", AddonStatusCreating, AddonPhaseApplied, AddonStatusCreating, false}, + {"failed from creating -> create_failed", AddonStatusCreating, AddonPhaseFailed, AddonStatusCreateFailed, true}, + {"failed from updating -> create_failed", AddonStatusUpdating, AddonPhaseFailed, AddonStatusCreateFailed, true}, + {"failed when active -> degraded", AddonStatusActive, AddonPhaseFailed, AddonStatusDegraded, true}, + {"failed when degraded -> sticky", AddonStatusDegraded, AddonPhaseFailed, AddonStatusDegraded, false}, + {"failed when create_failed -> sticky", AddonStatusCreateFailed, AddonPhaseFailed, AddonStatusCreateFailed, false}, + {"unknown phase -> no-op", AddonStatusCreating, AddonDeliveryPhase("bogus"), AddonStatusCreating, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, changed := nextAddonStatus(tc.cur, tc.phase) + assert.Equal(t, tc.wantStatus, got) + assert.Equal(t, tc.wantChanged, changed) + }) + } +} + +func TestApplyAddonStatusReport(t *testing.T) { + svc := setupTestService(t) + seedTestCluster(t, svc, "c1") + acctKV := acctKVForTest(t, svc) + r := &ClusterReconciler{acctKV: acctKV, clusterName: "c1"} + + seed := func(t *testing.T) { + t.Helper() + now := time.Now().UTC() + require.NoError(t, PutAddonRecord(acctKV, "c1", &AddonRecord{ + AddonName: "coredns", AddonVersion: "1.11.1", Status: AddonStatusCreating, + Arn: AddonARN("us-east-1", testAccountID, "c1", "coredns"), CreatedAt: now, ModifiedAt: now, + })) + } + get := func(t *testing.T) *AddonRecord { + t.Helper() + rec, err := GetAddonRecord(acctKV, "c1", "coredns") + require.NoError(t, err) + return rec + } + + t.Run("ready flips creating to active and clears health", func(t *testing.T) { + seed(t) + r.applyAddonStatusReport(AddonStatusReport{Addon: "coredns", Phase: AddonPhaseReady, Message: "ignored", TS: time.Now().Unix()}) + rec := get(t) + assert.Equal(t, AddonStatusActive, rec.Status) + assert.Empty(t, rec.Health) + }) + + t.Run("failed on active goes degraded and records message", func(t *testing.T) { + r.applyAddonStatusReport(AddonStatusReport{Addon: "coredns", Phase: AddonPhaseFailed, Message: "rollout stalled", TS: time.Now().Unix()}) + rec := get(t) + assert.Equal(t, AddonStatusDegraded, rec.Status) + assert.Equal(t, "rollout stalled", rec.Health) + }) + + t.Run("applied is a no-op", func(t *testing.T) { + seed(t) + before := get(t) + r.applyAddonStatusReport(AddonStatusReport{Addon: "coredns", Phase: AddonPhaseApplied, TS: time.Now().Unix()}) + after := get(t) + assert.Equal(t, AddonStatusCreating, after.Status) + assert.Equal(t, before.ModifiedAt, after.ModifiedAt, "no-op must not bump ModifiedAt") + }) + + t.Run("unknown addon is a no-op (no panic)", func(t *testing.T) { + r.applyAddonStatusReport(AddonStatusReport{Addon: "not-installed", Phase: AddonPhaseReady, TS: time.Now().Unix()}) + }) + + t.Run("empty addon is ignored", func(t *testing.T) { + r.applyAddonStatusReport(AddonStatusReport{Phase: AddonPhaseReady}) + }) +} diff --git a/spinifex/handlers/eks/addons.go b/spinifex/handlers/eks/addons.go index c887adc02..771ca1a36 100644 --- a/spinifex/handlers/eks/addons.go +++ b/spinifex/handlers/eks/addons.go @@ -1,8 +1,11 @@ package handlers_eks import ( + "encoding/json" "errors" + "fmt" "log/slog" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -101,6 +104,72 @@ func (s *EKSServiceImpl) CreateAddon(input *eks.CreateAddonInput, accountID stri return &eks.CreateAddonOutput{Addon: addonRecordToAWS(cluster, rec)}, nil } +// ListStagedAddonManifestsInput names the cluster whose staged add-on manifests +// to return. It is an internal control-plane request (not an AWS-SDK shape), +// served over NATS for the on-VM addon-sync agent via the internal-addons +// gateway route. +type ListStagedAddonManifestsInput struct { + ClusterName string `json:"clusterName"` +} + +// ListStagedAddonManifestsOutput carries the staged manifest descriptors, sorted +// by add-on name. +type ListStagedAddonManifestsOutput struct { + Manifests []StagedAddonManifest `json:"manifests"` +} + +// ListStagedAddonManifests returns the staged manifest descriptor for every +// add-on currently staged for delivery to a cluster, sorted by add-on name. +// The on-VM addon-sync agent fetches these (via the internal-addons gateway +// route) to render the baked bundles into the K3s auto-deploy dir; an add-on +// whose record was deleted has its staged manifest removed, so the agent treats +// absence here as "remove the locally-rendered manifest". +func (s *EKSServiceImpl) ListStagedAddonManifests(input *ListStagedAddonManifestsInput, accountID string) (*ListStagedAddonManifestsOutput, error) { + if input == nil || input.ClusterName == "" { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + acctKV, err := s.acctKVForCluster(accountID, input.ClusterName) + if err != nil { + return nil, err + } + keys, err := acctKV.Keys() + if err != nil { + if errors.Is(err, nats.ErrNoKeysFound) { + return &ListStagedAddonManifestsOutput{Manifests: []StagedAddonManifest{}}, nil + } + return nil, err + } + prefix := AddonsPrefix(input.ClusterName) + out := make([]StagedAddonManifest, 0) + for _, k := range keys { + if !strings.HasPrefix(k, prefix) || !strings.HasSuffix(k, "/manifest") { + continue + } + entry, err := acctKV.Get(k) + if err != nil { + if errors.Is(err, nats.ErrKeyNotFound) { + continue + } + return nil, err + } + var m StagedAddonManifest + if err := json.Unmarshal(entry.Value(), &m); err != nil { + return nil, fmt.Errorf("unmarshal staged manifest %s: %w", k, err) + } + out = append(out, m) + } + sortStagedManifests(out) + return &ListStagedAddonManifestsOutput{Manifests: out}, nil +} + +func sortStagedManifests(m []StagedAddonManifest) { + for i := 1; i < len(m); i++ { + for j := i; j > 0 && m[j-1].AddonName > m[j].AddonName; j-- { + m[j-1], m[j] = m[j], m[j-1] + } + } +} + // DescribeAddon returns one installed add-on's record. func (s *EKSServiceImpl) DescribeAddon(input *eks.DescribeAddonInput, accountID string) (*eks.DescribeAddonOutput, error) { if input == nil { diff --git a/spinifex/handlers/eks/addons_test.go b/spinifex/handlers/eks/addons_test.go index 2a7d9c17d..76fa7a1d1 100644 --- a/spinifex/handlers/eks/addons_test.go +++ b/spinifex/handlers/eks/addons_test.go @@ -239,3 +239,48 @@ func TestStagingInstaller_StagesManifest(t *testing.T) { require.NoError(t, err, "installer must stage a manifest for VM-side delivery") assert.Contains(t, string(entry.Value()), albController) } + +func TestListStagedAddonManifests(t *testing.T) { + svc := setupTestService(t) + seedTestCluster(t, svc, "c1") + + // Empty cluster: no staged manifests, no error. + out, err := svc.ListStagedAddonManifests(&ListStagedAddonManifestsInput{ClusterName: "c1"}, testAccountID) + require.NoError(t, err) + assert.Empty(t, out.Manifests) + + // Stage two addons; reader returns both, sorted by name, with config carried. + _, err = svc.CreateAddon(&eks.CreateAddonInput{ + ClusterName: aws.String("c1"), AddonName: aws.String("coredns"), + }, testAccountID) + require.NoError(t, err) + _, err = svc.CreateAddon(&eks.CreateAddonInput{ + ClusterName: aws.String("c1"), AddonName: aws.String(albController), + ServiceAccountRoleArn: aws.String("arn:aws:iam::111122223333:role/alb"), + }, testAccountID) + require.NoError(t, err) + + out, err = svc.ListStagedAddonManifests(&ListStagedAddonManifestsInput{ClusterName: "c1"}, testAccountID) + require.NoError(t, err) + require.Len(t, out.Manifests, 2) + assert.Equal(t, albController, out.Manifests[0].AddonName, "sorted by addon name") + assert.Equal(t, "coredns", out.Manifests[1].AddonName) + assert.Equal(t, "arn:aws:iam::111122223333:role/alb", out.Manifests[0].ServiceAccountRoleArn) + + // Deleting an addon unstages its manifest; reader drops it. + _, err = svc.DeleteAddon(&eks.DeleteAddonInput{ + ClusterName: aws.String("c1"), AddonName: aws.String(albController), + }, testAccountID) + require.NoError(t, err) + + out, err = svc.ListStagedAddonManifests(&ListStagedAddonManifestsInput{ClusterName: "c1"}, testAccountID) + require.NoError(t, err) + require.Len(t, out.Manifests, 1) + assert.Equal(t, "coredns", out.Manifests[0].AddonName) + + // Unknown cluster surfaces ResourceNotFound; empty cluster name is invalid. + _, err = svc.ListStagedAddonManifests(&ListStagedAddonManifestsInput{ClusterName: "missing"}, testAccountID) + require.EqualError(t, err, awserrors.ErrorEKSResourceNotFound) + _, err = svc.ListStagedAddonManifests(&ListStagedAddonManifestsInput{ClusterName: ""}, testAccountID) + require.EqualError(t, err, awserrors.ErrorInvalidParameterValue) +} diff --git a/spinifex/handlers/eks/cluster_reconciler.go b/spinifex/handlers/eks/cluster_reconciler.go index 0590cf069..df1d810c4 100644 --- a/spinifex/handlers/eks/cluster_reconciler.go +++ b/spinifex/handlers/eks/cluster_reconciler.go @@ -68,6 +68,14 @@ type ClusterReconciler struct { stateSubject string stateStaleAfter time.Duration latest atomic.Pointer[ServerStateReport] + + // Add-on delivery status source. When addonStatusSub is non-nil the + // reconciler subscribes to the per-cluster add-on status subject and CASes + // each AddonRecord (CREATING → ACTIVE/DEGRADED) from the on-VM addon-sync + // agent's reports. Add-on lifecycle is tracked per-resource (AWS parity), so + // this is a sibling of the cluster state-report, not folded into it. + addonStatusSub natsSubscriber + addonStatusSubject string } // ReconcilerOption tunes a ClusterReconciler. @@ -112,8 +120,19 @@ func WithStateStaleAfter(d time.Duration) ReconcilerOption { return func(r *ClusterReconciler) { r.stateStaleAfter = d } } -// NewClusterReconciler constructs a ClusterReconciler. healthURL is the probe -// target or empty to skip /healthz probing (bootstrap state alone drives transitions). +// WithAddonStatusSource configures the reconciler to consume per-add-on delivery +// status reports (subject) via sub and CAS the matching AddonRecord. The +// subscription is opened in Run and closed when it returns. +func WithAddonStatusSource(sub natsSubscriber, subject string) ReconcilerOption { + return func(r *ClusterReconciler) { + r.addonStatusSub = sub + r.addonStatusSubject = subject + } +} + +// NewClusterReconciler constructs a ClusterReconciler. healthURL is the +// fully-qualified probe target (e.g. https://nlb-dns:443/healthz) or empty +// to skip /healthz probing (CREATING transitions on bootstrap state alone). func NewClusterReconciler(leaderKV, acctKV nats.KeyValue, accountID, clusterName, holderID, healthURL string, opts ...ReconcilerOption) (*ClusterReconciler, error) { if leaderKV == nil { return nil, errors.New("eks: NewClusterReconciler nil leaderKV") @@ -220,6 +239,22 @@ func (r *ClusterReconciler) Run(ctx context.Context) error { defer func() { _ = sub.Unsubscribe() }() } + if r.addonStatusSub != nil && r.addonStatusSubject != "" { + sub, err := r.addonStatusSub.Subscribe(r.addonStatusSubject, func(m *nats.Msg) { + report, perr := unmarshalAddonStatusReport(m.Data) + if perr != nil { + slog.Warn("ClusterReconciler: bad addon status report", + "cluster", r.clusterName, "subject", m.Subject, "err", perr) + return + } + r.applyAddonStatusReport(report) + }) + if err != nil { + return fmt.Errorf("subscribe addon status %s: %w", r.addonStatusSubject, err) + } + defer func() { _ = sub.Unsubscribe() }() + } + refreshT := time.NewTicker(r.leaseRefresh) defer refreshT.Stop() reconcileT := time.NewTicker(r.interval) diff --git a/spinifex/handlers/eks/cluster_state.go b/spinifex/handlers/eks/cluster_state.go index 72a7639b1..dbba712d0 100644 --- a/spinifex/handlers/eks/cluster_state.go +++ b/spinifex/handlers/eks/cluster_state.go @@ -53,8 +53,16 @@ type ClusterMeta struct { // ControlPlaneMgmtIP is the CP VM's br-mgmt NIC address, used as the /healthz // probe target until authoritative in-VPC DNS is available. ControlPlaneMgmtIP string `json:"controlPlaneMgmtIp,omitempty"` - // ControlPlaneNodes lists placed CP server VMs; [0] is the primary for NLB - // and egress. Scalar ControlPlane* fields mirror [0] for back-compat. + // ManagedCPVPC holds the system-account control-plane VPC ("Set B") refs the + // NLB + CP VMs live in; nil for clusters created before this topology. Torn + // down by DeleteCluster. + ManagedCPVPC *ManagedCPVPC `json:"managedCpVpc,omitempty"` + // ControlPlaneNodes lists the placed control-plane server VMs. HA spread + // holds one entry per distinct host; the single-CP path holds one. [0] is the + // primary the NLB target + egress SNAT wire to until per-node NLB + // registration (231.7.3). The scalar ControlPlane* fields above mirror [0] + // for readers that predate this field (reconciler, teardown) and for clusters + // persisted before HA spread existed (empty ControlPlaneNodes). ControlPlaneNodes []ControlPlaneNode `json:"controlPlaneNodes,omitempty"` // ControlPlaneSpreadGroup is the spread placement-group name; "" for single-CP. ControlPlaneSpreadGroup string `json:"controlPlaneSpreadGroup,omitempty"` @@ -82,8 +90,26 @@ type ClusterMeta struct { Tags map[string]string `json:"tags,omitempty"` } -// ControlPlaneNode identifies one placed CP server VM. NodeID is the Spinifex -// host; empty for a single-CP launched on the local node. +// ManagedCPVPC records the spinifex-managed control-plane VPC ("Set B") built +// under the system account at CreateCluster: the AWS-managed-account analogue +// the customer never provisions. Holds the resource IDs DeleteCluster tears down +// in dependency order (NAT GW + EIP → route tables → subnets → IGW → VPC). The +// NLB lives in PublicSubnetId; the control-plane VM(s) in PrivateSubnetIds. +type ManagedCPVPC struct { + VpcId string `json:"vpcId,omitempty"` + IGWId string `json:"igwId,omitempty"` + PublicSubnetId string `json:"publicSubnetId,omitempty"` + PublicRouteTableId string `json:"publicRouteTableId,omitempty"` + PrivateSubnetIds []string `json:"privateSubnetIds,omitempty"` + PrivateRouteTableId string `json:"privateRouteTableId,omitempty"` + NatGatewayId string `json:"natGatewayId,omitempty"` + NatEIPAllocationID string `json:"natEipAllocationId,omitempty"` + NatEIPPublicIP string `json:"natEipPublicIp,omitempty"` +} + +// ControlPlaneNode identifies one placed control-plane server VM and the host +// it landed on. NodeID is the Spinifex host — distinct per entry under HA +// spread, empty for a single control plane launched on the local node. type ControlPlaneNode struct { NodeID string `json:"nodeId,omitempty"` InstanceID string `json:"instanceId"` diff --git a/spinifex/handlers/eks/cp_vpc.go b/spinifex/handlers/eks/cp_vpc.go new file mode 100644 index 000000000..8fddfff08 --- /dev/null +++ b/spinifex/handlers/eks/cp_vpc.go @@ -0,0 +1,518 @@ +package handlers_eks + +import ( + "errors" + "fmt" + "hash/fnv" + "log/slog" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/tags" +) + +// The managed control-plane VPC ("Set B") is the spinifex analogue of AWS EKS's +// hidden managed-account VPC: CreateCluster builds it automatically, it is owned +// by the system account (admin.SystemAccountID), and the customer never +// provisions or sees it. The internet-facing NLB lives in its public subnet and +// the k3s control-plane VM(s) in its private subnet(s); a NAT gateway gives the +// private CP egress for image pulls. It is composed from the real EC2 +// VPC-family APIs (not direct OVN mutation), so per-subnet egress gating +// (public 1000 reroute / NAT-GW reroute / private 1100 drop) is wired by the +// existing topology subscribers off the route-table events those APIs publish. +const ( + clusterEKSRoleCPVPC = "cp-vpc" + clusterEKSRolePublicSubnet = "cp-public" + clusterEKSRolePrivateSubnet = "cp-private" + clusterEKSRolePublicRT = "cp-public-rt" + clusterEKSRolePrivateRT = "cp-private-rt" + clusterEKSRoleCPNatGW = "cp-natgw" +) + +// cpVPCSupernetSecondOctet anchors the managed CP VPC address space at +// 10.252.0.0/14 (10.252–10.255). Each cluster gets a deterministic /22 carved by +// cpVPCCIDRs: a public /24 + up to three private /24s (HA 3-node etcd spread). +// VPCs are L3-isolated (separate OVN logical routers) and egress via distinct +// per-VPC external IPs, so a hash collision across clusters is non-fatal; a +// proper managed-IPAM allocator is a follow-up. +const cpVPCSupernetSecondOctet = 252 + +const maxCPPrivateSubnets = 3 + +// cpVPCPrivateSubnetCount is how many private CP subnets the managed VPC carves. +// One today: the placement rule (CP ⇒ private subnet) is identical for single- +// and multi-node clusters, so all control-plane VMs share the private subnet; +// per-AZ private-subnet spread is a follow-up sizing concern, not a placement +// fork. +const cpVPCPrivateSubnetCount = 1 + +// managedCPVPCFromRefs projects the resolved refs onto the persisted ClusterMeta +// shape used for teardown. +func managedCPVPCFromRefs(r *ManagedCPVPCRefs) *ManagedCPVPC { + if r == nil { + return nil + } + return &ManagedCPVPC{ + VpcId: r.VpcID, + IGWId: r.IGWID, + PublicSubnetId: r.PublicSubnetID, + PublicRouteTableId: r.PublicRouteTableID, + PrivateSubnetIds: r.PrivateSubnetIDs, + PrivateRouteTableId: r.PrivateRouteTableID, + NatGatewayId: r.NatGatewayID, + NatEIPAllocationID: r.NatEIPAllocationID, + NatEIPPublicIP: r.NatEIPPublicIP, + } +} + +// vpcProvisioner is the narrow VPC + subnet surface the managed CP VPC needs. +// The daemon adapts the concrete VPC service onto this. +type vpcProvisioner interface { + CreateVpc(input *ec2.CreateVpcInput, accountID string) (*ec2.CreateVpcOutput, error) + DeleteVpc(input *ec2.DeleteVpcInput, accountID string) (*ec2.DeleteVpcOutput, error) + DescribeVpcs(input *ec2.DescribeVpcsInput, accountID string) (*ec2.DescribeVpcsOutput, error) + CreateSubnet(input *ec2.CreateSubnetInput, accountID string) (*ec2.CreateSubnetOutput, error) + DeleteSubnet(input *ec2.DeleteSubnetInput, accountID string) (*ec2.DeleteSubnetOutput, error) + DescribeSubnets(input *ec2.DescribeSubnetsInput, accountID string) (*ec2.DescribeSubnetsOutput, error) +} + +// routeTableProvisioner is the narrow route-table surface the managed CP VPC +// needs to express public (0/0→IGW) and private (0/0→NAT GW) egress. +type routeTableProvisioner interface { + CreateRouteTable(input *ec2.CreateRouteTableInput, accountID string) (*ec2.CreateRouteTableOutput, error) + DeleteRouteTable(input *ec2.DeleteRouteTableInput, accountID string) (*ec2.DeleteRouteTableOutput, error) + DescribeRouteTables(input *ec2.DescribeRouteTablesInput, accountID string) (*ec2.DescribeRouteTablesOutput, error) + CreateRoute(input *ec2.CreateRouteInput, accountID string) (*ec2.CreateRouteOutput, error) + AssociateRouteTable(input *ec2.AssociateRouteTableInput, accountID string) (*ec2.AssociateRouteTableOutput, error) + DisassociateRouteTable(input *ec2.DisassociateRouteTableInput, accountID string) (*ec2.DisassociateRouteTableOutput, error) +} + +// natGatewayProvisioner is the narrow NAT-gateway surface the managed CP VPC +// needs to give the private control-plane subnet egress. +type natGatewayProvisioner interface { + CreateNatGateway(input *ec2.CreateNatGatewayInput, accountID string) (*ec2.CreateNatGatewayOutput, error) + DeleteNatGateway(input *ec2.DeleteNatGatewayInput, accountID string) (*ec2.DeleteNatGatewayOutput, error) + DescribeNatGateways(input *ec2.DescribeNatGatewaysInput, accountID string) (*ec2.DescribeNatGatewaysOutput, error) +} + +// CPVPCDeps bundles the EC2-family collaborators EnsureClusterCPVPC composes the +// managed control-plane VPC from. All calls run under the system account. +type CPVPCDeps struct { + VPC vpcProvisioner + IGW igwProvisioner + RT routeTableProvisioner + NGW natGatewayProvisioner + EIP eipProvisioner +} + +// cpVPCDeps adapts the service's EC2-family deps onto CPVPCDeps for the managed +// control-plane VPC build + teardown. +func (s *EKSServiceImpl) cpVPCDeps() CPVPCDeps { + return CPVPCDeps{ + VPC: s.deps.VPCMgr, + IGW: s.deps.IGW, + RT: s.deps.RouteTable, + NGW: s.deps.NATGW, + EIP: s.deps.EIP, + } +} + +// ManagedCPVPCRefs is the resolved set of managed CP VPC resource IDs + CIDRs. +// EnsureClusterCPVPC returns it; the caller persists it into ClusterMeta for +// placement (public/private subnet selection) and teardown. +type ManagedCPVPCRefs struct { + VpcID string + VpcCIDR string + IGWID string + PublicSubnetID string + PublicSubnetCIDR string + PublicRouteTableID string + PrivateSubnetIDs []string + PrivateSubnetCIDRs []string + PrivateRouteTableID string + NatGatewayID string + NatEIPAllocationID string + NatEIPPublicIP string +} + +// cpVPCCIDRs derives the managed CP VPC CIDR + subnet CIDRs deterministically +// from the cluster name. The /22 is carved from 10.252.0.0/14; subnet 0 (.0/24) +// is public, subnets 1..maxCPPrivateSubnets are private. +func cpVPCCIDRs(clusterName string, privateCount int) (vpcCIDR, publicCIDR string, privateCIDRs []string) { + h := fnv.New32a() + _, _ = h.Write([]byte(clusterName)) + idx := int(h.Sum32() % 256) // 256 /22 blocks in a /14 + + combined := idx * 4 // each /22 spans 4 third-octet values + second := cpVPCSupernetSecondOctet + combined/256 + third := combined % 256 + + vpcCIDR = fmt.Sprintf("10.%d.%d.0/22", second, third) + publicCIDR = fmt.Sprintf("10.%d.%d.0/24", second, third) + if privateCount < 1 { + privateCount = 1 + } + if privateCount > maxCPPrivateSubnets { + privateCount = maxCPPrivateSubnets + } + for k := 0; k < privateCount; k++ { + privateCIDRs = append(privateCIDRs, fmt.Sprintf("10.%d.%d.0/24", second, third+1+k)) + } + return vpcCIDR, publicCIDR, privateCIDRs +} + +// cpVPCAZ returns the AvailabilityZone name for subnet index k (cosmetic AWS +// parity; spinifex spreads CP by host placement group, not AZ). +func cpVPCAZ(region string, k int) string { + return fmt.Sprintf("%s%c", region, 'a'+k) +} + +func cpVPCTagSpec(resourceType, clusterName, role string) []*ec2.TagSpecification { + return []*ec2.TagSpecification{{ + ResourceType: aws.String(resourceType), + Tags: []*ec2.Tag{ + {Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByEKS)}, + {Key: aws.String(clusterEKSClusterTagKey), Value: aws.String(clusterName)}, + {Key: aws.String(clusterEKSRoleTagKey), Value: aws.String(role)}, + }, + }} +} + +func cpClusterRoleFilters(clusterName, role string) []*ec2.Filter { + return []*ec2.Filter{ + {Name: aws.String("tag:" + clusterEKSClusterTagKey), Values: aws.StringSlice([]string{clusterName})}, + {Name: aws.String("tag:" + clusterEKSRoleTagKey), Values: aws.StringSlice([]string{role})}, + } +} + +// EnsureClusterCPVPC builds (idempotently) the managed control-plane VPC under +// accountID (the system account) for clusterName: a VPC, an attached IGW, a +// public subnet routed to the IGW, privateCount private subnet(s) routed to a +// NAT gateway, and the NAT gateway itself. Each resource is describe-or-create +// keyed on the cluster + role tags so a relaunch after partial failure converges +// rather than duplicating. The route-table associations publish the topology +// events the per-subnet egress subscribers consume, so the OVN policy split +// (public 1000 / NAT-GW reroute / private 1100 drop) falls out without any +// direct OVN mutation here. +func EnsureClusterCPVPC(deps CPVPCDeps, accountID, clusterName, region string, privateCount int) (*ManagedCPVPCRefs, error) { + if clusterName == "" { + return nil, errors.New("eks: EnsureClusterCPVPC empty cluster name") + } + vpcCIDR, publicCIDR, privateCIDRs := cpVPCCIDRs(clusterName, privateCount) + refs := &ManagedCPVPCRefs{VpcCIDR: vpcCIDR, PublicSubnetCIDR: publicCIDR, PrivateSubnetCIDRs: privateCIDRs} + + vpcID, err := ensureCPVPC(deps.VPC, accountID, clusterName, vpcCIDR) + if err != nil { + return nil, err + } + refs.VpcID = vpcID + + if err := EnsureClusterIGW(deps.IGW, accountID, vpcID, clusterName); err != nil { + return nil, err + } + igw, err := attachedVPCIGW(deps.IGW, accountID, vpcID) + if err != nil { + return nil, err + } + if igw == nil { + return nil, fmt.Errorf("eks: cp vpc %s has no attached IGW after ensure", vpcID) + } + refs.IGWID = aws.StringValue(igw.InternetGatewayId) + + pubSubnet, err := ensureCPSubnet(deps.VPC, accountID, clusterName, vpcID, publicCIDR, clusterEKSRolePublicSubnet, cpVPCAZ(region, 0)) + if err != nil { + return nil, err + } + refs.PublicSubnetID = pubSubnet + + for k, cidr := range privateCIDRs { + priv, err := ensureCPSubnet(deps.VPC, accountID, clusterName, vpcID, cidr, clusterEKSRolePrivateSubnet, cpVPCAZ(region, k)) + if err != nil { + return nil, err + } + refs.PrivateSubnetIDs = append(refs.PrivateSubnetIDs, priv) + } + + // Public route table: 0.0.0.0/0 → IGW, associated to the public subnet. The + // association publishes vpc.add-igw-route → EnsureSubnetEgress (1000 reroute). + pubRT, err := ensureCPRouteTable(deps.RT, accountID, clusterName, vpcID, clusterEKSRolePublicRT, + &ec2.CreateRouteInput{DestinationCidrBlock: aws.String("0.0.0.0/0"), GatewayId: aws.String(refs.IGWID)}, + []string{refs.PublicSubnetID}) + if err != nil { + return nil, err + } + refs.PublicRouteTableID = pubRT + + // NAT gateway in the public subnet, fed by a dedicated EIP. The private CP + // subnets route 0.0.0.0/0 → this NAT GW for egress-only internet (image pulls). + natID, allocID, natIP, err := ensureCPNatGateway(deps, accountID, clusterName, refs.PublicSubnetID) + if err != nil { + return nil, err + } + refs.NatGatewayID = natID + refs.NatEIPAllocationID = allocID + refs.NatEIPPublicIP = natIP + + // Private route table: 0.0.0.0/0 → NAT GW, associated to every private subnet. + // The association publishes vpc.add-nat-gateway → EnsureNATGatewaySubnetEgress. + privRT, err := ensureCPRouteTable(deps.RT, accountID, clusterName, vpcID, clusterEKSRolePrivateRT, + &ec2.CreateRouteInput{DestinationCidrBlock: aws.String("0.0.0.0/0"), NatGatewayId: aws.String(natID)}, + refs.PrivateSubnetIDs) + if err != nil { + return nil, err + } + refs.PrivateRouteTableID = privRT + + slog.Info("EnsureClusterCPVPC: managed control-plane VPC ready", + "cluster", clusterName, "vpc", refs.VpcID, "publicSubnet", refs.PublicSubnetID, + "privateSubnets", refs.PrivateSubnetIDs, "natgw", refs.NatGatewayID) + return refs, nil +} + +func ensureCPVPC(vpcp vpcProvisioner, accountID, clusterName, cidr string) (string, error) { + out, err := vpcp.DescribeVpcs(&ec2.DescribeVpcsInput{Filters: cpClusterRoleFilters(clusterName, clusterEKSRoleCPVPC)}, accountID) + if err != nil { + return "", fmt.Errorf("eks: describe cp vpc for %s: %w", clusterName, err) + } + if out != nil && len(out.Vpcs) > 0 { + return aws.StringValue(out.Vpcs[0].VpcId), nil + } + created, err := vpcp.CreateVpc(&ec2.CreateVpcInput{ + CidrBlock: aws.String(cidr), + TagSpecifications: cpVPCTagSpec("vpc", clusterName, clusterEKSRoleCPVPC), + }, accountID) + if err != nil { + return "", fmt.Errorf("eks: create cp vpc for %s: %w", clusterName, err) + } + if created == nil || created.Vpc == nil || aws.StringValue(created.Vpc.VpcId) == "" { + return "", fmt.Errorf("eks: create cp vpc for %s: empty vpc id", clusterName) + } + return aws.StringValue(created.Vpc.VpcId), nil +} + +func ensureCPSubnet(vpcp vpcProvisioner, accountID, clusterName, vpcID, cidr, role, az string) (string, error) { + out, err := vpcp.DescribeSubnets(&ec2.DescribeSubnetsInput{Filters: append( + cpClusterRoleFilters(clusterName, role), + &ec2.Filter{Name: aws.String("cidr-block"), Values: aws.StringSlice([]string{cidr})}, + )}, accountID) + if err != nil { + return "", fmt.Errorf("eks: describe cp subnet %s (%s): %w", cidr, role, err) + } + if out != nil && len(out.Subnets) > 0 { + return aws.StringValue(out.Subnets[0].SubnetId), nil + } + created, err := vpcp.CreateSubnet(&ec2.CreateSubnetInput{ + VpcId: aws.String(vpcID), + CidrBlock: aws.String(cidr), + AvailabilityZone: aws.String(az), + TagSpecifications: cpVPCTagSpec("subnet", clusterName, role), + }, accountID) + if err != nil { + return "", fmt.Errorf("eks: create cp subnet %s (%s): %w", cidr, role, err) + } + if created == nil || created.Subnet == nil || aws.StringValue(created.Subnet.SubnetId) == "" { + return "", fmt.Errorf("eks: create cp subnet %s (%s): empty subnet id", cidr, role) + } + return aws.StringValue(created.Subnet.SubnetId), nil +} + +// ensureCPRouteTable describe-or-creates the role-tagged route table, installs +// the default route, and associates it to subnetIDs. Idempotent: a re-run reuses +// the existing table and its associations (AssociateRouteTable is a no-op for an +// already-associated subnet at the service layer). +func ensureCPRouteTable(rtp routeTableProvisioner, accountID, clusterName, vpcID, role string, route *ec2.CreateRouteInput, subnetIDs []string) (string, error) { + rtID, fresh, err := describeOrCreateCPRouteTable(rtp, accountID, clusterName, vpcID, role) + if err != nil { + return "", err + } + if fresh { + route.RouteTableId = aws.String(rtID) + if _, err := rtp.CreateRoute(route, accountID); err != nil { + return "", fmt.Errorf("eks: create cp route (%s) on %s: %w", role, rtID, err) + } + } + for _, sn := range subnetIDs { + if sn == "" { + continue + } + if _, err := rtp.AssociateRouteTable(&ec2.AssociateRouteTableInput{ + RouteTableId: aws.String(rtID), + SubnetId: aws.String(sn), + }, accountID); err != nil { + return "", fmt.Errorf("eks: associate cp route table %s → subnet %s: %w", rtID, sn, err) + } + } + return rtID, nil +} + +func describeOrCreateCPRouteTable(rtp routeTableProvisioner, accountID, clusterName, vpcID, role string) (rtID string, fresh bool, err error) { + out, err := rtp.DescribeRouteTables(&ec2.DescribeRouteTablesInput{Filters: cpClusterRoleFilters(clusterName, role)}, accountID) + if err != nil { + return "", false, fmt.Errorf("eks: describe cp route table (%s): %w", role, err) + } + if out != nil && len(out.RouteTables) > 0 { + return aws.StringValue(out.RouteTables[0].RouteTableId), false, nil + } + created, err := rtp.CreateRouteTable(&ec2.CreateRouteTableInput{ + VpcId: aws.String(vpcID), + TagSpecifications: cpVPCTagSpec("route-table", clusterName, role), + }, accountID) + if err != nil { + return "", false, fmt.Errorf("eks: create cp route table (%s): %w", role, err) + } + if created == nil || created.RouteTable == nil || aws.StringValue(created.RouteTable.RouteTableId) == "" { + return "", false, fmt.Errorf("eks: create cp route table (%s): empty id", role) + } + return aws.StringValue(created.RouteTable.RouteTableId), true, nil +} + +func ensureCPNatGateway(deps CPVPCDeps, accountID, clusterName, publicSubnetID string) (natID, allocID, publicIP string, err error) { + out, err := deps.NGW.DescribeNatGateways(&ec2.DescribeNatGatewaysInput{Filter: append( + cpClusterRoleFilters(clusterName, clusterEKSRoleCPNatGW), + &ec2.Filter{Name: aws.String("state"), Values: aws.StringSlice([]string{"available", "pending"})}, + )}, accountID) + if err != nil { + return "", "", "", fmt.Errorf("eks: describe cp nat gateway for %s: %w", clusterName, err) + } + if out != nil && len(out.NatGateways) > 0 { + ng := out.NatGateways[0] + var alloc, ip string + if len(ng.NatGatewayAddresses) > 0 { + alloc = aws.StringValue(ng.NatGatewayAddresses[0].AllocationId) + ip = aws.StringValue(ng.NatGatewayAddresses[0].PublicIp) + } + return aws.StringValue(ng.NatGatewayId), alloc, ip, nil + } + + eip, err := deps.EIP.AllocateAddress(&ec2.AllocateAddressInput{ + Domain: aws.String("vpc"), + TagSpecifications: cpVPCTagSpec("elastic-ip", clusterName, clusterEKSRoleCPNatGW), + }, accountID) + if err != nil { + return "", "", "", fmt.Errorf("eks: allocate cp nat gateway EIP for %s: %w", clusterName, err) + } + if eip == nil || aws.StringValue(eip.AllocationId) == "" { + return "", "", "", fmt.Errorf("eks: allocate cp nat gateway EIP for %s: empty allocation", clusterName) + } + allocID = aws.StringValue(eip.AllocationId) + publicIP = aws.StringValue(eip.PublicIp) + + created, err := deps.NGW.CreateNatGateway(&ec2.CreateNatGatewayInput{ + SubnetId: aws.String(publicSubnetID), + AllocationId: aws.String(allocID), + TagSpecifications: cpVPCTagSpec("natgateway", clusterName, clusterEKSRoleCPNatGW), + }, accountID) + if err != nil { + // Release the orphaned EIP so a failed NAT-GW create does not leak it. + if _, relErr := deps.EIP.ReleaseAddress(&ec2.ReleaseAddressInput{AllocationId: aws.String(allocID)}, accountID); relErr != nil { + slog.Warn("ensureCPNatGateway: failed to release EIP after NAT-GW create failure", "alloc", allocID, "err", relErr) + } + return "", "", "", fmt.Errorf("eks: create cp nat gateway for %s: %w", clusterName, err) + } + if created == nil || created.NatGateway == nil || aws.StringValue(created.NatGateway.NatGatewayId) == "" { + return "", "", "", fmt.Errorf("eks: create cp nat gateway for %s: empty id", clusterName) + } + return aws.StringValue(created.NatGateway.NatGatewayId), allocID, publicIP, nil +} + +// DeleteClusterCPVPC tears down the managed control-plane VPC for clusterName in +// dependency order: NAT gateway (+ its EIP) → route tables → subnets → IGW → +// VPC. Tag-driven (not meta-driven) so a partial create still converges to a +// clean delete. Best-effort: every step is attempted and failures are logged; +// the final VPC delete failure is returned so the caller can surface a leak. +// Safe to call when nothing was provisioned (all describes return empty). +func DeleteClusterCPVPC(deps CPVPCDeps, accountID, clusterName string) error { + if clusterName == "" { + return errors.New("eks: DeleteClusterCPVPC empty cluster name") + } + + // 1. NAT gateway + its EIP. DeleteNatGateway publishes the SNAT removal. + if ngOut, err := deps.NGW.DescribeNatGateways(&ec2.DescribeNatGatewaysInput{ + Filter: cpClusterRoleFilters(clusterName, clusterEKSRoleCPNatGW), + }, accountID); err != nil { + slog.Warn("DeleteClusterCPVPC: describe NAT gateways failed", "cluster", clusterName, "err", err) + } else if ngOut != nil { + for _, ng := range ngOut.NatGateways { + if state := aws.StringValue(ng.State); state == "deleted" || state == "deleting" { + continue + } + ngID := aws.StringValue(ng.NatGatewayId) + if _, err := deps.NGW.DeleteNatGateway(&ec2.DeleteNatGatewayInput{NatGatewayId: aws.String(ngID)}, accountID); err != nil { + slog.Warn("DeleteClusterCPVPC: delete NAT gateway failed", "natgw", ngID, "err", err) + } + for _, addr := range ng.NatGatewayAddresses { + if alloc := aws.StringValue(addr.AllocationId); alloc != "" { + if _, err := deps.EIP.ReleaseAddress(&ec2.ReleaseAddressInput{AllocationId: aws.String(alloc)}, accountID); err != nil { + slog.Warn("DeleteClusterCPVPC: release NAT EIP failed", "alloc", alloc, "err", err) + } + } + } + } + } + + // 2. Route tables: disassociate every subnet, then delete. Disassociation + // publishes the egress-reroute removal for each subnet. + for _, role := range []string{clusterEKSRolePublicRT, clusterEKSRolePrivateRT} { + rtOut, err := deps.RT.DescribeRouteTables(&ec2.DescribeRouteTablesInput{Filters: cpClusterRoleFilters(clusterName, role)}, accountID) + if err != nil { + slog.Warn("DeleteClusterCPVPC: describe route tables failed", "role", role, "err", err) + continue + } + if rtOut == nil { + continue + } + for _, rt := range rtOut.RouteTables { + rtID := aws.StringValue(rt.RouteTableId) + for _, assoc := range rt.Associations { + if aws.BoolValue(assoc.Main) { + continue + } + if aID := aws.StringValue(assoc.RouteTableAssociationId); aID != "" { + if _, err := deps.RT.DisassociateRouteTable(&ec2.DisassociateRouteTableInput{AssociationId: aws.String(aID)}, accountID); err != nil { + slog.Warn("DeleteClusterCPVPC: disassociate route table failed", "assoc", aID, "err", err) + } + } + } + if _, err := deps.RT.DeleteRouteTable(&ec2.DeleteRouteTableInput{RouteTableId: aws.String(rtID)}, accountID); err != nil { + slog.Warn("DeleteClusterCPVPC: delete route table failed", "rt", rtID, "err", err) + } + } + } + + // 3. Subnets (public + private). + for _, role := range []string{clusterEKSRolePublicSubnet, clusterEKSRolePrivateSubnet} { + snOut, err := deps.VPC.DescribeSubnets(&ec2.DescribeSubnetsInput{Filters: cpClusterRoleFilters(clusterName, role)}, accountID) + if err != nil { + slog.Warn("DeleteClusterCPVPC: describe subnets failed", "role", role, "err", err) + continue + } + if snOut == nil { + continue + } + for _, sn := range snOut.Subnets { + snID := aws.StringValue(sn.SubnetId) + if _, err := deps.VPC.DeleteSubnet(&ec2.DeleteSubnetInput{SubnetId: aws.String(snID)}, accountID); err != nil { + slog.Warn("DeleteClusterCPVPC: delete subnet failed", "subnet", snID, "err", err) + } + } + } + + // 4. VPC + its IGW. Resolve the VPC first so DeleteClusterIGW can detach. + vpcOut, err := deps.VPC.DescribeVpcs(&ec2.DescribeVpcsInput{Filters: cpClusterRoleFilters(clusterName, clusterEKSRoleCPVPC)}, accountID) + if err != nil { + return fmt.Errorf("eks: describe cp vpc for delete (%s): %w", clusterName, err) + } + if vpcOut == nil || len(vpcOut.Vpcs) == 0 { + return nil + } + vpcID := aws.StringValue(vpcOut.Vpcs[0].VpcId) + + if err := DeleteClusterIGW(deps.IGW, accountID, vpcID, clusterName); err != nil { + slog.Warn("DeleteClusterCPVPC: delete IGW failed", "vpc", vpcID, "err", err) + } + if _, err := deps.VPC.DeleteVpc(&ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, accountID); err != nil { + return fmt.Errorf("eks: delete cp vpc %s: %w", vpcID, err) + } + slog.Info("DeleteClusterCPVPC: managed control-plane VPC removed", "cluster", clusterName, "vpc", vpcID) + return nil +} diff --git a/spinifex/handlers/eks/cp_vpc_test.go b/spinifex/handlers/eks/cp_vpc_test.go new file mode 100644 index 000000000..b546ce5c2 --- /dev/null +++ b/spinifex/handlers/eks/cp_vpc_test.go @@ -0,0 +1,384 @@ +package handlers_eks + +import ( + "fmt" + "strings" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" +) + +// Fakes for the managed control-plane VPC ("Set B") collaborators. They model +// describe-or-create idempotency by storing each resource with the tag map its +// create spec carried, so a re-describe under the same cluster+role filters +// returns the existing resource rather than a duplicate. + +var ( + _ vpcProvisioner = (*fakeVPCProvisioner)(nil) + _ routeTableProvisioner = (*fakeRouteTableProvisioner)(nil) + _ natGatewayProvisioner = (*fakeNatGatewayProvisioner)(nil) +) + +func cpvpcTagsFromSpecs(specs []*ec2.TagSpecification) map[string]string { + m := map[string]string{} + for _, s := range specs { + if s == nil { + continue + } + for _, t := range s.Tags { + m[aws.StringValue(t.Key)] = aws.StringValue(t.Value) + } + } + return m +} + +func ec2TagsFromMap(m map[string]string) []*ec2.Tag { + out := make([]*ec2.Tag, 0, len(m)) + for k, v := range m { + out = append(out, &ec2.Tag{Key: aws.String(k), Value: aws.String(v)}) + } + return out +} + +// cpvpcMatchTagFilters reports whether tagMap satisfies every `tag:KEY` filter. +// Non-tag filters are ignored here (scalar filters are checked separately). +func cpvpcMatchTagFilters(tagMap map[string]string, filters []*ec2.Filter) bool { + for _, f := range filters { + name := aws.StringValue(f.Name) + if !strings.HasPrefix(name, "tag:") { + continue + } + key := strings.TrimPrefix(name, "tag:") + got, ok := tagMap[key] + if !ok { + return false + } + match := false + for _, v := range f.Values { + if aws.StringValue(v) == got { + match = true + break + } + } + if !match { + return false + } + } + return true +} + +// cpvpcMatchScalarFilter returns true when no filter named `name` is present, or +// one is present and value is among its values. +func cpvpcMatchScalarFilter(filters []*ec2.Filter, name, value string) bool { + for _, fl := range filters { + if aws.StringValue(fl.Name) != name { + continue + } + for _, v := range fl.Values { + if aws.StringValue(v) == value { + return true + } + } + return false + } + return true +} + +// --- VPC + subnets --- + +type fakeCPVPC struct { + id string + cidr string + tags map[string]string +} + +type fakeCPSubnet struct { + id string + vpc string + cidr string + tags map[string]string +} + +type fakeVPCProvisioner struct { + vpcs []*fakeCPVPC + subnets []*fakeCPSubnet + nextVPC int + nextSub int + + createVpcCalls []*ec2.CreateVpcInput + createVpcAccts []string + createSubnetCalls []*ec2.CreateSubnetInput + deleteVpcCalls []*ec2.DeleteVpcInput + deleteSubnetCalls []*ec2.DeleteSubnetInput + + createVpcErr error + createSubnetErr error +} + +func newFakeVPCProvisioner() *fakeVPCProvisioner { return &fakeVPCProvisioner{} } + +func (f *fakeVPCProvisioner) DescribeVpcs(input *ec2.DescribeVpcsInput, _ string) (*ec2.DescribeVpcsOutput, error) { + var out []*ec2.Vpc + for _, v := range f.vpcs { + if !cpvpcMatchTagFilters(v.tags, input.Filters) { + continue + } + out = append(out, &ec2.Vpc{VpcId: aws.String(v.id), CidrBlock: aws.String(v.cidr), Tags: ec2TagsFromMap(v.tags)}) + } + return &ec2.DescribeVpcsOutput{Vpcs: out}, nil +} + +func (f *fakeVPCProvisioner) CreateVpc(input *ec2.CreateVpcInput, accountID string) (*ec2.CreateVpcOutput, error) { + f.createVpcCalls = append(f.createVpcCalls, input) + f.createVpcAccts = append(f.createVpcAccts, accountID) + if f.createVpcErr != nil { + return nil, f.createVpcErr + } + f.nextVPC++ + v := &fakeCPVPC{ + id: fmt.Sprintf("vpc-cp%04d", f.nextVPC), + cidr: aws.StringValue(input.CidrBlock), + tags: cpvpcTagsFromSpecs(input.TagSpecifications), + } + f.vpcs = append(f.vpcs, v) + return &ec2.CreateVpcOutput{Vpc: &ec2.Vpc{VpcId: aws.String(v.id), CidrBlock: aws.String(v.cidr)}}, nil +} + +func (f *fakeVPCProvisioner) DeleteVpc(input *ec2.DeleteVpcInput, _ string) (*ec2.DeleteVpcOutput, error) { + f.deleteVpcCalls = append(f.deleteVpcCalls, input) + id := aws.StringValue(input.VpcId) + kept := f.vpcs[:0] + for _, v := range f.vpcs { + if v.id != id { + kept = append(kept, v) + } + } + f.vpcs = kept + return &ec2.DeleteVpcOutput{}, nil +} + +func (f *fakeVPCProvisioner) DescribeSubnets(input *ec2.DescribeSubnetsInput, _ string) (*ec2.DescribeSubnetsOutput, error) { + var out []*ec2.Subnet + for _, sn := range f.subnets { + if !cpvpcMatchTagFilters(sn.tags, input.Filters) { + continue + } + if !cpvpcMatchScalarFilter(input.Filters, "cidr-block", sn.cidr) { + continue + } + out = append(out, &ec2.Subnet{SubnetId: aws.String(sn.id), VpcId: aws.String(sn.vpc), CidrBlock: aws.String(sn.cidr), Tags: ec2TagsFromMap(sn.tags)}) + } + return &ec2.DescribeSubnetsOutput{Subnets: out}, nil +} + +func (f *fakeVPCProvisioner) CreateSubnet(input *ec2.CreateSubnetInput, _ string) (*ec2.CreateSubnetOutput, error) { + f.createSubnetCalls = append(f.createSubnetCalls, input) + if f.createSubnetErr != nil { + return nil, f.createSubnetErr + } + f.nextSub++ + sn := &fakeCPSubnet{ + id: fmt.Sprintf("subnet-cp%04d", f.nextSub), + vpc: aws.StringValue(input.VpcId), + cidr: aws.StringValue(input.CidrBlock), + tags: cpvpcTagsFromSpecs(input.TagSpecifications), + } + f.subnets = append(f.subnets, sn) + return &ec2.CreateSubnetOutput{Subnet: &ec2.Subnet{SubnetId: aws.String(sn.id), VpcId: input.VpcId, CidrBlock: input.CidrBlock}}, nil +} + +func (f *fakeVPCProvisioner) DeleteSubnet(input *ec2.DeleteSubnetInput, _ string) (*ec2.DeleteSubnetOutput, error) { + f.deleteSubnetCalls = append(f.deleteSubnetCalls, input) + id := aws.StringValue(input.SubnetId) + kept := f.subnets[:0] + for _, sn := range f.subnets { + if sn.id != id { + kept = append(kept, sn) + } + } + f.subnets = kept + return &ec2.DeleteSubnetOutput{}, nil +} + +// --- Route tables --- + +type fakeCPRouteTable struct { + id string + vpc string + tags map[string]string + assocs []*ec2.RouteTableAssociation +} + +type fakeRouteTableProvisioner struct { + tables []*fakeCPRouteTable + nextRT int + nextAssoc int + + createCalls []*ec2.CreateRouteTableInput + createRoutes []*ec2.CreateRouteInput + assocCalls []*ec2.AssociateRouteTableInput + disassoc []*ec2.DisassociateRouteTableInput + deleteCalls []*ec2.DeleteRouteTableInput + + createErr error +} + +func newFakeRouteTableProvisioner() *fakeRouteTableProvisioner { return &fakeRouteTableProvisioner{} } + +func (f *fakeRouteTableProvisioner) find(id string) *fakeCPRouteTable { + for _, rt := range f.tables { + if rt.id == id { + return rt + } + } + return nil +} + +func (f *fakeRouteTableProvisioner) DescribeRouteTables(input *ec2.DescribeRouteTablesInput, _ string) (*ec2.DescribeRouteTablesOutput, error) { + var out []*ec2.RouteTable + for _, rt := range f.tables { + if !cpvpcMatchTagFilters(rt.tags, input.Filters) { + continue + } + out = append(out, &ec2.RouteTable{ + RouteTableId: aws.String(rt.id), + VpcId: aws.String(rt.vpc), + Tags: ec2TagsFromMap(rt.tags), + Associations: rt.assocs, + }) + } + return &ec2.DescribeRouteTablesOutput{RouteTables: out}, nil +} + +func (f *fakeRouteTableProvisioner) CreateRouteTable(input *ec2.CreateRouteTableInput, _ string) (*ec2.CreateRouteTableOutput, error) { + f.createCalls = append(f.createCalls, input) + if f.createErr != nil { + return nil, f.createErr + } + f.nextRT++ + rt := &fakeCPRouteTable{ + id: fmt.Sprintf("rtb-cp%04d", f.nextRT), + vpc: aws.StringValue(input.VpcId), + tags: cpvpcTagsFromSpecs(input.TagSpecifications), + } + f.tables = append(f.tables, rt) + return &ec2.CreateRouteTableOutput{RouteTable: &ec2.RouteTable{RouteTableId: aws.String(rt.id), VpcId: input.VpcId}}, nil +} + +func (f *fakeRouteTableProvisioner) DeleteRouteTable(input *ec2.DeleteRouteTableInput, _ string) (*ec2.DeleteRouteTableOutput, error) { + f.deleteCalls = append(f.deleteCalls, input) + id := aws.StringValue(input.RouteTableId) + kept := f.tables[:0] + for _, rt := range f.tables { + if rt.id != id { + kept = append(kept, rt) + } + } + f.tables = kept + return &ec2.DeleteRouteTableOutput{}, nil +} + +func (f *fakeRouteTableProvisioner) CreateRoute(input *ec2.CreateRouteInput, _ string) (*ec2.CreateRouteOutput, error) { + f.createRoutes = append(f.createRoutes, input) + return &ec2.CreateRouteOutput{Return: aws.Bool(true)}, nil +} + +func (f *fakeRouteTableProvisioner) AssociateRouteTable(input *ec2.AssociateRouteTableInput, _ string) (*ec2.AssociateRouteTableOutput, error) { + f.assocCalls = append(f.assocCalls, input) + f.nextAssoc++ + assocID := fmt.Sprintf("rtbassoc-cp%04d", f.nextAssoc) + if rt := f.find(aws.StringValue(input.RouteTableId)); rt != nil { + rt.assocs = append(rt.assocs, &ec2.RouteTableAssociation{ + RouteTableAssociationId: aws.String(assocID), + RouteTableId: input.RouteTableId, + SubnetId: input.SubnetId, + Main: aws.Bool(false), + }) + } + return &ec2.AssociateRouteTableOutput{AssociationId: aws.String(assocID)}, nil +} + +func (f *fakeRouteTableProvisioner) DisassociateRouteTable(input *ec2.DisassociateRouteTableInput, _ string) (*ec2.DisassociateRouteTableOutput, error) { + f.disassoc = append(f.disassoc, input) + id := aws.StringValue(input.AssociationId) + for _, rt := range f.tables { + kept := rt.assocs[:0] + for _, a := range rt.assocs { + if aws.StringValue(a.RouteTableAssociationId) != id { + kept = append(kept, a) + } + } + rt.assocs = kept + } + return &ec2.DisassociateRouteTableOutput{}, nil +} + +// --- NAT gateway --- + +type fakeCPNatGateway struct { + id string + tags map[string]string + state string + addrs []*ec2.NatGatewayAddress +} + +type fakeNatGatewayProvisioner struct { + gws []*fakeCPNatGateway + nextNGW int + + createCalls []*ec2.CreateNatGatewayInput + deleteCalls []*ec2.DeleteNatGatewayInput + + createErr error +} + +func newFakeNatGatewayProvisioner() *fakeNatGatewayProvisioner { return &fakeNatGatewayProvisioner{} } + +func (f *fakeNatGatewayProvisioner) DescribeNatGateways(input *ec2.DescribeNatGatewaysInput, _ string) (*ec2.DescribeNatGatewaysOutput, error) { + var out []*ec2.NatGateway + for _, ng := range f.gws { + if !cpvpcMatchTagFilters(ng.tags, input.Filter) { + continue + } + if !cpvpcMatchScalarFilter(input.Filter, "state", ng.state) { + continue + } + out = append(out, &ec2.NatGateway{ + NatGatewayId: aws.String(ng.id), + State: aws.String(ng.state), + Tags: ec2TagsFromMap(ng.tags), + NatGatewayAddresses: ng.addrs, + }) + } + return &ec2.DescribeNatGatewaysOutput{NatGateways: out}, nil +} + +func (f *fakeNatGatewayProvisioner) CreateNatGateway(input *ec2.CreateNatGatewayInput, _ string) (*ec2.CreateNatGatewayOutput, error) { + f.createCalls = append(f.createCalls, input) + if f.createErr != nil { + return nil, f.createErr + } + f.nextNGW++ + ng := &fakeCPNatGateway{ + id: fmt.Sprintf("nat-cp%04d", f.nextNGW), + tags: cpvpcTagsFromSpecs(input.TagSpecifications), + state: "available", + addrs: []*ec2.NatGatewayAddress{{ + AllocationId: input.AllocationId, + PublicIp: aws.String("203.0.113.50"), + }}, + } + f.gws = append(f.gws, ng) + return &ec2.CreateNatGatewayOutput{NatGateway: &ec2.NatGateway{NatGatewayId: aws.String(ng.id), State: aws.String(ng.state)}}, nil +} + +func (f *fakeNatGatewayProvisioner) DeleteNatGateway(input *ec2.DeleteNatGatewayInput, _ string) (*ec2.DeleteNatGatewayOutput, error) { + f.deleteCalls = append(f.deleteCalls, input) + id := aws.StringValue(input.NatGatewayId) + for _, ng := range f.gws { + if ng.id == id { + ng.state = "deleted" + } + } + return &ec2.DeleteNatGatewayOutput{}, nil +} diff --git a/spinifex/handlers/eks/create_cluster_test.go b/spinifex/handlers/eks/create_cluster_test.go index 93f4a1bd7..4d7884f25 100644 --- a/spinifex/handlers/eks/create_cluster_test.go +++ b/spinifex/handlers/eks/create_cluster_test.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/eks" + "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/tags" "github.com/stretchr/testify/assert" @@ -105,34 +106,34 @@ func TestCreateCluster_FailedCreateThenDeleteReclaimsNLB(t *testing.T) { assert.ErrorIs(t, getErr, ErrClusterNotFound) } -// A post-placement failure (egress allocation) must leave the just-launched +// A post-placement failure (NLB target registration) 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) { +func TestCreateCluster_PostPlacementFailureLeavesControlPlaneRecorded(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") + // Placement succeeds and launches the CP VM, then target registration fails. + f.nlb.registerErr = errors.New("TargetGroupNotFound") _, err := f.svc.CreateCluster(createInput("alpha"), testAccountID, "") require.NoError(t, err) f.svc.WaitLaunches() - require.NotEmpty(t, f.inst.launchCalls, "control-plane VM was launched before the egress step") + require.NotEmpty(t, f.inst.launchCalls, "control-plane VM was launched before the register-targets 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") + assert.NotEmpty(t, meta.ControlPlaneInstanceID, "CP instance ID must be persisted before the register-targets step") + assert.NotEmpty(t, meta.ControlPlaneNodes, "CP node list must be persisted before the register-targets 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) { +// End-to-end: a create that fails after placement, then delete-cluster, must +// terminate the control-plane VM the partial create launched — driven by the CP +// refs the early persist recorded. +func TestCreateCluster_PostPlacementFailedThenDeleteTerminatesControlPlane(t *testing.T) { f := newEKSServiceFixture(t) - f.eip.allocateErr = errors.New("no addresses in hidden pool") + f.nlb.registerErr = errors.New("TargetGroupNotFound") _, err := f.svc.CreateCluster(createInput("alpha"), testAccountID, "") require.NoError(t, err) @@ -147,6 +148,29 @@ func TestCreateCluster_EgressFailedThenDeleteTerminatesControlPlane(t *testing.T assert.ErrorIs(t, getErr, ErrClusterNotFound) } +// A create whose async launch fails after the NLB is provisioned must eagerly +// tear down the infra it already built — EKS names are unique per create, so no +// same-name retry ever fires the FAILED-cluster reclaim, and the internet-facing +// LB VM would otherwise keep its allocated+associated EIP indefinitely. The +// FAILED meta is retained for observability; purge is idempotent so a later +// DeleteCluster re-purge is a no-op (mulga-siv-293). +func TestCreateCluster_FailedLaunchEagerlyPurgesInfra(t *testing.T) { + f := newEKSServiceFixture(t) + f.inst.launchErr = errors.New("no capacity") + + _, err := f.svc.CreateCluster(createInput("alpha"), testAccountID, "") + require.NoError(t, err) + f.svc.WaitLaunches() + + require.NotEmpty(t, f.nlb.createLBCalls, "launch provisioned an NLB before failing") + // No DeleteCluster here: the failure handler itself must reap the NLB/LB VM. + assert.NotEmpty(t, f.nlb.deleteLBCalls, "failed launch must eagerly tear down the NLB it provisioned") + + meta, getErr := GetClusterMeta(f.kv, "alpha") + require.NoError(t, getErr, "FAILED meta is retained for observability") + assert.Equal(t, ClusterStatusFailed, meta.Status) +} + // 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) { @@ -307,21 +331,37 @@ func TestCreateCluster_SkipsCreatorAdminWhenDisabled(t *testing.T) { assert.ErrorIs(t, err, ErrAccessEntryNotFound) } -func TestCreateCluster_AllocatesHiddenEgressEIP(t *testing.T) { +// CreateCluster builds the managed control-plane VPC ("Set B") under the system +// account — the AWS-managed-account analogue the customer never provisions. The +// control-plane runs in its private subnet and egresses via the CP VPC's NAT +// gateway (no per-cluster hidden-pool EIP), so the persisted meta carries the CP +// VPC refs and the NAT gateway is provisioned exactly once. +func TestCreateCluster_BuildsManagedCPVPCUnderSystemAccount(t *testing.T) { f := newEKSServiceFixture(t) - _, err := f.svc.CreateCluster(createInput("egress"), testAccountID, "") + _, err := f.svc.CreateCluster(createInput("setb"), testAccountID, "") require.NoError(t, err) f.svc.WaitLaunches() - meta, err := GetClusterMeta(f.kv, "egress") + meta, err := GetClusterMeta(f.kv, "setb") require.NoError(t, err) - assert.Equal(t, f.eip.publicIP, meta.EgressEIPPublicIP) - assert.Equal(t, f.eip.allocationID, meta.EgressEIPAllocationID) + require.NotNil(t, meta.ManagedCPVPC, "managed CP VPC refs must be persisted") + assert.NotEmpty(t, meta.ManagedCPVPC.VpcId) + assert.NotEmpty(t, meta.ManagedCPVPC.PublicSubnetId) + require.NotEmpty(t, meta.ManagedCPVPC.PrivateSubnetIds) + assert.NotEmpty(t, meta.ManagedCPVPC.NatGatewayId, "private CP subnet egresses via the NAT gateway") - require.Len(t, f.eip.allocateCalls, 1) - // The pool address is tagged ManagedBy=eks so it stays out of customer - // DescribeAddresses listings. + // The CP VPC is composed under the system account, not the caller's. + require.NotEmpty(t, f.vpcMgr.createVpcAccts) + for _, acct := range f.vpcMgr.createVpcAccts { + assert.Equal(t, admin.SystemAccountID(), acct, "managed CP VPC must be owned by the system account") + } + assert.Len(t, f.ngw.createCalls, 1, "exactly one NAT gateway for the CP VPC") + + // No per-cluster hidden-pool egress EIP is allocated anymore; the only EIP is + // the NAT gateway's, tagged ManagedBy=eks so it stays out of customer listings. + assert.Empty(t, meta.EgressEIPAllocationID, "hidden-pool egress EIP is gone — egress is via the NAT gateway") + require.Len(t, f.eip.allocateCalls, 1, "one EIP: the NAT gateway address") tagged := false for _, spec := range f.eip.allocateCalls[0].TagSpecifications { for _, tg := range spec.Tags { @@ -330,5 +370,5 @@ func TestCreateCluster_AllocatesHiddenEgressEIP(t *testing.T) { } } } - assert.True(t, tagged, "egress EIP must carry the ManagedBy=eks tag") + assert.True(t, tagged, "NAT gateway EIP must carry the ManagedBy=eks tag") } diff --git a/spinifex/handlers/eks/delete_cluster_test.go b/spinifex/handlers/eks/delete_cluster_test.go index 8b50de921..edf92b17c 100644 --- a/spinifex/handlers/eks/delete_cluster_test.go +++ b/spinifex/handlers/eks/delete_cluster_test.go @@ -38,6 +38,10 @@ type eksServiceFixture struct { eip *fakeEIPProvisioner sg *fakeSGProvisioner worker *fakeWorkerLauncher + vpcMgr *fakeVPCProvisioner + igw *fakeIGWProvisioner + ngw *fakeNatGatewayProvisioner + rt *fakeRouteTableProvisioner } func newEKSServiceFixture(t *testing.T) *eksServiceFixture { @@ -53,6 +57,10 @@ func newEKSServiceFixture(t *testing.T) *eksServiceFixture { eip := newFakeEIPProvisioner() sg := newFakeSGProvisioner() worker := newFakeWorkerLauncher() + vpcMgr := newFakeVPCProvisioner() + igw := newFakeIGWProvisioner() + ngw := newFakeNatGatewayProvisioner() + rt := newFakeRouteTableProvisioner() svc, err := NewEKSServiceImpl(EKSServiceDeps{ NATSConn: nc, @@ -72,13 +80,17 @@ func newEKSServiceFixture(t *testing.T) *eksServiceFixture { Image: ami, EIP: eip, Worker: worker, + VPCMgr: vpcMgr, + IGW: igw, + NATGW: ngw, + RouteTable: rt, PlacementGroup: &fakePlacer{}, Scheduler: &fakeHostScheduler{}, }) require.NoError(t, err) t.Cleanup(svc.Shutdown) - return &eksServiceFixture{svc: svc, kv: kv, nlb: nlb, inst: inst, vpc: vpc, ami: ami, eip: eip, sg: sg, worker: worker} + return &eksServiceFixture{svc: svc, kv: kv, nlb: nlb, inst: inst, vpc: vpc, ami: ami, eip: eip, sg: sg, worker: worker, vpcMgr: vpcMgr, igw: igw, ngw: ngw, rt: rt} } // deleteClusterFixture is an eksServiceFixture pre-seeded with a CREATING diff --git a/spinifex/handlers/eks/igw.go b/spinifex/handlers/eks/igw.go new file mode 100644 index 000000000..8de6311fd --- /dev/null +++ b/spinifex/handlers/eks/igw.go @@ -0,0 +1,141 @@ +package handlers_eks + +import ( + "errors" + "fmt" + "log/slog" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/tags" +) + +// igwProvisioner is the narrow Internet Gateway surface the cluster IGW helpers +// need. An internet-facing cluster endpoint allocates an external front-end IP +// and an OVN dnat_and_snat for it, but the external switch + localnet + gateway +// LRP that make that IP answerable on the physical wire are built only when an +// IGW is attached to the VPC. The control-plane NLB is a system instance — no +// customer provisions the VPC's IGW for it — so EKS ensures one itself. The +// daemon adapts the concrete IGW service onto this. +type igwProvisioner interface { + DescribeInternetGateways(input *ec2.DescribeInternetGatewaysInput, accountID string) (*ec2.DescribeInternetGatewaysOutput, error) + CreateInternetGateway(input *ec2.CreateInternetGatewayInput, accountID string) (*ec2.CreateInternetGatewayOutput, error) + AttachInternetGateway(input *ec2.AttachInternetGatewayInput, accountID string) (*ec2.AttachInternetGatewayOutput, error) + DetachInternetGateway(input *ec2.DetachInternetGatewayInput, accountID string) (*ec2.DetachInternetGatewayOutput, error) + DeleteInternetGateway(input *ec2.DeleteInternetGatewayInput, accountID string) (*ec2.DeleteInternetGatewayOutput, error) +} + +// EnsureClusterIGW guarantees vpcID has an attached Internet Gateway so an +// internet-facing cluster endpoint is reachable on the external network. +// Idempotent and non-destructive: if the VPC already has an attached IGW +// (customer-provisioned or from a prior launch) it is reused as-is and left +// untagged; only when none exists is a cluster-owned IGW created and attached. +func EnsureClusterIGW(igwp igwProvisioner, accountID, vpcID, clusterName string) error { + if vpcID == "" { + return errors.New("eks: EnsureClusterIGW empty vpc id") + } + if clusterName == "" { + return errors.New("eks: EnsureClusterIGW empty cluster name") + } + + existing, err := attachedVPCIGW(igwp, accountID, vpcID) + if err != nil { + return err + } + if existing != nil { + return nil + } + + out, err := igwp.CreateInternetGateway(&ec2.CreateInternetGatewayInput{ + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("internet-gateway"), + Tags: []*ec2.Tag{ + {Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByEKS)}, + {Key: aws.String(clusterEKSClusterTagKey), Value: aws.String(clusterName)}, + }, + }}, + }, accountID) + if err != nil { + return fmt.Errorf("eks: create cluster IGW for vpc %s: %w", vpcID, err) + } + if out == nil || out.InternetGateway == nil || aws.StringValue(out.InternetGateway.InternetGatewayId) == "" { + return fmt.Errorf("eks: create cluster IGW for vpc %s: empty gateway id", vpcID) + } + igwID := aws.StringValue(out.InternetGateway.InternetGatewayId) + + if _, err := igwp.AttachInternetGateway(&ec2.AttachInternetGatewayInput{ + InternetGatewayId: aws.String(igwID), + VpcId: aws.String(vpcID), + }, accountID); err != nil { + return fmt.Errorf("eks: attach cluster IGW %s to vpc %s: %w", igwID, vpcID, err) + } + slog.Info("EnsureClusterIGW: attached cluster IGW", "igw", igwID, "vpc", vpcID, "cluster", clusterName) + return nil +} + +// DeleteClusterIGW detaches and deletes the cluster-owned IGW attached to vpcID. +// Best-effort and ownership-scoped: it only removes an IGW that carries this +// cluster's managed-by tag, so a customer-provisioned IGW reused by +// EnsureClusterIGW is never deleted. +func DeleteClusterIGW(igwp igwProvisioner, accountID, vpcID, clusterName string) error { + if vpcID == "" || clusterName == "" { + return errors.New("eks: DeleteClusterIGW empty vpc id or cluster name") + } + + igw, err := attachedVPCIGW(igwp, accountID, vpcID) + if err != nil { + slog.Warn("DeleteClusterIGW: IGW lookup failed", "vpc", vpcID, "err", err) + return nil + } + if igw == nil || !ownedByCluster(igw, clusterName) { + return nil + } + igwID := aws.StringValue(igw.InternetGatewayId) + + if _, err := igwp.DetachInternetGateway(&ec2.DetachInternetGatewayInput{ + InternetGatewayId: aws.String(igwID), + VpcId: aws.String(vpcID), + }, accountID); err != nil { + slog.Warn("DeleteClusterIGW: detach failed", "igw", igwID, "vpc", vpcID, "err", err) + return nil + } + if _, err := igwp.DeleteInternetGateway(&ec2.DeleteInternetGatewayInput{ + InternetGatewayId: aws.String(igwID), + }, accountID); err != nil { + slog.Warn("DeleteClusterIGW: delete failed", "igw", igwID, "err", err) + return nil + } + slog.Info("DeleteClusterIGW: removed cluster IGW", "igw", igwID, "vpc", vpcID, "cluster", clusterName) + return nil +} + +// attachedVPCIGW returns the Internet Gateway attached to vpcID, or nil if none. +func attachedVPCIGW(igwp igwProvisioner, accountID, vpcID string) (*ec2.InternetGateway, error) { + out, err := igwp.DescribeInternetGateways(&ec2.DescribeInternetGatewaysInput{ + Filters: []*ec2.Filter{{ + Name: aws.String("attachment.vpc-id"), + Values: aws.StringSlice([]string{vpcID}), + }}, + }, accountID) + if err != nil { + return nil, fmt.Errorf("eks: describe IGWs for vpc %s: %w", vpcID, err) + } + if out == nil || len(out.InternetGateways) == 0 { + return nil, nil + } + return out.InternetGateways[0], nil +} + +// ownedByCluster reports whether igw carries this cluster's EKS managed-by tags. +func ownedByCluster(igw *ec2.InternetGateway, clusterName string) bool { + var managed, named bool + for _, t := range igw.Tags { + switch aws.StringValue(t.Key) { + case tags.ManagedByKey: + managed = aws.StringValue(t.Value) == tags.ManagedByEKS + case clusterEKSClusterTagKey: + named = aws.StringValue(t.Value) == clusterName + } + } + return managed && named +} diff --git a/spinifex/handlers/eks/igw_test.go b/spinifex/handlers/eks/igw_test.go new file mode 100644 index 000000000..2bdbb1bb4 --- /dev/null +++ b/spinifex/handlers/eks/igw_test.go @@ -0,0 +1,174 @@ +package handlers_eks + +import ( + "errors" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/tags" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fakeIGWProvisioner struct { + createCalls []*ec2.CreateInternetGatewayInput + attachCalls []*ec2.AttachInternetGatewayInput + detachCalls []*ec2.DetachInternetGatewayInput + deleteCalls []*ec2.DeleteInternetGatewayInput + describeCalls []*ec2.DescribeInternetGatewaysInput + + // gateways are returned by DescribeInternetGateways (filtered on + // attachment.vpc-id), keyed by IGW id. + gateways map[string]*ec2.InternetGateway + nextID string + createErr error +} + +var _ igwProvisioner = (*fakeIGWProvisioner)(nil) + +func newFakeIGWProvisioner() *fakeIGWProvisioner { + return &fakeIGWProvisioner{gateways: map[string]*ec2.InternetGateway{}, nextID: "igw-fake0001"} +} + +func (f *fakeIGWProvisioner) DescribeInternetGateways(input *ec2.DescribeInternetGatewaysInput, _ string) (*ec2.DescribeInternetGatewaysOutput, error) { + f.describeCalls = append(f.describeCalls, input) + var wantVPC string + for _, flt := range input.Filters { + if aws.StringValue(flt.Name) == "attachment.vpc-id" && len(flt.Values) > 0 { + wantVPC = aws.StringValue(flt.Values[0]) + } + } + var out []*ec2.InternetGateway + for _, igw := range f.gateways { + for _, att := range igw.Attachments { + if aws.StringValue(att.VpcId) == wantVPC { + out = append(out, igw) + } + } + } + return &ec2.DescribeInternetGatewaysOutput{InternetGateways: out}, nil +} + +func (f *fakeIGWProvisioner) CreateInternetGateway(input *ec2.CreateInternetGatewayInput, _ string) (*ec2.CreateInternetGatewayOutput, error) { + f.createCalls = append(f.createCalls, input) + if f.createErr != nil { + return nil, f.createErr + } + igw := &ec2.InternetGateway{ + InternetGatewayId: aws.String(f.nextID), + Tags: utils.MapToEC2Tags(utils.ExtractTags(input.TagSpecifications, "internet-gateway")), + } + f.gateways[f.nextID] = igw + return &ec2.CreateInternetGatewayOutput{InternetGateway: igw}, nil +} + +func (f *fakeIGWProvisioner) AttachInternetGateway(input *ec2.AttachInternetGatewayInput, _ string) (*ec2.AttachInternetGatewayOutput, error) { + f.attachCalls = append(f.attachCalls, input) + if igw, ok := f.gateways[aws.StringValue(input.InternetGatewayId)]; ok { + igw.Attachments = append(igw.Attachments, &ec2.InternetGatewayAttachment{ + VpcId: input.VpcId, + State: aws.String("available"), + }) + } + return &ec2.AttachInternetGatewayOutput{}, nil +} + +func (f *fakeIGWProvisioner) DetachInternetGateway(input *ec2.DetachInternetGatewayInput, _ string) (*ec2.DetachInternetGatewayOutput, error) { + f.detachCalls = append(f.detachCalls, input) + if igw, ok := f.gateways[aws.StringValue(input.InternetGatewayId)]; ok { + igw.Attachments = nil + } + return &ec2.DetachInternetGatewayOutput{}, nil +} + +func (f *fakeIGWProvisioner) DeleteInternetGateway(input *ec2.DeleteInternetGatewayInput, _ string) (*ec2.DeleteInternetGatewayOutput, error) { + f.deleteCalls = append(f.deleteCalls, input) + delete(f.gateways, aws.StringValue(input.InternetGatewayId)) + return &ec2.DeleteInternetGatewayOutput{}, nil +} + +// seedAttached registers an IGW already attached to vpcID with the given tags. +func (f *fakeIGWProvisioner) seedAttached(igwID, vpcID string, tagKV ...string) { + igw := &ec2.InternetGateway{ + InternetGatewayId: aws.String(igwID), + Attachments: []*ec2.InternetGatewayAttachment{{VpcId: aws.String(vpcID), State: aws.String("available")}}, + } + for i := 0; i+1 < len(tagKV); i += 2 { + igw.Tags = append(igw.Tags, &ec2.Tag{Key: aws.String(tagKV[i]), Value: aws.String(tagKV[i+1])}) + } + f.gateways[igwID] = igw +} + +func TestEnsureClusterIGW_FreshCreatesAndAttaches(t *testing.T) { + f := newFakeIGWProvisioner() + + require.NoError(t, EnsureClusterIGW(f, "acct", "vpc-1", "demo")) + + require.Len(t, f.createCalls, 1) + require.Len(t, f.attachCalls, 1) + assert.Equal(t, "vpc-1", aws.StringValue(f.attachCalls[0].VpcId)) + assert.Equal(t, "igw-fake0001", aws.StringValue(f.attachCalls[0].InternetGatewayId)) + + igw := f.gateways["igw-fake0001"] + require.NotNil(t, igw) + assert.True(t, ownedByCluster(igw, "demo"), "created IGW must carry cluster ownership tags") +} + +func TestEnsureClusterIGW_ExistingIsReusedNotRecreated(t *testing.T) { + f := newFakeIGWProvisioner() + f.seedAttached("igw-customer", "vpc-1") // untagged, customer-provisioned + + require.NoError(t, EnsureClusterIGW(f, "acct", "vpc-1", "demo")) + + assert.Empty(t, f.createCalls, "must not create when VPC already has an attached IGW") + assert.Empty(t, f.attachCalls) +} + +func TestEnsureClusterIGW_Idempotent(t *testing.T) { + f := newFakeIGWProvisioner() + require.NoError(t, EnsureClusterIGW(f, "acct", "vpc-1", "demo")) + require.NoError(t, EnsureClusterIGW(f, "acct", "vpc-1", "demo")) + assert.Len(t, f.createCalls, 1, "second call reuses the IGW created by the first") +} + +func TestDeleteClusterIGW_RemovesOnlyOwned(t *testing.T) { + f := newFakeIGWProvisioner() + f.seedAttached("igw-owned", "vpc-1", tags.ManagedByKey, tags.ManagedByEKS, clusterEKSClusterTagKey, "demo") + + require.NoError(t, DeleteClusterIGW(f, "acct", "vpc-1", "demo")) + + require.Len(t, f.detachCalls, 1) + require.Len(t, f.deleteCalls, 1) + assert.Equal(t, "igw-owned", aws.StringValue(f.deleteCalls[0].InternetGatewayId)) + assert.NotContains(t, f.gateways, "igw-owned") +} + +func TestDeleteClusterIGW_LeavesCustomerIGW(t *testing.T) { + f := newFakeIGWProvisioner() + f.seedAttached("igw-customer", "vpc-1") // untagged + + require.NoError(t, DeleteClusterIGW(f, "acct", "vpc-1", "demo")) + + assert.Empty(t, f.detachCalls, "must not touch a customer-provisioned IGW") + assert.Empty(t, f.deleteCalls) + assert.Contains(t, f.gateways, "igw-customer") +} + +func TestDeleteClusterIGW_WrongClusterTagNotOwned(t *testing.T) { + f := newFakeIGWProvisioner() + f.seedAttached("igw-other", "vpc-1", tags.ManagedByKey, tags.ManagedByEKS, clusterEKSClusterTagKey, "other-cluster") + + require.NoError(t, DeleteClusterIGW(f, "acct", "vpc-1", "demo")) + + assert.Empty(t, f.deleteCalls, "must not delete another cluster's IGW") +} + +func TestEnsureClusterIGW_CreateErrorPropagates(t *testing.T) { + f := newFakeIGWProvisioner() + f.createErr = errors.New("boom") + err := EnsureClusterIGW(f, "acct", "vpc-1", "demo") + require.Error(t, err) + assert.Contains(t, err.Error(), "create cluster IGW") +} diff --git a/spinifex/handlers/eks/k3s_server_vm.go b/spinifex/handlers/eks/k3s_server_vm.go index cb347a6bd..2276b4c62 100644 --- a/spinifex/handlers/eks/k3s_server_vm.go +++ b/spinifex/handlers/eks/k3s_server_vm.go @@ -83,9 +83,16 @@ const ( k3sResolvConf = "nameserver 1.1.1.1\nnameserver 8.8.8.8" ) -// K3sServerInput is the K3s server VM launcher's input shape. +// K3sServerInput is the launcher's input shape. AccountID is the INFRA account +// the ENI + VM are created under: the system account, since the control plane +// lives in the managed CP VPC (Set B). ClusterAccountID is the cluster-OWNER +// (customer) account that owns the cluster meta and namespaces its mgmt-plane +// identity — the VM bakes it into EKS_ACCOUNT_ID so its bootstrap publish, state +// report, and add-on fetch reach the customer cluster, not the system account. +// Region is carried for future region-aware AMI lookups but not consumed today. type K3sServerInput struct { AccountID string + ClusterAccountID string ClusterName string Region string SubnetID string @@ -304,6 +311,8 @@ func validateK3sServerInput(in K3sServerInput) error { switch { case in.AccountID == "": return errors.New("eks: K3sServerInput empty AccountID") + case in.ClusterAccountID == "": + return errors.New("eks: K3sServerInput empty ClusterAccountID") case in.ClusterName == "": return errors.New("eks: K3sServerInput empty ClusterName") case in.SubnetID == "": @@ -381,7 +390,7 @@ func buildK3sUserData(in K3sServerInput) string { "EKS_ACCESS_KEY=" + in.AccessKey, "EKS_SECRET_KEY=" + in.SecretKey, "EKS_REGION=" + in.Region, - "EKS_ACCOUNT_ID=" + in.AccountID, + "EKS_ACCOUNT_ID=" + in.ClusterAccountID, "EKS_CLUSTER_NAME=" + in.ClusterName, "EKS_NLB_ENDPOINT=" + nlbEndpoint, "EKS_OIDC_ISSUER=" + in.OIDCIssuer, diff --git a/spinifex/handlers/eks/k3s_server_vm_test.go b/spinifex/handlers/eks/k3s_server_vm_test.go index ca167e950..ec404b0dd 100644 --- a/spinifex/handlers/eks/k3s_server_vm_test.go +++ b/spinifex/handlers/eks/k3s_server_vm_test.go @@ -115,7 +115,8 @@ func (f *fakeK3sAMI) DescribeImages(input *ec2.DescribeImagesInput, _ string) (* func validK3sInput() K3sServerInput { return K3sServerInput{ - AccountID: "111122223333", + AccountID: "000000000000", + ClusterAccountID: "111122223333", ClusterName: "alpha", Region: "us-east-1", SubnetID: "subnet-aaa", @@ -143,6 +144,7 @@ func TestLaunchK3sServerVM_EmptyInputsRejected(t *testing.T) { in K3sServerInput }{ {"empty AccountID", mk(func(in *K3sServerInput) { in.AccountID = "" })}, + {"empty ClusterAccountID", mk(func(in *K3sServerInput) { in.ClusterAccountID = "" })}, {"empty ClusterName", mk(func(in *K3sServerInput) { in.ClusterName = "" })}, {"empty SubnetID", mk(func(in *K3sServerInput) { in.SubnetID = "" })}, {"empty ControlPlaneSGID", mk(func(in *K3sServerInput) { in.ControlPlaneSGID = "" })}, @@ -235,7 +237,7 @@ func TestLaunchK3sServerVM_HappyPath(t *testing.T) { assert.Equal(t, tags.ManagedByEKS, runIn.ManagedBy) assert.Equal(t, "ami-eks-server-001", runIn.ImageID) assert.Equal(t, defaultK3sServerInstanceType, runIn.InstanceType) - assert.Equal(t, "111122223333", runIn.AccountID) + assert.Equal(t, "000000000000", runIn.AccountID, "VM launches under the infra (system) account") assert.Equal(t, "eni-aaa111", runIn.ENIID) assert.Equal(t, "10.0.1.42", runIn.ENIIP) @@ -315,7 +317,12 @@ func TestLaunchK3sServerVM_UserDataContainsAllArtifacts(t *testing.T) { assert.Contains(t, udata, "EKS_ACCESS_KEY=AKIAEXAMPLE") assert.Contains(t, udata, "EKS_SECRET_KEY=s3cr3t-key") assert.Contains(t, udata, "EKS_REGION=us-east-1") + // EKS_ACCOUNT_ID is the cluster-OWNER account (ClusterAccountID), not the + // infra account the VM is launched under — the on-VM agents namespace their + // bootstrap publish / state report / add-on fetch by it, so it must reach the + // customer cluster, not the system account that owns the CP VPC + VM. assert.Contains(t, udata, "EKS_ACCOUNT_ID=111122223333") + assert.NotContains(t, udata, "EKS_ACCOUNT_ID=000000000000") assert.Contains(t, udata, "EKS_CLUSTER_NAME=alpha") assert.Contains(t, udata, "EKS_NLB_ENDPOINT=https://eks-alpha-lb-001.us-east-1.elb.spinifex.local:443") assert.Contains(t, udata, "EKS_OIDC_ISSUER=https://oidc.spinifex.local/clusters/111122223333/alpha") diff --git a/spinifex/handlers/eks/nlb.go b/spinifex/handlers/eks/nlb.go index efa735ee5..d90d271a4 100644 --- a/spinifex/handlers/eks/nlb.go +++ b/spinifex/handlers/eks/nlb.go @@ -13,6 +13,7 @@ import ( // nlbProvisioner is the narrow ELBv2 surface needed by cluster NLB helpers. type nlbProvisioner interface { CreateLoadBalancer(input *elbv2.CreateLoadBalancerInput, accountID string) (*elbv2.CreateLoadBalancerOutput, error) + CreateLoadBalancerSync(input *elbv2.CreateLoadBalancerInput, accountID string) (*elbv2.CreateLoadBalancerOutput, error) DescribeLoadBalancers(input *elbv2.DescribeLoadBalancersInput, accountID string) (*elbv2.DescribeLoadBalancersOutput, error) DeleteLoadBalancer(input *elbv2.DeleteLoadBalancerInput, accountID string) (*elbv2.DeleteLoadBalancerOutput, error) @@ -61,9 +62,27 @@ func ClusterTargetGroupName(clusterName string) string { return "eks-" + clusterName + "-cp" } -// EnsureClusterNLB provisions (or reuses) the cluster NLB, target group, and -// :443→:6443 TCP listener. Idempotent; ENI registration is done separately by -// RegisterClusterTarget. publicAccessCidrs narrows ingress for internet-facing LBs. +// EnsureClusterNLB provisions (or returns) the cluster's NLB + target group + +// :443→:6443 TCP listener. Idempotent on clusterName: existing resources are +// reused so DeleteCluster failure-retry and reconciler crash-recovery both +// converge without duplicate creates. +// +// The K3s server VM ENI IP is NOT registered here — Stage 4 +// (k3s_server_vm.go) calls RegisterClusterTarget once the VM has its ENI IP. +// +// internetFacing selects the NLB scheme: true ⇒ internet-facing (external-pool +// front-end IP, LAN-reachable, for a public cluster endpoint); false ⇒ internal +// (VPC-only). The LB is created synchronously (CreateLoadBalancerSync), so its +// reachable FrontendIP is known on return and used as the endpoint host + cert +// SAN. A cluster whose NLB yields no front-end IP has no usable apiserver +// endpoint, so EnsureClusterNLB fails loud with ErrClusterNLBFrontendIPUnavailable +// rather than ship the unresolvable DNS-name fallback. +// +// publicAccessCidrs narrows who may reach an internet-facing front-end below the +// scheme default (0.0.0.0/0). It maps the cluster's publicAccessCidrs onto the +// NLB managed-SG ingress; ignored for an internal NLB (its ingress already +// tracks the VPC CIDR) and for the wide-open default, which the LB carries out +// of the box. func EnsureClusterNLB(nlbp nlbProvisioner, accountID, clusterName string, subnetIDs []string, internetFacing bool, publicAccessCidrs []string) (*ClusterNLB, error) { if clusterName == "" { return nil, errors.New("eks: EnsureClusterNLB empty cluster name") @@ -96,16 +115,25 @@ func EnsureClusterNLB(nlbp nlbProvisioner, accountID, clusterName string, subnet return nil, fmt.Errorf("eks: set NLB ingress CIDRs for %s: %w", lbName, err) } } - // Best-effort front-end IP read-back; caller falls back to DNS name if empty. - if lb, err := lookupLBByName(nlbp, accountID, lbName); err == nil && lb != nil { - out.FrontendIP = frontendIPFromLB(lb, internetFacing) - } else if err != nil { - slog.Warn("EnsureClusterNLB: front-end IP read-back failed", "name", lbName, "err", err) + // FrontendIP is set synchronously by ensureClusterLB. An empty value means the + // backing LB never got a reachable address (e.g. no external IP pool), which + // would leave the apiserver cert SANed only to the unresolvable DNS name — + // fail the launch loud instead. + if out.FrontendIP == "" { + return nil, fmt.Errorf("eks: NLB %s: %w", lbName, ErrClusterNLBFrontendIPUnavailable) } return out, nil } -// narrowsPublicAccess reports whether publicAccessCidrs restrict below the 0.0.0.0/0 default. +// ErrClusterNLBFrontendIPUnavailable is returned by EnsureClusterNLB when the +// cluster's NLB came up without a reachable front-end IP, so no usable apiserver +// endpoint (with a matching cert SAN) can be published. +var ErrClusterNLBFrontendIPUnavailable = errors.New("eks: cluster NLB has no reachable front-end IP") + +// narrowsPublicAccess reports whether publicAccessCidrs restrict the front-end +// below the wide-open default an internet-facing NLB already carries. Empty (no +// override) or the lone 0.0.0.0/0 default are no-ops and skip the extra ELBv2 +// round-trip. func narrowsPublicAccess(cidrs []string) bool { if len(cidrs) == 0 { return false @@ -244,8 +272,11 @@ func ensureClusterLB(nlbp nlbProvisioner, accountID, clusterName, lbName string, if lb, err := lookupLBByName(nlbp, accountID, lbName); err != nil { return err } else if lb != nil { + // Idempotent re-entry: the LB already exists (and, since creation is + // synchronous, has already been launched), so its address is on the record. out.LoadBalancerArn = aws.StringValue(lb.LoadBalancerArn) out.DNSName = aws.StringValue(lb.DNSName) + out.FrontendIP = frontendIPFromLB(lb, internetFacing) if out.LoadBalancerArn == "" || out.DNSName == "" { return fmt.Errorf("eks: existing NLB %s missing arn or DNS name", lbName) } @@ -256,7 +287,11 @@ func ensureClusterLB(nlbp nlbProvisioner, accountID, clusterName, lbName string, if internetFacing { scheme = elbv2.LoadBalancerSchemeEnumInternetFacing } - created, err := nlbp.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + // Synchronous create: the cluster launch runs off the gateway responder, so it + // awaits the data-plane launch and gets back an LB carrying its front-end IP — + // which must be baked into the apiserver cert SAN before the control-plane VM + // boots (the async CreateLoadBalancer would return before the IP is allocated). + created, err := nlbp.CreateLoadBalancerSync(&elbv2.CreateLoadBalancerInput{ Name: aws.String(lbName), Type: aws.String(elbv2.LoadBalancerTypeEnumNetwork), Scheme: aws.String(scheme), @@ -275,6 +310,7 @@ func ensureClusterLB(nlbp nlbProvisioner, accountID, clusterName, lbName string, lb := created.LoadBalancers[0] out.LoadBalancerArn = aws.StringValue(lb.LoadBalancerArn) out.DNSName = aws.StringValue(lb.DNSName) + out.FrontendIP = frontendIPFromLB(lb, internetFacing) if out.LoadBalancerArn == "" || out.DNSName == "" { return fmt.Errorf("eks: CreateLoadBalancer returned empty arn or DNS name for %s", lbName) } diff --git a/spinifex/handlers/eks/nlb_test.go b/spinifex/handlers/eks/nlb_test.go index 263f2d320..f04b567e2 100644 --- a/spinifex/handlers/eks/nlb_test.go +++ b/spinifex/handlers/eks/nlb_test.go @@ -14,6 +14,7 @@ import ( type fakeNLBProvisioner struct { createLBCalls []*elbv2.CreateLoadBalancerInput + createLBSyncCalls []*elbv2.CreateLoadBalancerInput describeLBCalls []*elbv2.DescribeLoadBalancersInput deleteLBCalls []*elbv2.DeleteLoadBalancerInput createTGCalls []*elbv2.CreateTargetGroupInput @@ -25,6 +26,11 @@ type fakeNLBProvisioner struct { deregisterCalls []*elbv2.DeregisterTargetsInput setIngressCalls []setIngressCIDRsCall + // frontendIP is the address CreateLoadBalancerSync stamps onto the launched + // LB's AZ addresses (both public + private slots, so frontendIPFromLB resolves + // it for either scheme). Empty models a box with no external IP pool. + frontendIP string + lbByName map[string]*elbv2.LoadBalancer tgByName map[string]*elbv2.TargetGroup listenerByPort map[string]map[int64]*elbv2.Listener // lbArn → port → listener @@ -51,12 +57,34 @@ var _ nlbProvisioner = (*fakeNLBProvisioner)(nil) func newFakeNLBProvisioner() *fakeNLBProvisioner { return &fakeNLBProvisioner{ + frontendIP: "10.0.0.10", lbByName: map[string]*elbv2.LoadBalancer{}, tgByName: map[string]*elbv2.TargetGroup{}, listenerByPort: map[string]map[int64]*elbv2.Listener{}, } } +// CreateLoadBalancerSync models the synchronous path: it creates the LB (sharing +// CreateLoadBalancer's recording + storage) and stamps the launched front-end +// address onto the returned + stored LB, as a real sync launch would. +func (f *fakeNLBProvisioner) CreateLoadBalancerSync(input *elbv2.CreateLoadBalancerInput, accountID string) (*elbv2.CreateLoadBalancerOutput, error) { + f.createLBSyncCalls = append(f.createLBSyncCalls, input) + out, err := f.CreateLoadBalancer(input, accountID) + if err != nil || out == nil || len(out.LoadBalancers) == 0 { + return out, err + } + lb := out.LoadBalancers[0] + if f.frontendIP != "" && len(lb.AvailabilityZones) == 0 { + lb.AvailabilityZones = []*elbv2.AvailabilityZone{{ + LoadBalancerAddresses: []*elbv2.LoadBalancerAddress{{ + IpAddress: aws.String(f.frontendIP), + PrivateIPv4Address: aws.String(f.frontendIP), + }}, + }} + } + return out, nil +} + func (f *fakeNLBProvisioner) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInput, _ string) (*elbv2.CreateLoadBalancerOutput, error) { f.createLBCalls = append(f.createLBCalls, input) if f.createLBErr != nil { @@ -258,6 +286,9 @@ func TestEnsureClusterNLB_FreshCreatesAllThree(t *testing.T) { assert.NotEmpty(t, out.TargetGroupArn) assert.NotEmpty(t, out.ListenerArn) assert.Contains(t, out.DNSName, "eks-alpha") + // Synchronous create returns the launched front-end IP — no read-back race. + assert.Equal(t, "10.0.0.10", out.FrontendIP) + assert.Len(t, nlbp.createLBSyncCalls, 1) require.Len(t, nlbp.createLBCalls, 1) lbIn := nlbp.createLBCalls[0] @@ -285,6 +316,28 @@ func TestEnsureClusterNLB_FreshCreatesAllThree(t *testing.T) { assert.Equal(t, out.TargetGroupArn, aws.StringValue(lstIn.DefaultActions[0].TargetGroupArn)) } +func TestEnsureClusterNLB_NoFrontendIPFailsLoud(t *testing.T) { + nlbp := newFakeNLBProvisioner() + nlbp.frontendIP = "" // no external IP pool → LB comes up without a reachable address + + _, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, true, nil) + require.Error(t, err) + assert.ErrorIs(t, err, ErrClusterNLBFrontendIPUnavailable) + // The LB was still created (sync attempt) before the loud fail. + assert.Len(t, nlbp.createLBSyncCalls, 1) +} + +func TestEnsureClusterNLB_InternetFacingUsesPublicAddress(t *testing.T) { + nlbp := newFakeNLBProvisioner() + nlbp.frontendIP = "203.0.113.7" + + out, err := EnsureClusterNLB(nlbp, "111122223333", "alpha", []string{"subnet-aaa"}, true, nil) + require.NoError(t, err) + assert.Equal(t, "203.0.113.7", out.FrontendIP) + require.Len(t, nlbp.createLBSyncCalls, 1) + assert.Equal(t, elbv2.LoadBalancerSchemeEnumInternetFacing, aws.StringValue(nlbp.createLBSyncCalls[0].Scheme)) +} + func TestEnsureClusterNLB_IdempotentReusesExisting(t *testing.T) { nlbp := newFakeNLBProvisioner() diff --git a/spinifex/handlers/eks/service.go b/spinifex/handlers/eks/service.go index 5938bbaff..331bafab0 100644 --- a/spinifex/handlers/eks/service.go +++ b/spinifex/handlers/eks/service.go @@ -39,6 +39,10 @@ type EKSService interface { DeleteAddon(input *eks.DeleteAddonInput, accountID string) (*eks.DeleteAddonOutput, error) DescribeAddon(input *eks.DescribeAddonInput, accountID string) (*eks.DescribeAddonOutput, error) UpdateAddon(input *eks.UpdateAddonInput, accountID string) (*eks.UpdateAddonOutput, error) + // ListStagedAddonManifests is an internal control-plane method (not an + // AWS-SDK action): the on-VM addon-sync agent fetches staged manifests for a + // cluster via the internal-addons gateway route. + ListStagedAddonManifests(input *ListStagedAddonManifestsInput, accountID string) (*ListStagedAddonManifestsOutput, error) // OIDC identity-provider configs. AssociateIdentityProviderConfig(input *eks.AssociateIdentityProviderConfigInput, accountID string) (*eks.AssociateIdentityProviderConfigOutput, error) diff --git a/spinifex/handlers/eks/service_impl.go b/spinifex/handlers/eks/service_impl.go index d1cba6e36..e464544e9 100644 --- a/spinifex/handlers/eks/service_impl.go +++ b/spinifex/handlers/eks/service_impl.go @@ -17,9 +17,9 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/eks" + "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/config" - "github.com/mulgadc/spinifex/spinifex/tags" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" ) @@ -53,9 +53,19 @@ type EKSServiceDeps struct { Instance k3sInstanceLauncher Image k3sAMIResolver EIP eipProvisioner + IGW igwProvisioner Worker WorkerLauncher - // PlacementGroup and Scheduler support HA control-plane spread (NATS-backed). + // VPCMgr / NATGW / RouteTable compose the managed control-plane VPC ("Set B") + // from the real EC2 VPC-family APIs under the system account. The daemon + // adapts its concrete VPC / NAT-gateway / route-table services onto these. + VPCMgr vpcProvisioner + NATGW natGatewayProvisioner + RouteTable routeTableProvisioner + + // PlacementGroup reserves distinct hosts for HA control-plane spread; + // Scheduler answers the capacity + placement fan-out the spread path needs. + // The daemon wires the NATS implementations. PlacementGroup controlPlanePlacer Scheduler HostScheduler @@ -330,7 +340,7 @@ func (s *EKSServiceImpl) CreateCluster(input *eks.CreateClusterInput, accountID, s.launchWG.Go(func() { defer func() { if r := recover(); r != nil { - s.failClusterLaunch(acctKV, name, accountID, "launch panic", fmt.Errorf("%v", r)) + s.failClusterLaunch(acctKV, name, accountID, meta, "launch panic", fmt.Errorf("%v", r)) } }() s.launchClusterInfra(clusterLaunchCtx{ @@ -366,9 +376,26 @@ type clusterLaunchCtx struct { acctKV nats.KeyValue } -// failClusterLaunch logs a launch failure and marks the cluster FAILED so DescribeCluster surfaces the cause. -func (s *EKSServiceImpl) failClusterLaunch(kv nats.KeyValue, name, accountID, stage string, err error) { +// failClusterLaunch records an asynchronous control-plane launch failure: it +// marks the cluster FAILED (with reason, so DescribeCluster surfaces the cause +// and the 267.3 reclaim path can recreate cleanly) and logs the stage. The +// launch runs after the CREATING response was already sent, so a failure can +// only surface as status, never as the CreateCluster return value. +func (s *EKSServiceImpl) failClusterLaunch(kv nats.KeyValue, name, accountID string, meta *ClusterMeta, stage string, err error) { _ = logCreateErr(name, accountID, stage, err) + // Release any billable infra already provisioned this launch (LB VM + its + // associated EIP, egress EIP, NLB, CP VMs, OIDC) so a failed create never + // strands resources. Cluster names are unique, so the same-name reclaim that + // would otherwise purge a FAILED attempt never fires — without this, the + // internet-facing LB VM's EIP leaks permanently. Operate on the in-memory + // meta: a failure in PutClusterMeta itself leaves the freshest refs only + // here, not in the KV record. Best-effort; the FAILED status is recorded + // regardless (deleteMeta=false keeps the meta for DescribeCluster/reclaim). + if meta != nil { + if perr := s.purgeClusterInfra(accountID, name, meta, kv, false); perr != nil { + slog.Warn("failClusterLaunch: infra purge incomplete", "cluster", name, "stage", stage, "err", perr) + } + } if mErr := MarkClusterFailed(kv, name, stage+": "+err.Error()); mErr != nil && !errors.Is(mErr, ErrClusterNotFound) { slog.Warn("launchClusterInfra: MarkClusterFailed failed", "cluster", name, "err", mErr) } @@ -381,85 +408,110 @@ func (s *EKSServiceImpl) launchClusterInfra(lc clusterLaunchCtx) { callerPrincipalARN := lc.callerPrincipalARN name := lc.name region := lc.region - subnetIDs := lc.subnetIDs - vpcID := lc.vpcID publicAccess := lc.publicAccess publicCidrs := lc.publicCidrs input := lc.input meta := lc.meta acctKV := lc.acctKV - cpSG, ngSG, err := EnsureClusterSGs(s.deps.VPCSG, accountID, name, vpcID) + // Build the managed control-plane VPC ("Set B") under the system account: the + // AWS-managed-account analogue the customer never provisions. The + // internet-facing NLB lives in its public subnet and the control-plane VM(s) + // in its private subnet, so the NLB→CP hop is in-VPC (unaffected by the + // private-subnet egress drop) and the private CP egresses via the NAT gateway + // (DNS + docker.io image pulls). Composed from the real EC2 VPC-family APIs, + // so the per-subnet egress policies are wired by the topology subscribers. + sysAcct := admin.SystemAccountID() + cpRefs, err := EnsureClusterCPVPC(s.cpVPCDeps(), sysAcct, name, region, cpVPCPrivateSubnetCount) if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "ensure cluster SGs", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "ensure managed CP VPC", err) + return + } + meta.ManagedCPVPC = managedCPVPCFromRefs(cpRefs) + // Persist the CP VPC refs before any further fallible step so teardown can + // reclaim the VPC/subnets/IGW/NAT GW even if the launch fails after here. + if err := PutClusterMeta(acctKV, meta); err != nil { + s.failClusterLaunch(acctKV, name, accountID, meta, "persist managed CP VPC", err) + return + } + + cpSG, ngSG, err := EnsureClusterSGs(s.deps.VPCSG, sysAcct, name, cpRefs.VpcID) + if err != nil { + s.failClusterLaunch(acctKV, name, accountID, meta, "ensure cluster SGs", err) return } meta.ResourcesVpcConfig.SecurityGroupIds = []string{cpSG, ngSG} - vpcCIDR, err := s.deps.VPCSubnet.GetVPCCIDR(accountID, vpcID) - if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "resolve vpc cidr", err) + // The NLB's backing LB VM forwards the published endpoint to the apiserver + // from inside the CP VPC, so the control-plane SG must admit that hop (CP VPC + // CIDR) or the NLB target stays unhealthy and the endpoint never serves. + if err := EnsureControlPlaneIngress(s.deps.VPCSG, sysAcct, cpSG, cpRefs.VpcCIDR); err != nil { + s.failClusterLaunch(acctKV, name, accountID, meta, "ensure control-plane ingress", err) return } - if err := EnsureControlPlaneIngress(s.deps.VPCSG, accountID, cpSG, vpcCIDR); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "ensure control-plane ingress", err) - return - } - if err := EnsureControlPlaneHAIngress(s.deps.VPCSG, accountID, cpSG); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "ensure control-plane HA ingress", err) + // HA control planes run servers 2..N as join servers whose embedded etcd must + // peer with the quorum; without these self-referencing CP-SG rules a join + // registers but its etcd never replicates, so the node never reports Ready. + if err := EnsureControlPlaneHAIngress(s.deps.VPCSG, sysAcct, cpSG); err != nil { + s.failClusterLaunch(acctKV, name, accountID, meta, "ensure control-plane HA ingress", err) return } - nlb, err := EnsureClusterNLB(s.deps.NLB, accountID, name, subnetIDs, publicAccess, publicCidrs) + // The cluster NLB lives in the CP VPC public subnet. publicAccess selects the + // scheme (internet-facing external-pool IP vs internal VPC IP); the CP VPC IGW + // (attached by EnsureClusterCPVPC) makes an internet-facing front-end IP + // answerable on the wire. + nlb, err := EnsureClusterNLB(s.deps.NLB, sysAcct, name, []string{cpRefs.PublicSubnetID}, publicAccess, publicCidrs) if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "ensure cluster NLB", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "ensure cluster NLB", err) return } - // Prefer the NLB front-end IP for TLS SAN validation; fall back to DNS name. - endpointHost := nlb.FrontendIP - if endpointHost == "" { - endpointHost = nlb.DNSName - } - meta.Endpoint = "https://" + net.JoinHostPort(endpointHost, strconv.FormatInt(clusterNLBListenPort, 10)) + // The reachable front-end IP is the kubeconfig endpoint host so the apiserver + // serving cert (which SANs this IP) validates with TLS verification on. + // EnsureClusterNLB guarantees a non-empty FrontendIP (it fails the launch + // otherwise), so there is no DNS-name fallback to bake an unresolvable, + // non-SANed endpoint. + meta.Endpoint = "https://" + net.JoinHostPort(nlb.FrontendIP, strconv.FormatInt(clusterNLBListenPort, 10)) meta.EndpointIP = nlb.FrontendIP meta.NLBArn = nlb.LoadBalancerArn meta.NLBTargetGroupArn = nlb.TargetGroupArn // Persist NLB ARNs early so DeleteCluster can reclaim them on any later failure. if err := PutClusterMeta(acctKV, meta); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "persist NLB arns", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "persist NLB arns", err) return } oidcIssuer, err := ClusterOIDCIssuer(s.deps.GatewayBaseURL, region, accountID, name) if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "build OIDC issuer", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "build OIDC issuer", err) return } meta.OIDCIssuer = oidcIssuer privPEM, _, err := GenerateClusterOIDCKeypair(acctKV, name, s.deps.MasterKey) if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "generate OIDC keypair", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "generate OIDC keypair", err) return } pubPEM, err := PublicKeyPEMFromPrivate(privPEM) if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "derive OIDC public key", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "derive OIDC public key", err) return } joinToken, err := GenerateK3sClusterToken() if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "generate k3s cluster token", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "generate k3s cluster token", err) return } - cpNodes, spreadGroup, err := s.placeControlPlane(accountID, name, K3sServerInput{ - AccountID: accountID, + cpNodes, spreadGroup, err := s.placeControlPlane(sysAcct, name, K3sServerInput{ + AccountID: sysAcct, + ClusterAccountID: accountID, ClusterName: name, Region: region, - SubnetID: subnetIDs[0], + SubnetID: cpRefs.PrivateSubnetIDs[0], ControlPlaneSGID: cpSG, NLBDNS: nlb.DNSName, EndpointIP: nlb.FrontendIP, @@ -475,13 +527,14 @@ func (s *EKSServiceImpl) launchClusterInfra(lc clusterLaunchCtx) { }) if err != nil { if errors.Is(err, ErrEKSServerAMINotFound) { - s.failClusterLaunch(acctKV, name, accountID, "eks-server AMI not found", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "eks-server AMI not found", err) return } - s.failClusterLaunch(acctKV, name, accountID, "launch K3s VM", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "launch K3s VM", err) return } - // Mirror primary CP ([0]) into scalar fields; all nodes are NLB-registered for failover. + // Mirror the primary ([0]) into the scalar fields the reconciler and teardown + // read today. All CP nodes are NLB target-registered (failover). primary := cpNodes[0] meta.ControlPlaneNodes = cpNodes meta.ControlPlaneSpreadGroup = spreadGroup @@ -490,43 +543,31 @@ func (s *EKSServiceImpl) launchClusterInfra(lc clusterLaunchCtx) { meta.ControlPlaneENIIP = primary.ENIIP meta.ControlPlaneMgmtIP = primary.MgmtIP - // Persist CP IDs immediately; VMs are live now and must not be orphaned on later failure. + // 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 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.failClusterLaunch(acctKV, name, accountID, "persist control-plane ids", err) - return - } - - egressIP, egressAllocID, err := s.allocateClusterEgressIP(accountID, name) - if err != nil { - s.failClusterLaunch(acctKV, name, accountID, "allocate cluster egress IP", err) - return - } - meta.EgressEIPAllocationID = egressAllocID - meta.EgressEIPPublicIP = egressIP - utils.PublishEvent(s.deps.NATSConn, "vpc.add-system-egress", systemEgressEvent{ - VpcId: vpcID, - SubnetId: subnetIDs[0], - InstanceIp: primary.ENIIP, - ExternalIp: egressIP, - }) - - // Persist egress allocation so DeleteCluster can reclaim it on later failure. - if err := PutClusterMeta(acctKV, meta); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "persist egress allocation", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "persist control-plane ids", err) return } + // The control-plane VM pulls container images over the CP VPC's NAT gateway: + // it lives in the private subnet whose route table sends 0.0.0.0/0 → NAT GW, + // so no per-instance egress wiring is needed (the prior hidden-pool /32 SNAT + // bandaid is gone — the topology now expresses egress). cpENIIPs := make([]string, 0, len(cpNodes)) for _, n := range cpNodes { cpENIIPs = append(cpENIIPs, n.ENIIP) } - if err := RegisterClusterTargets(s.deps.NLB, accountID, nlb.TargetGroupArn, cpENIIPs); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "register NLB targets", err) + if err := RegisterClusterTargets(s.deps.NLB, sysAcct, nlb.TargetGroupArn, cpENIIPs); err != nil { + s.failClusterLaunch(acctKV, name, accountID, meta, "register NLB targets", err) return } if err := PutClusterMeta(acctKV, meta); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "persist final meta", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "persist final meta", err) return } @@ -535,7 +576,7 @@ func (s *EKSServiceImpl) launchClusterInfra(lc clusterLaunchCtx) { rec := newAccessEntryRecord(region, accountID, name, callerPrincipalARN, "", []string{"system:masters"}, AccessEntryTypeStandard, nil, time.Now().UTC()) if err := PutAccessEntryRecord(acctKV, rec); err != nil { - s.failClusterLaunch(acctKV, name, accountID, "seed cluster-creator admin access entry", err) + s.failClusterLaunch(acctKV, name, accountID, meta, "seed cluster-creator admin access entry", err) return } } else if bootstrapCreatorAdmin(input) { @@ -566,26 +607,6 @@ type systemEgressEvent struct { ExternalIp string `json:"external_ip"` } -// allocateClusterEgressIP allocates a hidden pool address for the cluster's -// control-plane VM egress, tagged ManagedBy=eks so it stays out of customer -// DescribeAddresses listings. -func (s *EKSServiceImpl) allocateClusterEgressIP(accountID, name string) (publicIP, allocationID string, err error) { - out, err := s.deps.EIP.AllocateAddress(&ec2.AllocateAddressInput{ - Domain: aws.String("vpc"), - TagSpecifications: []*ec2.TagSpecification{{ - ResourceType: aws.String("elastic-ip"), - Tags: []*ec2.Tag{{Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByEKS)}}, - }}, - }, accountID) - if err != nil { - return "", "", err - } - if out == nil || aws.StringValue(out.PublicIp) == "" || aws.StringValue(out.AllocationId) == "" { - return "", "", errors.New("eks: AllocateAddress returned incomplete EIP") - } - return aws.StringValue(out.PublicIp), aws.StringValue(out.AllocationId), nil -} - func (s *EKSServiceImpl) DescribeCluster(input *eks.DescribeClusterInput, accountID string) (*eks.DescribeClusterOutput, error) { name := aws.StringValue(input.Name) if name == "" { @@ -760,6 +781,14 @@ func (s *EKSServiceImpl) claimClusterName(accountID string, acctKV nats.KeyValue func (s *EKSServiceImpl) purgeClusterInfra(accountID, name string, meta *ClusterMeta, acctKV nats.KeyValue, deleteMeta bool) error { s.registry.Stop(accountID, name) + // infraAcct owns the NLB, CP VMs, and CP VPC. Clusters built with the managed + // CP VPC ("Set B") hold them under the system account; legacy clusters + // (ManagedCPVPC nil) under the customer account, matching how they launched. + infraAcct := accountID + if meta.ManagedCPVPC != nil { + infraAcct = admin.SystemAccountID() + } + var teardownErrs []error if err := ZeroizeClusterOIDCKey(acctKV, name); err != nil { @@ -770,18 +799,18 @@ func (s *EKSServiceImpl) purgeClusterInfra(accountID, name string, meta *Cluster // Deregister is best-effort: DeleteClusterNLB tears down the whole NLB // + target group, so a stale target registration cannot leak past it. if meta.NLBTargetGroupArn != "" && meta.ControlPlaneENIIP != "" { - if err := DeregisterClusterTarget(s.deps.NLB, accountID, meta.NLBTargetGroupArn, meta.ControlPlaneENIIP); err != nil { + if err := DeregisterClusterTarget(s.deps.NLB, infraAcct, meta.NLBTargetGroupArn, meta.ControlPlaneENIIP); err != nil { slog.Warn("purgeClusterInfra: deregister NLB target failed", "cluster", name, "err", err) } } - if err := DeleteClusterNLB(s.deps.NLB, accountID, name); err != nil { + if err := DeleteClusterNLB(s.deps.NLB, infraAcct, name); err != nil { teardownErrs = append(teardownErrs, fmt.Errorf("delete NLB: %w", err)) } } for _, cp := range controlPlaneTeardownNodes(meta) { if err := TerminateK3sServerVM(s.deps.VPCK3s, s.deps.Instance, - accountID, cp.InstanceID, cp.ENIID); err != nil { + infraAcct, cp.InstanceID, cp.ENIID); err != nil { teardownErrs = append(teardownErrs, fmt.Errorf("terminate K3s VM %s: %w", cp.InstanceID, err)) } } @@ -827,10 +856,31 @@ func (s *EKSServiceImpl) purgeClusterInfra(accountID, name string, meta *Cluster return errors.Join(teardownErrs...) } - if meta.ResourcesVpcConfig != nil && meta.ResourcesVpcConfig.VpcId != "" { + if meta.ManagedCPVPC != nil && meta.ManagedCPVPC.VpcId != "" { + // New topology: SGs + IGW + subnets + route tables + NAT GW + VPC all live + // in the managed CP VPC under the system account. DeleteClusterCPVPC removes + // the IGW + VPC after the SGs, so SG cleanup must precede it. + if err := DeleteClusterSGs(s.deps.VPCSG, infraAcct, name, meta.ManagedCPVPC.VpcId); err != nil { + slog.Warn("purgeClusterInfra: DeleteClusterSGs failed", "cluster", name, "err", err) + } + // The CP VPC holds a billable NAT-GW EIP, so a teardown failure is surfaced + // (not best-effort): returning before the KV sweep keeps the meta refs alive + // for a delete retry rather than orphaning the EIP. + if err := DeleteClusterCPVPC(s.cpVPCDeps(), infraAcct, name); err != nil { + return fmt.Errorf("delete managed CP VPC: %w", err) + } + } else if meta.ResourcesVpcConfig != nil && meta.ResourcesVpcConfig.VpcId != "" { + // Legacy topology: CP lived in the customer VPC; reclaim its SGs + the + // cluster-owned IGW (ownership-scoped — a reused customer IGW is left + // intact). IGW delete must precede any VPC delete to avoid DependencyViolation. if err := DeleteClusterSGs(s.deps.VPCSG, accountID, name, meta.ResourcesVpcConfig.VpcId); err != nil { slog.Warn("purgeClusterInfra: DeleteClusterSGs failed", "cluster", name, "err", err) } + if s.deps.IGW != nil { + if err := DeleteClusterIGW(s.deps.IGW, accountID, meta.ResourcesVpcConfig.VpcId, name); err != nil { + slog.Warn("purgeClusterInfra: DeleteClusterIGW failed", "cluster", name, "err", err) + } + } } // Release + delete the HA spread placement group (no-op for single-CP @@ -1457,9 +1507,11 @@ func (s *EKSServiceImpl) spawnReconciler(accountID, clusterName string, _ *Clust // k3s binds the apiserver to the VPC node-ip, unreachable from the host. The // CP publishes {healthz,node_count} on the mgmt bus the daemon already shares. stateSubject := StateSubject(accountID, clusterName) + addonStatusSubject := AddonStatusSubject(accountID, clusterName) spawn := func(ctx context.Context, _, _ string) (func(), error) { return RunClusterReconciler(ctx, s.leaderKV, acctKV, accountID, clusterName, s.deps.HolderID, "", - WithStateSource(s.deps.NATSConn, stateSubject)) + WithStateSource(s.deps.NATSConn, stateSubject), + WithAddonStatusSource(s.deps.NATSConn, addonStatusSubject)) } if err := s.registry.Spawn(s.bgCtx, accountID, clusterName, spawn); err != nil { slog.Error("spawnReconciler: registry spawn", "cluster", clusterName, "err", err) diff --git a/spinifex/handlers/eks/service_nats.go b/spinifex/handlers/eks/service_nats.go index 89534411a..bfee6c507 100644 --- a/spinifex/handlers/eks/service_nats.go +++ b/spinifex/handlers/eks/service_nats.go @@ -136,6 +136,10 @@ func (s *NATSEKSService) DescribeAddon(input *eks.DescribeAddonInput, accountID return utils.NATSRequest[eks.DescribeAddonOutput](s.natsConn, "eks.DescribeAddon", input, defaultTimeout, accountID) } +func (s *NATSEKSService) ListStagedAddonManifests(input *ListStagedAddonManifestsInput, accountID string) (*ListStagedAddonManifestsOutput, error) { + return utils.NATSRequest[ListStagedAddonManifestsOutput](s.natsConn, "eks.ListStagedAddonManifests", input, defaultTimeout, accountID) +} + func (s *NATSEKSService) UpdateAddon(input *eks.UpdateAddonInput, accountID string) (*eks.UpdateAddonOutput, error) { return utils.NATSRequest[eks.UpdateAddonOutput](s.natsConn, "eks.UpdateAddon", input, defaultTimeout, accountID) } diff --git a/spinifex/handlers/eks/state_report.go b/spinifex/handlers/eks/state_report.go index ccaa91c73..ef234144a 100644 --- a/spinifex/handlers/eks/state_report.go +++ b/spinifex/handlers/eks/state_report.go @@ -35,3 +35,49 @@ func unmarshalServerStateReport(data []byte) (ServerStateReport, error) { } return r, nil } + +// AddonStatusSubject returns the NATS subject the control-plane VM publishes a +// per-add-on delivery status to: "eks.addon.{accountID}.{clusterName}.status". +// One subject per cluster carries reports for every add-on (Addon names the +// add-on); the host-side AddonStatusReconciler CASes the matching AddonRecord. +// Distinct from StateSubject because add-on lifecycle is tracked per-resource, +// mirroring AWS EKS (DescribeAddon.status is independent of cluster status). +func AddonStatusSubject(accountID, clusterName string) string { + return fmt.Sprintf("eks.addon.%s.%s.status", accountID, clusterName) +} + +// AddonDeliveryPhase is the VM-observed delivery phase the on-VM addon-sync +// agent reports for one managed add-on. The reconciler maps it onto the +// AWS-visible AddonStatus. +type AddonDeliveryPhase string + +const ( + // AddonPhaseApplied: the rendered manifest was written to the K3s + // auto-deploy dir but pods are not yet Ready. Record stays CREATING. + AddonPhaseApplied AddonDeliveryPhase = "applied" + // AddonPhaseReady: the add-on's workloads rolled out successfully. Record + // flips to ACTIVE. + AddonPhaseReady AddonDeliveryPhase = "ready" + // AddonPhaseFailed: render or rollout failed. Record flips to DEGRADED + // (CREATE_FAILED if it never reached ACTIVE — decided by the reconciler). + AddonPhaseFailed AddonDeliveryPhase = "failed" +) + +// AddonStatusReport is the JSON payload of an add-on delivery report, emitted +// by scripts/images/eks-node/mulga-eks-addon-sync.sh via the "addon" channel of +// eks-gateway-publish. TS is the publish time in unix seconds. +type AddonStatusReport struct { + Addon string `json:"addon"` + Version string `json:"version"` + Phase AddonDeliveryPhase `json:"phase"` + Message string `json:"message,omitempty"` + TS int64 `json:"ts"` +} + +func unmarshalAddonStatusReport(data []byte) (AddonStatusReport, error) { + var r AddonStatusReport + if err := json.Unmarshal(data, &r); err != nil { + return AddonStatusReport{}, fmt.Errorf("eks: unmarshal addon status report: %w", err) + } + return r, nil +} diff --git a/spinifex/handlers/elbv2/arn_test.go b/spinifex/handlers/elbv2/arn_test.go index 617dea7bd..4947dea8b 100644 --- a/spinifex/handlers/elbv2/arn_test.go +++ b/spinifex/handlers/elbv2/arn_test.go @@ -48,7 +48,8 @@ func TestBuildLBAgentEnv(t *testing.T) { SystemSecretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", region: "ap-southeast-2", } - env := svc.buildLBAgentEnv("lb-abc123") + // A customer-account LB heartbeats over the WAN GatewayURL. + env := svc.buildLBAgentEnv("lb-abc123", "111122223333") lines := strings.Split(strings.TrimRight(env, "\n"), "\n") kvs := make(map[string]string, len(lines)) @@ -65,6 +66,26 @@ func TestBuildLBAgentEnv(t *testing.T) { assert.Equal(t, "ap-southeast-2", kvs["LB_REGION"]) } +// TestBuildLBAgentEnv_SystemAccountUsesMgmtGateway verifies a system-account LB +// (e.g. the EKS cluster control-plane NLB) heartbeats over the mgmt-bridge URL, +// not the WAN GatewayURL. +func TestBuildLBAgentEnv_SystemAccountUsesMgmtGateway(t *testing.T) { + svc := &ELBv2ServiceImpl{ + GatewayURL: "https://10.0.0.1:9999", + MgmtBridgeIP: "192.168.50.1", + SystemAccessKey: "AKIAIOSFODNN7EXAMPLE", + SystemSecretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + region: "ap-southeast-2", + } + env := svc.buildLBAgentEnv("lb-eks01", "000000000000") + assert.Contains(t, env, "LB_GATEWAY_URL=https://192.168.50.1:9999\n") + + // With no mgmt bridge, a system-account LB falls back to the WAN URL. + svc.MgmtBridgeIP = "" + env = svc.buildLBAgentEnv("lb-eks01", "000000000000") + assert.Contains(t, env, "LB_GATEWAY_URL=https://10.0.0.1:9999\n") +} + // TestSubnetCIDRForIP and TestSubnetGatewayIP cover the CIDR helpers. func TestSubnetCIDRForIP(t *testing.T) { assert.Equal(t, "10.0.1.5/24", subnetCIDRForIP("10.0.1.5", "10.0.1.0/24")) diff --git a/spinifex/handlers/elbv2/service_impl.go b/spinifex/handlers/elbv2/service_impl.go index b497a1c31..2771ed132 100644 --- a/spinifex/handlers/elbv2/service_impl.go +++ b/spinifex/handlers/elbv2/service_impl.go @@ -8,6 +8,7 @@ import ( "fmt" "log/slog" "net" + "net/url" "regexp" "slices" "sort" @@ -18,6 +19,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/config" handlers_acm "github.com/mulgadc/spinifex/spinifex/handlers/acm" @@ -246,10 +248,33 @@ func (s *ELBv2ServiceImpl) getSystemInstanceType() string { } // buildLBAgentEnv returns the KEY=value blob written to /etc/conf.d/lb-agent -// via fw_cfg on the direct-boot path. -func (s *ELBv2ServiceImpl) buildLBAgentEnv(lbID string) string { +// via fw_cfg on the direct-boot path. A system-account LB (e.g. the EKS +// cluster control-plane NLB) is a hidden system instance whose only path to the +// gateway is its on-link br-mgmt NIC, so it heartbeats over the mgmt-bridge URL +// — the same path the EKS control-plane VM uses — rather than out the WAN and +// back. A customer LB reaches the gateway via its VPC egress (the WAN URL). +func (s *ELBv2ServiceImpl) buildLBAgentEnv(lbID, accountID string) string { + gatewayURL := s.GatewayURL + if accountID == admin.SystemAccountID() { + gatewayURL = s.mgmtGatewayURL() + } return fmt.Sprintf("LB_LB_ID=%s\nLB_GATEWAY_URL=%s\nLB_ACCESS_KEY=%s\nLB_SECRET_KEY=%s\nLB_REGION=%s\n", - lbID, s.GatewayURL, s.SystemAccessKey, s.SystemSecretKey, s.region) + lbID, gatewayURL, s.SystemAccessKey, s.SystemSecretKey, s.region) +} + +// mgmtGatewayURL is the AWS gateway URL a system instance reaches over its +// br-mgmt NIC (on-link to MgmtBridgeIP). The port is taken from the configured +// WAN GatewayURL so both paths target the same AWSGW listener. Falls back to the +// WAN URL when no mgmt bridge exists (e.g. dev-shim networking). +func (s *ELBv2ServiceImpl) mgmtGatewayURL() string { + if s.MgmtBridgeIP == "" { + return s.GatewayURL + } + port := "9999" + if u, err := url.Parse(s.GatewayURL); err == nil && u.Port() != "" { + port = u.Port() + } + return "https://" + net.JoinHostPort(s.MgmtBridgeIP, port) } // subnetCIDRForIP returns "ip/prefixlen" from a host IP and subnet CIDR block. @@ -433,7 +458,7 @@ func (s *ELBv2ServiceImpl) launchLBVM(lbID, scheme string, eniIDs, subnets []str Scheme: scheme, AccountID: accountID, NICs: nics, - LBAgentEnv: s.buildLBAgentEnv(lbID), + LBAgentEnv: s.buildLBAgentEnv(lbID, accountID), CACert: s.CACert, } // Dev-mode only: forward HTTP/HTTPS ports from host for local testing. @@ -529,7 +554,7 @@ func (s *ELBv2ServiceImpl) RebuildSystemInstanceInput(ctx RecoveryContext) (*Sys Scheme: lb.Scheme, AccountID: lb.AccountID, NICs: nics, - LBAgentEnv: s.buildLBAgentEnv(lb.LoadBalancerID), + LBAgentEnv: s.buildLBAgentEnv(lb.LoadBalancerID, lb.AccountID), CACert: s.CACert, }, nil } @@ -1011,7 +1036,28 @@ func isCompatibleProtocol(listenerProto, tgProto string) bool { // --- Load Balancer operations --- +// CreateLoadBalancer provisions the LB and returns immediately with it in +// `provisioning` while the data-plane VM boots on a background goroutine +// (267.4: an inline multi-minute boot head-of-line-blocks the shared gateway +// responder, which then times out and re-publishes the create). Callers that +// need the LB's front-end address before proceeding — e.g. EKS, which bakes the +// IP into the control-plane apiserver cert SAN — must use CreateLoadBalancerSync. func (s *ELBv2ServiceImpl) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInput, accountID string) (*elbv2.CreateLoadBalancerOutput, error) { + return s.createLoadBalancer(input, accountID, false) +} + +// CreateLoadBalancerSync creates the LB and drives its data-plane launch +// synchronously, so the returned LB already carries its LoadBalancerAddresses +// (LaunchSystemInstance is a bounded request/reply that returns the allocated +// front-end IP before the multi-minute guest boot). Use only from an +// off-responder worker — the launch blocks up to the system-instance timeout, so +// running it on the shared gateway responder would reintroduce 267.4. The LB +// still flips provisioning → active on the lb-agent's first heartbeat. +func (s *ELBv2ServiceImpl) CreateLoadBalancerSync(input *elbv2.CreateLoadBalancerInput, accountID string) (*elbv2.CreateLoadBalancerOutput, error) { + return s.createLoadBalancer(input, accountID, true) +} + +func (s *ELBv2ServiceImpl) createLoadBalancer(input *elbv2.CreateLoadBalancerInput, accountID string, syncLaunch bool) (*elbv2.CreateLoadBalancerOutput, error) { if input.Name == nil || *input.Name == "" { return nil, errors.New(awserrors.ErrorMissingParameter) } @@ -1204,20 +1250,40 @@ func (s *ELBv2ServiceImpl) CreateLoadBalancer(input *elbv2.CreateLoadBalancerInp } if willLaunch { - // Snapshot inputs so the goroutine doesn't alias the caller's record. + // Snapshot the launch inputs; the async goroutine must not read the record + // the caller is about to return. lc := lbLaunchCtx{lbID: lbID, lbArn: lbArn, scheme: scheme, eniIDs: eniIDs, subnets: subnets, accountID: accountID} - s.launchWG.Go(func() { - defer func() { - if r := recover(); r != nil { - slog.Error("CreateLoadBalancer: async launch panic", "lbId", lc.lbID, "panic", r) - s.markLBFailed(lc.lbArn) - } - }() - s.launchLBVMAsync(lc) - }) + if syncLaunch { + // Drive the data-plane launch inline so the returned record carries the + // allocated front-end IP. The caller is off the gateway responder, so + // 267.4 does not apply. provisionLBDataPlane leaves the record marked + // failed on launch failure for diagnosis. + if err := s.provisionLBDataPlane(lc); err != nil { + slog.Error("CreateLoadBalancerSync: data-plane launch failed", "lbArn", lbArn, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + launched, lerr := s.store.GetLoadBalancerByArn(lbArn) + if lerr != nil || launched == nil { + slog.Error("CreateLoadBalancerSync: reload after launch failed", "lbArn", lbArn, "err", lerr) + return nil, errors.New(awserrors.ErrorServerInternal) + } + record = launched + } else { + s.launchWG.Go(func() { + defer func() { + if r := recover(); r != nil { + slog.Error("CreateLoadBalancer: async launch panic", "lbId", lc.lbID, "panic", r) + s.markLBFailed(lc.lbArn) + } + }() + s.launchLBVMAsync(lc) + }) + } } - slog.Info("CreateLoadBalancer accepted", "name", name, "lbArn", lbArn, "enis", len(eniIDs), "state", state, "accountID", accountID) + // Agent heartbeat will transition provisioning → active on first contact. + + slog.Info("CreateLoadBalancer accepted", "name", name, "lbArn", lbArn, "enis", len(eniIDs), "state", record.State, "accountID", accountID, "sync", syncLaunch) return &elbv2.CreateLoadBalancerOutput{ LoadBalancers: []*elbv2.LoadBalancer{s.lbRecordToSDK(record)}, @@ -1234,23 +1300,28 @@ type lbLaunchCtx struct { accountID string } -// launchLBVMAsync boots the LB-VM and folds the result into the persisted record. -// On failure marks the LB failed; on success stays provisioning until lb-agent heartbeats. -func (s *ELBv2ServiceImpl) launchLBVMAsync(lc lbLaunchCtx) { +// provisionLBDataPlane boots the LB-VM and folds the result into the persisted +// record: on success the instance/VPC-IP/host-ports and the per-AZ front-end IP +// are recorded and the LB stays provisioning until the lb-agent heartbeats; on +// failure the record is marked failed so the API reflects the broken data plane. +// Synchronous — launchLBVM (a bounded LaunchSystemInstance request/reply) returns +// the allocated front-end IP before the guest boot completes. Callers choose +// whether to run it inline (CreateLoadBalancerSync) or on a background goroutine +// (launchLBVMAsync). +func (s *ELBv2ServiceImpl) provisionLBDataPlane(lc lbLaunchCtx) error { launch := s.launchLBVM(lc.lbID, lc.scheme, lc.eniIDs, lc.subnets, lc.accountID) record, err := s.store.GetLoadBalancerByArn(lc.lbArn) if err != nil || record == nil { - slog.Error("CreateLoadBalancer: async launch cannot reload record", "lbArn", lc.lbArn, "err", err) - return + return fmt.Errorf("reload record for %s: %w", lc.lbArn, err) } if launch.failed { record.State = StateFailed if putErr := s.store.PutLoadBalancer(record); putErr != nil { - slog.Error("CreateLoadBalancer: async launch failed to persist failed state", "lbArn", lc.lbArn, "err", putErr) + return fmt.Errorf("persist failed state for %s: %w", lc.lbArn, putErr) } - return + return fmt.Errorf("lb-vm launch failed for %s", lc.lbArn) } record.InstanceID = launch.instanceID @@ -1260,7 +1331,17 @@ func (s *ELBv2ServiceImpl) launchLBVMAsync(lc lbLaunchCtx) { record.AvailZones[0].PublicIP = launch.publicIP } if putErr := s.store.PutLoadBalancer(record); putErr != nil { - slog.Error("CreateLoadBalancer: async launch failed to persist launch result", "lbArn", lc.lbArn, "err", putErr) + return fmt.Errorf("persist launch result for %s: %w", lc.lbArn, putErr) + } + return nil +} + +// launchLBVMAsync runs provisionLBDataPlane on the background goroutine spawned +// by CreateLoadBalancer; failures are logged (the record is already marked +// failed inside provisionLBDataPlane). Runs after CreateLoadBalancer has returned. +func (s *ELBv2ServiceImpl) launchLBVMAsync(lc lbLaunchCtx) { + if err := s.provisionLBDataPlane(lc); err != nil { + slog.Error("CreateLoadBalancer: async launch failed", "lbArn", lc.lbArn, "err", err) } } diff --git a/spinifex/handlers/elbv2/service_impl_lb_network_test.go b/spinifex/handlers/elbv2/service_impl_lb_network_test.go index 227b8b492..588fd0c62 100644 --- a/spinifex/handlers/elbv2/service_impl_lb_network_test.go +++ b/spinifex/handlers/elbv2/service_impl_lb_network_test.go @@ -138,6 +138,61 @@ func TestSetSecurityGroups_CrossAccount(t *testing.T) { assert.Contains(t, err.Error(), awserrors.ErrorELBv2LoadBalancerNotFound) } +// frontendPublicIP returns the first non-empty public IpAddress across the LB's +// AZ addresses, or "" when none is set yet. +func frontendPublicIP(lb *elbv2.LoadBalancer) string { + for _, az := range lb.AvailabilityZones { + for _, addr := range az.LoadBalancerAddresses { + if ip := aws.StringValue(addr.IpAddress); ip != "" { + return ip + } + } + } + return "" +} + +func TestCreateLoadBalancerSync_ReturnsFrontendIP(t *testing.T) { + svc, vpcSvc, mock := setupSubnetTestService(t) + mock.launchResult.PublicIP = "203.0.113.9" + vid := vpcID(t, vpcSvc) + sub := getTestSubnetID(t, vpcSvc, vid, "10.0.30.0/24", "us-east-1a") + + out, err := svc.CreateLoadBalancerSync(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("sync-lb"), + Type: aws.String("network"), + Scheme: aws.String(elbv2.LoadBalancerSchemeEnumInternetFacing), + Subnets: []*string{aws.String(sub)}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, out.LoadBalancers, 1) + // Sync create drove the launch inline — no WaitLaunches needed — so the + // returned LB already carries its front-end IP. + require.Len(t, mock.launchCalls, 1) + assert.Equal(t, "203.0.113.9", frontendPublicIP(out.LoadBalancers[0])) +} + +func TestCreateLoadBalancer_AsyncDefersFrontendIP(t *testing.T) { + svc, vpcSvc, mock := setupSubnetTestService(t) + mock.launchResult.PublicIP = "203.0.113.9" + vid := vpcID(t, vpcSvc) + sub := getTestSubnetID(t, vpcSvc, vid, "10.0.31.0/24", "us-east-1a") + + out, err := svc.CreateLoadBalancer(&elbv2.CreateLoadBalancerInput{ + Name: aws.String("async-lb"), + Type: aws.String("network"), + Scheme: aws.String(elbv2.LoadBalancerSchemeEnumInternetFacing), + Subnets: []*string{aws.String(sub)}, + }, testAccountID) + require.NoError(t, err) + // Async create returns provisioning before the launch goroutine runs — the + // front-end IP is not yet known (this is the race EKS must not read). + assert.Equal(t, StateProvisioning, aws.StringValue(out.LoadBalancers[0].State.Code)) + assert.Empty(t, frontendPublicIP(out.LoadBalancers[0])) + svc.WaitLaunches() + // Once the launch settles, the IP is on the persisted record. + assert.Equal(t, "203.0.113.9", frontendPublicIP(describeLB(t, svc, *out.LoadBalancers[0].LoadBalancerArn))) +} + // setupSubnetTestService wires a VPC-backed ELBv2 service with a launcher mock // so SetSubnets can exercise ENI create/delete plus the LB-VM relaunch. func setupSubnetTestService(t *testing.T) (*ELBv2ServiceImpl, *handlers_ec2_vpc.VPCServiceImpl, *mockSystemInstanceLauncher) { diff --git a/tests/e2e/eks/eks_test.go b/tests/e2e/eks/eks_test.go index 4f7cf41c0..2471a2810 100644 --- a/tests/e2e/eks/eks_test.go +++ b/tests/e2e/eks/eks_test.go @@ -30,9 +30,16 @@ const ( getTokenTpl = "k8s-aws-v1." ) -// TestEKS drives the EKS control-plane lifecycle: VPC/subnet creation, CreateCluster → ACTIVE -// (K3s VM + NLB boot), kubeconfig artifact assembly, AccessEntry CRUD, get-token, and -// DeleteCluster. One fixture is shared across subtests to avoid repeated control-plane boot cost. +// TestEKS drives the EKS control-plane lifecycle against the local awsgw +// endpoint: a customer VPC/subnet (Set A), CreateCluster → ACTIVE (spinifex +// auto-builds the managed control-plane VPC — Set B — and spreads the K3s +// control plane behind an internet-facing NLB), kubeconfig artifact assembly, +// AccessEntry CRUD, get-token, kubectl reachability against the published +// endpoint, managed-addon delivery, and DeleteCluster → gone. +// +// One cluster fixture is shared across the subtests (create once, delete in +// Cleanup) — control-plane boot is the slowest step, so re-creating per subtest +// would blow the suite timeout on dev nodes. func TestEKS(t *testing.T) { env := harness.LoadEnv(t) artifacts := harness.ArtifactDir(t, env) @@ -141,6 +148,66 @@ func TestEKS(t *testing.T) { t.Logf("kubectl get nodes:\n%s", out) }) + t.Run("AddonDelivery", func(t *testing.T) { + requireClusterReady(t, fx) + // End-to-end managed-addon delivery: CreateAddon stages a manifest + // descriptor host-side; the on-VM addon-sync agent pulls it through the + // gateway, renders the baked bundle into the K3s auto-deploy dir, and + // reports ready, which flips the record to ACTIVE. spinifex-noop is the + // fixture (Namespace + ConfigMap) — no dependency on the CSI/LB bundles. + const addon = "spinifex-noop" + _, err := c.EKS.CreateAddon(&eks.CreateAddonInput{ + ClusterName: aws.String(fx.ClusterName), + AddonName: aws.String(addon), + }) + require.NoError(t, err, "create-addon") + + // Record reaches ACTIVE once the VM confirms delivery. Generous envelope: + // one addon-sync tick (30s) + k3s auto-deploy apply, plus control-plane + // stabilisation slack. + harness.EventuallyErr(t, func() error { + out, derr := c.EKS.DescribeAddon(&eks.DescribeAddonInput{ + ClusterName: aws.String(fx.ClusterName), + AddonName: aws.String(addon), + }) + if derr != nil { + return fmt.Errorf("describe-addon: %w", derr) + } + if s := aws.StringValue(out.Addon.Status); s != eks.AddonStatusActive { + return fmt.Errorf("addon status %q, want ACTIVE", s) + } + return nil + }, 5*time.Minute, 10*time.Second) + + // The fixture's objects must exist on the cluster. + kcPath := writeKubeconfig(t, artifacts, fx.Cluster) + kc := harness.NewKubectl(t, kcPath, getTokenEnv(t, env)) + out, err := kc.Run(30*time.Second, "get", "configmap", "spinifex-noop", + "-n", "spinifex-noop", "-o", `jsonpath={.data.marker}`) + require.NoError(t, err, "addon ConfigMap must exist: %s", out) + assert.Contains(t, out, "delivered", "addon ConfigMap must carry the marker") + + // DeleteAddon unstages the manifest; the agent GCs the rendered file and + // k3s' auto-deploy controller removes the objects. + _, err = c.EKS.DeleteAddon(&eks.DeleteAddonInput{ + ClusterName: aws.String(fx.ClusterName), + AddonName: aws.String(addon), + }) + require.NoError(t, err, "delete-addon") + + harness.EventuallyErr(t, func() error { + out, runErr := kc.Run(30*time.Second, "get", "namespace", "spinifex-noop", + "--ignore-not-found", "-o", `jsonpath={.metadata.name}`) + if runErr != nil { + return fmt.Errorf("kubectl get namespace: %v\n%s", runErr, out) + } + if strings.TrimSpace(out) != "" { + return fmt.Errorf("namespace still present") + } + return nil + }, 3*time.Minute, 10*time.Second) + }) + t.Run("DeleteCluster", func(t *testing.T) { requireClusterReady(t, fx) _, err := c.EKS.DeleteCluster(&eks.DeleteClusterInput{Name: aws.String(fx.ClusterName)})