diff --git a/pkg/util/limitrange/limitrange.go b/pkg/util/limitrange/limitrange.go index 943e0a2ccf..98fbde5faf 100644 --- a/pkg/util/limitrange/limitrange.go +++ b/pkg/util/limitrange/limitrange.go @@ -81,25 +81,62 @@ func (s Summary) validatePodSpecContainers(containers []corev1.Container, path * } // TotalRequests computes the total resource requests of a pod. -// total = sum(max(sum(.containers[].requests), initContainers[].requests), overhead) +// +// total = Max( Max(each InitContainerUse) , Sum(SidecarContainers) + Sum(Containers) ) + pod overhead +// +// where: +// +// InitContainerUse(i) = Sum(SidecarContainers with index < i) + InitContainer(i) func TotalRequests(ps *corev1.PodSpec) corev1.ResourceList { - total := corev1.ResourceList{} + sumContainers := calculateMainContainersResources(ps.Containers) + maxInitContainers := calculateInitContainersResources(ps.InitContainers) + sumSidecarContainers := calculateSidecarContainersResources(ps.InitContainers) + + total := resource.MergeResourceListKeepMax( + maxInitContainers, + resource.MergeResourceListKeepSum(sumSidecarContainers, sumContainers), + ) + // add the overhead + total = resource.MergeResourceListKeepSum(total, ps.Overhead) + return total +} - // add the resource from the main containers - for i := range ps.Containers { - total = resource.MergeResourceListKeepSum(total, ps.Containers[i].Resources.Requests) +func calculateMainContainersResources(containers []corev1.Container) corev1.ResourceList { + total := corev1.ResourceList{} + for i := range containers { + total = resource.MergeResourceListKeepSum(total, containers[i].Resources.Requests) } + return total +} - // take into account the maximum of any init containers - for i := range ps.InitContainers { - total = resource.MergeResourceListKeepMax(total, ps.InitContainers[i].Resources.Requests) +func calculateInitContainersResources(initContainers []corev1.Container) corev1.ResourceList { + maxInitContainerUsage := corev1.ResourceList{} + sidecarRunningUsage := corev1.ResourceList{} + for i := range initContainers { + if isSidecarContainer(initContainers[i]) { + sidecarRunningUsage = resource.MergeResourceListKeepSum(sidecarRunningUsage, initContainers[i].Resources.Requests) + } else { + initContainerUse := resource.MergeResourceListKeepSum(initContainers[i].Resources.Requests, sidecarRunningUsage) + maxInitContainerUsage = resource.MergeResourceListKeepMax(maxInitContainerUsage, initContainerUse) + } } + return maxInitContainerUsage +} - // add the overhead - total = resource.MergeResourceListKeepSum(total, ps.Overhead) +func calculateSidecarContainersResources(initContainers []corev1.Container) corev1.ResourceList { + total := corev1.ResourceList{} + for i := range initContainers { + if isSidecarContainer(initContainers[i]) { + total = resource.MergeResourceListKeepSum(total, initContainers[i].Resources.Requests) + } + } return total } +func isSidecarContainer(container corev1.Container) bool { + return container.RestartPolicy != nil && *container.RestartPolicy == corev1.ContainerRestartPolicyAlways +} + // ValidatePodSpec verifies if the provided podSpec (ps) first into the boundaries of the summary (s). func (s Summary) ValidatePodSpec(ps *corev1.PodSpec, path *field.Path) []string { reasons := []string{} diff --git a/pkg/util/limitrange/limitrange_test.go b/pkg/util/limitrange/limitrange_test.go index eed8c66460..536f5b0f68 100644 --- a/pkg/util/limitrange/limitrange_test.go +++ b/pkg/util/limitrange/limitrange_test.go @@ -160,69 +160,184 @@ func TestSummarize(t *testing.T) { } func TestTotalRequest(t *testing.T) { - containers := []corev1.Container{ - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - corev1.ResourceMemory: resource.MustParse("2Gi"), + cases := map[string]struct { + podSpec *corev1.PodSpec + want corev1.ResourceList + }{ + "pod without init containers. request sum(containers)": { + podSpec: &corev1.PodSpec{ + Containers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), }, }, - }, - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1500m"), - "example.com/gpu": resource.MustParse("2"), - }, + want: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("2.5"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + "example.com/gpu": resource.MustParse("2"), }, }, - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("4"), - corev1.ResourceMemory: resource.MustParse("2Gi"), + "pod only with regular init containers. request max( max(each initContainerUse), sum(containers) )": { + podSpec: &corev1.PodSpec{ + InitContainers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), }, + Containers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), + }, + }, + want: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("4"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + "example.com/gpu": resource.MustParse("2"), }, }, - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1500m"), - "example.com/gpu": resource.MustParse("2"), + "pod only with sidecar containers. request max( max(each initContainerUse), sum(sidecarContainers) + sum(containers) )": { + podSpec: &corev1.PodSpec{ + InitContainers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + AsSidecar(). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + AsSidecar(). + Obj(), }, + Containers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), + }, + }, + want: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("8"), + corev1.ResourceMemory: resource.MustParse("4Gi"), + "example.com/gpu": resource.MustParse("4"), }, }, - } - cases := map[string]struct { - podSpec *corev1.PodSpec - want corev1.ResourceList - }{ - "sum up main containers": { + "pod only with regular init and sidecar containers. request max( max(each InitContainerUse), sum(sidecarContainers) )": { podSpec: &corev1.PodSpec{ - Containers: containers[:2], + InitContainers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "1Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "2"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "3"). + WithResourceReq(corev1.ResourceMemory, "3Gi"). + AsSidecar(). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq(corev1.ResourceMemory, "4Gi"). + AsSidecar(). + Obj(), + }, }, want: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("2500m"), - corev1.ResourceMemory: resource.MustParse("2Gi"), - "example.com/gpu": resource.MustParse("2"), + corev1.ResourceCPU: resource.MustParse("7"), + corev1.ResourceMemory: resource.MustParse("7Gi"), }, }, - "one init wants more": { + "pod with regular init and sidecar containers. request max( max(each InitContainer), sum(sidecarContainers) + sum(containers) )": { podSpec: &corev1.PodSpec{ - InitContainers: containers[2:], - Containers: containers[:2], + InitContainers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "1Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "2"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "3"). + WithResourceReq(corev1.ResourceMemory, "3Gi"). + AsSidecar(). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq(corev1.ResourceMemory, "4Gi"). + AsSidecar(). + Obj(), + }, + Containers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "2"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq("example.com/gpu", "4"). + Obj(), + }, }, want: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("4000m"), - corev1.ResourceMemory: resource.MustParse("2Gi"), - "example.com/gpu": resource.MustParse("2"), + corev1.ResourceCPU: resource.MustParse("13"), + corev1.ResourceMemory: resource.MustParse("9Gi"), + "example.com/gpu": resource.MustParse("4"), }, }, - "adds overhead": { + "adds overhead. request max( max(each InitContainer), sum(sidecarContainers) + sum(containers) ) + overhead": { podSpec: &corev1.PodSpec{ - InitContainers: containers[2:], - Containers: containers[:2], + InitContainers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + WithResourceReq("example.com/gpu", "2"). + AsSidecar(). + Obj(), + }, + Containers: []corev1.Container{ + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), + }, Overhead: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("1"), corev1.ResourceMemory: resource.MustParse("1Gi"), @@ -230,9 +345,9 @@ func TestTotalRequest(t *testing.T) { }, }, want: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("5000m"), - corev1.ResourceMemory: resource.MustParse("3Gi"), - "example.com/gpu": resource.MustParse("3"), + corev1.ResourceCPU: resource.MustParse("5"), + corev1.ResourceMemory: resource.MustParse("5Gi"), + "example.com/gpu": resource.MustParse("5"), }, }, } @@ -249,42 +364,26 @@ func TestTotalRequest(t *testing.T) { func TestValidatePodSpec(t *testing.T) { podSpec := &corev1.PodSpec{ Containers: []corev1.Container{ - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1"), - corev1.ResourceMemory: resource.MustParse("2Gi"), - "example.com/mainContainerGpu": resource.MustParse("2"), - }, - }, - }, - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1500m"), - "example.com/gpu": resource.MustParse("2"), - }, - }, - }, + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + WithResourceReq("example.com/mainContainerGpu", "2"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + Obj(), }, InitContainers: []corev1.Container{ - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("4"), - corev1.ResourceMemory: resource.MustParse("2Gi"), - }, - }, - }, - { - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("1500m"), - "example.com/gpu": resource.MustParse("2"), - "example.com/initContainerGpu": resource.MustParse("2"), - }, - }, - }, + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "4"). + WithResourceReq(corev1.ResourceMemory, "2Gi"). + Obj(), + *testingutil.MakeContainer(). + WithResourceReq(corev1.ResourceCPU, "1.5"). + WithResourceReq("example.com/gpu", "2"). + WithResourceReq("example.com/initContainerGpu", "2"). + Obj(), }, Overhead: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("1"), diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index 6efa611e7e..3401049d93 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -32,6 +32,7 @@ import ( kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + utilResource "sigs.k8s.io/kueue/pkg/util/resource" ) // PriorityClassWrapper wraps a PriorityClass. @@ -1029,3 +1030,39 @@ func (mkc *MultiKueueClusterWrapper) Generation(num int64) *MultiKueueClusterWra mkc.ObjectMeta.Generation = num return mkc } + +// ContainerWrapper wraps a corev1.Container. +type ContainerWrapper struct{ corev1.Container } + +// MakeContainer wraps a ContainerWrapper with an empty ResourceList. +func MakeContainer() *ContainerWrapper { + return &ContainerWrapper{ + corev1.Container{ + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{}, + }, + }, + } +} + +// Obj returns the inner corev1.Container. +func (c *ContainerWrapper) Obj() *corev1.Container { + return &c.Container +} + +// WithResourceReq appends a resource request to the container. +func (c *ContainerWrapper) WithResourceReq(resourceName corev1.ResourceName, quantity string) *ContainerWrapper { + requests := utilResource.MergeResourceListKeepFirst(c.Container.Resources.Requests, corev1.ResourceList{ + resourceName: resource.MustParse(quantity), + }) + c.Container.Resources.Requests = requests + + return c +} + +// AsSidecar makes the container a sidecar when used as an Init Container. +func (c *ContainerWrapper) AsSidecar() *ContainerWrapper { + c.Container.RestartPolicy = ptr.To(corev1.ContainerRestartPolicyAlways) + + return c +}