You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Persist NIC/fabric/rail topology in AICR's snapshot by consuming k8s-launch-kit (l8k) as a Go library that implements AICR's Collector interface (pkg/collector/types.go:27).
Background
l8k already encodes the discovery logic AICR needs (kernel module graph from /sys/module, GPU topology via nvidia-smi, NIC inventory via NicDevice CRs, machine-type detection, node-label patching). Today that logic is invoked by the l8k CLI via a mutating flow:
Patch/create NicClusterPolicy so the network-operator stands up nic-configuration-daemon.
Exec into those daemon pods to collect kernel/hardware data.
Read NicDevice CRs.
Patch node labels.
Defer-cleanup the patch.
See pkg/networkoperatorplugin/discovery.go:99-260. This violates AICR's collector contract (read-only, bounded, no side effects). Phase 1 must reconcile that.
Options
Option A — Static collector via presets + releases catalog (no cluster contact)
Wrap pkg/presets.LoadPreset(machineType, gpuType) and pkg/networkoperatorplugin.LookupRelease. Caller (or AICR's existing GPU collector) supplies machine type + GPU product; the new collector returns l8k's LaunchKubernetesConfig populated from baked-in presets.
Pros: ships immediately. Zero footprint. No cluster mutation. No new RBAC.
Cons: no live data — degenerate "collector" that just looks up a table. Doesn't validate actual hardware. Useful as a fallback / offline mode, not as the primary path.
Extend Factory (pkg/collector/factory.go:30) with CreateNetworkCollector() Collector + functional option to enable. Note the factory is a closed interface today — adding a method is a breaking change for external implementers (audit first).
New package pkg/collector/network implementing Collector. Embed l8k's LaunchKubernetesConfig as a typed sub-section in *measurement.Measurement (define the type in pkg/measurement rather than depending on l8k's struct directly, to insulate AICR from upstream churn).
Snapshotter wiring: add to pkg/snapshotter agent + aicr snapshot CLI. Gate behind a flag (--collect-network) while shape stabilizes.
--no-cluster mode: collector returns empty/skipped measurement; never touches the cluster.
RBAC: if Option B/C lands, document the pod-exec/list-pods/list-NicDevices delta and update Helm chart RBAC for aicrd.
Tests: unit-level table-driven tests with a fake client.Client; KWOK e2e for the live-discovery path (gated on cluster having network-operator).
Open questions
Module path: l8k advertises github.com/nvidia/k8s-launch-kit (lowercase) but the GitHub org is NVIDIA. Confirm the canonical Go module path before vendoring.
Transitive dep cost: importing pkg/networkoperatorplugin pulls Mellanox/network-operator, Mellanox/nic-configuration-operator, controller-runtime, and Mellanox CRD modules. Acceptable, or do we ask l8k to split the read-only discovery into a leaf package with thinner deps?
Versioning: l8k has no v1.0.0 tag. Pin by SHA, vendor with replace, or ask upstream for a semver tag before we depend on it?
Namespace assumptions: l8k's alternateNamespace fallback assumes one of two well-known namespaces — does this break AICR's multi-tenant target?
Schema stability: if LaunchKubernetesConfig shape changes, snapshot YAML changes downstream. Should AICR vendor its own DTO and translate at the boundary?
Acceptance criteria
aicr snapshot --collect-network populates a network (or equivalent) section with NIC/fabric/rail data when network-operator is present.
When network-operator is absent or --no-cluster is set, the collector emits a skipped/empty measurement and the snapshot still succeeds.
No mutation of the target cluster from the collector path. (Option B is explicitly rejected unless we revisit the policy.)
Unit tests for happy path, no-cluster path, and l8k-not-installed path.
Documentation: docs/contributor/collectors.md (new section), docs/user/cli-reference.md (new flag).
Part of #827.
Goal
Persist NIC/fabric/rail topology in AICR's snapshot by consuming k8s-launch-kit (l8k) as a Go library that implements AICR's
Collectorinterface (pkg/collector/types.go:27).Background
l8k already encodes the discovery logic AICR needs (kernel module graph from
/sys/module, GPU topology vianvidia-smi, NIC inventory viaNicDeviceCRs, machine-type detection, node-label patching). Today that logic is invoked by thel8kCLI via a mutating flow:NicClusterPolicyso the network-operator stands upnic-configuration-daemon.NicDeviceCRs.See
pkg/networkoperatorplugin/discovery.go:99-260. This violates AICR's collector contract (read-only, bounded, no side effects). Phase 1 must reconcile that.Options
Option A — Static collector via
presets+releasescatalog (no cluster contact)Wrap
pkg/presets.LoadPreset(machineType, gpuType)andpkg/networkoperatorplugin.LookupRelease. Caller (or AICR's existing GPU collector) supplies machine type + GPU product; the new collector returns l8k'sLaunchKubernetesConfigpopulated from baked-in presets.Option B — Wrap
DiscoverClusterConfigas-isAICR collector calls l8k's existing mutating
DiscoverClusterConfigdirectly.Collectorcontract: mutatesNicClusterPolicy, requires pod-exec RBAC.discovery.go:201) may conflict with multi-tenant deployments.Option C — Read-only discovery upstream, then wrap (recommended)
File an l8k issue requesting
DiscoverReadOnly(ctx, client) (*ClusterConfig, error):reuseExistingbranch (discovery.go:112-126) whennic-configuration-daemonis already running.ErrNotInstalled(or similar) when not installed.NicClusterPolicy. Does not patch node labels.AICR collector calls it; falls back to Option A semantics when not installed.
Recommended: Option C, with Option A wired in as the no-cluster fallback.
Work breakdown
DiscoverReadOnly. Tracked separately so AICR work isn't blocked on the entire feature.Factory(pkg/collector/factory.go:30) withCreateNetworkCollector() Collector+ functional option to enable. Note the factory is a closed interface today — adding a method is a breaking change for external implementers (audit first).pkg/collector/networkimplementingCollector. Embed l8k'sLaunchKubernetesConfigas a typed sub-section in*measurement.Measurement(define the type inpkg/measurementrather than depending on l8k's struct directly, to insulate AICR from upstream churn).pkg/snapshotteragent +aicr snapshotCLI. Gate behind a flag (--collect-network) while shape stabilizes.--no-clustermode: collector returns empty/skipped measurement; never touches the cluster.aicrd.client.Client; KWOK e2e for the live-discovery path (gated on cluster having network-operator).Open questions
github.com/nvidia/k8s-launch-kit(lowercase) but the GitHub org isNVIDIA. Confirm the canonical Go module path before vendoring.pkg/networkoperatorpluginpullsMellanox/network-operator,Mellanox/nic-configuration-operator, controller-runtime, and Mellanox CRD modules. Acceptable, or do we ask l8k to split the read-only discovery into a leaf package with thinner deps?v1.0.0tag. Pin by SHA, vendor withreplace, or ask upstream for a semver tag before we depend on it?alternateNamespacefallback assumes one of two well-known namespaces — does this break AICR's multi-tenant target?LaunchKubernetesConfigshape changes, snapshot YAML changes downstream. Should AICR vendor its own DTO and translate at the boundary?Acceptance criteria
aicr snapshot --collect-networkpopulates anetwork(or equivalent) section with NIC/fabric/rail data when network-operator is present.--no-clusteris set, the collector emits a skipped/empty measurement and the snapshot still succeeds.docs/contributor/collectors.md(new section),docs/user/cli-reference.md(new flag).