From b41a193cbcea9a00a4417e4d0c1163ab76b911c8 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Wed, 15 Jul 2026 00:39:13 +1000 Subject: [PATCH] fix(eks): make provider-id.sh the single k3s node-label authority for nodegroup + GPU labels (#539) --- scripts/images/eks-node-gpu/k3s-agent.service | 5 +- .../mulga-eks-provider-id.service | 9 +++- .../0.17.4/10-daemonset.yaml | 3 +- .../eks-node/mulga-eks-provider-id.initd | 10 ++-- .../images/eks-node/mulga-eks-provider-id.sh | 52 ++++++++++++++----- spinifex/handlers/eks/k3s_server_vm.go | 2 +- spinifex/handlers/eks/nodegroup_test.go | 2 +- spinifex/handlers/eks/nodegroup_userdata.go | 48 ++++++----------- .../handlers/eks/nodegroup_userdata_test.go | 49 +++++++++-------- 9 files changed, 105 insertions(+), 75 deletions(-) diff --git a/scripts/images/eks-node-gpu/k3s-agent.service b/scripts/images/eks-node-gpu/k3s-agent.service index cb1147fdf..4d94db4ab 100644 --- a/scripts/images/eks-node-gpu/k3s-agent.service +++ b/scripts/images/eks-node-gpu/k3s-agent.service @@ -20,8 +20,9 @@ TasksMax=infinity LimitNOFILE=1048576 LimitNPROC=infinity LimitCORE=infinity -# Cloud-init drops K3S_URL/K3S_TOKEN/K3S_NODE_NAME/K3S_NODE_LABEL here at -# RunInstances time per the Nodegroup bootstrap flow. +# Cloud-init drops K3S_URL/K3S_TOKEN/K3S_NODE_NAME (plus SPINIFEX_NODEGROUP and, +# for GPU workers, SPINIFEX_GPU_NODE) here at RunInstances time per the Nodegroup +# bootstrap flow; mulga-eks-provider-id reads the SPINIFEX_* vars for node-labels. EnvironmentFile=-/etc/spinifex-eks/agent.env # Tee k3s (and its embedded kubelet's klog) stderr to the serial console as well # as journald: Ubuntu logs to journald only, which is off-box-invisible on a diff --git a/scripts/images/eks-node-gpu/mulga-eks-provider-id.service b/scripts/images/eks-node-gpu/mulga-eks-provider-id.service index 9d96eb0d0..51ab53549 100644 --- a/scripts/images/eks-node-gpu/mulga-eks-provider-id.service +++ b/scripts/images/eks-node-gpu/mulga-eks-provider-id.service @@ -1,6 +1,13 @@ [Unit] Description=Set K3s provider-id + topology labels from IMDS for EBS CSI -After=network-online.target +# After cloud-config.service so cloud-init has written /etc/spinifex-eks/agent.env +# (write_files completes by the config stage): this script folds the nodegroup + +# GPU node-labels (read from agent.env) into the same node-label list, so agent.env +# must exist before it runs. NOT cloud-final.service: that unit is ordered after +# multi-user.target and k3s-agent.service (ordered After this unit) is +# WantedBy=multi-user.target, so chaining onto cloud-final forms an ordering cycle +# that makes systemd skip both this unit and k3s-agent, so the worker never joins. +After=network-online.target cloud-config.service Wants=network-online.target Before=k3s-agent.service diff --git a/scripts/images/eks-node/addons/nvidia-device-plugin/0.17.4/10-daemonset.yaml b/scripts/images/eks-node/addons/nvidia-device-plugin/0.17.4/10-daemonset.yaml index a661e49da..aff55a3dc 100644 --- a/scripts/images/eks-node/addons/nvidia-device-plugin/0.17.4/10-daemonset.yaml +++ b/scripts/images/eks-node/addons/nvidia-device-plugin/0.17.4/10-daemonset.yaml @@ -1,6 +1,7 @@ # nvidia-device-plugin 0.17.4 — CDI-mode DaemonSet exposing nvidia.com/gpu # allocatable on GPU-tainted nodes. Gated to GPU nodes via nodeSelector on the -# K3S_NODE_LABEL nvidia.com/gpu.present=true (nodegroup_userdata.go); inert +# node-label nvidia.com/gpu.present=true (written by mulga-eks-provider-id from +# the SPINIFEX_GPU_NODE agent.env signal); inert # elsewhere. --device-list-strategy=cdi-cri makes the plugin generate a CDI spec # for GPU workloads (containerd 2.x resolves the CRI-passed CDI devices; no # enable_cdi drop-in needed on k3s v1.32.5). The plugin's OWN container needs diff --git a/scripts/images/eks-node/mulga-eks-provider-id.initd b/scripts/images/eks-node/mulga-eks-provider-id.initd index d32f7cd0e..b48c97468 100644 --- a/scripts/images/eks-node/mulga-eks-provider-id.initd +++ b/scripts/images/eks-node/mulga-eks-provider-id.initd @@ -2,14 +2,16 @@ # mulga-eks-provider-id — boot oneshot that writes a K3s drop-in from IMDS # before K3s starts: the kubelet provider-id plus region/zone topology labels, # the node metadata the aws-ebs-csi-driver needs for NodeGetInfo + provisioning. -# Ordered after local (which installs the IMDS on-link route) and before both -# K3s roles. Role-agnostic: enabled on every node (server + agent). +# Also folds the worker's nodegroup + GPU node-labels (from agent.env) into the +# same node-label list, so it must run after cloud-init has written agent.env +# (cloud-config/cloud-final stages, same as eks-node-role) and before the role +# selector starts K3s. Role-agnostic: enabled on every node (server + agent). description="Set K3s provider-id + topology labels from IMDS for EBS CSI" depend() { - after net local - before k3s k3s-agent + after net local cloud-config cloud-final + before k3s k3s-agent eks-node-role } start() { diff --git a/scripts/images/eks-node/mulga-eks-provider-id.sh b/scripts/images/eks-node/mulga-eks-provider-id.sh index d636d20ee..9950a5014 100644 --- a/scripts/images/eks-node/mulga-eks-provider-id.sh +++ b/scripts/images/eks-node/mulga-eks-provider-id.sh @@ -17,13 +17,22 @@ set -eu # family"). This drop-in supplies all four; region is derived from the AZ # (trailing zone letters stripped). # -# K3s merges /etc/rancher/k3s/config.yaml.d/*.yaml over config.yaml; list keys -# such as kubelet-arg are concatenated, so this adds the flag without touching -# the cloud-init-written main config. Role-agnostic: runs on server + agent. +# This drop-in is also the SINGLE writer of the node-label list. K3s replaces +# (not merges) node-label across config.yaml.d files, so the last-sorted file +# wins outright and any other node-label drop-in would silently wipe the rest. +# The worker's nodegroup label (and, on GPU workers, the nvidia.com/gpu.present +# label + NoSchedule taint) therefore ride here too, read from agent.env, so +# they share the one list with the topology labels. On the K3s server there is +# no agent.env, so only the topology labels are written — behavior unchanged. +# +# kubelet-arg lists ARE concatenated by k3s across drop-ins, so the provider-id +# flag coexists with other files' kubelet-arg without touching the main config. +# Role-agnostic: runs on server + agent. IMDS="http://169.254.169.254/latest" DROPIN_DIR=/etc/rancher/k3s/config.yaml.d DROPIN="${DROPIN_DIR}/10-provider-id.yaml" +AGENT_ENV=/etc/spinifex-eks/agent.env LOGTAG="mulga-eks-provider-id" log() { echo "[${LOGTAG}] $*"; } @@ -79,13 +88,32 @@ case "${itype}" in *) itype="m5.large" ;; esac +# Fold the worker's nodegroup + GPU node-labels into the same list. agent.env is +# written by cloud-init and absent on the server; source it only if present. +nodegroup="" +gpu_node="" +if [ -f "${AGENT_ENV}" ]; then + # shellcheck disable=SC1090 + . "${AGENT_ENV}" + nodegroup="${SPINIFEX_NODEGROUP:-}" + gpu_node="${SPINIFEX_GPU_NODE:-}" +fi + mkdir -p "${DROPIN_DIR}" -cat > "${DROPIN}" < "${DROPIN}" +log "wrote provider-id aws:///${az}/${iid}, region=${region} zone=${az} type=${itype} nodegroup=${nodegroup:-} gpu=${gpu_node:-false}" diff --git a/spinifex/handlers/eks/k3s_server_vm.go b/spinifex/handlers/eks/k3s_server_vm.go index 4e3339806..2c0adec44 100644 --- a/spinifex/handlers/eks/k3s_server_vm.go +++ b/spinifex/handlers/eks/k3s_server_vm.go @@ -72,7 +72,7 @@ const ( k3sSnapshotEnvPath = "/etc/spinifex-eks/etcd-snapshot.env" // agentEnvPath is the env file the k3s-agent OpenRC service sources for - // K3S_URL/K3S_TOKEN/K3S_NODE_NAME/K3S_NODE_LABEL. + // K3S_URL/K3S_TOKEN/K3S_NODE_NAME. agentEnvPath = "/etc/spinifex-eks/agent.env" // k3sGatewayCAPath is the on-VM gateway TLS CA cert PEM path. diff --git a/spinifex/handlers/eks/nodegroup_test.go b/spinifex/handlers/eks/nodegroup_test.go index 1d767d2bf..507cc3586 100644 --- a/spinifex/handlers/eks/nodegroup_test.go +++ b/spinifex/handlers/eks/nodegroup_test.go @@ -286,7 +286,7 @@ func TestBuildAgentUserData_Shape(t *testing.T) { assert.Contains(t, ud, "K3S_URL=https://10.254.0.9:443") assert.Contains(t, ud, "K3S_TOKEN=K10secret::server:xyz") assert.Contains(t, ud, "K3S_NODE_NAME=c1-ng1-abc123de") - assert.Contains(t, ud, "eks.amazonaws.com/nodegroup=ng1") + assert.Contains(t, ud, "SPINIFEX_NODEGROUP=ng1") assert.Contains(t, ud, agentEnvPath) // IMDS is served at the host tap, so no in-guest on-link route is emitted. assert.NotContains(t, ud, "169.254.169.254") diff --git a/spinifex/handlers/eks/nodegroup_userdata.go b/spinifex/handlers/eks/nodegroup_userdata.go index 86492642d..17eacbce8 100644 --- a/spinifex/handlers/eks/nodegroup_userdata.go +++ b/spinifex/handlers/eks/nodegroup_userdata.go @@ -61,9 +61,12 @@ func buildAgentUserData(in agentUserDataInput) string { }, "\n") // The ECR credential provider reads /etc/spinifex-eks/agent.env, so the EKS_* - // settings live alongside the K3s join vars here. The nodegroup label rides a - // node-label config drop-in below, not K3S_NODE_LABEL (k3s has no such env). - agentBody := strings.Join([]string{ + // settings live alongside the K3s join vars here. SPINIFEX_NODEGROUP (and + // SPINIFEX_GPU_NODE for GPU workers) are consumed by mulga-eks-provider-id.sh, + // the single writer of the k3s node-label list: k3s replaces (not merges) the + // node-label list across config.yaml.d files, so all node-labels must come + // from one drop-in or the last-sorted file silently wipes the rest. + agentLines := []string{ "K3S_URL=" + k3sURL, "K3S_TOKEN=" + in.JoinToken, "K3S_NODE_NAME=" + in.NodeName, @@ -71,7 +74,12 @@ func buildAgentUserData(in agentUserDataInput) string { "EKS_GATEWAY_CA=" + k3sGatewayCAPath, "EKS_REGION=" + in.Region, "EKS_ACCOUNT_ID=" + in.AccountID, - }, "\n") + "SPINIFEX_NODEGROUP=" + in.NodegroupName, + } + if in.GPUEnabled { + agentLines = append(agentLines, "SPINIFEX_GPU_NODE=true") + } + agentBody := strings.Join(agentLines, "\n") // The mirror endpoint is dialed by IP when the gateway IP is known: the gateway // cert carries an IP SAN, so TLS validates without DNS, and there is no internal @@ -124,42 +132,20 @@ func buildAgentUserData(in agentUserDataInput) string { ) credProviderConfig := strings.Join(credLines, "\n") - // The eks.amazonaws.com/nodegroup label must ride a k3s node-label config - // drop-in: k3s has no K3S_NODE_LABEL env, so agent.env cannot set it. The - // nodegroup readiness gate (waitWorkersReady) buckets Ready nodes by this - // label, so a worker without it never counts toward its nodegroup's ACTIVE - // transition. k3s concatenates node-label lists across config.yaml.d drop-ins. - nodegroupLabelDropin := strings.Join([]string{ - "node-label:", - " - \"eks.amazonaws.com/nodegroup=" + in.NodegroupName + "\"", - }, "\n") - + // buildAgentUserData writes NO config.yaml.d node-label/node-taint drop-in: + // mulga-eks-provider-id.sh (AMI-baked) is the sole node-label writer. It reads + // SPINIFEX_NODEGROUP/SPINIFEX_GPU_NODE from agent.env and folds the nodegroup + // label (plus nvidia.com/gpu.present + the GPU NoSchedule taint) into the same + // 10-provider-id.yaml node-label list it writes for the EBS CSI topology labels. files := []userDataFile{ {Path: k3sFirstBootEnvPath, Perms: "0600", Body: envBody}, {Path: agentEnvPath, Perms: "0600", Body: agentBody}, {Path: k3sGatewayCAPath, Perms: "0644", Body: strings.TrimRight(in.GatewayCACert, "\n")}, {Path: "/etc/rancher/k3s/registries.yaml", Perms: "0644", Body: registriesYAML}, {Path: "/etc/rancher/k3s/config.yaml.d/20-ecr-credential-provider.yaml", Perms: "0644", Body: credProviderDropin}, - {Path: "/etc/rancher/k3s/config.yaml.d/15-nodegroup-label.yaml", Perms: "0644", Body: nodegroupLabelDropin}, {Path: "/etc/spinifex-eks/credential-provider-config.yaml", Perms: "0644", Body: credProviderConfig}, } - // GPU nodes advertise nvidia.com/gpu.present=true (so the device-plugin - // DaemonSet nodeSelector matches) and default to NoSchedule so ordinary - // workloads don't consume scarce GPU capacity. Both ride a separate - // node-label/node-taint drop-in; k3s merges it over the nodegroup-label one. - if in.GPUEnabled { - gpuDropin := strings.Join([]string{ - "node-label:", - " - \"nvidia.com/gpu.present=true\"", - "node-taint:", - " - \"nvidia.com/gpu=present:NoSchedule\"", - }, "\n") - files = append(files, userDataFile{ - Path: "/etc/rancher/k3s/config.yaml.d/20-gpu.yaml", Perms: "0644", Body: gpuDropin, - }) - } - var buf strings.Builder buf.WriteString("#cloud-config\n") diff --git a/spinifex/handlers/eks/nodegroup_userdata_test.go b/spinifex/handlers/eks/nodegroup_userdata_test.go index 2ac098305..8aee4d4ce 100644 --- a/spinifex/handlers/eks/nodegroup_userdata_test.go +++ b/spinifex/handlers/eks/nodegroup_userdata_test.go @@ -96,31 +96,36 @@ func baseTestInput() agentUserDataInput { } } -func TestBuildAgentUserData_GPUEnabledAddsLabelAndTaint(t *testing.T) { +func TestBuildAgentUserData_GPUEnabledSignalsNodegroupAndGPU(t *testing.T) { in := baseTestInput() in.GPUEnabled = true in.GPUVendor = "nvidia" got := buildAgentUserData(in) - // The label must ride a node-label config drop-in, not K3S_NODE_LABEL (k3s - // has no such env var — it would be a no-op and the device-plugin - // nodeSelector would never match). - if !strings.Contains(got, "/etc/rancher/k3s/config.yaml.d/20-gpu.yaml") { - t.Errorf("expected GPU drop-in path, got:\n%s", got) + // buildAgentUserData writes NO node-label/node-taint config drop-in: + // mulga-eks-provider-id.sh is the single node-label writer (k3s replaces, + // not merges, node-label across config.yaml.d files). The worker signals its + // nodegroup + GPU role via agent.env; the AMI script folds the labels + taint + // into the one node-label list it already writes for the topology labels. + if strings.Contains(got, "config.yaml.d/10-nodegroup.yaml") { + t.Errorf("must not write a nodegroup node-label drop-in, got:\n%s", got) } - if !strings.Contains(got, "node-label:") || !strings.Contains(got, "nvidia.com/gpu.present=true") { - t.Errorf("expected GPU node-label drop-in value, got:\n%s", got) + if strings.Contains(got, "node-label:") || strings.Contains(got, "node-taint:") { + t.Errorf("user-data must not carry any node-label/node-taint drop-in, got:\n%s", got) } - if !strings.Contains(got, "nvidia.com/gpu=present:NoSchedule") { - t.Errorf("expected GPU taint value, got:\n%s", got) + if !strings.Contains(got, "SPINIFEX_NODEGROUP=ng-1") { + t.Errorf("expected SPINIFEX_NODEGROUP in agent.env, got:\n%s", got) } - if strings.Contains(got, "K3S_NODE_LABEL=eks.amazonaws.com/nodegroup=ng-1,nvidia.com/gpu.present=true") { - t.Errorf("GPU label must not ride the no-op K3S_NODE_LABEL env, got:\n%s", got) + if !strings.Contains(got, "SPINIFEX_GPU_NODE=true") { + t.Errorf("expected SPINIFEX_GPU_NODE=true in agent.env for a GPU nodegroup, got:\n%s", got) + } + if strings.Contains(got, "K3S_NODE_LABEL") { + t.Errorf("label must not ride the no-op K3S_NODE_LABEL env, got:\n%s", got) } } -func TestBuildAgentUserData_NonGPUHasNoLabelOrTaint(t *testing.T) { +func TestBuildAgentUserData_NonGPUSignalsNodegroupOnly(t *testing.T) { in := baseTestInput() got := buildAgentUserData(in) @@ -128,17 +133,17 @@ func TestBuildAgentUserData_NonGPUHasNoLabelOrTaint(t *testing.T) { if strings.Contains(got, "nvidia.com/gpu") { t.Errorf("non-GPU nodegroup user-data must not reference nvidia.com/gpu, got:\n%s", got) } - if strings.Contains(got, "20-gpu.yaml") { - t.Errorf("non-GPU nodegroup user-data must not carry the GPU drop-in, got:\n%s", got) + if strings.Contains(got, "SPINIFEX_GPU_NODE") { + t.Errorf("non-GPU nodegroup must not set SPINIFEX_GPU_NODE, got:\n%s", got) } - // The nodegroup label must ride a node-label config drop-in, not the no-op - // K3S_NODE_LABEL env — waitWorkersReady buckets Ready nodes by this label, - // so a worker missing it never counts toward its nodegroup's ACTIVE gate. - if !strings.Contains(got, "/etc/rancher/k3s/config.yaml.d/15-nodegroup-label.yaml") { - t.Errorf("expected nodegroup-label drop-in path, got:\n%s", got) + if strings.Contains(got, "node-label:") || strings.Contains(got, "node-taint:") { + t.Errorf("user-data must not carry any node-label/node-taint drop-in, got:\n%s", got) } - if !strings.Contains(got, `"eks.amazonaws.com/nodegroup=ng-1"`) { - t.Errorf("expected nodegroup label in the node-label drop-in, got:\n%s", got) + // The worker signals its nodegroup via agent.env; mulga-eks-provider-id.sh + // turns it into the eks.amazonaws.com/nodegroup label that gates the + // nodegroup reaching ACTIVE (waitWorkersReady buckets Ready nodes by it). + if !strings.Contains(got, "SPINIFEX_NODEGROUP=ng-1") { + t.Errorf("expected SPINIFEX_NODEGROUP in agent.env, got:\n%s", got) } if strings.Contains(got, "K3S_NODE_LABEL") { t.Errorf("nodegroup label must not ride the no-op K3S_NODE_LABEL env, got:\n%s", got)