-
Notifications
You must be signed in to change notification settings - Fork 14.5k
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
doc(etcd-maintenance): add reference to etcd-defrag CronJob #43394
Conversation
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
here is the preview: https://deploy-preview-43394--kubernetes-io-main-staging.netlify.app/docs/tasks/administer-cluster/configure-upgrade-etcd/#maintaining-etcd-clusters |
thx @clementnuss . Since it's my personal repo (I might donate the project to etcd-io if other etcd maintainers have no objection), so I avoid giving |
cc @neolit123 |
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 on the tool itself.
i think this falls under third party content and it neesd the following tag outside of a note:
{{% thirdparty-content %}}
it will generate a note like this:
Note: This section links to third party projects that provide functionality required by Kubernetes. The Kubernetes project authors aren't responsible for these projects, which are listed alphabetically. To add a project to this list, read the content guide before submitting a change. More information.
guide about third party content:
https://kubernetes.io/docs/contribute/style/content-guide/#third-party-content
the k/website maintainers will provide further instructions.
It can also be useful to run the defragmentation tool as a Kubernetes CronJob, | ||
[as documented here](https://github.com/ahrtr/etcd-defrag/blob/main/doc/etcd-defrag-cronjob.yaml) | ||
, to make sure that defragmentation happens regularly. |
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 doesn't match our style guide, and it makes the note longer.
How about editing the README for https://github.com/ahrtr/etcd-defrag to mention the example manifest?
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 tried to improve the style according to https://kubernetes.io/docs/contribute/style/style-guide/
It's already mentioned in the README of the etcd-defrag
tool. I however feel like this will make more people aware of the possibility to run a defragmentation tool in a CronJob, which is beneficial to cluster operators.
regarding the note length, it will indeed add one sentence, but that's the purpose of documentation to make information available where it can be useful, and that doesn't come without a new sentence 😅
Instead of augmenting this page for a third party tool, I'm thinking whether we can document this at https://kubernetes.io/docs/reference/tools/. |
the note was already there 😉 |
The reference to the third-party tool on the page was already there (thankfully, I wouldn't have found it otherwise). This PR is only about mentioning the existence of a CronJob that makes using the tool simpler and periodic. While I agree that a "reference tools" page, a bit like some "awesome-xyz" Github repo, could be nice, I think I wouldn't have consulted it, and in my opinion this really makes sense to reference tools in the context where they can be useful. |
Here is my updated take on this. Just one new line, hopefully more conformant to the style guide this time 🙃 @sftim I think it would be greatly beneficial to document that possibility in this part of the documentation. For example, I wouldn't have known about |
/label tide/merge-method-squash |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tengqm 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 |
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.
OK, this works.
I still like notes to be short.
/lgtm
LGTM label has been added. Git tree hash: 97f0fae37f0b5030a4b3a595f07eee07a9e21939
|
Please run the following commands to reset the CI flow:
|
…es#43394) * doc(etcd-maintenance): add reference to etcd-defrag CronJob * doc: improve style according to style guide * chore: fix file name
…es#43394) * doc(etcd-maintenance): add reference to etcd-defrag CronJob * doc: improve style according to style guide * chore: fix file name
With the recent documentation of a Kubernetes CronJob for the
etcd-defrag
tool, I think it would be valuable to also reference that possibility in the official documentation.Indeed, running the tool as a CronJob has several benefits:
/cc @ahrtr