Skip to content

Commit

Permalink
Merge pull request kubernetes#128186 from sreeram-venkitesh/117767-in…
Browse files Browse the repository at this point in the history
…-place-pod-vertical-scaling-version-skew

Updated version skew strategy for InPlacePodVerticalScaling
  • Loading branch information
k8s-ci-robot authored Nov 8, 2024
2 parents 4d10ae8 + 851dbf2 commit 46b3d9b
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 0 deletions.
21 changes: 21 additions & 0 deletions pkg/apis/core/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5503,6 +5503,10 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
allErrs = append(allErrs, field.Invalid(specPath, newPod.Status.QOSClass, "Pod QOS Class may not change as a result of resizing"))
}

if !isPodResizeRequestSupported(*oldPod) {
allErrs = append(allErrs, field.Forbidden(specPath, "Pod running on node without support for resize"))
}

// Ensure that only CPU and memory resources are mutable.
originalCPUMemPodSpec := *newPod.Spec.DeepCopy()
var newContainers []core.Container
Expand Down Expand Up @@ -5543,6 +5547,23 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
return allErrs
}

// isPodResizeRequestSupported checks whether the pod is running on a node with InPlacePodVerticalScaling enabled.
func isPodResizeRequestSupported(pod core.Pod) bool {
// TODO: Remove this after GA+3 releases of InPlacePodVerticalScaling
// This code handles the version skew as described in the KEP.
// For handling version skew we're only allowing to update the Pod's Resources
// if the Pod already has Pod.Status.ContainerStatuses[i].Resources. This means
// that the apiserver would only allow updates to Pods running on Nodes with
// the InPlacePodVerticalScaling feature gate enabled.
for _, c := range pod.Status.ContainerStatuses {
if c.State.Running != nil {
return c.Resources != nil
}
}
// No running containers. We cannot tell whether the node supports resize at this point, so we assume it does.
return true
}

// ValidatePodBinding tests if required fields in the pod binding are legal.
func ValidatePodBinding(binding *core.Binding) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down
68 changes: 68 additions & 0 deletions pkg/apis/core/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25203,6 +25203,74 @@ func TestValidatePodResize(t *testing.T) {
new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", ""), podtest.SetOS(core.Windows)),
err: "Forbidden: windows pods cannot be resized",
},
{
test: "Pod with nil Resource field in Status",
old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
ContainerStatuses: []core.ContainerStatus{{
ContainerID: "docker://numbers",
Image: "nginx:alpine",
Name: "main",
Ready: true,
Started: proto.Bool(true),
Resources: nil,
State: core.ContainerState{
Running: &core.ContainerStateRunning{
StartedAt: metav1.NewTime(time.Now()),
},
},
}},
})),
new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
err: "Forbidden: Pod running on node without support for resize",
},
{
test: "Pod with non-nil Resources field in Status",
old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
ContainerStatuses: []core.ContainerStatus{{
ContainerID: "docker://numbers",
Image: "nginx:alpine",
Name: "main",
Ready: true,
Started: proto.Bool(true),
Resources: &core.ResourceRequirements{},
State: core.ContainerState{
Running: &core.ContainerStateRunning{
StartedAt: metav1.NewTime(time.Now()),
},
},
}},
})),
new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
err: "",
},
{
test: "Pod without running containers",
old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
ContainerStatuses: []core.ContainerStatus{},
})),
new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
err: "",
},
{
test: "Pod with containers which are not running yet",
old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
ContainerStatuses: []core.ContainerStatus{{
ContainerID: "docker://numbers",
Image: "nginx:alpine",
Name: "main",
Ready: true,
Started: proto.Bool(true),
Resources: &core.ResourceRequirements{},
State: core.ContainerState{
Waiting: &core.ContainerStateWaiting{
Reason: "PodInitializing",
},
},
}},
})),
new: mkPod(core.ResourceList{}, getResources("200m", "0", "1Gi", "")),
err: "",
},
}

for _, test := range tests {
Expand Down

0 comments on commit 46b3d9b

Please sign in to comment.