-
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
ImageGCMaxAge beta bump #45153
ImageGCMaxAge beta bump #45153
Conversation
👷 Deploy Preview for kubernetes-io-vnext-staging processing.
|
/retitle [WIP]: ImageGCMaxAge beta bump |
/sig node |
/milestone 1.30 |
Hello @haircommander 👋 ! Please take a look at Documenting for a release - PR Ready for Review to get your PR ready for review before the deadline Tuesday March 12th 2024 18:00 PST. Thank you! |
b4da67c
to
2300ca1
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.
Please exclude changes to the following generated doc pages:
- content/en/docs/reference/command-line-tools-reference/kube-apiserver.md
- content/en/docs/reference/command-line-tools-reference/kube-controller-manager.md
- content/en/docs/reference/command-line-tools-reference/kube-proxy.md
- content/en/docs/reference/command-line-tools-reference/kube-scheduler.md
- content/en/docs/reference/command-line-tools-reference/kubelet.md
- content/en/docs/reference/instrumentation/metrics.md
These files are autogenerated as part of the release process, and the generated docs are committed into this repository separately. Hence, there's no need to modify them within this pull request.
Hey @haircommander! Just a reminder that this needs to be reviewed and ready to merge by Docs Freeze on March 26, including a technical review. An exception request will be required if you cannot meet that deadline. Thank you! |
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.
Please omit the generated files from this PR; it's simpler to generate them once just before release day. Look for auto_generated: true
at the start of the files.
I also added some inline feedback.
This feature does not track image usage across Kubelet reboots. If the Kubelet | ||
is rebooted, the image age is reset, causing the Kubelet to wait the full |
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 kubelet doesn't have a bootloader, so:
This feature does not track image usage across Kubelet reboots. If the Kubelet | |
is rebooted, the image age is reset, causing the Kubelet to wait the full | |
This feature does not track image usage across kubelet restarts. If the kubelet | |
is restarted, the tracked image age is reset, causing the kubelet to wait the full | |
(updated)
This feature does not track image usage across Kubelet reboots. If the Kubelet | ||
is rebooted, the image age is reset, causing the Kubelet to wait the full | ||
`ImageMaximumGCAge` duration before qualifying images for garbage collection | ||
based on image age |
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.
(nit)
based on image age | |
based on image age. |
--- | ||
Enables the kubelet configuration field `imageMaximumGCAge`, allowing an administrator to specify the age after which an image will be garbage collected. | ||
Enables the kubelet configuration field `imageMaximumGCAge`, allowing an administrator to specify the age after which an image will be garbage collected. |
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.
Even better: leave this line unchanged.
Hey @SergeyKanzhelev @bsdnet @mrunalp @dchen1107! Just a reminder that this PR needs a technical review by Docs Freeze on March 26. An exception request will be required if the deadline is missed. Thanks! |
I'm reaching out from the Docs team. Just checking in as we approach Docs Freeze on Tuesday March 26th 18:00 PDT. This documentation appears to still be under review. To meet the Docs Freeze, this PR must have a technical review as well as @kubernetes/sig-node-leads Can we get a technical review for these docs to make the Docs Freeze deadline of March 26? Thanks! |
@haircommander hello, Release Lead Shadow for 1.30, acting as Docs shadow here; as @drewhagen mentioned above, the Docs Freeze is fast approaching (Tuesday March 26th 18:00 PDT). From what I've seen, the technical review is pending (cc @SergeyKanzhelev @bsdnet @mrunalp @dchen1107 ), and there are also some pending review item from SIG Docs + others that need to be closed - could you take a look? I think that with that the |
2300ca1
to
a93f668
Compare
thanks for everyone's patience, I have updated the document with the suggestions |
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
for sig-node
{{< note >}} | ||
```suggestion | ||
This feature does not track image usage across kubelet restarts. If the kubelet | ||
is restarted, the tracked image age is reset, causing the kubelet to wait the full | ||
`ImageMaximumGCAge` duration before qualifying images for garbage collection | ||
based on image age. | ||
``` | ||
|
||
{{< /note>}} |
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 content within the markdown block was suggested (here) for addition in the PR, it seems that the backticks (```) should not be included in the actual change. If that is the case, it would be appropriate to remove these backticks.
{{< note >}} | |
```suggestion | |
This feature does not track image usage across kubelet restarts. If the kubelet | |
is restarted, the tracked image age is reset, causing the kubelet to wait the full | |
`ImageMaximumGCAge` duration before qualifying images for garbage collection | |
based on image age. | |
``` | |
{{< /note>}} | |
{{< note >}} | |
This feature does not track image usage across kubelet restarts. If the kubelet | |
is restarted, the tracked image age is reset, causing the kubelet to wait the full | |
`ImageMaximumGCAge` duration before qualifying images for garbage collection | |
based on image age. | |
{{< /note>}} |
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
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.
Please exclude the changes made to this generated file.
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.
a93f668
to
eb29831
Compare
updated! thanks @dipesh-rawat |
- stage: beta | ||
defaultValue: true | ||
fromVersion: "1.30" |
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 changes to the stages
block appear incorrect. Please refer to the documentation here for further information. Additionally, I've provided some suggested changes below for your consideration. I hope these recommendations prove helpful.
- stage: beta | |
defaultValue: true | |
fromVersion: "1.30" | |
- stage: alpha | |
defaultValue: false | |
fromVersion: "1.29" | |
toVersion: "1.29" | |
- stage: beta | |
defaultValue: true | |
fromVersion: "1.30" |
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.
updated!
Signed-off-by: Peter Hunt <pehunt@redhat.com>
eb29831
to
067648a
Compare
@haircommander , did your forced-push address @sftim review? Mentioning it because it still appears as "changes requested", which could be in the way. |
Yup it looks like the changes requested in this post may be blocking merge. |
LGTM from sig-node side. |
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.
Thanks!
/lgtm
/approve
LGTM label has been added. Git tree hash: b64b446355a9d9e0cd3514e9fe527544b0aa8b8b
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dipesh-rawat, mrunalp 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 |
/milestone 1.30 |
as well as document accompanying metric
KEP: kubernetes/enhancements#4210