-
Notifications
You must be signed in to change notification settings - Fork 228
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
[kueuectl] Add stop/resume workload commands #2134
[kueuectl] Add stop/resume workload commands #2134
Conversation
Skipping CI for Draft Pull Request. |
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
/ok-to-test |
/cc @alculquicondor |
/test pull-kueue-test-integration-main |
/assign @mwielgus |
c95ab1e
to
9cfe622
Compare
/assign @trasc |
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
LGTM label has been added. Git tree hash: ccdfda80562a74d75effec764cefe76ba53dcb20
|
eb7e263
to
454841c
Compare
Anything to add @mwielgus ? |
|
||
const ( | ||
resumeExample = ` # Resume the workload | ||
kueuectl resume workload my-workload` |
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.
For a follow up, but it would be awesome to support any job object, and kueuectl automatically finds the workload for it.
| workload | wl | kueue.x-k8s.io/v1beta1 | true | Workload | | ||
|
||
|
||
## Create |
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 don't necessarily oppose all these details, but I think we can get away without them.
Using help
in the CLI should be more than enough.
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.
@mwielgus, wdyt?
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 think this will be hard to maintain. I prefer we don't add all this detail.
@mwielgus, wdyt?
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.
Let's not add everything for now.
let's maybe stop at a 2 level hierarchy:
kubectl-kueue
index
installation
commands
create
resume
stop
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.
Done
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 on splitting this
weight: 10 | ||
description: > | ||
The kubectl-kueue plugin, kueuectl, allows you to manage Kueue 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.
shouldn't this have the contents that are currently on kubectl-kueue.md?
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.
Fixed
/test pull-kueue-test-integration-main |
bba0c3e
to
b88c020
Compare
294ac31
to
d76614d
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.
/lgtm
LGTM label has been added. Git tree hash: f4978c87454e8c703646e771ad0ea525bae9dbe2
|
/assign @alculquicondor |
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.
site/content/en/docs/reference/kubectl-kueue/commands/create/_index.md
to avoid stutter
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.
Done
d76614d
to
e946f9a
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.
It looks like you forgot to change the folder name
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.
Oops. Fixed.
e946f9a
to
3b17bc4
Compare
/lgtm |
LGTM label has been added. Git tree hash: 820076759cdc9efdf7fab22de51cfb06ff25238b
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, mbobrovskyi 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 |
/release-note-edit
|
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #2132 and #2133
Special notes for your reviewer:
Does this PR introduce a user-facing change?