From 8b23497ae76ba4116d893e4d97ab9736198e4d99 Mon Sep 17 00:00:00 2001 From: vinay kulkarni Date: Tue, 28 Feb 2023 07:30:31 +0000 Subject: [PATCH] Restructure naming of resource resize restart policy --- pkg/api/pod/util_test.go | 4 +- pkg/apis/core/types.go | 22 ++++----- pkg/apis/core/validation/validation.go | 8 ++-- pkg/apis/core/validation/validation_test.go | 48 +++++++++---------- pkg/kubelet/container/helpers_test.go | 16 +++---- .../kuberuntime/kuberuntime_manager.go | 6 +-- .../kuberuntime/kuberuntime_manager_test.go | 8 ++-- staging/src/k8s.io/api/core/v1/types.go | 22 ++++----- test/e2e/node/pod_resize.go | 14 +++--- 9 files changed, 74 insertions(+), 74 deletions(-) diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 82ed4a9ed7f..69da98eb5da 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -2290,8 +2290,8 @@ func TestDropInPlacePodVerticalScaling(t *testing.T) { Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("200m")}, }, ResizePolicy: []api.ContainerResizePolicy{ - {ResourceName: api.ResourceCPU, Policy: api.RestartNotRequired}, - {ResourceName: api.ResourceMemory, Policy: api.RestartRequired}, + {ResourceName: api.ResourceCPU, RestartPolicy: api.RestartNotRequired}, + {ResourceName: api.ResourceMemory, RestartPolicy: api.RestartContainer}, }, }, }, diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index dbe73a6f755..7d31e34502c 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2138,31 +2138,31 @@ const ( PullIfNotPresent PullPolicy = "IfNotPresent" ) -// ResourceResizePolicy specifies how Kubernetes should handle resource resize. -type ResourceResizePolicy string +// ResourceResizeRestartPolicy specifies how to handle container resource resize. +type ResourceResizeRestartPolicy string -// These are the valid resource resize policy values: +// These are the valid resource resize restart policy values: const ( - // RestartNotRequired tells Kubernetes to resize the container in-place + // 'RestartNotRequired' means Kubernetes will try to resize the container // without restarting it, if possible. Kubernetes may however choose to // restart the container if it is unable to actuate resize without a // restart. For e.g. the runtime doesn't support restart-free resizing. - RestartNotRequired ResourceResizePolicy = "RestartNotRequired" - // 'RestartRequired' tells Kubernetes to resize the container in-place + RestartNotRequired ResourceResizeRestartPolicy = "RestartNotRequired" + // 'RestartContainer' means Kubernetes will resize the container in-place // by stopping and starting the container when new resources are applied. // This is needed for legacy applications. For e.g. java apps using the // -xmxN flag which are unable to use resized memory without restarting. - RestartRequired ResourceResizePolicy = "RestartRequired" + RestartContainer ResourceResizeRestartPolicy = "RestartContainer" ) -// ContainerResizePolicy represents resource resize policy for a single container. +// ContainerResizePolicy represents resource resize policy for the container. type ContainerResizePolicy struct { - // Name of the resource type to which this resource resize policy applies. + // Name of the resource to which this resource resize policy applies. // Supported values: cpu, memory. ResourceName ResourceName - // Resource resize policy applicable to the specified resource name. + // Restart policy to apply when specified resource is resized. // If not specified, it defaults to RestartNotRequired. - Policy ResourceResizePolicy + RestartPolicy ResourceResizeRestartPolicy } // PreemptionPolicy describes a policy for if/when to preempt a pod. diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 61d0fb3af42..ed8c4e3801b 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3016,7 +3016,7 @@ func validatePullPolicy(policy core.PullPolicy, fldPath *field.Path) field.Error } var supportedResizeResources = sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) -var supportedResizePolicies = sets.NewString(string(core.RestartNotRequired), string(core.RestartRequired)) +var supportedResizePolicies = sets.NewString(string(core.RestartNotRequired), string(core.RestartContainer)) func validateResizePolicy(policyList []core.ContainerResizePolicy, fldPath *field.Path) field.ErrorList { allErrors := field.ErrorList{} @@ -3035,12 +3035,12 @@ func validateResizePolicy(policyList []core.ContainerResizePolicy, fldPath *fiel default: allErrors = append(allErrors, field.NotSupported(fldPath, p.ResourceName, supportedResizeResources.List())) } - switch p.Policy { - case core.RestartNotRequired, core.RestartRequired: + switch p.RestartPolicy { + case core.RestartNotRequired, core.RestartContainer: case "": allErrors = append(allErrors, field.Required(fldPath, "")) default: - allErrors = append(allErrors, field.NotSupported(fldPath, p.Policy, supportedResizePolicies.List())) + allErrors = append(allErrors, field.NotSupported(fldPath, p.RestartPolicy, supportedResizePolicies.List())) } } return allErrors diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index d20f0ffa532..23f1d6ee56c 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -7009,7 +7009,7 @@ func TestValidatePullPolicy(t *testing.T) { func TestValidateResizePolicy(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true)() tSupportedResizeResources := sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) - tSupportedResizePolicies := sets.NewString(string(core.RestartNotRequired), string(core.RestartRequired)) + tSupportedResizePolicies := sets.NewString(string(core.RestartNotRequired), string(core.RestartContainer)) type T struct { PolicyList []core.ContainerResizePolicy ExpectError bool @@ -7018,22 +7018,22 @@ func TestValidateResizePolicy(t *testing.T) { testCases := map[string]T{ "ValidCPUandMemoryPolicies": { []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, - {ResourceName: "memory", Policy: "RestartRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, + {ResourceName: "memory", RestartPolicy: "RestartContainer"}, }, false, nil, }, "ValidCPUPolicy": { []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartContainer"}, }, false, nil, }, "ValidMemoryPolicy": { []core.ContainerResizePolicy{ - {ResourceName: "memory", Policy: "RestartNotRequired"}, + {ResourceName: "memory", RestartPolicy: "RestartNotRequired"}, }, false, nil, @@ -7045,39 +7045,39 @@ func TestValidateResizePolicy(t *testing.T) { }, "ValidCPUandInvalidMemoryPolicy": { []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, - {ResourceName: "memory", Policy: "Restarrrt"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, + {ResourceName: "memory", RestartPolicy: "Restarrrt"}, }, true, - field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceResizePolicy("Restarrrt"), tSupportedResizePolicies.List())}, + field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceResizeRestartPolicy("Restarrrt"), tSupportedResizePolicies.List())}, }, "ValidMemoryandInvalidCPUPolicy": { []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequirrred"}, - {ResourceName: "memory", Policy: "RestartRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequirrred"}, + {ResourceName: "memory", RestartPolicy: "RestartContainer"}, }, true, - field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceResizePolicy("RestartNotRequirrred"), tSupportedResizePolicies.List())}, + field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceResizeRestartPolicy("RestartNotRequirrred"), tSupportedResizePolicies.List())}, }, "InvalidResourceNameValidPolicy": { []core.ContainerResizePolicy{ - {ResourceName: "cpuuu", Policy: "RestartNotRequired"}, + {ResourceName: "cpuuu", RestartPolicy: "RestartNotRequired"}, }, true, field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceName("cpuuu"), tSupportedResizeResources.List())}, }, "ValidResourceNameMissingPolicy": { []core.ContainerResizePolicy{ - {ResourceName: "memory", Policy: ""}, + {ResourceName: "memory", RestartPolicy: ""}, }, true, field.ErrorList{field.Required(field.NewPath("field"), "")}, }, "RepeatedPolicies": { []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, - {ResourceName: "memory", Policy: "RestartRequired"}, - {ResourceName: "cpu", Policy: "RestartRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, + {ResourceName: "memory", RestartPolicy: "RestartContainer"}, + {ResourceName: "cpu", RestartPolicy: "RestartContainer"}, }, true, field.ErrorList{field.Duplicate(field.NewPath("field").Index(2), core.ResourceCPU)}, @@ -7475,7 +7475,7 @@ func TestValidateEphemeralContainers(t *testing.T) { ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", ResizePolicy: []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, }, }, }, @@ -7702,8 +7702,8 @@ func TestValidateContainers(t *testing.T) { Name: "resources-resize-policy", Image: "image", ResizePolicy: []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, - {ResourceName: "memory", Policy: "RestartRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, + {ResourceName: "memory", RestartPolicy: "RestartContainer"}, }, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", @@ -9477,7 +9477,7 @@ func TestValidatePodSpec(t *testing.T) { Name: "initctr", Image: "initimage", ResizePolicy: []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, }, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", @@ -9488,7 +9488,7 @@ func TestValidatePodSpec(t *testing.T) { Name: "ctr", Image: "image", ResizePolicy: []core.ContainerResizePolicy{ - {ResourceName: "cpu", Policy: "RestartNotRequired"}, + {ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, }, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File", @@ -20473,7 +20473,7 @@ func TestValidateOSFields(t *testing.T) { "Containers[*].Ports", "Containers[*].ReadinessProbe", "Containers[*].Resources", - "Containers[*].ResizePolicy[*].Policy", + "Containers[*].ResizePolicy[*].RestartPolicy", "Containers[*].ResizePolicy[*].ResourceName", "Containers[*].SecurityContext.RunAsNonRoot", "Containers[*].Stdin", @@ -20499,7 +20499,7 @@ func TestValidateOSFields(t *testing.T) { "EphemeralContainers[*].EphemeralContainerCommon.Ports", "EphemeralContainers[*].EphemeralContainerCommon.ReadinessProbe", "EphemeralContainers[*].EphemeralContainerCommon.Resources", - "EphemeralContainers[*].EphemeralContainerCommon.ResizePolicy[*].Policy", + "EphemeralContainers[*].EphemeralContainerCommon.ResizePolicy[*].RestartPolicy", "EphemeralContainers[*].EphemeralContainerCommon.ResizePolicy[*].ResourceName", "EphemeralContainers[*].EphemeralContainerCommon.Stdin", "EphemeralContainers[*].EphemeralContainerCommon.StdinOnce", @@ -20527,7 +20527,7 @@ func TestValidateOSFields(t *testing.T) { "InitContainers[*].Ports", "InitContainers[*].ReadinessProbe", "InitContainers[*].Resources", - "InitContainers[*].ResizePolicy[*].Policy", + "InitContainers[*].ResizePolicy[*].RestartPolicy", "InitContainers[*].ResizePolicy[*].ResourceName", "InitContainers[*].Stdin", "InitContainers[*].StdinOnce", diff --git a/pkg/kubelet/container/helpers_test.go b/pkg/kubelet/container/helpers_test.go index e02f7479a04..113d6270956 100644 --- a/pkg/kubelet/container/helpers_test.go +++ b/pkg/kubelet/container/helpers_test.go @@ -915,10 +915,10 @@ func TestHashContainerWithoutResources(t *testing.T) { cpu200m := resource.MustParse("200m") mem100M := resource.MustParse("100Mi") mem200M := resource.MustParse("200Mi") - cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, Policy: v1.RestartNotRequired} - memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, Policy: v1.RestartNotRequired} - cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, Policy: v1.RestartRequired} - memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, Policy: v1.RestartRequired} + cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartNotRequired} + memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartNotRequired} + cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer} + memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartContainer} type testCase struct { name string @@ -938,7 +938,7 @@ func TestHashContainerWithoutResources(t *testing.T) { }, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartRequired, memPolicyRestartNotRequired}, }, - 0x86a4393c, + 0xbf0fc03d, }, { "Burstable pod with memory policy restart required", @@ -951,7 +951,7 @@ func TestHashContainerWithoutResources(t *testing.T) { }, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired}, }, - 0x73a18cce, + 0x97dd7301, }, { "Guaranteed pod with CPU policy restart required", @@ -964,7 +964,7 @@ func TestHashContainerWithoutResources(t *testing.T) { }, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartRequired, memPolicyRestartNotRequired}, }, - 0x86a4393c, + 0xbf0fc03d, }, { "Guaranteed pod with memory policy restart required", @@ -977,7 +977,7 @@ func TestHashContainerWithoutResources(t *testing.T) { }, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired}, }, - 0x73a18cce, + 0x97dd7301, }, } for _, tc := range tests { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager.go b/pkg/kubelet/kuberuntime/kuberuntime_manager.go index ab0766d459c..c18a50a13f3 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager.go @@ -580,15 +580,15 @@ func (m *kubeGenericRuntimeManager) computePodResizeAction(pod *v1.Pod, containe cpuRequest: currentCPURequest, } - resizePolicy := make(map[v1.ResourceName]v1.ResourceResizePolicy) + resizePolicy := make(map[v1.ResourceName]v1.ResourceResizeRestartPolicy) for _, pol := range container.ResizePolicy { - resizePolicy[pol.ResourceName] = pol.Policy + resizePolicy[pol.ResourceName] = pol.RestartPolicy } determineContainerResize := func(rName v1.ResourceName, specValue, statusValue int64) (resize, restart bool) { if specValue == statusValue { return false, false } - if resizePolicy[rName] == v1.RestartRequired { + if resizePolicy[rName] == v1.RestartContainer { return true, true } return true, false diff --git a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go index 0747ea91e0a..333ba4ca7eb 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go @@ -1654,10 +1654,10 @@ func TestComputePodActionsForPodResize(t *testing.T) { cpu200m := resource.MustParse("200m") mem100M := resource.MustParse("100Mi") mem200M := resource.MustParse("200Mi") - cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, Policy: v1.RestartNotRequired} - memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, Policy: v1.RestartNotRequired} - cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, Policy: v1.RestartRequired} - memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, Policy: v1.RestartRequired} + cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartNotRequired} + memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartNotRequired} + cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer} + memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartContainer} for desc, test := range map[string]struct { podResizePolicyFn func(*v1.Pod) diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 3f3af74c390..3456e08ad70 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -2263,31 +2263,31 @@ const ( PullIfNotPresent PullPolicy = "IfNotPresent" ) -// ResourceResizePolicy specifies how Kubernetes should handle resource resize. -type ResourceResizePolicy string +// ResourceResizeRestartPolicy specifies how to handle container resource resize. +type ResourceResizeRestartPolicy string -// These are the valid resource resize policy values: +// These are the valid resource resize restart policy values: const ( - // RestartNotRequired tells Kubernetes to resize the container in-place + // 'RestartNotRequired' means Kubernetes will try to resize the container // without restarting it, if possible. Kubernetes may however choose to // restart the container if it is unable to actuate resize without a // restart. For e.g. the runtime doesn't support restart-free resizing. - RestartNotRequired ResourceResizePolicy = "RestartNotRequired" - // 'RestartRequired' tells Kubernetes to resize the container in-place + RestartNotRequired ResourceResizeRestartPolicy = "RestartNotRequired" + // 'RestartContainer' means Kubernetes will resize the container in-place // by stopping and starting the container when new resources are applied. // This is needed for legacy applications. For e.g. java apps using the // -xmxN flag which are unable to use resized memory without restarting. - RestartRequired ResourceResizePolicy = "RestartRequired" + RestartContainer ResourceResizeRestartPolicy = "RestartContainer" ) -// ContainerResizePolicy represents resource resize policy for a single container. +// ContainerResizePolicy represents resource resize policy for the container. type ContainerResizePolicy struct { - // Name of the resource type to which this resource resize policy applies. + // Name of the resource to which this resource resize policy applies. // Supported values: cpu, memory. ResourceName ResourceName `json:"resourceName" protobuf:"bytes,1,opt,name=resourceName,casttype=ResourceName"` - // Resource resize policy applicable to the specified resource name. + // Restart policy to apply when specified resource is resized. // If not specified, it defaults to RestartNotRequired. - Policy ResourceResizePolicy `json:"policy" protobuf:"bytes,2,opt,name=policy,casttype=ResourceResizePolicy"` + RestartPolicy ResourceResizeRestartPolicy `json:"restartPolicy" protobuf:"bytes,2,opt,name=restartPolicy,casttype=ResourceResizeRestartPolicy"` } // PreemptionPolicy describes a policy for if/when to preempt a pod. diff --git a/test/e2e/node/pod_resize.go b/test/e2e/node/pod_resize.go index 8582eff71df..003d65a6390 100644 --- a/test/e2e/node/pod_resize.go +++ b/test/e2e/node/pod_resize.go @@ -74,8 +74,8 @@ type TestContainerInfo struct { Name string Resources *ContainerResources Allocations *ContainerAllocations - CPUPolicy *v1.ResourceResizePolicy - MemPolicy *v1.ResourceResizePolicy + CPUPolicy *v1.ResourceResizeRestartPolicy + MemPolicy *v1.ResourceResizeRestartPolicy RestartCount int32 } @@ -146,11 +146,11 @@ func getTestResourceInfo(tcInfo TestContainerInfo) (v1.ResourceRequirements, v1. } if tcInfo.CPUPolicy != nil { - cpuPol := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, Policy: *tcInfo.CPUPolicy} + cpuPol := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: *tcInfo.CPUPolicy} resizePol = append(resizePol, cpuPol) } if tcInfo.MemPolicy != nil { - memPol := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, Policy: *tcInfo.MemPolicy} + memPol := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: *tcInfo.MemPolicy} resizePol = append(resizePol, memPol) } return res, alloc, resizePol @@ -501,7 +501,7 @@ func doPodResizeTests() { } noRestart := v1.RestartNotRequired - doRestart := v1.RestartRequired + doRestart := v1.RestartContainer tests := []testCase{ { name: "Guaranteed QoS pod, one container - increase CPU & memory", @@ -1010,7 +1010,7 @@ func doPodResizeTests() { }, }, { - name: "Guaranteed QoS pod, one container - increase CPU (RestartNotRequired) & memory (RestartRequired)", + name: "Guaranteed QoS pod, one container - increase CPU (RestartNotRequired) & memory (RestartContainer)", containers: []TestContainerInfo{ { Name: "c1", @@ -1033,7 +1033,7 @@ func doPodResizeTests() { }, }, { - name: "Burstable QoS pod, one container - decrease CPU (RestartRequired) & memory (RestartNotRequired)", + name: "Burstable QoS pod, one container - decrease CPU (RestartContainer) & memory (RestartNotRequired)", containers: []TestContainerInfo{ { Name: "c1",