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
Moving Kubelet kernel-memcg-notification to configuration file #91863
Moving Kubelet kernel-memcg-notification to configuration file #91863
Conversation
Hi @knabben. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
/assign @derekwaynecarr @dashpole Should the cluster/gce/*.sh, be renamed in this PR? https://cs.k8s.io/?q=experimental-kernel-memcg-notification&i=nope&files=&repos= |
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
/ok-to-test |
5de004e
to
d4ab61f
Compare
d4ab61f
to
4cb7c53
Compare
/approve |
/assign @liggitt |
cmd/kubelet/app/options/options.go
Outdated
@@ -543,4 +539,7 @@ func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfig | |||
fs.StringSliceVar(&c.EnforceNodeAllocatable, "enforce-node-allocatable", c.EnforceNodeAllocatable, "A comma separated list of levels of node allocatable enforcement to be enforced by kubelet. Acceptable options are 'none', 'pods', 'system-reserved', and 'kube-reserved'. If the latter two options are specified, '--system-reserved-cgroup' and '--kube-reserved-cgroup' must also be set, respectively. If 'none' is specified, no additional options should be set. See https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources/ for more details.") | |||
fs.StringVar(&c.SystemReservedCgroup, "system-reserved-cgroup", c.SystemReservedCgroup, "Absolute name of the top level cgroup that is used to manage non-kubernetes components for which compute resources were reserved via '--system-reserved' flag. Ex. '/system-reserved'. [default='']") | |||
fs.StringVar(&c.KubeReservedCgroup, "kube-reserved-cgroup", c.KubeReservedCgroup, "Absolute name of the top level cgroup that is used to manage kubernetes components for which compute resources were reserved via '--kube-reserved' flag. Ex. '/kube-reserved'. [default='']") | |||
|
|||
// Graduated experimental flags, kept for backward compatibility | |||
fs.BoolVar(&c.KernelMemcgNotification, "experimental-kernel-memcg-notification", c.KernelMemcgNotification, "Use kernelMemcgNotification configuration.") |
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.
add a target release for removal?
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.
@liggitt PTAL
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dashpole, knabben, liggitt, mtaufen 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 |
105f3ed
to
61aff55
Compare
61aff55
to
c39cf28
Compare
/test pull-kubernetes-kubemark-e2e-gce-big |
/lgtm |
/hold cancel |
/retest |
/retest Review the full test history for this PR. Silence the bot with an |
1 similar comment
/retest Review the full test history for this PR. Silence the bot with an |
/test pull-kubernetes-integration |
I know this is merged, but can we change the PR and the description to match the actual flag name? |
hi @knabben, sorry to bother you. I'm trying to work on this issue and this is very similar to this pr. This is my commit. I want to ask for help for building the binary. It keep failing when I tried to build kubelet binary by the command This is my first time to modify the code related to api change. So could you give me some hints about this problem? Thanks a lot. |
What type of PR is this?
/kind cleanup
/area kubelet
/area kubelet-api
What this PR does / why we need it:
Moving Kubelet --kernel-memcg-notification flags to Kubelet configuration file.
Which issue(s) this PR fixes:
Refs #86843
Special notes for your reviewer:
This flag was renamed from --experimental-kernel-memcg-notification to --kernel-memcg-notification
Does this PR introduce a user-facing change?: