Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit init containers from limitrange default request calculations #4769

Merged
merged 1 commit into from
May 3, 2022
Merged

Omit init containers from limitrange default request calculations #4769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions pkg/internal/limitrange/transformer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,17 @@ func NewTransformer(ctx context.Context, namespace string, lister corev1listers.
// computed if there is multiple LimitRange to satisfy the most (if we can).
// Count the number of containers (that we know) in the Pod.
// This should help us find the smallest request to apply to containers
// We are adding +1 to the number of container to take into account the init containers.
// The reason to use +1 only is that, k8s treats request on all init container as one (getting the max of all)
nbContainers := len(p.Spec.Containers) + 1
// FIXME(vdemeester) maxLimitRequestRatio to support later
defaultRequests := getDefaultRequest(limitRange, nbContainers)
nbContainers := len(p.Spec.Containers)
// FIXME(#4230) maxLimitRequestRatio to support later
defaultLimits := getDefaultLimits(limitRange)

defaultInitRequests := getDefaultInitContainerRequest(limitRange)
for i := range p.Spec.InitContainers {
// We are trying to set the smallest requests possible
if p.Spec.InitContainers[i].Resources.Requests == nil {
p.Spec.InitContainers[i].Resources.Requests = defaultRequests
p.Spec.InitContainers[i].Resources.Requests = defaultInitRequests
} else {
for _, name := range resourceNames {
setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Requests, defaultRequests)
setRequestsOrLimits(name, p.Spec.InitContainers[i].Resources.Requests, defaultInitRequests)
}
}
// We are trying to set the highest limits possible
Expand All @@ -73,6 +70,7 @@ func NewTransformer(ctx context.Context, namespace string, lister corev1listers.
}
}

defaultRequests := getDefaultAppContainerRequest(limitRange, nbContainers)
for i := range p.Spec.Containers {
if p.Spec.Containers[i].Resources.Requests == nil {
p.Spec.Containers[i].Resources.Requests = defaultRequests
Expand All @@ -99,7 +97,9 @@ func setRequestsOrLimits(name corev1.ResourceName, dst, src corev1.ResourceList)
}
}

func getDefaultRequest(limitRange *corev1.LimitRange, nbContainers int) corev1.ResourceList {
// Returns the default requests to use for each app container, determined by dividing the LimitRange default requests
// among the app containers, and applying the LimitRange minimum if necessary
func getDefaultAppContainerRequest(limitRange *corev1.LimitRange, nbContainers int) corev1.ResourceList {
// Support only Type Container to start with
var r corev1.ResourceList = map[corev1.ResourceName]resource.Quantity{}
for _, item := range limitRange.Spec.Limits {
Expand All @@ -126,6 +126,23 @@ func getDefaultRequest(limitRange *corev1.LimitRange, nbContainers int) corev1.R
return r
}

// Returns the default requests to use for each init container, determined by the LimitRange default requests and minimums
func getDefaultInitContainerRequest(limitRange *corev1.LimitRange) corev1.ResourceList {
// Support only Type Container to start with
var r corev1.ResourceList
for _, item := range limitRange.Spec.Limits {
// Only support LimitTypeContainer
if item.Type == corev1.LimitTypeContainer {
if item.DefaultRequest != nil {
r = item.DefaultRequest
} else if item.Min != nil {
r = item.Min
}
}
}
return r
}

func takeTheMax(requestQ, defaultQ, maxQ resource.Quantity) resource.Quantity {
var q resource.Quantity = requestQ
if defaultQ.Cmp(q) > 0 {
Expand Down
108 changes: 47 additions & 61 deletions pkg/internal/limitrange/transformer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ func TestTransformerOneContainer(t *testing.T) {
want: corev1.PodSpec{
InitContainers: []corev1.Container{{
// No resources set
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{},
},
Resources: corev1.ResourceRequirements{},
}},
Containers: []corev1.Container{{
Resources: corev1.ResourceRequirements{
Expand Down Expand Up @@ -105,11 +103,6 @@ func TestTransformerOneContainer(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.Quantity{},
corev1.ResourceMemory: resource.Quantity{},
corev1.ResourceEphemeralStorage: resource.Quantity{},
},
},
}},
Containers: []corev1.Container{{
Expand Down Expand Up @@ -151,18 +144,18 @@ func TestTransformerOneContainer(t *testing.T) {
InitContainers: []corev1.Container{{
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Containers: []corev1.Container{{
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand Down Expand Up @@ -201,9 +194,9 @@ func TestTransformerOneContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand All @@ -215,9 +208,9 @@ func TestTransformerOneContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand Down Expand Up @@ -385,9 +378,9 @@ func TestTransformerOneContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("700m"),
corev1.ResourceMemory: resource.MustParse("70Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("700Mi"),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand All @@ -399,9 +392,9 @@ func TestTransformerOneContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("700m"),
corev1.ResourceMemory: resource.MustParse("70Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("700Mi"),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand Down Expand Up @@ -461,9 +454,7 @@ func TestTransformerMultipleContainer(t *testing.T) {
want: corev1.PodSpec{
InitContainers: []corev1.Container{{
// No resources set
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{},
},
Resources: corev1.ResourceRequirements{},
}},
Containers: []corev1.Container{{
Resources: corev1.ResourceRequirements{
Expand Down Expand Up @@ -510,11 +501,6 @@ func TestTransformerMultipleContainer(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.Quantity{},
corev1.ResourceMemory: resource.Quantity{},
corev1.ResourceEphemeralStorage: resource.Quantity{},
},
},
}},
Containers: []corev1.Container{{
Expand Down Expand Up @@ -572,26 +558,26 @@ func TestTransformerMultipleContainer(t *testing.T) {
InitContainers: []corev1.Container{{
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("333m"),
corev1.ResourceMemory: *resource.NewQuantity(34952533, resource.BinarySI),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(357913941, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("1000m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Containers: []corev1.Container{{
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("333m"),
corev1.ResourceMemory: *resource.NewQuantity(34952533, resource.BinarySI),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(357913941, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
},
},
}, {
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("333m"),
corev1.ResourceMemory: *resource.NewQuantity(34952533, resource.BinarySI),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(357913941, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
},
},
}},
Expand Down Expand Up @@ -633,9 +619,9 @@ func TestTransformerMultipleContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("333m"),
corev1.ResourceMemory: *resource.NewQuantity(34952533, resource.BinarySI),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(357913941, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand All @@ -647,9 +633,9 @@ func TestTransformerMultipleContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("333m"),
corev1.ResourceMemory: *resource.NewQuantity(34952533, resource.BinarySI),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(357913941, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
},
},
}, {
Expand All @@ -660,9 +646,9 @@ func TestTransformerMultipleContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("333m"),
corev1.ResourceMemory: *resource.NewQuantity(34952533, resource.BinarySI),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(357913941, resource.BinarySI),
corev1.ResourceCPU: resource.MustParse("500m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
corev1.ResourceEphemeralStorage: *resource.NewQuantity(536870912, resource.BinarySI),
},
},
}},
Expand Down Expand Up @@ -785,9 +771,9 @@ func TestTransformerMultipleContainer(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("700m"),
corev1.ResourceMemory: resource.MustParse("70Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("700Mi"),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand Down Expand Up @@ -925,9 +911,9 @@ func TestTransformerOneContainerMultipleLimitRange(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("800m"),
corev1.ResourceMemory: resource.MustParse("80Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("800Mi"),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand All @@ -939,9 +925,9 @@ func TestTransformerOneContainerMultipleLimitRange(t *testing.T) {
corev1.ResourceEphemeralStorage: resource.MustParse("2Gi"),
},
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("800m"),
corev1.ResourceMemory: resource.MustParse("80Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("800Mi"),
corev1.ResourceCPU: resource.MustParse("1"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
corev1.ResourceEphemeralStorage: resource.MustParse("1Gi"),
},
},
}},
Expand Down