Conversation
8a51baa to
4a2b17e
Compare
📝 WalkthroughWalkthroughAdds Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to The feature remains usable through CRDs, but OLM metadata and regression coverage are incomplete. Address the generated CSV and test assertions before relying on the combined repository behavior. Comment |
4a2b17e to
dd333ec
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (3)
api/nvidia/v1/clusterpolicy_types.go-1284-1288 (1)
1284-1288: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRegenerate the CSV metadata for
useHostSubscription.The CRD schemas and deepcopy files already include this field. However, the generated CSV contains no descriptor for either API type, although both Go fields use
+operator-sdk:gen-csvmarkers. Regenerate the CSV so OLM metadata exposesuseHostSubscription.controllers/transforms_test.go-4800-4800 (1)
4800-4800: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssert the custom repository volume and mount in the combined case.
TransformDriveraddstest-repo-configand mountscustom.repoat/etc/yum.repos.d/custom.repo. The current helpers inspect only subscription-prefixed volumes, so the test still passes if either custom repository contribution is removed. Add assertions for both resources.internal/state/driver_test.go-671-682 (1)
671-682: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the custom repository volume and mount in the combined case.
When
repoConfigEnabledanduseHostSubscriptionare both true,getDriverAdditionalConfigsadds thetest-repo-configConfigMap volume and mount. The test only checks subscription entries and unique mount paths, so removing the custom repository volume and mount would leave every assertion passing. Add assertions for both to cover the required coexistence behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: e10c688b-e835-415e-884b-391d65097ece
⛔ Files ignored due to path filters (6)
bundle/manifests/nvidia.com_clusterpolicies.yamlis excluded by!bundle/manifests/nvidia.com_*.yamlbundle/manifests/nvidia.com_nvidiadrivers.yamlis excluded by!bundle/manifests/nvidia.com_*.yamlconfig/crd/bases/nvidia.com_clusterpolicies.yamlis excluded by!config/crd/bases/**config/crd/bases/nvidia.com_nvidiadrivers.yamlis excluded by!config/crd/bases/**deployments/gpu-operator/crds/nvidia.com_clusterpolicies.yamlis excluded by!deployments/gpu-operator/crds/**deployments/gpu-operator/crds/nvidia.com_nvidiadrivers.yamlis excluded by!deployments/gpu-operator/crds/**
📒 Files selected for processing (9)
api/nvidia/v1/clusterpolicy_types.goapi/nvidia/v1alpha1/nvidiadriver_types.gocontrollers/object_controls.gocontrollers/transforms_test.godeployments/gpu-operator/templates/nvidiadriver.yamldeployments/gpu-operator/values.yamlinternal/consts/consts.gointernal/state/driver_test.gointernal/state/driver_volumes.go
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.
dd333ec to
15944ad
Compare
15944ad to
a69d6be
Compare
a69d6be to
6da402f
Compare
08a509c to
05fc8ec
Compare
Signed-off-by: Rahul Sharma <rahulsharm@nvidia.com>
05fc8ec to
3c174b2
Compare
Description
Fixes: #2817
Fixes RHEL driver installation when a custom repository is used alongside the host’s Red Hat subscription repositories.
Users can now configure:
This mounts both the custom repository and the host RHEL subscription configuration, allowing DNF to access BaseOS/AppStream packages as well as additional custom repositories.
The default behavior is unchanged: when
useHostSubscriptionis omitted, repoConfig remains suitable for air-gapped installations and host subscription mounts are skipped.Checklist
make lint)make validate-generated-assets)make validate-modules)Testing