Address comments

This commit is contained in:
Luiz Antonio 2025-04-28 14:24:28 -04:00
parent 986d7c875a
commit 3eba409c62
3 changed files with 236 additions and 86 deletions

View File

@ -80,8 +80,10 @@ func (client *specClient) GetPodSpecs() ([]*BasicPodSpec, error) {
}
return podSpecs, nil
}
func newBasicPodSpec(pod *v1.Pod) *BasicPodSpec {
containerSpecs, initContainerSpecs := newContainerSpecs(pod)
containerSpecs := newContainerSpecs(pod, pod.Spec.Containers, false /* isInitContainer */)
initContainerSpecs := newContainerSpecs(pod, pod.Spec.InitContainers, true /* isInitContainer */)
basicPodSpec := &BasicPodSpec{
ID: podID(pod),
@ -93,34 +95,33 @@ func newBasicPodSpec(pod *v1.Pod) *BasicPodSpec {
return basicPodSpec
}
func newContainerSpecs(pod *v1.Pod) (containerSpecs []BasicContainerSpec, initContainerSpecs []BasicContainerSpec) {
for _, container := range pod.Spec.Containers {
containerSpec := newContainerSpec(pod, container)
func newContainerSpecs(pod *v1.Pod, containers []v1.Container, isInitContainer bool) []BasicContainerSpec {
var containerSpecs []BasicContainerSpec
for _, container := range containers {
containerSpec := newContainerSpec(pod, container, isInitContainer)
containerSpecs = append(containerSpecs, containerSpec)
}
for _, initContainer := range pod.Spec.InitContainers {
initContainerSpec := newContainerSpec(pod, initContainer)
initContainerSpecs = append(initContainerSpecs, initContainerSpec)
}
return containerSpecs, initContainerSpecs
return containerSpecs
}
func newContainerSpec(pod *v1.Pod, container v1.Container) BasicContainerSpec {
func newContainerSpec(pod *v1.Pod, container v1.Container, isInitContainer bool) BasicContainerSpec {
containerSpec := BasicContainerSpec{
ID: model.ContainerID{
PodID: podID(pod),
ContainerName: container.Name,
},
Image: container.Image,
Request: calculateRequestedResources(pod, container),
Request: calculateRequestedResources(pod, container, isInitContainer),
}
return containerSpec
}
func calculateRequestedResources(pod *v1.Pod, container v1.Container) model.Resources {
requests, _ := resourcehelpers.ContainerRequestsAndLimits(container.Name, pod)
func calculateRequestedResources(pod *v1.Pod, container v1.Container, isInitContainer bool) model.Resources {
requestsAndLimitsFn := resourcehelpers.ContainerRequestsAndLimits
if isInitContainer {
requestsAndLimitsFn = resourcehelpers.InitContainerRequestsAndLimits
}
requests, _ := requestsAndLimitsFn(container.Name, pod)
cpuQuantity := requests[v1.ResourceCPU]
cpuMillicores := cpuQuantity.MilliValue()

View File

@ -22,15 +22,15 @@ import (
)
// ContainerRequestsAndLimits returns a copy of the actual resource requests and
// limits of a given container or initContainer:
// limits of a given container:
//
// - If in-place pod updates feature [1] is enabled, the actual resource requests
// are stored in the container/initContainer status field.
// are stored in the container status field.
// - Otherwise, fallback to the resource requests defined in the pod spec.
//
// [1] https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1287-in-place-update-pod-resources
func ContainerRequestsAndLimits(containerName string, pod *v1.Pod) (v1.ResourceList, v1.ResourceList) {
cs := containerStatusForContainer(containerName, pod)
cs := containerStatusFor(containerName, pod)
if cs != nil && cs.Resources != nil {
return cs.Resources.Requests.DeepCopy(), cs.Resources.Limits.DeepCopy()
}
@ -44,30 +44,59 @@ func ContainerRequestsAndLimits(containerName string, pod *v1.Pod) (v1.ResourceL
return nil, nil
}
// InitContainerRequestsAndLimits returns a copy of the actual resource requests
// and limits of a given initContainer:
//
// - If in-place pod updates feature [1] is enabled, the actual resource requests
// are stored in the initContainer status field.
// - Otherwise, fallback to the resource requests defined in the pod spec.
//
// [1] https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1287-in-place-update-pod-resources
func InitContainerRequestsAndLimits(initContainerName string, pod *v1.Pod) (v1.ResourceList, v1.ResourceList) {
cs := initContainerStatusFor(initContainerName, pod)
if cs != nil && cs.Resources != nil {
return cs.Resources.Requests.DeepCopy(), cs.Resources.Limits.DeepCopy()
}
klog.V(6).InfoS("initContainer resources not found in initContainerStatus for initContainer. Falling back to resources defined in the pod spec. This is expected for clusters with in-place pod updates feature disabled.", "initContainer", initContainerName, "initContainerStatus", cs)
initContainer := findInitContainer(initContainerName, pod)
if initContainer != nil {
return initContainer.Resources.Requests.DeepCopy(), initContainer.Resources.Limits.DeepCopy()
}
return nil, nil
}
func findContainer(containerName string, pod *v1.Pod) *v1.Container {
for i, container := range pod.Spec.Containers {
if container.Name == containerName {
return &pod.Spec.Containers[i]
}
}
return nil
}
func findInitContainer(initContainerName string, pod *v1.Pod) *v1.Container {
for i, initContainer := range pod.Spec.InitContainers {
if initContainer.Name == containerName {
if initContainer.Name == initContainerName {
return &pod.Spec.InitContainers[i]
}
}
return nil
}
func containerStatusForContainer(containerName string, pod *v1.Pod) *v1.ContainerStatus {
func containerStatusFor(containerName string, pod *v1.Pod) *v1.ContainerStatus {
for i, containerStatus := range pod.Status.ContainerStatuses {
if containerStatus.Name == containerName {
return &pod.Status.ContainerStatuses[i]
}
}
return nil
}
func initContainerStatusFor(initContainerName string, pod *v1.Pod) *v1.ContainerStatus {
for i, initContainerStatus := range pod.Status.InitContainerStatuses {
if initContainerStatus.Name == containerName {
if initContainerStatus.Name == initContainerName {
return &pod.Status.InitContainerStatuses[i]
}
}

View File

@ -58,31 +58,6 @@ func TestContainerRequestsAndLimits(t *testing.T) {
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
},
{
desc: "Prefer resource requests from initContainer status",
containerName: "init-container",
pod: test.Pod().AddInitContainer(
test.Container().WithName("init-container").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("1Mi")).
WithCPULimit(resource.MustParse("1")).
WithMemLimit(resource.MustParse("1Mi")).Get()).
AddInitContainerStatus(
test.ContainerStatus().WithName("init-container").
WithCPURequest(resource.MustParse("5")).
WithMemRequest(resource.MustParse("50Mi")).
WithCPULimit(resource.MustParse("6")).
WithMemLimit(resource.MustParse("60Mi")).Get()).
Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("5"),
apiv1.ResourceMemory: resource.MustParse("50Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("6"),
apiv1.ResourceMemory: resource.MustParse("60Mi"),
},
},
{
desc: "No container status, get resources from pod spec",
containerName: "container",
@ -101,24 +76,6 @@ func TestContainerRequestsAndLimits(t *testing.T) {
apiv1.ResourceMemory: resource.MustParse("20Mi"),
},
},
{
desc: "No initContainer status, get resources from pod spec",
containerName: "init-container",
pod: test.Pod().AddInitContainer(
test.Container().WithName("init-container").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("1"),
apiv1.ResourceMemory: resource.MustParse("10Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("2"),
apiv1.ResourceMemory: resource.MustParse("20Mi"),
},
},
{
desc: "Only containerStatus, get resources from containerStatus",
containerName: "container",
@ -137,24 +94,6 @@ func TestContainerRequestsAndLimits(t *testing.T) {
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
},
{
desc: "Only initContainerStatus, get resources from initContainerStatus",
containerName: "init-container",
pod: test.Pod().AddInitContainerStatus(
test.ContainerStatus().WithName("init-container").
WithCPURequest(resource.MustParse("0")).
WithMemRequest(resource.MustParse("30Mi")).
WithCPULimit(resource.MustParse("4")).
WithMemLimit(resource.MustParse("40Mi")).Get()).Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("0"),
apiv1.ResourceMemory: resource.MustParse("30Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("4"),
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
},
{
desc: "Inexistent container",
containerName: "inexistent-container",
@ -163,11 +102,35 @@ func TestContainerRequestsAndLimits(t *testing.T) {
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).
AddInitContainer(test.Container().WithName("init-container").Get()).Get(),
WithMemLimit(resource.MustParse("20Mi")).Get()).Get(),
wantRequests: nil,
wantLimits: nil,
},
{
desc: "Init container with the same name as the container is ignored",
containerName: "container-1",
pod: test.Pod().AddInitContainer(
test.Container().WithName("container-1").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).
AddContainer(
test.Container().WithName("container-1").
WithCPURequest(resource.MustParse("4")).
WithMemRequest(resource.MustParse("40Mi")).
WithCPULimit(resource.MustParse("5")).
WithMemLimit(resource.MustParse("50Mi")).Get()).
Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("4"),
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("5"),
apiv1.ResourceMemory: resource.MustParse("50Mi"),
},
},
{
desc: "Container with no requests or limits returns non-nil resources",
containerName: "container",
@ -202,8 +165,6 @@ func TestContainerRequestsAndLimits(t *testing.T) {
WithMemRequest(resource.MustParse("5Mi")).
WithCPULimit(resource.MustParse("5")).
WithMemLimit(resource.MustParse("5Mi")).Get()).
AddInitContainer(
test.Container().WithName("init-container").Get()).
Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("3"),
@ -223,3 +184,162 @@ func TestContainerRequestsAndLimits(t *testing.T) {
})
}
}
func TestInitContainerRequestsAndLimits(t *testing.T) {
testCases := []struct {
desc string
initContainerName string
pod *apiv1.Pod
wantRequests apiv1.ResourceList
wantLimits apiv1.ResourceList
}{
{
desc: "Prefer resource requests from initContainer status",
initContainerName: "init-container",
pod: test.Pod().AddInitContainer(
test.Container().WithName("init-container").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).
AddInitContainerStatus(
test.ContainerStatus().WithName("init-container").
WithCPURequest(resource.MustParse("3")).
WithMemRequest(resource.MustParse("30Mi")).
WithCPULimit(resource.MustParse("4")).
WithMemLimit(resource.MustParse("40Mi")).Get()).Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("3"),
apiv1.ResourceMemory: resource.MustParse("30Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("4"),
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
},
{
desc: "No initContainer status, get resources from pod spec",
initContainerName: "init-container",
pod: test.Pod().AddInitContainer(
test.Container().WithName("init-container").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("1"),
apiv1.ResourceMemory: resource.MustParse("10Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("2"),
apiv1.ResourceMemory: resource.MustParse("20Mi"),
},
},
{
desc: "Only initContainerStatus, get resources from initContainerStatus",
initContainerName: "init-container",
pod: test.Pod().AddInitContainerStatus(
test.ContainerStatus().WithName("init-container").
WithCPURequest(resource.MustParse("0")).
WithMemRequest(resource.MustParse("30Mi")).
WithCPULimit(resource.MustParse("4")).
WithMemLimit(resource.MustParse("40Mi")).Get()).Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("0"),
apiv1.ResourceMemory: resource.MustParse("30Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("4"),
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
},
{
desc: "Inexistent initContainer",
initContainerName: "inexistent-init-container",
pod: test.Pod().AddInitContainer(
test.Container().WithName("init-container").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).Get(),
wantRequests: nil,
wantLimits: nil,
},
{
desc: "Container with the same name as the initContainer is ignored",
initContainerName: "container-1",
pod: test.Pod().AddInitContainer(
test.Container().WithName("container-1").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).
AddContainer(
test.Container().WithName("container-1").
WithCPURequest(resource.MustParse("4")).
WithMemRequest(resource.MustParse("40Mi")).
WithCPULimit(resource.MustParse("5")).
WithMemLimit(resource.MustParse("50Mi")).Get()).
Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("1"),
apiv1.ResourceMemory: resource.MustParse("10Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("2"),
apiv1.ResourceMemory: resource.MustParse("20Mi"),
},
},
{
desc: "InitContainer with no requests or limits returns non-nil resources",
initContainerName: "init-container",
pod: test.Pod().AddInitContainer(test.Container().WithName("init-container").Get()).Get(),
wantRequests: apiv1.ResourceList{},
wantLimits: apiv1.ResourceList{},
},
{
desc: "2 init containers",
initContainerName: "init-container-1",
pod: test.Pod().AddInitContainer(
test.Container().WithName("init-container-1").
WithCPURequest(resource.MustParse("1")).
WithMemRequest(resource.MustParse("10Mi")).
WithCPULimit(resource.MustParse("2")).
WithMemLimit(resource.MustParse("20Mi")).Get()).
AddInitContainerStatus(
test.ContainerStatus().WithName("init-container-1").
WithCPURequest(resource.MustParse("3")).
WithMemRequest(resource.MustParse("30Mi")).
WithCPULimit(resource.MustParse("4")).
WithMemLimit(resource.MustParse("40Mi")).Get()).
AddInitContainer(
test.Container().WithName("init-container-2").
WithCPURequest(resource.MustParse("5")).
WithMemRequest(resource.MustParse("5Mi")).
WithCPULimit(resource.MustParse("5")).
WithMemLimit(resource.MustParse("5Mi")).Get()).
AddInitContainerStatus(
test.ContainerStatus().WithName("init-container-2").
WithCPURequest(resource.MustParse("5")).
WithMemRequest(resource.MustParse("5Mi")).
WithCPULimit(resource.MustParse("5")).
WithMemLimit(resource.MustParse("5Mi")).Get()).
Get(),
wantRequests: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("3"),
apiv1.ResourceMemory: resource.MustParse("30Mi"),
},
wantLimits: apiv1.ResourceList{
apiv1.ResourceCPU: resource.MustParse("4"),
apiv1.ResourceMemory: resource.MustParse("40Mi"),
},
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
gotRequests, gotLimits := InitContainerRequestsAndLimits(tc.initContainerName, tc.pod)
assert.Equal(t, tc.wantRequests, gotRequests, "requests don't match")
assert.Equal(t, tc.wantLimits, gotLimits, "limits don't match")
})
}
}