spec.terminationGracePeriodSeconds allow it to be set to 1s if it was previously negative

This commit is contained in:
Shiming Zhang 2021-06-28 11:49:04 +08:00
parent 657d93c4cc
commit 40593fa4d3
2 changed files with 48 additions and 1 deletions

View File

@ -3973,6 +3973,7 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
// 1. spec.containers[*].image // 1. spec.containers[*].image
// 2. spec.initContainers[*].image // 2. spec.initContainers[*].image
// 3. spec.activeDeadlineSeconds // 3. spec.activeDeadlineSeconds
// 4. spec.terminationGracePeriodSeconds
containerErrs, stop := ValidateContainerUpdates(newPod.Spec.Containers, oldPod.Spec.Containers, specPath.Child("containers")) containerErrs, stop := ValidateContainerUpdates(newPod.Spec.Containers, oldPod.Spec.Containers, specPath.Child("containers"))
allErrs = append(allErrs, containerErrs...) allErrs = append(allErrs, containerErrs...)
@ -4039,11 +4040,17 @@ func ValidatePodUpdate(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
// tolerations are checked before the deep copy, so munge those too // tolerations are checked before the deep copy, so munge those too
mungedPodSpec.Tolerations = oldPod.Spec.Tolerations // +k8s:verify-mutation:reason=clone mungedPodSpec.Tolerations = oldPod.Spec.Tolerations // +k8s:verify-mutation:reason=clone
// Relax validation of immutable fields to allow it to be set to 1 if it was previously negative.
if oldPod.Spec.TerminationGracePeriodSeconds != nil && *oldPod.Spec.TerminationGracePeriodSeconds < 0 &&
mungedPodSpec.TerminationGracePeriodSeconds != nil && *mungedPodSpec.TerminationGracePeriodSeconds == 1 {
mungedPodSpec.TerminationGracePeriodSeconds = oldPod.Spec.TerminationGracePeriodSeconds // +k8s:verify-mutation:reason=clone
}
if !apiequality.Semantic.DeepEqual(mungedPodSpec, oldPod.Spec) { if !apiequality.Semantic.DeepEqual(mungedPodSpec, oldPod.Spec) {
// This diff isn't perfect, but it's a helluva lot better an "I'm not going to tell you what the difference is". // This diff isn't perfect, but it's a helluva lot better an "I'm not going to tell you what the difference is".
//TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff //TODO: Pinpoint the specific field that causes the invalid error after we have strategic merge diff
specDiff := diff.ObjectDiff(mungedPodSpec, oldPod.Spec) specDiff := diff.ObjectDiff(mungedPodSpec, oldPod.Spec)
allErrs = append(allErrs, field.Forbidden(specPath, fmt.Sprintf("pod updates may not change fields other than `spec.containers[*].image`, `spec.initContainers[*].image`, `spec.activeDeadlineSeconds` or `spec.tolerations` (only additions to existing tolerations)\n%v", specDiff))) allErrs = append(allErrs, field.Forbidden(specPath, fmt.Sprintf("pod updates may not change fields other than `spec.containers[*].image`, `spec.initContainers[*].image`, `spec.activeDeadlineSeconds`, `spec.tolerations` (only additions to existing tolerations) or `spec.terminationGracePeriodSeconds` (allow it to be set to 1 if it was previously negative)\n%v", specDiff)))
} }
return allErrs return allErrs

View File

@ -9803,6 +9803,46 @@ func TestValidatePodUpdate(t *testing.T) {
"spec: Forbidden: pod updates", "spec: Forbidden: pod updates",
"removed priority class name", "removed priority class name",
}, },
{
core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
TerminationGracePeriodSeconds: utilpointer.Int64Ptr(1),
},
},
core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1),
},
},
"",
"update termination grace period seconds",
},
{
core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
TerminationGracePeriodSeconds: utilpointer.Int64Ptr(0),
},
},
core.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
},
Spec: core.PodSpec{
TerminationGracePeriodSeconds: utilpointer.Int64Ptr(-1),
},
},
"spec: Forbidden: pod updates",
"update termination grace period seconds not 1",
},
} }
for _, test := range tests { for _, test := range tests {