github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-09 20:17:41 +00:00
Code Issues Projects Releases Wiki Activity

Resolved latest review comments

Browse Source
This commit is contained in:
vivzbansal 2024-12-17 07:56:13 +00:00
parent 242dec3e34
commit 5889da1bbc
4 changed files with 86 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
pkg/apis/core/validation/validation.go
View File

@ -5649,22 +5649,29 @@ func ValidatePodResize(newPod, oldPod *core.Pod, opts PodValidationOptions) fiel
originalCPUMemPodSpec.Containers = newContainers
// Ensure that only CPU and memory resources are mutable for restartable init containers.
// Also ensure that resources are immutable for non-restartable init containers.
var newInitContainers []core.Container
if utilfeature.DefaultFeatureGate.Enabled(features.SidecarContainers) {
for ix, container := range originalCPUMemPodSpec.InitContainers {
if isRestartableInitContainer(&container) { // restartable init container
dropCPUMemoryResourcesFromContainer(&container, &oldPod.Spec.InitContainers[ix])
if !apiequality.Semantic.DeepEqual(container, oldPod.Spec.InitContainers[ix]) {
// This likely means that the user has made changes to resources other than CPU and memory for sidecar container.
specDiff := cmp.Diff(oldPod.Spec.InitContainers[ix], container)
errs := field.Forbidden(specPath, fmt.Sprintf("only cpu and memory resources for sidecar containers are mutable\n%v", specDiff))
allErrs = append(allErrs, errs)
return allErrs
}
} else if !apiequality.Semantic.DeepEqual(container, oldPod.Spec.InitContainers[ix]) { // non-restartable init container
// This likely means that the user has modified resources of non-sidecar init container.
specDiff := cmp.Diff(oldPod.Spec.InitContainers[ix], container)
errs := field.Forbidden(specPath, fmt.Sprintf("resources for non-sidecar init containers are immutable\n%v", specDiff))
allErrs = append(allErrs, errs)
return allErrs
}
newInitContainers = append(newInitContainers, container)
}
originalCPUMemPodSpec.InitContainers = newInitContainers
if !apiequality.Semantic.DeepEqual(originalCPUMemPodSpec.InitContainers, oldPod.Spec.InitContainers) {
// This likely means that the user has modified non-sidecar container resources.
specDiff := cmp.Diff(oldPod.Spec.InitContainers, originalCPUMemPodSpec.InitContainers)
errs := field.Forbidden(specPath, fmt.Sprintf("cpu and memory resources for only sidecar containers are mutable\n%v", specDiff))
allErrs = append(allErrs, errs)
return allErrs
}
}
if !apiequality.Semantic.DeepEqual(originalCPUMemPodSpec, oldPod.Spec) {

6
pkg/apis/core/validation/validation_test.go
View File

@ -25911,7 +25911,7 @@ func TestValidatePodResize(t *testing.T) {
test: "Pod QoS unchanged, burstable -> burstable, remove cpu and memory requests",
old: mkPodWithInitContainers(getResources("100m", "100Mi", "", ""), getResources("100m", "", "", ""), ""),
new: mkPodWithInitContainers(core.ResourceList{}, getResources("100m", "", "", ""), ""),
err: "spec: Forbidden: cpu and memory resources for only sidecar containers are mutable",
err: "spec: Forbidden: resources for non-sidecar init containers are immutable",
}, {
test: "Pod with nil Resource field in Status",
old: mkPod(core.ResourceList{}, getResources("100m", "0", "1Gi", ""), podtest.SetStatus(core.PodStatus{
@ -25993,7 +25993,7 @@ func TestValidatePodResize(t *testing.T) {
test: "storage limit change for sidecar containers",
old: mkPodWithInitContainers(core.ResourceList{}, getResources("100m", "100Mi", "2Gi", ""), core.ContainerRestartPolicyAlways),
new: mkPodWithInitContainers(core.ResourceList{}, getResources("100m", "100Mi", "1Gi", ""), core.ContainerRestartPolicyAlways),
err: "spec: Forbidden: cpu and memory resources for only sidecar containers are mutable",
err: "spec: Forbidden: only cpu and memory resources for sidecar containers are mutable",
}, {
test: "cpu request change for sidecar containers",
old: mkPodWithInitContainers(getResources("200m", "0", "", ""), core.ResourceList{}, core.ContainerRestartPolicyAlways),
@ -26008,7 +26008,7 @@ func TestValidatePodResize(t *testing.T) {
test: "storage request change for sidecar containers",
old: mkPodWithInitContainers(getResources("100m", "0", "1Gi", ""), core.ResourceList{}, core.ContainerRestartPolicyAlways),
new: mkPodWithInitContainers(getResources("100m", "0", "2Gi", ""), core.ResourceList{}, core.ContainerRestartPolicyAlways),
err: "spec: Forbidden: cpu and memory resources for only sidecar containers are mutable",
err: "spec: Forbidden: only cpu and memory resources for sidecar containers are mutable",
},
}

91
pkg/kubelet/kubelet_pods_test.go
View File

@ -6880,44 +6880,59 @@ func TestAllocatedResourcesMatchStatus(t *testing.T) {
}}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
allocatedPod := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: v1.PodSpec{
Containers: []v1.Container{{
Name: "c",
Resources: test.allocatedResources,
}},
InitContainers: []v1.Container{{
Name: "c1-init",
Resources: test.allocatedResources,
RestartPolicy: &containerRestartPolicyAlways,
}},
},
}
state := kubecontainer.ContainerStateRunning
if test.statusTerminated {
state = kubecontainer.ContainerStateExited
}
podStatus := &kubecontainer.PodStatus{
Name: "test",
ContainerStatuses: []*kubecontainer.Status{
{
Name: "c",
State: state,
Resources: test.statusResources,
for _, isSidecarContainer := range []bool{false, true} {
t.Run(test.name, func(t *testing.T) {
var podStatus *kubecontainer.PodStatus
state := kubecontainer.ContainerStateRunning
if test.statusTerminated {
state = kubecontainer.ContainerStateExited
}
allocatedPod := v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
{
Name: "c1-init",
State: state,
Resources: test.statusResources,
},
},
}
match := allocatedResourcesMatchStatus(&allocatedPod, podStatus)
assert.Equal(t, test.expectMatch, match)
})
}
if isSidecarContainer {
allocatedPod.Spec = v1.PodSpec{
InitContainers: []v1.Container{{
Name: "c1-init",
Resources: test.allocatedResources,
RestartPolicy: &containerRestartPolicyAlways,
}},
}
podStatus = &kubecontainer.PodStatus{
Name: "test",
ContainerStatuses: []*kubecontainer.Status{
{
Name: "c1-init",
State: state,
Resources: test.statusResources,
},
},
}
} else {
allocatedPod.Spec = v1.PodSpec{
Containers: []v1.Container{{
Name: "c",
Resources: test.allocatedResources,
}},
}
podStatus = &kubecontainer.PodStatus{
Name: "test",
ContainerStatuses: []*kubecontainer.Status{
{
Name: "c",
State: state,
Resources: test.statusResources,
},
},
}
}
match := allocatedResourcesMatchStatus(&allocatedPod, podStatus)
assert.Equal(t, test.expectMatch, match)
})
}
}
}

21
staging/src/k8s.io/kubectl/pkg/util/resource/resource.go
View File

@ -45,16 +45,15 @@ func podRequests(pod *corev1.Pod) corev1.ResourceList {
for i := range pod.Status.ContainerStatuses {
containerStatuses[pod.Status.ContainerStatuses[i].Name] = &pod.Status.ContainerStatuses[i]
}
for i := range pod.Status.InitContainerStatuses {
containerStatuses[pod.Status.InitContainerStatuses[i].Name] = &pod.Status.InitContainerStatuses[i]
}
for _, container := range pod.Spec.Containers {
containerReqs := container.Resources.Requests
cs, found := containerStatuses[container.Name]
if found && cs.Resources != nil {
if pod.Status.Resize == corev1.PodResizeStatusInfeasible {
containerReqs = cs.Resources.Requests.DeepCopy()
} else {
containerReqs = max(container.Resources.Requests, cs.Resources.Requests)
}
containerReqs = determineContainerReqs(pod, &container, cs)
}
addResourceList(reqs, containerReqs)
}
@ -66,6 +65,10 @@ func podRequests(pod *corev1.Pod) corev1.ResourceList {
containerReqs := container.Resources.Requests
if container.RestartPolicy != nil && *container.RestartPolicy == corev1.ContainerRestartPolicyAlways {
cs, found := containerStatuses[container.Name]
if found && cs.Resources != nil {
containerReqs = determineContainerReqs(pod, &container, cs)
}
// and add them to the resulting cumulative container requests
addResourceList(reqs, containerReqs)
@ -137,6 +140,14 @@ func podLimits(pod *corev1.Pod) corev1.ResourceList {
return limits
}
// determineContainerReqs will return a copy of the container requests based on if resizing is feasible or not.
func determineContainerReqs(pod *corev1.Pod, container *corev1.Container, cs *corev1.ContainerStatus) corev1.ResourceList {
if pod.Status.Resize == corev1.PodResizeStatusInfeasible {
return cs.Resources.Requests.DeepCopy()
}
return max(container.Resources.Requests, cs.Resources.Requests)
}
// max returns the result of max(a, b) for each named resource and is only used if we can't
// accumulate into an existing resource list
func max(a corev1.ResourceList, b corev1.ResourceList) corev1.ResourceList {