Expand nodeFeatureDiscovery CRD #39
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
ArangoGutierrez
force-pushed
the
nfd-worker-configmap
branch
from
0603218
to
6ef0a3b
Compare
|
Fixes: #18 |
|
/cc mythi |
|
@ArangoGutierrez: GitHub didn't allow me to request PR reviews from the following users: mythi. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
As we plan to cut a release for NFD, I also plan to cut a release for the operator, could we get a review on this Patch? |
manifests/0700_cr.yaml
Outdated
| image: node-feature-discovery | ||
| repository: k8s.gcr.io/nfd | ||
| version: v0.6.0 |
There was a problem hiding this comment.
is there a use-case to keep these separate? My preference would be to keep just image . FWIW, I'm also not sure about namespace if the deployment gets created in the CR namespace.
There was a problem hiding this comment.
This granularity help us during testing, is almost the same I know, but I prefer to visualize it this way. I am ok with a single var holding the image info.
The namespace is there as a placeholder, I am not modifying this, current CR has image and namespace
There was a problem hiding this comment.
As I wrote, my preference is just image. This would be consistent with what pod spec has so less confusion for the user too.
There was a problem hiding this comment.
Thanks @ArangoGutierrez for the patch! From what I can tell, this looks basically good, just few minor comments/suggestions.
BTW, try running go mod tidy again. For me it tidies go.mod and go.sum even more
pkg/controller/nodefeaturediscovery/nodefeaturediscovery_controls.go
Outdated
Show resolved
Hide resolved
|
Will squash once we agree on the patch state |
|
Thanks @ArangoGutierrez ! I think this greatly increases the usability. I'd be ready to merge. Has anybody really verified this change? 😉 I haven't... |
|
One more comment/wish I actually have is that I think it would be good to reflect this change in the documentation (README) somehow. Even a short section about the operand configuration or smth. WDYT? |
More documentation never hurt anyone! sure, good idea! |
This Patch expands the nodeFeatureDiscovery CRD to enable users to define the Operand image provenance regirsty/image:version as well as the imaePullPolicy, that by default has been set to "Always" Also now the nodeFeatureDiscovery CRD holds the binaryData for the NFD-Worker configuration file, allowing users to define the desired nfd-worker.conf by editing the CR. To achieve this we move the api from v1alpha1 to v1 and extend the CRD and the controller to watch for the updated resources. extra: go mod tidy removed 2 non needed deps from go.mod simplify ImagePolicy function pkg/apis/nfd/v1/nodefeaturediscovery_types.go : ImagePolicy simplification Document new changes to CRD Signed-off-by: Carlos Eduardo Arango Gutierrez <carangog@redhat.com> Co-authored-by: Markus Lehtonen <markus.lehtonen@intel.com>
ArangoGutierrez
force-pushed
the
nfd-worker-configmap
branch
from
c9260dc
to
97bc00e
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ArangoGutierrez, marquiz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This Patch expands the nodeFeatureDiscovery CRD to enable users to
define the Operand image provenance regirsty/image:version as well as
the imaePullPolicy, that by default has been set to "Always"
Also now the nodeFeatureDiscovery CRD holds the binaryData for the
NFD-Worker configuration file, allowing users to define the desired
nfd-worker.conf by editing the CR.
To achieve this we move the api from v1alpha1 to v1 and extend the CRD
and the controller to watch for the updated resources.
extra: go mod tidy removed 2 non needed deps from go.mod
Fixes: #18
Signed-off-by: Carlos Eduardo Arango Gutierrez carangog@redhat.com