Configure MemoryRequest for InPlace pod resize in cgroupv2 systems #121218
Conversation
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Karthik-K-N 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 |
k8s-ci-robot
added
area/kubelet
sig/node
| @@ -227,6 +230,11 @@ func (m *kubeGenericRuntimeManager) calculateLinuxResources(cpuRequest, cpuLimit | |||
| // more info. | |||
| "memory.oom.group": "1", | |||
| } | |||
| if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.InPlacePodVerticalScaling) { | |||
There was a problem hiding this comment.
Could you add a positive and negative test for this feature? You can set features during unit tests so we should verify that this function behaves as expected with feature gate on/off.
There was a problem hiding this comment.
Sure will add, Just wanted to know opinion about the overall approach so just differed it.
Karthik-K-N
force-pushed
the
memory-request
branch
from
1584ba0
to
76517e8
Compare
|
/test pull-kubernetes-unit |
|
/cc @vinaykul |
|
/cc @kubernetes/sig-node-pr-reviews |
|
@Jeffwan: 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/test-infra repository. |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
|
@Karthik-K-N @vinaykul I have been looking into the cgroup v2 memory controllers as a part of https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/2570-memory-qos From what I understand, setting memory.min will ensure the cgroup retains the configured minimum memory which kind of makes it unreclaimable. If there's no unprotected reclaimable memory available, OOM killer is invoked. So, IMO setting memory.min could lead to more OOM killer invocations since it makes memory unreclaimable in cgroups. I would recommend testing different configurations before we set memory.min value. |
|
/cc @ndixita |
@ndixita Thanks for your feedback, I can check the behaviors on various configurations. Please let me know if you have any specific configurations to be tested. |
|
@Karthik-K-N Sure, let's touch base again after kubecon next week. Please reach out on slack and we can discuss. |
sure thank you. |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
k8s-ci-robot
removed
the
lifecycle/stale
|
@Jeffwan I'd not recommend setting memory.min equal to the requested memory. I have tried the cgroupv2 controller, and it results in more OOM kills since kernel will reserve memory.min amount of memory even when the usage by Pod is below that value. The extra memory not being used will result in that unused memory being unavailable to other workloads. |
|
/assign |
What type of PR is this?
/kind feature
What this PR does / why we need it:
cgroupv2 systems provides the ability to minimum memory values with memory.min , This PR adds ability configure minimum memory request for container
Which issue(s) this PR fixes:
Fixes #114202
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
KEP: kubernetes/enhancements#1287
Previous PR: #102884