-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Document NFD for GPU Labeling #44915
Document NFD for GPU Labeling #44915
Conversation
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
/cc @marquiz |
1a1b976
to
8e84f8d
Compare
e7e734c
to
b9cabb2
Compare
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.
Nits and a (likely unwelcome) widening of the scope to identify some of the existing content as the manual method of labeling. I might be waltzing you into work that the doc maintainers don't want, so let's see what they say. Thanks as always for updating the docs!
b9cabb2
to
4032470
Compare
Thanks /sig node |
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.
Hi; thanks for the pull request.
Here's my feedback on the changes.
4032470
to
31c34dc
Compare
31c34dc
to
0d6fc88
Compare
kind: Pod | ||
metadata: | ||
name: example-vector-add | ||
spec: | ||
# You can use Kubernetes node affinity to schedule this Pod onto a node | ||
# that provides the kind of GPU that its container needs in order to work | ||
affinity: | ||
nodeAffinity: | ||
requiredDuringSchedulingIgnoredDuringExecution: | ||
nodeSelectorTerms: | ||
- matchExpressions: | ||
- key: "gpu.gpu-vendor.example/installed-memory" | ||
operator: Gt # (greater than) | ||
values: ["40535"] | ||
- key: "gpu.gpu-vendor.example/family" | ||
operator: In | ||
values: | ||
- Helium # example product family | ||
- Neon # example product family | ||
restartPolicy: Never | ||
containers: | ||
- name: example-vector-add | ||
image: "registry.example/example-vector-add:v42" | ||
resources: | ||
limits: | ||
gpu-vendor.example/example-gpu: 1 # requesting 1 GPU | ||
{{< /highlight >}} |
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.
This seems to just be an extension of the example above, with the added affinity section, correct?
However, from reading the docs top-to-bottom, this implies that a label called gpu.gpu-vendor.example/installed-memory
will somehow magically be added to the node by NFD (which isn't true, an NFD plugin is needed to add this).
We should extend the text to make it clear that an NFD plugin is needed (in addition to NFD itself) in order to provide any vendor specific labels.
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.
How about now?
1e687dd
to
c8d9eed
Compare
Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
c8d9eed
to
c4937ba
Compare
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.
This looks right.
- Document NFD for GPU Labeling #44915 (review) added LGTM
- Document NFD for GPU Labeling #44915 (comment) provided feedback
- I judge that the feedback is addressed
It looks OK to publish this as a change to the live site.
/lgtm
/approve
Thanks everyone!
LGTM label has been added. Git tree hash: f051228a1757be5c042884074869c5865761ae5d
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eero-t, sftim 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 |
Actually I fixed some highlighting. |
OK, it's less wrong now. |
LGTM label has been added. Git tree hash: f7cb87194f3090aa8dc8aac3251584f53886dc86
|
Thanks a lot! |
This patch documents the official k8s-sig NFD project for automated node labeling, and restructures the
Automatic node labelling
section atscheduling-gpus
page