-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Split out step resource request management
Before this change, all steps' resource requests (*not* limits) were zeroed out unless that value for that metric represented the maximum across all steps. If step 2 requested the maximum memory of all steps, but not the max CPU, its memory request would persist but its CPU request would be zeroed out. After this change, the max request for each resource type (CPU, memory, ephermeral storage) is calculated, then applied to the *last* step's container. The effect is the same, unless Pods' resource requests are updated to account for exited containers (afaik they're not), and this is simpler to express and explain. And since all the code for this is in its own method and file, it's easier to test in isolation.
- Loading branch information
Showing
4 changed files
with
138 additions
and
130 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package pod | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/resource" | ||
| ) | ||
|
|
||
| var zeroQty = resource.MustParse("0") | ||
|
|
||
| func allZeroQty() corev1.ResourceList { | ||
| return corev1.ResourceList{ | ||
| corev1.ResourceCPU: zeroQty, | ||
| corev1.ResourceMemory: zeroQty, | ||
| corev1.ResourceEphemeralStorage: zeroQty, | ||
| } | ||
| } | ||
|
|
||
| func resolveResourceRequests(containers []corev1.Container) []corev1.Container { | ||
| max := allZeroQty() | ||
| for _, c := range containers { | ||
| for k, v := range c.Resources.Requests { | ||
| if v.Cmp(max[k]) > 0 { | ||
| max[k] = v | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Set resource requests for all steps but the theh last container to | ||
| // zero. | ||
| for i := range containers[:len(containers)-1] { | ||
| containers[i].Resources.Requests = allZeroQty() | ||
| } | ||
| // Set the last container's request to the max of all resources. | ||
| containers[len(containers)-1].Resources.Requests = max | ||
| return containers | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| package pod | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/google/go-cmp/cmp" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/resource" | ||
| ) | ||
|
|
||
| var resourceQuantityCmp = cmp.Comparer(func(x, y resource.Quantity) bool { | ||
| return x.Cmp(y) == 0 | ||
| }) | ||
|
|
||
| func TestResolveResourceRequests(t *testing.T) { | ||
| for _, c := range []struct { | ||
| desc string | ||
| in, want []corev1.Container | ||
| }{{ | ||
| desc: "three steps, no requests", | ||
| in: []corev1.Container{{}, {}, {}}, | ||
| want: []corev1.Container{{ | ||
| Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, | ||
| }, { | ||
| Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, | ||
| }, { | ||
| Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, | ||
| }}, | ||
| }, { | ||
| desc: "requests are moved, limits aren't changed", | ||
| in: []corev1.Container{{ | ||
| Resources: corev1.ResourceRequirements{ | ||
| Requests: corev1.ResourceList{ | ||
| corev1.ResourceCPU: resource.MustParse("10"), | ||
| }, | ||
| }, | ||
| }, { | ||
| Resources: corev1.ResourceRequirements{ | ||
| Requests: corev1.ResourceList{ | ||
| corev1.ResourceMemory: resource.MustParse("10Gi"), | ||
| }, | ||
| Limits: corev1.ResourceList{ | ||
| corev1.ResourceMemory: resource.MustParse("11Gi"), | ||
| }, | ||
| }, | ||
| }, { | ||
| Resources: corev1.ResourceRequirements{ | ||
| Requests: corev1.ResourceList{ | ||
| corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), | ||
| }, | ||
| Limits: corev1.ResourceList{ | ||
| corev1.ResourceMemory: resource.MustParse("100Gi"), | ||
| }, | ||
| }, | ||
| }}, | ||
| want: []corev1.Container{{ | ||
| // All zeroed out. | ||
| Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, | ||
| }, { | ||
| // Requests zeroed out, limits remain. | ||
| Resources: corev1.ResourceRequirements{ | ||
| Requests: allZeroQty(), | ||
| Limits: corev1.ResourceList{ | ||
| corev1.ResourceMemory: resource.MustParse("11Gi"), | ||
| }, | ||
| }, | ||
| }, { | ||
| // Requests to the max, limits remain. | ||
| Resources: corev1.ResourceRequirements{ | ||
| Requests: corev1.ResourceList{ | ||
| corev1.ResourceCPU: resource.MustParse("10"), | ||
| corev1.ResourceMemory: resource.MustParse("10Gi"), | ||
| corev1.ResourceEphemeralStorage: resource.MustParse("100Gi"), | ||
| }, | ||
| Limits: corev1.ResourceList{ | ||
| corev1.ResourceMemory: resource.MustParse("100Gi"), | ||
| }, | ||
| }, | ||
| }}, | ||
| }} { | ||
| t.Run(c.desc, func(t *testing.T) { | ||
| got := resolveResourceRequests(c.in) | ||
| if d := cmp.Diff(c.want, got, resourceQuantityCmp); d != "" { | ||
| t.Errorf("Diff(-want, +got): %s", d) | ||
| } | ||
| }) | ||
| } | ||
| } |