This is an automated email from the ASF dual-hosted git repository.
mani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git
The following commit(s) were added to refs/heads/master by this push:
new b625a931 [YUNIKORN-3206] pod resource calculation incorrect with
native sidecars (#999)
b625a931 is described below
commit b625a93184ba33c1420db986a86b828497421d7f
Author: Wilfred Spiegelenburg <[email protected]>
AuthorDate: Wed Jan 28 10:53:09 2026 +0530
[YUNIKORN-3206] pod resource calculation incorrect with native sidecars
(#999)
Native sidecar containers are counted twice when calculation the maximum
used resources during the init phase.
If the pod has large init containers and small "real" containers the
overall pod size will be incorrect. In most cases the init containers
will be small and the issue will go undetected.
use updateMax instead of for loop
Closes: #999
Signed-off-by: Manikandan R <[email protected]>
---
pkg/common/resource.go | 34 +++-----
pkg/common/resource_test.go | 185 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 194 insertions(+), 25 deletions(-)
diff --git a/pkg/common/resource.go b/pkg/common/resource.go
index 7f06cfba..9328a957 100644
--- a/pkg/common/resource.go
+++ b/pkg/common/resource.go
@@ -162,37 +162,23 @@ func updateMax(left *si.Resource, right *si.Resource) {
}
func checkInitContainerRequest(pod *v1.Pod, containersResources *si.Resource,
containerStatuses map[string]*v1.ContainerStatus) *si.Resource {
- updatedRes := containersResources
-
- // update total pod resource usage with sidecar containers
- for _, c := range pod.Spec.InitContainers {
- if isSideCarContainer(&c) {
- sideCarResources := computeContainerResource(pod, &c,
containerStatuses)
- updatedRes = Add(updatedRes, sideCarResources)
- }
- }
-
+ initMax := NewResourceBuilder().Build()
var sideCarRequests *si.Resource // cumulative value of sidecar
requests so far
for _, c := range pod.Spec.InitContainers {
ICResource := computeContainerResource(pod, &c,
containerStatuses)
+ // Add this container's resources to the resources for native
sidecars that are already running
+ currentIC := Add(ICResource, sideCarRequests)
+ // if this is a native sidecar it keeps running: track it like
that for the next init container
if isSideCarContainer(&c) {
sideCarRequests = Add(sideCarRequests, ICResource)
}
- ICResource = Add(ICResource, sideCarRequests)
- for resourceName, ICRequest := range ICResource.Resources {
- containersRequests, exist :=
updatedRes.Resources[resourceName]
- // additional resource request from init cont, add it
to request.
- if !exist {
- updatedRes.Resources[resourceName] = ICRequest
- continue
- }
- if ICRequest.GetValue() > containersRequests.GetValue()
{
- updatedRes.Resources[resourceName] = ICRequest
- }
- }
+ updateMax(initMax, currentIC)
}
-
- return updatedRes
+ // Add all the native sidecar resources to the pod size
+ containersResources = Add(containersResources, sideCarRequests)
+ // if IC run is requesting more resources than combined sidecars and
other resources at any point adjust the pod size
+ updateMax(containersResources, initMax)
+ return containersResources
}
func isSideCarContainer(c *v1.Container) bool {
diff --git a/pkg/common/resource_test.go b/pkg/common/resource_test.go
index a34020cb..fd691ad4 100644
--- a/pkg/common/resource_test.go
+++ b/pkg/common/resource_test.go
@@ -217,7 +217,7 @@ func TestParsePodResource(t *testing.T) {
resK8s = siResourceFromList(k8res.PodRequests(pod,
k8res.PodResourcesOptions{ExcludeOverhead: false}))
assert.Assert(t, Equals(resK8s, res), "K8s pod resource request
calculation yielded different results")
- // test initcontainer and container resouce compare
+ // test init container and container resource compare
initContainers := make([]v1.Container, 0, 2)
initc1Resources := make(map[v1.ResourceName]resource.Quantity)
initc1Resources[v1.ResourceMemory] = resource.MustParse("4096M")
@@ -361,6 +361,189 @@ func TestParsePodResource(t *testing.T) {
assert.Assert(t, Equals(resK8s, res), "K8s pod resource request
calculation yielded different results")
}
+//nolint:funlen
+func TestInitContainerPodResources(t *testing.T) {
+ containers := make([]v1.Container, 0, 2)
+ initContainers := make([]v1.Container, 0, 4)
+ alwaysRestart := v1.ContainerRestartPolicyAlways
+
+ // pod
+ pod := &v1.Pod{
+ TypeMeta: apis.TypeMeta{
+ Kind: "Pod",
+ APIVersion: "v1",
+ },
+ ObjectMeta: apis.ObjectMeta{
+ Name: "pod-resource-test-00001",
+ UID: "UID-00001",
+ },
+ Spec: v1.PodSpec{
+ Containers: containers,
+ InitContainers: initContainers,
+ },
+ }
+
+ // restartable + non-restartable init containers {mem,CPU}
+ // IC1{1024M} sidecar
+ // IC2{256M}
+ // C1{512M}
+ // usage calculation:
+ // IC max is total(IC1, IC2): {1280M}
+ // Resource request for pod is max(C1 + IC1, IC max): {1536M}
+ initContainers = initContainers[:0]
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic1",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("1024M"),
+ },
+ },
+ RestartPolicy: &alwaysRestart,
+ })
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic2",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("256M"),
+ },
+ },
+ })
+ containers = containers[:0]
+ containers = append(containers, v1.Container{
+ Name: "container-main",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("512M"),
+ },
+ },
+ })
+ pod.Spec.InitContainers = initContainers
+ pod.Spec.Containers = containers
+ res := GetPodResource(pod)
+ assert.Equal(t, res.Resources[siCommon.Memory].GetValue(),
int64(1536000000))
+ resK8s := siResourceFromList(k8res.PodRequests(pod,
k8res.PodResourcesOptions{}))
+ assert.Assert(t, Equals(resK8s, res), "K8s pod resource request
calculation yielded different results")
+
+ // restartable + non-restartable init containers {mem,CPU}
+ // C1{512M, 1000m}
+ // IC1{1024M, 1000m}
+ // IC2{1024M, 1000m} sidecar
+ // usage calculation:
+ // When IC1 starts, necessary resource is Req1=IC1 {1024M, 1000m}
+ // When IC2 starts, IC1 is finished necessary resource is Req2=IC2
{1024m, 1000m}
+ // IC max is max(Req1, Req2): {1024M, 1000m}
+ // Resource request for pod is max(C1 + IC2, IC max): {1536M, 2000m}
+ initContainers = initContainers[:0]
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic1",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("1024M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ "nvidia.com/gpu": resource.MustParse("1"),
+ },
+ },
+ })
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic2",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("1024M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ },
+ },
+ RestartPolicy: &alwaysRestart,
+ })
+ containers = containers[:0]
+ containers = append(containers, v1.Container{
+ Name: "container-main",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("512M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ },
+ },
+ })
+ pod.Spec.InitContainers = initContainers
+ pod.Spec.Containers = containers
+ res = GetPodResource(pod)
+ assert.Equal(t, res.Resources[siCommon.Memory].GetValue(),
int64(1536000000))
+ assert.Equal(t, res.Resources[siCommon.CPU].GetValue(), int64(2000))
+ resK8s = siResourceFromList(k8res.PodRequests(pod,
k8res.PodResourcesOptions{}))
+ assert.Assert(t, Equals(resK8s, res), "K8s pod resource request
calculation yielded different results")
+
+ // restartable + non-restartable init containers {mem,CPU, GPU}
+ // C1{512M, 1000m}
+ // C1{512M, 1000m}
+ // IC1{1024M, 1000m} sidecar
+ // IC2{4096M, 1000m, 1GPU}
+ // IC1{512M, 100m} sidecar
+ // usage calculation:
+ // When IC1 starts, necessary resource is Req1=IC1 {1024M, 1000m}
+ // When IC2 starts, IC1 is sidecar resource is Req2=IC1+IC2 {5120M,
2000m, 1GPU}
+ // when IC3 starts, IC1 is sidecar resource is Req3 not changed
+ // IC max is max Req3: {5120m, 2000m, 1GPU}
+ // Resource request for pod is max(C1 + C2 + IC1 + IC3, IC max):
{5120M, 3100m, 1GPU}
+ initContainers = initContainers[:0]
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic1",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("1024M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ },
+ },
+ RestartPolicy: &alwaysRestart,
+ })
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic2",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("4096M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ "nvidia.com/gpu": resource.MustParse("1"),
+ },
+ },
+ })
+ initContainers = append(initContainers, v1.Container{
+ Name: "container-ic3",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("512"),
+ v1.ResourceCPU: resource.MustParse("100m"),
+ },
+ },
+ RestartPolicy: &alwaysRestart,
+ })
+ containers = containers[:0]
+ containers = append(containers, v1.Container{
+ Name: "container-main1",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("512M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ },
+ },
+ })
+ containers = append(containers, v1.Container{
+ Name: "container-main2",
+ Resources: v1.ResourceRequirements{
+ Requests: map[v1.ResourceName]resource.Quantity{
+ v1.ResourceMemory: resource.MustParse("512M"),
+ v1.ResourceCPU: resource.MustParse("1"),
+ },
+ },
+ })
+ pod.Spec.InitContainers = initContainers
+ pod.Spec.Containers = containers
+ res = GetPodResource(pod)
+ assert.Equal(t, res.Resources[siCommon.Memory].GetValue(),
int64(5120000000))
+ assert.Equal(t, res.Resources[siCommon.CPU].GetValue(), int64(3100))
+ assert.Equal(t, res.Resources["nvidia.com/gpu"].GetValue(), int64(1))
+ resK8s = siResourceFromList(k8res.PodRequests(pod,
k8res.PodResourcesOptions{}))
+ assert.Assert(t, Equals(resK8s, res), "K8s pod resource request
calculation yielded different results")
+}
+
func siResourceFromList(list v1.ResourceList) *si.Resource {
builder := NewResourceBuilder()
for name, val := range list {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
