-
Notifications
You must be signed in to change notification settings - Fork 256
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
Simplify the user-facing documentation for JobSet in MultiKueue #2318
Simplify the user-facing documentation for JobSet in MultiKueue #2318
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
/assign @alculquicondor @tenzen-y |
c7ddb9c
to
ae6cb73
Compare
ae6cb73
to
7d776e5
Compare
7d776e5
to
9fb3db1
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.
Only nit.
As mentioned in the [MultiKueue Overview](/docs/concepts/multikueue/#jobset) section, in the manager cluster only the JobSet CRDs should be installed, you can do this by running: | ||
If you are using Kueue in version 0.7.0 or newer install the JobSet on the | ||
management cluster (see [JobSet Installation](https://jobset.sigs.k8s.io/docs/installation/) | ||
for more details). We recommend using JobSet 0.5.1+ for MultiKueue. |
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.
Version 0.5.0 is needed , it should not be just a recommendation.
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.
Yeah, I understand that the support started from 0.5.0. but it contains some relevant bugs:
- Do not default the managedBy field jobset#528
- Fix path for the error when attempting to mutate managedBy jobset#527
Actually, our users already hit the issues, which made the investigations unnecessarily harder. Thus we bumped the version suggested in the JobSet page: kubernetes-sigs/jobset#577.
I would prefer not to mention the old version at all in the docs, because this may increase a chance for some users installing 0.5.0.
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.
My problem was with "We recommend to ... ", multikueue is not working with versions lower then 0.5 .
As mentioned in the [MultiKueue Overview](/docs/concepts/multikueue/#jobset) section, in the manager cluster only the JobSet CRDs should be installed, you can do this by running: | ||
If you are using Kueue in version 0.7.0 or newer install the JobSet on the | ||
management cluster (see [JobSet Installation](https://jobset.sigs.k8s.io/docs/installation/) | ||
for more details). We recommend using JobSet 0.5.1+ for MultiKueue. |
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 more details). We recommend using JobSet 0.5.1+ for MultiKueue. | |
for more details). We recommend using JobSet 0.5.1 or newer for MultiKueue. |
I'm fine putting it as a requirement, rather a recommendation.
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 update the "or newer". Regarding the requirement or recommendation. I rephrased to just say please install. 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.
/approve
/hold |
622a5b2
to
5adde02
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
/approve
/cherry-pick website
LGTM label has been added. Git tree hash: cce0ff40074ad5e01eebeb0557a418f099af41e3
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, mimowo 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 |
@alculquicondor can we also cancel the hold? |
/hold cancel |
/cherry-pick website |
@tenzen-y: new pull request created: #2356 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-sigs/prow repository. |
…rnetes-sigs#2318) * Simplify the user-facing documentation for JobSet in MultiKueue * Review remarks * remarks
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Simplify the user-facing documentation. The support in 0.6.0 had limitations which are not worth documenting.
Let's recommend Kueue 0.7.0+ with JobSet 0.5.1+
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
The main issue is that in 0.6.x we didn't support the managedBy mechanism, and for that reason users couldn't install the Jobset controller on management, so the documentation will differ significantly for 0.6 and 0.7.
Does this PR introduce a user-facing change?