-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: document NodeFeature API #903
docs: document NodeFeature API #903
Conversation
Skipping CI for Draft Pull Request. |
a85af5e
to
518fcca
Compare
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Rebased. A big reduction in number of commits but still quite massive |
518fcca
to
f5b615d
Compare
e64ea5e
to
00591e8
Compare
00591e8
to
e421121
Compare
/assign |
Now that all the remaining bits have been split into separate PRs I rebased this and re-scoped to be only about the documentation (which was the last remaining piece). Ready for review but depends on #991 |
10a4f6c
to
16328ea
Compare
#991 was merged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Note that RBAC rules must be created for each extension for them to be able to | ||
create and manipulate NodeFeature objects in their namespace. | ||
|
||
Support for NodeFeature CRD API is enabled with the `-enable-nodefeature-api` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking of which, we should enable this via WorkerConfig, not to have to set the flag if it is already running
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I specifically didn't want to have that for now. I had some reasoning in the commit message:
2374944
We can add that later if needed, but after careful consideration, I'd say
LGTM label has been added. Git tree hash: 8f34567e89e4e72e9ce9581e4debfcb6112b50b4
|
[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 |
@fmuyassarov wanna take a look? |
Yes, please. Will do it in few hours and if looks good will remove the hold. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some nits, otherwise looks good!
docs/usage/nfd-master.md
Outdated
updating node objects. More specifically, it is able to modify node labels, | ||
taints and extended resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shall we add annotations as well here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I tried to say is that it modifies these attributes on client requests. I now changed the wording to
More specifically, it modifies node labels, taints and
extended resources based on requests from nfd-workers and 3rd party extensions.
Better? We could also talk about annotations but I don't know if that's something worth mentioning as it's not a "feature" available for users (not yet, at least)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
16328ea
to
1cef0ea
Compare
Document the usage of the NodeFeature CRD API. Also re-organize the documentation a bit, moving the description of NodeFeatureRule controller from customization guide to nfd-master usage page.
1cef0ea
to
3209c14
Compare
@fmuyassarov thanks for the review. Comments addressed, PTAL |
Looks good now. |
LGTM label has been added. Git tree hash: 884cdf69d4be14864bab3d84720245678c3fa699
|
/hold cancel |
Document the usage of the NodeFeature CRD API. Also re-organize the
documentation a bit, moving the description of NodeFeatureRule
controller from customization guide to nfd-master usage page.