Support namespaced NFR, update some dependencies and create namespace in helm#34
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the operator and Helm chart to better support environments where NFD’s NodeFeatureRule CRD is namespaced, and to make Helm installs work without pre-labeling the target namespace for DRA admin access.
Changes:
- Detect NFD
nodefeaturerules.nfd.k8s-sigs.ioCRD scope at runtime and use a namespacedNodeFeatureRulewhen required. - Add a Helm
Namespacemanifest that appliesresource.kubernetes.io/admin-access: "true"to the release namespace. - Update Helm-related e2e tests and bump Go/dependency versions.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
internal/controller/misc_controller.go |
Adds CRD-scope detection and uses it to properly address namespaced vs cluster-scoped NFR objects. |
internal/controller/misc_controller_test.go |
Adds unit tests covering both cluster-scoped and namespace-scoped NFD CRD behavior. |
charts/gpu-base-operator/templates/namespace.yaml |
Creates/labels the release namespace to enable DRA admin access without manual steps. |
test/e2e/e2e_test.go |
Adjusts Helm e2e flows to rely on namespace creation behavior during installs. |
go.mod / go.sum |
Updates Go version directive and bumps golang.org/x/* dependency versions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tkatila
force-pushed
the
nfr-namespaced-and-helm-fixes
branch
from
b7db673 to
8f5d9d9
Compare
Contributor
Author
|
Noticed a bug in the helm namespace change, I'll update the PR once more. |
Creation can be avoided by setting 'createNamespace' to false. Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
Signed-off-by: Tuomas Katila <tuomas.katila@intel.com>
tkatila
force-pushed
the
nfr-namespaced-and-helm-fixes
branch
from
8f5d9d9 to
40047a2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
README changes were left outside of this PR. Reasoning is that the current 0.2.1 still depends on namespace being created before helm install. On the next release, I'll also update the
main's README to reflect the change.Fixes #17
Fixes #16