Set default resize policy only for specified resource types, rename RestartNotRequired -> NotRequired

This commit is contained in:
vinay kulkarni 2023-03-10 01:57:14 +00:00
parent 9411050448
commit 9a805db010
10 changed files with 47 additions and 48 deletions

View File

@ -2290,7 +2290,7 @@ func TestDropInPlacePodVerticalScaling(t *testing.T) {
Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("200m")}, Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("200m")},
}, },
ResizePolicy: []api.ContainerResizePolicy{ ResizePolicy: []api.ContainerResizePolicy{
{ResourceName: api.ResourceCPU, RestartPolicy: api.RestartNotRequired}, {ResourceName: api.ResourceCPU, RestartPolicy: api.NotRequired},
{ResourceName: api.ResourceMemory, RestartPolicy: api.RestartContainer}, {ResourceName: api.ResourceMemory, RestartPolicy: api.RestartContainer},
}, },
}, },

View File

@ -2143,11 +2143,11 @@ type ResourceResizeRestartPolicy string
// These are the valid resource resize restart policy values: // These are the valid resource resize restart policy values:
const ( const (
// 'RestartNotRequired' means Kubernetes will try to resize the container // 'NotRequired' means Kubernetes will try to resize the container
// without restarting it, if possible. Kubernetes may however choose to // without restarting it, if possible. Kubernetes may however choose to
// restart the container if it is unable to actuate resize without a // restart the container if it is unable to actuate resize without a
// restart. For e.g. the runtime doesn't support restart-free resizing. // restart. For e.g. the runtime doesn't support restart-free resizing.
RestartNotRequired ResourceResizeRestartPolicy = "RestartNotRequired" NotRequired ResourceResizeRestartPolicy = "NotRequired"
// 'RestartContainer' means Kubernetes will resize the container in-place // 'RestartContainer' means Kubernetes will resize the container in-place
// by stopping and starting the container when new resources are applied. // by stopping and starting the container when new resources are applied.
// This is needed for legacy applications. For e.g. java apps using the // This is needed for legacy applications. For e.g. java apps using the
@ -2161,7 +2161,7 @@ type ContainerResizePolicy struct {
// Supported values: cpu, memory. // Supported values: cpu, memory.
ResourceName ResourceName ResourceName ResourceName
// Restart policy to apply when specified resource is resized. // Restart policy to apply when specified resource is resized.
// If not specified, it defaults to RestartNotRequired. // If not specified, it defaults to NotRequired.
RestartPolicy ResourceResizeRestartPolicy RestartPolicy ResourceResizeRestartPolicy
} }

View File

@ -159,25 +159,27 @@ func SetDefaults_Pod(obj *v1.Pod) {
} }
} }
} }
if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) { if utilfeature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) &&
// For normal containers, set resize restart policy to default value (RestartNotRequired), if not specified. obj.Spec.Containers[i].Resources.Requests != nil {
// For normal containers, set resize restart policy to default value (NotRequired), if not specified.
resizePolicySpecified := make(map[v1.ResourceName]bool) resizePolicySpecified := make(map[v1.ResourceName]bool)
for _, p := range obj.Spec.Containers[i].ResizePolicy { for _, p := range obj.Spec.Containers[i].ResizePolicy {
resizePolicySpecified[p.ResourceName] = true resizePolicySpecified[p.ResourceName] = true
} }
if _, found := resizePolicySpecified[v1.ResourceCPU]; !found { setDefaultResizePolicy := func(resourceName v1.ResourceName) {
obj.Spec.Containers[i].ResizePolicy = append(obj.Spec.Containers[i].ResizePolicy, if _, found := resizePolicySpecified[resourceName]; !found {
v1.ContainerResizePolicy{ obj.Spec.Containers[i].ResizePolicy = append(obj.Spec.Containers[i].ResizePolicy,
ResourceName: v1.ResourceCPU, v1.ContainerResizePolicy{
RestartPolicy: v1.RestartNotRequired, ResourceName: resourceName,
}) RestartPolicy: v1.NotRequired,
})
}
} }
if _, found := resizePolicySpecified[v1.ResourceMemory]; !found { if _, exists := obj.Spec.Containers[i].Resources.Requests[v1.ResourceCPU]; exists {
obj.Spec.Containers[i].ResizePolicy = append(obj.Spec.Containers[i].ResizePolicy, setDefaultResizePolicy(v1.ResourceCPU)
v1.ContainerResizePolicy{ }
ResourceName: v1.ResourceMemory, if _, exists := obj.Spec.Containers[i].Resources.Requests[v1.ResourceMemory]; exists {
RestartPolicy: v1.RestartNotRequired, setDefaultResizePolicy(v1.ResourceMemory)
})
} }
} }
} }

View File

@ -322,9 +322,6 @@ func testPodDefaults(t *testing.T, featuresEnabled bool) {
".Spec.Volumes[0].VolumeSource.ScaleIO.StorageMode": `"ThinProvisioned"`, ".Spec.Volumes[0].VolumeSource.ScaleIO.StorageMode": `"ThinProvisioned"`,
".Spec.Volumes[0].VolumeSource.Secret.DefaultMode": `420`, ".Spec.Volumes[0].VolumeSource.Secret.DefaultMode": `420`,
} }
if featuresEnabled {
expectedDefaults[".Spec.Containers[0].ResizePolicy"] = `[{"resourceName":"cpu","restartPolicy":"RestartNotRequired"},{"resourceName":"memory","restartPolicy":"RestartNotRequired"}]`
}
defaults := detectDefaults(t, pod, reflect.ValueOf(pod)) defaults := detectDefaults(t, pod, reflect.ValueOf(pod))
if !reflect.DeepEqual(expectedDefaults, defaults) { if !reflect.DeepEqual(expectedDefaults, defaults) {
t.Errorf("Defaults for PodSpec changed. This can cause spurious restarts of containers on API server upgrade.") t.Errorf("Defaults for PodSpec changed. This can cause spurious restarts of containers on API server upgrade.")

View File

@ -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 supportedResizeResources = sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory))
var supportedResizePolicies = sets.NewString(string(core.RestartNotRequired), string(core.RestartContainer)) var supportedResizePolicies = sets.NewString(string(core.NotRequired), string(core.RestartContainer))
func validateResizePolicy(policyList []core.ContainerResizePolicy, fldPath *field.Path) field.ErrorList { func validateResizePolicy(policyList []core.ContainerResizePolicy, fldPath *field.Path) field.ErrorList {
allErrors := field.ErrorList{} allErrors := field.ErrorList{}
@ -3036,7 +3036,7 @@ func validateResizePolicy(policyList []core.ContainerResizePolicy, fldPath *fiel
allErrors = append(allErrors, field.NotSupported(fldPath, p.ResourceName, supportedResizeResources.List())) allErrors = append(allErrors, field.NotSupported(fldPath, p.ResourceName, supportedResizeResources.List()))
} }
switch p.RestartPolicy { switch p.RestartPolicy {
case core.RestartNotRequired, core.RestartContainer: case core.NotRequired, core.RestartContainer:
case "": case "":
allErrors = append(allErrors, field.Required(fldPath, "")) allErrors = append(allErrors, field.Required(fldPath, ""))
default: default:

View File

@ -7009,7 +7009,7 @@ func TestValidatePullPolicy(t *testing.T) {
func TestValidateResizePolicy(t *testing.T) { func TestValidateResizePolicy(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InPlacePodVerticalScaling, true)()
tSupportedResizeResources := sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory)) tSupportedResizeResources := sets.NewString(string(core.ResourceCPU), string(core.ResourceMemory))
tSupportedResizePolicies := sets.NewString(string(core.RestartNotRequired), string(core.RestartContainer)) tSupportedResizePolicies := sets.NewString(string(core.NotRequired), string(core.RestartContainer))
type T struct { type T struct {
PolicyList []core.ContainerResizePolicy PolicyList []core.ContainerResizePolicy
ExpectError bool ExpectError bool
@ -7018,7 +7018,7 @@ func TestValidateResizePolicy(t *testing.T) {
testCases := map[string]T{ testCases := map[string]T{
"ValidCPUandMemoryPolicies": { "ValidCPUandMemoryPolicies": {
[]core.ContainerResizePolicy{ []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
{ResourceName: "memory", RestartPolicy: "RestartContainer"}, {ResourceName: "memory", RestartPolicy: "RestartContainer"},
}, },
false, false,
@ -7033,7 +7033,7 @@ func TestValidateResizePolicy(t *testing.T) {
}, },
"ValidMemoryPolicy": { "ValidMemoryPolicy": {
[]core.ContainerResizePolicy{ []core.ContainerResizePolicy{
{ResourceName: "memory", RestartPolicy: "RestartNotRequired"}, {ResourceName: "memory", RestartPolicy: "NotRequired"},
}, },
false, false,
nil, nil,
@ -7045,7 +7045,7 @@ func TestValidateResizePolicy(t *testing.T) {
}, },
"ValidCPUandInvalidMemoryPolicy": { "ValidCPUandInvalidMemoryPolicy": {
[]core.ContainerResizePolicy{ []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
{ResourceName: "memory", RestartPolicy: "Restarrrt"}, {ResourceName: "memory", RestartPolicy: "Restarrrt"},
}, },
true, true,
@ -7061,7 +7061,7 @@ func TestValidateResizePolicy(t *testing.T) {
}, },
"InvalidResourceNameValidPolicy": { "InvalidResourceNameValidPolicy": {
[]core.ContainerResizePolicy{ []core.ContainerResizePolicy{
{ResourceName: "cpuuu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpuuu", RestartPolicy: "NotRequired"},
}, },
true, true,
field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceName("cpuuu"), tSupportedResizeResources.List())}, field.ErrorList{field.NotSupported(field.NewPath("field"), core.ResourceName("cpuuu"), tSupportedResizeResources.List())},
@ -7075,7 +7075,7 @@ func TestValidateResizePolicy(t *testing.T) {
}, },
"RepeatedPolicies": { "RepeatedPolicies": {
[]core.ContainerResizePolicy{ []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
{ResourceName: "memory", RestartPolicy: "RestartContainer"}, {ResourceName: "memory", RestartPolicy: "RestartContainer"},
{ResourceName: "cpu", RestartPolicy: "RestartContainer"}, {ResourceName: "cpu", RestartPolicy: "RestartContainer"},
}, },
@ -7475,7 +7475,7 @@ func TestValidateEphemeralContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent", ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File", TerminationMessagePolicy: "File",
ResizePolicy: []core.ContainerResizePolicy{ ResizePolicy: []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
}, },
}, },
}, },
@ -7702,7 +7702,7 @@ func TestValidateContainers(t *testing.T) {
Name: "resources-resize-policy", Name: "resources-resize-policy",
Image: "image", Image: "image",
ResizePolicy: []core.ContainerResizePolicy{ ResizePolicy: []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
{ResourceName: "memory", RestartPolicy: "RestartContainer"}, {ResourceName: "memory", RestartPolicy: "RestartContainer"},
}, },
ImagePullPolicy: "IfNotPresent", ImagePullPolicy: "IfNotPresent",
@ -9477,7 +9477,7 @@ func TestValidatePodSpec(t *testing.T) {
Name: "initctr", Name: "initctr",
Image: "initimage", Image: "initimage",
ResizePolicy: []core.ContainerResizePolicy{ ResizePolicy: []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
}, },
ImagePullPolicy: "IfNotPresent", ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File", TerminationMessagePolicy: "File",
@ -9488,7 +9488,7 @@ func TestValidatePodSpec(t *testing.T) {
Name: "ctr", Name: "ctr",
Image: "image", Image: "image",
ResizePolicy: []core.ContainerResizePolicy{ ResizePolicy: []core.ContainerResizePolicy{
{ResourceName: "cpu", RestartPolicy: "RestartNotRequired"}, {ResourceName: "cpu", RestartPolicy: "NotRequired"},
}, },
ImagePullPolicy: "IfNotPresent", ImagePullPolicy: "IfNotPresent",
TerminationMessagePolicy: "File", TerminationMessagePolicy: "File",

View File

@ -915,8 +915,8 @@ func TestHashContainerWithoutResources(t *testing.T) {
cpu200m := resource.MustParse("200m") cpu200m := resource.MustParse("200m")
mem100M := resource.MustParse("100Mi") mem100M := resource.MustParse("100Mi")
mem200M := resource.MustParse("200Mi") mem200M := resource.MustParse("200Mi")
cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartNotRequired} cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.NotRequired}
memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartNotRequired} memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.NotRequired}
cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer} cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer}
memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartContainer} memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartContainer}
@ -938,7 +938,7 @@ func TestHashContainerWithoutResources(t *testing.T) {
}, },
ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartRequired, memPolicyRestartNotRequired}, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartRequired, memPolicyRestartNotRequired},
}, },
0xbf0fc03d, 0x5f62cb4c,
}, },
{ {
"Burstable pod with memory policy restart required", "Burstable pod with memory policy restart required",
@ -951,7 +951,7 @@ func TestHashContainerWithoutResources(t *testing.T) {
}, },
ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired}, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired},
}, },
0x97dd7301, 0xcdab9e00,
}, },
{ {
"Guaranteed pod with CPU policy restart required", "Guaranteed pod with CPU policy restart required",
@ -964,7 +964,7 @@ func TestHashContainerWithoutResources(t *testing.T) {
}, },
ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartRequired, memPolicyRestartNotRequired}, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartRequired, memPolicyRestartNotRequired},
}, },
0xbf0fc03d, 0x5f62cb4c,
}, },
{ {
"Guaranteed pod with memory policy restart required", "Guaranteed pod with memory policy restart required",
@ -977,7 +977,7 @@ func TestHashContainerWithoutResources(t *testing.T) {
}, },
ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired}, ResizePolicy: []v1.ContainerResizePolicy{cpuPolicyRestartNotRequired, memPolicyRestartRequired},
}, },
0x97dd7301, 0xcdab9e00,
}, },
} }
for _, tc := range tests { for _, tc := range tests {

View File

@ -1654,8 +1654,8 @@ func TestComputePodActionsForPodResize(t *testing.T) {
cpu200m := resource.MustParse("200m") cpu200m := resource.MustParse("200m")
mem100M := resource.MustParse("100Mi") mem100M := resource.MustParse("100Mi")
mem200M := resource.MustParse("200Mi") mem200M := resource.MustParse("200Mi")
cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartNotRequired} cpuPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.NotRequired}
memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartNotRequired} memPolicyRestartNotRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.NotRequired}
cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer} cpuPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceCPU, RestartPolicy: v1.RestartContainer}
memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartContainer} memPolicyRestartRequired := v1.ContainerResizePolicy{ResourceName: v1.ResourceMemory, RestartPolicy: v1.RestartContainer}

View File

@ -2268,11 +2268,11 @@ type ResourceResizeRestartPolicy string
// These are the valid resource resize restart policy values: // These are the valid resource resize restart policy values:
const ( const (
// 'RestartNotRequired' means Kubernetes will try to resize the container // 'NotRequired' means Kubernetes will try to resize the container
// without restarting it, if possible. Kubernetes may however choose to // without restarting it, if possible. Kubernetes may however choose to
// restart the container if it is unable to actuate resize without a // restart the container if it is unable to actuate resize without a
// restart. For e.g. the runtime doesn't support restart-free resizing. // restart. For e.g. the runtime doesn't support restart-free resizing.
RestartNotRequired ResourceResizeRestartPolicy = "RestartNotRequired" NotRequired ResourceResizeRestartPolicy = "NotRequired"
// 'RestartContainer' means Kubernetes will resize the container in-place // 'RestartContainer' means Kubernetes will resize the container in-place
// by stopping and starting the container when new resources are applied. // by stopping and starting the container when new resources are applied.
// This is needed for legacy applications. For e.g. java apps using the // This is needed for legacy applications. For e.g. java apps using the
@ -2286,7 +2286,7 @@ type ContainerResizePolicy struct {
// Supported values: cpu, memory. // Supported values: cpu, memory.
ResourceName ResourceName `json:"resourceName" protobuf:"bytes,1,opt,name=resourceName,casttype=ResourceName"` ResourceName ResourceName `json:"resourceName" protobuf:"bytes,1,opt,name=resourceName,casttype=ResourceName"`
// Restart policy to apply when specified resource is resized. // Restart policy to apply when specified resource is resized.
// If not specified, it defaults to RestartNotRequired. // If not specified, it defaults to NotRequired.
RestartPolicy ResourceResizeRestartPolicy `json:"restartPolicy" protobuf:"bytes,2,opt,name=restartPolicy,casttype=ResourceResizeRestartPolicy"` RestartPolicy ResourceResizeRestartPolicy `json:"restartPolicy" protobuf:"bytes,2,opt,name=restartPolicy,casttype=ResourceResizeRestartPolicy"`
} }

View File

@ -157,7 +157,7 @@ func getTestResourceInfo(tcInfo TestContainerInfo) (v1.ResourceRequirements, v1.
} }
func initDefaultResizePolicy(containers []TestContainerInfo) { func initDefaultResizePolicy(containers []TestContainerInfo) {
noRestart := v1.RestartNotRequired noRestart := v1.NotRequired
setDefaultPolicy := func(ci *TestContainerInfo) { setDefaultPolicy := func(ci *TestContainerInfo) {
if ci.CPUPolicy == nil { if ci.CPUPolicy == nil {
ci.CPUPolicy = &noRestart ci.CPUPolicy = &noRestart
@ -500,7 +500,7 @@ func doPodResizeTests() {
expected []TestContainerInfo expected []TestContainerInfo
} }
noRestart := v1.RestartNotRequired noRestart := v1.NotRequired
doRestart := v1.RestartContainer doRestart := v1.RestartContainer
tests := []testCase{ tests := []testCase{
{ {
@ -1010,7 +1010,7 @@ func doPodResizeTests() {
}, },
}, },
{ {
name: "Guaranteed QoS pod, one container - increase CPU (RestartNotRequired) & memory (RestartContainer)", name: "Guaranteed QoS pod, one container - increase CPU (NotRequired) & memory (RestartContainer)",
containers: []TestContainerInfo{ containers: []TestContainerInfo{
{ {
Name: "c1", Name: "c1",
@ -1033,7 +1033,7 @@ func doPodResizeTests() {
}, },
}, },
{ {
name: "Burstable QoS pod, one container - decrease CPU (RestartContainer) & memory (RestartNotRequired)", name: "Burstable QoS pod, one container - decrease CPU (RestartContainer) & memory (NotRequired)",
containers: []TestContainerInfo{ containers: []TestContainerInfo{
{ {
Name: "c1", Name: "c1",