Add option to skip individual component validators (toolkit, cuda, plugin) - #2867
Add option to skip individual component validators (toolkit, cuda, plugin)#2867udaykishore-resu wants to merge 1 commit into
Conversation
Add a 'skip' option to the toolkit, cuda and plugin validator specs of the ClusterPolicy (validator.toolkit.skip, validator.cuda.skip, validator.plugin.skip). When set, the operator passes SKIP_VALIDATION=true to the corresponding init container of the nvidia-operator-validator DaemonSet and the nvidia-validator binary only creates the component readiness status file instead of running the validation. The init containers are intentionally kept rather than removed, because other operands (device-plugin, gpu-feature-discovery, dcgm-exporter, mig-manager, mps-control-daemon) block on the 'toolkit-ready' status file and the node-status-exporter metrics report on all status files. This is needed e.g. on GKE with COS, where the container toolkit and the device plugin are preinstalled (toolkit.enabled=false, devicePlugin.enabled=false) but the toolkit-validation init container still fails, since GKE only injects nvidia-smi into containers that request an nvidia.com/gpu resource. Related to NVIDIA/gpu-operator issue 1460 Signed-off-by: Uday Kishore Resu <udaykishoresu2@gmail.com>
📝 WalkthroughWalkthroughThe change adds optional skip settings for plugin, toolkit, and CUDA validators. Helm values and templates expose these settings. The controller passes Merge Risk: 🟠 High · up to The new skip configuration may be discarded by installed CRDs or overridden at runtime, making validation run despite the requested setting. These issues should be fixed before merge. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
controllers/object_controls.go-2462-2462 (1)
2462-2462: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReserve
SKIP_VALIDATIONafter component environment processing.A component Env entry named
SKIP_VALIDATIONis applied after the skip setting and replaces"true". For example,skip: trueplusenv: [{name: SKIP_VALIDATION, value: "false"}]runs validation instead of skipping it. Apply the reserved variable after user Env entries, or reject that Env name.
controllers/object_controls.go#L2462-L2462: Set the CUDA skip variable after CUDA Env processing.controllers/object_controls.go#L2491-L2491: Set the plugin skip variable after plugin Env processing.controllers/object_controls.go#L2517-L2517: Set the toolkit skip variable after toolkit Env processing.cmd/nvidia-validator/main_test.go-370-380 (1)
370-380: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest the skip behavior through
validateComponent.This test calls
skipComponentValidationdirectly. It passes ifvalidateComponentno longer dispatches whenskipValidationFlagis true. It also derives expected filenames from implementation constants.Set
skipValidationFlagin the test, callvalidateComponent, and assert the fixedtoolkit-ready,cuda-ready, andplugin-readyfilenames. This prevents regressions that run validation or break dependent readiness-file consumers.As per path instructions, flag tests that mirror the implementation instead of checking behavior.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: d584c8db-fcc4-41d4-a5fc-d30256c75d6e
⛔ Files ignored due to path filters (4)
api/nvidia/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated.*.gobundle/manifests/nvidia.com_clusterpolicies.yamlis excluded by!bundle/manifests/nvidia.com_*.yamlconfig/crd/bases/nvidia.com_clusterpolicies.yamlis excluded by!config/crd/bases/**deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yamlis excluded by!deployments/gpu-operator/crds/**
📒 Files selected for processing (7)
api/nvidia/v1/clusterpolicy_types.gocmd/nvidia-validator/main.gocmd/nvidia-validator/main_test.gocontrollers/object_controls.gocontrollers/transforms_test.godeployments/gpu-operator/templates/clusterpolicy.yamldeployments/gpu-operator/values.yaml
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors=true | ||
| // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.displayName="Skip NVIDIA Device Plugin validation" | ||
| // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:booleanSwitch" | ||
| Skip *bool `json:"skip,omitempty"` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate the CRD and deepcopy assets.
The supplied change adds three CRD fields but does not update generated assets. A cluster with the existing CRD schema will not persist these skip values, so the controller will not receive the requested configuration. Regenerate and commit the CRD, CSV, and deepcopy outputs.
As per path instructions, “Editing these types requires regenerating the deepcopy and CRD assets.”
Also applies to: 459-459, 486-486
Source: Path instructions
1 participant
Description
Adds
validator.toolkit.skip,validator.cuda.skipandvalidator.plugin.skipto the ClusterPolicy (and matching Helm values). When set, the operator passesSKIP_VALIDATION=trueto the corresponding init container of thenvidia-operator-validatorDaemonSet, and thenvidia-validatorbinary only writes the component readiness status file (toolkit-ready/cuda-ready/plugin-ready) instead of running the validation.The init containers are deliberately kept rather than removed:
toolkit-readygates the device-plugin, GFD, DCGM exporter, MIG manager and MPS control daemon init containers, and node-status-exporter watches all of the status files. Skippingdriveris intentionally not supported becausedriver-readycarries the driver-root contract consumed by other operands.Motivation: on GKE (COS) the toolkit and device plugin are preinstalled (
toolkit.enabled=false,devicePlugin.enabled=false), buttoolkit-validationstill fails because GKE only injectsnvidia-smiinto containers that requestnvidia.com/gpu, while the validator relies onNVIDIA_VISIBLE_DEVICES=all.Usage:
Fixes #1460
Checklist
make lint)make validate-generated-assets)make validate-modules)Testing
TestTransformValidatorComponentand a GKE-shaped end-to-end case inTestTransformValidator(controllers/transforms_test.go);TestSkipComponentValidationincmd/nvidia-validator/main_test.go.go test ./controllers/... ./api/... ./cmd/nvidia-validator/... ./internal/...passes;make lint,make generate manifestsproduce no diff.