-
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
Updated container garbage collection with accurate information #48001
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
/sig node |
Hi @dipesh-rawat, @Princesso, |
### Container garbage collection {#container-image-garbage-collection} (deprecated) | ||
|
||
{{< caution >}} | ||
Container garbage collection is deprecated infavour of `--eviction-hard` or |
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.
in favour
*
|
||
{{< caution >}} | ||
Container garbage collection is deprecated infavour of `--eviction-hard` or | ||
`--eviction-soft` and Will be removed in a future version. For more details on |
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.
will
*
thanks! content wise it looks good, I just have a couple of stylistic nits |
@haircommander, |
Thanks |
Thnks for the review @haircommander, @esotsal /assign @Princesso |
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 does need an update before we can merge it, to make sure the heading is right.
/lgtm cancel
### Container garbage collection {#container-image-garbage-collection} (deprecated) | ||
|
||
{{< caution >}} | ||
Container garbage collection is deprecated in favour of `--eviction-hard` or | ||
`--eviction-soft` and will be removed in a future version. For more details on | ||
eviction policy, see [node-pressure-eviction](/docs/concepts/scheduling-eviction/node-pressure-eviction/). | ||
{{< /caution >}} |
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.
These command line arguments are deprecated, and the format isn't right.
We also avoid statements about the future, although we do make reasonable exceptions to that general guidance.
Try this:
### Container garbage collection {#container-image-garbage-collection} (deprecated) | |
{{< caution >}} | |
Container garbage collection is deprecated in favour of `--eviction-hard` or | |
`--eviction-soft` and will be removed in a future version. For more details on | |
eviction policy, see [node-pressure-eviction](/docs/concepts/scheduling-eviction/node-pressure-eviction/). | |
{{< /caution >}} | |
### Legacy garbage collection for containers (deprecated) {#container-image-garbage-collection} | |
{{< feature-state for_k8s_version="v1.9" state="deprecated" >}} | |
{{< note >}} | |
Container garbage collection is deprecated, in favor of using the kubelet configuration file | |
entries `evictionHard` and `evictionSoft` to manage overall Pod | |
{{< glossary_tooltip text="eviction" term_id="eviction" >}}. | |
{{< /note >}} |
ℹ️ I am not sure about which version deprecated setting these command line arguments.
Am I right that eviction is always at the Pod level, whereas these deprecated settings apply at the container level?
The kubelet garbage collects unused containers based on the following variables, | ||
which you can define: | ||
|
||
* `MinAge`: the minimum age at which the kubelet can garbage collect a | ||
container. Disable by setting to `0`. | ||
* `MaxPerPodContainer`: the maximum number of dead containers each Pod | ||
can have. Disable by setting to less than `0`. | ||
* `MinAge`: the minimum age at which the kubelet can garbage collect a container. | ||
Disable by setting a kubelet config flag `--minimum-container-ttl-duration` to `0`. | ||
* `MaxPerPodContainer`: the maximum number of dead containers each Pod can have. | ||
Disable by setting a kubelet config flag `--maximum-dead-containers-per-container` | ||
to less than `0`. | ||
* `MaxContainers`: the maximum number of dead containers the cluster can have. | ||
Disable by setting to less than `0`. | ||
Disable by setting a kubelet config flag `--maximum-dead-containers` | ||
to less than `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.
Is this right?
The kubelet garbage collects unused containers based on the following variables, | |
which you can define: | |
* `MinAge`: the minimum age at which the kubelet can garbage collect a | |
container. Disable by setting to `0`. | |
* `MaxPerPodContainer`: the maximum number of dead containers each Pod | |
can have. Disable by setting to less than `0`. | |
* `MinAge`: the minimum age at which the kubelet can garbage collect a container. | |
Disable by setting a kubelet config flag `--minimum-container-ttl-duration` to `0`. | |
* `MaxPerPodContainer`: the maximum number of dead containers each Pod can have. | |
Disable by setting a kubelet config flag `--maximum-dead-containers-per-container` | |
to less than `0`. | |
* `MaxContainers`: the maximum number of dead containers the cluster can have. | |
Disable by setting to less than `0`. | |
Disable by setting a kubelet config flag `--maximum-dead-containers` | |
to less than `0`. | |
The kubelet garbage collects dead containers within existing Pods. This garbage collection is | |
based on the values for several related command line arguments, that are all deprecated: | |
`--minimum-container-ttl-duration` | |
: the minimum age at which the kubelet can garbage collect a dead container. Disable by setting to 0. Setting a value for `--minimum-container-ttl-duration` is deprecated. | |
`--maximum-dead-containers` | |
: limits the total number of dead containers for this node. The default is unlimited. You can set this to an integer that is zero or higher in order to set legacy dead container limits, node-wide. Setting a value for `--maximum-dead-containers` is deprecated. | |
`--maximum-dead-containers-per-container`: the maximum number of dead containers each Pod | |
can have. The default value for this setting is 1; however, you can specify unlimited dead containers by setting this value to 0. Note that setting a value for `--maximum-dead-containers-per-container` is deprecated. |
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 agree we should refer folks to the kubelet configuration options as we're redirecting folks away from using the CLI
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.
WDYT ^ @T-Lakshmi ?
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.
@haircommander,
Yes, I also agree to mention about CLI usage of these arguments.
But my concern is the below statement is true or not?
Container garbage collection is deprecated in favour of
--eviction-hard
or--eviction-soft
and will be removed in a future version.
also when container GC got deprecated and what are the recommended way to use as alternate to container GC?
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.
Also here it mentioned old logs are stored outside of container's context.
Is that true? if true, in which location old logs are storing now?
Please share your inputs.
I think sig-node leaders may be able to clarify this. Hello @kubernetes/sig-node-leads, @kubernetes/sig-node-bugs, |
|
||
{{< caution >}} | ||
Container garbage collection is deprecated in favour of `--eviction-hard` or | ||
`--eviction-soft` and will be removed in a future version. For more details on |
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.
container gc and eviction are different things ...
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.
Hello @tengqm,
container gc and eviction are different things ...
Can you clarify on the below points of container gc?
- Is
--minimum-container-ttl-duration
,--maximum-dead-containers
and--maximum-dead-containers-per-container
related to container gc? - In which version container gc got deprecated?
- Is there any alternate recommended way for container garbage collection? What it is?
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.
Container GC is a task performed by kubelet regularly. The command line flags you mentioned are about container GC. The things that got deprecated, IIRC, are the command line flags. The intent of the deprecation was to reduce knobs for users to tune.
With the mechanism still there in kubelet, container GC won't go away any time soon. I'm not aware of anyone pushing this forward. In other words, the "deprecation" itself may get rotten (or deprecated).
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 for sharing feedback @tengqm.
What others say on this? Please share your opinion.
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.
Container GC is a task performed by kubelet regularly. The command line flags you mentioned are about container GC. The things that got deprecated, IIRC, are the command line flags.
That looks right. Most kubelet command line arguments are deprecated (you should use the configuration file), and also the settings for container / container image GC were a bit legacy anyway.
/cc @kubernetes/sig-node-pr-reviews |
@T-Lakshmi: GitHub didn't allow me to request PR reviews from the following users: kubernetes/sig-node-pr-reviews. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. 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. |
@T-Lakshmi: GitHub didn't allow me to request PR reviews from the following users: kubernetes/sig-node-pr-reviews. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. 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. |
/unassign |
/cc @SergeyKanzhelev @haircommander WDYT ? |
Added PR to sig node weekly meeting Agenda. @SergeyKanzhelev @mrunalp @haircommander Please have a look! |
Sorry, I wasn't looking at the deprecations around this in a while. It will simplify reviews if you can provide easy to check links on what and where we announced was deprecated. I will likely have time to look at this only after KubeCon |
This PR was created as response to this comment discussed in sig-node some time ago. Googling i found deprecation is mentioned here With git blame found deprecation comment has been commited 7 years ago in 1.9.0 here |
Fixes: kubernetes/kubernetes#127157