diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 20eb9bbd..612b44f4 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -55,8 +55,8 @@ These structures propagate across every provider and engine. Changing them in a | Surface | Why it's load-bearing | |---|---| | `pkg/topology/` — `Graph`, the `Vertex` tree, and topology constants | Every provider returns it; every engine consumes it. A shape change ripples to all of them. | -| Helm `provider.name` / `engine.name` / `topologyNodeLabels` | External contract for operators deploying Topograph. | -| The four default label keys `network.topology.nvidia.com/{accelerator,leaf,spine,core}` | Consumed by downstream projects (KAI Scheduler, NVSentinel, Kueue). | +| Helm `provider.name` / `engine.name` | External contract for operators deploying Topograph. | +| The variable fabric labels `network.topology.nvidia.com/tier-N` and single accelerator label `network.topology.nvidia.com/accelerator` | Consumed by downstream projects (KAI Scheduler, NVSentinel, Kueue); fabric tier 0 is closest to the node. | ## 2. Setup and Installation @@ -138,7 +138,7 @@ type Provider interface { } ``` -A provider returns a `*topology.Graph` of the discovered topology. `Tiers` is the root of the switch hierarchy; `Domains` is a `topology.DomainMap` mapping accelerator/block domains to hosts, with each finalized domain carrying the enumerated ID used by block-topology output. Leaf vertices are compute nodes; interior tier vertices are switches. Return `*httperr.Error` so the API server can propagate the correct HTTP status code — plain `error` is not acceptable at this boundary. +A provider returns a `*topology.Graph` of the discovered topology. Providers using `ClusterTopology` populate `InstanceTopology.FabricTiers` closest-first and the optional single `InstanceTopology.AcceleratorID`, then call `ToGraph`; the fabric path has no fixed depth. `Graph.Tiers` is the fabric hierarchy, and `Graph.Domains` is the `topology/block` source. Leaf vertices are compute nodes; interior tier vertices are switches. Return `*httperr.Error` so the API server can propagate the correct HTTP status code — plain `error` is not acceptable at this boundary. ### Adding a new provider @@ -169,7 +169,7 @@ Engines are much rarer (four exist: slurm, k8s, nfd, slinky). Follow the same re ### Label and annotation reference -Label keys written by the Kubernetes and Slinky engines are documented in `docs/reference/node-labels.md`. Do not invent new keys in provider or engine code — values flow through the canonical graph; keys are configured via Helm `topologyNodeLabels`. +Label keys written by the Kubernetes engine are documented in `docs/reference/node-labels.md`. Do not invent new keys in provider code — values flow through the canonical graph. Optional custom keys are configured through the k8s engine's closest-first `fabricLabels` array and singular `acceleratorLabel`; when `fabricLabels` is provided, only explicitly listed fabric tiers are labeled. ## 5. Pull Request Guidelines diff --git a/AGENTS.md b/AGENTS.md index fd97cffa..6c71adf7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,8 +55,8 @@ These structures propagate across every provider and engine. Changing them in a | Surface | Why it's load-bearing | |---|---| | `pkg/topology/` — `Graph`, the `Vertex` tree, and topology constants | Every provider returns it; every engine consumes it. A shape change ripples to all of them. | -| Helm `provider.name` / `engine.name` / `topologyNodeLabels` | External contract for operators deploying Topograph. | -| The four default label keys `network.topology.nvidia.com/{accelerator,leaf,spine,core}` | Consumed by downstream projects (KAI Scheduler, NVSentinel, Kueue). | +| Helm `provider.name` / `engine.name` | External contract for operators deploying Topograph. | +| The variable fabric labels `network.topology.nvidia.com/tier-N` and single accelerator label `network.topology.nvidia.com/accelerator` | Consumed by downstream projects (KAI Scheduler, NVSentinel, Kueue); fabric tier 0 is closest to the node. | ## 2. Setup and Installation @@ -138,7 +138,7 @@ type Provider interface { } ``` -A provider returns a `*topology.Graph` of the discovered topology. `Tiers` is the root of the switch hierarchy; `Domains` is a `topology.DomainMap` mapping accelerator/block domains to hosts, with each finalized domain carrying the enumerated ID used by block-topology output. Leaf vertices are compute nodes; interior tier vertices are switches. Return `*httperr.Error` so the API server can propagate the correct HTTP status code — plain `error` is not acceptable at this boundary. +A provider returns a `*topology.Graph` of the discovered topology. Providers using `ClusterTopology` populate `InstanceTopology.FabricTiers` closest-first and the optional single `InstanceTopology.AcceleratorID`, then call `ToGraph`; the fabric path has no fixed depth. `Graph.Tiers` is the fabric hierarchy, and `Graph.Domains` is the `topology/block` source. Leaf vertices are compute nodes; interior tier vertices are switches. Return `*httperr.Error` so the API server can propagate the correct HTTP status code — plain `error` is not acceptable at this boundary. ### Adding a new provider @@ -169,7 +169,7 @@ Engines are much rarer (four exist: slurm, k8s, nfd, slinky). Follow the same re ### Label and annotation reference -Label keys written by the Kubernetes and Slinky engines are documented in `docs/reference/node-labels.md`. Do not invent new keys in provider or engine code — values flow through the canonical graph; keys are configured via Helm `topologyNodeLabels`. +Label keys written by the Kubernetes engine are documented in `docs/reference/node-labels.md`. Do not invent new keys in provider code — values flow through the canonical graph. Optional custom keys are configured through the k8s engine's closest-first `fabricLabels` array and singular `acceleratorLabel`; when `fabricLabels` is provided, only explicitly listed fabric tiers are labeled. ## 5. Pull Request Guidelines diff --git a/CHANGELOG.md b/CHANGELOG.md index 25fb4a9b..714df703 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- **BREAKING:** Fabric topology now uses variable, closest-first tiers labeled `network.topology.nvidia.com/tier-N`; `InstanceTopology.FabricTiers` and graph conversion support arbitrary fabric depth. Accelerator topology remains a single `AcceleratorID`/`Graph.Domains` dimension labeled `network.topology.nvidia.com/accelerator`. The fixed `leaf`, `spine`, and `core` keys and process-wide Helm/CLI label overrides are replaced by optional `fabricLabels` and `acceleratorLabel` parameters on the `k8s` engine. - Simulation model node names are now treated as hostnames; the model-backed test provider generates their instance IDs with an `i-` prefix. - The node-observer and node-data-broker are now rendered directly by the main Topograph Helm chart instead of local subcharts. Their existing `node-observer.*` and `node-data-broker.*` values paths are unchanged. - **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). diff --git a/charts/topograph/templates/NOTES.txt b/charts/topograph/templates/NOTES.txt index 1110ca98..dc032304 100644 --- a/charts/topograph/templates/NOTES.txt +++ b/charts/topograph/templates/NOTES.txt @@ -47,6 +47,7 @@ NOTE: The NFD engine writes NodeFeature and NodeFeatureGroup resources in {{ .Values.nfdNamespace }}. This must be the namespace where NFD master runs. {{- end }} + {{- if not .Values.serviceAccount.create }} 2. Topograph is configured to use the existing ServiceAccount "{{ include "topograph.serviceAccountName" . }}". diff --git a/charts/topograph/templates/deployment.yaml b/charts/topograph/templates/deployment.yaml index 3a1bd9c9..f3e93018 100644 --- a/charts/topograph/templates/deployment.yaml +++ b/charts/topograph/templates/deployment.yaml @@ -50,9 +50,6 @@ spec: - /usr/local/bin/topograph args: - -v={{ .Values.verbosity }} - {{- range $key, $value := .Values.topologyNodeLabels }} - - -k8s-topology-key-{{ $key }}={{ $value }} - {{- end }} {{- if or .Values.nodeDataBroker.enabled $hasGcpCredentials $isNFDEngine .Values.env }} env: {{- if .Values.nodeDataBroker.enabled }} diff --git a/charts/topograph/tests/deployment_test.yaml b/charts/topograph/tests/deployment_test.yaml index 63fe01bd..34cfd241 100644 --- a/charts/topograph/tests/deployment_test.yaml +++ b/charts/topograph/tests/deployment_test.yaml @@ -272,20 +272,14 @@ tests: name: NODE_DATA_BROKER_NAMESPACE template: templates/deployment.yaml - - it: passes the verbosity flag and topology key args + - it: passes the verbosity flag set: verbosity: 5 - topologyNodeLabels: - accelerator: network.topology.nvidia.com/accelerator asserts: - contains: path: spec.template.spec.containers[0].args content: -v=5 template: templates/deployment.yaml - - contains: - path: spec.template.spec.containers[0].args - content: -k8s-topology-key-accelerator=network.topology.nvidia.com/accelerator - template: templates/deployment.yaml - it: renders user-supplied environment variables, initContainers, and lifecycle set: diff --git a/charts/topograph/values.schema.json b/charts/topograph/values.schema.json index 0e76e417..73f16058 100644 --- a/charts/topograph/values.schema.json +++ b/charts/topograph/values.schema.json @@ -50,7 +50,7 @@ }, "params": { "type": "object", - "description": "Engine-specific parameters. 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. 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." } }, "required": ["name"] @@ -119,7 +119,6 @@ "initContainers": { "type": "array" }, "lifecycle": { "type": "object" }, "config": { "type": "object" }, - "topologyNodeLabels": { "type": "object" }, "podAnnotations": { "type": "object" }, "podLabels": { "type": "object" }, "podSecurityContext": { "type": "object" }, diff --git a/charts/topograph/values.yaml b/charts/topograph/values.yaml index d2cbeeec..55e72034 100644 --- a/charts/topograph/values.yaml +++ b/charts/topograph/values.yaml @@ -67,12 +67,6 @@ config: # Optional secret with API credentials # credentialsSecret: -#topologyNodeLabels: -# accelerator: network.topology.nvidia.com/accelerator -# leaf: network.topology.nvidia.com/leaf -# spine: network.topology.nvidia.com/spine -# core: network.topology.nvidia.com/core - podAnnotations: {} podLabels: {} diff --git a/cmd/topograph/main.go b/cmd/topograph/main.go index 29f5bd7f..10ac114a 100644 --- a/cmd/topograph/main.go +++ b/cmd/topograph/main.go @@ -28,19 +28,13 @@ import ( "github.com/NVIDIA/topograph/internal/version" "github.com/NVIDIA/topograph/pkg/config" - "github.com/NVIDIA/topograph/pkg/engines/k8s" "github.com/NVIDIA/topograph/pkg/server" ) func main() { var cfg string - var labelAccelerator, labelLeaf, labelSpine, labelCore string var ver bool flag.StringVar(&cfg, "c", "/etc/topograph/topograph-config.yaml", "config file") - flag.StringVar(&labelAccelerator, "k8s-topology-key-accelerator", k8s.DefaultLabelAccelerator, "K8s node label for accelerated network type") - flag.StringVar(&labelLeaf, "k8s-topology-key-leaf", k8s.DefaultLabelLeaf, "K8s node label for the cluster's lower network tier") - flag.StringVar(&labelSpine, "k8s-topology-key-spine", k8s.DefaultLabelSpine, "K8s node label for the cluster's middle network tier") - flag.StringVar(&labelCore, "k8s-topology-key-core", k8s.DefaultLabelCore, "K8s node label for the cluster's top network tier") flag.BoolVar(&ver, "version", false, "show the version") klog.InitFlags(nil) @@ -52,8 +46,6 @@ func main() { os.Exit(0) } - k8s.InitLabels(labelAccelerator, labelLeaf, labelSpine, labelCore) - if err := mainInternal(cfg); err != nil { klog.Error(err.Error()) os.Exit(1) diff --git a/docs/api.md b/docs/api.md index abb82506..c3141d30 100644 --- a/docs/api.md +++ b/docs/api.md @@ -90,6 +90,8 @@ Topograph exposes three endpoints for interacting with the service. Below are th - **namespace**: Used in: [`slinky`]. The required namespace where the SLURM cluster is running. The NFD namespace is deployment-scoped and cannot be supplied in a topology request; Helm deployments configure it with the top-level `nfdNamespace` value. - **podSelector**: Used in: [`slinky`]. A required Kubernetes label selector for pods running SLURM nodes. - **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 single accelerator domain. Defaults to `network.topology.nvidia.com/accelerator`. - **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. diff --git a/docs/design/nfd-engine-sdd.md b/docs/design/nfd-engine-sdd.md index 87e1c482..5d5c38ff 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 accelerator domain, leaf switch, spine switch, and core switch. +group for each distinct fabric-tier or accelerator 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 @@ -21,9 +21,9 @@ NFD CRs for consumers that already watch NFD. Topograph already maps topology into four Kubernetes label dimensions: - `network.topology.nvidia.com/accelerator` -- `network.topology.nvidia.com/leaf` -- `network.topology.nvidia.com/spine` -- `network.topology.nvidia.com/core` +- `network.topology.nvidia.com/tier-0` +- `network.topology.nvidia.com/tier-1` +- `network.topology.nvidia.com/tier-2` NFD `NodeFeatureGroup` is an alpha NFD API. NFD master watches `NodeFeatureGroup` objects, evaluates their feature-group rules, and writes the @@ -72,9 +72,9 @@ spec: topograph.network: elements: accelerator: nvl3 - leaf: leaf-12 - spine: spine-2 - core: core-1 + fabric-tier-0: leaf-12 + fabric-tier-1: spine-2 + fabric-tier-2: core-1 ``` Example generated group for one leaf switch: @@ -83,20 +83,20 @@ Example generated group for one leaf switch: apiVersion: nfd.k8s-sigs.io/v1alpha1 kind: NodeFeatureGroup metadata: - name: topograph-leaf-x3f91c4a0 + name: topograph-fabric-tier-0-leaf-12-... labels: app.kubernetes.io/managed-by: topograph - topograph.nvidia.com/group-type: leaf + topograph.nvidia.com/group-type: fabric-tier-0 annotations: - topograph.nvidia.com/label-key: network.topology.nvidia.com/leaf + topograph.nvidia.com/label-key: network.topology.nvidia.com/tier-0 topograph.nvidia.com/label-value: leaf-12 spec: featureGroupRules: - - name: leaf equals leaf-12 + - name: fabric-tier-0 equals leaf-12 matchFeatures: - feature: topograph.network matchExpressions: - leaf: + fabric-tier-0: op: In value: ["leaf-12"] ``` @@ -127,7 +127,7 @@ 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 accelerator, leaf, spine, and core values. + produce identical values at every discovered fabric tier and accelerator 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 @@ -151,7 +151,7 @@ label name is the same on every node, but the label value differs from one switc to another. The current Kubernetes label model handles this naturally: pod affinity can use -`topologyKey: network.topology.nvidia.com/leaf`, and Kubernetes compares values +`topologyKey: network.topology.nvidia.com/tier-0`, and Kubernetes compares values on candidate nodes. `NodeFeatureGroup` exposes precomputed groups instead, so the consumer needs extra logic to choose among them. @@ -190,7 +190,7 @@ overhead. Assumptions: - 10,000 `NodeFeature` objects, one per node. -- Four topology attributes per node: accelerator, leaf, spine, and core. +- One topology attribute for every discovered fabric tier and accelerator domain. - Each node appears in one `NodeFeatureGroup.status.nodes` list per topology dimension, so status contains about 40,000 node references total. - Average node names and topology values are short, roughly 10-30 characters. @@ -220,7 +220,7 @@ small patches to reduce write amplification. ## Test Plan -- Unit-test graph-to-group generation for accelerator, leaf, spine, and core. +- Unit-test graph-to-group generation across variable fabric tiers and accelerator 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 diff --git a/docs/engines/graph.md b/docs/engines/graph.md index e55ae71e..0576767f 100644 --- a/docs/engines/graph.md +++ b/docs/engines/graph.md @@ -6,7 +6,12 @@ The engine preserves the provider/engine boundary: providers still discover topo ## Output -By default, the generated JSON is returned in the `/v1/topology` response: +By default, the generated JSON is returned in the `/v1/topology` response. + +Each instance entry contains: +- `id` — the provider instance ID +- `network_layers` — switch names or IDs in closest-first order (tier 0 is the switch directly connected to the node, increasing outward); the depth is variable and provider-dependent +- `labels` — per-instance topology labels inherited from the model or provider (e.g., accelerator domain) ```json { diff --git a/docs/engines/k8s.md b/docs/engines/k8s.md index f99fd23e..4442dc21 100644 --- a/docs/engines/k8s.md +++ b/docs/engines/k8s.md @@ -4,12 +4,16 @@ Topograph is a tool designed to enhance scheduling decisions in Kubernetes clust ## Overview -Topograph maps both the multi-tier network hierarchy and accelerated network domains (such as NVLink) using node labels. -Most cloud providers expose three levels of network topology through their APIs. To provide a unified view, Topograph assigns four labels to each node: -* `network.topology.nvidia.com/accelerator`: Identifies high-speed interconnect domains, such as NVLink. If the node already has `nvidia.com/gpu.clique`, Topograph leaves the accelerator label unset and uses the GPU Operator label as the accelerator-domain signal. -* `network.topology.nvidia.com/leaf`: Indicates the switches directly connected to compute nodes. -* `network.topology.nvidia.com/spine`: Represents the next tier of switches above the leaf level. -* `network.topology.nvidia.com/core`: Denotes the top-level switches. +Topograph maps network-fabric locality as a variable-depth label family and +accelerator-network locality as a single label: + +* `network.topology.nvidia.com/tier-N` identifies fabric switch tiers. +* `network.topology.nvidia.com/accelerator` identifies the accelerator domain. + +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 +label remains unset and the GPU Operator label is authoritative. The names of these node labels are configurable via the [Helm chart](https://github.com/NVIDIA/topograph/tree/main/charts/topograph). @@ -17,9 +21,9 @@ For example, if a node belongs to NVLink domain `nvl1` and connects to switch `s ``` network.topology.nvidia.com/accelerator: nvl1 - network.topology.nvidia.com/leaf: s1 - network.topology.nvidia.com/spine: s2 - network.topology.nvidia.com/core: s3 + network.topology.nvidia.com/tier-0: s1 + network.topology.nvidia.com/tier-1: s2 + network.topology.nvidia.com/tier-2: s3 ```

Design

@@ -57,12 +61,12 @@ graph TB The GPU Operator device plugin sets `nvidia.com/gpu.clique` on nodes with Multi-Node NVLink (MNNVL) GPUs (e.g., GB200 NVL72). This label identifies the NVLink clique a node belongs to and can be used as a topology key for Pod placement. -Topograph treats `nvidia.com/gpu.clique` as the authoritative accelerator-domain node label when it is already present: +Topograph treats `nvidia.com/gpu.clique` as the authoritative accelerator node label when it is already present: -- On **MNNVL systems**: if `nvidia.com/gpu.clique` exists on a node, the k8s engine does not write Topograph's configured accelerator label for that node. It still writes `leaf`, `spine`, and `core` labels from the provider topology. +- On **MNNVL systems**: if `nvidia.com/gpu.clique` exists on a node, the k8s engine does not write the accelerator label 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. -In addition to NVLink domain membership, Topograph provides the IB switch hierarchy (`leaf`, `spine`, `core`) — giving schedulers both dimensions of topology simultaneously. +In addition to NVLink domain membership, Topograph provides the full IB switch hierarchy as numbered fabric tiers, giving schedulers both dimensions simultaneously. For `infiniband-k8s`, operators can set `provider.params.useGpuCliqueLabel: true` so the provider reads the GPU Operator's existing clique label instead of collecting the same value through a `nvidia-smi` exec in the GPU Operator device-plugin DaemonSet. @@ -88,7 +92,7 @@ closer network proximity. operator: In values: - myapp - topologyKey: network.topology.nvidia.com/spine + topologyKey: network.topology.nvidia.com/tier-1 - weight: 90 podAffinityTerm: labelSelector: @@ -97,15 +101,15 @@ closer network proximity. operator: In values: - myapp - topologyKey: network.topology.nvidia.com/leaf + topologyKey: network.topology.nvidia.com/tier-0 ``` -Pods are prioritized to be placed on nodes sharing the label `network.topology.nvidia.com/leaf`. +Pods are prioritized to be placed on nodes sharing the label `network.topology.nvidia.com/tier-0`. These nodes are connected to the same network switch, ensuring the lowest latency for communication. -Nodes with the label `network.topology.nvidia.com/spine` are next in priority. +Nodes with the label `network.topology.nvidia.com/tier-1` are next in priority. Pods on these nodes will still be relatively close, but with slightly higher latency. -In the three-tier network, all nodes will share the same `network.topology.nvidia.com/core` label, +In the three-tier network, all nodes will share the same `network.topology.nvidia.com/tier-2` label, so it doesn’t need to be included in pod affinity settings. Since the default Kubernetes scheduler places one pod at a time, the placement may vary depending on where @@ -130,6 +134,13 @@ provider: name: aws engine: name: k8s + params: + # Optional closest-first fabric keys and single accelerator key. + # Additional fabric tiers are omitted when the array is configured. + fabricLabels: + - example.com/rack + - example.com/pod + acceleratorLabel: example.com/nvl-domain ``` ## Exposing the Topograph API diff --git a/docs/engines/nfd.md b/docs/engines/nfd.md index d008b4a2..0cb66abd 100644 --- a/docs/engines/nfd.md +++ b/docs/engines/nfd.md @@ -8,7 +8,7 @@ It creates: - one `NodeFeature` per topology node, carrying Topograph topology as `spec.features.attributes.topograph.network.elements` -- one `NodeFeatureGroup` per distinct accelerator, leaf, spine, and core value +- one `NodeFeatureGroup` per distinct fabric tier or accelerator domain value NFD master evaluates those features and writes matching nodes to `NodeFeatureGroup.status.nodes`. @@ -108,9 +108,9 @@ spec: topograph.network: elements: accelerator: nvl3 - leaf: leaf-12 - spine: spine-2 - core: core-1 + fabric-tier-0: leaf-12 + fabric-tier-1: spine-2 + fabric-tier-2: core-1 ``` For each distinct value, it writes a matching `NodeFeatureGroup`: @@ -119,22 +119,22 @@ For each distinct value, it writes a matching `NodeFeatureGroup`: apiVersion: nfd.k8s-sigs.io/v1alpha1 kind: NodeFeatureGroup metadata: - name: topograph-leaf-leaf-12-... + name: topograph-fabric-tier-0-leaf-12-... namespace: node-feature-discovery labels: app.kubernetes.io/managed-by: topograph topograph.nvidia.com/engine: nfd - topograph.nvidia.com/group-type: leaf + topograph.nvidia.com/group-type: fabric-tier-0 annotations: - topograph.nvidia.com/label-key: network.topology.nvidia.com/leaf + topograph.nvidia.com/label-key: network.topology.nvidia.com/tier-0 topograph.nvidia.com/label-value: leaf-12 spec: featureGroupRules: - - name: leaf equals leaf-12 + - name: fabric-tier-0 equals leaf-12 matchFeatures: - feature: topograph.network matchExpressions: - leaf: + fabric-tier-0: op: In value: ["leaf-12"] ``` @@ -146,8 +146,8 @@ 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 accelerator attribute instead of the value derived from the provider graph. The matching `NodeFeatureGroup` records -`nvidia.com/gpu.clique` as its source label key. Leaf, spine, and core topology -attributes are still published. +`nvidia.com/gpu.clique` as its source label key. All fabric-tier attributes +are still published. ## Caveats diff --git a/docs/engines/slinky.md b/docs/engines/slinky.md index 1bda9fe3..f0cc7b0e 100644 --- a/docs/engines/slinky.md +++ b/docs/engines/slinky.md @@ -79,7 +79,7 @@ engine: ### Using `nvidia.com/gpu.clique` 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 `InstanceTopology.AcceleratorID` describes a broader provider domain than the GPU Operator clique label. +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. 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. diff --git a/docs/modeling.md b/docs/modeling.md index a9544f2f..287aaf2b 100644 --- a/docs/modeling.md +++ b/docs/modeling.md @@ -4,8 +4,8 @@ Topograph models are YAML files used to simulate discovered topology without que A model describes the same canonical topology that real providers eventually produce: -- A switch tree, used for Slurm `topology/tree` output and Kubernetes `leaf` / `spine` / `core` labels -- Node membership in hardware/connectivity blocks, used for block topology and optional accelerator labels +- A variable-depth switch tree, used for Slurm `topology/tree` output and Kubernetes `network.topology.nvidia.com/tier-N` labels +- Node membership in one accelerator domain, used for block topology and the optional `network.topology.nvidia.com/accelerator` label - Optional per-node labels used by provider simulations Model loading lives in `pkg/models`. Model fixtures live under `tests/models/`. diff --git a/docs/providers/infiniband.md b/docs/providers/infiniband.md index 19032e6b..b643920a 100644 --- a/docs/providers/infiniband.md +++ b/docs/providers/infiniband.md @@ -86,7 +86,7 @@ For the Slurm engine, verify the generated `topology.conf` reflects the expected ### How It Works 1. Runs `ibnetdiscover` by exec-ing into a node-data-broker pod on each node to map the switch tree -2. On NVIDIA GPU nodes: reads NVLink clique IDs from the `topograph.nvidia.com/cluster-id` node annotations set by the node-data-broker. If `useGpuCliqueLabel` is enabled, it reads `nvidia.com/gpu.clique` directly instead. The accelerator domain value is `ClusterUUID.CliqueId` — the same format as `nvidia.com/gpu.clique` set by the GPU Operator device plugin on MNNVL systems. When the k8s engine sees `nvidia.com/gpu.clique` already present on a node, it does not write a duplicate Topograph accelerator label for that node. +2. On NVIDIA GPU nodes: reads NVLink clique IDs from the `topograph.nvidia.com/cluster-id` node annotations set by the node-data-broker. If `useGpuCliqueLabel` is enabled, it reads `nvidia.com/gpu.clique` directly instead. The accelerator ID is `ClusterUUID.CliqueId` — the same format as `nvidia.com/gpu.clique` set by the GPU Operator device plugin on MNNVL systems. When the k8s engine sees `nvidia.com/gpu.clique` already present on a node, it does not write the duplicate accelerator label for that node. 3. Combines the switch tree and any NVLink clique data into the topology graph ### Configuration diff --git a/docs/providers/lambdai.md b/docs/providers/lambdai.md index c228766f..267624b7 100644 --- a/docs/providers/lambdai.md +++ b/docs/providers/lambdai.md @@ -180,7 +180,7 @@ helm install topograph oci://ghcr.io/nvidia/topograph/topograph \ kubectl get nodes --show-labels | grep network.topology.nvidia ``` -Only `leaf`/`spine`/`core` labels appear until the API returns `nvlink` data (see the note above); the `accelerator` label follows once it does. +Only fabric `tier-N` labels appear until the API returns `nvlink` data (see the note above); the accelerator label follows once it does. ## Verifying the Output diff --git a/docs/reference/node-labels.md b/docs/reference/node-labels.md index 7bccc313..b93c565b 100644 --- a/docs/reference/node-labels.md +++ b/docs/reference/node-labels.md @@ -8,18 +8,21 @@ Labels are set by the [Kubernetes engine](../engines/k8s.md) (`engine: k8s`) and ### Default label keys +Topograph publishes a variable-depth fabric label family and one accelerator +label. Fabric tier `0` is closest to the compute node, and tier numbers increase +outwards. Only entries present in the discovered topology are written. + | Label key | Topology type | Semantics | |---|---|---| -| `network.topology.nvidia.com/accelerator` | Block (`topology/block`) | Accelerated interconnect domain identifier — nodes that share the same value are in the same accelerated domain. Exact semantics are provider-dependent: for MNNVL-aware providers (DRA, InfiniBand, Lambda AI) the value is an NVL Partition identifier (Fabric-Manager-derived `.`, identifying a logical sub-domain within the physical NVL Domain); for the AWS provider it is the AWS CapacityBlockId (a reservation-scoped identifier co-extensive with an UltraServer — i.e., the NVL Domain — on P6e-GB200). If `nvidia.com/gpu.clique` already exists on a Kubernetes node, the k8s engine does not write this label for that node. See the provider matrix below. | -| `network.topology.nvidia.com/leaf` | Tree (`topology/tree`) | Leaf switch identifier — top-of-rack or first-tier fabric switch | -| `network.topology.nvidia.com/spine` | Tree (`topology/tree`) | Spine switch identifier — second-tier aggregation switch | -| `network.topology.nvidia.com/core` | Tree (`topology/tree`) | Core switch identifier — third tier, present in large three-tier fabrics | +| `network.topology.nvidia.com/accelerator` | Accelerator | Accelerator-interconnect locality. If `nvidia.com/gpu.clique` exists, the k8s engine leaves this label unset for that node. | +| `network.topology.nvidia.com/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. | -Labels are **additive**: a node that belongs to both a block topology (NVLink domain) and a tree topology (switch fabric) normally carries both `accelerator` and `leaf`/`spine`/`core` simultaneously. The exception is nodes that already have `nvidia.com/gpu.clique`; for those, the k8s engine leaves the accelerator domain on `nvidia.com/gpu.clique` and only writes the switch-hierarchy labels. +Labels are **additive**: a node can carry every discovered fabric tier and its +accelerator domain simultaneously. Not all providers produce both topology types: -| Provider | Block (`accelerator`) | Tree (`leaf`/`spine`/`core`) | +| Provider | Accelerator domains | Fabric tiers | |---|---|---| | `aws` | Yes (CapacityBlockId) | Yes | | `cw` | No | Yes (InfiniBand switch hierarchy) | @@ -39,20 +42,20 @@ Not all providers produce both topology types: On non-MNNVL systems (e.g., DGX B200, B300), the GPU fabric never reaches `GPU_FABRIC_STATE_COMPLETED`, so `nvidia.com/gpu.clique` is not set at all. On these systems, Topograph with an InfiniBand provider is the only source of network topology for scheduling decisions. -### Choosing between `accelerator` and `nvidia.com/gpu.clique` for scheduling +### Choosing between the accelerator label and `nvidia.com/gpu.clique` for scheduling -Workload schedulers consuming topology labels may need to choose between Topograph's `network.topology.nvidia.com/accelerator` and the NVIDIA GPU Operator's `nvidia.com/gpu.clique`. The k8s engine automatically avoids writing `accelerator` on nodes where `nvidia.com/gpu.clique` is already present, so schedulers can use `gpu.clique` for those nodes and fall back to `accelerator` where it is absent: +Workload schedulers consuming topology labels may need to choose between Topograph's `network.topology.nvidia.com/accelerator` and the NVIDIA GPU Operator's `nvidia.com/gpu.clique`. The k8s engine automatically avoids writing the accelerator label on nodes where `nvidia.com/gpu.clique` is already present, so schedulers can use `gpu.clique` for those nodes and fall back to the accelerator label where it is absent: -- **MNNVL hardware + Fabric Manager completed + NVL Partition granularity desired:** use `nvidia.com/gpu.clique`. On the AWS provider this is finer granularity than `accelerator` (which carries the CapacityBlockId, i.e., the NVL Domain). On DRA, InfiniBand, and Lambda AI providers the two labels carry the same value. +- **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 DRA, 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 `network.topology.nvidia.com/accelerator`. - **Slurm clusters (no Kubernetes node labels):** neither label applies. Consumers read Slurm's `topology.conf` directly. **Caveats when preferring `nvidia.com/gpu.clique`:** - The label encodes node identity within MNNVL domains, not fabric proximity between them. NVL Partition is encoded as the full `.` value; NVL Domain is encoded as the `ClusterUUID` prefix. A scheduler can therefore distinguish racks — two nodes with different `ClusterUUID` are in different NVL Domains — and act on that distinction (same-Domain affinity to pack a job onto a single rack, cross-Domain anti-affinity to spread independent jobs across racks). What the label does **not** encode is the *physical proximity* between Domains: `ClusterUUID`s are opaque identifiers, so the label cannot tell a scheduler which racks share a top-of-rack switch, an aggregation tier, or a core. For cross-rack proximity-aware placement, Topograph populates the following labels from the InfiniBand or NetQ providers regardless of whether `gpu.clique` is present: - - **Same top-of-rack switch** (cross-rack within a first-tier fabric) — Topograph's `leaf` label. - - **Same second-tier aggregation** (typically Scalable-Unit / pod-scale grouping above individual racks) — Topograph's `spine` label. - - **Same third-tier aggregation** (present in large three-tier fabrics — typically cross-SU grouping in multi-SU SuperPOD deployments) — Topograph's `core` label. + - **Same top-of-rack switch** (cross-rack within a first-tier fabric) — Topograph's fabric tier 0 label. + - **Same second-tier aggregation** (typically Scalable-Unit / pod-scale grouping above individual racks) — Topograph's fabric tier 1 label. + - **Same third-tier aggregation** (present in large three-tier fabrics — typically cross-SU grouping in multi-SU SuperPOD deployments) — Topograph's fabric tier 2 label. These labels are also relevant for mixed-workload fragmentation avoidance (see [`docs/engines/k8s.md` § Mixed Workload Considerations](../engines/k8s.md#mixed-workload-considerations)). - The label is refreshed by GPU Feature Discovery at its configured interval (the k8s-device-plugin default is 60s) rather than propagated instantly. Fabric-state changes in the window between refreshes are not yet reflected in the label. @@ -64,7 +67,13 @@ Label values are used as-is when they are 63 characters or shorter (the Kubernet ### Configuring label keys -The default `network.topology.nvidia.com/` prefix is configurable via the Helm `topologyNodeLabels` value. If you need to map topograph's topology layers to a custom label schema, override the keys at deploy time. The label _values_ (topology identifiers) are always derived from the provider's topology discovery and cannot be configured. +The `k8s` engine accepts an optional closest-first `fabricLabels` array and an +optional singular `acceleratorLabel`. When `fabricLabels` is omitted, Topograph +uses `network.topology.nvidia.com/tier-N` for every discovered fabric tier. When +provided, only explicitly listed tiers are labeled; additional tiers are +omitted. `acceleratorLabel` defaults to +`network.topology.nvidia.com/accelerator`. Every configured key must be a valid +Kubernetes label key. Label values always come from provider discovery. ### Relationship to upstream standardization (KEP-4962) @@ -130,9 +139,9 @@ transformers: # ... existing labels ... # Topograph topology labels (requires Topograph deployed in the cluster) - "network.topology.nvidia.com/accelerator" - - "network.topology.nvidia.com/leaf" - - "network.topology.nvidia.com/spine" - - "network.topology.nvidia.com/core" + - "network.topology.nvidia.com/tier-0" + - "network.topology.nvidia.com/tier-1" + - "network.topology.nvidia.com/tier-2" ``` See NVSentinel's [`docs/INTEGRATIONS.md` § Topology Awareness (Topograph)](https://github.com/NVIDIA/NVSentinel/blob/main/docs/INTEGRATIONS.md#topology-awareness-topograph). diff --git a/internal/kwok/nodes_test.go b/internal/kwok/nodes_test.go index 93055c74..e13606da 100644 --- a/internal/kwok/nodes_test.go +++ b/internal/kwok/nodes_test.go @@ -33,8 +33,8 @@ func TestNodesFromModel(t *testing.T) { node := nodes[0] require.Equal(t, "i21", node.Name) require.Equal(t, map[string]string{ - NodeSelectorKey: NodeSelectorValue, - models.LabelAccelerator: "nvl2", + NodeSelectorKey: NodeSelectorValue, + topology.KeyTopologyAccelerator: "nvl2", }, node.Labels) require.Equal(t, map[string]string{ NodeSelectorKey: NodeSelectorValue, @@ -69,7 +69,7 @@ func TestNodesFromModelUsesDerivedRegionAndLabels(t *testing.T) { require.Equal(t, "us-west", node.Annotations[topology.KeyNodeRegion]) require.Equal(t, "us-west", node.Labels[models.LabelTopologyRegion]) require.Equal(t, "zone2", node.Labels[models.LabelTopologyZone]) - require.Equal(t, "nvl3", node.Labels[models.LabelAccelerator]) + require.Equal(t, "nvl3", node.Labels[topology.KeyTopologyAccelerator]) } func TestMarshalNodeManifest(t *testing.T) { diff --git a/pkg/engines/k8s/engine.go b/pkg/engines/k8s/engine.go index 3bae74a1..5bd0f650 100644 --- a/pkg/engines/k8s/engine.go +++ b/pkg/engines/k8s/engine.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package k8s @@ -42,9 +31,14 @@ type K8sEngine struct { type Params struct { // NodeSelector (optional) specifies nodes participating in the topology NodeSelector map[string]string `mapstructure:"nodeSelector"` + // FabricLabels optionally sets label keys by closest-first tier. + FabricLabels []string `mapstructure:"fabricLabels"` + // AcceleratorLabel optionally sets the accelerator label key. + AcceleratorLabel string `mapstructure:"acceleratorLabel"` // derived fields nodeListOpt *metav1.ListOptions + labelKeys *TopologyLabelKeys } func NamedLoader() (string, engines.Loader) { @@ -79,6 +73,10 @@ func getParameters(params engines.Config) (*Params, error) { if err := config.Decode(params, p); err != nil { return nil, err } + p.labelKeys = NewTopologyLabelKeys(p.FabricLabels, p.AcceleratorLabel) + if err := p.labelKeys.Validate(); err != nil { + return nil, err + } if len(p.NodeSelector) != 0 { p.nodeListOpt = &metav1.ListOptions{ @@ -90,7 +88,7 @@ func getParameters(params engines.Config) (*Params, error) { } func (eng *K8sEngine) GenerateOutput(ctx context.Context, graph *topology.Graph, _ map[string]any) ([]byte, *httperr.Error) { - if err := NewTopologyLabeler().ApplyNodeLabels(ctx, graph, eng); err != nil { + if err := NewTopologyLabeler(eng.params.labelKeys).ApplyNodeLabels(ctx, graph, eng); err != nil { return nil, httperr.NewError(http.StatusBadGateway, err.Error()) } diff --git a/pkg/engines/k8s/engine_test.go b/pkg/engines/k8s/engine_test.go index 2496847b..856a1356 100644 --- a/pkg/engines/k8s/engine_test.go +++ b/pkg/engines/k8s/engine_test.go @@ -22,7 +22,9 @@ func TestGetParameters(t *testing.T) { { name: "Case 1: no params", params: nil, - ret: &Params{}, + ret: &Params{ + labelKeys: NewTopologyLabelKeys(nil, ""), + }, }, { name: "Case 2: bad params", @@ -30,14 +32,53 @@ func TestGetParameters(t *testing.T) { err: "could not decode configuration: 1 error(s) decoding:\n\n* 'nodeSelector' expected a map, got 'float64'", }, { - name: "Case 3: valid input", - params: map[string]any{"nodeSelector": map[string]string{"key": "val"}}, + name: "Case 3: valid input", + params: map[string]any{ + "nodeSelector": map[string]string{"key": "val"}, + "fabricLabels": []string{"example.com/rack", "example.com/pod"}, + "acceleratorLabel": "example.com/nvl", + }, ret: &Params{ - NodeSelector: map[string]string{"key": "val"}, + NodeSelector: map[string]string{"key": "val"}, + FabricLabels: []string{"example.com/rack", "example.com/pod"}, + AcceleratorLabel: "example.com/nvl", nodeListOpt: &metav1.ListOptions{ LabelSelector: "key=val", }, + labelKeys: NewTopologyLabelKeys( + []string{"example.com/rack", "example.com/pod"}, + "example.com/nvl", + ), + }, + }, + { + name: "Case 4: reject duplicate topology label keys", + params: map[string]any{ + "fabricLabels": []string{"example.com/shared"}, + "acceleratorLabel": "example.com/shared", + }, + err: `topology label key "example.com/shared" is configured for both fabricLabels[0] and acceleratorLabel`, + }, + { + name: "Case 5: reject invalid topology label key", + params: map[string]any{ + "fabricLabels": []string{"not a label"}, + }, + err: `fabricLabels[0] "not a label" is not a valid Kubernetes label key`, + }, + { + name: "Case 6: reject duplicate label within one family", + params: map[string]any{ + "fabricLabels": []string{"example.com/shared", "example.com/shared"}, + }, + err: `topology label key "example.com/shared" is configured for both fabricLabels[0] and fabricLabels[1]`, + }, + { + name: "Case 7: reject empty custom label key", + params: map[string]any{ + "fabricLabels": []string{""}, }, + err: `fabricLabels[0] "" is not a valid Kubernetes label key`, }, } diff --git a/pkg/engines/k8s/kubernetes.go b/pkg/engines/k8s/kubernetes.go index 648bd74c..985263cf 100644 --- a/pkg/engines/k8s/kubernetes.go +++ b/pkg/engines/k8s/kubernetes.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package k8s @@ -20,6 +9,7 @@ import ( "context" "maps" "net/http" + "strconv" "strings" corev1 "k8s.io/api/core/v1" @@ -46,32 +36,65 @@ func (eng *K8sEngine) AddNodeLabels(ctx context.Context, nodeName string, labels return err } - MergeNodeLabels(node, labels) + MergeNodeLabels(node, labels, eng.params.labelKeys) _, err = eng.client.CoreV1().Nodes().Update(ctx, node, metav1.UpdateOptions{}) return err } -func MergeNodeLabels(node *corev1.Node, labels map[string]string) { +func MergeNodeLabels(node *corev1.Node, labels map[string]string, keys *TopologyLabelKeys) { if node.Labels == nil { node.Labels = make(map[string]string) } - labels = skipAcceleratorLabelWhenGPUCliqueExists(node, labels) + labels = skipAcceleratorLabelWhenGPUCliqueExists(node, labels, keys) + removeManagedTopologyLabels(node.Labels, keys) maps.Copy(node.Labels, labels) } -func skipAcceleratorLabelWhenGPUCliqueExists(node *corev1.Node, labels map[string]string) map[string]string { - if topologyLabelKeys.Accelerator == "" || strings.TrimSpace(node.Labels[topology.KeyNvidiaGPUClique]) == "" { +func removeManagedTopologyLabels(labels map[string]string, keys *TopologyLabelKeys) { + for key := range labels { + if key == topology.KeyNvidiaGPUClique { + continue + } + if isManagedLevelLabel(key, keys) { + delete(labels, key) + } + } +} + +func isManagedLevelLabel(key string, keys *TopologyLabelKeys) bool { + if key == topology.KeyTopologyAccelerator { + return true + } + for _, configured := range append(append([]string(nil), keys.Fabric...), keys.Accelerator) { + if configured != "" && key == configured { + return true + } + } + for _, prefix := range []string{topology.KeyFabricTierPrefix} { + if strings.HasPrefix(key, prefix) { + level, err := strconv.Atoi(strings.TrimPrefix(key, prefix)) + if err == nil && level >= 0 { + return true + } + } + } + return false +} + +func skipAcceleratorLabelWhenGPUCliqueExists(node *corev1.Node, labels map[string]string, keys *TopologyLabelKeys) map[string]string { + acceleratorLabel := keys.AcceleratorKey() + if strings.TrimSpace(node.Labels[topology.KeyNvidiaGPUClique]) == "" { return labels } filtered := maps.Clone(labels) - delete(filtered, topologyLabelKeys.Accelerator) + delete(filtered, acceleratorLabel) - if topologyLabelKeys.Accelerator != topology.KeyNvidiaGPUClique { - delete(node.Labels, topologyLabelKeys.Accelerator) + if acceleratorLabel != topology.KeyNvidiaGPUClique { + delete(node.Labels, acceleratorLabel) } return filtered diff --git a/pkg/engines/k8s/kubernetes_test.go b/pkg/engines/k8s/kubernetes_test.go index 143a32c0..548a0594 100644 --- a/pkg/engines/k8s/kubernetes_test.go +++ b/pkg/engines/k8s/kubernetes_test.go @@ -73,8 +73,6 @@ func TestGetComputeInstances(t *testing.T) { } func TestMergeNodeLabels(t *testing.T) { - InitLabels(DefaultLabelAccelerator, DefaultLabelLeaf, DefaultLabelSpine, DefaultLabelCore) - testCases := []struct { name string acceleratorLabel string @@ -109,23 +107,26 @@ func TestMergeNodeLabels(t *testing.T) { node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", - DefaultLabelAccelerator: "old-domain", - DefaultLabelLeaf: "old-leaf", - "workload.example/label": "keep", + topology.KeyNvidiaGPUClique: "cluster-a.0", + topology.KeyTopologyAccelerator: "old-domain", + topology.FabricTierKey(0): "old-leaf", + topology.FabricTierKey(3): "stale-fabric", + "network.topology.nvidia.com/core": "legacy-core", + "workload.example/label": "keep", }, }, }, in: map[string]string{ - DefaultLabelAccelerator: "api-domain", - DefaultLabelLeaf: "new-leaf", - DefaultLabelSpine: "new-spine", + topology.KeyTopologyAccelerator: "api-domain", + topology.FabricTierKey(0): "new-leaf", + topology.FabricTierKey(1): "new-spine", }, out: map[string]string{ - topology.KeyNvidiaGPUClique: "cluster-a.0", - DefaultLabelLeaf: "new-leaf", - DefaultLabelSpine: "new-spine", - "workload.example/label": "keep", + topology.KeyNvidiaGPUClique: "cluster-a.0", + topology.FabricTierKey(0): "new-leaf", + topology.FabricTierKey(1): "new-spine", + "network.topology.nvidia.com/core": "legacy-core", + "workload.example/label": "keep", }, }, { @@ -140,24 +141,53 @@ func TestMergeNodeLabels(t *testing.T) { }, in: map[string]string{ topology.KeyNvidiaGPUClique: "api-domain", - DefaultLabelLeaf: "new-leaf", - DefaultLabelSpine: "new-spine", + topology.FabricTierKey(0): "new-leaf", + }, + out: map[string]string{ + topology.KeyNvidiaGPUClique: "cluster-a.0", + topology.FabricTierKey(0): "new-leaf", + }, + }, + { + name: "Case 6: custom accelerator label still protects GPU clique", + acceleratorLabel: "custom.example/accelerator", + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + topology.KeyNvidiaGPUClique: "cluster-a.0", + }, + }, + }, + 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", - DefaultLabelLeaf: "new-leaf", - DefaultLabelSpine: "new-spine", + topology.FabricTierKey(0): "new-leaf", + topology.FabricTierKey(1): "new-spine", + }, + }, + { + name: "Case 7: apply accelerator label when GPU clique is absent", + node: &corev1.Node{}, + in: map[string]string{ + topology.KeyTopologyAccelerator: "api-domain", + }, + out: map[string]string{ + topology.KeyTopologyAccelerator: "api-domain", }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + keys := NewTopologyLabelKeys(nil, "") if tc.acceleratorLabel != "" { - InitLabels(tc.acceleratorLabel, DefaultLabelLeaf, DefaultLabelSpine, DefaultLabelCore) - defer InitLabels(DefaultLabelAccelerator, DefaultLabelLeaf, DefaultLabelSpine, DefaultLabelCore) + keys = NewTopologyLabelKeys(nil, tc.acceleratorLabel) } - MergeNodeLabels(tc.node, tc.in) + MergeNodeLabels(tc.node, tc.in, keys) require.Equal(t, tc.out, tc.node.Labels) }) } diff --git a/pkg/engines/k8s/labeler.go b/pkg/engines/k8s/labeler.go index 4f815dce..637beba0 100644 --- a/pkg/engines/k8s/labeler.go +++ b/pkg/engines/k8s/labeler.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package k8s @@ -20,42 +9,68 @@ import ( "context" "fmt" "hash/fnv" + "slices" + "strings" - "github.com/NVIDIA/topograph/pkg/topology" -) + "k8s.io/apimachinery/pkg/util/validation" -const ( - DefaultLabelAccelerator = "network.topology.nvidia.com/accelerator" - DefaultLabelLeaf = "network.topology.nvidia.com/leaf" - DefaultLabelSpine = "network.topology.nvidia.com/spine" - DefaultLabelCore = "network.topology.nvidia.com/core" + "github.com/NVIDIA/topograph/pkg/topology" ) type TopologyLabelKeys struct { + Fabric []string Accelerator string - Leaf string - Spine string - Core string } -var topologyLabelKeys = TopologyLabelKeys{ - Accelerator: DefaultLabelAccelerator, - Leaf: DefaultLabelLeaf, - Spine: DefaultLabelSpine, - Core: DefaultLabelCore, +// NewTopologyLabelKeys creates an independent copy of the configured +// closest-first fabric label keys and accelerator label key. +func NewTopologyLabelKeys(fabric []string, accelerator string) *TopologyLabelKeys { + if accelerator == "" { + accelerator = topology.KeyTopologyAccelerator + } + return &TopologyLabelKeys{ + Fabric: slices.Clone(fabric), + Accelerator: accelerator, + } } -func InitLabels(accelerator, leaf, spine, core string) { - topologyLabelKeys = TopologyLabelKeys{ - Accelerator: accelerator, - Leaf: leaf, - Spine: spine, - Core: core, +// Validate checks that configured keys are valid and unique across both label +// families. +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 previous, ok := seen[key]; ok { + return fmt.Errorf("topology label key %q is configured for both %s and %s", key, previous, location) + } + seen[key] = location + return nil } + for tier, key := range keys.Fabric { + if err := validate(fmt.Sprintf("fabricLabels[%d]", tier), key); err != nil { + return err + } + } + return validate("acceleratorLabel", keys.Accelerator) +} + +// FabricKey returns the fabric key for tier. Defaults are used when no custom +// fabric keys were supplied; an empty result means the tier is omitted. +func (keys *TopologyLabelKeys) FabricKey(tier int) string { + if len(keys.Fabric) == 0 { + return topology.FabricTierKey(tier) + } + if tier >= 0 && tier < len(keys.Fabric) { + return keys.Fabric[tier] + } + return "" } -func CurrentTopologyLabelKeys() TopologyLabelKeys { - return topologyLabelKeys +// AcceleratorKey returns the configured accelerator label key. +func (keys *TopologyLabelKeys) AcceleratorKey() string { + return keys.Accelerator } // map nodename:[label name: label value] @@ -67,14 +82,20 @@ type Labeler interface { type topologyLabeler struct { mapper map[string]string + keys *TopologyLabelKeys } -func NewTopologyLabeler() *topologyLabeler { +// NewTopologyLabeler creates a graph-to-label translator using the supplied +// topology label keys. +func NewTopologyLabeler(keys *TopologyLabelKeys) *topologyLabeler { return &topologyLabeler{ mapper: make(map[string]string), + keys: keys, } } +// ApplyNodeLabels builds the desired labels and delegates each node update to +// the supplied Labeler. func (l *topologyLabeler) ApplyNodeLabels(ctx context.Context, graph *topology.Graph, labeler Labeler) error { nodeMap, err := l.BuildNodeLabels(graph) if err != nil { @@ -90,6 +111,8 @@ func (l *topologyLabeler) ApplyNodeLabels(ctx context.Context, graph *topology.G return nil } +// BuildNodeLabels converts accelerator domains and fabric tiers into desired +// Kubernetes labels keyed by node name. func (l *topologyLabeler) BuildNodeLabels(graph *topology.Graph) (NodeLabelMap, error) { nodeMap := make(NodeLabelMap) @@ -116,23 +139,30 @@ func (l *topologyLabeler) BuildNodeLabels(graph *topology.Graph) (NodeLabelMap, return nodeMap, nil } +// getDomainLabels adds one accelerator label for each node in each domain. func (l *topologyLabeler) getDomainLabels(domains topology.DomainMap, nodeMap NodeLabelMap) error { + labelKey := l.keys.AcceleratorKey() for domainName, domain := range domains { for nodeName := range domain { + if nodeName == "" { + continue + } labels, ok := nodeMap[nodeName] if !ok { labels = make(map[string]string) nodeMap[nodeName] = labels } - if val, ok := labels[topologyLabelKeys.Accelerator]; ok { + if val, ok := labels[labelKey]; ok { return fmt.Errorf("multiple accelerator labels %s, %s for node %s", val, domainName, nodeName) } - labels[topologyLabelKeys.Accelerator] = l.checkLabel(domainName) + labels[labelKey] = l.checkLabel(domainName) } } return nil } +// getTierLabels walks the fabric tree and records closest-first switch labels +// when it reaches each compute-node vertex. func (l *topologyLabeler) getTierLabels(v *topology.Vertex, nodeMap NodeLabelMap, layers []string) error { if len(v.Vertices) == 0 { // compute node if len(layers) != 0 { @@ -140,23 +170,20 @@ func (l *topologyLabeler) getTierLabels(v *topology.Vertex, nodeMap NodeLabelMap return fmt.Errorf("instance ID mismatch: expected %s, got %s", v.ID, layers[0]) } nodeName := v.Name + if nodeName == "" { + return nil + } labels, ok := nodeMap[nodeName] if !ok { labels = make(map[string]string) nodeMap[nodeName] = labels } - switchNetworkHierarchy := [...]string{ - topologyLabelKeys.Leaf, - topologyLabelKeys.Spine, - topologyLabelKeys.Core, - } for i, sw := range layers[1:] { - if len(sw) == 0 { + labelKey := l.keys.FabricKey(i) + if len(sw) == 0 || labelKey == "" { break } - if i < len(switchNetworkHierarchy) { - labels[(switchNetworkHierarchy[i])] = l.checkLabel(sw) - } + labels[labelKey] = l.checkLabel(sw) } } return nil @@ -171,8 +198,8 @@ func (l *topologyLabeler) getTierLabels(v *topology.Vertex, nodeMap NodeLabelMap return nil } -// checkLabel checks the length of the label value. -// If more than 63 characters (Kubernetes limit), it will replace it with hash +// checkLabel preserves valid-length values and hashes values that exceed the +// Kubernetes 63-character label-value limit. func (l *topologyLabeler) checkLabel(val string) string { v, ok := l.mapper[val] if ok { diff --git a/pkg/engines/k8s/labeler_test.go b/pkg/engines/k8s/labeler_test.go index 73e98a30..007761c2 100644 --- a/pkg/engines/k8s/labeler_test.go +++ b/pkg/engines/k8s/labeler_test.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package k8s @@ -23,6 +12,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/NVIDIA/topograph/pkg/topology" "github.com/NVIDIA/topograph/pkg/translate" ) @@ -39,113 +29,209 @@ func (l *testLabeler) AddNodeLabels(_ context.Context, nodeName string, labels m } func TestApplyNodeLabelsWithTree(t *testing.T) { - InitLabels(DefaultLabelAccelerator, DefaultLabelLeaf, DefaultLabelSpine, DefaultLabelCore) root, _ := translate.GetTreeTestSet(true) labeler := &testLabeler{data: make(map[string]map[string]string)} data := map[string]map[string]string{ - "Node201": {"network.topology.nvidia.com/leaf": "S2", "network.topology.nvidia.com/spine": "S1"}, - "Node202": {"network.topology.nvidia.com/leaf": "S2", "network.topology.nvidia.com/spine": "S1"}, - "Node205": {"network.topology.nvidia.com/leaf": "S2", "network.topology.nvidia.com/spine": "S1"}, - "Node304": {"network.topology.nvidia.com/leaf": "xf946c4acef2d5939", "network.topology.nvidia.com/spine": "S1"}, - "Node305": {"network.topology.nvidia.com/leaf": "xf946c4acef2d5939", "network.topology.nvidia.com/spine": "S1"}, - "Node306": {"network.topology.nvidia.com/leaf": "xf946c4acef2d5939", "network.topology.nvidia.com/spine": "S1"}, + "Node201": {"network.topology.nvidia.com/tier-0": "S2", "network.topology.nvidia.com/tier-1": "S1"}, + "Node202": {"network.topology.nvidia.com/tier-0": "S2", "network.topology.nvidia.com/tier-1": "S1"}, + "Node205": {"network.topology.nvidia.com/tier-0": "S2", "network.topology.nvidia.com/tier-1": "S1"}, + "Node304": {"network.topology.nvidia.com/tier-0": "xf946c4acef2d5939", "network.topology.nvidia.com/tier-1": "S1"}, + "Node305": {"network.topology.nvidia.com/tier-0": "xf946c4acef2d5939", "network.topology.nvidia.com/tier-1": "S1"}, + "Node306": {"network.topology.nvidia.com/tier-0": "xf946c4acef2d5939", "network.topology.nvidia.com/tier-1": "S1"}, } - err := NewTopologyLabeler().ApplyNodeLabels(context.TODO(), root, labeler) + err := NewTopologyLabeler(NewTopologyLabelKeys(nil, "")).ApplyNodeLabels(context.TODO(), root, labeler) require.NoError(t, err) require.Equal(t, data, labeler.data) } func TestApplyNodeLabelsWithBlock(t *testing.T) { - InitLabels(DefaultLabelAccelerator, DefaultLabelLeaf, DefaultLabelSpine, DefaultLabelCore) root, _ := translate.GetBlockWithMultiIBTestSet() labeler := &testLabeler{data: make(map[string]map[string]string)} data := map[string]map[string]string{ "Node104": { "network.topology.nvidia.com/accelerator": "B1", - "network.topology.nvidia.com/leaf": "S2", - "network.topology.nvidia.com/spine": "S1", - "network.topology.nvidia.com/core": "IB2", + "network.topology.nvidia.com/tier-0": "S2", + "network.topology.nvidia.com/tier-1": "S1", + "network.topology.nvidia.com/tier-2": "IB2", }, "Node105": { "network.topology.nvidia.com/accelerator": "B1", - "network.topology.nvidia.com/leaf": "S2", - "network.topology.nvidia.com/spine": "S1", - "network.topology.nvidia.com/core": "IB2", + "network.topology.nvidia.com/tier-0": "S2", + "network.topology.nvidia.com/tier-1": "S1", + "network.topology.nvidia.com/tier-2": "IB2", }, "Node106": { "network.topology.nvidia.com/accelerator": "B1", - "network.topology.nvidia.com/leaf": "S2", - "network.topology.nvidia.com/spine": "S1", - "network.topology.nvidia.com/core": "IB2", + "network.topology.nvidia.com/tier-0": "S2", + "network.topology.nvidia.com/tier-1": "S1", + "network.topology.nvidia.com/tier-2": "IB2", }, "Node201": { "network.topology.nvidia.com/accelerator": "B2", - "network.topology.nvidia.com/leaf": "S3", - "network.topology.nvidia.com/spine": "S1", - "network.topology.nvidia.com/core": "IB2", + "network.topology.nvidia.com/tier-0": "S3", + "network.topology.nvidia.com/tier-1": "S1", + "network.topology.nvidia.com/tier-2": "IB2", }, "Node202": { "network.topology.nvidia.com/accelerator": "B2", - "network.topology.nvidia.com/leaf": "S3", - "network.topology.nvidia.com/spine": "S1", - "network.topology.nvidia.com/core": "IB2", + "network.topology.nvidia.com/tier-0": "S3", + "network.topology.nvidia.com/tier-1": "S1", + "network.topology.nvidia.com/tier-2": "IB2", }, "Node205": { "network.topology.nvidia.com/accelerator": "B2", - "network.topology.nvidia.com/leaf": "S3", - "network.topology.nvidia.com/spine": "S1", - "network.topology.nvidia.com/core": "IB2", + "network.topology.nvidia.com/tier-0": "S3", + "network.topology.nvidia.com/tier-1": "S1", + "network.topology.nvidia.com/tier-2": "IB2", }, "Node301": { "network.topology.nvidia.com/accelerator": "B3", - "network.topology.nvidia.com/leaf": "S5", - "network.topology.nvidia.com/spine": "S4", - "network.topology.nvidia.com/core": "IB1", + "network.topology.nvidia.com/tier-0": "S5", + "network.topology.nvidia.com/tier-1": "S4", + "network.topology.nvidia.com/tier-2": "IB1", }, "Node302": { "network.topology.nvidia.com/accelerator": "B3", - "network.topology.nvidia.com/leaf": "S5", - "network.topology.nvidia.com/spine": "S4", - "network.topology.nvidia.com/core": "IB1", + "network.topology.nvidia.com/tier-0": "S5", + "network.topology.nvidia.com/tier-1": "S4", + "network.topology.nvidia.com/tier-2": "IB1", }, "Node303": { "network.topology.nvidia.com/accelerator": "B3", - "network.topology.nvidia.com/leaf": "S5", - "network.topology.nvidia.com/spine": "S4", - "network.topology.nvidia.com/core": "IB1", + "network.topology.nvidia.com/tier-0": "S5", + "network.topology.nvidia.com/tier-1": "S4", + "network.topology.nvidia.com/tier-2": "IB1", }, "Node401": { "network.topology.nvidia.com/accelerator": "B4", - "network.topology.nvidia.com/leaf": "S6", - "network.topology.nvidia.com/spine": "S4", - "network.topology.nvidia.com/core": "IB1", + "network.topology.nvidia.com/tier-0": "S6", + "network.topology.nvidia.com/tier-1": "S4", + "network.topology.nvidia.com/tier-2": "IB1", }, "Node402": { "network.topology.nvidia.com/accelerator": "B4", - "network.topology.nvidia.com/leaf": "S6", - "network.topology.nvidia.com/spine": "S4", - "network.topology.nvidia.com/core": "IB1", + "network.topology.nvidia.com/tier-0": "S6", + "network.topology.nvidia.com/tier-1": "S4", + "network.topology.nvidia.com/tier-2": "IB1", }, "Node403": { "network.topology.nvidia.com/accelerator": "B4", - "network.topology.nvidia.com/leaf": "S6", - "network.topology.nvidia.com/spine": "S4", - "network.topology.nvidia.com/core": "IB1", + "network.topology.nvidia.com/tier-0": "S6", + "network.topology.nvidia.com/tier-1": "S4", + "network.topology.nvidia.com/tier-2": "IB1", }, } - err := NewTopologyLabeler().ApplyNodeLabels(context.TODO(), root, labeler) + err := NewTopologyLabeler(NewTopologyLabelKeys(nil, "")).ApplyNodeLabels(context.TODO(), root, labeler) require.NoError(t, err) require.Equal(t, data, labeler.data) } -func TestInitLabels(t *testing.T) { - InitLabels("a", "b", "c", "d") - require.Equal(t, TopologyLabelKeys{ - Accelerator: "a", - Leaf: "b", - Spine: "c", - Core: "d", - }, CurrentTopologyLabelKeys()) +func TestTopologyLabelKeysUseDefaultsWhenCustomLabelsAreOmitted(t *testing.T) { + keys := NewTopologyLabelKeys(nil, "") + require.Equal(t, topology.FabricTierKey(2), keys.FabricKey(2)) + require.Equal(t, topology.KeyTopologyAccelerator, keys.AcceleratorKey()) +} + +func TestTopologyLabelKeysUseOnlyConfiguredLabels(t *testing.T) { + keys := NewTopologyLabelKeys( + []string{"example.com/rack", "example.com/pod"}, + "example.com/nvl", + ) + require.Equal(t, "example.com/rack", keys.FabricKey(0)) + require.Equal(t, "example.com/pod", keys.FabricKey(1)) + require.Empty(t, keys.FabricKey(2)) + require.Equal(t, "example.com/nvl", keys.AcceleratorKey()) + + fabricOnly := NewTopologyLabelKeys([]string{"example.com/rack"}, "") + require.Equal(t, "example.com/rack", fabricOnly.FabricKey(0)) + require.Equal(t, topology.KeyTopologyAccelerator, fabricOnly.AcceleratorKey()) +} + +func TestBuildNodeLabelsWithVariableLevels(t *testing.T) { + node := &topology.Vertex{ID: "instance-1", Name: "node-1"} + fabric0 := &topology.Vertex{ID: "fabric-0", Vertices: map[string]*topology.Vertex{node.ID: node}} + fabric1 := &topology.Vertex{ID: "fabric-1", Vertices: map[string]*topology.Vertex{fabric0.ID: fabric0}} + fabric2 := &topology.Vertex{ID: "fabric-2", Vertices: map[string]*topology.Vertex{fabric1.ID: fabric1}} + fabric3 := &topology.Vertex{ID: "fabric-3", Vertices: map[string]*topology.Vertex{fabric2.ID: fabric2}} + graph := &topology.Graph{ + Tiers: &topology.Vertex{Vertices: map[string]*topology.Vertex{fabric3.ID: fabric3}}, + Domains: topology.DomainMap{"accelerator": {"node-1": &topology.HostInfo{}}}, + } + + keys := NewTopologyLabelKeys( + []string{"example.com/fabric-0", "example.com/fabric-1"}, + "example.com/accelerator", + ) + labels, err := NewTopologyLabeler(keys).BuildNodeLabels(graph) + require.NoError(t, err) + require.Equal(t, map[string]string{ + "example.com/fabric-0": "fabric-0", + "example.com/fabric-1": "fabric-1", + "example.com/accelerator": "accelerator", + }, labels["node-1"]) +} + +func TestBuildNodeLabelsWithMixedDepthSharedRoot(t *testing.T) { + cluster := topology.NewClusterTopology() + cluster.Append(&topology.InstanceTopology{ + InstanceID: "instance-1", + FabricTiers: topology.ClosestFirstFabricTiers("leaf-1", "shared-root"), + }) + cluster.Append(&topology.InstanceTopology{ + InstanceID: "instance-2", + FabricTiers: topology.ClosestFirstFabricTiers("leaf-2", "spine-2", "shared-root"), + }) + graph := cluster.ToGraph("test", []topology.ComputeInstances{{ + Instances: map[string]string{ + "instance-1": "node-1", + "instance-2": "node-2", + }, + }}, 0, false) + + labels, err := NewTopologyLabeler(NewTopologyLabelKeys(nil, "")).BuildNodeLabels(graph) + require.NoError(t, err) + require.Equal(t, map[string]string{ + topology.FabricTierKey(0): "leaf-1", + topology.FabricTierKey(1): "shared-root", + }, labels["node-1"]) + require.Equal(t, map[string]string{ + topology.FabricTierKey(0): "leaf-2", + topology.FabricTierKey(1): "spine-2", + topology.FabricTierKey(2): "shared-root", + }, labels["node-2"]) +} + +func TestApplyNodeLabelsSkipsUnnamedComputeNodes(t *testing.T) { + unnamed := &topology.Vertex{ID: "instance-unnamed"} + valid := &topology.Vertex{ID: "instance-valid", Name: "node-valid"} + leaf := &topology.Vertex{ + ID: "leaf", + Vertices: map[string]*topology.Vertex{ + unnamed.ID: unnamed, + valid.ID: valid, + }, + } + graph := &topology.Graph{ + Tiers: &topology.Vertex{Vertices: map[string]*topology.Vertex{leaf.ID: leaf}}, + Domains: topology.DomainMap{ + "accelerator": { + "": &topology.HostInfo{}, + "node-valid": &topology.HostInfo{}, + }, + }, + } + labeler := &testLabeler{data: make(map[string]map[string]string)} + + err := NewTopologyLabeler(NewTopologyLabelKeys(nil, "")).ApplyNodeLabels(context.Background(), graph, labeler) + + require.NoError(t, err) + require.NotContains(t, labeler.data, "") + require.Equal(t, map[string]map[string]string{ + "node-valid": { + topology.KeyTopologyAccelerator: "accelerator", + topology.FabricTierKey(0): "leaf", + }, + }, labeler.data) } diff --git a/pkg/engines/nfd/engine.go b/pkg/engines/nfd/engine.go index 293fb2a7..2befae74 100644 --- a/pkg/engines/nfd/engine.go +++ b/pkg/engines/nfd/engine.go @@ -120,7 +120,9 @@ func (eng *NfdEngine) GenerateOutput(ctx context.Context, graph *topology.Graph, } } - nodeLabels, err := k8sengine.NewTopologyLabeler().BuildNodeLabels(graph) + nodeLabels, err := k8sengine.NewTopologyLabeler( + k8sengine.NewTopologyLabelKeys(nil, ""), + ).BuildNodeLabels(graph) if err != nil { return nil, httperr.NewError(http.StatusBadGateway, err.Error()) } diff --git a/pkg/engines/nfd/engine_test.go b/pkg/engines/nfd/engine_test.go index d9330983..d20eb9c2 100644 --- a/pkg/engines/nfd/engine_test.go +++ b/pkg/engines/nfd/engine_test.go @@ -85,13 +85,6 @@ func TestGetParametersDefaults(t *testing.T) { } func TestGenerateOutputCreatesNodeFeaturesAndGroups(t *testing.T) { - k8sengine.InitLabels( - k8sengine.DefaultLabelAccelerator, - k8sengine.DefaultLabelLeaf, - k8sengine.DefaultLabelSpine, - k8sengine.DefaultLabelCore, - ) - client := k8sfake.NewSimpleClientset( &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node-a"}}, &corev1.Node{ObjectMeta: metav1.ObjectMeta{ @@ -128,17 +121,17 @@ 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{ - topologyTypeAccelerator: "nvl-a", - topologyTypeLeaf: "leaf-1", - topologyTypeSpine: "spine-1", + topologyTypeAccelerator: "nvl-a", + 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{ - topologyTypeAccelerator: "cluster.0", - topologyTypeLeaf: "leaf-1", - topologyTypeSpine: "spine-1", + topologyTypeAccelerator: "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{}) @@ -146,28 +139,21 @@ func TestGenerateOutputCreatesNodeFeaturesAndGroups(t *testing.T) { require.Len(t, groups.Items, 6) require.Equal(t, testNFDNamespace, groups.Items[0].GetNamespace()) - leafGroup := findGroup(t, groups.Items, topologyTypeLeaf, "leaf-1") - require.Equal(t, []interface{}{"leaf-1"}, groupRuleValues(t, leafGroup, topologyTypeLeaf)) + 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, topologyTypeAccelerator, "cluster.0") require.Equal(t, topology.KeyNvidiaGPUClique, cliqueGroup.GetAnnotations()[annotationTopologyLabelKey]) require.Equal(t, []interface{}{"cluster.0"}, groupRuleValues(t, cliqueGroup, topologyTypeAccelerator)) } func TestGenerateOutputCleansStaleObjects(t *testing.T) { - k8sengine.InitLabels( - k8sengine.DefaultLabelAccelerator, - k8sengine.DefaultLabelLeaf, - k8sengine.DefaultLabelSpine, - k8sengine.DefaultLabelCore, - ) - - staleFeature, err := makeNodeFeature("stale-node", map[string]string{topologyTypeLeaf: "stale-leaf"}) + staleFeature, err := makeNodeFeature("stale-node", map[string]string{topologyTypeFabric + "0": "stale-leaf"}) require.NoError(t, err) - staleGroup, err := makeNodeFeatureGroup(topologyTypeLeaf, "stale-leaf", k8sengine.DefaultLabelLeaf) + staleGroup, err := makeNodeFeatureGroup(topologyTypeFabric+"0", "stale-leaf", topology.FabricTierKey(0)) require.NoError(t, err) - retainedFeature, err := makeNodeFeature("node-a", map[string]string{topologyTypeLeaf: "old-leaf"}) + retainedFeature, err := makeNodeFeature("node-a", map[string]string{topologyTypeFabric + "0": "old-leaf"}) require.NoError(t, err) - retainedGroup, err := makeNodeFeatureGroup(topologyTypeLeaf, "leaf-1", k8sengine.DefaultLabelLeaf) + retainedGroup, err := makeNodeFeatureGroup(topologyTypeFabric+"0", "leaf-1", topology.FabricTierKey(0)) require.NoError(t, err) staleFeature.SetNamespace(testNFDNamespace) staleGroup.SetNamespace(testNFDNamespace) @@ -216,16 +202,9 @@ func TestGenerateOutputCleansStaleObjects(t *testing.T) { } func TestGenerateOutputRejectsEmptyDesiredStateWithCleanup(t *testing.T) { - k8sengine.InitLabels( - k8sengine.DefaultLabelAccelerator, - k8sengine.DefaultLabelLeaf, - k8sengine.DefaultLabelSpine, - k8sengine.DefaultLabelCore, - ) - - staleFeature, err := makeNodeFeature("stale-node", map[string]string{topologyTypeLeaf: "stale-leaf"}) + staleFeature, err := makeNodeFeature("stale-node", map[string]string{topologyTypeFabric + "0": "stale-leaf"}) require.NoError(t, err) - staleGroup, err := makeNodeFeatureGroup(topologyTypeLeaf, "stale-leaf", k8sengine.DefaultLabelLeaf) + staleGroup, err := makeNodeFeatureGroup(topologyTypeFabric+"0", "stale-leaf", topology.FabricTierKey(0)) require.NoError(t, err) staleFeature.SetNamespace(testNFDNamespace) staleGroup.SetNamespace(testNFDNamespace) @@ -273,8 +252,8 @@ func TestGenerateOutputAllowsEmptyDesiredStateWithoutCleanup(t *testing.T) { func TestUpsertObjectRemovesStaleTopologyAttributes(t *testing.T) { existing, err := makeNodeFeature("node-a", map[string]string{ - topologyTypeLeaf: "leaf-1", - topologyTypeCore: "stale-core", + topologyTypeFabric + "0": "leaf-1", + topologyTypeFabric + "2": "stale-core", }) require.NoError(t, err) existing.SetNamespace(testNFDNamespace) @@ -290,7 +269,7 @@ func TestUpsertObjectRemovesStaleTopologyAttributes(t *testing.T) { namespace: testNFDNamespace, } desired, err := makeNodeFeature("node-a", map[string]string{ - topologyTypeLeaf: "leaf-1", + topologyTypeFabric + "0": "leaf-1", }) require.NoError(t, err) @@ -300,22 +279,15 @@ func TestUpsertObjectRemovesStaleTopologyAttributes(t *testing.T) { Get(context.Background(), existing.GetName(), metav1.GetOptions{}) require.NoError(t, err) require.Equal(t, map[string]string{ - topologyTypeLeaf: "leaf-1", + topologyTypeFabric + "0": "leaf-1", }, attributeElements(t, *updated, nfdFeatureSet)) require.Equal(t, "true", updated.GetLabels()["example.com/retain"]) } func TestBuildNFDObjectsRejectsInvalidNFDNodeNameLabelValue(t *testing.T) { - k8sengine.InitLabels( - k8sengine.DefaultLabelAccelerator, - k8sengine.DefaultLabelLeaf, - k8sengine.DefaultLabelSpine, - k8sengine.DefaultLabelCore, - ) - nodeLabels := k8sengine.NodeLabelMap{ "node-name-that-is-too-long-for-a-kubernetes-label-value-because-it-has-more-than-sixty-three-characters": { - k8sengine.DefaultLabelLeaf: "leaf-1", + topology.FabricTierKey(0): "leaf-1", }, } @@ -323,6 +295,45 @@ func TestBuildNFDObjectsRejectsInvalidNFDNodeNameLabelValue(t *testing.T) { require.ErrorContains(t, err, "cannot be used as") } +func TestBuildNFDObjectsDoesNotGroupShadowedAcceleratorDomains(t *testing.T) { + nodeLabels := k8sengine.NodeLabelMap{ + "node-a": { + topology.KeyTopologyAccelerator: "provider-domain-a", + topology.FabricTierKey(0): "leaf-a", + }, + "node-b": { + topology.KeyTopologyAccelerator: "provider-domain-b", + }, + } + + features, groups, err := buildNFDObjects(nodeLabels, map[string]string{"node-a": "gpu-clique-a"}) + + require.NoError(t, err) + require.Len(t, features, 2) + require.Equal(t, "gpu-clique-a", attributeElements(t, *features[0], nfdFeatureSet)[topologyTypeAccelerator]) + require.Equal(t, "provider-domain-b", attributeElements(t, *features[1], nfdFeatureSet)[topologyTypeAccelerator]) + require.Len(t, groups, 3) + acceleratorValues := make(map[string]struct{}) + for _, group := range groups { + if group.GetLabels()[labelGroupType] == topologyTypeAccelerator { + acceleratorValues[group.GetAnnotations()[annotationTopologyValue]] = struct{}{} + } + } + require.Equal(t, map[string]struct{}{ + "gpu-clique-a": {}, + "provider-domain-b": {}, + }, acceleratorValues) +} + +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.KeyTopologyAccelerator) + require.True(t, ok) + require.Equal(t, topologyTypeAccelerator, kind) +} + func testGraph() *topology.Graph { domains := topology.NewDomainMap() domains.AddHost("nvl-a", "inst-a", "node-a") diff --git a/pkg/engines/nfd/objects.go b/pkg/engines/nfd/objects.go index 233d422b..61a23c95 100644 --- a/pkg/engines/nfd/objects.go +++ b/pkg/engines/nfd/objects.go @@ -13,6 +13,7 @@ import ( "fmt" "maps" "slices" + "strconv" "strings" "golang.org/x/sync/errgroup" @@ -40,10 +41,8 @@ const ( nfdNodeFeatureKind = "NodeFeature" nfdNodeFeatureGroupKind = "NodeFeatureGroup" + topologyTypeFabric = "fabric-tier-" topologyTypeAccelerator = "accelerator" - topologyTypeLeaf = "leaf" - topologyTypeSpine = "spine" - topologyTypeCore = "core" labelNFDNodeName = "nfd.node.kubernetes.io/node-name" labelManagedBy = "app.kubernetes.io/managed-by" @@ -75,16 +74,6 @@ 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) { - keys := k8sengine.CurrentTopologyLabelKeys() - configuredLabels := [...]struct { - kind string - key string - }{ - {kind: topologyTypeAccelerator, key: keys.Accelerator}, - {kind: topologyTypeLeaf, key: keys.Leaf}, - {kind: topologyTypeSpine, key: keys.Spine}, - {kind: topologyTypeCore, key: keys.Core}, - } groupValues := make(map[string]map[string]string) nodeFeatures := make([]*unstructured.Unstructured, 0, len(nodeLabels)) @@ -95,31 +84,34 @@ func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[stri } labels := nodeLabels[nodeName] - elements := make(map[string]string, len(configuredLabels)) - for _, label := range configuredLabels { - labelKey := label.key - value := "" - if label.kind == topologyTypeAccelerator { - if gpuCliqueValue := gpuCliqueValues[nodeName]; gpuCliqueValue != "" { - labelKey = topology.KeyNvidiaGPUClique - value = gpuCliqueValue - } - } - if labelKey == "" { + gpuCliqueValue := strings.TrimSpace(gpuCliqueValues[nodeName]) + elements := make(map[string]string, len(labels)) + for _, labelKey := range slices.Sorted(maps.Keys(labels)) { + kind, ok := topologyKind(labelKey) + if !ok { continue } - if value == "" { - value = strings.TrimSpace(labels[labelKey]) + if kind == topologyTypeAccelerator && gpuCliqueValue != "" { + continue } + value := strings.TrimSpace(labels[labelKey]) if value == "" { continue } - elements[label.kind] = value - if _, ok := groupValues[label.kind]; !ok { - groupValues[label.kind] = make(map[string]string) + elements[kind] = value + if _, ok := groupValues[kind]; !ok { + groupValues[kind] = make(map[string]string) } - groupValues[label.kind][value] = labelKey + groupValues[kind][value] = labelKey + } + if gpuCliqueValue != "" { + kind := topologyTypeAccelerator + elements[kind] = gpuCliqueValue + if _, ok := groupValues[kind]; !ok { + groupValues[kind] = make(map[string]string) + } + groupValues[kind][gpuCliqueValue] = topology.KeyNvidiaGPUClique } if len(elements) == 0 { continue @@ -133,11 +125,8 @@ func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[stri } nodeFeatureGroups := make([]*unstructured.Unstructured, 0) - for _, kind := range topologyTypeOrder { - valuesByLabelKey, ok := groupValues[kind] - if !ok { - continue - } + for _, kind := range slices.Sorted(maps.Keys(groupValues)) { + valuesByLabelKey := groupValues[kind] for _, value := range slices.Sorted(maps.Keys(valuesByLabelKey)) { nodeFeatureGroup, err := makeNodeFeatureGroup(kind, value, valuesByLabelKey[value]) if err != nil { @@ -150,6 +139,19 @@ func buildNFDObjects(nodeLabels k8sengine.NodeLabelMap, gpuCliqueValues map[stri return nodeFeatures, nodeFeatureGroups, nil } +func topologyKind(labelKey string) (string, bool) { + if labelKey == topology.KeyTopologyAccelerator { + return topologyTypeAccelerator, true + } + if strings.HasPrefix(labelKey, topology.KeyFabricTierPrefix) { + level := strings.TrimPrefix(labelKey, topology.KeyFabricTierPrefix) + if _, err := strconv.Atoi(level); err == nil { + return topologyTypeFabric + level, true + } + } + return "", false +} + // makeNodeFeature builds the NFD feature data published for one node. func makeNodeFeature(nodeName string, elements map[string]string) (*unstructured.Unstructured, error) { obj := &nfdv1alpha1.NodeFeature{ @@ -369,13 +371,6 @@ func objectNames(objects []*unstructured.Unstructured) map[string]struct{} { return names } -var topologyTypeOrder = []string{ - topologyTypeAccelerator, - topologyTypeLeaf, - topologyTypeSpine, - topologyTypeCore, -} - // stableObjectName produces a deterministic DNS label from a prefix and value. func stableObjectName(prefix, value string) string { prefix = dnsLabelPart(prefix) diff --git a/pkg/engines/slinky/engine.go b/pkg/engines/slinky/engine.go index 549bf465..42e363de 100644 --- a/pkg/engines/slinky/engine.go +++ b/pkg/engines/slinky/engine.go @@ -335,12 +335,12 @@ func withGPUCliqueDomains(graph *topology.Graph, clusterNodes *clusterNodes) (*t } if graph == nil { - graph = &topology.Graph{} + graph = &topology.Graph{Domains: domains} } else { cloned := *graph + cloned.Domains = domains graph = &cloned } - graph.Domains = domains return graph, nil } diff --git a/pkg/models/model.go b/pkg/models/model.go index 16358780..cb09022b 100644 --- a/pkg/models/model.go +++ b/pkg/models/model.go @@ -32,7 +32,6 @@ import ( const ( LabelTopologyRegion = "topology.kubernetes.io/region" LabelTopologyZone = "topology.kubernetes.io/zone" - LabelAccelerator = topology.KeyTopologyAccelerator ) // Switch is a switch vertex in a simulation model YAML tree (tests/models). @@ -279,7 +278,7 @@ func addInstanceRegion(regions map[string]map[string]string, labels map[string]s r = make(map[string]string) regions[region] = r } - r[fmt.Sprintf("i-%s", hostName)] = hostName + r[getInstanceID(hostName)] = hostName } func (m *Model) setInstances(regions map[string]map[string]string) { @@ -332,12 +331,12 @@ func (model *Model) ToGraph(instances []topology.ComputeInstances) (*topology.Gr domainMap := topology.NewDomainMap() for hostName := range model.Nodes { - instance2node[fmt.Sprintf("i-%s", hostName)] = hostName + instance2node[getInstanceID(hostName)] = hostName } // Create all the vertices for each node. for hostName := range model.Nodes { - instanceID := fmt.Sprintf("i-%s", hostName) + instanceID := getInstanceID(hostName) nodeVertexMap[hostName] = &topology.Vertex{ ID: instanceID, Name: instance2node[instanceID], @@ -350,11 +349,11 @@ func (model *Model) ToGraph(instances []topology.ComputeInstances) (*topology.Gr swRootMap[sw.Name] = true } - // Initializes accelerator domain membership from node labels. + // Initializes accelerator-network membership. for hostName, instance := range model.Nodes { - if accelerator := instance.AcceleratorID(); accelerator != "" { - instanceID := fmt.Sprintf("i-%s", hostName) - domainMap.AddHost(accelerator, instanceID, instance2node[instanceID]) + if domain := instance.AcceleratorID(); domain != "" { + instanceID := getInstanceID(hostName) + domainMap.AddHost(domain, instanceID, instance2node[instanceID]) } } @@ -377,10 +376,7 @@ func (model *Model) ToGraph(instances []topology.ComputeInstances) (*topology.Gr treeRoot.Vertices[k] = swVertexMap[k] } } - graph := &topology.Graph{ - Tiers: treeRoot, - } - + graph := &topology.Graph{Tiers: treeRoot} if len(domainMap) != 0 { graph.Domains = domainMap } @@ -396,7 +392,7 @@ func (model *Model) graphInstanceMap(computeInstances []topology.ComputeInstance instances := make(map[string]topology.Instance) for hostName, inst := range model.Nodes { - instanceID := fmt.Sprintf("i-%s", hostName) + instanceID := getInstanceID(hostName) if len(wanted) != 0 { if _, ok := wanted[instanceID]; !ok { continue @@ -433,3 +429,7 @@ func requestedInstanceIDs(computeInstances []topology.ComputeInstances) map[stri } return ids } + +func getInstanceID(hostName string) string { + return fmt.Sprintf("i-%s", hostName) +} diff --git a/pkg/models/model_test.go b/pkg/models/model_test.go index de8307a0..b0897185 100644 --- a/pkg/models/model_test.go +++ b/pkg/models/model_test.go @@ -22,13 +22,14 @@ import ( "github.com/stretchr/testify/require" "github.com/NVIDIA/topograph/pkg/topology" + "github.com/NVIDIA/topograph/pkg/translate" ) -func acceleratorLabels(accelerator string) map[string]string { - if accelerator == "" { +func acceleratorDomainLabels(domain string) map[string]string { + if domain == "" { return nil } - return map[string]string{LabelAccelerator: accelerator} + return map[string]string{topology.KeyTopologyAccelerator: domain} } func TestNewModelFromFileMedium(t *testing.T) { @@ -44,31 +45,31 @@ func TestNewModelFromFileMedium(t *testing.T) { { Switch: "sw11", Nodes: []string{"1101", "1102"}, - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, { Switch: "sw12", Nodes: []string{"1201", "1202"}, - Labels: acceleratorLabels("nvl2"), + Labels: acceleratorDomainLabels("nvl2"), }, { Switch: "sw13", Nodes: []string{"1301", "1302"}, - Labels: acceleratorLabels("nvl3"), + Labels: acceleratorDomainLabels("nvl3"), }, { Switch: "sw14", Nodes: []string{"1401", "1402"}, - Labels: acceleratorLabels("nvl4"), + Labels: acceleratorDomainLabels("nvl4"), }, }, cfg.CapacityBlocks) require.Equal(t, &topology.Instance{ ID: "1101", Labels: map[string]string{ - LabelTopologyRegion: "us-west", - LabelTopologyZone: "zone1", - LabelAccelerator: "nvl1", + LabelTopologyRegion: "us-west", + LabelTopologyZone: "zone1", + topology.KeyTopologyAccelerator: "nvl1", }, NetLayers: []string{"sw11", "sw21", "sw3"}, }, cfg.Nodes["1101"]) @@ -101,9 +102,9 @@ func TestNewModelFromFileNVL72(t *testing.T) { require.Equal(t, &topology.Instance{ ID: "node2215", Labels: map[string]string{ - LabelTopologyRegion: "us-east", - LabelTopologyZone: "zone1", - LabelAccelerator: "nvl-2-2", + LabelTopologyRegion: "us-east", + LabelTopologyZone: "zone1", + topology.KeyTopologyAccelerator: "nvl-2-2", }, NetLayers: []string{"leaf-2-2", "spine-2", "core"}, }, cfg.Nodes["node2215"]) @@ -144,29 +145,29 @@ blocks: "n1": { ID: "n1", NetLayers: []string{"leaf", "core"}, - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, "n2": { ID: "n2", NetLayers: []string{"leaf", "core"}, - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, "n3": { ID: "n3", NetLayers: []string{"leaf", "core"}, - Labels: acceleratorLabels("nvl2"), + Labels: acceleratorDomainLabels("nvl2"), }, }, CapacityBlocks: []CapacityBlock{ { Switch: "leaf", Nodes: []string{"n1", "n2"}, - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, { Switch: "leaf", Nodes: []string{"n3"}, - Labels: acceleratorLabels("nvl2"), + Labels: acceleratorDomainLabels("nvl2"), }, }, Instances: []topology.ComputeInstances{ @@ -189,17 +190,17 @@ blocks: Nodes: map[string]*topology.Instance{ "n1": { ID: "n1", - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, "n2": { ID: "n2", - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, }, CapacityBlocks: []CapacityBlock{ { Nodes: []string{"n1", "n2"}, - Labels: acceleratorLabels("nvl1"), + Labels: acceleratorDomainLabels("nvl1"), }, }, Instances: []topology.ComputeInstances{ @@ -343,4 +344,8 @@ blocks: require.Equal(t, "node1", instance2node["i-node1"]) require.Equal(t, "node1", graph.Tiers.Vertices["leaf"].Vertices["i-node1"].Name) require.Contains(t, graph.Instances, "i-node1") + require.Nil(t, graph.Domains) + + _, err = translate.NewNetworkTopology(graph, &translate.Config{Plugin: topology.TopologyBlock}) + require.EqualError(t, err, "missing block topology") } diff --git a/pkg/providers/aws/instance_topology.go b/pkg/providers/aws/instance_topology.go index 98fa34e9..55c5819e 100644 --- a/pkg/providers/aws/instance_topology.go +++ b/pkg/providers/aws/instance_topology.go @@ -102,10 +102,8 @@ func (p *baseProvider) generateRegionInstanceTopology(ctx context.Context, pageS func convert(inst *types.InstanceTopology) *topology.InstanceTopology { topo := &topology.InstanceTopology{ - InstanceID: *inst.InstanceId, - LeafID: inst.NetworkNodes[2], - SpineID: inst.NetworkNodes[1], - CoreID: inst.NetworkNodes[0], + InstanceID: *inst.InstanceId, + FabricTiers: topology.RootFirstFabricTiers(inst.NetworkNodes...), } if inst.CapacityBlockId != nil { topo.AcceleratorID = *inst.CapacityBlockId diff --git a/pkg/providers/aws/provider.go b/pkg/providers/aws/provider.go index fe2f9f44..3ef0dbc5 100644 --- a/pkg/providers/aws/provider.go +++ b/pkg/providers/aws/provider.go @@ -188,7 +188,7 @@ func (p *baseProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int klog.Infof("Extracted topology for %d instances", topo.Len()) - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, false), nil + return topo.ToGraph(NAME, instances, p.trimTiers, false), nil } type Provider struct { diff --git a/pkg/providers/aws/provider_sim.go b/pkg/providers/aws/provider_sim.go index b620760c..d7c2e9e0 100644 --- a/pkg/providers/aws/provider_sim.go +++ b/pkg/providers/aws/provider_sim.go @@ -104,12 +104,12 @@ func (client *simClient) DescribeInstanceTopology(ctx context.Context, params *e for j := len(node.NetLayers) - 1; j >= 0; j-- { netLayers = append(netLayers, node.NetLayers[j]) } - acceleratorID := node.AcceleratorID() + domainID := node.Labels[topology.KeyTopologyAccelerator] instTopo := types.InstanceTopology{ InstanceId: &instanceId, AvailabilityZone: &az, ZoneId: &az, - CapacityBlockId: &acceleratorID, + CapacityBlockId: &domainID, NetworkNodes: netLayers, } instances = append(instances, instTopo) @@ -200,5 +200,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, false), nil + return p.ToGraph(NAME_SIM, topo, instances, false), nil } diff --git a/pkg/providers/dsx/instance_topology.go b/pkg/providers/dsx/instance_topology.go index 2bd05ce4..56c8c8b4 100644 --- a/pkg/providers/dsx/instance_topology.go +++ b/pkg/providers/dsx/instance_topology.go @@ -42,7 +42,7 @@ func (p *baseProvider) generateInstanceTopology(ctx context.Context, pageSize *i return responseToClusterTopology(response, cis), nil } -// responseToClusterTopology maps switch/node API output to per-instance records for ToThreeTierGraph. +// responseToClusterTopology maps switch/node API output to per-instance records for ToGraph. func responseToClusterTopology(response *TopologyResponse, cis []topology.ComputeInstances) *topology.ClusterTopology { want := make(map[string]struct{}) for _, ci := range cis { @@ -73,11 +73,11 @@ func responseToClusterTopology(response *TopologyResponse, cis []topology.Comput //create the instance topology inst := &topology.InstanceTopology{ - InstanceID: n.NodeID, - LeafID: leafID, - SpineID: spineID, - CoreID: coreID, - AcceleratorID: n.AcceleratedNetworkID, + InstanceID: n.NodeID, + FabricTiers: topology.ClosestFirstFabricTiers(leafID, spineID, coreID), + } + if n.AcceleratedNetworkID != "" { + inst.AcceleratorID = n.AcceleratedNetworkID } klog.V(4).Infof("Adding instance topology %s", inst.String()) topo.Append(inst) diff --git a/pkg/providers/dsx/provider.go b/pkg/providers/dsx/provider.go index 36daf419..a46db1b7 100644 --- a/pkg/providers/dsx/provider.go +++ b/pkg/providers/dsx/provider.go @@ -60,7 +60,7 @@ func (p *baseProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int klog.Infof("Extracted topology for %d instances", topo.Len()) - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, false), nil + return topo.ToGraph(NAME, instances, p.trimTiers, false), nil } type Provider struct { diff --git a/pkg/providers/dsx/provider_sim.go b/pkg/providers/dsx/provider_sim.go index 22d2a0d6..2c7a5c3c 100644 --- a/pkg/providers/dsx/provider_sim.go +++ b/pkg/providers/dsx/provider_sim.go @@ -61,7 +61,7 @@ func (client *simClient) GetTopology(ctx context.Context, _ string, nodeIDs []st if !exists { continue } - swInfo.Nodes = append(swInfo.Nodes, NodeInfo{NodeID: nodeName, AcceleratedNetworkID: node.AcceleratorID()}) + swInfo.Nodes = append(swInfo.Nodes, NodeInfo{NodeID: nodeName, AcceleratedNetworkID: node.Labels[topology.KeyTopologyAccelerator]}) } } else { //If it is not a leaf switch, add the child switches to the switch info @@ -125,5 +125,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, false), nil + return p.ToGraph(NAME_SIM, topo, instances, false), nil } diff --git a/pkg/providers/dsx/provider_sim_test.go b/pkg/providers/dsx/provider_sim_test.go index 98e224da..bd607d18 100644 --- a/pkg/providers/dsx/provider_sim_test.go +++ b/pkg/providers/dsx/provider_sim_test.go @@ -360,13 +360,27 @@ func TestProviderSimWithNVLink(t *testing.T) { require.Nil(t, httpErr) require.NotNil(t, topo) - // NVLink / accelerator domain is emitted as graph domains (same as AWS ToThreeTierGraph path). + // NVLink / accelerator domain is emitted as graph domains (same as AWS ToGraph path). domains := topo.Domains require.NotNil(t, domains) _, hasNVL := domains["nvl1"] require.True(t, hasNVL, "cluster model places cb1 nodes in NVLink domain nvl1 under topology/block") } +func TestResponseToClusterTopologyOmitsEmptyAccelerator(t *testing.T) { + response := &TopologyResponse{Switches: map[string]SwitchInfo{ + "leaf": {Nodes: []NodeInfo{{NodeID: "n1"}}}, + }} + instances := []topology.ComputeInstances{{ + Instances: map[string]string{"n1": "node1"}, + }} + + topo := responseToClusterTopology(response, instances) + + require.Len(t, topo.Instances, 1) + require.Empty(t, topo.Instances[0].AcceleratorID) +} + func TestLoaderSimMissingModelFile(t *testing.T) { ctx := context.Background() diff --git a/pkg/providers/gcp/instance_topology.go b/pkg/providers/gcp/instance_topology.go index c984c2a2..dc475f0d 100644 --- a/pkg/providers/gcp/instance_topology.go +++ b/pkg/providers/gcp/instance_topology.go @@ -97,11 +97,13 @@ func (p *baseProvider) generateRegionInstanceTopology(ctx context.Context, clien } inst := &topology.InstanceTopology{ InstanceID: instanceId, - CoreID: instance.ResourceStatus.PhysicalHostTopology.GetCluster(), - SpineID: instance.ResourceStatus.PhysicalHostTopology.GetBlock(), - LeafID: instance.ResourceStatus.PhysicalHostTopology.GetSubblock(), + FabricTiers: topology.ClosestFirstFabricTiers( + instance.ResourceStatus.PhysicalHostTopology.GetSubblock(), + instance.ResourceStatus.PhysicalHostTopology.GetBlock(), + instance.ResourceStatus.PhysicalHostTopology.GetCluster(), + ), } - inst.AcceleratorID = inst.LeafID + inst.AcceleratorID = inst.FabricTiers[0].ID klog.Infof("Adding topology: %s", inst.String()) topo.Append(inst) } diff --git a/pkg/providers/gcp/provider.go b/pkg/providers/gcp/provider.go index f6d173c2..cb414d74 100644 --- a/pkg/providers/gcp/provider.go +++ b/pkg/providers/gcp/provider.go @@ -165,7 +165,7 @@ func (p *baseProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int return nil, err } - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, false), nil + return topo.ToGraph(NAME, instances, p.trimTiers, false), nil } type Provider struct { diff --git a/pkg/providers/gcp/provider_sim.go b/pkg/providers/gcp/provider_sim.go index 413dd44d..09a9c8c5 100644 --- a/pkg/providers/gcp/provider_sim.go +++ b/pkg/providers/gcp/provider_sim.go @@ -184,5 +184,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, false), nil + return p.ToGraph(NAME_SIM, topo, instances, false), nil } diff --git a/pkg/providers/lambdai/instance_topology.go b/pkg/providers/lambdai/instance_topology.go index da8aced3..6bd3ae22 100644 --- a/pkg/providers/lambdai/instance_topology.go +++ b/pkg/providers/lambdai/instance_topology.go @@ -52,17 +52,8 @@ func (p *baseProvider) generateRegionInstanceTopology(ctx context.Context, clien InstanceID: inst.ID, } - for indx := range len(inst.NetworkPath) { - switch indx { - case 0: - t.LeafID = inst.NetworkPath[indx].ID - case 1: - t.SpineID = inst.NetworkPath[indx].ID - case 2: - t.CoreID = inst.NetworkPath[indx].ID - default: - klog.Warningf("unsupported size %d of topology path for instance %q", len(inst.NetworkPath), inst.ID) - } + for _, hop := range inst.NetworkPath { + t.FabricTiers = append(t.FabricTiers, topology.FabricTier{ID: hop.ID}) } if inst.NVLink != nil { diff --git a/pkg/providers/lambdai/provider.go b/pkg/providers/lambdai/provider.go index 49b83ddd..8f2fd247 100644 --- a/pkg/providers/lambdai/provider.go +++ b/pkg/providers/lambdai/provider.go @@ -212,7 +212,7 @@ func (p *baseProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int return nil, err } - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, false), nil + return topo.ToGraph(NAME, instances, p.trimTiers, false), nil } type Provider struct { diff --git a/pkg/providers/lambdai/provider_sim.go b/pkg/providers/lambdai/provider_sim.go index 48377755..de2c8db4 100644 --- a/pkg/providers/lambdai/provider_sim.go +++ b/pkg/providers/lambdai/provider_sim.go @@ -65,7 +65,7 @@ func (c *simClient) InstanceList(ctx context.Context, req *InstanceListRequest) NetworkPath: netPath, //TODO: check whether the below mapping is correct NVLink: &NVLinkInfo{ - DomainID: node.AcceleratorID(), + DomainID: node.Labels[topology.KeyTopologyAccelerator], CliqueID: "simulation", }, } @@ -151,5 +151,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, false), nil + return p.ToGraph(NAME_SIM, topo, instances, false), nil } diff --git a/pkg/providers/nebius/instance_topology.go b/pkg/providers/nebius/instance_topology.go index 458182b5..9e066bbb 100644 --- a/pkg/providers/nebius/instance_topology.go +++ b/pkg/providers/nebius/instance_topology.go @@ -17,6 +17,8 @@ import ( "github.com/NVIDIA/topograph/pkg/topology" ) +const nebiusFabricTierCount = 3 + func (p *baseProvider) generateInstanceTopology(ctx context.Context, pageSize *int, cis []topology.ComputeInstances) (*topology.ClusterTopology, *httperr.Error) { client, err := p.clientFactory(pageSize) if err != nil { @@ -70,15 +72,11 @@ func (p *baseProvider) generateRegionInstanceTopology(ctx context.Context, clien } path := ibTopology.GetPath() - switch len(path) { - case 3: - inst.CoreID = path[0] - inst.SpineID = path[1] - inst.LeafID = path[2] - default: - klog.Warningf("unsupported size %d of topology path for node %q", len(path), hostname) + if len(path) != nebiusFabricTierCount { + klog.Warningf("invalid topology path for node %q: expected %d tiers, got %d", hostname, nebiusFabricTierCount, len(path)) continue } + inst.FabricTiers = topology.RootFirstFabricTiers(path...) klog.Infof("Adding topology: %s", inst.String()) topo.Append(inst) diff --git a/pkg/providers/nebius/instance_topology_test.go b/pkg/providers/nebius/instance_topology_test.go new file mode 100644 index 00000000..6c30cc9c --- /dev/null +++ b/pkg/providers/nebius/instance_topology_test.go @@ -0,0 +1,66 @@ +/* + * Copyright 2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 + */ + +package nebius + +import ( + "context" + "fmt" + "testing" + + common "github.com/nebius/gosdk/proto/nebius/common/v1" + compute "github.com/nebius/gosdk/proto/nebius/compute/v1" + "github.com/stretchr/testify/require" + + "github.com/NVIDIA/topograph/pkg/topology" +) + +type instanceTopologyClient struct { + response *compute.ListInstancesResponse +} + +func (c *instanceTopologyClient) ProjectID() string { + return "project" +} + +func (c *instanceTopologyClient) PageSize() int64 { + return 100 +} + +func (c *instanceTopologyClient) GetComputeInstanceList(context.Context, *compute.ListInstancesRequest) (*compute.ListInstancesResponse, error) { + return c.response, nil +} + +func TestGenerateRegionInstanceTopologyRequiresThreeTiers(t *testing.T) { + for tierCount := 0; tierCount <= 4; tierCount++ { + t.Run(fmt.Sprintf("%d tiers", tierCount), func(t *testing.T) { + path := []string{"root", "middle", "leaf", "extra"}[:tierCount] + client := &instanceTopologyClient{response: &compute.ListInstancesResponse{ + Items: []*compute.Instance{{ + Metadata: &common.ResourceMetadata{Id: "instance-1"}, + Status: &compute.InstanceStatus{ + GpuClusterTopology: &compute.InstanceStatus_InfinibandTopologyPath{ + InfinibandTopologyPath: &compute.InstanceStatusInfinibandTopologyPath{Path: path}, + }, + }, + }}, + }} + cluster := topology.NewClusterTopology() + ci := &topology.ComputeInstances{ + Region: "region", + Instances: map[string]string{"instance-1": "node-1"}, + } + + httpErr := (&baseProvider{}).generateRegionInstanceTopology(context.Background(), client, cluster, ci) + + require.Nil(t, httpErr) + if tierCount != nebiusFabricTierCount { + require.Empty(t, cluster.Instances) + return + } + require.Equal(t, topology.ClosestFirstFabricTiers("leaf", "middle", "root"), cluster.Instances[0].FabricTiers) + }) + } +} diff --git a/pkg/providers/nebius/provider.go b/pkg/providers/nebius/provider.go index c9bacf2c..f40e2df9 100644 --- a/pkg/providers/nebius/provider.go +++ b/pkg/providers/nebius/provider.go @@ -201,7 +201,7 @@ func (p *baseProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int return nil, err } - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, false), nil + return topo.ToGraph(NAME, instances, p.trimTiers, false), nil } type Provider struct { diff --git a/pkg/providers/nebius/provider_sim.go b/pkg/providers/nebius/provider_sim.go index 6d093b24..754818dd 100644 --- a/pkg/providers/nebius/provider_sim.go +++ b/pkg/providers/nebius/provider_sim.go @@ -60,7 +60,7 @@ func (c *simClient) GetComputeInstanceList(ctx context.Context, req *compute.Lis node := c.model.Nodes[c.instanceIDs[indx]] instance := &compute.Instance{ Spec: &compute.InstanceSpec{ - NvlInstanceGroupId: node.AcceleratorID(), + NvlInstanceGroupId: node.Labels[topology.KeyTopologyAccelerator], }, Status: &compute.InstanceStatus{}, } @@ -74,7 +74,10 @@ func (c *simClient) GetComputeInstanceList(ctx context.Context, req *compute.Lis if c.apiErr == errTopologyPath { path = []string{} } else { - path = []string{node.NetLayers[2], node.NetLayers[1], node.NetLayers[0]} + path = make([]string, len(node.NetLayers)) + for i, layer := range node.NetLayers { + path[len(node.NetLayers)-i-1] = layer + } } instance.Status.GpuClusterTopology = &compute.InstanceStatus_InfinibandTopologyPath{ InfinibandTopologyPath: &compute.InstanceStatusInfinibandTopologyPath{ @@ -164,5 +167,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, false), nil + return p.ToGraph(NAME_SIM, topo, instances, false), nil } diff --git a/pkg/providers/nscale/instance_topology.go b/pkg/providers/nscale/instance_topology.go index 4e68aafc..1137e2c0 100644 --- a/pkg/providers/nscale/instance_topology.go +++ b/pkg/providers/nscale/instance_topology.go @@ -62,18 +62,7 @@ func (p *baseProvider) generateRegionInstanceTopology(ctx context.Context, topo InstanceID: inst.ID, } - for indx := range minPathSize(inst.NetworkPath) { - switch indx { - case 0: - t.CoreID = inst.NetworkPath[indx] - case 1: - t.SpineID = inst.NetworkPath[indx] - case 2: - t.LeafID = inst.NetworkPath[indx] - default: - klog.Warningf("unsupported size %d of topology path for instance %q", len(inst.NetworkPath), inst.ID) - } - } + t.FabricTiers = topology.RootFirstFabricTiers(inst.NetworkPath...) if inst.BlockID != nil { t.AcceleratorID = *inst.BlockID @@ -84,12 +73,3 @@ func (p *baseProvider) generateRegionInstanceTopology(ctx context.Context, topo } } } - -func minPathSize(path []string) int { - n := len(path) - if n > 3 { - // return one extra index to print warning - return 4 - } - return n -} diff --git a/pkg/providers/nscale/provider.go b/pkg/providers/nscale/provider.go index a7417a65..44a0156a 100644 --- a/pkg/providers/nscale/provider.go +++ b/pkg/providers/nscale/provider.go @@ -203,7 +203,7 @@ func (p *baseProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int return nil, err } - return topo.ToThreeTierGraph(NAME, instances, p.params.TrimTiers, false), nil + return topo.ToGraph(NAME, instances, p.params.TrimTiers, false), nil } // Instances2NodeMap implements slurm.instanceMapper diff --git a/pkg/providers/nscale/provider_sim.go b/pkg/providers/nscale/provider_sim.go index 75002ef0..310d788d 100644 --- a/pkg/providers/nscale/provider_sim.go +++ b/pkg/providers/nscale/provider_sim.go @@ -56,9 +56,9 @@ func (c *simClient) Topology(ctx context.Context, _ string, pageSize, offset int ID: node.ID, NetworkPath: path, } - acceleratorID := node.AcceleratorID() - if acceleratorID != "" { - instance.BlockID = &acceleratorID + domainID := node.Labels[topology.KeyTopologyAccelerator] + if domainID != "" { + instance.BlockID = &domainID } resp = append(resp, instance) @@ -116,5 +116,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, false), nil + return p.ToGraph(NAME_SIM, topo, instances, false), nil } diff --git a/pkg/providers/oci/instance_topology.go b/pkg/providers/oci/instance_topology.go index 1dcf3e68..45769571 100644 --- a/pkg/providers/oci/instance_topology.go +++ b/pkg/providers/oci/instance_topology.go @@ -88,9 +88,9 @@ func convert(host *core.ComputeHostSummary) (*topology.InstanceTopology, error) topo := &topology.InstanceTopology{ InstanceID: *host.InstanceId, - LeafID: *host.LocalBlockId, - SpineID: *host.NetworkBlockId, - CoreID: *host.HpcIslandId, + FabricTiers: topology.ClosestFirstFabricTiers( + *host.LocalBlockId, *host.NetworkBlockId, *host.HpcIslandId, + ), } if host.GpuMemoryFabricId != nil { diff --git a/pkg/providers/oci/instance_topology_test.go b/pkg/providers/oci/instance_topology_test.go index 886135d8..7a1a19cd 100644 --- a/pkg/providers/oci/instance_topology_test.go +++ b/pkg/providers/oci/instance_topology_test.go @@ -26,11 +26,10 @@ import ( ) func TestConvert(t *testing.T) { + leaf, spine, root := "leaf", "net", "core" valid := &topology.InstanceTopology{ - InstanceID: "id", - LeafID: "leaf", - SpineID: "net", - CoreID: "core", + InstanceID: "id", + FabricTiers: topology.ClosestFirstFabricTiers(leaf, spine, root), } testCases := []struct { @@ -55,7 +54,7 @@ func TestConvert(t *testing.T) { name: "Case 3: missing NetworkBlockId", host: &core.ComputeHostSummary{ InstanceId: &valid.InstanceID, - LocalBlockId: &valid.LeafID, + LocalBlockId: &leaf, }, err: `missing NetworkBlockId for instance "id"`, }, @@ -63,8 +62,8 @@ func TestConvert(t *testing.T) { name: "Case 4: missing HpcIslandId", host: &core.ComputeHostSummary{ InstanceId: &valid.InstanceID, - LocalBlockId: &valid.LeafID, - NetworkBlockId: &valid.SpineID, + LocalBlockId: &leaf, + NetworkBlockId: &spine, }, err: `missing HpcIslandId for instance "id"`, }, @@ -72,9 +71,9 @@ func TestConvert(t *testing.T) { name: "Case 5: valid input", host: &core.ComputeHostSummary{ InstanceId: &valid.InstanceID, - LocalBlockId: &valid.LeafID, - NetworkBlockId: &valid.SpineID, - HpcIslandId: &valid.CoreID, + LocalBlockId: &leaf, + NetworkBlockId: &spine, + HpcIslandId: &root, }, topo: valid, }, diff --git a/pkg/providers/oci/provider_api.go b/pkg/providers/oci/provider_api.go index 0a55ca53..a433b26a 100644 --- a/pkg/providers/oci/provider_api.go +++ b/pkg/providers/oci/provider_api.go @@ -186,7 +186,7 @@ func (p *apiProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, return nil, err } - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, true), nil + return topo.ToGraph(NAME, instances, p.trimTiers, true), nil } func (p *apiProvider) generateInstanceTopology(ctx context.Context, pageSize *int, cis []topology.ComputeInstances) (*topology.ClusterTopology, *httperr.Error) { diff --git a/pkg/providers/oci/provider_imds.go b/pkg/providers/oci/provider_imds.go index 118f9941..b32e4614 100644 --- a/pkg/providers/oci/provider_imds.go +++ b/pkg/providers/oci/provider_imds.go @@ -52,7 +52,7 @@ func (p *imdsProvider) GenerateTopologyConfig(ctx context.Context, _ *int, insta return nil, httperr.NewError(http.StatusInternalServerError, err.Error()) } - return topo.ToThreeTierGraph(NAME, instances, p.trimTiers, true), nil + return topo.ToGraph(NAME, instances, p.trimTiers, true), nil } func (p *imdsProvider) generateInstanceTopology(ctx context.Context, cis []topology.ComputeInstances) (*topology.ClusterTopology, error) { @@ -82,9 +82,7 @@ func (p *imdsProvider) getComputeHostInfo(ctx context.Context, ci topology.Compu if nodeTopology, ok := topoMap[node]; ok { topo.Instances = append(topo.Instances, &topology.InstanceTopology{ InstanceID: instanceID, - LeafID: nodeTopology.LocalBlock, - SpineID: nodeTopology.NetworkBlock, - CoreID: nodeTopology.HPCIslandId, + FabricTiers: topology.ClosestFirstFabricTiers(nodeTopology.LocalBlock, nodeTopology.NetworkBlock, nodeTopology.HPCIslandId), AcceleratorID: nodeTopology.GpuMemoryFabric, }) } diff --git a/pkg/providers/oci/provider_sim.go b/pkg/providers/oci/provider_sim.go index db26d5cd..95e1c84b 100644 --- a/pkg/providers/oci/provider_sim.go +++ b/pkg/providers/oci/provider_sim.go @@ -99,9 +99,9 @@ func (c *simClient) ListComputeHosts(ctx context.Context, req core.ListComputeHo host.HpcIslandId = ptr.String(node.NetLayers[i]) } } - acceleratorID := node.AcceleratorID() - if acceleratorID != "" { - host.GpuMemoryFabricId = &acceleratorID + domainID := node.Labels[topology.KeyTopologyAccelerator] + if domainID != "" { + host.GpuMemoryFabricId = &domainID } resp.Items = append(resp.Items, host) } @@ -182,5 +182,5 @@ func (p *simProvider) GenerateTopologyConfig(ctx context.Context, pageSize *int, if err != nil { return nil, err } - return p.ToThreeTierGraph(NAME_SIM, topo, instances, true), nil + return p.ToGraph(NAME_SIM, topo, instances, true), nil } diff --git a/pkg/providers/providers_sim.go b/pkg/providers/providers_sim.go index 29aae4da..846928a3 100644 --- a/pkg/providers/providers_sim.go +++ b/pkg/providers/providers_sim.go @@ -81,10 +81,10 @@ func (p *BaseSimProvider) AttachInstances(topo *topology.ClusterTopology) { topo.AttachInstances(p.instances) } -// ToThreeTierGraph converts provider topology with the shared simulation settings. -func (p *BaseSimProvider) ToThreeTierGraph(provider string, topo *topology.ClusterTopology, instances []topology.ComputeInstances, normalize bool) *topology.Graph { +// ToGraph converts provider topology with the shared simulation settings. +func (p *BaseSimProvider) ToGraph(provider string, topo *topology.ClusterTopology, instances []topology.ComputeInstances, normalize bool) *topology.Graph { p.AttachInstances(topo) - return topo.ToThreeTierGraph(provider, instances, p.trimTiers, normalize) + return topo.ToGraph(provider, instances, p.trimTiers, normalize) } // GetComputeInstances returns model-derived compute instances for engines that need them. diff --git a/pkg/topology/graph.go b/pkg/topology/graph.go index 2b1fee6e..7cf8fdd2 100644 --- a/pkg/topology/graph.go +++ b/pkg/topology/graph.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package topology @@ -27,26 +16,38 @@ import ( "github.com/NVIDIA/topograph/pkg/metrics" ) -type band int - -const ( - leafBand band = iota + 1 - spineBand - coreBand -) - type ClusterTopology struct { Instances []*InstanceTopology } +// FabricTier identifies one fabric layer. InstanceTopology stores tiers +// closest-first, so index zero is the switch closest to the compute node. +type FabricTier struct { + ID string + Name string // optional normalized / display name +} + +// ClosestFirstFabricTiers converts closest-first switch IDs to fabric tiers. +func ClosestFirstFabricTiers(ids ...string) []FabricTier { + tiers := make([]FabricTier, len(ids)) + for i, id := range ids { + tiers[i].ID = id + } + return tiers +} + +// RootFirstFabricTiers converts root-first switch IDs to closest-first tiers. +func RootFirstFabricTiers(ids ...string) []FabricTier { + tiers := make([]FabricTier, len(ids)) + for i, id := range ids { + tiers[len(ids)-i-1].ID = id + } + return tiers +} + type InstanceTopology struct { InstanceID string - LeafID string - LeafName string // optional - SpineID string - SpineName string // optional - CoreID string - CoreName string // optional + FabricTiers []FabricTier AcceleratorID string // Instance optionally carries enriched metadata for instance-oriented output. Instance *Instance @@ -54,27 +55,17 @@ type InstanceTopology struct { func (inst *InstanceTopology) String() string { var buf strings.Builder - buf.WriteString("Instance:" + inst.InstanceID) - if len(inst.LeafID) != 0 { - buf.WriteString(" Leaf:" + inst.LeafID) - if len(inst.LeafName) != 0 { - buf.WriteString(" (" + inst.LeafName + ")") - } - } - if len(inst.SpineID) != 0 { - buf.WriteString(" Spine:" + inst.SpineID) - if len(inst.SpineName) != 0 { - buf.WriteString(" (" + inst.SpineName + ")") - } - } - if len(inst.CoreID) != 0 { - buf.WriteString(" Core:" + inst.CoreID) - if len(inst.CoreName) != 0 { - buf.WriteString(" (" + inst.CoreName + ")") + fmt.Fprintf(&buf, "Instance:%s", inst.InstanceID) + for index, tier := range inst.FabricTiers { + if tier.ID != "" { + fmt.Fprintf(&buf, " Fabric-Tier-%d:%s", index, tier.ID) + if tier.Name != "" { + fmt.Fprintf(&buf, " (%s)", tier.Name) + } } } - if len(inst.AcceleratorID) != 0 { - buf.WriteString(" Accelerator:" + inst.AcceleratorID) + if inst.AcceleratorID != "" { + fmt.Fprintf(&buf, " Accelerator:%s", inst.AcceleratorID) } return buf.String() @@ -92,14 +83,18 @@ func (c *ClusterTopology) Len() int { return len(c.Instances) } -func (c *ClusterTopology) ToThreeTierGraph(provider string, cis []ComputeInstances, trimTiers int, normalize bool) *Graph { +func (c *ClusterTopology) ToGraph(provider string, cis []ComputeInstances, trimTiers int, normalize bool) *Graph { i2n := make(map[string]string) for _, ci := range cis { maps.Copy(i2n, ci.Instances) } forest := make(map[string]*Vertex) - nodes := make(map[string]*Vertex) + type tierKey struct { + level int + id string + } + nodes := make(map[tierKey]*Vertex) domainMap := NewDomainMap() if normalize { @@ -121,33 +116,37 @@ func (c *ClusterTopology) ToThreeTierGraph(provider string, cis []ComputeInstanc ID: inst.InstanceID, } - if len(inst.AcceleratorID) != 0 { + if inst.AcceleratorID != "" { domainMap.AddHost(inst.AcceleratorID, inst.InstanceID, nodeName) } if inst.Instance != nil { instances[inst.InstanceID] = inst.toInstance(trimTiers) } - swNames := [3]string{inst.LeafName, inst.SpineName, inst.CoreName} - - for i, swID := range trimmedTiers(inst, trimTiers) { + for level, tier := range trimmedTiers(inst, trimTiers) { + swID := tier.ID if len(swID) == 0 { continue } - sw, ok := nodes[swID] + key := tierKey{level: level, id: swID} + sw, ok := nodes[key] if !ok { sw = &Vertex{ ID: swID, - Name: swNames[i], + Name: tier.Name, Vertices: make(map[string]*Vertex), } - nodes[swID] = sw + nodes[key] = sw } sw.Vertices[instance.ID] = instance instance = sw } - forest[instance.ID] = instance + if root, ok := forest[instance.ID]; ok { + mergeVertices(root, instance) + } else { + forest[instance.ID] = instance + } } if len(i2n) != 0 { @@ -172,10 +171,7 @@ func (c *ClusterTopology) ToThreeTierGraph(provider string, cis []ComputeInstanc } maps.Copy(treeRoot.Vertices, forest) - graph := &Graph{ - Tiers: treeRoot, - } - + graph := &Graph{Tiers: treeRoot} if len(domainMap) != 0 { graph.Domains = domainMap } @@ -186,6 +182,27 @@ func (c *ClusterTopology) ToThreeTierGraph(provider string, cis []ComputeInstanc return graph } +// mergeVertices preserves branches that use the same switch ID at different +// depths by recursively merging their children. +func mergeVertices(dst, src *Vertex) { + if dst == src { + return + } + if dst.Name == "" { + dst.Name = src.Name + } + if dst.Vertices == nil { + dst.Vertices = make(map[string]*Vertex) + } + for id, child := range src.Vertices { + if existing, ok := dst.Vertices[id]; ok { + mergeVertices(existing, child) + continue + } + dst.Vertices[id] = child + } +} + func (c *ClusterTopology) AttachInstances(instances map[string]Instance) { for _, topo := range c.Instances { instance, ok := instances[topo.InstanceID] @@ -200,58 +217,50 @@ func (c *ClusterTopology) AttachInstances(instances map[string]Instance) { func (c *ClusterTopology) Normalize() { // sort by network hierarchy sort.Slice(c.Instances, func(i, j int) bool { - if c.Instances[i].CoreID != c.Instances[j].CoreID { - return c.Instances[i].CoreID < c.Instances[j].CoreID - } - - if c.Instances[i].SpineID != c.Instances[j].SpineID { - return c.Instances[i].SpineID < c.Instances[j].SpineID - } - - if c.Instances[i].LeafID != c.Instances[j].LeafID { - return c.Instances[i].LeafID < c.Instances[j].LeafID + a, b := c.Instances[i].FabricTiers, c.Instances[j].FabricTiers + for level := max(len(a), len(b)) - 1; level >= 0; level-- { + var aID, bID string + if level < len(a) { + aID = a[level].ID + } + if level < len(b) { + bID = b[level].ID + } + if aID != bID { + return aID < bID + } } return c.Instances[i].InstanceID < c.Instances[j].InstanceID }) // normalize switch names - bandCounts := map[band]int{leafBand: 0, spineBand: 0, coreBand: 0} - - switches := make(map[string]string) + levelCounts := make(map[int]int) + switches := make(map[int]map[string]string) for i, inst := range c.Instances { - name, ok := switches[inst.LeafID] - if !ok { - bandCounts[leafBand]++ - name = fmt.Sprintf("switch.%d.%d", leafBand, bandCounts[leafBand]) - switches[inst.LeafID] = name - } - c.Instances[i].LeafName = name - - name, ok = switches[inst.SpineID] - if !ok { - bandCounts[spineBand]++ - name = fmt.Sprintf("switch.%d.%d", spineBand, bandCounts[spineBand]) - switches[inst.SpineID] = name - } - c.Instances[i].SpineName = name - - name, ok = switches[inst.CoreID] - if !ok { - bandCounts[coreBand]++ - name = fmt.Sprintf("switch.%d.%d", coreBand, bandCounts[coreBand]) - switches[inst.CoreID] = name + for level, tier := range inst.FabricTiers { + if tier.ID == "" { + continue + } + if switches[level] == nil { + switches[level] = make(map[string]string) + } + name, ok := switches[level][tier.ID] + if !ok { + levelCounts[level]++ + name = fmt.Sprintf("switch.%d.%d", level+1, levelCounts[level]) + switches[level][tier.ID] = name + } + c.Instances[i].FabricTiers[level].Name = name } - c.Instances[i].CoreName = name } } -func trimmedTiers(inst *InstanceTopology, trimTiers int) []string { - tiers := []string{inst.LeafID, inst.SpineID, inst.CoreID} - n := len(tiers) - for i := 0; i < trimTiers && i < n; i++ { - tiers[n-i-1] = "" - } +func trimmedTiers(inst *InstanceTopology, trimTiers int) []FabricTier { + trim := min(max(0, trimTiers), len(inst.FabricTiers)) + keep := len(inst.FabricTiers) - trim + tiers := make([]FabricTier, keep) + copy(tiers, inst.FabricTiers[:keep]) return tiers } @@ -271,18 +280,16 @@ func (inst *InstanceTopology) toInstance(trimTiers int) Instance { } func (inst *InstanceTopology) networkLayers(trimTiers int) []string { - ids := trimmedTiers(inst, trimTiers) - names := []string{inst.LeafName, inst.SpineName, inst.CoreName} layers := []string{} - for i, id := range ids { - if id == "" { + for _, tier := range trimmedTiers(inst, trimTiers) { + if tier.ID == "" { continue } - if names[i] != "" { - layers = append(layers, names[i]) + if tier.Name != "" { + layers = append(layers, tier.Name) continue } - layers = append(layers, id) + layers = append(layers, tier.ID) } return layers } diff --git a/pkg/topology/graph_test.go b/pkg/topology/graph_test.go index d8acf3fb..fb079650 100644 --- a/pkg/topology/graph_test.go +++ b/pkg/topology/graph_test.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package topology @@ -26,29 +15,21 @@ var ( instances = []*InstanceTopology{ { InstanceID: "i-001", - CoreID: "nn-77777777", - SpineID: "nn-55555555", - LeafID: "nn-11111111", + FabricTiers: ClosestFirstFabricTiers("nn-11111111", "nn-55555555", "nn-77777777"), AcceleratorID: "acc-111111", }, { InstanceID: "i-002", - CoreID: "nn-77777777", - SpineID: "nn-55555555", - LeafID: "nn-22222222", + FabricTiers: ClosestFirstFabricTiers("nn-22222222", "nn-55555555", "nn-77777777"), AcceleratorID: "acc-222222", }, { - InstanceID: "i-003", - CoreID: "nn-77777777", - SpineID: "nn-66666666", - LeafID: "nn-33333333", + InstanceID: "i-003", + FabricTiers: ClosestFirstFabricTiers("nn-33333333", "nn-66666666", "nn-77777777"), }, { - InstanceID: "i-004", - CoreID: "nn-77777777", - SpineID: "nn-66666666", - LeafID: "nn-44444444", + InstanceID: "i-004", + FabricTiers: ClosestFirstFabricTiers("nn-44444444", "nn-66666666", "nn-77777777"), }, } @@ -69,17 +50,17 @@ var ( } ) -func TestToThreeTierGraphNoNorm(t *testing.T) { +func TestToGraphNoNorm(t *testing.T) { topo := NewClusterTopology() for _, inst := range instances { topo.Append(inst) } require.Equal(t, len(instances), topo.Len()) - inst0 := "Instance:i-001 Leaf:nn-11111111 Spine:nn-55555555 Core:nn-77777777 Accelerator:acc-111111" + inst0 := "Instance:i-001 Fabric-Tier-0:nn-11111111 Fabric-Tier-1:nn-55555555 Fabric-Tier-2:nn-77777777 Accelerator:acc-111111" require.Equal(t, inst0, topo.Instances[0].String()) - inst2 := "Instance:i-003 Leaf:nn-33333333 Spine:nn-66666666 Core:nn-77777777" + inst2 := "Instance:i-003 Fabric-Tier-0:nn-33333333 Fabric-Tier-1:nn-66666666 Fabric-Tier-2:nn-77777777" require.Equal(t, inst2, topo.Instances[2].String()) v31 := &Vertex{ID: "nn-11111111", Vertices: map[string]*Vertex{"i-001": n1}} @@ -124,11 +105,11 @@ func TestToThreeTierGraphNoNorm(t *testing.T) { Domains: domains, } - graph := topo.ToThreeTierGraph("test", []ComputeInstances{{Instances: i2n}}, 0, false) + graph := topo.ToGraph("test", []ComputeInstances{{Instances: i2n}}, 0, false) require.Equal(t, expected, graph) } -func TestToThreeTierGraphNorm(t *testing.T) { +func TestToGraphNorm(t *testing.T) { topo := NewClusterTopology() for _, inst := range instances { topo.Append(inst) @@ -180,23 +161,21 @@ func TestToThreeTierGraphNorm(t *testing.T) { Domains: domains, } - graph := topo.ToThreeTierGraph("test", []ComputeInstances{{Instances: i2n}}, 0, true) + graph := topo.ToGraph("test", []ComputeInstances{{Instances: i2n}}, 0, true) require.Equal(t, expected, graph) - inst0 := "Instance:i-001 Leaf:nn-11111111 (switch.1.1) Spine:nn-55555555 (switch.2.1) Core:nn-77777777 (switch.3.1) Accelerator:acc-111111" + inst0 := "Instance:i-001 Fabric-Tier-0:nn-11111111 (switch.1.1) Fabric-Tier-1:nn-55555555 (switch.2.1) Fabric-Tier-2:nn-77777777 (switch.3.1) Accelerator:acc-111111" require.Equal(t, inst0, topo.Instances[0].String()) - inst2 := "Instance:i-003 Leaf:nn-33333333 (switch.1.3) Spine:nn-66666666 (switch.2.2) Core:nn-77777777 (switch.3.1)" + inst2 := "Instance:i-003 Fabric-Tier-0:nn-33333333 (switch.1.3) Fabric-Tier-1:nn-66666666 (switch.2.2) Fabric-Tier-2:nn-77777777 (switch.3.1)" require.Equal(t, inst2, topo.Instances[2].String()) } -func TestToThreeTierGraphIncludesInstanceData(t *testing.T) { +func TestToGraphIncludesInstanceData(t *testing.T) { topo := NewClusterTopology() topo.Append(&InstanceTopology{ InstanceID: "i-001", - LeafID: "leaf-1", - SpineID: "spine-1", - CoreID: "core-1", + FabricTiers: ClosestFirstFabricTiers("leaf-1", "spine-1", "core-1"), AcceleratorID: "nvl-1", Instance: &Instance{ ID: "i-001", @@ -205,7 +184,7 @@ func TestToThreeTierGraphIncludesInstanceData(t *testing.T) { }, }) - graph := topo.ToThreeTierGraph("test", []ComputeInstances{ + graph := topo.ToGraph("test", []ComputeInstances{ { Region: "region-1", Instances: map[string]string{"i-001": "node1"}, @@ -235,9 +214,7 @@ func TestTrimTiers(t *testing.T) { name: "Case 1: trim none", trimTiers: 0, in: InstanceTopology{ - CoreID: "core1", - SpineID: "spine1", - LeafID: "leaf1", + FabricTiers: ClosestFirstFabricTiers("leaf1", "spine1", "core1"), }, out: []string{"leaf1", "spine1", "core1"}, }, @@ -245,48 +222,114 @@ func TestTrimTiers(t *testing.T) { name: "Case 2: trim 1 tier", trimTiers: 1, in: InstanceTopology{ - CoreID: "core1", - SpineID: "spine1", - LeafID: "leaf1", + FabricTiers: ClosestFirstFabricTiers("leaf1", "spine1", "core1"), }, - out: []string{"leaf1", "spine1", ""}, + out: []string{"leaf1", "spine1"}, }, { name: "Case 3: trim 2 tiers", trimTiers: 2, in: InstanceTopology{ - CoreID: "core1", - SpineID: "spine1", - LeafID: "leaf1", + FabricTiers: ClosestFirstFabricTiers("leaf1", "spine1", "core1"), }, - out: []string{"leaf1", "", ""}, + out: []string{"leaf1"}, }, { name: "Case 4: trim all tiers", trimTiers: 3, in: InstanceTopology{ - CoreID: "core1", - SpineID: "spine1", - LeafID: "leaf1", + FabricTiers: ClosestFirstFabricTiers("leaf1", "spine1", "core1"), }, - out: []string{"", "", ""}, + out: []string{}, }, { name: "Case 5: trim more than available", trimTiers: 10, in: InstanceTopology{ - CoreID: "core1", - SpineID: "spine1", - LeafID: "leaf1", + FabricTiers: ClosestFirstFabricTiers("leaf1", "spine1", "core1"), }, - out: []string{"", "", ""}, + out: []string{}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { inst := tt.in - require.Equal(t, tt.out, trimmedTiers(&inst, tt.trimTiers)) + tiers := trimmedTiers(&inst, tt.trimTiers) + ids := make([]string, len(tiers)) + for i := range tiers { + ids[i] = tiers[i].ID + } + require.Equal(t, tt.out, ids) }) } } + +func TestToGraphSupportsVariableTierCount(t *testing.T) { + topo := NewClusterTopology() + topo.Append(&InstanceTopology{ + InstanceID: "instance-1", + FabricTiers: ClosestFirstFabricTiers("fabric-0", "fabric-1", "fabric-2", "fabric-3"), + AcceleratorID: "accelerator", + }) + + graph := topo.ToGraph("test", []ComputeInstances{{ + Instances: map[string]string{"instance-1": "node-1"}, + }}, 0, false) + + vertex := graph.Tiers.Vertices["fabric-3"] + for _, id := range []string{"fabric-2", "fabric-1", "fabric-0", "instance-1"} { + require.NotNil(t, vertex) + vertex = vertex.Vertices[id] + } + require.Equal(t, "node-1", vertex.Name) + require.Contains(t, graph.Domains["accelerator"], "node-1") +} + +func TestToGraphKeepsSameIDAtDifferentLevelsDistinct(t *testing.T) { + topo := NewClusterTopology() + topo.Append(&InstanceTopology{ + InstanceID: "instance-1", + FabricTiers: ClosestFirstFabricTiers("shared", "shared"), + }) + + graph := topo.ToGraph("test", []ComputeInstances{{ + Instances: map[string]string{"instance-1": "node-1"}, + }}, 0, false) + + outer := graph.Tiers.Vertices["shared"] + inner := outer.Vertices["shared"] + require.NotSame(t, outer, inner) + require.Equal(t, "node-1", inner.Vertices["instance-1"].Name) +} + +func TestToGraphMergesMixedDepthPathsAtSharedRoot(t *testing.T) { + topo := NewClusterTopology() + topo.Append(&InstanceTopology{ + InstanceID: "instance-1", + FabricTiers: ClosestFirstFabricTiers("leaf-1", "shared-root"), + }) + topo.Append(&InstanceTopology{ + InstanceID: "instance-2", + FabricTiers: ClosestFirstFabricTiers("leaf-2", "spine-2", "shared-root"), + }) + + graph := topo.ToGraph("test", []ComputeInstances{{ + Instances: map[string]string{ + "instance-1": "node-1", + "instance-2": "node-2", + }, + }}, 0, false) + + root := graph.Tiers.Vertices["shared-root"] + require.NotNil(t, root) + require.Contains(t, root.Vertices, "leaf-1") + require.Contains(t, root.Vertices, "spine-2") + require.Equal(t, "node-1", root.Vertices["leaf-1"].Vertices["instance-1"].Name) + require.Equal(t, "node-2", root.Vertices["spine-2"].Vertices["leaf-2"].Vertices["instance-2"].Name) +} + +func TestTrimTiersTreatsNegativeAsZero(t *testing.T) { + inst := &InstanceTopology{FabricTiers: ClosestFirstFabricTiers("fabric-0", "fabric-1")} + require.Equal(t, inst.FabricTiers, trimmedTiers(inst, -1)) +} diff --git a/pkg/topology/instances.go b/pkg/topology/instances.go index fd94038f..2155669a 100644 --- a/pkg/topology/instances.go +++ b/pkg/topology/instances.go @@ -6,6 +6,7 @@ package topology import ( "fmt" + "strconv" ) // Instances is the top-level JSON envelope for instance-oriented topology export. @@ -33,6 +34,10 @@ func AcceleratorID(labels map[string]string) string { return labels[KeyNvidiaGPUClique] } +func FabricTierKey(tier int) string { + return KeyFabricTierPrefix + strconv.Itoa(tier) +} + // String summarizes an instance for logging (simulation / derived fields). func (inst *Instance) String() string { return fmt.Sprintf("Instance: %s Labels: %v NetLayers: %v", diff --git a/pkg/topology/topology.go b/pkg/topology/topology.go index 780596b1..2224ecf8 100644 --- a/pkg/topology/topology.go +++ b/pkg/topology/topology.go @@ -1,17 +1,6 @@ /* - * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2024-2026 NVIDIA CORPORATION + * SPDX-License-Identifier: Apache-2.0 */ package topology @@ -48,7 +37,9 @@ const ( KeyNvidiaGPUClique = "nvidia.com/gpu.clique" KeyNvidiaGPUProduct = "nvidia.com/gpu.product" - // Topograph default node labels + // Topograph default node labels. Fabric tier zero is closest to the compute + // node. + KeyFabricTierPrefix = "network.topology.nvidia.com/tier-" KeyTopologyAccelerator = "network.topology.nvidia.com/accelerator" // ConfigMap annotation keys for metadata tracking diff --git a/pkg/translate/topology.go b/pkg/translate/topology.go index b1e09173..8443cf58 100644 --- a/pkg/translate/topology.go +++ b/pkg/translate/topology.go @@ -175,18 +175,23 @@ func toBlockInfos(domains topology.DomainMap) []*blockInfo { } func (nt *NetworkTopology) initBlocks(graph *topology.Graph) { - if graph == nil || graph.Domains == nil { + if graph == nil { + klog.Warning("block topology data not found") + return + } + domains := graph.Domains + if domains == nil { klog.Warning("block topology data not found") return } - if len(graph.Domains) == 0 { + if len(domains) == 0 { klog.Warning("no blocks found in block topology") return } - nt.domains = graph.Domains - domainBlocks := toBlockInfos(graph.Domains) + nt.domains = domains + domainBlocks := toBlockInfos(domains) nt.blocks = make([]*blockInfo, 0, len(domainBlocks)) indx := 0 @@ -194,7 +199,7 @@ func (nt *NetworkTopology) initBlocks(graph *topology.Graph) { for _, bInfo := range domainBlocks { bInfo.indx = indx for _, node := range bInfo.nodes { - hostInfo := graph.Domains[bInfo.name][node] + hostInfo := domains[bInfo.name][node] if hostInfo == nil { klog.Warningf("initBlocks: missing host info for node %q in domain %q", node, bInfo.name) continue diff --git a/pkg/translate/topology_test.go b/pkg/translate/topology_test.go index f6f50104..cd6db7f0 100644 --- a/pkg/translate/topology_test.go +++ b/pkg/translate/topology_test.go @@ -123,6 +123,13 @@ func TestValidateConfig(t *testing.T) { }, err: "missing block topology", }, + { + name: "Case 3.1: nil block root", + cfg: &Config{ + Plugin: topology.TopologyBlock, + }, + err: "missing block topology", + }, { name: "Case 4: mutually exclusive parameters", root: emptyRoot, @@ -159,6 +166,14 @@ func TestValidateConfig(t *testing.T) { }, err: `missing block topology for topology "topo"`, }, + { + name: "Case 7.1: nil block root in topology spec", + cfg: &Config{ + Topologies: map[string]*TopologySpec{ + "topo": {Plugin: topology.TopologyBlock}}, + }, + err: `missing block topology for topology "topo"`, + }, { name: "Case 8: missing nodes in topology spec", root: blockRoot,