Add NFD topology-updater module with startup taint remover (#696)

osdc/docs/node-utilization-optimization.md

@@ -20,7 +20,7 @@ Each node's allocatable resources are reduced by:
 
 1. **Kubelet reserved CPU**: 60m (core 1) + 10m (core 2) + 5m/core (cores 3-4) + 2.5m/core (cores 5+)
 2. **Kubelet reserved memory**: 255Mi + 11Mi per max_pod (ENI-based) + 100Mi eviction threshold
-3. **DaemonSet overhead**: 460m CPU / 1158Mi memory (non-GPU), 560m CPU / 1414Mi memory (GPU nodes). Includes NodeLocal DNSCache (NLD) at 25m CPU / 100Mi memory per node, deployed on every node via DaemonSet.
+3. **DaemonSet overhead**: 460m CPU / 1158Mi memory (non-GPU), 560m CPU / 1414Mi memory (GPU nodes). Includes NodeLocal DNSCache (NLD) at 25m CPU / 100Mi memory per node, deployed on every node via DaemonSet. **p5 (H100) nodes only** carry an additional 60m CPU / 96Mi memory — NFD topology-updater (50m/64Mi) + taint-remover (10m/32Mi), currently only pinned to `node-fleet: p5` for NUMA-aware scheduling — for 620m CPU / 1510Mi total on this GPU fleet type. Other GPU fleets (g5/g6) stay at 560m/1414Mi.
 
 Each workflow pod also includes runner-container-hooks containers (320m CPU + 522Mi memory) on top of the job container's resources. The runner-orchestrator pod itself (750m CPU / 1Gi memory) lives on the dedicated `c7i-runner` NodePool and does not consume resources on the workflow node — see `c7i-runner` notes below.
 
@@ -186,7 +186,7 @@ The savings compound: nodes with better packing serve more concurrent runners, r
 
 Some runners (46c/85Gi, 94c/192Gi, 48c/384Gi) consistently achieve only 74-76% utilization even on perfectly-matched instance types. This is because:
 
-1. **Overhead is fixed**: Kubelet + DaemonSet overhead (~700m CPU, ~1.6Gi memory non-GPU / ~1.9Gi GPU; includes NLD at 25m CPU / 100Mi memory per node) is constant regardless of instance size
+1. **Overhead is fixed**: Kubelet + DaemonSet overhead (~700m CPU, ~1.6Gi memory non-GPU / ~1.9Gi GPU, ~2.0Gi on p5 with NFD; includes NLD at 25m CPU / 100Mi memory per node) is constant regardless of instance size
 2. **Large runners don't divide evenly**: A 94c runner on a 128c node leaves 34c unused — only 1 pod fits
 3. **Hooks tax**: Each workflow pod adds 320m CPU + 522Mi memory for runner-container-hooks, which compounds at large sizes (the runner-orchestrator pod itself lives on `c7i-runner`, so workflow nodes are no longer charged the 750m/1Gi orchestrator)
 

osdc/modules/nfd/deploy.sh

@@ -0,0 +1,77 @@
+#!/usr/bin/env bash
+set -euo pipefail
+#
+# Deploy NFD topology-updater for NUMA-aware scheduling.
+# Called by: just deploy-module <cluster> nfd
+#
+# Args: $1=cluster-id  $2=cluster-name  $3=region
+#
+# Installs the Node Feature Discovery Helm chart with only the
+# topology-updater enabled. This publishes NodeResourceTopology CRDs
+# that the numa-scheduler reads to make NUMA-aware placement decisions.
+#
+# Also deploys a taint-remover DaemonSet that removes the
+# node-init.osdc.io/nfd-topology startup taint from p5 nodes,
+# unblocking workflow scheduling after NFD starts.
+
+CLUSTER="$1"
+export CNAME="$2"
+export REGION="$3"
+MODULE_DIR="$(cd "$(dirname "$0")" && pwd)"
+REPO_ROOT="${OSDC_ROOT:-$(cd "$MODULE_DIR/../.." && pwd)}"
+UPSTREAM_ROOT="${OSDC_UPSTREAM:-$REPO_ROOT}"
+# shellcheck source=/dev/null
+source "$UPSTREAM_ROOT/scripts/mise-activate.sh"
+# shellcheck source=/dev/null
+source "$UPSTREAM_ROOT/scripts/helm-upgrade.sh"
+CFG="$UPSTREAM_ROOT/scripts/cluster-config.py"
+
+NFD_VERSION=$(uv run "$CFG" "$CLUSTER" nfd.version "0.17.1")
+NFD_UPDATE_INTERVAL=$(uv run "$CFG" "$CLUSTER" nfd.update_interval "15s")
+
+helm repo add nfd https://kubernetes-sigs.github.io/node-feature-discovery/charts 2>/dev/null || true
+helm repo update nfd
+
+echo "Installing NFD topology-updater v${NFD_VERSION}..."
+helm_upgrade_if_changed nfd nfd \
+  --create-namespace \
+  --history-max 3 \
+  -f "$MODULE_DIR/helm/values.yaml" \
+  --set topologyUpdater.updateInterval="${NFD_UPDATE_INTERVAL}" \
+  --timeout 5m \
+  --wait \
+  nfd/node-feature-discovery \
+  --version "${NFD_VERSION}"
+
+echo "NFD topology-updater deployed."
+
+# --- Deploy taint-remover DaemonSet ---
+# Two ConfigMaps:
+#   nfd-taint-remover-script — wait-for-nrt.py (polls for NRT, then removes taint)
+#   nfd-taint-remover-lib    — shared taint_remover.py library
+# Then apply the DaemonSet that mounts both.
+TAINT_REMOVER_LIB="$UPSTREAM_ROOT/base/kubernetes/node-taint-remover/lib/taint_remover.py"
+WAIT_SCRIPT="$MODULE_DIR/scripts/wait-for-nrt.py"
+if [[ ! -f "$TAINT_REMOVER_LIB" ]]; then
+  echo "WARNING: taint_remover.py not found at $TAINT_REMOVER_LIB — skipping taint-remover deploy" >&2
+elif [[ ! -f "$WAIT_SCRIPT" ]]; then
+  echo "WARNING: wait-for-nrt.py not found at $WAIT_SCRIPT — skipping taint-remover deploy" >&2
+else
+  echo "Deploying NFD taint-remover..."
+  python3 -c "import ast,sys; ast.parse(open(sys.argv[1]).read())" "$WAIT_SCRIPT"
+
+  kubectl create configmap nfd-taint-remover-script \
+    --from-file="wait-for-nrt.py=$WAIT_SCRIPT" \
+    -n nfd \
+    --dry-run=client \
+    -o yaml | kubectl apply -f -
+
+  kubectl create configmap nfd-taint-remover-lib \
+    --from-file="taint_remover.py=$TAINT_REMOVER_LIB" \
+    -n nfd \
+    --dry-run=client \
+    -o yaml | kubectl apply -f -
+
+  kubectl apply -f "$MODULE_DIR/kubernetes/nfd-taint-remover.yaml"
+  echo "NFD taint-remover deployed."
+fi

osdc/modules/nfd/helm/values.yaml

@@ -0,0 +1,58 @@
+# Node Feature Discovery — topology-updater only
+# Chart: https://github.com/kubernetes-sigs/node-feature-discovery/tree/master/deployment/helm/node-feature-discovery
+#
+# We only need the topology-updater component, which publishes
+# NodeResourceTopology CRDs per node. The NFD worker (feature detection)
+# and master (label reconciliation) are disabled — we don't need node
+# feature labels, just per-NUMA-zone resource visibility for the
+# numa-scheduler.
+
+master:
+  enable: false
+
+worker:
+  enable: false
+
+topologyUpdater:
+  enable: true
+  createCRDs: true
+
+  # Poll kubelet PodResources API every 15s (default 60s). Needs to be
+  # faster than the ARC capacity provisioner interval (30s) so NRT
+  # objects reflect current GPU allocations before placeholder pods are
+  # evaluated by the NUMA-aware scheduler.
+  updateInterval: 15s
+
+  # Scoped to H100 nodes (p5 fleet) where the packed pool uses
+  # single-numa-node topology policy. Only these multi-NUMA nodes
+  # need NRT data for the numa-scheduler to prevent
+  # TopologyAffinityError on 4-GPU jobs.
+  nodeSelector:
+    node-fleet: p5
+
+  # Tolerate every taint so the topology-updater schedules on p5
+  # nodes regardless of their taint set (node-fleet, instance-type,
+  # nvidia.com/gpu, startup taints). The nodeSelector above controls
+  # placement; tolerations just grant access.
+  tolerations:
+    - operator: Exists
+
+  resources:
+    requests:
+      cpu: 50m
+      memory: 64Mi
+    limits:
+      cpu: 200m
+      memory: 128Mi
+
+# The topology-updater needs the NRT CRD. The NFD chart installs it
+# when topologyUpdater is enabled.
+gc:
+  enable: true
+  nodeSelector:
+    role: base-infrastructure
+  tolerations:
+    - key: CriticalAddonsOnly
+      operator: Equal
+      value: "true"
+      effect: NoSchedule

osdc/modules/nfd/kubernetes/nfd-taint-remover.yaml

@@ -0,0 +1,133 @@
+# NFD startup-taint remover — waits for NFD to publish a
+# NodeResourceTopology object for this node, then removes the
+# node-init.osdc.io/nfd-topology startup taint.
+#
+# The NFD Helm chart doesn't support custom init containers, so we run
+# a separate DaemonSet. The taint is added by generate_nodepools.py
+# (STARTUP_TAINTS registry) and prevents workflow pods from scheduling
+# before NRT data is available for the numa-scheduler.
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: nfd-taint-remover
+  namespace: nfd
+  labels:
+    app: nfd-taint-remover
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: nfd-taint-remover
+rules:
+  - apiGroups: [""]
+    resources: ["nodes"]
+    verbs: ["get", "patch"]
+  - apiGroups: ["topology.node.k8s.io"]
+    resources: ["noderesourcetopologies"]
+    verbs: ["get"]
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: nfd-taint-remover
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: nfd-taint-remover
+subjects:
+  - kind: ServiceAccount
+    name: nfd-taint-remover
+    namespace: nfd
+---
+apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  name: nfd-taint-remover
+  namespace: nfd
+  labels:
+    app: nfd-taint-remover
+    app.kubernetes.io/name: nfd-taint-remover
+    app.kubernetes.io/component: node-configuration
+spec:
+  selector:
+    matchLabels:
+      app: nfd-taint-remover
+
+  updateStrategy:
+    type: RollingUpdate
+    rollingUpdate:
+      maxUnavailable: 1
+
+  template:
+    metadata:
+      labels:
+        app: nfd-taint-remover
+
+    spec:
+      priorityClassName: system-node-critical
+
+      # Match the same nodes as the NFD topology-updater.
+      nodeSelector:
+        node-fleet: p5
+
+      # Tolerate every taint — same rationale as node-performance-tuning:
+      # must coexist with all node-init.osdc.io/* startup taints without
+      # creating chicken-and-egg deadlocks.
+      tolerations:
+        - operator: Exists
+
+      serviceAccountName: nfd-taint-remover
+
+      initContainers:
+        - name: wait-and-remove-taint
+          image: public.ecr.aws/amazonlinux/amazonlinux:2023
+          command: ["/bin/bash", "-c"]
+          args:
+            - python3 /scripts/wait-for-nrt.py
+
+          env:
+            - name: NODE_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: spec.nodeName
+
+          volumeMounts:
+            - name: scripts
+              mountPath: /scripts
+              readOnly: true
+            - name: taint-remover-lib
+              mountPath: /scripts/taint-remover
+              readOnly: true
+
+          resources:
+            requests:
+              cpu: 10m
+              memory: 32Mi
+            limits:
+              cpu: 100m
+              memory: 64Mi
+
+      containers:
+        - name: sleep
+          image: public.ecr.aws/docker/library/alpine:3.19
+          command: ["/bin/sh", "-c", "echo 'NFD taint removed. Sleeping...'; sleep infinity"]
+
+          resources:
+            requests:
+              cpu: 10m
+              memory: 32Mi
+            limits:
+              cpu: 50m
+              memory: 64Mi
+
+      volumes:
+        - name: scripts
+          configMap:
+            name: nfd-taint-remover-script
+            defaultMode: 0755
+
+        - name: taint-remover-lib
+          configMap:
+            name: nfd-taint-remover-lib
+            defaultMode: 0755

osdc/modules/nfd/scripts/wait-for-nrt.py

@@ -0,0 +1,106 @@
+#!/usr/bin/env python3
+"""Wait for NFD to publish a NodeResourceTopology object for this node,
+then remove the node-init.osdc.io/nfd-topology startup taint.
+
+Polls the Kubernetes API for an NRT object matching NODE_NAME. Once found,
+delegates to the shared taint_remover.py library to remove the taint.
+
+Fails open after TIMEOUT_SECONDS: if NFD hasn't published by then, the
+taint is removed anyway to prevent permanently stranding the node.
+
+Usage: wait-for-nrt.py
+
+Environment:
+  NODE_NAME (required, from Downward API)
+  KUBERNETES_SERVICE_HOST / _PORT (set automatically inside cluster)
+"""
+
+from __future__ import annotations
+
+import logging
+import os
+import sys
+import time
+import urllib.error
+
+# The shared taint-remover library is mounted at /scripts/taint-remover/. We
+# reuse its in-cluster API plumbing (URL building, SA token, TLS context, and
+# the HTTP request helper) so this script only owns the NRT-specific polling.
+sys.path.insert(0, "/scripts/taint-remover")
+
+from taint_remover import _k8s_api, _read_token, _request, _ssl_context, remove_taint_forever
+
+TAINT_KEY = "node-init.osdc.io/nfd-topology"
+POLL_INTERVAL = 5  # seconds between NRT checks
+TIMEOUT_SECONDS = 600  # 10 minutes — fail open if exceeded
+NRT_API_VERSION = "v1alpha2"  # storage version for NFD 0.17.x
+NRT_API_PATH = f"/apis/topology.node.k8s.io/{NRT_API_VERSION}/noderesourcetopologies"
+
+log = logging.getLogger("wait-for-nrt")
+
+
+def _get_nrt(node_name: str) -> int:
+    """GET the NRT object for this node. Returns the HTTP status code.
+
+    Transport errors (timeouts, connection resets) propagate to the caller,
+    which treats them as transient and retries. HTTP error responses come
+    back as their status code via the shared _request helper.
+    """
+    url = f"{_k8s_api()}{NRT_API_PATH}/{node_name}"
+    status, _ = _request("GET", url, _read_token(), _ssl_context())
+    return status
+
+
+def _wait_for_nrt(node_name: str) -> None:
+    """Poll until an NRT object exists for this node, or the timeout elapses.
+
+    Logs the outcome (found, or fail-open on timeout) and returns. The caller
+    removes the taint unconditionally — failing open keeps a slow or broken
+    NFD from permanently stranding the node.
+    """
+    deadline = time.monotonic() + TIMEOUT_SECONDS
+
+    log.info(
+        "Waiting for NodeResourceTopology object for node '%s' (timeout %ds)...",
+        node_name,
+        TIMEOUT_SECONDS,
+    )
+    while time.monotonic() < deadline:
+        try:
+            status = _get_nrt(node_name)
+            if status == 200:
+                log.info("NRT object found for node '%s'.", node_name)
+                return
+            if status == 404:
+                log.info("NRT not yet published for '%s' — retrying in %ds.", node_name, POLL_INTERVAL)
+            elif 500 <= status < 600 or status == 429:
+                log.warning("Transient error (HTTP %d) checking NRT — retrying.", status)
+            else:
+                log.warning("Unexpected HTTP %d checking NRT — retrying.", status)
+        except (urllib.error.URLError, TimeoutError, ConnectionError, OSError) as e:
+            log.warning("Transient error checking NRT: %s — retrying.", e)
+
+        time.sleep(POLL_INTERVAL)
+
+    log.warning(
+        "Timeout (%ds) waiting for NRT on node '%s' — removing taint anyway (fail-open).",
+        TIMEOUT_SECONDS,
+        node_name,
+    )
+
+
+def main() -> int:
+    logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s")
+
+    node_name = os.environ.get("NODE_NAME")
+    if not node_name:
+        log.error("NODE_NAME env var not set (Downward API spec.nodeName)")
+        return 1
+
+    _wait_for_nrt(node_name)
+    remove_taint_forever(TAINT_KEY)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

osdc/modules/nfd/tests/smoke/conftest.py

@@ -0,0 +1 @@
+from smoke_conftest import *  # noqa: F403