From bf4382819131b9346960a710c01334746994d42b Mon Sep 17 00:00:00 2001 From: Dmitry Shmulevich <17212177+dmitsh@users.noreply.github.com> Date: Tue, 11 Aug 2026 14:16:06 -0700 Subject: [PATCH] feat(engines): make accelerator domain source labels configurable Signed-off-by: Dmitry Shmulevich <17212177+dmitsh@users.noreply.github.com> --- CHANGELOG.md | 6 +- charts/topograph/templates/_validation.tpl | 9 + charts/topograph/tests/validation_test.yaml | 11 + charts/topograph/values.schema.json | 9 +- .../values.slinky.block-example.yaml | 2 +- .../values.slinky.ib.block-example.yaml | 7 +- .../values.slinky.partition-example.yaml | 2 +- charts/topograph/values.yaml | 10 +- demos/dra-slinky/values.dra-slinky.kwok.yaml | 1 - docs/api.md | 2 +- docs/design/nfd-engine-sdd.md | 33 ++- docs/engines/k8s.md | 39 ++- docs/engines/nfd.md | 26 +- docs/engines/slinky.md | 20 +- docs/reference/node-labels.md | 37 ++- internal/k8s/utils.go | 11 + internal/k8s/utils_test.go | 32 ++- pkg/engines/k8s/engine.go | 12 + pkg/engines/k8s/engine_test.go | 25 ++ pkg/engines/k8s/kubernetes.go | 66 +++-- pkg/engines/k8s/kubernetes_test.go | 68 ++--- pkg/engines/k8s/labeler.go | 8 +- pkg/engines/nfd/engine.go | 20 +- pkg/engines/nfd/engine_test.go | 128 ++++++--- pkg/engines/nfd/objects.go | 36 ++- pkg/engines/slinky/engine.go | 53 ++-- pkg/engines/slinky/engine_test.go | 242 ++++++++++++++---- pkg/topology/graph.go | 2 +- pkg/topology/graph_test.go | 7 +- pkg/topology/instances.go | 11 - tests/ci/values.slinky-dra-block.yaml | 1 - 31 files changed, 670 insertions(+), 266 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd2f8f2f..10edc243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Pluggable accelerator-domain discovery for InfiniBand providers, independently selectable from fabric discovery with `nvidia-smi`, an explicitly configured Kubernetes Node label, or no accelerator source. Discovery is disabled when `accelerator` is omitted or empty; a non-empty section must set `source` explicitly. Helm defaults the `nvidia-smi` workload location to the `gpu-operator` namespace and `nvidia-device-plugin-daemonset` DaemonSet when those values are omitted. - Helm `kubeClient.qps` and `kubeClient.burst` values for tuning the DRA provider and the Kubernetes, NFD, and Slinky engine clients through deployment-level `KUBE_QPS` and `KUBE_BURST` settings. - The Kubernetes engine now publishes `accelerator.topograph.run/sub-domain` when a provider supplies `InstanceTopology.XclrSubDomainID`. -- The NFD engine now publishes separate `xclr-domain` and `xclr-sub-domain` attributes and groups. +- The NFD engine now publishes separate `accelerator-domain` and `accelerator-sub-domain` attributes and groups. - The graph engine now includes `accelerator.topograph.run/sub-domain` in instance labels when supplied alongside an XCLR domain. - Exported Go constant `topology.KeyTopologyXclrSubDomain` for the `accelerator.topograph.run/sub-domain` label key. - Slurm and Slinky block topology configurations can set `blockName.nodeNameRegexp` and `blockName.format` to derive unique block names from site-specific node naming conventions. @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- **BREAKING:** The Kubernetes, NFD, and Slinky engines now use the optional `acceleratorDomainSourceLabel` parameter to select an existing Kubernetes Node label as the authoritative accelerator-domain source. There is no default source label, so `nvidia.com/gpu.clique` is no longer implicitly authoritative. The Slinky `useGpuCliqueLabel` parameter has been removed. The k8s engine does not allow `acceleratorLabel` and `acceleratorDomainSourceLabel` to be configured together. - InfiniBand providers now query NVL partition IDs with the `nvidia-smi` CSV query interface, merge identical per-GPU rows, reject unavailable (`N/A`) fields, and normalize the result to `ClusterUUID.CliqueId`. - **BREAKING:** The default Kubernetes topology labels now use the vendor-neutral Topograph domains `fabric.topograph.run/tier-N`, `accelerator.topograph.run/domain`, and `accelerator.topograph.run/sub-domain`. Consumers must update topology keys, selectors, allowlists, and scheduling policies to use the new labels. - The node-observer now processes its existing topology-generation triggers through a client-go rate-limiting work queue, coalescing event bursts into a single cluster-wide reconciliation while preserving existing trigger and retry behavior. @@ -42,7 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **BREAKING (Helm chart `0.5.0` → `0.6.0`):** the chart now ships a hardened default security context across the API server, node-observer, and node-data-broker: non-root (`runAsNonRoot`, UID/GID `65532`), `seccompProfile: RuntimeDefault`, `allowPrivilegeEscalation: false`, `readOnlyRootFilesystem: true`, and all capabilities dropped — satisfying the Kubernetes `restricted` Pod Security Standard out of the box. This changes the default runtime posture of every workload; operators who relied on root, a writable rootfs, or added capabilities must override the relevant keys (see the migration note below). `appVersion` is unchanged (`v0.5.0`; no binary change). - Go toolchain bumped to **1.26.5** (`go.mod`, `Dockerfile`, CI) to address reachable stdlib vulnerabilities reported by `govulncheck`. - Slinky partition discovery now prefers the Slinky controller pod and falls back to a login pod, so clusters without optional login pods can still discover partitions ([#362](https://github.com/NVIDIA/topograph/pull/362)). -- Slinky engine `useGpuCliqueLabel` now emits an actionable diagnostic when no block domains can be built: the error reports how many nodes were scanned and why each was skipped (no Slurm mapping, missing `nvidia.com/gpu.clique` label, or missing the node-data-broker-written `topograph.nvidia.com/instance` annotation), and lists the offending node names. When no Kubernetes nodes are selected at all, it reports a distinct error pointing at the engine `nodeSelector`. +- Slinky engine label-backed block-domain generation emits an actionable diagnostic when no domains can be built: the error reports the configured source label, how many nodes were scanned, and why each was skipped (no Slurm mapping, missing source label, or a missing/empty node-data-broker-written `topograph.nvidia.com/instance` annotation), and lists the offending node names. When no Kubernetes nodes are selected at all, it reports a distinct error pointing at the engine `nodeSelector`. - Simulation model YAML schema simplified: compute nodes are now declared through `blocks[].nodes`, with optional `blocks[].switch` attachment and `blocks[].labels`; the older separate `nodes` and `capacity_blocks` sections were removed from fixtures and docs ([#394](https://github.com/NVIDIA/topograph/pull/394)). ### Fixed @@ -58,7 +59,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Slinky dynamic-node reconciliation now reuses listed Node annotations, skips unchanged nodes without a per-node GET, and patches only changed topology annotations, substantially reducing Kubernetes client-side throttling on large clusters. - Corrected DRA provider guidance to document its Slinky-only block-topology scope, dependency on pre-existing `nvidia.com/gpu.clique` labels, and inability to guide placement across NVLink partitions without backend-fabric topology. - The NFD engine now rejects an empty generated object set when cleanup is enabled, preserving the last published topology instead of deleting every Topograph-managed NFD object after an empty provider result or over-narrow node selection. -- The NFD engine now groups nodes with `nvidia.com/gpu.clique` by that authoritative accelerator value instead of omitting their accelerator attribute. - The NFD engine now publishes the `system.name/nodename` attribute required by NFD to populate `NodeFeatureGroup.status.nodes`, including for simulated KWOK nodes where no NFD worker executes. - The DRA provider now matches nodes using the `topograph.nvidia.com/instance` annotation instead of assuming the instance ID equals the Kubernetes node name. - `kwok-nodes` now maps generated instance IDs back to model hostnames when naming Kubernetes nodes and writing the Topograph instance annotation. diff --git a/charts/topograph/templates/_validation.tpl b/charts/topograph/templates/_validation.tpl index 4eda04c2..b9bba24a 100644 --- a/charts/topograph/templates/_validation.tpl +++ b/charts/topograph/templates/_validation.tpl @@ -12,6 +12,15 @@ {{- fail "env.NFD_NAMESPACE is managed by the chart for the nfd engine; configure nfdNamespace instead" }} {{- end }} +{{- if eq .Values.engine.name "k8s" }} +{{- $engineParams := default dict .Values.engine.params }} +{{- $acceleratorLabel := trim (toString (get $engineParams "acceleratorLabel")) }} +{{- $acceleratorDomainSourceLabel := trim (toString (get $engineParams "acceleratorDomainSourceLabel")) }} +{{- if and (ne $acceleratorLabel "") (ne $acceleratorDomainSourceLabel "") }} + {{- fail "engine.params.acceleratorLabel and engine.params.acceleratorDomainSourceLabel cannot be set together for the k8s engine" }} +{{- end }} +{{- end }} + {{- if hasKey (default dict .Values.env) "KUBE_QPS" }} {{- fail "env.KUBE_QPS is managed by the chart; configure kubeClient.qps instead" }} {{- end }} diff --git a/charts/topograph/tests/validation_test.yaml b/charts/topograph/tests/validation_test.yaml index 89437512..74161fcb 100644 --- a/charts/topograph/tests/validation_test.yaml +++ b/charts/topograph/tests/validation_test.yaml @@ -61,6 +61,17 @@ tests: - failedTemplate: errorMessage: "env.NFD_NAMESPACE is managed by the chart for the nfd engine; configure nfdNamespace instead" + - it: rejects accelerator output customization with an accelerator domain source label + set: + engine: + name: k8s + params: + acceleratorLabel: example.com/output-domain + acceleratorDomainSourceLabel: example.com/source-domain + asserts: + - failedTemplate: + errorMessage: "engine.params.acceleratorLabel and engine.params.acceleratorDomainSourceLabel cannot be set together for the k8s engine" + - it: rejects overriding the chart-managed Kubernetes QPS environment variable set: env: diff --git a/charts/topograph/values.schema.json b/charts/topograph/values.schema.json index ee371e11..e055d335 100644 --- a/charts/topograph/values.schema.json +++ b/charts/topograph/values.schema.json @@ -49,7 +49,14 @@ }, "params": { "type": "object", - "description": "Engine-specific parameters. The k8s engine accepts a closest-first fabricLabels array and a singular acceleratorLabel for the accelerator domain; accelerator sub-domains use the fixed accelerator.topograph.run/sub-domain key. Omitted values use the default label keys; when fabricLabels is provided, additional fabric tiers are omitted. For slinky, useGpuCliqueLabel=true reads nvidia.com/gpu.clique as the topology/block domain source." + "description": "Engine-specific parameters. The k8s engine accepts a closest-first fabricLabels array and a singular acceleratorLabel for the accelerator domain; accelerator sub-domains use the fixed accelerator.topograph.run/sub-domain key. Omitted values use the default label keys; when fabricLabels is provided, additional fabric tiers are omitted. The k8s, nfd, and slinky engines accept acceleratorDomainSourceLabel to select an existing Kubernetes Node label as the authoritative accelerator-domain source. There is no default. The k8s engine does not allow acceleratorLabel and acceleratorDomainSourceLabel to be set together.", + "properties": { + "acceleratorDomainSourceLabel": { + "type": "string", + "minLength": 1, + "description": "Optional Kubernetes Node label key used as the authoritative accelerator-domain source by the k8s, nfd, and slinky engines. Nodes without a non-empty value retain provider domains with k8s and nfd. Slinky excludes them from label-backed block domains and fails if no usable domains remain." + } + } } }, "required": ["name"] diff --git a/charts/topograph/values.slinky.block-example.yaml b/charts/topograph/values.slinky.block-example.yaml index 1c889448..c1302ee6 100644 --- a/charts/topograph/values.slinky.block-example.yaml +++ b/charts/topograph/values.slinky.block-example.yaml @@ -19,7 +19,7 @@ engine: app.kubernetes.io/component: compute plugin: topology/block blockSizes: [4] - # useGpuCliqueLabel: true + # acceleratorDomainSourceLabel: example.com/accelerator-domain topologyConfigPath: topology.conf topologyConfigmapName: slurm-config useDynamicNodes: false diff --git a/charts/topograph/values.slinky.ib.block-example.yaml b/charts/topograph/values.slinky.ib.block-example.yaml index d538800c..7fc2e7b1 100644 --- a/charts/topograph/values.slinky.ib.block-example.yaml +++ b/charts/topograph/values.slinky.ib.block-example.yaml @@ -39,9 +39,10 @@ engine: # Base block size for topology/block. Additional entries, if used, must # be larger power-of-two multiples of the previous size. blockSizes: [4] - # For block topology, use nvidia.com/gpu.clique as the block-domain - # source. Keep this aligned with provider.params.accelerator. - useGpuCliqueLabel: true + # For block topology, use this existing Node label as the authoritative + # block-domain source. This engine setting is independent of provider + # discovery, even when both happen to select the same label. + acceleratorDomainSourceLabel: nvidia.com/gpu.clique # Key inside the target ConfigMap that Slinky mounts as topology.conf. topologyConfigPath: topology.conf # Target Slinky ConfigMap updated by Topograph with generated topology diff --git a/charts/topograph/values.slinky.partition-example.yaml b/charts/topograph/values.slinky.partition-example.yaml index c15044d6..061dc9f5 100644 --- a/charts/topograph/values.slinky.partition-example.yaml +++ b/charts/topograph/values.slinky.partition-example.yaml @@ -44,7 +44,7 @@ engine: clusterDefault: true topologyConfigPath: topology.conf topologyConfigmapName: slurm-config - # useGpuCliqueLabel: true + # acceleratorDomainSourceLabel: example.com/accelerator-domain useDynamicNodes: true configUpdateMode: skeleton-only diff --git a/charts/topograph/values.yaml b/charts/topograph/values.yaml index b5cbf4d3..5e7f4c8c 100644 --- a/charts/topograph/values.yaml +++ b/charts/topograph/values.yaml @@ -26,9 +26,13 @@ engine: # name: "k8s", "nfd", "slinky", "slurm" or "graph" name: k8s # params: - # # For slinky topology/block output, use the GPU Operator's existing - # # Kubernetes node label as the block-domain source. - # useGpuCliqueLabel: true + # # For the k8s, nfd, and slinky engines, optionally use an existing + # # Kubernetes Node label as the authoritative accelerator-domain source. + # # There is no default. The k8s engine does not allow acceleratorLabel + # # to be customized when this parameter is set. Nodes missing the label + # # retain provider domains with k8s and nfd; Slinky excludes them from + # # label-backed block domains and fails if no usable domains remain. + # acceleratorDomainSourceLabel: example.com/accelerator-domain # Kubernetes client rate limits for the DRA provider and the k8s, NFD, and # Slinky engines. The chart passes configured values to the Topograph deployment diff --git a/demos/dra-slinky/values.dra-slinky.kwok.yaml b/demos/dra-slinky/values.dra-slinky.kwok.yaml index 81046179..48d1c72a 100644 --- a/demos/dra-slinky/values.dra-slinky.kwok.yaml +++ b/demos/dra-slinky/values.dra-slinky.kwok.yaml @@ -17,7 +17,6 @@ engine: blockName: nodeNameRegexp: '^srv([0-9]{2})' format: 'rack${1}' - useGpuCliqueLabel: true topologyConfigPath: topology.conf topologyConfigmapName: slurm-config-extra useDynamicNodes: true diff --git a/docs/api.md b/docs/api.md index 66024df4..80e5b44a 100644 --- a/docs/api.md +++ b/docs/api.md @@ -99,9 +99,9 @@ Topograph exposes three endpoints for interacting with the service. Below are th - **nodeSelector**: (optional) Used in: [`k8s`, `nfd`, `slinky`]. A Kubernetes node label map that filters which nodes participate in topology generation. - **fabricLabels**: (optional) Used in: [`k8s`]. Closest-first array of Kubernetes label keys for fabric tiers. If omitted, every discovered fabric tier uses its default numbered key; if provided, tiers beyond the array are omitted. - **acceleratorLabel**: (optional) Used in: [`k8s`]. Kubernetes label key for the accelerator domain. Defaults to `accelerator.topograph.run/domain`. The optional accelerator sub-domain always uses `accelerator.topograph.run/sub-domain`. + - **acceleratorDomainSourceLabel**: (optional) Used in: [`k8s`, `nfd`, `slinky`]. Existing Kubernetes Node label used as the authoritative accelerator-domain source. There is no default. Source-label overrides suppress provider accelerator sub-domains. For `k8s` and `nfd`, nodes without a non-empty source value retain their provider-derived accelerator domains. For Slinky block topology, those nodes are excluded from label-backed domains; generation fails if no usable label-backed domains can be constructed. For `k8s`, this parameter cannot be set together with `acceleratorLabel`, and the source label is never overwritten. - **topologyConfigmapName**: Used in: [`slinky`]. The required name of the ConfigMap containing the topology config. - **useDynamicNodes**: (optional) Used in: [`slinky`]. If `true`, Kubernetes nodes matched by the Node Selector will be annotated with the topology spec. - - **useGpuCliqueLabel**: (optional) Used in: [`slinky`]. If `true`, `topology/block` domains are built from the GPU Operator's `nvidia.com/gpu.clique` node label instead of provider accelerator-domain data. - **configUpdateMode**: (optional) Used in: [`slinky`]. By default, the full topology YAML is written in the Slurm ConfigMap. `skeleton-only` overrides to include switches or blocks only (no node lines); `none` skips updating the topology key in the ConfigMap. - **nodes**: (optional) Supplies the cluster nodes used for topology generation as an array of regions mapping instance IDs to node names. diff --git a/docs/design/nfd-engine-sdd.md b/docs/design/nfd-engine-sdd.md index 1c413271..86f6c1df 100644 --- a/docs/design/nfd-engine-sdd.md +++ b/docs/design/nfd-engine-sdd.md @@ -9,7 +9,7 @@ Implemented. Add an experimental `nfd` engine that converts Topograph's canonical `topology.Graph` into Node Feature Discovery (NFD) `NodeFeatureGroup` objects. The engine creates one group for each distinct topology label value, such as one -group for each distinct fabric tier, XCLR domain, or XCLR sub-domain value. +group for each distinct fabric tier, accelerator domain, or accelerator sub-domain value. This should not replace the current `k8s` engine. The `k8s` engine writes node labels that can be consumed by native Kubernetes affinity and topology-aware @@ -18,7 +18,7 @@ NFD CRs for consumers that already watch NFD. ## Background -Topograph maps topology into optional XCLR dimensions and a variable-depth +Topograph maps topology into optional accelerator dimensions and a variable-depth fabric label family: - `accelerator.topograph.run/domain` @@ -74,8 +74,8 @@ spec: nodename: node-a topograph.network: elements: - xclr-domain: nvl3 - xclr-sub-domain: nvl3.rack01 + accelerator-domain: nvl3 + accelerator-sub-domain: nvl3.rack01 fabric-tier-0: leaf-12 fabric-tier-1: spine-2 fabric-tier-2: core-1 @@ -119,6 +119,11 @@ Initial parameters: graph. An empty generated object set is rejected while cleanup is enabled so a transient empty provider result cannot delete the entire published topology. +- `acceleratorDomainSourceLabel`: optional Kubernetes Node label key. A + non-empty value replaces the provider accelerator domain for that node, + suppresses the provider accelerator sub-domain, and is recorded as the source + key on the corresponding `NodeFeatureGroup`. Nodes without a usable source + value retain provider-derived accelerator attributes. The NFD master namespace is deployment-scoped rather than request-scoped. Helm configures it through the top-level `nfdNamespace` value and passes it to the @@ -131,8 +136,8 @@ returns an error if `NFD_NAMESPACE` is unset or blank. - Add `pkg/engines/nfd` with the standard `NamedLoader`. - Register it in `pkg/registry/registry.go`. - Factor the current `k8s` label projection into a shared helper so both engines - produce identical values at every discovered fabric tier, XCLR domain, and - XCLR sub-domain. + produce identical values at every discovered fabric tier, accelerator domain, + and accelerator sub-domain. - Use the dynamic Kubernetes client or generated NFD client types, depending on whether the project wants to pin an NFD API dependency. - Update Helm RBAC to allow create, update, patch, list, watch, and delete for @@ -191,13 +196,13 @@ Two NFD-side extensions could make this model more practical: For a 10,000-node cluster, the live custom-resource payload depends on the number of topology dimensions and the number of distinct values within each dimension. Let `D` be the average number of published dimensions per node: all -discovered fabric tiers plus any XCLR domain and sub-domain. +discovered fabric tiers plus any accelerator domain and sub-domain. Assumptions: - 10,000 `NodeFeature` objects, one per node. -- One topology attribute for every discovered fabric tier, XCLR domain, and - XCLR sub-domain. +- One topology attribute for every discovered fabric tier, accelerator domain, + and accelerator sub-domain. - Each node appears in one `NodeFeatureGroup.status.nodes` list per topology dimension, so status contains about `10,000 × D` node references total. - Average node names and topology values are short, roughly 10-30 characters. @@ -228,14 +233,14 @@ patches to reduce write amplification. ## Test Plan -- Unit-test graph-to-group generation across variable fabric tiers, XCLR - domains, and XCLR sub-domains. +- Unit-test graph-to-group generation across variable fabric tiers, accelerator + domains, and accelerator sub-domains. - Verify long and invalid topology values produce stable CR names. - Verify stale Topograph-managed groups are removed when `cleanup` is enabled. - Verify an empty generated object set returns an error and preserves existing objects when `cleanup` is enabled. -- Verify nodes with `nvidia.com/gpu.clique` replace the provider XCLR domain - with the clique value and suppress the provider XCLR sub-domain, matching the - `k8s` engine's `skipXclrLabelsWhenGPUCliqueExists` behavior. +- Verify nodes with a configured custom accelerator-domain source label replace + the provider accelerator domain and suppress the provider accelerator + sub-domain, while nodes without the label retain provider values. - Add a fake dynamic-client test that applies generated `NodeFeature` and `NodeFeatureGroup` objects without requiring a live NFD deployment. diff --git a/docs/engines/k8s.md b/docs/engines/k8s.md index a22a0470..0c466122 100644 --- a/docs/engines/k8s.md +++ b/docs/engines/k8s.md @@ -14,10 +14,12 @@ accelerator-network locality as a two-level label hierarchy: Fabric tier 0 is closest to the compute node, and tier numbers increase outwards. Topograph writes only the fabric tiers enabled by the label -configuration. If a node already has `nvidia.com/gpu.clique`, the accelerator -domain label remains unset and the GPU Operator label is authoritative. -The accelerator sub-domain label is also omitted because it is only valid -alongside the XCLR domain label. +configuration. When `acceleratorDomainSourceLabel` is configured and a node +has a non-empty value for that label, the existing label is authoritative and +is not overwritten. The managed accelerator domain and sub-domain labels are +omitted for that node. When the parameter is omitted, provider-supplied +accelerator domains are used and no Kubernetes label receives special +treatment. The fabric and accelerator-domain label names are configurable via the [Helm chart](https://github.com/NVIDIA/topograph/tree/main/charts/topograph). @@ -66,14 +68,27 @@ graph TB scheduler -. "schedules Pods onto nodes;\nTopology Manager handles\nresource alignment inside each node" .-> tm ``` -### Relationship to `nvidia.com/gpu.clique` +### Using an existing accelerator-domain label -Some GPU Operator deployments expose `nvidia.com/gpu.clique` on nodes with Multi-Node NVLink (MNNVL) GPUs (e.g., GB200 NVL72). When present, this label identifies the NVLink clique a node belongs to and can be used as a topology key for Pod placement; its presence is not guaranteed on every MNNVL cluster. +Set `engine.params.acceleratorDomainSourceLabel` when another component already +publishes the accelerator domain as a Kubernetes Node label: -Topograph treats `nvidia.com/gpu.clique` as the authoritative accelerator node label when it is already present: +```yaml +engine: + name: k8s + params: + acceleratorDomainSourceLabel: example.com/accelerator-domain +``` + +For each node with a non-empty value, the k8s engine preserves that label, +removes its managed accelerator domain and sub-domain labels, and continues to +publish all configured fabric tiers. Nodes without the source label use the +provider-supplied accelerator domain and sub-domain. `acceleratorLabel` cannot +be customized when `acceleratorDomainSourceLabel` is set. -- On **MNNVL systems**: if `nvidia.com/gpu.clique` exists on a node, the k8s engine does not write the accelerator domain or sub-domain labels for that node. It still writes every configured fabric tier. -- On **non-MNNVL systems** (e.g., DGX B200, B300): `nvidia.com/gpu.clique` is not set (see the [node labels reference](../reference/node-labels.md) for the Fabric Manager init and `GPU_FABRIC_STATE_COMPLETED` details). Topograph writes the configured accelerator label when the selected provider supplies an accelerator domain. +There is no default source label. For example, operators who want the NVIDIA +GPU Operator's `nvidia.com/gpu.clique` label to be authoritative must select it +explicitly. Its presence alone has no effect on engine output. In addition to NVLink domain membership, Topograph provides the full IB switch hierarchy as numbered fabric tiers, giving schedulers both dimensions simultaneously. @@ -144,7 +159,7 @@ provider: engine: name: k8s params: - # Optional closest-first fabric keys and accelerator-domain key. + # Optional closest-first fabric keys and accelerator-domain output key. # The sub-domain key remains accelerator.topograph.run/sub-domain. # Additional fabric tiers are omitted when the array is configured. fabricLabels: @@ -152,6 +167,10 @@ engine: - example.com/pod acceleratorLabel: example.com/nvl-domain + # Alternatively, use an existing Node label as the accelerator-domain + # source. Do not set acceleratorLabel when enabling this option. + # acceleratorDomainSourceLabel: example.com/accelerator-domain + # Shared by supported Kubernetes-backed providers and engines. kubeClient: qps: 50 diff --git a/docs/engines/nfd.md b/docs/engines/nfd.md index 00dd5601..9fb4af58 100644 --- a/docs/engines/nfd.md +++ b/docs/engines/nfd.md @@ -8,8 +8,8 @@ It creates: - one `NodeFeature` per topology node, carrying Topograph topology as `spec.features.attributes.topograph.network.elements` -- one `NodeFeatureGroup` per distinct fabric tier, XCLR domain, or XCLR - sub-domain value +- one `NodeFeatureGroup` per distinct fabric tier, accelerator domain, or + accelerator sub-domain value NFD master evaluates those features and writes matching nodes to `NodeFeatureGroup.status.nodes`. @@ -17,7 +17,7 @@ NFD master evaluates those features and writes matching nodes to Fabric topology is variable-depth. The engine publishes `fabric-tier-N` for every discovered tier, where tier 0 is closest to the node and higher tiers progress outward; there is no fixed number of fabric attributes or groups. -XCLR domain and optional sub-domain attributes are published separately when +Accelerator domain and optional sub-domain attributes are published separately when the provider supplies them. ## When to Use @@ -69,6 +69,7 @@ engine: nodeSelector: nvidia.com/gpu.present: "true" cleanup: true + acceleratorDomainSourceLabel: example.com/accelerator-domain kubeClient: qps: 50 burst: 100 @@ -81,6 +82,7 @@ Parameters: |---|---:|---|---| | `nodeSelector` | No | all nodes | Limits the Kubernetes nodes used as provider input. Same meaning as the `k8s` engine selector. | | `cleanup` | No | `true` | Deletes stale Topograph-managed `NodeFeature` and `NodeFeatureGroup` objects that are no longer present in the generated topology. If generation produces no objects, the engine returns an error and preserves the existing topology. | +| `acceleratorDomainSourceLabel` | No | none | Existing Kubernetes Node label used as the authoritative accelerator-domain source. Values replace the provider domain and suppress the provider sub-domain for matching nodes. | `nfdNamespace` is a deployment-level Helm value, not an engine request parameter. It must be the namespace where NFD master runs because NFD updates @@ -128,8 +130,8 @@ spec: nodename: node-a topograph.network: elements: - xclr-domain: nvl3 - xclr-sub-domain: nvl3.rack01 + accelerator-domain: nvl3 + accelerator-sub-domain: nvl3.rack01 fabric-tier-0: leaf-12 fabric-tier-1: spine-2 fabric-tier-2: core-1 @@ -165,12 +167,14 @@ Topograph includes `system.name.elements.nodename` so NFD can populate group membership even when an NFD worker does not run on the node, as with simulated KWOK nodes. Topograph does not write `status.nodes`; NFD owns status updates. -If a Kubernetes node already has `nvidia.com/gpu.clique`, the engine uses that -label's value as the authoritative `xclr-domain` attribute instead of the value -derived from the provider graph. The matching `NodeFeatureGroup` records -`nvidia.com/gpu.clique` as its source label key. The provider-supplied `xclr-sub-domain` is suppressed alongside `xclr-domain` -(matching the `k8s` engine behaviour); all fabric-tier attributes are still -published. +When `acceleratorDomainSourceLabel` is configured and a Kubernetes node has a +non-empty value for that label, the engine uses the value as the authoritative +`accelerator-domain` NFD attribute instead of the provider domain. The matching +`NodeFeatureGroup` records the configured key as its source label. The +provider-supplied `accelerator-sub-domain` is suppressed for that node, while all +fabric-tier attributes are still published. Nodes without the configured label +retain their provider domain and sub-domain. When the parameter is omitted, no +existing Kubernetes label receives special treatment. ## Caveats diff --git a/docs/engines/slinky.md b/docs/engines/slinky.md index 28557950..095ca542 100644 --- a/docs/engines/slinky.md +++ b/docs/engines/slinky.md @@ -90,9 +90,13 @@ engine: `blockName.nodeNameRegexp` uses Go regular-expression syntax and may match anywhere in the node name; use anchors when needed. `blockName.format` uses Go regexp expansion syntax, including numeric captures such as `${1}` and named captures such as `${domain}`. Every node in a non-empty block must match and produce the same non-empty name, and names must be unique across blocks. Invalid expressions, unmatched nodes, inconsistent names within a block, and duplicate names are rejected. Empty complemented blocks retain their generated names. -### Using `nvidia.com/gpu.clique` for block topology +### Using an existing Node label for block topology -On MNNVL Kubernetes clusters, the NVIDIA GPU Operator can label nodes with `nvidia.com/gpu.clique`. When `useGpuCliqueLabel` is enabled, the Slinky engine uses that label as the source for `topology/block` domains instead of the accelerator domains returned by the provider. This is useful with cloud API providers whose accelerator ID describes a broader provider domain than the GPU Operator clique label. +Set `acceleratorDomainSourceLabel` when another component already publishes the +desired accelerator domain as a Kubernetes Node label. For `topology/block`, +the Slinky engine uses that label instead of accelerator domains returned by +the provider. There is no default source label, and existing labels receive no +special treatment when the parameter is omitted. The option only affects block topology. Tree topology still comes from the selected provider, and the engine still maps Kubernetes nodes to Slurm nodes through the configured slurmd pod selector. @@ -108,10 +112,18 @@ engine: blockSizes: [8, 16] topologyConfigmapName: slurm-config topologyConfigPath: topology.conf - useGpuCliqueLabel: true + acceleratorDomainSourceLabel: example.com/accelerator-domain ``` -If `useGpuCliqueLabel` is enabled for a block topology and no matching nodes have the `nvidia.com/gpu.clique` label plus the Topograph instance annotation, topology generation fails with a `502` error instead of falling back to provider accelerator domains. +Nodes without the configured label retain the existing label-backed behavior: +they are skipped rather than falling back individually to provider domains. If +no usable domains can be built from the configured label and the Topograph +instance annotation, topology generation fails with an actionable `502` error +that reports the configured key and why nodes were skipped. Replacing provider +domains also suppresses provider accelerator sub-domains. + +For example, an operator may explicitly select +`nvidia.com/gpu.clique`, but the Slinky engine no longer assumes that key. ### Kubernetes API rate limiting diff --git a/docs/reference/node-labels.md b/docs/reference/node-labels.md index 5f72c571..5c2f9e66 100644 --- a/docs/reference/node-labels.md +++ b/docs/reference/node-labels.md @@ -10,7 +10,7 @@ Scheduler, gang-scheduling plugins, topology-aware bin-packers) and observability tools to reason about network locality. The [Slinky engine](../engines/slinky.md) does not write these labels; it writes Slurm topology configuration to a ConfigMap and can consume the existing -`nvidia.com/gpu.clique` label for `topology/block`. +label selected by `acceleratorDomainSourceLabel` for `topology/block`. ### Default label keys @@ -21,7 +21,7 @@ are written. | Label key | Topology type | Semantics | |---|---|---| -| `accelerator.topograph.run/domain` | Accelerator | Accelerator-interconnect locality. If `nvidia.com/gpu.clique` exists, the k8s engine leaves this label unset for that node. | +| `accelerator.topograph.run/domain` | Accelerator | Accelerator-interconnect locality. Omitted for a node when a configured `acceleratorDomainSourceLabel` supplies the authoritative value. | | `accelerator.topograph.run/sub-domain` | Accelerator | Optional locality nested within the accelerator domain. Written only when `accelerator.topograph.run/domain` is also present. | | `fabric.topograph.run/tier-N` | Fabric | Switch-fabric locality at tier `N`. Tier 0 is the switch closest to the node; each higher tier is the next switch tier outward. There is no fixed maximum depth. | @@ -51,7 +51,21 @@ The DRA provider is intentionally omitted: its supported use is with the Slinky engine, where it converts existing `nvidia.com/gpu.clique` labels into Slurm `topology/block` domains rather than writing Kubernetes topology labels. -**Relationship to `nvidia.com/gpu.clique`**: Some GPU Operator deployments expose `nvidia.com/gpu.clique` on nodes with Multi-Node NVLink (MNNVL) GPUs; it is not guaranteed to be present on every MNNVL cluster. The k8s engine treats that label as authoritative when present and does not write Topograph's configured accelerator domain or sub-domain labels for that node, regardless of whether the selected provider also returned accelerator topology from API data. For Slinky block topology, setting `engine.params.useGpuCliqueLabel: true` makes the Slinky engine build `topology/block` domains from `nvidia.com/gpu.clique` instead of provider accelerator-domain data. For `infiniband-k8s`, setting `provider.params.accelerator.source: kubernetes-label` with `kubernetesLabel.key: nvidia.com/gpu.clique` selects the same label without collecting a duplicate value through `nvidia-smi`. The `netq` provider uses a `DomainUUID` from the NMX management API — a different identifier that refers to the same physical domain but cannot be compared as a string. +**Existing accelerator-domain source labels**: The k8s, NFD, and Slinky engines +can explicitly select any valid Kubernetes Node label key with +`engine.params.acceleratorDomainSourceLabel`. A non-empty value is authoritative +for that node and suppresses the provider accelerator sub-domain. When the +parameter is omitted, engines use provider-supplied accelerator domains and do +not give any Kubernetes label special treatment. This engine setting is +independent of `provider.params.accelerator.kubernetesLabel.key`, which controls +provider discovery. + +Some GPU Operator deployments expose `nvidia.com/gpu.clique` on nodes with +Multi-Node NVLink (MNNVL) GPUs; operators may select it explicitly as either an +engine source label or an `infiniband-k8s` provider discovery label. The `netq` +provider instead uses a `DomainUUID` from the NMX management API—a different +identifier that refers to the same physical domain but cannot be compared as a +string. [NVIDIA Fabric Manager](https://docs.nvidia.com/datacenter/tesla/fabric-manager-user-guide/) runs at node init on MNNVL-capable hardware, discovers the NVLink fabric across GPUs, and registers each GPU with [NVML](https://docs.nvidia.com/deploy/nvml-api/) (NVIDIA Management Library — a C API that exposes per-GPU state). The GPU Operator's IMEX labeler writes `nvidia.com/gpu.clique` only once NVML reports the node's fabric state as `GPU_FABRIC_STATE_COMPLETED` — meaning Fabric Manager finished initialization successfully and the node is part of an NVLink domain. @@ -59,7 +73,14 @@ On non-MNNVL systems (e.g., DGX B200, B300), the GPU fabric never reaches `GPU_F ### Choosing between the accelerator label and `nvidia.com/gpu.clique` for scheduling -Workload schedulers consuming topology labels may need to choose between Topograph's `accelerator.topograph.run/domain` and the NVIDIA GPU Operator's `nvidia.com/gpu.clique`. The k8s engine automatically avoids writing the XCLR domain and sub-domain labels on nodes where `nvidia.com/gpu.clique` is already present, so schedulers can use `gpu.clique` for those nodes and fall back to the XCLR labels where it is absent: +Workload schedulers consuming topology labels may need to choose between +Topograph's `accelerator.topograph.run/domain` and the NVIDIA GPU Operator's +`nvidia.com/gpu.clique`. To make the GPU Operator label authoritative, configure +`engine.params.acceleratorDomainSourceLabel: nvidia.com/gpu.clique`. The k8s +engine then preserves that source label and omits its managed accelerator domain +and sub-domain labels on nodes where the source has a non-empty value. Without +that configuration, Topograph publishes provider-derived accelerator labels +normally: - **MNNVL hardware + Fabric Manager completed + NVL Partition granularity desired:** use `nvidia.com/gpu.clique`. On the AWS provider this is finer granularity than the accelerator label (which carries the CapacityBlockId, i.e., the NVL Domain). On InfiniBand and Lambda AI providers the two labels carry the same value. - **MNNVL but Fabric Manager not yet completed, or non-MNNVL hardware:** `nvidia.com/gpu.clique` is absent. Use `accelerator.topograph.run/domain`. @@ -89,8 +110,12 @@ provided, only explicitly listed tiers are labeled; additional tiers are omitted. `acceleratorLabel` defaults to `accelerator.topograph.run/domain`. Every configured key must be a valid Kubernetes label key. The accelerator sub-domain key is fixed at -`accelerator.topograph.run/sub-domain`. Label values always come from provider -discovery. +`accelerator.topograph.run/sub-domain`. Fabric and managed accelerator label +values normally come from provider discovery. When +`acceleratorDomainSourceLabel` is configured and a node has a non-empty value, +the k8s engine preserves that existing source label as authoritative and omits +its managed accelerator domain and sub-domain labels for the node. Nodes +without a usable source value retain the provider-derived labels. ### Relationship to upstream standardization (KEP-4962) diff --git a/internal/k8s/utils.go b/internal/k8s/utils.go index 3aaaae38..030b156b 100644 --- a/internal/k8s/utils.go +++ b/internal/k8s/utils.go @@ -14,6 +14,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -50,6 +51,16 @@ func IsPodReady(pod *corev1.Pod) bool { return false } +// ValidateLabelKey validates a required Kubernetes label key and identifies +// its configuration location in any returned error. +func ValidateLabelKey(location, key string) error { + if errs := validation.IsQualifiedName(key); len(errs) != 0 { + return fmt.Errorf("%s %q is not a valid Kubernetes label key: %s", + location, key, strings.Join(errs, "; ")) + } + return nil +} + func GetDaemonSetPods(ctx context.Context, client kubernetes.Interface, name, namespace, nodename string) (*corev1.PodList, error) { ds, err := client.AppsV1().DaemonSets(namespace).Get(ctx, name, metav1.GetOptions{}) if err != nil { diff --git a/internal/k8s/utils_test.go b/internal/k8s/utils_test.go index ea39ddaa..14994fba 100644 --- a/internal/k8s/utils_test.go +++ b/internal/k8s/utils_test.go @@ -1,5 +1,5 @@ /* - * Copyright 2025 NVIDIA CORPORATION + * Copyright 2025-2026 NVIDIA CORPORATION * SPDX-License-Identifier: Apache-2.0 */ @@ -80,3 +80,33 @@ func TestIsPodReady(t *testing.T) { }) } } + +func TestValidateLabelKey(t *testing.T) { + testCases := []struct { + name string + key string + err string + }{ + {name: "valid", key: "example.com/accelerator-domain"}, + { + name: "empty", + err: `acceleratorDomainSourceLabel "" is not a valid Kubernetes label key`, + }, + { + name: "invalid", + key: "not a label", + err: `acceleratorDomainSourceLabel "not a label" is not a valid Kubernetes label key`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := ValidateLabelKey("acceleratorDomainSourceLabel", tc.key) + if tc.err == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tc.err) + }) + } +} diff --git a/pkg/engines/k8s/engine.go b/pkg/engines/k8s/engine.go index 0bc8cfa5..c1c82517 100644 --- a/pkg/engines/k8s/engine.go +++ b/pkg/engines/k8s/engine.go @@ -7,6 +7,7 @@ package k8s import ( "context" + "fmt" "net/http" corev1 "k8s.io/api/core/v1" @@ -40,6 +41,9 @@ type Params struct { FabricLabels []string `mapstructure:"fabricLabels"` // AcceleratorLabel optionally sets the accelerator label key. AcceleratorLabel string `mapstructure:"acceleratorLabel"` + // AcceleratorDomainSourceLabel optionally selects an existing Kubernetes + // Node label as the authoritative accelerator-domain source. + AcceleratorDomainSourceLabel string `mapstructure:"acceleratorDomainSourceLabel"` // derived fields nodeListOpt *metav1.ListOptions @@ -82,6 +86,14 @@ func getParameters(params engines.Config) (*Params, error) { if err := config.Decode(params, p); err != nil { return nil, err } + if p.AcceleratorLabel != "" && p.AcceleratorDomainSourceLabel != "" { + return nil, fmt.Errorf("engine parameters acceleratorLabel and acceleratorDomainSourceLabel cannot be set together") + } + if p.AcceleratorDomainSourceLabel != "" { + if err := internalk8s.ValidateLabelKey("acceleratorDomainSourceLabel", p.AcceleratorDomainSourceLabel); err != nil { + return nil, err + } + } p.labelKeys = NewTopologyLabelKeys(p.FabricLabels, p.AcceleratorLabel) if err := p.labelKeys.Validate(); err != nil { return nil, err diff --git a/pkg/engines/k8s/engine_test.go b/pkg/engines/k8s/engine_test.go index e29eca0d..3282b229 100644 --- a/pkg/engines/k8s/engine_test.go +++ b/pkg/engines/k8s/engine_test.go @@ -89,6 +89,31 @@ func TestGetParameters(t *testing.T) { }, err: `topology label key "accelerator.topograph.run/sub-domain" is configured for both acceleratorLabel and xclrSubDomainLabel`, }, + { + name: "Case 9: accelerator domain source label", + params: map[string]any{ + "acceleratorDomainSourceLabel": "example.com/accelerator-domain", + }, + ret: &Params{ + AcceleratorDomainSourceLabel: "example.com/accelerator-domain", + labelKeys: NewTopologyLabelKeys(nil, ""), + }, + }, + { + name: "Case 10: reject invalid accelerator domain source label", + params: map[string]any{ + "acceleratorDomainSourceLabel": "not a label", + }, + err: `acceleratorDomainSourceLabel "not a label" is not a valid Kubernetes label key`, + }, + { + name: "Case 11: reject customized accelerator output with source label", + params: map[string]any{ + "acceleratorLabel": "example.com/output-domain", + "acceleratorDomainSourceLabel": "example.com/source-domain", + }, + err: "engine parameters acceleratorLabel and acceleratorDomainSourceLabel cannot be set together", + }, } for _, tc := range testCases { diff --git a/pkg/engines/k8s/kubernetes.go b/pkg/engines/k8s/kubernetes.go index 337978e3..7f7ccf50 100644 --- a/pkg/engines/k8s/kubernetes.go +++ b/pkg/engines/k8s/kubernetes.go @@ -65,7 +65,12 @@ func (eng *K8sEngine) AddNodeLabels(ctx context.Context, nodeName string, labels return nil } - desiredLabels := mergeNodeLabels(node.Labels, labels, eng.params.labelKeys) + desiredLabels := mergeNodeLabels( + node.Labels, + labels, + eng.params.labelKeys, + eng.params.AcceleratorDomainSourceLabel, + ) if maps.Equal(node.Labels, desiredLabels) { return nil } @@ -156,21 +161,48 @@ func nodeLabelPatch(current, desired map[string]string) ([]byte, error) { }) } -func mergeNodeLabels(current, labels map[string]string, keys *TopologyLabelKeys) map[string]string { +// mergeNodeLabels builds the complete desired label set for a Node. It retains +// labels owned by other controllers, removes stale Topograph-managed labels, +// and then applies the labels generated from the current topology graph. A +// configured accelerator-domain source label remains externally owned and is +// preserved unchanged. +func mergeNodeLabels(current, labels map[string]string, keys *TopologyLabelKeys, acceleratorDomainSourceLabel string) map[string]string { desired := maps.Clone(current) if desired == nil { desired = make(map[string]string) } - labels = skipXclrLabelsWhenGPUCliqueExists(desired, labels, keys) - removeManagedTopologyLabels(desired, keys) + labels = skipAcceleratorLabelsWhenSourceExists(desired, labels, keys, acceleratorDomainSourceLabel) + removeManagedTopologyLabels(desired, keys, acceleratorDomainSourceLabel) maps.Copy(desired, labels) return desired } -func removeManagedTopologyLabels(labels map[string]string, keys *TopologyLabelKeys) { +// skipAcceleratorLabelsWhenSourceExists removes provider-derived accelerator +// output when the Node already carries a non-empty configured source label. +// The source value is authoritative for the accelerator domain, and the +// provider sub-domain must be suppressed because it may not belong to that +// replacement domain. Nodes without a usable source value retain the generated +// provider domain and sub-domain labels. +func skipAcceleratorLabelsWhenSourceExists(currentLabels, labels map[string]string, keys *TopologyLabelKeys, acceleratorDomainSourceLabel string) map[string]string { + if acceleratorDomainSourceLabel == "" || strings.TrimSpace(currentLabels[acceleratorDomainSourceLabel]) == "" { + return labels + } + + filtered := maps.Clone(labels) + delete(filtered, keys.XclrDomainKey()) + delete(filtered, keys.XclrSubDomainKey()) + + return filtered +} + +// removeManagedTopologyLabels clears every current label that the k8s engine +// may own so labels omitted from the latest topology are reconciled away. The +// configured accelerator-domain source label is excluded because the engine +// consumes that existing Node metadata but must never manage or overwrite it. +func removeManagedTopologyLabels(labels map[string]string, keys *TopologyLabelKeys, acceleratorDomainSourceLabel string) { for key := range labels { - if key == topology.KeyNvidiaGPUClique { + if acceleratorDomainSourceLabel != "" && key == acceleratorDomainSourceLabel { continue } if isManagedLevelLabel(key, keys) { @@ -179,6 +211,10 @@ func removeManagedTopologyLabels(labels map[string]string, keys *TopologyLabelKe } } +// isManagedLevelLabel reports whether key is a default or configured topology +// output key owned by the k8s engine. Numbered default fabric keys are matched +// by shape so stale tiers are removed even when they are absent from the +// current graph or configured key list. func isManagedLevelLabel(key string, keys *TopologyLabelKeys) bool { if key == topology.KeyTopologyXclrDomain || key == topology.KeyTopologyXclrSubDomain { return true @@ -190,8 +226,8 @@ func isManagedLevelLabel(key string, keys *TopologyLabelKeys) bool { } } for _, prefix := range []string{topology.KeyFabricTierPrefix} { - if strings.HasPrefix(key, prefix) { - level, err := strconv.Atoi(strings.TrimPrefix(key, prefix)) + if levelValue, ok := strings.CutPrefix(key, prefix); ok { + level, err := strconv.Atoi(levelValue) if err == nil && level >= 0 { return true } @@ -199,17 +235,3 @@ func isManagedLevelLabel(key string, keys *TopologyLabelKeys) bool { } return false } - -func skipXclrLabelsWhenGPUCliqueExists(currentLabels, labels map[string]string, keys *TopologyLabelKeys) map[string]string { - xclrDomainLabel := keys.XclrDomainKey() - xclrSubDomainLabel := keys.XclrSubDomainKey() - if strings.TrimSpace(currentLabels[topology.KeyNvidiaGPUClique]) == "" { - return labels - } - - filtered := maps.Clone(labels) - delete(filtered, xclrDomainLabel) - delete(filtered, xclrSubDomainLabel) - - return filtered -} diff --git a/pkg/engines/k8s/kubernetes_test.go b/pkg/engines/k8s/kubernetes_test.go index 46048c5a..4a7739f2 100644 --- a/pkg/engines/k8s/kubernetes_test.go +++ b/pkg/engines/k8s/kubernetes_test.go @@ -347,11 +347,12 @@ func countKubernetesActions(actions []k8stesting.Action, verb, resource string) func TestMergeNodeLabels(t *testing.T) { testCases := []struct { - name string - acceleratorLabel string - node *corev1.Node - in map[string]string - out map[string]string + name string + acceleratorLabel string + acceleratorDomainSourceLabel string + node *corev1.Node + in map[string]string + out map[string]string }{ { name: "Case 1: no labels", @@ -376,11 +377,12 @@ func TestMergeNodeLabels(t *testing.T) { out: map[string]string{"a": "1", "b": "2", "c": "3", "d": "4"}, }, { - name: "Case 4: skip accelerator when GPU clique exists", + name: "Case 4: source label overrides provider accelerator domain", + acceleratorDomainSourceLabel: "example.com/accelerator-domain", node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", + "example.com/accelerator-domain": "source-domain", topology.KeyTopologyXclrDomain: "old-domain", topology.KeyTopologyXclrSubDomain: "old-sub-domain", topology.FabricTierKey(0): "old-leaf", @@ -397,56 +399,51 @@ func TestMergeNodeLabels(t *testing.T) { topology.FabricTierKey(1): "new-spine", }, out: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", - topology.FabricTierKey(0): "new-leaf", - topology.FabricTierKey(1): "new-spine", - "fabric.topograph.run/core": "legacy-core", - "workload.example/label": "keep", + "example.com/accelerator-domain": "source-domain", + topology.FabricTierKey(0): "new-leaf", + topology.FabricTierKey(1): "new-spine", + "fabric.topograph.run/core": "legacy-core", + "workload.example/label": "keep", }, }, { - name: "Case 5: do not overwrite GPU clique when it is the configured accelerator label", - acceleratorLabel: topology.KeyNvidiaGPUClique, + name: "Case 5: omitted source label uses provider accelerator domain", node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", + "example.com/accelerator-domain": "unconfigured-source-domain", }, }, }, in: map[string]string{ - topology.KeyNvidiaGPUClique: "api-domain", - topology.FabricTierKey(0): "new-leaf", + topology.KeyTopologyXclrDomain: "provider-domain", + topology.KeyTopologyXclrSubDomain: "provider-sub-domain", }, out: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", - topology.FabricTierKey(0): "new-leaf", + "example.com/accelerator-domain": "unconfigured-source-domain", + topology.KeyTopologyXclrDomain: "provider-domain", + topology.KeyTopologyXclrSubDomain: "provider-sub-domain", }, }, { - name: "Case 6: custom accelerator label still protects GPU clique", + name: "Case 6: customized accelerator output without source label", acceleratorLabel: "custom.example/accelerator", - node: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", - }, - }, - }, + node: &corev1.Node{}, in: map[string]string{ "custom.example/accelerator": "api-domain", topology.FabricTierKey(0): "new-leaf", topology.FabricTierKey(1): "new-spine", }, out: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", - topology.FabricTierKey(0): "new-leaf", - topology.FabricTierKey(1): "new-spine", + "custom.example/accelerator": "api-domain", + topology.FabricTierKey(0): "new-leaf", + topology.FabricTierKey(1): "new-spine", }, }, { - name: "Case 7: apply accelerator label when GPU clique is absent", - node: &corev1.Node{}, + name: "Case 7: configured source missing uses provider accelerator domain", + acceleratorDomainSourceLabel: "example.com/accelerator-domain", + node: &corev1.Node{}, in: map[string]string{ topology.KeyTopologyXclrDomain: "api-domain", topology.KeyTopologyXclrSubDomain: "api-sub-domain", @@ -475,7 +472,12 @@ func TestMergeNodeLabels(t *testing.T) { if tc.acceleratorLabel != "" { keys = NewTopologyLabelKeys(nil, tc.acceleratorLabel) } - tc.node.Labels = mergeNodeLabels(tc.node.Labels, tc.in, keys) + tc.node.Labels = mergeNodeLabels( + tc.node.Labels, + tc.in, + keys, + tc.acceleratorDomainSourceLabel, + ) require.Equal(t, tc.out, tc.node.Labels) }) } diff --git a/pkg/engines/k8s/labeler.go b/pkg/engines/k8s/labeler.go index daef0809..50dec75d 100644 --- a/pkg/engines/k8s/labeler.go +++ b/pkg/engines/k8s/labeler.go @@ -10,10 +10,8 @@ import ( "fmt" "hash/fnv" "slices" - "strings" - - "k8s.io/apimachinery/pkg/util/validation" + internalk8s "github.com/NVIDIA/topograph/internal/k8s" "github.com/NVIDIA/topograph/pkg/topology" ) @@ -41,8 +39,8 @@ func NewTopologyLabelKeys(fabric []string, xclrDomain string) *TopologyLabelKeys func (keys *TopologyLabelKeys) Validate() error { seen := make(map[string]string) validate := func(location, key string) error { - if errs := validation.IsQualifiedName(key); len(errs) != 0 { - return fmt.Errorf("%s %q is not a valid Kubernetes label key: %s", location, key, strings.Join(errs, "; ")) + if err := internalk8s.ValidateLabelKey(location, key); err != nil { + return err } if previous, ok := seen[key]; ok { return fmt.Errorf("topology label key %q is configured for both %s and %s", key, previous, location) diff --git a/pkg/engines/nfd/engine.go b/pkg/engines/nfd/engine.go index 695a79c3..4defa760 100644 --- a/pkg/engines/nfd/engine.go +++ b/pkg/engines/nfd/engine.go @@ -45,6 +45,9 @@ type Params struct { NodeSelector map[string]string `mapstructure:"nodeSelector"` // Cleanup deletes stale Topograph-managed NFD objects. Defaults to true. Cleanup bool `mapstructure:"cleanup"` + // AcceleratorDomainSourceLabel optionally selects an existing Kubernetes + // Node label as the authoritative accelerator-domain source. + AcceleratorDomainSourceLabel string `mapstructure:"acceleratorDomainSourceLabel"` // derived fields nodeListOpt *metav1.ListOptions @@ -97,6 +100,11 @@ func getParameters(params engines.Config) (*Params, error) { if err := config.Decode(params, p); err != nil { return nil, err } + if p.AcceleratorDomainSourceLabel != "" { + if err := internalk8s.ValidateLabelKey("acceleratorDomainSourceLabel", p.AcceleratorDomainSourceLabel); err != nil { + return nil, err + } + } if len(p.NodeSelector) != 0 { p.nodeListOpt = &metav1.ListOptions{ LabelSelector: labels.Set(p.NodeSelector).String(), @@ -130,7 +138,11 @@ func (eng *NfdEngine) GenerateOutput(ctx context.Context, graph *topology.Graph, return nil, httperr.NewError(http.StatusBadGateway, err.Error()) } - nodeFeatures, nodeFeatureGroups, err := buildNFDObjects(nodeLabels, gpuCliqueValues(nodes)) + nodeFeatures, nodeFeatureGroups, err := buildNFDObjects( + nodeLabels, + acceleratorDomainSourceValues(nodes, eng.params.AcceleratorDomainSourceLabel), + eng.params.AcceleratorDomainSourceLabel, + ) if err != nil { return nil, httperr.NewError(http.StatusBadRequest, err.Error()) } @@ -152,14 +164,14 @@ func (eng *NfdEngine) GenerateOutput(ctx context.Context, graph *topology.Graph, return fmt.Appendf(nil, "OK nodeFeatures=%d nodeFeatureGroups=%d\n", len(nodeFeatures), len(nodeFeatureGroups)), nil } -func gpuCliqueValues(nodes *corev1.NodeList) map[string]string { +func acceleratorDomainSourceValues(nodes *corev1.NodeList, sourceLabel string) map[string]string { out := make(map[string]string) - if nodes == nil { + if nodes == nil || sourceLabel == "" { return out } for _, node := range nodes.Items { - if value := strings.TrimSpace(node.Labels[topology.KeyNvidiaGPUClique]); value != "" { + if value := strings.TrimSpace(node.Labels[sourceLabel]); value != "" { out[node.Name] = value } } diff --git a/pkg/engines/nfd/engine_test.go b/pkg/engines/nfd/engine_test.go index 26a365d7..2ba23e2e 100644 --- a/pkg/engines/nfd/engine_test.go +++ b/pkg/engines/nfd/engine_test.go @@ -82,6 +82,20 @@ func TestGetParametersDefaults(t *testing.T) { params, err := getParameters(nil) require.NoError(t, err) require.True(t, params.Cleanup) + require.Empty(t, params.AcceleratorDomainSourceLabel) +} + +func TestGetParametersAcceleratorDomainSourceLabel(t *testing.T) { + params, err := getParameters(map[string]any{ + "acceleratorDomainSourceLabel": "example.com/accelerator-domain", + }) + require.NoError(t, err) + require.Equal(t, "example.com/accelerator-domain", params.AcceleratorDomainSourceLabel) + + _, err = getParameters(map[string]any{ + "acceleratorDomainSourceLabel": "not a label", + }) + require.ErrorContains(t, err, `acceleratorDomainSourceLabel "not a label" is not a valid Kubernetes label key`) } func TestResolveComputeInstancesCachesNodesWhenInstancesAreSupplied(t *testing.T) { @@ -113,7 +127,7 @@ func TestGenerateOutputCreatesNodeFeaturesAndGroups(t *testing.T) { &corev1.Node{ObjectMeta: metav1.ObjectMeta{ Name: "node-b", Labels: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster.0", + "example.com/accelerator-domain": "cluster.0", }, }}, &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-c"}}, @@ -128,8 +142,11 @@ func TestGenerateOutputCreatesNodeFeaturesAndGroups(t *testing.T) { eng := &NfdEngine{ client: client, dynamicClient: dynamicClient, - params: &Params{Cleanup: true}, - namespace: testNFDNamespace, + params: &Params{ + Cleanup: true, + AcceleratorDomainSourceLabel: "example.com/accelerator-domain", + }, + namespace: testNFDNamespace, } out, httpErr := eng.GenerateOutput(context.Background(), testGraph(), nil) @@ -144,18 +161,18 @@ func TestGenerateOutputCreatesNodeFeaturesAndGroups(t *testing.T) { nodeA := findNodeFeature(t, features.Items, "node-a") require.Equal(t, map[string]string{nfdNodeName: "node-a"}, attributeElements(t, nodeA, nfdSystemName)) require.Equal(t, map[string]string{ - topologyTypeXclrDomain: "nvl-a", - topologyTypeXclrSubDomain: "nvl-a.rack-1", - topologyTypeFabric + "0": "leaf-1", - topologyTypeFabric + "1": "spine-1", + topologyTypeAcceleratorDomain: "nvl-a", + topologyTypeAcceleratorSubDomain: "nvl-a.rack-1", + topologyTypeFabric + "0": "leaf-1", + topologyTypeFabric + "1": "spine-1", }, attributeElements(t, nodeA, nfdFeatureSet)) nodeB := findNodeFeature(t, features.Items, "node-b") require.Equal(t, map[string]string{nfdNodeName: "node-b"}, attributeElements(t, nodeB, nfdSystemName)) require.Equal(t, map[string]string{ - topologyTypeXclrDomain: "cluster.0", - topologyTypeFabric + "0": "leaf-1", - topologyTypeFabric + "1": "spine-1", + topologyTypeAcceleratorDomain: "cluster.0", + topologyTypeFabric + "0": "leaf-1", + topologyTypeFabric + "1": "spine-1", }, attributeElements(t, nodeB, nfdFeatureSet)) groups, err := dynamicClient.Resource(nodeFeatureGroupGVR).Namespace(testNFDNamespace).List(context.Background(), metav1.ListOptions{}) @@ -165,12 +182,12 @@ func TestGenerateOutputCreatesNodeFeaturesAndGroups(t *testing.T) { leafGroup := findGroup(t, groups.Items, topologyTypeFabric+"0", "leaf-1") require.Equal(t, []interface{}{"leaf-1"}, groupRuleValues(t, leafGroup, topologyTypeFabric+"0")) - cliqueGroup := findGroup(t, groups.Items, topologyTypeXclrDomain, "cluster.0") - require.Equal(t, topology.KeyNvidiaGPUClique, cliqueGroup.GetAnnotations()[annotationTopologyLabelKey]) - require.Equal(t, []interface{}{"cluster.0"}, groupRuleValues(t, cliqueGroup, topologyTypeXclrDomain)) - subDomainGroup := findGroup(t, groups.Items, topologyTypeXclrSubDomain, "nvl-a.rack-1") + cliqueGroup := findGroup(t, groups.Items, topologyTypeAcceleratorDomain, "cluster.0") + require.Equal(t, "example.com/accelerator-domain", cliqueGroup.GetAnnotations()[annotationTopologyLabelKey]) + require.Equal(t, []interface{}{"cluster.0"}, groupRuleValues(t, cliqueGroup, topologyTypeAcceleratorDomain)) + subDomainGroup := findGroup(t, groups.Items, topologyTypeAcceleratorSubDomain, "nvl-a.rack-1") require.Equal(t, topology.KeyTopologyXclrSubDomain, subDomainGroup.GetAnnotations()[annotationTopologyLabelKey]) - require.Equal(t, []interface{}{"nvl-a.rack-1"}, groupRuleValues(t, subDomainGroup, topologyTypeXclrSubDomain)) + require.Equal(t, []interface{}{"nvl-a.rack-1"}, groupRuleValues(t, subDomainGroup, topologyTypeAcceleratorSubDomain)) } func TestGenerateOutputCleansStaleObjects(t *testing.T) { @@ -318,11 +335,11 @@ func TestBuildNFDObjectsRejectsInvalidNFDNodeNameLabelValue(t *testing.T) { }, } - _, _, err := buildNFDObjects(nodeLabels, nil) + _, _, err := buildNFDObjects(nodeLabels, nil, "") require.ErrorContains(t, err, "cannot be used as") } -func TestBuildNFDObjectsSuppressesSubDomainWhenGPUCliqueExists(t *testing.T) { +func TestBuildNFDObjectsUsesConfiguredAcceleratorDomainSource(t *testing.T) { nodeLabels := k8sengine.NodeLabelMap{ "node-a": { topology.KeyTopologyXclrDomain: "provider-domain-a", @@ -332,44 +349,85 @@ func TestBuildNFDObjectsSuppressesSubDomainWhenGPUCliqueExists(t *testing.T) { "node-b": { topology.KeyTopologyXclrDomain: "provider-domain-b", }, + "node-c": { + topology.KeyTopologyXclrDomain: "source-domain-a", + }, } - features, groups, err := buildNFDObjects(nodeLabels, map[string]string{"node-a": "gpu-clique-a"}) + features, groups, err := buildNFDObjects( + nodeLabels, + map[string]string{"node-a": "source-domain-a"}, + "example.com/accelerator-domain", + ) require.NoError(t, err) - require.Len(t, features, 2) + require.Len(t, features, 3) nodeAElements := attributeElements(t, *features[0], nfdFeatureSet) - require.Equal(t, "gpu-clique-a", nodeAElements[topologyTypeXclrDomain]) - require.Empty(t, nodeAElements[topologyTypeXclrSubDomain], "sub-domain must be suppressed when GPU clique is present") - require.Equal(t, "provider-domain-b", attributeElements(t, *features[1], nfdFeatureSet)[topologyTypeXclrDomain]) - require.Len(t, groups, 3, "xclr-domain×2 + fabric-tier-0×1; no xclr-sub-domain group when clique suppresses it") - xclrDomainValues := make(map[string]struct{}) - xclrSubDomainValues := make(map[string]struct{}) + require.Equal(t, "source-domain-a", nodeAElements[topologyTypeAcceleratorDomain]) + require.Empty(t, nodeAElements[topologyTypeAcceleratorSubDomain], "sub-domain must be suppressed when a source-label value replaces the provider domain") + require.Equal(t, "provider-domain-b", attributeElements(t, *features[1], nfdFeatureSet)[topologyTypeAcceleratorDomain]) + require.Equal(t, "source-domain-a", attributeElements(t, *features[2], nfdFeatureSet)[topologyTypeAcceleratorDomain]) + require.Len(t, groups, 3, "accelerator-domain×2 + fabric-tier-0×1; no accelerator-sub-domain group when the source label suppresses it") + acceleratorDomainValues := make(map[string]struct{}) + acceleratorSubDomainValues := make(map[string]struct{}) + acceleratorDomainSourceKey := "" for _, group := range groups { switch group.GetLabels()[labelGroupType] { - case topologyTypeXclrDomain: - xclrDomainValues[group.GetAnnotations()[annotationTopologyValue]] = struct{}{} - case topologyTypeXclrSubDomain: - xclrSubDomainValues[group.GetAnnotations()[annotationTopologyValue]] = struct{}{} + case topologyTypeAcceleratorDomain: + value := group.GetAnnotations()[annotationTopologyValue] + acceleratorDomainValues[value] = struct{}{} + if value == "source-domain-a" { + acceleratorDomainSourceKey = group.GetAnnotations()[annotationTopologyLabelKey] + } + case topologyTypeAcceleratorSubDomain: + acceleratorSubDomainValues[group.GetAnnotations()[annotationTopologyValue]] = struct{}{} } } require.Equal(t, map[string]struct{}{ - "gpu-clique-a": {}, + "source-domain-a": {}, "provider-domain-b": {}, - }, xclrDomainValues) - require.Empty(t, xclrSubDomainValues, "no xclr-sub-domain NodeFeatureGroup must be created for a clique node") + }, acceleratorDomainValues) + require.Equal(t, "example.com/accelerator-domain", acceleratorDomainSourceKey) + require.Empty(t, acceleratorSubDomainValues, "no accelerator-sub-domain NodeFeatureGroup must be created for a source-label node") +} + +func TestBuildNFDObjectsUsesProviderAcceleratorDomainWhenSourceIsOmitted(t *testing.T) { + nodeLabels := k8sengine.NodeLabelMap{ + "node-a": { + topology.KeyTopologyXclrDomain: "provider-domain-a", + topology.KeyTopologyXclrSubDomain: "provider-sub-domain-a", + }, + } + nodes := &corev1.NodeList{Items: []corev1.Node{{ObjectMeta: metav1.ObjectMeta{ + Name: "node-a", + Labels: map[string]string{ + "nvidia.com/gpu.clique": "legacy-clique-domain", + }, + }}}} + + features, _, err := buildNFDObjects( + nodeLabels, + acceleratorDomainSourceValues(nodes, ""), + "", + ) + + require.NoError(t, err) + require.Len(t, features, 1) + elements := attributeElements(t, *features[0], nfdFeatureSet) + require.Equal(t, "provider-domain-a", elements[topologyTypeAcceleratorDomain]) + require.Equal(t, "provider-sub-domain-a", elements[topologyTypeAcceleratorSubDomain]) } -func TestTopologyKindUsesFabricTierAndXclrLabels(t *testing.T) { +func TestTopologyKindUsesFabricTierAndAcceleratorLabels(t *testing.T) { kind, ok := topologyKind(topology.FabricTierKey(2)) require.True(t, ok) require.Equal(t, topologyTypeFabric+"2", kind) kind, ok = topologyKind(topology.KeyTopologyXclrDomain) require.True(t, ok) - require.Equal(t, topologyTypeXclrDomain, kind) + require.Equal(t, topologyTypeAcceleratorDomain, kind) kind, ok = topologyKind(topology.KeyTopologyXclrSubDomain) require.True(t, ok) - require.Equal(t, topologyTypeXclrSubDomain, kind) + require.Equal(t, topologyTypeAcceleratorSubDomain, kind) } func testGraph() *topology.Graph { diff --git a/pkg/engines/nfd/objects.go b/pkg/engines/nfd/objects.go index 5b1f54d9..53ebd2fb 100644 --- a/pkg/engines/nfd/objects.go +++ b/pkg/engines/nfd/objects.go @@ -39,11 +39,11 @@ const ( nfdSystemName = "system.name" nfdNodeName = "nodename" - nfdNodeFeatureKind = "NodeFeature" - nfdNodeFeatureGroupKind = "NodeFeatureGroup" - topologyTypeFabric = "fabric-tier-" - topologyTypeXclrDomain = "xclr-domain" - topologyTypeXclrSubDomain = "xclr-sub-domain" + nfdNodeFeatureKind = "NodeFeature" + nfdNodeFeatureGroupKind = "NodeFeatureGroup" + topologyTypeFabric = "fabric-tier-" + topologyTypeAcceleratorDomain = "accelerator-domain" + topologyTypeAcceleratorSubDomain = "accelerator-sub-domain" labelNFDNodeName = "nfd.node.kubernetes.io/node-name" labelManagedBy = "app.kubernetes.io/managed-by" @@ -74,7 +74,11 @@ var ( // buildNFDObjects converts node topology labels into per-node features and // groups for each distinct topology value. -func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[string]string) ([]*unstructured.Unstructured, []*unstructured.Unstructured, error) { +func buildNFDObjects( + nodeLabels k8sengine.NodeLabelMap, + acceleratorDomainSourceValues map[string]string, + acceleratorDomainSourceLabel string, +) ([]*unstructured.Unstructured, []*unstructured.Unstructured, error) { groupValues := make(map[string]map[string]string) nodeFeatures := make([]*unstructured.Unstructured, 0, len(nodeLabels)) @@ -85,14 +89,14 @@ func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[stri } labels := nodeLabels[nodeName] - gpuCliqueValue := strings.TrimSpace(gpuCliqueValues[nodeName]) + acceleratorDomainSourceValue := strings.TrimSpace(acceleratorDomainSourceValues[nodeName]) elements := make(map[string]string, len(labels)) for _, labelKey := range slices.Sorted(maps.Keys(labels)) { kind, ok := topologyKind(labelKey) if !ok { continue } - if (kind == topologyTypeXclrDomain || kind == topologyTypeXclrSubDomain) && gpuCliqueValue != "" { + if (kind == topologyTypeAcceleratorDomain || kind == topologyTypeAcceleratorSubDomain) && acceleratorDomainSourceValue != "" { continue } value := strings.TrimSpace(labels[labelKey]) @@ -104,15 +108,19 @@ func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[stri if _, ok := groupValues[kind]; !ok { groupValues[kind] = make(map[string]string) } + if kind == topologyTypeAcceleratorDomain && acceleratorDomainSourceLabel != "" && + groupValues[kind][value] == acceleratorDomainSourceLabel { + continue + } groupValues[kind][value] = labelKey } - if gpuCliqueValue != "" { - kind := topologyTypeXclrDomain - elements[kind] = gpuCliqueValue + if acceleratorDomainSourceValue != "" { + kind := topologyTypeAcceleratorDomain + elements[kind] = acceleratorDomainSourceValue if _, ok := groupValues[kind]; !ok { groupValues[kind] = make(map[string]string) } - groupValues[kind][gpuCliqueValue] = topology.KeyNvidiaGPUClique + groupValues[kind][acceleratorDomainSourceValue] = acceleratorDomainSourceLabel } if len(elements) == 0 { continue @@ -142,10 +150,10 @@ func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[stri func topologyKind(labelKey string) (string, bool) { if labelKey == topology.KeyTopologyXclrDomain { - return topologyTypeXclrDomain, true + return topologyTypeAcceleratorDomain, true } if labelKey == topology.KeyTopologyXclrSubDomain { - return topologyTypeXclrSubDomain, true + return topologyTypeAcceleratorSubDomain, true } if level, ok := strings.CutPrefix(labelKey, topology.KeyFabricTierPrefix); ok { if _, err := strconv.Atoi(level); err == nil { diff --git a/pkg/engines/slinky/engine.go b/pkg/engines/slinky/engine.go index aedab504..1a059371 100644 --- a/pkg/engines/slinky/engine.go +++ b/pkg/engines/slinky/engine.go @@ -91,9 +91,9 @@ type Params struct { ConfigPath string `mapstructure:"topologyConfigPath"` // UseDynamicNodes specifies whether to use dynamic nodes for reporting: true or false UseDynamicNodes bool `mapstructure:"useDynamicNodes" default:"false"` - // UseGPUCliqueLabel uses the GPU Operator's nvidia.com/gpu.clique node label - // as the block-domain source for topology/block output. - UseGPUCliqueLabel bool `mapstructure:"useGpuCliqueLabel"` + // AcceleratorDomainSourceLabel optionally selects an existing Kubernetes + // Node label as the authoritative block-domain source. + AcceleratorDomainSourceLabel string `mapstructure:"acceleratorDomainSourceLabel"` // ConfigUpdateMode specifies the mode for updating the slurm config: valid values {"none", "skeleton-only"} ConfigUpdateMode string `mapstructure:"configUpdateMode,omitempty"` // Topologies specifies per-partition topology configuration @@ -150,6 +150,11 @@ func getParameters(params engines.Config) (*Params, error) { if err := config.Decode(params, p); err != nil { return nil, err } + if p.AcceleratorDomainSourceLabel != "" { + if err := k8s.ValidateLabelKey("acceleratorDomainSourceLabel", p.AcceleratorDomainSourceLabel); err != nil { + return nil, err + } + } // Validate config update mode if len(p.ConfigUpdateMode) != 0 && p.ConfigUpdateMode != ConfigUpdateModeNone && p.ConfigUpdateMode != ConfigUpdateModeSkeletonOnly { return nil, fmt.Errorf("invalid configUpdateMode: %s, must be either %s, or %s", p.ConfigUpdateMode, ConfigUpdateModeNone, ConfigUpdateModeSkeletonOnly) @@ -196,7 +201,7 @@ func isEmptySelector(sel *metav1.LabelSelector) bool { } func (eng *SlinkyEngine) ResolveComputeInstances(ctx context.Context, instances []topology.ComputeInstances, _ any) ([]topology.ComputeInstances, *httperr.Error) { - if len(instances) != 0 && !eng.params.UseDynamicNodes && !eng.params.UseGPUCliqueLabel { + if len(instances) != 0 && !eng.params.UseDynamicNodes { return instances, nil } @@ -301,7 +306,7 @@ func getComputeInstances(nodes *corev1.NodeList, nodeMap map[string]string) ([]t return cis, nil } -func withGPUCliqueDomains(graph *topology.Graph, clusterNodes *clusterNodes) (*topology.Graph, *httperr.Error) { +func withLabelBackedDomains(graph *topology.Graph, clusterNodes *clusterNodes, sourceLabel string) (*topology.Graph, *httperr.Error) { // No nodes selected at all is a distinct failure from "nodes exist but none // matched": it usually points at a too-narrow engine nodeSelector. if len(clusterNodes.nodes.Items) == 0 { @@ -313,10 +318,10 @@ func withGPUCliqueDomains(graph *topology.Graph, clusterNodes *clusterNodes) (*t // Diagnostic counters to explain why no domains were built. The instance // annotation is written per-node by the node-data-broker DaemonSet, so a - // node with the clique label but no annotation points at a broker that has + // node with the source label but no annotation points at a broker that has // not (yet) annotated that specific node. totalNodes := len(clusterNodes.nodes.Items) - var noSlurmName, noCliqueLabel int + var noSlurmName, noSourceLabel int missingAnnotation := []string{} for _, node := range clusterNodes.nodes.Items { @@ -327,30 +332,30 @@ func withGPUCliqueDomains(graph *topology.Graph, clusterNodes *clusterNodes) (*t continue } - gpuClique := strings.TrimSpace(node.Labels[topology.KeyNvidiaGPUClique]) - if gpuClique == "" { - noCliqueLabel++ - klog.V(4).Infof("Skipping node %s (SLURM node %s): missing/empty %q label", node.Name, slurmName, topology.KeyNvidiaGPUClique) + acceleratorDomain := strings.TrimSpace(node.Labels[sourceLabel]) + if acceleratorDomain == "" { + noSourceLabel++ + klog.V(4).Infof("Skipping node %s (SLURM node %s): missing/empty %q label", node.Name, slurmName, sourceLabel) continue } - instance, ok := node.Annotations[topology.KeyNodeInstance] - if !ok { + instance := strings.TrimSpace(node.Annotations[topology.KeyNodeInstance]) + if instance == "" { missingAnnotation = append(missingAnnotation, node.Name) - klog.Warningf("node %s (SLURM node %s) has label %s=%q but is missing annotation %q (expected from node-data-broker on that node)", - node.Name, slurmName, topology.KeyNvidiaGPUClique, gpuClique, topology.KeyNodeInstance) + klog.Warningf("node %s (SLURM node %s) has label %s=%q but has a missing/empty annotation %q (expected from node-data-broker on that node)", + node.Name, slurmName, sourceLabel, acceleratorDomain, topology.KeyNodeInstance) continue } - domains.AddHost(gpuClique, instance, slurmName) + domains.AddHost(acceleratorDomain, instance, slurmName) } if len(domains) == 0 { return nil, httperr.NewError(http.StatusBadGateway, - fmt.Sprintf("useGpuCliqueLabel=true but no matching nodes found; check label %q and annotation %q. "+ - "Scanned %d node(s): %d without a SLURM node mapping (no Ready slurmd pod), %d missing the %q label, %d with the label but missing the %q annotation%s", - topology.KeyNvidiaGPUClique, topology.KeyNodeInstance, - totalNodes, noSlurmName, noCliqueLabel, topology.KeyNvidiaGPUClique, + fmt.Sprintf("acceleratorDomainSourceLabel=%q produced no usable label-backed domains; check label %q and annotation %q. "+ + "Scanned %d node(s): %d without a SLURM node mapping (no Ready slurmd pod), %d missing the %q label, %d with the label but a missing/empty %q annotation%s", + sourceLabel, sourceLabel, topology.KeyNodeInstance, + totalNodes, noSlurmName, noSourceLabel, sourceLabel, len(missingAnnotation), topology.KeyNodeInstance, formatMissingAnnotationNodes(missingAnnotation))) } @@ -365,8 +370,8 @@ func withGPUCliqueDomains(graph *topology.Graph, clusterNodes *clusterNodes) (*t return graph, nil } -// formatMissingAnnotationNodes renders the node names that carry the GPU clique -// label but lack the node-data-broker-written instance annotation. The list is +// formatMissingAnnotationNodes renders the node names that carry the configured +// source label but lack the node-data-broker-written instance annotation. The list is // capped so the error message stays bounded on large clusters. func formatMissingAnnotationNodes(nodes []string) string { if len(nodes) == 0 { @@ -447,12 +452,12 @@ func (eng *SlinkyEngine) GenerateOutput(ctx context.Context, graph *topology.Gra return clusterNodeData, httpErr } - if p.UseGPUCliqueLabel && usesBlockTopology(cfg) { + if p.AcceleratorDomainSourceLabel != "" && usesBlockTopology(cfg) { clusterNodeData, httpErr := loadClusterNodes() if httpErr != nil { return nil, httpErr } - graph, httpErr = withGPUCliqueDomains(graph, clusterNodeData) + graph, httpErr = withLabelBackedDomains(graph, clusterNodeData, p.AcceleratorDomainSourceLabel) if httpErr != nil { return nil, httpErr } diff --git a/pkg/engines/slinky/engine_test.go b/pkg/engines/slinky/engine_test.go index ceac47db..5cb1d85a 100644 --- a/pkg/engines/slinky/engine_test.go +++ b/pkg/engines/slinky/engine_test.go @@ -39,6 +39,8 @@ import ( "github.com/NVIDIA/topograph/pkg/translate" ) +const testAcceleratorDomainSourceLabel = "example.com/accelerator-domain" + func TestGetParameters(t *testing.T) { podSelector := map[string]any{ "matchLabels": map[string]string{"key": "value"}, @@ -223,23 +225,34 @@ func TestGetParameters(t *testing.T) { }, }, { - name: "Case 10: use GPU clique label", + name: "Case 10: accelerator domain source label", params: map[string]any{ - topology.KeyNamespace: "namespace", - topology.KeyPodSelector: podSelector, - topology.KeyTopoConfigPath: "path", - topology.KeyTopoConfigmapName: "name", - "useGpuCliqueLabel": true, + topology.KeyNamespace: "namespace", + topology.KeyPodSelector: podSelector, + topology.KeyTopoConfigPath: "path", + topology.KeyTopoConfigmapName: "name", + "acceleratorDomainSourceLabel": testAcceleratorDomainSourceLabel, }, ret: &Params{ - Namespace: "namespace", - PodSelector: labelSelector, - ConfigPath: "path", - ConfigMapName: "name", - UseGPUCliqueLabel: true, - podListOpt: &metav1.ListOptions{LabelSelector: "key=value"}, + Namespace: "namespace", + PodSelector: labelSelector, + ConfigPath: "path", + ConfigMapName: "name", + AcceleratorDomainSourceLabel: testAcceleratorDomainSourceLabel, + podListOpt: &metav1.ListOptions{LabelSelector: "key=value"}, }, }, + { + name: "Case 11: reject invalid accelerator domain source label", + params: map[string]any{ + topology.KeyNamespace: "namespace", + topology.KeyPodSelector: podSelector, + topology.KeyTopoConfigPath: "path", + topology.KeyTopoConfigmapName: "name", + "acceleratorDomainSourceLabel": "not a label", + }, + err: `acceleratorDomainSourceLabel "not a label" is not a valid Kubernetes label key`, + }, } for _, tc := range testCases { @@ -364,13 +377,17 @@ func TestResolveComputeInstancesCachesClusterNodesWhenOutputNeedsThem(t *testing require.Empty(t, client.Actions()) } -func TestResolveComputeInstancesDoesNotLoadUnusedClusterNodes(t *testing.T) { +func TestResolveComputeInstancesDoesNotLoadNodesForSourceLabelAlone(t *testing.T) { client := fake.NewSimpleClientset() + client.PrependReactor("list", "nodes", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("unexpected Node list") + }) eng := &SlinkyEngine{ client: client, params: &Params{ - nodeListOpt: &metav1.ListOptions{}, - podListOpt: &metav1.ListOptions{}, + AcceleratorDomainSourceLabel: testAcceleratorDomainSourceLabel, + nodeListOpt: &metav1.ListOptions{}, + podListOpt: &metav1.ListOptions{}, }, } instances := []topology.ComputeInstances{{ @@ -386,7 +403,7 @@ func TestResolveComputeInstancesDoesNotLoadUnusedClusterNodes(t *testing.T) { require.Empty(t, client.Actions()) } -func TestWithGPUCliqueDomains(t *testing.T) { +func TestWithLabelBackedDomains(t *testing.T) { ctx := context.Background() client := fake.NewSimpleClientset() @@ -394,28 +411,28 @@ func TestWithGPUCliqueDomains(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "k8s-node-0", - Labels: map[string]string{topology.KeyNvidiaGPUClique: "clique-a"}, + Labels: map[string]string{testAcceleratorDomainSourceLabel: "clique-a"}, Annotations: map[string]string{topology.KeyNodeInstance: "instance-0"}, }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "k8s-node-1", - Labels: map[string]string{topology.KeyNvidiaGPUClique: " clique-b "}, + Labels: map[string]string{testAcceleratorDomainSourceLabel: " clique-b "}, Annotations: map[string]string{topology.KeyNodeInstance: "instance-1"}, }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "k8s-node-no-instance", - Labels: map[string]string{topology.KeyNvidiaGPUClique: "clique-c"}, + Labels: map[string]string{testAcceleratorDomainSourceLabel: "clique-c"}, Annotations: map[string]string{}, }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "k8s-node-no-pod", - Labels: map[string]string{topology.KeyNvidiaGPUClique: "clique-d"}, + Labels: map[string]string{testAcceleratorDomainSourceLabel: "clique-d"}, Annotations: map[string]string{topology.KeyNodeInstance: "instance-3"}, }, }, @@ -450,7 +467,7 @@ func TestWithGPUCliqueDomains(t *testing.T) { clusterNodes, httpErr := eng.getClusterNodes(ctx) require.Nil(t, httpErr) - got, httpErr := withGPUCliqueDomains(graph, clusterNodes) + got, httpErr := withLabelBackedDomains(graph, clusterNodes, testAcceleratorDomainSourceLabel) require.Nil(t, httpErr) require.NotSame(t, graph, got) require.Same(t, graph.Tiers, got.Tiers) @@ -462,7 +479,7 @@ func TestWithGPUCliqueDomains(t *testing.T) { require.Equal(t, existingDomains, graph.Domains) } -func TestWithGPUCliqueDomainsNoMatchingNodes(t *testing.T) { +func TestWithLabelBackedDomainsNoMatchingNodes(t *testing.T) { ctx := context.Background() client := fake.NewSimpleClientset() @@ -487,15 +504,15 @@ func TestWithGPUCliqueDomainsNoMatchingNodes(t *testing.T) { clusterNodes, httpErr := eng.getClusterNodes(ctx) require.Nil(t, httpErr) - got, httpErr := withGPUCliqueDomains(&topology.Graph{}, clusterNodes) + got, httpErr := withLabelBackedDomains(&topology.Graph{}, clusterNodes, testAcceleratorDomainSourceLabel) require.Nil(t, got) - require.ErrorContains(t, httpErr, "useGpuCliqueLabel=true but no matching nodes found") - // The node maps to a SLURM node but has no clique label. + require.ErrorContains(t, httpErr, `acceleratorDomainSourceLabel="example.com/accelerator-domain" produced no usable label-backed domains`) + // The node maps to a SLURM node but has no configured source label. require.ErrorContains(t, httpErr, "Scanned 1 node(s)") - require.ErrorContains(t, httpErr, fmt.Sprintf("1 missing the %q label", topology.KeyNvidiaGPUClique)) + require.ErrorContains(t, httpErr, fmt.Sprintf("1 missing the %q label", testAcceleratorDomainSourceLabel)) } -func TestWithGPUCliqueDomainsNoSelectedNodes(t *testing.T) { +func TestWithLabelBackedDomainsNoSelectedNodes(t *testing.T) { // No Kubernetes nodes selected at all (e.g. a too-narrow nodeSelector) must // produce a distinct, actionable error rather than the generic no-match one. clusterNodes := &clusterNodes{ @@ -503,28 +520,38 @@ func TestWithGPUCliqueDomainsNoSelectedNodes(t *testing.T) { nodeMap: map[string]string{}, } - got, httpErr := withGPUCliqueDomains(&topology.Graph{}, clusterNodes) + got, httpErr := withLabelBackedDomains(&topology.Graph{}, clusterNodes, testAcceleratorDomainSourceLabel) require.Nil(t, got) require.NotNil(t, httpErr) require.ErrorContains(t, httpErr, "no selected Kubernetes nodes found; check engine nodeSelector") } -func TestWithGPUCliqueDomainsMissingBrokerAnnotation(t *testing.T) { +func TestWithLabelBackedDomainsMissingBrokerAnnotation(t *testing.T) { ctx := context.Background() client := fake.NewSimpleClientset() - // Node has the GPU clique label but is missing the node-data-broker-written - // instance annotation - the exact scenario that produces the error in the field. - _, err := client.CoreV1().Nodes().Create(ctx, &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "k8s-node-0", - Labels: map[string]string{topology.KeyNvidiaGPUClique: "clique-a"}, - }, - }, metav1.CreateOptions{}) - require.NoError(t, err) + // Each Node has the configured source label but an unusable + // node-data-broker-written instance annotation. + for index, annotations := range []map[string]string{ + nil, + {topology.KeyNodeInstance: ""}, + {topology.KeyNodeInstance: " \t "}, + } { + nodeName := fmt.Sprintf("k8s-node-%d", index) + _, err := client.CoreV1().Nodes().Create(ctx, &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Labels: map[string]string{testAcceleratorDomainSourceLabel: "clique-a"}, + Annotations: annotations, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) - _, err = client.CoreV1().Pods("test-ns").Create(ctx, makeReadySlurmdPod("pod-0", "k8s-node-0", "slurm-0"), metav1.CreateOptions{}) - require.NoError(t, err) + _, err = client.CoreV1().Pods("test-ns").Create(ctx, + makeReadySlurmdPod(fmt.Sprintf("pod-%d", index), nodeName, fmt.Sprintf("slurm-%d", index)), + metav1.CreateOptions{}) + require.NoError(t, err) + } eng := &SlinkyEngine{ client: client, @@ -536,13 +563,13 @@ func TestWithGPUCliqueDomainsMissingBrokerAnnotation(t *testing.T) { clusterNodes, httpErr := eng.getClusterNodes(ctx) require.Nil(t, httpErr) - got, httpErr := withGPUCliqueDomains(&topology.Graph{}, clusterNodes) + got, httpErr := withLabelBackedDomains(&topology.Graph{}, clusterNodes, testAcceleratorDomainSourceLabel) require.Nil(t, got) - require.ErrorContains(t, httpErr, fmt.Sprintf("1 with the label but missing the %q annotation", topology.KeyNodeInstance)) - require.ErrorContains(t, httpErr, "nodes missing annotation: k8s-node-0") + require.ErrorContains(t, httpErr, fmt.Sprintf("3 with the label but a missing/empty %q annotation", topology.KeyNodeInstance)) + require.ErrorContains(t, httpErr, "nodes missing annotation: k8s-node-0, k8s-node-1, k8s-node-2") } -func TestGenerateOutputUsesGPUCliqueDomains(t *testing.T) { +func TestGenerateOutputUsesConfiguredAcceleratorDomainSource(t *testing.T) { ctx := context.Background() client := fake.NewSimpleClientset() @@ -550,14 +577,14 @@ func TestGenerateOutputUsesGPUCliqueDomains(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{ Name: "k8s-node-0", - Labels: map[string]string{topology.KeyNvidiaGPUClique: "clique-a"}, + Labels: map[string]string{testAcceleratorDomainSourceLabel: "clique-a"}, Annotations: map[string]string{topology.KeyNodeInstance: "instance-0"}, }, }, { ObjectMeta: metav1.ObjectMeta{ Name: "k8s-node-1", - Labels: map[string]string{topology.KeyNvidiaGPUClique: "clique-b"}, + Labels: map[string]string{testAcceleratorDomainSourceLabel: "clique-b"}, Annotations: map[string]string{topology.KeyNodeInstance: "instance-1"}, }, }, @@ -575,8 +602,18 @@ func TestGenerateOutputUsesGPUCliqueDomains(t *testing.T) { } providerDomains := topology.NewDomainMap() - providerDomains.AddHost("provider-domain", "instance-0", "alpha") - providerDomains.AddHost("provider-domain", "instance-1", "beta") + providerDomains.AddHostInfo(&topology.HostInfo{ + Domain: "provider-domain", + SubDomain: "provider-sub-domain-a", + InstanceID: "instance-0", + HostName: "alpha", + }) + providerDomains.AddHostInfo(&topology.HostInfo{ + Domain: "provider-domain", + SubDomain: "provider-sub-domain-b", + InstanceID: "instance-1", + HostName: "beta", + }) eng := &SlinkyEngine{ client: client, @@ -585,11 +622,11 @@ func TestGenerateOutputUsesGPUCliqueDomains(t *testing.T) { Plugin: topology.TopologyBlock, BlockSizes: []int{1}, }, - Namespace: "test-ns", - ConfigMapName: "slurm-config", - ConfigPath: "topology.conf", - UseGPUCliqueLabel: true, - podListOpt: &metav1.ListOptions{LabelSelector: "app=slinky"}, + Namespace: "test-ns", + ConfigMapName: "slurm-config", + ConfigPath: "topology.conf", + AcceleratorDomainSourceLabel: testAcceleratorDomainSourceLabel, + podListOpt: &metav1.ListOptions{LabelSelector: "app=slinky"}, }, } @@ -607,6 +644,105 @@ BlockSizes=1 `, cm.Data["topology.conf"]) } +func TestGenerateOutputUsesProviderDomainsWhenSourceIsOmitted(t *testing.T) { + ctx := context.Background() + client := fake.NewSimpleClientset() + for _, resource := range []string{"nodes", "pods"} { + client.PrependReactor("list", resource, func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("unexpected Kubernetes list") + }) + } + providerDomains := topology.NewDomainMap() + providerDomains.AddHost("provider-domain-a", "instance-0", "alpha") + providerDomains.AddHost("provider-domain-b", "instance-1", "beta") + + eng := &SlinkyEngine{ + client: client, + params: &Params{ + BaseParams: slurm.BaseParams{ + Plugin: topology.TopologyBlock, + BlockSizes: []int{1}, + }, + Namespace: "test-ns", + ConfigMapName: "slurm-config", + ConfigPath: "topology.conf", + }, + } + + result, httpErr := eng.GenerateOutput(ctx, &topology.Graph{Domains: providerDomains}, nil) + require.Nil(t, httpErr) + require.Equal(t, []byte("OK\n"), result) + + cm, err := client.CoreV1().ConfigMaps("test-ns").Get(ctx, "slurm-config", metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, `# block001=provider-domain-a +BlockName=block001 Nodes=alpha +# block002=provider-domain-b +BlockName=block002 Nodes=beta +BlockSizes=1 +`, cm.Data["topology.conf"]) + require.Zero(t, countClientActions(client.Actions(), "list", "nodes")) + require.Zero(t, countClientActions(client.Actions(), "list", "pods")) +} + +func TestGenerateOutputDoesNotLoadNodesForNonBlockTopology(t *testing.T) { + testCases := []struct { + name string + baseParams slurm.BaseParams + topologies map[string]*Topology + }{ + { + name: "tree", + baseParams: slurm.BaseParams{Plugin: topology.TopologyTree}, + }, + { + name: "flat", + topologies: map[string]*Topology{ + "default": { + Topology: slurm.Topology{ + Plugin: topology.TopologyFlat, + Default: true, + }, + }, + }, + }, + } + + model, err := models.NewModelFromFile("small-tree.yaml") + require.NoError(t, err) + instanceToNode := make(map[string]string, len(model.Nodes)) + for hostName := range model.Nodes { + instanceToNode[fmt.Sprintf("i-%s", hostName)] = hostName + } + graph, _ := model.ToGraph([]topology.ComputeInstances{{Instances: instanceToNode}}) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := fake.NewSimpleClientset() + for _, resource := range []string{"nodes", "pods"} { + client.PrependReactor("list", resource, func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("unexpected Kubernetes list") + }) + } + eng := &SlinkyEngine{ + client: client, + params: &Params{ + BaseParams: tc.baseParams, + Topologies: tc.topologies, + AcceleratorDomainSourceLabel: testAcceleratorDomainSourceLabel, + ConfigUpdateMode: ConfigUpdateModeNone, + }, + } + + result, httpErr := eng.GenerateOutput(context.Background(), graph, nil) + + require.Nil(t, httpErr) + require.Equal(t, []byte("OK\n"), result) + require.Empty(t, client.Actions()) + }) + } +} + func TestUsesBlockTopology(t *testing.T) { require.False(t, usesBlockTopology(nil)) require.False(t, usesBlockTopology(&translate.Config{Plugin: topology.TopologyTree})) diff --git a/pkg/topology/graph.go b/pkg/topology/graph.go index 9db53b57..827ea363 100644 --- a/pkg/topology/graph.go +++ b/pkg/topology/graph.go @@ -279,7 +279,7 @@ func (inst *InstanceTopology) toInstance(trimTiers int) Instance { instance.ID = inst.InstanceID } instance.NetworkLayers = inst.networkLayers(trimTiers) - if instance.AcceleratorID() == "" && inst.XclrDomainID != "" { + if instance.Labels[KeyTopologyXclrDomain] == "" && inst.XclrDomainID != "" { if instance.Labels == nil { instance.Labels = make(map[string]string) } diff --git a/pkg/topology/graph_test.go b/pkg/topology/graph_test.go index 180b458e..3d1ee48a 100644 --- a/pkg/topology/graph_test.go +++ b/pkg/topology/graph_test.go @@ -234,7 +234,7 @@ func TestToInstanceOmitsXclrSubDomainWithoutDomainLabel(t *testing.T) { require.NotContains(t, instance.Labels, KeyTopologyXclrSubDomain) } -func TestToInstanceOmitsXclrSubDomainWhenGPUCliqueSupersedesDomain(t *testing.T) { +func TestToInstanceIgnoresUnrelatedGPUCliqueLabel(t *testing.T) { inst := &InstanceTopology{ InstanceID: "i-001", XclrDomainID: "discovered-domain", @@ -248,8 +248,9 @@ func TestToInstanceOmitsXclrSubDomainWhenGPUCliqueSupersedesDomain(t *testing.T) instance := inst.toInstance(0) - require.NotContains(t, instance.Labels, KeyTopologyXclrDomain) - require.NotContains(t, instance.Labels, KeyTopologyXclrSubDomain) + require.Equal(t, "gpu-clique", instance.Labels[KeyNvidiaGPUClique]) + require.Equal(t, "discovered-domain", instance.Labels[KeyTopologyXclrDomain]) + require.Equal(t, "discovered-sub-domain", instance.Labels[KeyTopologyXclrSubDomain]) } func TestTrimTiers(t *testing.T) { diff --git a/pkg/topology/instances.go b/pkg/topology/instances.go index 00b9875b..7dd47808 100644 --- a/pkg/topology/instances.go +++ b/pkg/topology/instances.go @@ -23,17 +23,6 @@ type Instance struct { NetLayers []string `yaml:"-" json:"-"` } -func (inst Instance) AcceleratorID() string { - return AcceleratorID(inst.Labels) -} - -func AcceleratorID(labels map[string]string) string { - if accelerator := labels[KeyTopologyXclrDomain]; accelerator != "" { - return accelerator - } - return labels[KeyNvidiaGPUClique] -} - func FabricTierKey(tier int) string { return KeyFabricTierPrefix + strconv.Itoa(tier) } diff --git a/tests/ci/values.slinky-dra-block.yaml b/tests/ci/values.slinky-dra-block.yaml index 27d78491..e49593eb 100644 --- a/tests/ci/values.slinky-dra-block.yaml +++ b/tests/ci/values.slinky-dra-block.yaml @@ -20,7 +20,6 @@ engine: blockName: nodeNameRegexp: '^srv([0-9]{2})' format: 'rack${1}' - useGpuCliqueLabel: true topologyConfigPath: topology.conf topologyConfigmapName: slurm-config-extra useDynamicNodes: true