diff --git a/osdc/clusters.yaml b/osdc/clusters.yaml index 2463ef0d..7b19983b 100644 --- a/osdc/clusters.yaml +++ b/osdc/clusters.yaml @@ -205,6 +205,8 @@ clusters: - arc - nodepools - nodepools-h100 # H100 only — B200 has no capacity reservation in us-west-1 + - nfd # NUMA topology data for p5 nodes + - numa-scheduler # NUMA-aware secondary scheduler - arc-runners-h100 - pypi-cache - cache-enforcer @@ -285,6 +287,8 @@ clusters: - nodepools - nodepools-b200 - nodepools-h100 + - nfd # NUMA topology data for p5 nodes + - numa-scheduler # NUMA-aware secondary scheduler - arc-runners - arc-runners-b200 - arc-runners-h100 diff --git a/osdc/modules/nfd/deploy.sh b/osdc/modules/nfd/deploy.sh new file mode 100755 index 00000000..2d544bbc --- /dev/null +++ b/osdc/modules/nfd/deploy.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +set -euo pipefail +# +# Deploy NFD topology-updater for NUMA-aware scheduling. +# Called by: just deploy-module 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 --- +# Create ConfigMap from the shared taint_remover.py library, then apply +# the DaemonSet that uses it to remove node-init.osdc.io/nfd-topology. +TAINT_REMOVER_LIB="$UPSTREAM_ROOT/base/kubernetes/node-taint-remover/lib/taint_remover.py" +if [[ ! -f "$TAINT_REMOVER_LIB" ]]; then + echo "WARNING: taint_remover.py not found at $TAINT_REMOVER_LIB — skipping taint-remover deploy" >&2 +else + echo "Deploying NFD taint-remover..." + 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 diff --git a/osdc/modules/nfd/helm/values.yaml b/osdc/modules/nfd/helm/values.yaml new file mode 100644 index 00000000..27744fe4 --- /dev/null +++ b/osdc/modules/nfd/helm/values.yaml @@ -0,0 +1,50 @@ +# 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 + + # 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. +topologyGC: + enable: true diff --git a/osdc/modules/nfd/kubernetes/nfd-taint-remover.yaml b/osdc/modules/nfd/kubernetes/nfd-taint-remover.yaml new file mode 100644 index 00000000..26a9c545 --- /dev/null +++ b/osdc/modules/nfd/kubernetes/nfd-taint-remover.yaml @@ -0,0 +1,115 @@ +# NFD startup-taint remover — removes node-init.osdc.io/nfd-topology +# from p5 nodes after the NFD topology-updater DaemonSet starts. +# +# The NFD Helm chart doesn't support custom init containers, so we run +# a separate minimal DaemonSet to remove the startup taint. The taint +# is added by generate_nodepools.py (STARTUP_TAINTS registry) and +# prevents workflow pods from scheduling before NRT data is available. +--- +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"] +--- +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 +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: remove-taint + image: public.ecr.aws/docker/library/python:3.12-alpine + command: ["python3", "/scripts/taint-remover/taint_remover.py", "node-init.osdc.io/nfd-topology"] + + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + + volumeMounts: + - name: taint-remover-lib + mountPath: /scripts/taint-remover + readOnly: true + + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + cpu: 100m + memory: 64Mi + + containers: + - name: pause + image: registry.k8s.io/pause:3.9 + resources: + requests: + cpu: 1m + memory: 8Mi + limits: + cpu: 10m + memory: 16Mi + + volumes: + - name: taint-remover-lib + configMap: + name: nfd-taint-remover-lib + defaultMode: 0755 diff --git a/osdc/modules/nfd/tests/smoke/conftest.py b/osdc/modules/nfd/tests/smoke/conftest.py new file mode 100644 index 00000000..8dfd070c --- /dev/null +++ b/osdc/modules/nfd/tests/smoke/conftest.py @@ -0,0 +1 @@ +from smoke_conftest import * # noqa: F403 diff --git a/osdc/modules/nfd/tests/smoke/test_nfd.py b/osdc/modules/nfd/tests/smoke/test_nfd.py new file mode 100644 index 00000000..7b66a454 --- /dev/null +++ b/osdc/modules/nfd/tests/smoke/test_nfd.py @@ -0,0 +1,104 @@ +"""Smoke tests for NFD (Node Feature Discovery) topology-updater. + +Validates that the NFD namespace exists, the Helm release is deployed, +the topology-updater DaemonSet is healthy on all nodes, and +NodeResourceTopology CRDs are being published. +""" + +from __future__ import annotations + +import subprocess + +import pytest +from helpers import ( + assert_daemonset_healthy, + find_helm_release, + run_kubectl, +) + +pytestmark = [pytest.mark.live] + +NAMESPACE = "nfd" + + +# ============================================================================ +# Namespace +# ============================================================================ + + +class TestNFDNamespace: + """Verify NFD namespace exists.""" + + def test_namespace_exists(self, all_namespaces: dict) -> None: + ns_names = [ns["metadata"]["name"] for ns in all_namespaces.get("items", [])] + assert NAMESPACE in ns_names, f"Namespace '{NAMESPACE}' not found" + + +# ============================================================================ +# Helm Release +# ============================================================================ + + +class TestNFDHelm: + """Verify NFD Helm release is deployed.""" + + def test_helm_release_deployed(self, all_helm_releases: list[dict]) -> None: + release = find_helm_release(all_helm_releases, "nfd", namespace=NAMESPACE) + assert release is not None, "Helm release 'nfd' not found in 'nfd' namespace" + status = release.get("status", "") + assert status == "deployed", f"NFD Helm release status is '{status}', expected 'deployed'" + + +# ============================================================================ +# Topology-updater DaemonSet +# ============================================================================ + + +class TestTopologyUpdater: + """Verify topology-updater DaemonSet is healthy on p5 nodes.""" + + def test_topology_updater_healthy(self, all_daemonsets: dict, all_nodes: dict) -> None: + assert_daemonset_healthy( + all_daemonsets, + all_nodes, + NAMESPACE, + name_contains="topology-updater", + allow_zero=True, # NFD targets p5 (H100) nodes only; clusters without p5 have 0 pods + ) + + +class TestTaintRemover: + """Verify nfd-taint-remover DaemonSet is healthy on p5 nodes.""" + + def test_taint_remover_healthy(self, all_daemonsets: dict, all_nodes: dict) -> None: + assert_daemonset_healthy( + all_daemonsets, + all_nodes, + NAMESPACE, + name_contains="taint-remover", + allow_zero=True, # Same p5-only scope as topology-updater + ) + + +# ============================================================================ +# NodeResourceTopology CRD +# ============================================================================ + + +class TestNodeResourceTopology: + """Verify NodeResourceTopology CRD is installed. + + The NRT CRD is the key output of NFD topology-updater — without it + the numa-scheduler has no NUMA visibility. The CRD is installed by + the NFD Helm chart when topologyUpdater is enabled. + """ + + def test_nrt_crd_exists(self) -> None: + try: + result = run_kubectl( + ["get", "crd", "noderesourcetopologies.topology.node.k8s.io", "-o", "json"], + ) + except subprocess.CalledProcessError: + pytest.fail("NodeResourceTopology CRD not found — NFD chart may not have installed it") + name = result.get("metadata", {}).get("name", "") + assert "noderesourcetopologies" in name diff --git a/osdc/modules/nodepools/scripts/python/generate_nodepools.py b/osdc/modules/nodepools/scripts/python/generate_nodepools.py index e3208bf0..2e9cf0c1 100755 --- a/osdc/modules/nodepools/scripts/python/generate_nodepools.py +++ b/osdc/modules/nodepools/scripts/python/generate_nodepools.py @@ -93,6 +93,16 @@ # leaving the registry entry in place would taint every new node with a # taint nothing removes, hanging workload scheduling indefinitely. The # AMI-version gates in clusters.yaml track when both can be retired. + { + "module": "nfd", + "key": "node-init.osdc.io/nfd-topology", + "value": "true", + "effect": "NoSchedule", + # NFD topology-updater only targets p5 nodes (nodeSelector: node-fleet: p5). + # Only emit the taint on nodepools where NFD actually runs — otherwise the + # node would be tainted with nothing to remove it. + "applies_when": lambda d: d.get("fleet_name") == "p5", + }, ] diff --git a/osdc/modules/numa-scheduler/deploy.sh b/osdc/modules/numa-scheduler/deploy.sh new file mode 100755 index 00000000..a5ca18e1 --- /dev/null +++ b/osdc/modules/numa-scheduler/deploy.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +set -euo pipefail +# +# Deploy NUMA-aware secondary scheduler (scheduler-plugins). +# Called by: just deploy-module numa-scheduler +# +# Args: $1=cluster-id $2=cluster-name $3=region +# +# Downloads the scheduler-plugins Helm chart from the GitHub release +# and installs it as a secondary scheduler named "numa-scheduler" with +# NodeResourceTopologyMatch enabled. Pods that set +# schedulerName: numa-scheduler get NUMA-aware placement. + +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" + +CHART_VERSION=$(uv run "$CFG" "$CLUSTER" numa_scheduler.chart_version "0.34.7") +SCHEDULER_REPLICAS=$(uv run "$CFG" "$CLUSTER" numa_scheduler.replicas "2") + +# --- Download chart from GitHub release --- +CHART_TGZ="scheduler-plugins-${CHART_VERSION}.tgz" +CHART_DIR=$(mktemp -d) +trap 'rm -rf "$CHART_DIR"' EXIT + +CHART_URL="https://github.com/kubernetes-sigs/scheduler-plugins/releases/download/v${CHART_VERSION}/${CHART_TGZ}" +echo "Downloading scheduler-plugins chart v${CHART_VERSION}..." +curl -fsSL "$CHART_URL" -o "${CHART_DIR}/${CHART_TGZ}" + +echo "Installing numa-scheduler (scheduler-plugins v${CHART_VERSION})..." +helm_upgrade_if_changed numa-scheduler numa-scheduler \ + --create-namespace \ + --history-max 3 \ + -f "$MODULE_DIR/helm/values.yaml" \ + --set scheduler.replicaCount="${SCHEDULER_REPLICAS}" \ + --timeout 5m \ + --wait \ + "${CHART_DIR}/${CHART_TGZ}" + +echo "numa-scheduler deployed." diff --git a/osdc/modules/numa-scheduler/helm/values.yaml b/osdc/modules/numa-scheduler/helm/values.yaml new file mode 100644 index 00000000..c5fb3858 --- /dev/null +++ b/osdc/modules/numa-scheduler/helm/values.yaml @@ -0,0 +1,57 @@ +# Scheduler-plugins — NUMA-aware secondary scheduler +# Chart source: https://github.com/kubernetes-sigs/scheduler-plugins +# (vendored from manifests/install/charts/as-a-second-scheduler/) +# +# Deploys a second kube-scheduler named "numa-scheduler" with the +# NodeResourceTopologyMatch plugin enabled. Pods that set +# schedulerName: numa-scheduler will be filtered/scored by per-NUMA-zone +# resource availability (read from NodeResourceTopology CRDs published +# by the NFD topology-updater). +# +# Nothing uses this scheduler until runner definitions set +# scheduler_name: numa-scheduler — deploying it is a no-op. + +scheduler: + name: numa-scheduler + image: registry.k8s.io/scheduler-plugins/kube-scheduler:v0.34.7 + replicaCount: 2 + leaderElect: true + + # Run on base-infrastructure nodes alongside other control plane components. + nodeSelector: + role: base-infrastructure + + tolerations: + - key: CriticalAddonsOnly + operator: Equal + value: "true" + effect: NoSchedule + + resources: + requests: + cpu: 200m + memory: 256Mi + limits: + cpu: 500m + memory: 512Mi + +# The controller is needed for Coscheduling/CapacityScheduling plugins. +# We only use NodeResourceTopologyMatch, so disable it. +controller: + replicaCount: 0 + +# Only enable NodeResourceTopologyMatch — we don't need Coscheduling, +# CapacityScheduling, or other plugins. +plugins: + enabled: + - NodeResourceTopologyMatch + disabled: [] + +pluginConfig: + - name: NodeResourceTopologyMatch + args: + scoringStrategy: + # Pack pods onto already-busy NUMA zones so other zones stay + # fully free for large (4-GPU) requests. Without this, small + # pods spread across sockets and fragment both. + type: MostAllocated diff --git a/osdc/modules/numa-scheduler/tests/smoke/conftest.py b/osdc/modules/numa-scheduler/tests/smoke/conftest.py new file mode 100644 index 00000000..8dfd070c --- /dev/null +++ b/osdc/modules/numa-scheduler/tests/smoke/conftest.py @@ -0,0 +1 @@ +from smoke_conftest import * # noqa: F403 diff --git a/osdc/modules/numa-scheduler/tests/smoke/test_numa_scheduler.py b/osdc/modules/numa-scheduler/tests/smoke/test_numa_scheduler.py new file mode 100644 index 00000000..c0067707 --- /dev/null +++ b/osdc/modules/numa-scheduler/tests/smoke/test_numa_scheduler.py @@ -0,0 +1,101 @@ +"""Smoke tests for the NUMA-aware secondary scheduler. + +Validates that the numa-scheduler namespace exists, the Helm release is +deployed, the scheduler Deployment is ready, and pods are running on +base-infrastructure nodes. +""" + +from __future__ import annotations + +import pytest +from helpers import ( + assert_deployment_ready, + filter_pods, + find_helm_release, +) + +pytestmark = [pytest.mark.live] + +NAMESPACE = "numa-scheduler" + + +# ============================================================================ +# Namespace +# ============================================================================ + + +class TestNUMASchedulerNamespace: + """Verify numa-scheduler namespace exists.""" + + def test_namespace_exists(self, all_namespaces: dict) -> None: + ns_names = [ns["metadata"]["name"] for ns in all_namespaces.get("items", [])] + assert NAMESPACE in ns_names, f"Namespace '{NAMESPACE}' not found" + + +# ============================================================================ +# Helm Release +# ============================================================================ + + +class TestNUMASchedulerHelm: + """Verify numa-scheduler Helm release is deployed.""" + + def test_helm_release_deployed(self, all_helm_releases: list[dict]) -> None: + release = find_helm_release(all_helm_releases, "numa-scheduler", namespace=NAMESPACE) + assert release is not None, "Helm release 'numa-scheduler' not found in 'numa-scheduler' namespace" + status = release.get("status", "") + assert status == "deployed", f"numa-scheduler Helm release status is '{status}', expected 'deployed'" + + +# ============================================================================ +# Scheduler Deployment +# ============================================================================ + + +class TestNUMASchedulerDeployment: + """Verify the numa-scheduler Deployment is ready.""" + + def test_deployment_ready(self, all_deployments: dict) -> None: + assert_deployment_ready(all_deployments, NAMESPACE, "numa-scheduler") + + +# ============================================================================ +# Scheduler Pods +# ============================================================================ + + +class TestNUMASchedulerPods: + """Verify scheduler pods are running on base-infrastructure nodes.""" + + def test_pods_running(self, all_pods: dict) -> None: + pods = filter_pods(all_pods, namespace=NAMESPACE) + running = [p for p in pods if p.get("status", {}).get("phase") == "Running"] + assert len(running) >= 1, f"Expected at least 1 Running numa-scheduler pod, found {len(running)}" + + def test_pods_on_base_infrastructure_nodes(self, all_pods: dict, all_nodes: dict) -> None: + """Verify scheduler pods run on base-infrastructure nodes. + + The numa-scheduler values.yaml sets nodeSelector: role: base-infrastructure + to colocate with other control plane components. This test catches + misconfigurations that would schedule the secondary scheduler onto + GPU worker nodes. + """ + pods = filter_pods(all_pods, namespace=NAMESPACE) + running = [p for p in pods if p.get("status", {}).get("phase") == "Running"] + if not running: + pytest.skip("No running numa-scheduler pods to verify node placement") + + node_labels: dict[str, dict] = {} + for node in all_nodes.get("items", []): + name = node.get("metadata", {}).get("name", "") + labels = node.get("metadata", {}).get("labels", {}) + node_labels[name] = labels + + for pod in running: + node_name = pod.get("spec", {}).get("nodeName", "") + labels = node_labels.get(node_name, {}) + role = labels.get("role", "") + assert role == "base-infrastructure", ( + f"Pod '{pod['metadata']['name']}' is on node '{node_name}' " + f"with role='{role}', expected 'base-infrastructure'" + )