-
Notifications
You must be signed in to change notification settings - Fork 40.9k
Alpha node swap support #102823
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
Alpha node swap support #102823
Conversation
Skipping CI for Draft Pull Request. |
/test verify |
@ehashman: The specified target(s) for
Use
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. |
/test pull-kubernetes-verify |
/test pull-kubernetes-verify |
/priority important-soon |
f8d27c8
to
1e13159
Compare
/test pull-kubernetes-node-kubelet-swap-ubuntu |
@ehashman: The specified target(s) for
Use
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. |
Ah, still waiting on kubernetes/test-infra#22453 to merge. |
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.
Two updates requested:
- update pod level cgroup creation to match container cgroup creation
- minor nit on the case stmt to enumerate all options for readability
Thanks!
switch m.memorySwapBehavior { | ||
case kubelettypes.UnlimitedSwap: | ||
// -1 = unlimited swap | ||
lc.Resources.MemorySwapLimitInBytes = -1 |
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.
just recording this as this swaps out of my own mental cache sometimes.
if a container has a defined memory limit X, it will still have MemoryLimitInBytes=X
, but it may now use unbounded additional swap by setting MemorySwapLimitInBytes
if UnlimitedSwap
is enabled. This is consistent with existing behavior where --fail-swap-on was false because no kubelet enforced limit was written.
@@ -89,6 +90,21 @@ func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.C | |||
|
|||
lc.Resources.HugepageLimits = GetHugepageLimitsFromResources(container.Resources) | |||
|
|||
if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.NodeSwapEnabled) { |
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 think we need a similar change in ResourceConfigForPod
for pod level cgroup settings created by pod cgroup manager. I would expect them to match the container settings. I think memory backed volumes could ultimately use swap, but would like @sjenning to confirm. Either way, the cgroup settings for memory should match pod and container scopes.
@ehashman We can skip the The default value for pod cgroup will basically be unbounded in practice, and it is not until the beta criteria that its value conceptually could vary further than unbounded. |
thanks for updates. /lgtm |
/assign @liggitt |
Alpha features job appears to have been broken since July 1: https://prow.k8s.io/job-history/gs/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-e2e-gce-alpha-features?buildId=1410844642957594624 tracked in #103440 We should skip that for this and rely on the ubuntu/fedora swap jobs. /test pull-kubernetes-node-kubelet-swap-ubuntu |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, ehashman, liggitt 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 |
/retest Review the full test history for this PR. Silence the bot with an |
@ehashman: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
Here is a second batch for feature gate updates in 1.22. - EndpointSliceProxying kubernetes/kubernetes#103451 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 - LogarithmicScaleDown kubernetes/kubernetes#101767 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - DynamicKubeletConfig kubernetes/kubernetes#102966 - NodeSwapEnabled kubernetes/kubernetes#102823 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 Some of these needs more detailed documentation.
Here is a second batch for feature gate updates in 1.22. - EndpointSliceProxying kubernetes/kubernetes#103451 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - DynamicKubeletConfig kubernetes/kubernetes#102966 - NodeSwapEnabled kubernetes/kubernetes#102823 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 Some of these needs more detailed documentation.
Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - NodeSwap kubernetes/kubernetes#102823, kubernetes/kubernetes#103553 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.
Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - NodeSwap kubernetes/kubernetes#102823, kubernetes/kubernetes#103553 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.
* Upgrade to k8s.io/*@v0.22.2 in go.mod * [automated] make revendor * [automated] make generate * [automated] make revendor github.com/go-openapi/spec seems to be orphaned after previous make generate * Upgrade to [email protected] in go.mod Also, upgrade setup-envtest (doesn't have a tagged release yet, so use release commit instead) * [automated] make revendor * Upgrade to [email protected] in go.mod * [automated] make revendor * Add missing WarningsOn{Create,Update} to rest strategies * Replace dot imports for github.com/onsi/gomega/types Fix linting errors: `Assertion` redeclared in this block (typecheck) * Switch to typed values for WebhookInstallOptions.*Webhooks ref kubernetes-sigs/controller-runtime#1626 * RequestCertificate now takes an optional requestedDuration ref kubernetes/kubernetes#99494 * Switch to matchers.DeepEqual to test semantic equality Maps (e.g. labels, selectors, resource requirements) might be sorted differently than expected. Hence, use semantic equality instead of strict equality, as this is what matters to us. Also, DeepEqual outputs yaml and adds a nice diff indicator instead of printing some large confusing go struct representation. * Add new memorySwap field to expected kubelet config ref kubernetes/kubernetes#102823 * Round condition.lastUpdateTime to seconds in test There were several changes in the fake clients that might cause the failure to happen just now. * Correct unit tests falsely succeeding These tests were not preparing the test objects correctly: they only updated them in memory but not on the fake client. This wasn't caught until now because the fake client mimicked the real json decoder, which didn't unset fields not present on the server. Now that the fake client zeroes fields, the tests started failing (which is correct). So fix the tests. ref kubernetes-sigs/controller-runtime#1651 * Remove workarounds for missing zeroing in json decoder Now that the c-r client zeroes fields before decoding into the object, we can drop our workarounds for this, so basically drop kutil.CreateResetObjectFunc and its usages. ref kubernetes-sigs/controller-runtime#1640 * Drop setting webhook gvk explicitly in envtest webhookConfig.SetGroupVersionKind is not needed anymore with kubernetes-sigs/controller-runtime#1665 * Add some follow-up TODO comments * [automated] make generate but with go 1.16.9 * Address review comments
* Upgrade to k8s.io/*@v0.22.2 in go.mod * [automated] make revendor * [automated] make generate * [automated] make revendor github.com/go-openapi/spec seems to be orphaned after previous make generate * Upgrade to [email protected] in go.mod Also, upgrade setup-envtest (doesn't have a tagged release yet, so use release commit instead) * [automated] make revendor * Upgrade to [email protected] in go.mod * [automated] make revendor * Add missing WarningsOn{Create,Update} to rest strategies * Replace dot imports for github.com/onsi/gomega/types Fix linting errors: `Assertion` redeclared in this block (typecheck) * Switch to typed values for WebhookInstallOptions.*Webhooks ref kubernetes-sigs/controller-runtime#1626 * RequestCertificate now takes an optional requestedDuration ref kubernetes/kubernetes#99494 * Switch to matchers.DeepEqual to test semantic equality Maps (e.g. labels, selectors, resource requirements) might be sorted differently than expected. Hence, use semantic equality instead of strict equality, as this is what matters to us. Also, DeepEqual outputs yaml and adds a nice diff indicator instead of printing some large confusing go struct representation. * Add new memorySwap field to expected kubelet config ref kubernetes/kubernetes#102823 * Round condition.lastUpdateTime to seconds in test There were several changes in the fake clients that might cause the failure to happen just now. * Correct unit tests falsely succeeding These tests were not preparing the test objects correctly: they only updated them in memory but not on the fake client. This wasn't caught until now because the fake client mimicked the real json decoder, which didn't unset fields not present on the server. Now that the fake client zeroes fields, the tests started failing (which is correct). So fix the tests. ref kubernetes-sigs/controller-runtime#1651 * Remove workarounds for missing zeroing in json decoder Now that the c-r client zeroes fields before decoding into the object, we can drop our workarounds for this, so basically drop kutil.CreateResetObjectFunc and its usages. ref kubernetes-sigs/controller-runtime#1640 * Drop setting webhook gvk explicitly in envtest webhookConfig.SetGroupVersionKind is not needed anymore with kubernetes-sigs/controller-runtime#1665 * Add some follow-up TODO comments * [automated] make generate but with go 1.16.9 * Address review comments
* Upgrade to k8s.io/*@v0.22.2 in go.mod * [automated] make revendor * [automated] make generate * [automated] make revendor github.com/go-openapi/spec seems to be orphaned after previous make generate * Upgrade to [email protected] in go.mod Also, upgrade setup-envtest (doesn't have a tagged release yet, so use release commit instead) * [automated] make revendor * Upgrade to [email protected] in go.mod * [automated] make revendor * Add missing WarningsOn{Create,Update} to rest strategies * Replace dot imports for github.com/onsi/gomega/types Fix linting errors: `Assertion` redeclared in this block (typecheck) * Switch to typed values for WebhookInstallOptions.*Webhooks ref kubernetes-sigs/controller-runtime#1626 * RequestCertificate now takes an optional requestedDuration ref kubernetes/kubernetes#99494 * Switch to matchers.DeepEqual to test semantic equality Maps (e.g. labels, selectors, resource requirements) might be sorted differently than expected. Hence, use semantic equality instead of strict equality, as this is what matters to us. Also, DeepEqual outputs yaml and adds a nice diff indicator instead of printing some large confusing go struct representation. * Add new memorySwap field to expected kubelet config ref kubernetes/kubernetes#102823 * Round condition.lastUpdateTime to seconds in test There were several changes in the fake clients that might cause the failure to happen just now. * Correct unit tests falsely succeeding These tests were not preparing the test objects correctly: they only updated them in memory but not on the fake client. This wasn't caught until now because the fake client mimicked the real json decoder, which didn't unset fields not present on the server. Now that the fake client zeroes fields, the tests started failing (which is correct). So fix the tests. ref kubernetes-sigs/controller-runtime#1651 * Remove workarounds for missing zeroing in json decoder Now that the c-r client zeroes fields before decoding into the object, we can drop our workarounds for this, so basically drop kutil.CreateResetObjectFunc and its usages. ref kubernetes-sigs/controller-runtime#1640 * Drop setting webhook gvk explicitly in envtest webhookConfig.SetGroupVersionKind is not needed anymore with kubernetes-sigs/controller-runtime#1665 * Add some follow-up TODO comments * [automated] make generate but with go 1.16.9 * Address review comments
What type of PR is this?
/kind feature
/kind api-change
/sig node
What this PR does / why we need it:
Adds swap support per KEP-2400.
Which issue(s) this PR fixes:
Fixes #53533.
Special notes for your reviewer:
Design details in https://github.com/kubernetes/enhancements/blob/master/keps/sig-node/2400-node-swap/README.md#design-details
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: