From 0d75a6393e913ea0101489fdc67dedcf9e0561d6 Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Tue, 18 Aug 2026 13:38:46 +0200 Subject: [PATCH 1/4] Fix NIC interface template reconciliation wait Treat InterfaceNameMismatch as retryable and bound interface template reconciliation to five minutes. This lets udev renames settle while keeping later manifest verification gated. Signed-off-by: Alexander Maslennikov --- README.md | 7 +- docs/advanced/deployment.md | 6 +- pkg/cmd/deploy.go | 4 +- .../crstate/nicconfig.go | 11 ++- .../crstate/nicconfig_test.go | 8 +- pkg/networkoperatorplugin/deploy.go | 94 +++++++++++++++---- pkg/networkoperatorplugin/deploy_test.go | 49 ++++++++++ skills/k8s-launch-kit-deploy/SKILL.md | 9 +- skills/k8s-launch-kit-troubleshoot/SKILL.md | 2 +- .../references/common-failures.md | 16 +++- 10 files changed, 169 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 8303cb5f..db8f82b4 100644 --- a/README.md +++ b/README.md @@ -350,7 +350,12 @@ phase. It auto-prefers to `` itself. `--dry-run` does a server-side dry run. `--deploy-timeout` caps the whole apply+reconcile phase end-to-end (e.g. `--deploy-timeout 90m`); without it, deploy polls indefinitely — right for SR-IOV on large clusters -where reconciliation can take an hour. +where reconciliation can take an hour. `NicInterfaceNameTemplate` is the +bounded exception: an `InterfaceNameMismatch` remains `IN-PROGRESS` for up to +five minutes while the NIC configuration daemon retries the udev rename. This +gates verification of later manifests without aborting on the first transient +mismatch. If the names still do not match after five minutes, deployment fails +with the per-device mismatch details. A shorter `--deploy-timeout` still wins. When Launch Kit installs or upgrades the Network Operator chart, its Helm post-renderer adds the same version annotation to chart-rendered resources. diff --git a/docs/advanced/deployment.md b/docs/advanced/deployment.md index 5a565cf1..71f2fc9d 100644 --- a/docs/advanced/deployment.md +++ b/docs/advanced/deployment.md @@ -81,9 +81,11 @@ Kind-specific checks include per-component Network Operator state, SR-IOV per-no For `NicConfigurationTemplate` and `NicFirmwareTemplate`, Launch Kit first waits for the operator to publish matched device names in `status.nicDevices` and for that name set to reflect the current `nodeSelector`, NIC type, PCI-address, serial-number, and part-number selectors. It then evaluates only those `NicDevice` objects and waits for the corresponding `spec.configuration` or `spec.firmware` field to reflect the current template payload. A successful device condition is accepted only after its `observedGeneration` catches up with the `NicDevice` generation. A configuration template checks `FirmwareUpdateInProgress` only when the matched device carries `spec.firmware`; without a deployed firmware template, a stale firmware condition from an older device generation does not block configuration reconciliation. Other discovered NICs do not block on configuration or firmware state. Changed templates are also observation-gated before this status is accepted, so status left by an earlier generation cannot produce a false success. +For `NicInterfaceNameTemplate`, `InterfaceNameMismatch` is retryable because the NIC configuration daemon can publish it while newly-written udev rules are still taking effect. Launch Kit keeps that template `IN-PROGRESS` for up to five minutes. Since phase-4 verification is ordered, the template gates later checks during this window. A persistent mismatch fails deployment after the local timeout and retains the per-node and per-port mismatch details. + ## Timeout -The default deploy budget is unbounded because SR-IOV and driver reconciliation can exceed a small fixed timeout on large clusters. +The default deploy budget is unbounded because SR-IOV and driver reconciliation can exceed a small fixed timeout on large clusters. `NicInterfaceNameTemplate` is the only bounded exception: its retryable interface-name reconciliation window is five minutes. A shorter deploy-wide timeout takes precedence. Bound the entire Helm, apply, and reconciliation operation to a maintenance window: @@ -93,7 +95,7 @@ l8k deploy \ --deploy-timeout 90m ``` -There is no independent per-manifest deadline. +Other manifests have no independent per-manifest deadline. ## Server-Side Dry Run diff --git a/pkg/cmd/deploy.go b/pkg/cmd/deploy.go index b8bc142d..1f36c5be 100644 --- a/pkg/cmd/deploy.go +++ b/pkg/cmd/deploy.go @@ -63,7 +63,9 @@ and applies them in four phases: Use --deploy-timeout to bound the entire end-to-end run (e.g. for a maintenance window). Without the flag the deploy waits indefinitely for reconciliation — appropriate for large SR-IOV clusters where a single -policy can take an hour or more. +policy can take an hour or more. NicInterfaceNameTemplate is the bounded +exception: an interface-name mismatch is retried for up to five minutes so +udev rules can settle, then fails before later verification proceeds. If /network-operator/ exists (the layout 'l8k generate' produces), that subdirectory is used. Otherwise itself diff --git a/pkg/networkoperatorplugin/crstate/nicconfig.go b/pkg/networkoperatorplugin/crstate/nicconfig.go index 977ed6d3..45959277 100644 --- a/pkg/networkoperatorplugin/crstate/nicconfig.go +++ b/pkg/networkoperatorplugin/crstate/nicconfig.go @@ -528,10 +528,11 @@ func classifyFirmware(device *unstructured.Unstructured, byType map[string]map[s } } -// classifyInterfaceName inspects InterfaceNameApplied. Mismatch is the -// silent-failure case we want to catch: udev rules didn't apply, so -// downstream SR-IOV selectors that key on the new name will match -// nothing. +// classifyInterfaceName inspects InterfaceNameApplied. A mismatch is +// retryable because the NIC operator can report it while newly-written udev +// rules are still taking effect. The deploy state machine gives +// NicInterfaceNameTemplate its own bounded reconciliation window, so a +// persistent mismatch still fails before downstream verification proceeds. func classifyInterfaceName(byType map[string]map[string]interface{}) (CRState, string) { cond, ok := byType[consts.InterfaceNameCondition] if !ok { @@ -545,7 +546,7 @@ func classifyInterfaceName(byType map[string]map[string]interface{}) (CRState, s case reason == consts.InterfaceNameAppliedReason && status == "True": return StateSuccess, "InterfaceNameApplied" case reason == consts.InterfaceNameMismatchReason: - return StateError, fallbackMessage(message, "interface name mismatch — udev rules did not apply") + return StateInProgress, fallbackMessage(message, "interface name mismatch — waiting for udev rules to apply") default: return StateInProgress, fallbackMessage(fmt.Sprintf("InterfaceNameApplied=%s reason=%s", status, reason), "InterfaceNameApplied unknown") } diff --git a/pkg/networkoperatorplugin/crstate/nicconfig_test.go b/pkg/networkoperatorplugin/crstate/nicconfig_test.go index d05dc15d..50603374 100644 --- a/pkg/networkoperatorplugin/crstate/nicconfig_test.go +++ b/pkg/networkoperatorplugin/crstate/nicconfig_test.go @@ -216,8 +216,10 @@ func TestNicInterfaceNameTemplate_AppliedSuccessfully(t *testing.T) { assert.Equal(t, StateSuccess, res.State) } -func TestNicInterfaceNameTemplate_MismatchIsError(t *testing.T) { - // Silent-failure case: udev rules didn't apply, names didn't take. +func TestNicInterfaceNameTemplate_MismatchIsInProgress(t *testing.T) { + // The operator can publish a mismatch while newly-written udev rules are + // still taking effect. Deploy keeps polling this state under a bounded + // NicInterfaceNameTemplate reconciliation timeout. manifest := nicTemplateManifest(nicopKindInterfaceNameTemplate, "tpl", "ns", map[string]string{"role": "worker"}) live := manifest.DeepCopy() c := newClient(t, @@ -234,7 +236,7 @@ func TestNicInterfaceNameTemplate_MismatchIsError(t *testing.T) { v := nicTemplateValidator(templateKindInterfaceName) res, err := v(context.Background(), c, manifest) require.NoError(t, err) - assert.Equal(t, StateError, res.State) + assert.Equal(t, StateInProgress, res.State) assert.Contains(t, res.Reason, "interface name mismatch") } diff --git a/pkg/networkoperatorplugin/deploy.go b/pkg/networkoperatorplugin/deploy.go index e3f7e034..fe30500a 100644 --- a/pkg/networkoperatorplugin/deploy.go +++ b/pkg/networkoperatorplugin/deploy.go @@ -93,6 +93,16 @@ type DeployOptions struct { // helper cadence so logs feel familiar. const deployPollInterval = 3 * time.Second +// nicInterfaceNameTemplateReconcileTimeout bounds the one phase-4 resource +// whose normal reconciliation can temporarily report a failure-shaped +// condition. The NIC operator writes InterfaceNameMismatch while udev rules +// are still taking effect, then retries. Treating that first mismatch as +// terminal aborts deployment before the remaining resources can be checked; +// treating it as unbounded progress can hang forever when --deploy-timeout is +// unset. This per-template window preserves the gate while keeping a real +// rename failure bounded. A shorter deploy-wide context deadline still wins. +const nicInterfaceNameTemplateReconcileTimeout = 5 * time.Minute + // appliedManifest pairs an applied "other" manifest with the // information phase 4 needs to gate on controller observation: // awaitObservationAfterRV holds the resourceVersion the server @@ -149,12 +159,12 @@ func (p *NetworkOperatorPlugin) DeployProfile(ctx context.Context, profile *prof // until each reaches a terminal state (success/error) or the deploy // context is cancelled. Skipped in dry-run mode. // -// Per-manifest deadlines have been removed: the only timeout that applies -// is whatever the caller threads into ctx (typically wrapped via -// context.WithTimeout for a maintenance-window-sized budget). When ctx -// has no deadline, the deploy waits indefinitely for reconciliation — -// which is the right default for SR-IOV configuration on large clusters, -// where a single policy can easily exceed any small per-manifest budget. +// Most manifests have no per-resource deadline: the caller can thread a +// deploy-wide timeout into ctx, and without one the deploy waits indefinitely +// for long-running SR-IOV or driver reconciliation. NicInterfaceNameTemplate +// is the exception: it has a five-minute local window because its normal +// InterfaceNameMismatch condition is retryable, but a permanent udev rename +// failure must not block an otherwise-unbounded deployment forever. // // When opts.DryRun is true the apply path uses server-side dry-run // (client.DryRunAll) so the cluster validates manifests without @@ -445,8 +455,9 @@ func applyAndWait(ctx context.Context, c client.Client, registry *crstate.Regist // pollUntilTerminal polls the registry's Validator for obj until it // reports StateSuccess or StateError. not-deployed transitions trigger a -// single re-apply (object vanished between apply and poll); the only -// exit condition besides terminal state is ctx.Done(). +// single re-apply (object vanished between apply and poll). Besides a terminal +// state or ctx.Done(), kinds with a manifestReconcileTimeout also exit when +// that local reconciliation window expires. // // awaitObservationAfterRV is the resourceVersion the server returned // from the apply Patch. When non-empty, the poll loop refuses to act @@ -467,6 +478,19 @@ func applyAndWait(ctx context.Context, c client.Client, registry *crstate.Regist // so a noisy 3-second polling loop only emits a fresh line when // something actually changed. func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.Registry, obj *unstructured.Unstructured, label, awaitObservationAfterRV string) error { + return pollUntilTerminalWithReconcileTimeout( + ctx, c, registry, obj, label, awaitObservationAfterRV, manifestReconcileTimeout(obj), + ) +} + +func pollUntilTerminalWithReconcileTimeout( + ctx context.Context, + c client.Client, + registry *crstate.Registry, + obj *unstructured.Unstructured, + label, awaitObservationAfterRV string, + reconcileTimeout time.Duration, +) error { uiOutput := ui.FromContext(ctx) progress := uiOutput.StartProgress(fmt.Sprintf("Waiting for %s to reconcile", label)) log.Log.Info("Waiting for manifest to reconcile", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) @@ -474,6 +498,15 @@ func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.R ticker := time.NewTicker(deployPollInterval) defer ticker.Stop() + var reconcileTimer *time.Timer + var reconcileTimeoutC <-chan time.Time + if reconcileTimeout > 0 { + reconcileTimer = time.NewTimer(reconcileTimeout) + reconcileTimeoutC = reconcileTimer.C + defer reconcileTimer.Stop() + uiOutput.Info(" %s reconciliation timeout: %s", label, reconcileTimeout) + } + var lastReason string reportProgress := func(reason string) { if reason == lastReason || reason == "" { @@ -495,6 +528,24 @@ func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.R progress.Update(fmt.Sprintf("%s: %s", label, reason)) } } + timeoutError := func() error { + progress.Fail(fmt.Sprintf("Timed out after %s while waiting for %s", reconcileTimeout, label)) + if lastReason != "" { + return fmt.Errorf("%s timed out after %s waiting to reconcile: %s", label, reconcileTimeout, lastReason) + } + return fmt.Errorf("%s timed out after %s waiting to reconcile", label, reconcileTimeout) + } + waitForNextPoll := func() error { + select { + case <-ctx.Done(): + progress.Fail(fmt.Sprintf("Cancelled or timed out while waiting for %s", label)) + return ctx.Err() + case <-reconcileTimeoutC: + return timeoutError() + case <-ticker.C: + return nil + } + } for { if err := ctx.Err(); err != nil { @@ -519,11 +570,8 @@ func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.R // uniformly below. case live.GetResourceVersion() == awaitObservationAfterRV: reportProgress("waiting for controller to observe new spec") - select { - case <-ctx.Done(): - progress.Fail(fmt.Sprintf("Cancelled or timed out while waiting for %s", label)) - return ctx.Err() - case <-ticker.C: + if err := waitForNextPoll(); err != nil { + return err } continue default: @@ -569,15 +617,25 @@ func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.R } } - select { - case <-ctx.Done(): - progress.Fail(fmt.Sprintf("Cancelled or timed out while waiting for %s", label)) - return ctx.Err() - case <-ticker.C: + if err := waitForNextPoll(); err != nil { + return err } } } +func manifestReconcileTimeout(obj *unstructured.Unstructured) time.Duration { + if obj == nil { + return 0 + } + gvk := obj.GroupVersionKind() + if gvk.Group == "configuration.net.nvidia.com" && + gvk.Version == "v1alpha1" && + gvk.Kind == "NicInterfaceNameTemplate" { + return nicInterfaceNameTemplateReconcileTimeout + } + return 0 +} + // applyUnstructuredWithRetry wraps applyUnstructured with the legacy // Pod-specific retry path (up to 3 attempts, 30s apart). Non-Pod kinds // surface the first apply error unmodified. diff --git a/pkg/networkoperatorplugin/deploy_test.go b/pkg/networkoperatorplugin/deploy_test.go index 1e39854a..7b023e23 100644 --- a/pkg/networkoperatorplugin/deploy_test.go +++ b/pkg/networkoperatorplugin/deploy_test.go @@ -17,9 +17,16 @@ package networkoperatorplugin import ( + "context" "testing" + "time" + "github.com/nvidia/k8s-launch-kit/pkg/networkoperatorplugin/crstate" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" ) func TestSniffKind(t *testing.T) { @@ -127,3 +134,45 @@ metadata: } }) } + +func TestManifestReconcileTimeout(t *testing.T) { + interfaceTemplate := &unstructured.Unstructured{} + interfaceTemplate.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "configuration.net.nvidia.com", + Version: "v1alpha1", + Kind: "NicInterfaceNameTemplate", + }) + assert.Equal(t, 5*time.Minute, manifestReconcileTimeout(interfaceTemplate)) + + configMap := &unstructured.Unstructured{} + configMap.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"}) + assert.Zero(t, manifestReconcileTimeout(configMap)) + assert.Zero(t, manifestReconcileTimeout(nil)) +} + +func TestPollUntilTerminal_InterfaceNameMismatchTimesOut(t *testing.T) { + gvk := schema.GroupVersionKind{ + Group: "configuration.net.nvidia.com", + Version: "v1alpha1", + Kind: "NicInterfaceNameTemplate", + } + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + obj.SetName("nic-rename") + + registry := crstate.NewRegistry() + registry.Register(gvk, func(context.Context, client.Client, *unstructured.Unstructured) (crstate.Result, error) { + return crstate.Result{ + State: crstate.StateInProgress, + Reason: "worker-1/0000:05:00.0: interface name mismatch", + }, nil + }) + + err := pollUntilTerminalWithReconcileTimeout( + context.Background(), nil, registry, obj, + "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out after 10ms") + assert.Contains(t, err.Error(), "interface name mismatch") +} diff --git a/skills/k8s-launch-kit-deploy/SKILL.md b/skills/k8s-launch-kit-deploy/SKILL.md index 96abaa1e..f209b5bb 100644 --- a/skills/k8s-launch-kit-deploy/SKILL.md +++ b/skills/k8s-launch-kit-deploy/SKILL.md @@ -1,6 +1,6 @@ --- name: k8s-launch-kit-deploy -version: 1.3.3 +version: 1.3.4 description: "Use this skill when the user wants to deploy generated NVIDIA networking manifests to a Kubernetes cluster using k8s-launch-kit (l8k). Activate for: applying manifests, deploying to cluster, the `l8k deploy` subcommand or the legacy --deploy flag on `l8k generate`, applying generated files, or any mention of pushing l8k output to a live cluster. Even if the user just says 'apply these' or 'push to cluster' after generating manifests, use this skill." metadata: requires: @@ -111,6 +111,13 @@ unrelated device configuration state does not block deployment. A matched device has `spec.firmware`; configuration-only deployments ignore a stale firmware condition. +`NicInterfaceNameTemplate` gates verification of the manifests that follow it. +Treat `InterfaceNameMismatch` as retryable for up to five minutes because the +NIC configuration daemon can publish that condition while new udev rules are +still taking effect. If every targeted device reaches `InterfaceNameApplied`, +continue verification. If a mismatch persists for five minutes, fail with the +per-device details. A shorter deploy-wide `--deploy-timeout` takes precedence. + During preflight, do not classify `SriovNetworkPoolConfig`, `SriovNetworkNodePolicy`, or `OVSNetwork` objects labeled with `spectrumx.nvidia.com/owner-name` as strays. They are child resources generated diff --git a/skills/k8s-launch-kit-troubleshoot/SKILL.md b/skills/k8s-launch-kit-troubleshoot/SKILL.md index 52f6d153..725351db 100644 --- a/skills/k8s-launch-kit-troubleshoot/SKILL.md +++ b/skills/k8s-launch-kit-troubleshoot/SKILL.md @@ -1,6 +1,6 @@ --- name: k8s-launch-kit-troubleshoot -version: 1.1.1 +version: 1.1.2 description: "Use this skill when the user has problems with NVIDIA Network Operator on Kubernetes, or wants to analyze a sosreport diagnostic dump. Activate for: OFED driver crashes, SR-IOV pods failing, NicClusterPolicy errors, network operator pod issues, RDMA not working, NIC configuration failures, pods stuck in CrashLoopBackOff or ContainerCreating with network annotations, VF allocation issues, or when the user mentions 'troubleshoot', 'debug', 'sosreport', 'diagnose', or describes any NVIDIA networking failure -- even if they don't explicitly ask for troubleshooting." metadata: requires: diff --git a/skills/k8s-launch-kit-troubleshoot/references/common-failures.md b/skills/k8s-launch-kit-troubleshoot/references/common-failures.md index 2be86425..5f5fadb8 100644 --- a/skills/k8s-launch-kit-troubleshoot/references/common-failures.md +++ b/skills/k8s-launch-kit-troubleshoot/references/common-failures.md @@ -101,10 +101,12 @@ advertising RDMA-capable resources. ## 6. NIC Name Template Not Applied **Symptom**: NIC interfaces on nodes are not renamed according to the expected naming -pattern (e.g., `eth_r0`, `eth_r1`). PCI addresses are used instead of friendly names. +pattern (e.g., `eth_r0`, `eth_r1`). PCI addresses are used instead of friendly names, +or deploy waits on `InterfaceNameMismatch` and fails after five minutes. -**Root cause**: The NIC configuration operator is not deployed, or -`deployNicInterfaceNameTemplate` is not enabled. +**Root cause**: The NIC configuration operator is not deployed, +`deployNicInterfaceNameTemplate` is not enabled, or the generated udev rules +have not taken effect on every targeted node. **Remediation**: 1. Verify `nicConfigurationOperator.deployNicInterfaceNameTemplate: true` in your config. @@ -112,8 +114,12 @@ pattern (e.g., `eth_r0`, `eth_r1`). PCI addresses are used instead of friendly n `kubectl get pods -n nvidia-network-operator -l app=nic-configuration-operator` 3. NIC name templates are only enabled when needed: merged groups with cross-rail PCI conflicts, or `rdma_shared` deployment with empty NetworkInterface fields. -4. If the operator is running but templates are not applied, check its logs for errors. -5. A node reboot may be required for NIC renaming to take effect. +4. Inspect the per-device reason with + `kubectl get nicdevice -n nvidia-network-operator -o yaml`; the + `InterfaceNameApplied` condition lists expected and actual names. +5. Launch Kit treats a mismatch as retryable for five minutes while udev rules settle. + If it persists, check the NIC configuration daemon logs and udev rules. +6. A node reboot may be required for NIC renaming to take effect. --- From 3443a23e0e6f0fd95d2c5c11e9eaae72aeec8658 Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Tue, 18 Aug 2026 13:38:55 +0200 Subject: [PATCH 2/4] Fix Spectrum-X workload rail names Use v1alpha2 railTopology names for NAD annotations, device-plugin requests, and DRA selectors in RA2.2 and RA2.3 workloads. Preserve the explicit legacy naming used by the RA2.1 profile. Signed-off-by: Alexander Maslennikov --- docs/user/spectrum-x.md | 7 ++ pkg/networkoperatorplugin/grouping_test.go | 12 +- .../spectrumx_railpool_test.go | 113 ++++++++++++++++++ pkg/networkoperatorplugin/workload.go | 37 +++++- pkg/networkoperatorplugin/workload_test.go | 67 +++++++++-- .../85-resourceclaimtemplate.yaml | 4 +- .../90-example-daemonset.yaml | 10 +- profiles/spectrum-x-ra2.2/README.md | 6 +- .../spectrum-x/85-resourceclaimtemplate.yaml | 4 +- profiles/spectrum-x/90-example-daemonset.yaml | 10 +- profiles/spectrum-x/README.md | 6 +- .../references/profiles-summary.md | 8 +- 12 files changed, 244 insertions(+), 40 deletions(-) diff --git a/docs/user/spectrum-x.md b/docs/user/spectrum-x.md index 16a33182..05d9b0e4 100644 --- a/docs/user/spectrum-x.md +++ b/docs/user/spectrum-x.md @@ -207,6 +207,13 @@ l8k generate \ --topology-file ./topology.json ``` +For the RA2.2 and RA2.3 v1alpha2 profiles, every +`SpectrumXRailPoolConfig.spec.railTopology[].name` is consumer-visible. The +Spectrum-X Operator uses it as both the NetworkAttachmentDefinition name and +the device-plugin resource suffix. Launch Kit therefore renders `rail0` and +`nvidia.com/rail0` for a per-rail workload, or `rail0p0` and +`nvidia.com/rail0p0` for a per-rail-plane workload. + Both `ipv4` and `ipv6` generate complete nv-ipam CIDRPools. IPv4 preserves the existing per-node `/31` allocation. IPv6 uses the standard Spectrum-X layout: diff --git a/pkg/networkoperatorplugin/grouping_test.go b/pkg/networkoperatorplugin/grouping_test.go index 59b24934..1d707106 100644 --- a/pkg/networkoperatorplugin/grouping_test.go +++ b/pkg/networkoperatorplugin/grouping_test.go @@ -525,14 +525,14 @@ func TestSpectrumXDRAOptInRendering(t *testing.T) { require.Contains(t, claims, "kind: ResourceClaimTemplate") require.Contains(t, claims, "deviceClassName: gpu.nvidia.com") require.Contains(t, claims, "deviceClassName: sriovnetwork.k8snetworkplumbingwg.io") - require.Contains(t, claims, `device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_0"`) + require.Contains(t, claims, `device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail0"`) require.Contains(t, claims, `device.attributes["resource.kubernetes.io"].pcieRoot == "pci0000:00"`) workload := rendered["90-example-daemonset-gpu-model-y.yaml"] require.Contains(t, workload, "resourceClaims:") require.Contains(t, workload, "resourceClaimTemplateName: rail-0-template-gpu-model-y") require.Contains(t, workload, "claims:") - require.NotContains(t, workload, "nvidia.com/rail_0: \"1\"", + require.NotContains(t, workload, "nvidia.com/rail0: \"1\"", "DRA workload must use resource claims instead of device-plugin resource requests") } @@ -545,8 +545,8 @@ func TestSpectrumXDRAOptInRenderingSWPLB(t *testing.T) { claims := rendered["85-resourceclaimtemplate-gpu-model-y.yaml"] require.Contains(t, claims, "name: rail-0-plane-0-template-gpu-model-y") require.Contains(t, claims, "name: rail-0-plane-1-template-gpu-model-y") - require.Contains(t, claims, `device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_0_plane_0"`) - require.Contains(t, claims, `device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_0_plane_1"`) + require.Contains(t, claims, `device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail0p0"`) + require.Contains(t, claims, `device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail0p1"`) require.NotContains(t, claims, "count: 2", "swplb DRA must request one VF per rail-plane claim") @@ -555,7 +555,7 @@ func TestSpectrumXDRAOptInRenderingSWPLB(t *testing.T) { require.Contains(t, workload, "- name: rail-0-plane-1") require.Contains(t, workload, "resourceClaimTemplateName: rail-0-plane-0-template-gpu-model-y") require.Contains(t, workload, "resourceClaimTemplateName: rail-0-plane-1-template-gpu-model-y") - require.NotContains(t, workload, "nvidia.com/rail_0_plane_0: \"1\"", + require.NotContains(t, workload, "nvidia.com/rail0p0: \"1\"", "swplb DRA workload must use resource claims instead of device-plugin resource requests") } @@ -575,7 +575,7 @@ func TestSpectrumXDRADisabledByDefault(t *testing.T) { workload := rendered["90-example-daemonset-gpu-model-y.yaml"] require.NotContains(t, workload, "resourceClaims:") - require.Contains(t, workload, "nvidia.com/rail_0: \"1\"", + require.Contains(t, workload, "nvidia.com/rail0: \"1\"", "non-DRA mode must keep device-plugin resource requests") } diff --git a/pkg/networkoperatorplugin/spectrumx_railpool_test.go b/pkg/networkoperatorplugin/spectrumx_railpool_test.go index a281326b..946f14be 100644 --- a/pkg/networkoperatorplugin/spectrumx_railpool_test.go +++ b/pkg/networkoperatorplugin/spectrumx_railpool_test.go @@ -18,6 +18,7 @@ package networkoperatorplugin import ( "path/filepath" + "strings" "testing" "github.com/nvidia/k8s-launch-kit/pkg/config" @@ -25,6 +26,118 @@ import ( "sigs.k8s.io/yaml" ) +func TestSpectrumXWorkloadNamesMatchRailTopology(t *testing.T) { + tests := []struct { + name, profileDir, version, mode string + planes int + wantNames []string + }{ + { + name: "RA2.2 hwplb", profileDir: "spectrum-x-ra2.2", version: "RA2.2", mode: "hwplb", + planes: 4, wantNames: []string{"rail0", "rail1"}, + }, + { + name: "RA2.2 swplb", profileDir: "spectrum-x-ra2.2", version: "RA2.2", mode: "swplb", + planes: 2, wantNames: []string{"rail0p0", "rail0p1", "rail1p0", "rail1p1"}, + }, + { + name: "RA2.3 hwplb", profileDir: "spectrum-x", version: "RA2.3", mode: "hwplb", + planes: 4, wantNames: []string{"rail0", "rail1"}, + }, + { + name: "RA2.3 swplb", profileDir: "spectrum-x", version: "RA2.3", mode: "swplb", + planes: 2, wantNames: []string{"rail0p0", "rail0p1", "rail1p0", "rail1p1"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cfg := &config.LaunchKitConfig{ + NetworkOperator: &config.NetworkOperatorConfig{Namespace: "network-operator"}, + CurrentNetworkNamespace: "default", + SpectrumX: &config.SpectrumXConfig{ + HWPLB: &config.SpectrumXInterfaceNamePrefixConfig{ + NetdevPrefix: "eth_r%rail_id%_p%plane_id%", + }, + SWPLB: &config.SpectrumXInterfaceNamePrefixConfig{ + NetdevPrefix: "eth_r%rail_id%_p%plane_id%", + }, + }, + Profile: &config.Profile{ + Fabric: "ethernet", Deployment: "sriov", Multirail: true, + SpectrumX: &config.ProfileSpectrumX{ + Enable: true, SPCXVersion: test.version, + MultiplaneMode: test.mode, NumberOfPlanes: test.planes, + }, + }, + Validation: config.DefaultValidationConfig(), + ClusterConfig: []config.ClusterConfig{{ + PFs: []config.PFConfig{ + {Traffic: "east-west", Rail: intPtr(0)}, + {Traffic: "east-west", Rail: intPtr(1)}, + }, + }}, + } + + profilePath := filepath.Join("..", "..", "profiles", test.profileDir) + railPools, err := ProcessTemplate(filepath.Join(profilePath, "80-spectrumxrailpoolconfig.yaml"), cfg, "") + require.NoError(t, err) + railPool := railPools["80-spectrumxrailpoolconfig.yaml"] + require.NotEmpty(t, railPool) + + var railPoolObject struct { + Spec struct { + RailTopology []struct { + Name string `yaml:"name"` + } `yaml:"railTopology"` + } `yaml:"spec"` + } + require.NoError(t, yaml.Unmarshal([]byte(railPool), &railPoolObject)) + var topologyNames []string + for _, rail := range railPoolObject.Spec.RailTopology { + topologyNames = append(topologyNames, rail.Name) + } + require.Equal(t, test.wantNames, topologyNames) + + workloads, err := ProcessTemplate(filepath.Join(profilePath, "90-example-daemonset.yaml"), cfg, "") + require.NoError(t, err) + workload := workloads["90-example-daemonset.yaml"] + require.NotEmpty(t, workload) + + var daemonSet struct { + Spec struct { + Template struct { + Metadata struct { + Annotations map[string]string `yaml:"annotations"` + } `yaml:"metadata"` + Spec struct { + Containers []struct { + Resources struct { + Requests map[string]string `yaml:"requests"` + Limits map[string]string `yaml:"limits"` + } `yaml:"resources"` + } `yaml:"containers"` + } `yaml:"spec"` + } `yaml:"template"` + } `yaml:"spec"` + } + require.NoError(t, yaml.Unmarshal([]byte(workload), &daemonSet)) + require.Equal(t, strings.Join(topologyNames, ","), + daemonSet.Spec.Template.Metadata.Annotations["k8s.v1.cni.cncf.io/networks"]) + require.NotEmpty(t, daemonSet.Spec.Template.Spec.Containers) + requests := daemonSet.Spec.Template.Spec.Containers[0].Resources.Requests + limits := daemonSet.Spec.Template.Spec.Containers[0].Resources.Limits + require.Len(t, requests, len(topologyNames)) + require.Len(t, limits, len(topologyNames)) + for _, railName := range topologyNames { + resourceName := "nvidia.com/" + railName + require.Equal(t, "1", requests[resourceName]) + require.Equal(t, "1", limits[resourceName]) + } + }) + } +} + func TestSpectrumXRailPoolTemplatesStaySchemaCompatibleAndBounded(t *testing.T) { tests := []struct { name string diff --git a/pkg/networkoperatorplugin/workload.go b/pkg/networkoperatorplugin/workload.go index 3f26e811..dfad5982 100644 --- a/pkg/networkoperatorplugin/workload.go +++ b/pkg/networkoperatorplugin/workload.go @@ -120,12 +120,12 @@ func buildNetworkAnnotation(cfg *config.LaunchKitConfig, group *config.ClusterCo if cfg.Profile.SpectrumX.MultiplaneMode == "swplb" { for i := range ewPFs { for p := 0; p < cfg.Profile.SpectrumX.NumberOfPlanes; p++ { - parts = append(parts, fmt.Sprintf("rail-%d-plane-%d", i, p)) + parts = append(parts, spectrumXNetworkName(cfg.Profile.SpectrumX, i, p)) } } } else { for i := range ewPFs { - parts = append(parts, fmt.Sprintf("rail-%d", i)) + parts = append(parts, spectrumXNetworkName(cfg.Profile.SpectrumX, i, 0)) } } return strings.Join(parts, ",") @@ -189,12 +189,12 @@ func buildNetworkResources(cfg *config.LaunchKitConfig, group *config.ClusterCon if cfg.Profile.SpectrumX.MultiplaneMode == "swplb" { for i := range ewPFs { for p := 0; p < cfg.Profile.SpectrumX.NumberOfPlanes; p++ { - resources[fmt.Sprintf("nvidia.com/rail_%d_plane_%d", i, p)] = "1" + resources["nvidia.com/"+spectrumXResourceName(cfg.Profile.SpectrumX, i, p)] = "1" } } } else { for i := range ewPFs { - resources[fmt.Sprintf("nvidia.com/rail_%d", i)] = "1" + resources["nvidia.com/"+spectrumXResourceName(cfg.Profile.SpectrumX, i, 0)] = "1" } } return resources @@ -239,6 +239,35 @@ func buildNetworkResources(cfg *config.LaunchKitConfig, group *config.ClusterCon return resources } +// spectrumXNetworkName returns the NetworkAttachmentDefinition name created +// for a rail. RA2.2+ derives the name directly from railTopology[].name; +// RA2.1 creates the OVSNetwork resources explicitly with legacy separators. +func spectrumXNetworkName(spcx *config.ProfileSpectrumX, rail, plane int) string { + if spcx.SPCXVersion == "RA2.1" { + if spcx.MultiplaneMode == "swplb" { + return fmt.Sprintf("rail-%d-plane-%d", rail, plane) + } + return fmt.Sprintf("rail-%d", rail) + } + if spcx.MultiplaneMode == "swplb" { + return fmt.Sprintf("rail%dp%d", rail, plane) + } + return fmt.Sprintf("rail%d", rail) +} + +// spectrumXResourceName returns the device-plugin resource name created for a +// rail. It matches railTopology[].name in RA2.2+, while RA2.1 uses the explicit +// SriovNetworkNodePolicy resourceName from its legacy profile. +func spectrumXResourceName(spcx *config.ProfileSpectrumX, rail, plane int) string { + if spcx.SPCXVersion == "RA2.1" { + if spcx.MultiplaneMode == "swplb" { + return fmt.Sprintf("rail_%d_plane_%d", rail, plane) + } + return fmt.Sprintf("rail_%d", rail) + } + return spectrumXNetworkName(spcx, rail, plane) +} + // buildNodeAffinity builds a Kubernetes node affinity structure from a label selector map. func buildNodeAffinity(nodeSelector map[string]string) map[string]interface{} { var expressions []interface{} diff --git a/pkg/networkoperatorplugin/workload_test.go b/pkg/networkoperatorplugin/workload_test.go index 3095e1be..10f93506 100644 --- a/pkg/networkoperatorplugin/workload_test.go +++ b/pkg/networkoperatorplugin/workload_test.go @@ -245,11 +245,12 @@ func TestBuildNetworkAnnotation(t *testing.T) { assert.Equal(t, "ipoib-net", got) }) - t.Run("spectrum-x swplb", func(t *testing.T) { + t.Run("spectrum-x RA2.2 swplb", func(t *testing.T) { cfg := &config.LaunchKitConfig{ Profile: &config.Profile{ SpectrumX: &config.ProfileSpectrumX{ Enable: true, + SPCXVersion: "RA2.2", MultiplaneMode: "swplb", NumberOfPlanes: 2, }, @@ -262,14 +263,15 @@ func TestBuildNetworkAnnotation(t *testing.T) { }, } got := buildNetworkAnnotation(cfg, group) - assert.Equal(t, "rail-0-plane-0,rail-0-plane-1,rail-1-plane-0,rail-1-plane-1", got) + assert.Equal(t, "rail0p0,rail0p1,rail1p0,rail1p1", got) }) - t.Run("spectrum-x hwplb", func(t *testing.T) { + t.Run("spectrum-x RA2.3 hwplb", func(t *testing.T) { cfg := &config.LaunchKitConfig{ Profile: &config.Profile{ SpectrumX: &config.ProfileSpectrumX{ Enable: true, + SPCXVersion: "RA2.3", MultiplaneMode: "hwplb", }, }, @@ -281,7 +283,25 @@ func TestBuildNetworkAnnotation(t *testing.T) { }, } got := buildNetworkAnnotation(cfg, group) - assert.Equal(t, "rail-0,rail-1", got) + assert.Equal(t, "rail0,rail1", got) + }) + + t.Run("spectrum-x RA2.1 keeps legacy names", func(t *testing.T) { + cfg := &config.LaunchKitConfig{ + Profile: &config.Profile{ + SpectrumX: &config.ProfileSpectrumX{ + Enable: true, + SPCXVersion: "RA2.1", + MultiplaneMode: "swplb", + NumberOfPlanes: 2, + }, + }, + } + group := &config.ClusterConfig{ + PFs: []config.PFConfig{{Traffic: "east-west", Rail: intPtr(0)}}, + } + got := buildNetworkAnnotation(cfg, group) + assert.Equal(t, "rail-0-plane-0,rail-0-plane-1", got) }) t.Run("nil profile returns empty", func(t *testing.T) { @@ -328,11 +348,12 @@ func TestBuildNetworkResources(t *testing.T) { assert.Equal(t, map[string]string{"rdma/shared_rdma": "1"}, got) }) - t.Run("spectrum-x swplb", func(t *testing.T) { + t.Run("spectrum-x RA2.2 swplb", func(t *testing.T) { cfg := &config.LaunchKitConfig{ Profile: &config.Profile{ SpectrumX: &config.ProfileSpectrumX{ Enable: true, + SPCXVersion: "RA2.2", MultiplaneMode: "swplb", NumberOfPlanes: 2, }, @@ -346,18 +367,19 @@ func TestBuildNetworkResources(t *testing.T) { } got := buildNetworkResources(cfg, group) assert.Equal(t, map[string]string{ - "nvidia.com/rail_0_plane_0": "1", - "nvidia.com/rail_0_plane_1": "1", - "nvidia.com/rail_1_plane_0": "1", - "nvidia.com/rail_1_plane_1": "1", + "nvidia.com/rail0p0": "1", + "nvidia.com/rail0p1": "1", + "nvidia.com/rail1p0": "1", + "nvidia.com/rail1p1": "1", }, got) }) - t.Run("spectrum-x hwplb", func(t *testing.T) { + t.Run("spectrum-x RA2.3 hwplb", func(t *testing.T) { cfg := &config.LaunchKitConfig{ Profile: &config.Profile{ SpectrumX: &config.ProfileSpectrumX{ Enable: true, + SPCXVersion: "RA2.3", MultiplaneMode: "hwplb", }, }, @@ -370,8 +392,29 @@ func TestBuildNetworkResources(t *testing.T) { } got := buildNetworkResources(cfg, group) assert.Equal(t, map[string]string{ - "nvidia.com/rail_0": "1", - "nvidia.com/rail_1": "1", + "nvidia.com/rail0": "1", + "nvidia.com/rail1": "1", + }, got) + }) + + t.Run("spectrum-x RA2.1 keeps legacy names", func(t *testing.T) { + cfg := &config.LaunchKitConfig{ + Profile: &config.Profile{ + SpectrumX: &config.ProfileSpectrumX{ + Enable: true, + SPCXVersion: "RA2.1", + MultiplaneMode: "swplb", + NumberOfPlanes: 2, + }, + }, + } + group := &config.ClusterConfig{ + PFs: []config.PFConfig{{Traffic: "east-west", Rail: intPtr(0)}}, + } + got := buildNetworkResources(cfg, group) + assert.Equal(t, map[string]string{ + "nvidia.com/rail_0_plane_0": "1", + "nvidia.com/rail_0_plane_1": "1", }, got) }) diff --git a/profiles/spectrum-x-ra2.2/85-resourceclaimtemplate.yaml b/profiles/spectrum-x-ra2.2/85-resourceclaimtemplate.yaml index 00a8ac90..a52a19a0 100644 --- a/profiles/spectrum-x-ra2.2/85-resourceclaimtemplate.yaml +++ b/profiles/spectrum-x-ra2.2/85-resourceclaimtemplate.yaml @@ -41,7 +41,7 @@ spec: count: 1 selectors: - cel: - expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_{{$railIdx}}_plane_{{$planeIdx}}"' + expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail{{$railIdx}}p{{$planeIdx}}"' {{- if $vfRoot }} - cel: expression: 'device.attributes["resource.kubernetes.io"].pcieRoot == "{{$vfRoot}}"' @@ -77,7 +77,7 @@ spec: count: {{$planes}} selectors: - cel: - expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_{{$railIdx}}"' + expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail{{$railIdx}}"' {{- if $vfRoot }} - cel: expression: 'device.attributes["resource.kubernetes.io"].pcieRoot == "{{$vfRoot}}"' diff --git a/profiles/spectrum-x-ra2.2/90-example-daemonset.yaml b/profiles/spectrum-x-ra2.2/90-example-daemonset.yaml index 43ceaa71..45f9bbdc 100644 --- a/profiles/spectrum-x-ra2.2/90-example-daemonset.yaml +++ b/profiles/spectrum-x-ra2.2/90-example-daemonset.yaml @@ -16,7 +16,7 @@ spec: {{- $planes := .Profile.SpectrumX.NumberOfPlanes }} {{- $railCount := railCount .ClusterConfig.PFs $planes }} {{- $swplb := eq .Profile.SpectrumX.MultiplaneMode "swplb" }} - k8s.v1.cni.cncf.io/networks: {{ $first := true }}{{- range $railIdx := untilStep 0 $railCount 1 }}{{- if $swplb }}{{- range $planeIdx := untilStep 0 $planes 1 }}{{- if not $first }},{{ end }}{{- $first = false }}rail-{{$railIdx}}-plane-{{$planeIdx}}{{- end }}{{- else }}{{- if not $first }},{{ end }}{{- $first = false }}rail-{{$railIdx}}{{- end }}{{- end }} + k8s.v1.cni.cncf.io/networks: {{ $first := true }}{{- range $railIdx := untilStep 0 $railCount 1 }}{{- if $swplb }}{{- range $planeIdx := untilStep 0 $planes 1 }}{{- if not $first }},{{ end }}{{- $first = false }}rail{{$railIdx}}p{{$planeIdx}}{{- end }}{{- else }}{{- if not $first }},{{ end }}{{- $first = false }}rail{{$railIdx}}{{- end }}{{- end }} spec: {{- if $.NetworkOperator.ImagePullSecrets }} imagePullSecrets: @@ -83,10 +83,10 @@ spec: {{- range $railIdx := untilStep 0 $railCount 1 }} {{- if $swplb }} {{- range $planeIdx := untilStep 0 $planes 1 }} - nvidia.com/rail_{{$railIdx}}_plane_{{$planeIdx}}: "1" + nvidia.com/rail{{$railIdx}}p{{$planeIdx}}: "1" {{- end }} {{- else }} - nvidia.com/rail_{{$railIdx}}: "1" + nvidia.com/rail{{$railIdx}}: "1" {{- end }} {{- end }} limits: @@ -96,10 +96,10 @@ spec: {{- range $railIdx := untilStep 0 $railCount 1 }} {{- if $swplb }} {{- range $planeIdx := untilStep 0 $planes 1 }} - nvidia.com/rail_{{$railIdx}}_plane_{{$planeIdx}}: "1" + nvidia.com/rail{{$railIdx}}p{{$planeIdx}}: "1" {{- end }} {{- else }} - nvidia.com/rail_{{$railIdx}}: "1" + nvidia.com/rail{{$railIdx}}: "1" {{- end }} {{- end }} {{- end }} diff --git a/profiles/spectrum-x-ra2.2/README.md b/profiles/spectrum-x-ra2.2/README.md index 3efd8595..302fe744 100644 --- a/profiles/spectrum-x-ra2.2/README.md +++ b/profiles/spectrum-x-ra2.2/README.md @@ -113,6 +113,8 @@ The profile generates the following Kubernetes Custom Resources: - Single `v1alpha2` resource with `railTopology[]`. In swplb, one entry per rail-plane; otherwise one entry per rail grouping all planes. `draEnabled` is rendered from `profile.spectrumX.useDRA`; the default is explicit `false`. + - Each topology name is also the operator-generated NetworkAttachmentDefinition + and device-plugin resource name: `rail0` per rail or `rail0p0` per rail-plane. 6. **ResourceClaimTemplate** (`85-resourceclaimtemplate.yaml`) - Rendered only when `profile.spectrumX.useDRA: true`. Each template requests @@ -120,7 +122,9 @@ The profile generates the following Kubernetes Custom Resources: 7. **Example DaemonSet** (`90-example-daemonset.yaml`) - Example workload requesting one VF per rail (non-swplb) or per rail-plane (swplb). - In DRA mode, it references the generated `ResourceClaimTemplate` resources instead. + Its network annotations and resource requests use the corresponding + `railTopology[].name`. In DRA mode, it references the generated + `ResourceClaimTemplate` resources instead. `NicFirmwareSource` and `NicFirmwareTemplate` must be applied separately by the operator; l8k does not generate them. diff --git a/profiles/spectrum-x/85-resourceclaimtemplate.yaml b/profiles/spectrum-x/85-resourceclaimtemplate.yaml index 00a8ac90..a52a19a0 100644 --- a/profiles/spectrum-x/85-resourceclaimtemplate.yaml +++ b/profiles/spectrum-x/85-resourceclaimtemplate.yaml @@ -41,7 +41,7 @@ spec: count: 1 selectors: - cel: - expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_{{$railIdx}}_plane_{{$planeIdx}}"' + expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail{{$railIdx}}p{{$planeIdx}}"' {{- if $vfRoot }} - cel: expression: 'device.attributes["resource.kubernetes.io"].pcieRoot == "{{$vfRoot}}"' @@ -77,7 +77,7 @@ spec: count: {{$planes}} selectors: - cel: - expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail_{{$railIdx}}"' + expression: 'device.attributes["k8s.cni.cncf.io"].resourceName == "nvidia.com/rail{{$railIdx}}"' {{- if $vfRoot }} - cel: expression: 'device.attributes["resource.kubernetes.io"].pcieRoot == "{{$vfRoot}}"' diff --git a/profiles/spectrum-x/90-example-daemonset.yaml b/profiles/spectrum-x/90-example-daemonset.yaml index 43ceaa71..45f9bbdc 100644 --- a/profiles/spectrum-x/90-example-daemonset.yaml +++ b/profiles/spectrum-x/90-example-daemonset.yaml @@ -16,7 +16,7 @@ spec: {{- $planes := .Profile.SpectrumX.NumberOfPlanes }} {{- $railCount := railCount .ClusterConfig.PFs $planes }} {{- $swplb := eq .Profile.SpectrumX.MultiplaneMode "swplb" }} - k8s.v1.cni.cncf.io/networks: {{ $first := true }}{{- range $railIdx := untilStep 0 $railCount 1 }}{{- if $swplb }}{{- range $planeIdx := untilStep 0 $planes 1 }}{{- if not $first }},{{ end }}{{- $first = false }}rail-{{$railIdx}}-plane-{{$planeIdx}}{{- end }}{{- else }}{{- if not $first }},{{ end }}{{- $first = false }}rail-{{$railIdx}}{{- end }}{{- end }} + k8s.v1.cni.cncf.io/networks: {{ $first := true }}{{- range $railIdx := untilStep 0 $railCount 1 }}{{- if $swplb }}{{- range $planeIdx := untilStep 0 $planes 1 }}{{- if not $first }},{{ end }}{{- $first = false }}rail{{$railIdx}}p{{$planeIdx}}{{- end }}{{- else }}{{- if not $first }},{{ end }}{{- $first = false }}rail{{$railIdx}}{{- end }}{{- end }} spec: {{- if $.NetworkOperator.ImagePullSecrets }} imagePullSecrets: @@ -83,10 +83,10 @@ spec: {{- range $railIdx := untilStep 0 $railCount 1 }} {{- if $swplb }} {{- range $planeIdx := untilStep 0 $planes 1 }} - nvidia.com/rail_{{$railIdx}}_plane_{{$planeIdx}}: "1" + nvidia.com/rail{{$railIdx}}p{{$planeIdx}}: "1" {{- end }} {{- else }} - nvidia.com/rail_{{$railIdx}}: "1" + nvidia.com/rail{{$railIdx}}: "1" {{- end }} {{- end }} limits: @@ -96,10 +96,10 @@ spec: {{- range $railIdx := untilStep 0 $railCount 1 }} {{- if $swplb }} {{- range $planeIdx := untilStep 0 $planes 1 }} - nvidia.com/rail_{{$railIdx}}_plane_{{$planeIdx}}: "1" + nvidia.com/rail{{$railIdx}}p{{$planeIdx}}: "1" {{- end }} {{- else }} - nvidia.com/rail_{{$railIdx}}: "1" + nvidia.com/rail{{$railIdx}}: "1" {{- end }} {{- end }} {{- end }} diff --git a/profiles/spectrum-x/README.md b/profiles/spectrum-x/README.md index 5d98da95..1105d003 100644 --- a/profiles/spectrum-x/README.md +++ b/profiles/spectrum-x/README.md @@ -126,6 +126,8 @@ The profile generates the following Kubernetes Custom Resources: - Single `v1alpha2` resource with `railTopology[]`. In swplb, one entry per rail-plane; otherwise one entry per rail grouping all planes. `draEnabled` is rendered from `profile.spectrumX.useDRA`; the default is explicit `false`. + - Each topology name is also the operator-generated NetworkAttachmentDefinition + and device-plugin resource name: `rail0` per rail or `rail0p0` per rail-plane. 7. **ResourceClaimTemplate** (`85-resourceclaimtemplate.yaml`) - Rendered only when `profile.spectrumX.useDRA: true`. Each template requests @@ -133,7 +135,9 @@ The profile generates the following Kubernetes Custom Resources: 8. **Example DaemonSet** (`90-example-daemonset.yaml`) - Example workload requesting one VF per rail (non-swplb) or per rail-plane (swplb). - In DRA mode, it references the generated `ResourceClaimTemplate` resources instead. + Its network annotations and resource requests use the corresponding + `railTopology[].name`. In DRA mode, it references the generated + `ResourceClaimTemplate` resources instead. `NicFirmwareSource` and `NicFirmwareTemplate` must be applied separately by the operator; l8k does not generate them. diff --git a/skills/k8s-launch-kit-generate/references/profiles-summary.md b/skills/k8s-launch-kit-generate/references/profiles-summary.md index 3a91e1f8..2eaf33a0 100644 --- a/skills/k8s-launch-kit-generate/references/profiles-summary.md +++ b/skills/k8s-launch-kit-generate/references/profiles-summary.md @@ -94,7 +94,9 @@ direct-drain controllers. modes. It deploys the Spectrum-X profile through a ConfigMap and emits one `SpectrumXRailPoolConfig` (`v1alpha2`). In `swplb`, `railTopology[]` splits each rail into per-plane entries; in other modes one entry per rail groups - all planes. + all planes. The operator uses each topology name for both its + NetworkAttachmentDefinition and device-plugin resource: `rail0` per rail or + `rail0p0` per rail-plane. - **Templates**: - `10-nicclusterpolicy.yaml` -- NicClusterPolicy (with `nicFirmwareStorage` and `spectrumXOperator.xPlane`) @@ -123,7 +125,9 @@ direct-drain controllers. - **Node Capabilities**: `sriov: true`, `rdma: true` - **Description**: RA2.2 variant of the consolidated v1alpha2 profile. In `swplb`, `railTopology[]` splits each rail into per-plane entries; in other - modes one entry per rail groups all planes. + modes one entry per rail groups all planes. Generated workloads consume the + topology names directly (`rail0` or `rail0p0`) for both the network + annotation and `nvidia.com/` resource request. - **Templates**: - `10-nicclusterpolicy.yaml` -- NicClusterPolicy with Spectrum-X Operator - `25-nicinterfacenametemplate.yaml` -- Multi-rail interface naming From d54782cd416cdf3e6b683bac92663aa32496f6cc Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Tue, 18 Aug 2026 13:47:48 +0200 Subject: [PATCH 3/4] Scope interface mismatch retry handling Keep InterfaceNameMismatch terminal for one-shot validation and mark it retryable for deploy. Start the five-minute window only after deploy observes that retryable mismatch, leaving normal initialization unbounded. Signed-off-by: Alexander Maslennikov --- README.md | 11 ++-- docs/advanced/deployment.md | 4 +- pkg/cmd/deploy.go | 5 +- .../crstate/nicconfig.go | 55 +++++++++------- .../crstate/nicconfig_test.go | 9 +-- pkg/networkoperatorplugin/crstate/state.go | 13 ++-- pkg/networkoperatorplugin/deploy.go | 64 +++++++++++-------- pkg/networkoperatorplugin/deploy_test.go | 43 +++++++++++-- skills/k8s-launch-kit-deploy/SKILL.md | 10 +-- 9 files changed, 136 insertions(+), 78 deletions(-) diff --git a/README.md b/README.md index db8f82b4..f9da5677 100644 --- a/README.md +++ b/README.md @@ -351,11 +351,12 @@ to `` itself. `--dry-run` does a server-side dry run. `--deploy-timeout` caps the whole apply+reconcile phase end-to-end (e.g. `--deploy-timeout 90m`); without it, deploy polls indefinitely — right for SR-IOV on large clusters where reconciliation can take an hour. `NicInterfaceNameTemplate` is the -bounded exception: an `InterfaceNameMismatch` remains `IN-PROGRESS` for up to -five minutes while the NIC configuration daemon retries the udev rename. This -gates verification of later manifests without aborting on the first transient -mismatch. If the names still do not match after five minutes, deployment fails -with the per-device mismatch details. A shorter `--deploy-timeout` still wins. +bounded exception: once an `InterfaceNameMismatch` is observed, Launch Kit +retries it for up to five minutes while the NIC configuration daemon retries +the udev rename. This gates verification of later manifests without aborting +on the first transient mismatch. If the names still do not match after five +minutes, deployment fails with the per-device mismatch details. A shorter +`--deploy-timeout` still wins. Initial discovery remains unbounded. When Launch Kit installs or upgrades the Network Operator chart, its Helm post-renderer adds the same version annotation to chart-rendered resources. diff --git a/docs/advanced/deployment.md b/docs/advanced/deployment.md index 71f2fc9d..9073881e 100644 --- a/docs/advanced/deployment.md +++ b/docs/advanced/deployment.md @@ -81,11 +81,11 @@ Kind-specific checks include per-component Network Operator state, SR-IOV per-no For `NicConfigurationTemplate` and `NicFirmwareTemplate`, Launch Kit first waits for the operator to publish matched device names in `status.nicDevices` and for that name set to reflect the current `nodeSelector`, NIC type, PCI-address, serial-number, and part-number selectors. It then evaluates only those `NicDevice` objects and waits for the corresponding `spec.configuration` or `spec.firmware` field to reflect the current template payload. A successful device condition is accepted only after its `observedGeneration` catches up with the `NicDevice` generation. A configuration template checks `FirmwareUpdateInProgress` only when the matched device carries `spec.firmware`; without a deployed firmware template, a stale firmware condition from an older device generation does not block configuration reconciliation. Other discovered NICs do not block on configuration or firmware state. Changed templates are also observation-gated before this status is accepted, so status left by an earlier generation cannot produce a false success. -For `NicInterfaceNameTemplate`, `InterfaceNameMismatch` is retryable because the NIC configuration daemon can publish it while newly-written udev rules are still taking effect. Launch Kit keeps that template `IN-PROGRESS` for up to five minutes. Since phase-4 verification is ordered, the template gates later checks during this window. A persistent mismatch fails deployment after the local timeout and retains the per-node and per-port mismatch details. +For `NicInterfaceNameTemplate`, `InterfaceNameMismatch` is retryable because the NIC configuration daemon can publish it while newly-written udev rules are still taking effect. Launch Kit starts a five-minute retry window when it first observes that mismatch. Initial device discovery and other ordinary in-progress states remain unbounded. Since phase-4 verification is ordered, the template gates later checks during this window. A persistent mismatch fails deployment after the local timeout and retains the per-node and per-port mismatch details. ## Timeout -The default deploy budget is unbounded because SR-IOV and driver reconciliation can exceed a small fixed timeout on large clusters. `NicInterfaceNameTemplate` is the only bounded exception: its retryable interface-name reconciliation window is five minutes. A shorter deploy-wide timeout takes precedence. +The default deploy budget is unbounded because SR-IOV and driver reconciliation can exceed a small fixed timeout on large clusters. `NicInterfaceNameTemplate` is the only bounded exception: after the first `InterfaceNameMismatch`, its retry window is five minutes. A shorter deploy-wide timeout takes precedence. Bound the entire Helm, apply, and reconciliation operation to a maintenance window: diff --git a/pkg/cmd/deploy.go b/pkg/cmd/deploy.go index 1f36c5be..f56c79a2 100644 --- a/pkg/cmd/deploy.go +++ b/pkg/cmd/deploy.go @@ -64,8 +64,9 @@ Use --deploy-timeout to bound the entire end-to-end run (e.g. for a maintenance window). Without the flag the deploy waits indefinitely for reconciliation — appropriate for large SR-IOV clusters where a single policy can take an hour or more. NicInterfaceNameTemplate is the bounded -exception: an interface-name mismatch is retried for up to five minutes so -udev rules can settle, then fails before later verification proceeds. +exception: once observed, an interface-name mismatch is retried for up to +five minutes so udev rules can settle, then fails before later verification +proceeds. Initial device discovery remains unbounded. If /network-operator/ exists (the layout 'l8k generate' produces), that subdirectory is used. Otherwise itself diff --git a/pkg/networkoperatorplugin/crstate/nicconfig.go b/pkg/networkoperatorplugin/crstate/nicconfig.go index 45959277..d8913cb3 100644 --- a/pkg/networkoperatorplugin/crstate/nicconfig.go +++ b/pkg/networkoperatorplugin/crstate/nicconfig.go @@ -120,9 +120,11 @@ func nicTemplateValidator(kind templateKind) Validator { } var ( - contributing []unstructured.Unstructured - details = make(map[string]string) - anyInProgress bool + contributing []unstructured.Unstructured + details = make(map[string]string) + anyInProgress bool + anyRetryableError bool + anyNonRetryableError bool ) if templateUsesMatchedDeviceStatus(kind) { devicesByName := make(map[string]*unstructured.Unstructured, len(devices.Items)) @@ -189,15 +191,18 @@ func nicTemplateValidator(kind templateKind) Validator { } // 4. Classify each contributing device. - var anyError bool for i := range contributing { d := &contributing[i] label := deviceLabel(d) - state, reason := classifyDevice(d, kind) + state, reason, retryable := classifyDevice(d, kind) details[label] = reason switch state { case StateError: - anyError = true + if retryable { + anyRetryableError = true + } else { + anyNonRetryableError = true + } case StateInProgress: anyInProgress = true case StateSuccess: @@ -209,8 +214,14 @@ func nicTemplateValidator(kind templateKind) Validator { } switch { - case anyError: - return Result{State: StateError, Reason: summarizeNodeStates(details), Details: details, Source: src}, nil + case anyRetryableError || anyNonRetryableError: + return Result{ + State: StateError, + Reason: summarizeNodeStates(details), + Details: details, + Source: src, + Retryable: anyRetryableError && !anyNonRetryableError, + }, nil case anyInProgress: return Result{State: StateInProgress, Reason: summarizeNodeStates(details), Details: details, Source: src}, nil default: @@ -470,7 +481,7 @@ func deviceLabel(d *unstructured.Unstructured) string { // classifyDevice maps the relevant condition type+status+reason to one of // the four states. The mapping mirrors nic-configuration-operator's // controller (internal/controller/nicdevice_controller.go). -func classifyDevice(d *unstructured.Unstructured, kind templateKind) (CRState, string) { +func classifyDevice(d *unstructured.Unstructured, kind templateKind) (CRState, string, bool) { conds, _, _ := unstructured.NestedSlice(d.Object, "status", "conditions") byType := map[string]map[string]interface{}{} for _, raw := range conds { @@ -488,11 +499,13 @@ func classifyDevice(d *unstructured.Unstructured, kind templateKind) (CRState, s case templateKindInterfaceName: return classifyInterfaceName(byType) case templateKindConfiguration: - return classifyConfiguration(d, byType) + state, reason := classifyConfiguration(d, byType) + return state, reason, false case templateKindFirmware: - return classifyFirmware(d, byType) + state, reason := classifyFirmware(d, byType) + return state, reason, false default: - return StateInProgress, "unknown template kind" + return StateInProgress, "unknown template kind", false } } @@ -528,15 +541,13 @@ func classifyFirmware(device *unstructured.Unstructured, byType map[string]map[s } } -// classifyInterfaceName inspects InterfaceNameApplied. A mismatch is -// retryable because the NIC operator can report it while newly-written udev -// rules are still taking effect. The deploy state machine gives -// NicInterfaceNameTemplate its own bounded reconciliation window, so a -// persistent mismatch still fails before downstream verification proceeds. -func classifyInterfaceName(byType map[string]map[string]interface{}) (CRState, string) { +// classifyInterfaceName inspects InterfaceNameApplied. A mismatch remains an +// error for one-shot validation, but is marked retryable so the deploy state +// machine can give newly-written udev rules a bounded reconciliation window. +func classifyInterfaceName(byType map[string]map[string]interface{}) (CRState, string, bool) { cond, ok := byType[consts.InterfaceNameCondition] if !ok { - return StateInProgress, "InterfaceNameApplied condition not yet set" + return StateInProgress, "InterfaceNameApplied condition not yet set", false } reason, _, _ := unstructured.NestedString(cond, "reason") status, _, _ := unstructured.NestedString(cond, "status") @@ -544,11 +555,11 @@ func classifyInterfaceName(byType map[string]map[string]interface{}) (CRState, s switch { case reason == consts.InterfaceNameAppliedReason && status == "True": - return StateSuccess, "InterfaceNameApplied" + return StateSuccess, "InterfaceNameApplied", false case reason == consts.InterfaceNameMismatchReason: - return StateInProgress, fallbackMessage(message, "interface name mismatch — waiting for udev rules to apply") + return StateError, fallbackMessage(message, "interface name mismatch — udev rules did not apply"), true default: - return StateInProgress, fallbackMessage(fmt.Sprintf("InterfaceNameApplied=%s reason=%s", status, reason), "InterfaceNameApplied unknown") + return StateInProgress, fallbackMessage(fmt.Sprintf("InterfaceNameApplied=%s reason=%s", status, reason), "InterfaceNameApplied unknown"), false } } diff --git a/pkg/networkoperatorplugin/crstate/nicconfig_test.go b/pkg/networkoperatorplugin/crstate/nicconfig_test.go index 50603374..0523b08c 100644 --- a/pkg/networkoperatorplugin/crstate/nicconfig_test.go +++ b/pkg/networkoperatorplugin/crstate/nicconfig_test.go @@ -216,10 +216,10 @@ func TestNicInterfaceNameTemplate_AppliedSuccessfully(t *testing.T) { assert.Equal(t, StateSuccess, res.State) } -func TestNicInterfaceNameTemplate_MismatchIsInProgress(t *testing.T) { +func TestNicInterfaceNameTemplate_MismatchIsRetryableError(t *testing.T) { // The operator can publish a mismatch while newly-written udev rules are - // still taking effect. Deploy keeps polling this state under a bounded - // NicInterfaceNameTemplate reconciliation timeout. + // still taking effect. One-shot validation must retain the error state; + // deploy uses the retryable marker to apply its bounded wait policy. manifest := nicTemplateManifest(nicopKindInterfaceNameTemplate, "tpl", "ns", map[string]string{"role": "worker"}) live := manifest.DeepCopy() c := newClient(t, @@ -236,7 +236,8 @@ func TestNicInterfaceNameTemplate_MismatchIsInProgress(t *testing.T) { v := nicTemplateValidator(templateKindInterfaceName) res, err := v(context.Background(), c, manifest) require.NoError(t, err) - assert.Equal(t, StateInProgress, res.State) + assert.Equal(t, StateError, res.State) + assert.True(t, res.Retryable) assert.Contains(t, res.Reason, "interface name mismatch") } diff --git a/pkg/networkoperatorplugin/crstate/state.go b/pkg/networkoperatorplugin/crstate/state.go index 67409707..8bc439f8 100644 --- a/pkg/networkoperatorplugin/crstate/state.go +++ b/pkg/networkoperatorplugin/crstate/state.go @@ -49,12 +49,15 @@ const ( // short human-readable summary; Details carries structured per-companion // information (e.g. per-node syncStatus for SR-IOV) for richer reports. // Source identifies the object that produced the result (Kind/Name) for -// log breadcrumbs. +// log breadcrumbs. Retryable marks a StateError that a polling caller may +// retry under its own bounded policy; one-shot callers still treat the state +// as an error. type Result struct { - State CRState - Reason string - Details map[string]string - Source string + State CRState + Reason string + Details map[string]string + Source string + Retryable bool } // Validator inspects a manifest object plus whatever companion CRs it diff --git a/pkg/networkoperatorplugin/deploy.go b/pkg/networkoperatorplugin/deploy.go index fe30500a..15ad17b4 100644 --- a/pkg/networkoperatorplugin/deploy.go +++ b/pkg/networkoperatorplugin/deploy.go @@ -93,14 +93,10 @@ type DeployOptions struct { // helper cadence so logs feel familiar. const deployPollInterval = 3 * time.Second -// nicInterfaceNameTemplateReconcileTimeout bounds the one phase-4 resource -// whose normal reconciliation can temporarily report a failure-shaped -// condition. The NIC operator writes InterfaceNameMismatch while udev rules -// are still taking effect, then retries. Treating that first mismatch as -// terminal aborts deployment before the remaining resources can be checked; -// treating it as unbounded progress can hang forever when --deploy-timeout is -// unset. This per-template window preserves the gate while keeping a real -// rename failure bounded. A shorter deploy-wide context deadline still wins. +// nicInterfaceNameTemplateReconcileTimeout bounds the retry window after a +// NicInterfaceNameTemplate first reports InterfaceNameMismatch. Initial NIC +// discovery and other ordinary in-progress states remain governed only by the +// deploy-wide context. A shorter deploy-wide context deadline still wins. const nicInterfaceNameTemplateReconcileTimeout = 5 * time.Minute // appliedManifest pairs an applied "other" manifest with the @@ -456,8 +452,8 @@ func applyAndWait(ctx context.Context, c client.Client, registry *crstate.Regist // pollUntilTerminal polls the registry's Validator for obj until it // reports StateSuccess or StateError. not-deployed transitions trigger a // single re-apply (object vanished between apply and poll). Besides a terminal -// state or ctx.Done(), kinds with a manifestReconcileTimeout also exit when -// that local reconciliation window expires. +// state or ctx.Done(), retryable errors with a configured local window exit if +// they do not reconcile before that window expires. // // awaitObservationAfterRV is the resourceVersion the server returned // from the apply Patch. When non-empty, the poll loop refuses to act @@ -478,18 +474,18 @@ func applyAndWait(ctx context.Context, c client.Client, registry *crstate.Regist // so a noisy 3-second polling loop only emits a fresh line when // something actually changed. func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.Registry, obj *unstructured.Unstructured, label, awaitObservationAfterRV string) error { - return pollUntilTerminalWithReconcileTimeout( - ctx, c, registry, obj, label, awaitObservationAfterRV, manifestReconcileTimeout(obj), + return pollUntilTerminalWithRetryableErrorTimeout( + ctx, c, registry, obj, label, awaitObservationAfterRV, manifestRetryableErrorTimeout(obj), ) } -func pollUntilTerminalWithReconcileTimeout( +func pollUntilTerminalWithRetryableErrorTimeout( ctx context.Context, c client.Client, registry *crstate.Registry, obj *unstructured.Unstructured, label, awaitObservationAfterRV string, - reconcileTimeout time.Duration, + retryableErrorTimeout time.Duration, ) error { uiOutput := ui.FromContext(ctx) progress := uiOutput.StartProgress(fmt.Sprintf("Waiting for %s to reconcile", label)) @@ -498,14 +494,13 @@ func pollUntilTerminalWithReconcileTimeout( ticker := time.NewTicker(deployPollInterval) defer ticker.Stop() - var reconcileTimer *time.Timer - var reconcileTimeoutC <-chan time.Time - if reconcileTimeout > 0 { - reconcileTimer = time.NewTimer(reconcileTimeout) - reconcileTimeoutC = reconcileTimer.C - defer reconcileTimer.Stop() - uiOutput.Info(" %s reconciliation timeout: %s", label, reconcileTimeout) - } + var retryableErrorTimer *time.Timer + var retryableErrorTimeoutC <-chan time.Time + defer func() { + if retryableErrorTimer != nil { + retryableErrorTimer.Stop() + } + }() var lastReason string reportProgress := func(reason string) { @@ -528,19 +523,29 @@ func pollUntilTerminalWithReconcileTimeout( progress.Update(fmt.Sprintf("%s: %s", label, reason)) } } + startRetryableErrorTimer := func() { + if retryableErrorTimeout <= 0 || retryableErrorTimer != nil { + return + } + retryableErrorTimer = time.NewTimer(retryableErrorTimeout) + retryableErrorTimeoutC = retryableErrorTimer.C + uiOutput.Info(" %s retryable-error timeout: %s", label, retryableErrorTimeout) + } timeoutError := func() error { - progress.Fail(fmt.Sprintf("Timed out after %s while waiting for %s", reconcileTimeout, label)) + progress.Fail(fmt.Sprintf("Timed out after %s while waiting for %s", retryableErrorTimeout, label)) if lastReason != "" { - return fmt.Errorf("%s timed out after %s waiting to reconcile: %s", label, reconcileTimeout, lastReason) + return fmt.Errorf("%s timed out after %s waiting for retryable error to reconcile: %s", + label, retryableErrorTimeout, lastReason) } - return fmt.Errorf("%s timed out after %s waiting to reconcile", label, reconcileTimeout) + return fmt.Errorf("%s timed out after %s waiting for retryable error to reconcile", + label, retryableErrorTimeout) } waitForNextPoll := func() error { select { case <-ctx.Done(): progress.Fail(fmt.Sprintf("Cancelled or timed out while waiting for %s", label)) return ctx.Err() - case <-reconcileTimeoutC: + case <-retryableErrorTimeoutC: return timeoutError() case <-ticker.C: return nil @@ -596,6 +601,11 @@ func pollUntilTerminalWithReconcileTimeout( "kind", obj.GetKind(), "name", obj.GetName(), "reason", res.Reason) return nil case crstate.StateError: + if res.Retryable && retryableErrorTimeout > 0 { + startRetryableErrorTimer() + reportProgress(res.Reason) + break + } progress.Fail(fmt.Sprintf("%s error: %s", label, res.Reason)) log.Log.Error(nil, "Manifest reported error", "kind", obj.GetKind(), "name", obj.GetName(), "reason", res.Reason) @@ -623,7 +633,7 @@ func pollUntilTerminalWithReconcileTimeout( } } -func manifestReconcileTimeout(obj *unstructured.Unstructured) time.Duration { +func manifestRetryableErrorTimeout(obj *unstructured.Unstructured) time.Duration { if obj == nil { return 0 } diff --git a/pkg/networkoperatorplugin/deploy_test.go b/pkg/networkoperatorplugin/deploy_test.go index 7b023e23..6de7ed8d 100644 --- a/pkg/networkoperatorplugin/deploy_test.go +++ b/pkg/networkoperatorplugin/deploy_test.go @@ -135,19 +135,19 @@ metadata: }) } -func TestManifestReconcileTimeout(t *testing.T) { +func TestManifestRetryableErrorTimeout(t *testing.T) { interfaceTemplate := &unstructured.Unstructured{} interfaceTemplate.SetGroupVersionKind(schema.GroupVersionKind{ Group: "configuration.net.nvidia.com", Version: "v1alpha1", Kind: "NicInterfaceNameTemplate", }) - assert.Equal(t, 5*time.Minute, manifestReconcileTimeout(interfaceTemplate)) + assert.Equal(t, 5*time.Minute, manifestRetryableErrorTimeout(interfaceTemplate)) configMap := &unstructured.Unstructured{} configMap.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "ConfigMap"}) - assert.Zero(t, manifestReconcileTimeout(configMap)) - assert.Zero(t, manifestReconcileTimeout(nil)) + assert.Zero(t, manifestRetryableErrorTimeout(configMap)) + assert.Zero(t, manifestRetryableErrorTimeout(nil)) } func TestPollUntilTerminal_InterfaceNameMismatchTimesOut(t *testing.T) { @@ -163,12 +163,13 @@ func TestPollUntilTerminal_InterfaceNameMismatchTimesOut(t *testing.T) { registry := crstate.NewRegistry() registry.Register(gvk, func(context.Context, client.Client, *unstructured.Unstructured) (crstate.Result, error) { return crstate.Result{ - State: crstate.StateInProgress, - Reason: "worker-1/0000:05:00.0: interface name mismatch", + State: crstate.StateError, + Reason: "worker-1/0000:05:00.0: interface name mismatch", + Retryable: true, }, nil }) - err := pollUntilTerminalWithReconcileTimeout( + err := pollUntilTerminalWithRetryableErrorTimeout( context.Background(), nil, registry, obj, "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, ) @@ -176,3 +177,31 @@ func TestPollUntilTerminal_InterfaceNameMismatchTimesOut(t *testing.T) { assert.Contains(t, err.Error(), "timed out after 10ms") assert.Contains(t, err.Error(), "interface name mismatch") } + +func TestPollUntilTerminal_InterfaceNameInitializationDoesNotUseMismatchTimeout(t *testing.T) { + gvk := schema.GroupVersionKind{ + Group: "configuration.net.nvidia.com", + Version: "v1alpha1", + Kind: "NicInterfaceNameTemplate", + } + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + obj.SetName("nic-rename") + + registry := crstate.NewRegistry() + registry.Register(gvk, func(context.Context, client.Client, *unstructured.Unstructured) (crstate.Result, error) { + return crstate.Result{ + State: crstate.StateInProgress, + Reason: "waiting for nic-configuration-operator to discover devices", + }, nil + }) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond) + defer cancel() + err := pollUntilTerminalWithRetryableErrorTimeout( + ctx, nil, registry, obj, + "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, + ) + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) +} diff --git a/skills/k8s-launch-kit-deploy/SKILL.md b/skills/k8s-launch-kit-deploy/SKILL.md index f209b5bb..938257a3 100644 --- a/skills/k8s-launch-kit-deploy/SKILL.md +++ b/skills/k8s-launch-kit-deploy/SKILL.md @@ -112,10 +112,12 @@ matched device has `spec.firmware`; configuration-only deployments ignore a stale firmware condition. `NicInterfaceNameTemplate` gates verification of the manifests that follow it. -Treat `InterfaceNameMismatch` as retryable for up to five minutes because the -NIC configuration daemon can publish that condition while new udev rules are -still taking effect. If every targeted device reaches `InterfaceNameApplied`, -continue verification. If a mismatch persists for five minutes, fail with the +Treat `InterfaceNameMismatch` as retryable because the NIC configuration +daemon can publish that condition while new udev rules are still taking +effect. Start the five-minute retry window only when the first mismatch is +observed; initial device discovery and other ordinary in-progress states stay +unbounded. If every targeted device reaches `InterfaceNameApplied`, continue +verification. If a mismatch persists for five minutes, fail with the per-device details. A shorter deploy-wide `--deploy-timeout` takes precedence. During preflight, do not classify `SriovNetworkPoolConfig`, From 9dd012a8cb0be61a9fe7c9d3f112fdf8c17ad0d3 Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Tue, 18 Aug 2026 13:53:09 +0200 Subject: [PATCH 4/4] Reset interface mismatch timer after recovery Stop the local retry timer when a retryable interface-name mismatch clears and reconciliation returns to an ordinary in-progress state. Add a transition test that verifies the deploy-wide context resumes control. Signed-off-by: Alexander Maslennikov --- pkg/networkoperatorplugin/deploy.go | 27 +++++++++++++-- pkg/networkoperatorplugin/deploy_test.go | 42 ++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/pkg/networkoperatorplugin/deploy.go b/pkg/networkoperatorplugin/deploy.go index 15ad17b4..492f52a4 100644 --- a/pkg/networkoperatorplugin/deploy.go +++ b/pkg/networkoperatorplugin/deploy.go @@ -475,7 +475,8 @@ func applyAndWait(ctx context.Context, c client.Client, registry *crstate.Regist // something actually changed. func pollUntilTerminal(ctx context.Context, c client.Client, registry *crstate.Registry, obj *unstructured.Unstructured, label, awaitObservationAfterRV string) error { return pollUntilTerminalWithRetryableErrorTimeout( - ctx, c, registry, obj, label, awaitObservationAfterRV, manifestRetryableErrorTimeout(obj), + ctx, c, registry, obj, label, awaitObservationAfterRV, + manifestRetryableErrorTimeout(obj), deployPollInterval, ) } @@ -486,12 +487,16 @@ func pollUntilTerminalWithRetryableErrorTimeout( obj *unstructured.Unstructured, label, awaitObservationAfterRV string, retryableErrorTimeout time.Duration, + pollInterval time.Duration, ) error { uiOutput := ui.FromContext(ctx) progress := uiOutput.StartProgress(fmt.Sprintf("Waiting for %s to reconcile", label)) log.Log.Info("Waiting for manifest to reconcile", "kind", obj.GetKind(), "name", obj.GetName(), "namespace", obj.GetNamespace()) - ticker := time.NewTicker(deployPollInterval) + if pollInterval <= 0 { + pollInterval = deployPollInterval + } + ticker := time.NewTicker(pollInterval) defer ticker.Stop() var retryableErrorTimer *time.Timer @@ -531,6 +536,19 @@ func pollUntilTerminalWithRetryableErrorTimeout( retryableErrorTimeoutC = retryableErrorTimer.C uiOutput.Info(" %s retryable-error timeout: %s", label, retryableErrorTimeout) } + stopRetryableErrorTimer := func() { + if retryableErrorTimer == nil { + return + } + if !retryableErrorTimer.Stop() { + select { + case <-retryableErrorTimer.C: + default: + } + } + retryableErrorTimer = nil + retryableErrorTimeoutC = nil + } timeoutError := func() error { progress.Fail(fmt.Sprintf("Timed out after %s while waiting for %s", retryableErrorTimeout, label)) if lastReason != "" { @@ -611,6 +629,7 @@ func pollUntilTerminalWithRetryableErrorTimeout( "kind", obj.GetKind(), "name", obj.GetName(), "reason", res.Reason) return fmt.Errorf("%s/%s: %s", obj.GetKind(), obj.GetName(), res.Reason) case crstate.StateNotDeployed: + stopRetryableErrorTimer() // Object vanished between apply and poll (admission // webhook race, manual kubectl delete). Re-apply once // and continue polling. @@ -623,6 +642,10 @@ func pollUntilTerminalWithRetryableErrorTimeout( } reportProgress("re-applied after disappearance") case crstate.StateInProgress: + // The retryable mismatch cleared. Return to the normal, + // deploy-wide reconciliation budget while other devices + // continue initializing. + stopRetryableErrorTimer() reportProgress(res.Reason) } } diff --git a/pkg/networkoperatorplugin/deploy_test.go b/pkg/networkoperatorplugin/deploy_test.go index 6de7ed8d..a3779218 100644 --- a/pkg/networkoperatorplugin/deploy_test.go +++ b/pkg/networkoperatorplugin/deploy_test.go @@ -171,7 +171,7 @@ func TestPollUntilTerminal_InterfaceNameMismatchTimesOut(t *testing.T) { err := pollUntilTerminalWithRetryableErrorTimeout( context.Background(), nil, registry, obj, - "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, + "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, time.Millisecond, ) require.Error(t, err) assert.Contains(t, err.Error(), "timed out after 10ms") @@ -200,8 +200,46 @@ func TestPollUntilTerminal_InterfaceNameInitializationDoesNotUseMismatchTimeout( defer cancel() err := pollUntilTerminalWithRetryableErrorTimeout( ctx, nil, registry, obj, - "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, + "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, time.Millisecond, ) require.Error(t, err) assert.ErrorIs(t, err, context.DeadlineExceeded) } + +func TestPollUntilTerminal_ClearedInterfaceNameMismatchStopsMismatchTimeout(t *testing.T) { + gvk := schema.GroupVersionKind{ + Group: "configuration.net.nvidia.com", + Version: "v1alpha1", + Kind: "NicInterfaceNameTemplate", + } + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gvk) + obj.SetName("nic-rename") + + validationCalls := 0 + registry := crstate.NewRegistry() + registry.Register(gvk, func(context.Context, client.Client, *unstructured.Unstructured) (crstate.Result, error) { + validationCalls++ + if validationCalls == 1 { + return crstate.Result{ + State: crstate.StateError, + Reason: "worker-1/0000:05:00.0: interface name mismatch", + Retryable: true, + }, nil + } + return crstate.Result{ + State: crstate.StateInProgress, + Reason: "worker-2: waiting for InterfaceNameApplied condition", + }, nil + }) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond) + defer cancel() + err := pollUntilTerminalWithRetryableErrorTimeout( + ctx, nil, registry, obj, + "NicInterfaceNameTemplate/nic-rename", "", 10*time.Millisecond, time.Millisecond, + ) + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.Greater(t, validationCalls, 1) +}