diff --git a/README.md b/README.md index 7be419f..e92f27b 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,13 @@ Apply the generated deployment files to your Kubernetes cluster by using --deplo The deploy step installs (or upgrades) the `nvidia/network-operator` Helm chart in-process before applying the post-install CRs. The chart version and Helm repository URL are taken from the embedded release catalog and can be selected via `--network-operator-release `. Each profile renders a per-profile `values.yaml` next to the CR manifests; `l8k deploy` reads that file and runs the install. When a release already exists with different values, deploy fails fast — pass `--overwrite-existing` to promote to `helm upgrade --install`. +Deploy preflight does not treat `SriovNetworkPoolConfig`, +`SriovNetworkNodePolicy`, or `OVSNetwork` objects labeled with +`spectrumx.nvidia.com/owner-name` as conflicting resources. The Spectrum-X +operator derives and owns those objects from `SpectrumXRailPoolConfig`, so a +deploy restarted after the rail-pool controller has run remains idempotent and +does not require `--overwrite-existing`. + ### AI Agent / Automation Support Use --output json for structured machine-readable output (single JSON object to stdout). Use --yes to auto-confirm prompts, --quiet to suppress informational output, and --dry-run to preview deployments. @@ -346,7 +353,8 @@ registry (with SR-IOV silent-failure detection, NicConfigurationTemplate and NicFirmwareTemplate validation scoped to the operator-populated `status.nicDevices`, current template-payload and device-generation checks, matched-set checks for node, NIC type, PCI, serial-number, and part-number -selectors, condition-Reason classification, +selectors, condition-Reason classification, and firmware-condition gating +only for devices carrying `spec.firmware`, NicClusterPolicy appliedStates breakdown, etc.); and (3) a data-plane connectivity matrix — apply the example DaemonSet, wait for it to roll out completely (`numberReady == diff --git a/docs/advanced/deployment.md b/docs/advanced/deployment.md index 6da763a..5a565cf 100644 --- a/docs/advanced/deployment.md +++ b/docs/advanced/deployment.md @@ -39,12 +39,20 @@ Before applying custom resources, Launch Kit compares the bundle with the cluste | Helm chart version | Installed chart differs from the selected release. | | Helm values | Installed user values differ from generated `values.yaml`. | | Component versions | Live `NicClusterPolicy` component versions differ from the release catalog. | -| Stray resources | Managed Network Operator CRs exist but are not in the generated bundle. | +| Stray resources | l8k-managed Network Operator CRs exist but are not in the generated bundle. Spectrum-X operator-generated `SriovNetworkPoolConfig`, `SriovNetworkNodePolicy`, and `OVSNetwork` objects are excluded. | Without `--overwrite-existing`, any mismatch stops deployment and all detected drift is reported together. With `--overwrite-existing`, Launch Kit authorizes the Helm upgrade, deletes stray managed CRs, and lets server-side apply converge owned `NicClusterPolicy` fields. This can remove resources, so inspect the preflight report before enabling it. +The Spectrum-X operator labels the SR-IOV pool configs, node policies, and OVS +networks it derives from `SpectrumXRailPoolConfig` with +`spectrumx.nvidia.com/owner-name`. Launch Kit leaves these controller-owned +objects out of stray detection and remediation. This makes a restarted +Spectrum-X deployment idempotent after the controller has created its child +resources, while unlabelled resources of the same kinds remain protected by +the normal conflict check. + ## Apply Order After Helm and preflight, deployment proceeds in four manifest phases: @@ -71,7 +79,7 @@ The shared resource-state registry classifies generated objects as: Kind-specific checks include per-component Network Operator state, SR-IOV per-node sync, matched PF counts, NIC configuration templates, IP pools, and Spectrum-X rail configuration. -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. 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 `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. ## Timeout diff --git a/docs/user/validation.md b/docs/user/validation.md index 77d1853..08aa705 100644 --- a/docs/user/validation.md +++ b/docs/user/validation.md @@ -28,9 +28,9 @@ When `--user-config` is omitted, Launch Kit checks `./cluster-config.yaml`, the Connectivity is skipped when manifests are missing, errored, or still in progress. -Manifest-state checks for `NicConfigurationTemplate` and `NicFirmwareTemplate` use the operator-populated `status.nicDevices` list as the matched device set. An empty list, a list that does not yet reflect the current node, NIC type, PCI-address, serial-number, and part-number selectors, a missing named `NicDevice`, a device spec that does not yet reflect the current template payload, or a device condition with a stale `observedGeneration` remains `IN-PROGRESS`. Unrelated discovered devices are used only to verify selector freshness; their configuration and firmware state is ignored. +Manifest-state checks for `NicConfigurationTemplate` and `NicFirmwareTemplate` use the operator-populated `status.nicDevices` list as the matched device set. An empty list, a list that does not yet reflect the current node, NIC type, PCI-address, serial-number, and part-number selectors, a missing named `NicDevice`, a device spec that does not yet reflect the current template payload, or a relevant device condition with a stale `observedGeneration` remains `IN-PROGRESS`. `NicConfigurationTemplate` considers `FirmwareUpdateInProgress` relevant only when the matched device has `spec.firmware`; a stale firmware condition cannot block a configuration-only deployment. Unrelated discovered devices are used only to verify selector freshness; their configuration and firmware state is ignored. -Preflight uses the same checks as deployment: Helm chart version, generated Helm values, component versions, and stray managed CRs. Validation never remediates drift. +Preflight uses the same checks as deployment: Helm chart version, generated Helm values, component versions, and stray l8k-managed CRs. SR-IOV pool configs, node policies, and OVS networks labeled with `spectrumx.nvidia.com/owner-name` are controller-owned outputs of `SpectrumXRailPoolConfig`, so they are not reported as strays. Validation never remediates drift. ## Validation Modes diff --git a/pkg/networkoperatorplugin/crstate/nicconfig.go b/pkg/networkoperatorplugin/crstate/nicconfig.go index 8cc0159..977ed6d 100644 --- a/pkg/networkoperatorplugin/crstate/nicconfig.go +++ b/pkg/networkoperatorplugin/crstate/nicconfig.go @@ -551,10 +551,10 @@ func classifyInterfaceName(byType map[string]map[string]interface{}) (CRState, s } } -// classifyConfiguration inspects ConfigUpdateInProgress (primary) and -// FirmwareUpdateInProgress (when present). Reason is the authoritative -// classifier — Status=False with Reason=UpdateSuccessful is the *success* -// terminal state, not failure. +// classifyConfiguration inspects ConfigUpdateInProgress (primary) and, when +// the device is targeted by a NicFirmwareTemplate, FirmwareUpdateInProgress. +// Reason is the authoritative classifier — Status=False with +// Reason=UpdateSuccessful is the *success* terminal state, not failure. func classifyConfiguration(device *unstructured.Unstructured, byType map[string]map[string]interface{}) (CRState, string) { if cond, ok := byType[consts.ConfigUpdateInProgressCondition]; ok { if pending, reason := conditionGenerationPending(device, cond, consts.ConfigUpdateInProgressCondition); pending { @@ -589,7 +589,15 @@ func classifyConfiguration(device *unstructured.Unstructured, byType map[string] } // At this point ConfigUpdateInProgress.Reason=UpdateSuccessful. - // If firmware section is also present, gate on its terminal state. + // Firmware conditions can remain on a NicDevice after a configuration-only + // update increments its generation. Ignore that stale condition unless a + // NicFirmwareTemplate populated spec.firmware for this device. + if !deviceTargetedByTemplate(device, templateKindFirmware) { + return StateSuccess, "config reconciled (no firmware spec)" + } + + // A firmware payload is present, so gate on its terminal state when the + // operator has published the corresponding condition. if cond, ok := byType[consts.FirmwareUpdateInProgressCondition]; ok { if pending, reason := conditionGenerationPending(device, cond, consts.FirmwareUpdateInProgressCondition); pending { return StateInProgress, reason diff --git a/pkg/networkoperatorplugin/crstate/nicconfig_test.go b/pkg/networkoperatorplugin/crstate/nicconfig_test.go index 90a7527..d05dc15 100644 --- a/pkg/networkoperatorplugin/crstate/nicconfig_test.go +++ b/pkg/networkoperatorplugin/crstate/nicconfig_test.go @@ -547,7 +547,7 @@ func TestNicConfigurationTemplate_AllReasonsClassified(t *testing.T) { func TestNicConfigurationTemplate_FirmwareGate(t *testing.T) { // ConfigUpdateInProgress=False/UpdateSuccessful gates on - // FirmwareUpdateInProgress when present. + // FirmwareUpdateInProgress when spec.firmware is present. cases := []struct { name string fwReason string @@ -565,15 +565,16 @@ func TestNicConfigurationTemplate_FirmwareGate(t *testing.T) { t.Run(tc.name, func(t *testing.T) { manifest := nicTemplateManifest(nicopKindConfigurationTemplate, "tpl", "ns", map[string]string{"role": "worker"}) live := withMatchedDevices(manifest.DeepCopy(), "dev-1") + device := withFirmwareSpec(nicDevice("dev-1", "ns", "worker-1", + []map[string]interface{}{port("0000:1a:00.0")}, + false, true, + []map[string]interface{}{ + condition(consts.ConfigUpdateInProgressCondition, "False", consts.UpdateSuccessfulReason, ""), + condition(consts.FirmwareUpdateInProgressCondition, tc.fwStatus, tc.fwReason, ""), + })) c := newClient(t, live, - nicDevice("dev-1", "ns", "worker-1", - []map[string]interface{}{port("0000:1a:00.0")}, - false, true, - []map[string]interface{}{ - condition(consts.ConfigUpdateInProgressCondition, "False", consts.UpdateSuccessfulReason, ""), - condition(consts.FirmwareUpdateInProgressCondition, tc.fwStatus, tc.fwReason, ""), - }), + device, node("worker-1", map[string]string{"role": "worker"}), ) v := nicTemplateValidator(templateKindConfiguration) @@ -584,6 +585,28 @@ func TestNicConfigurationTemplate_FirmwareGate(t *testing.T) { } } +func TestNicConfigurationTemplate_IgnoresStaleFirmwareConditionWithoutFirmwareSpec(t *testing.T) { + manifest := nicTemplateManifest(nicopKindConfigurationTemplate, "tpl", "ns", map[string]string{"role": "worker"}) + live := withMatchedDevices(manifest.DeepCopy(), "dev-1") + device := nicDevice("dev-1", "ns", "worker-1", + []map[string]interface{}{port("0000:1a:00.0")}, + false, true, + []map[string]interface{}{ + conditionWithGeneration(consts.ConfigUpdateInProgressCondition, "False", consts.UpdateSuccessfulReason, "", 3), + conditionWithGeneration(consts.FirmwareUpdateInProgressCondition, "False", consts.DeviceFwMatchReason, "", 2), + }) + device.SetGeneration(3) + + res, err := nicTemplateValidator(templateKindConfiguration)(context.Background(), newClient(t, + live, + device, + node("worker-1", map[string]string{"role": "worker"}), + ), manifest) + require.NoError(t, err) + assert.Equal(t, StateSuccess, res.State) + assert.Contains(t, res.Details["worker-1/0000:1a:00.0"], "no firmware spec") +} + func TestNicConfigurationTemplate_AggregatesAcrossDevices(t *testing.T) { // Two devices: one success, one error. Aggregate must be error. manifest := nicTemplateManifest(nicopKindConfigurationTemplate, "tpl", "ns", map[string]string{"role": "worker"}) diff --git a/pkg/networkoperatorplugin/preflight/strays.go b/pkg/networkoperatorplugin/preflight/strays.go index b9931e3..5d84997 100644 --- a/pkg/networkoperatorplugin/preflight/strays.go +++ b/pkg/networkoperatorplugin/preflight/strays.go @@ -27,6 +27,14 @@ type kindInfo struct { ClusterScoped bool } +const spectrumXOwnerNameLabel = "spectrumx.nvidia.com/owner-name" + +var spectrumXOperatorGeneratedKinds = map[schema.GroupKind]struct{}{ + {Group: "sriovnetwork.openshift.io", Kind: "SriovNetworkNodePolicy"}: {}, + {Group: "sriovnetwork.openshift.io", Kind: "SriovNetworkPoolConfig"}: {}, + {Group: "sriovnetwork.openshift.io", Kind: "OVSNetwork"}: {}, +} + // managedKinds is the hardcoded set of CR Kinds the Network Operator // chart manages and that `l8k generate` may render. Listed exhaustively // (not derived from the rendered manifests) so the check catches strays @@ -37,7 +45,10 @@ type kindInfo struct { // Operator-created service CRs (NicDevice, SriovNetworkNodeState, // SriovOperatorConfig) are deliberately excluded — they are created by // the operator itself per node / per install and have no l8k-rendered -// counterpart to compare against. +// counterpart to compare against. Spectrum-X-derived child resources share +// Kinds that l8k also renders for other profiles, so they remain in this list +// and are filtered by isSpectrumXOperatorGenerated using their ownership +// label. var managedKinds = []kindInfo{ // Cluster-scoped — singletons or per-group resources. {GVK: gvk("mellanox.com", "v1alpha1", "NicClusterPolicy"), ClusterScoped: true}, @@ -120,6 +131,9 @@ func CheckStrayCRs(ctx context.Context, in Inputs) Result { } for i := range list.Items { obj := &list.Items[i] + if isSpectrumXOperatorGenerated(obj, kind.GVK) { + continue + } ref := ObjectRef{ GVK: kind.GVK, Namespace: obj.GetNamespace(), @@ -153,6 +167,18 @@ func CheckStrayCRs(ctx context.Context, in Inputs) Result { return r } +// isSpectrumXOperatorGenerated identifies the SR-IOV/OVS resources synthesized +// by the Spectrum-X operator from a SpectrumXRailPoolConfig. These objects are +// intentionally absent from l8k's generated bundle and are reconciled and +// cleaned up by their owning operator, so they must not be treated as l8k +// strays during a resumed deployment. +func isSpectrumXOperatorGenerated(obj *unstructured.Unstructured, gvk schema.GroupVersionKind) bool { + if _, ok := spectrumXOperatorGeneratedKinds[gvk.GroupKind()]; !ok { + return false + } + return obj.GetLabels()[spectrumXOwnerNameLabel] != "" +} + // refKey serialises an ObjectRef to a deterministic string identifier // used as the map key inside expectedRefSet. func refKey(r ObjectRef) string { diff --git a/pkg/networkoperatorplugin/preflight/strays_test.go b/pkg/networkoperatorplugin/preflight/strays_test.go index 8182ec9..672d709 100644 --- a/pkg/networkoperatorplugin/preflight/strays_test.go +++ b/pkg/networkoperatorplugin/preflight/strays_test.go @@ -136,6 +136,48 @@ func TestCheckStrayCRs_IgnoresStrayInOtherNamespace(t *testing.T) { assert.Empty(t, r.Mismatches) } +func TestCheckStrayCRs_IgnoresSpectrumXOperatorGeneratedResources(t *testing.T) { + namespace := "nvidia-network-operator" + childKinds := []schema.GroupVersionKind{ + {Group: "sriovnetwork.openshift.io", Version: "v1", Kind: "SriovNetworkNodePolicy"}, + {Group: "sriovnetwork.openshift.io", Version: "v1", Kind: "SriovNetworkPoolConfig"}, + {Group: "sriovnetwork.openshift.io", Version: "v1", Kind: "OVSNetwork"}, + } + objects := make([]runtime.Object, 0, len(childKinds)*2) + for _, childGVK := range childKinds { + operatorGenerated := newCR(childGVK, namespace, "rail0") + operatorGenerated.SetLabels(map[string]string{spectrumXOwnerNameLabel: "rails"}) + objects = append(objects, operatorGenerated) + + // Preserve the existing protection for an unlabelled resource of the + // same Kind: only Spectrum-X operator output is exempt. + objects = append(objects, newCR(childGVK, namespace, "unmanaged")) + } + + r := CheckStrayCRs(context.Background(), Inputs{ + KubeClient: newFakeClientWith(t, objects...).Build(), + OperatorNamespace: namespace, + }) + require.False(t, r.Skipped) + require.Len(t, r.Mismatches, len(childKinds)) + for _, mismatch := range r.Mismatches { + assert.Contains(t, mismatch.Path, "/unmanaged") + } +} + +func TestCheckStrayCRs_DoesNotIgnoreOwnerLabelOnOtherKinds(t *testing.T) { + gvk := schema.GroupVersionKind{Group: "sriovnetwork.openshift.io", Version: "v1", Kind: "SriovNetwork"} + stray := newCR(gvk, "nvidia-network-operator", "rail0") + stray.SetLabels(map[string]string{spectrumXOwnerNameLabel: "rails"}) + + r := CheckStrayCRs(context.Background(), Inputs{ + KubeClient: newFakeClientWith(t, stray).Build(), + OperatorNamespace: "nvidia-network-operator", + }) + require.Len(t, r.Mismatches, 1) + assert.Equal(t, "SriovNetwork/nvidia-network-operator/rail0", r.Mismatches[0].Path) +} + func TestScanGeneratedManifests_FiltersValuesAndExamples(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "values.yaml"), []byte("nfd:\n enabled: true\n"), 0o644)) diff --git a/skills/k8s-launch-kit-deploy/SKILL.md b/skills/k8s-launch-kit-deploy/SKILL.md index 6a2e35a..d6863ad 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.1 +version: 1.3.2 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: @@ -45,7 +45,7 @@ l8k generate --user-config --fabric --deployment-type - | `--deployment-files` | — | Directory with manifests to apply (default `./deployment`) | | `--kubeconfig` | — | Path to kubeconfig with cluster-admin access (falls back to `$KUBECONFIG`) | | `--dry-run` | — | Server-side dry-run (`client.DryRunAll`) — cluster validates without persisting | -| `--overwrite-existing` | — | When a `network-operator` helm release already exists with values that differ from the freshly rendered `values.yaml`, promote Phase 0 to `helm upgrade --install`. Off by default to avoid clobbering an out-of-band install. | +| `--overwrite-existing` | — | Converge detected l8k-owned drift: upgrade a mismatched Helm release, delete conflicting generated-resource kinds, and rewrite owned policy fields. Spectrum-X operator-generated child resources are excluded from conflicts. | ## Examples @@ -93,7 +93,17 @@ part-number selectors. l8k validates only those named `NicDevice` objects and waits for their corresponding `spec.configuration` or `spec.firmware` to reflect the current template payload and for device conditions to observe the current device generation; -unrelated device configuration state does not block deployment. +unrelated device configuration state does not block deployment. A +`NicConfigurationTemplate` gates on `FirmwareUpdateInProgress` only when the +matched device has `spec.firmware`; configuration-only deployments ignore a +stale firmware condition. + +During preflight, do not classify `SriovNetworkPoolConfig`, +`SriovNetworkNodePolicy`, or `OVSNetwork` objects labeled with +`spectrumx.nvidia.com/owner-name` as strays. They are child resources generated +and reconciled by the Spectrum-X operator from `SpectrumXRailPoolConfig`, not +manifests owned by l8k. Unlabelled objects of the same kinds remain subject to +the normal conflict check. ```bash kubectl get nicclusterpolicy -o yaml # Check policy state diff --git a/skills/k8s-launch-kit-validate/SKILL.md b/skills/k8s-launch-kit-validate/SKILL.md index 27c22ed..87107db 100644 --- a/skills/k8s-launch-kit-validate/SKILL.md +++ b/skills/k8s-launch-kit-validate/SKILL.md @@ -1,6 +1,6 @@ --- name: k8s-launch-kit-validate -version: 1.0.1 +version: 1.0.2 description: "Use this skill when the user wants to verify that an NVIDIA networking deployment matches the configuration that produced it. Activate for: 'is my deployment correct', 'are all the manifests applied', 'does the network operator version match', 'verify deployment', 'check cluster state against config', or any question about whether the cluster reflects what l8k generated. Wraps the `l8k validate` subcommand." metadata: requires: @@ -31,12 +31,21 @@ Operator release. NIC type, PCI-address, serial-number, and part-number selectors and validate only the named devices. Their propagated template payload and condition `observedGeneration` must also be current; unrelated - discovered NIC configuration state is ignored. Each manifest is reported + discovered NIC configuration state is ignored. A configuration template + checks `FirmwareUpdateInProgress` only for a matched device with + `spec.firmware`, so a stale firmware condition does not block a deployment + with no `NicFirmwareTemplate`. Each manifest is reported `READY`, `IN-PROGRESS`, `ERROR`, or `MISSING`. 3. **Connectivity matrix.** By default, `l8k validate` applies the generated example DaemonSet, waits for ready pods, and runs source-bound `icmp`, `rping`, and `ib_write_bw` tests. The default mode is `strict`. +Validation also reports deploy-preflight drift without remediating it. +`SriovNetworkPoolConfig`, `SriovNetworkNodePolicy`, and `OVSNetwork` objects +labeled with `spectrumx.nvidia.com/owner-name` are excluded from stray results +because the Spectrum-X operator owns them as children of +`SpectrumXRailPoolConfig`. + Exit code is non-zero (4) on any missing manifest, version mismatch, or gating connectivity failure. Version checks soft-skip when prerequisites are absent — no `cluster-config.yaml`, no Helm release Secret, etc.