-
Notifications
You must be signed in to change notification settings - Fork 348
docs: add object retention page #5271
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
base: main
Are you sure you want to change the base?
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 project configuration. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mykysha The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
site/content/en/docs/tasks/manage/setup_object_retention_policy.md
Outdated
Show resolved
Hide resolved
c72f950
to
1d9d137
Compare
cc @mwysokin would you like to give the first pass? |
Sorry for the delay. I'll have time to review this on Friday (tomorrow). |
{{% alert title="Note" color="primary" %}} | ||
If you update an existing Kueue installation you may need to restart the | ||
`kueue-controller-manager` pod in order for Kueue to pick up the updated | ||
configuration. In that case run: | ||
|
||
```shell | ||
kubectl delete pods --all -n kueue-system | ||
``` |
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 think we need this. This should be discussed in the referenced part of the doc: (/docs/installation#install-a-custom-configured-released-version
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.
Surely, will remove this part. Was using waitforpodsready doc as an example, which has this section as well
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 see, I guess this is old style, I think it is ok to keep it in one place only
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.
Removed the alert from the doc
|
||
|
||
## Example | ||
|
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.
The example here isn't really example currently.
Maybe in this example propose Kueue configuration which is using the delay of 1min, and also configure waitforPodsReady. Then describe 2 scenarios:
- workload happily finished and collected
- workload which hits waitForPodsReady and gets deactivated. For example, set up Kueue quota bigger than the amount of physical not capacity, to simulate node failure.
wdyt @mwysokin ?
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.
Assuming no response from @mwysokin I would structure the example as proposed. Please adjust the doc.
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.
IMO what is missing is the good example as mentioned.
other than that looks good
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.
Reworked the example here, wondering whether this is understandable enough or whether I should add yaml for the resources to be created, so that it would be guaranteed to work and demonstrate what is intended?
1d9d137
to
7e08866
Compare
cc @mszadkow @mbobrovskyi it will be great if you can also give it a pass |
|
||
This guide shows you how to enable and configure optional object retention | ||
policies in Kueue to automatically delete finished or deactivated Workloads | ||
after a specified time. By default, Kueue leaves all Workload objects in etcd |
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.
not sure we have to mention etcd, or maybe make a direct link between Kueue and etc, as this is more indirect
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.
Changed the description to "cluster", hope it makes it more understandable and relevant
7e08866
to
295029c
Compare
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Add a documentation page for the object retention functionality
Which issue(s) this PR fixes:
Fixes #5261
Special notes for your reviewer:
Does this PR introduce a user-facing change?