diff --git a/pkg/apis/pipeline/v1/merge.go b/pkg/apis/pipeline/v1/merge.go index 9585668c89b..b500ef87587 100644 --- a/pkg/apis/pipeline/v1/merge.go +++ b/pkg/apis/pipeline/v1/merge.go @@ -20,6 +20,7 @@ import ( "encoding/json" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/strategicpatch" ) @@ -67,6 +68,65 @@ func MergeStepsWithStepTemplate(template *StepTemplate, steps []Step) ([]Step, e return steps, nil } +// MergeStepsWithSpecs takes a possibly nil list of overrides and a +// list of steps, merging each of the steps with the overrides' resource requirements, if +// it's not nil, and returning the resulting list. +func MergeStepsWithSpecs(steps []Step, overrides []TaskRunStepSpec) ([]Step, error) { + stepNameToOverride := make(map[string]TaskRunStepSpec, len(overrides)) + for _, o := range overrides { + stepNameToOverride[o.Name] = o + } + for i, s := range steps { + o, found := stepNameToOverride[s.Name] + if !found { + continue + } + merged := v1.ResourceRequirements{} + err := mergeObjWithTemplate(&s.ComputeResources, &o.ComputeResources, &merged) + if err != nil { + return nil, err + } + steps[i].ComputeResources = merged + } + return steps, nil +} + +// MergeSidecarsWithSpecs takes a possibly nil list of overrides and a +// list of sidecars, merging each of the sidecars with the overrides' resource requirements, if +// it's not nil, and returning the resulting list. +func MergeSidecarsWithSpecs(sidecars []Sidecar, overrides []TaskRunSidecarSpec) ([]Sidecar, error) { + if len(overrides) == 0 { + return sidecars, nil + } + sidecarNameToOverride := make(map[string]TaskRunSidecarSpec, len(overrides)) + for _, o := range overrides { + sidecarNameToOverride[o.Name] = o + } + for i, s := range sidecars { + o, found := sidecarNameToOverride[s.Name] + if !found { + continue + } + merged := v1.ResourceRequirements{} + err := mergeObjWithTemplate(&s.ComputeResources, &o.ComputeResources, &merged) + if err != nil { + return nil, err + } + sidecars[i].ComputeResources = merged + } + return sidecars, nil +} + +// mergeObjWithTemplate merges obj with template and updates out to reflect the merged result. +// template, obj, and out should point to the same type. out points to the zero value of that type. +func mergeObjWithTemplate(template, obj, out interface{}) error { + md, err := getMergeData(template, out) + if err != nil { + return err + } + return mergeObjWithTemplateBytes(md, obj, out) +} + // getMergeData serializes the template and empty object to get the intermediate results necessary for // merging an object of the same type with this template. // This function is provided to avoid repeatedly serializing an identical template. diff --git a/pkg/apis/pipeline/v1/merge_test.go b/pkg/apis/pipeline/v1/merge_test.go index 587dd88d6e5..3c7db97963c 100644 --- a/pkg/apis/pipeline/v1/merge_test.go +++ b/pkg/apis/pipeline/v1/merge_test.go @@ -196,3 +196,321 @@ func TestMergeStepsWithStepTemplate(t *testing.T) { }) } } + +func TestMergeStepSpec(t *testing.T) { + tcs := []struct { + name string + steps []v1.Step + stepOverrides []v1.TaskRunStepSpec + want []v1.Step + }{{ + name: "no spec overrides", + steps: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }}, + want: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }}, + }, { + name: "not all steps overridden", + steps: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }, { + Name: "bar", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }}, + stepOverrides: []v1.TaskRunStepSpec{{ + Name: "bar", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + want: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }, { + Name: "bar", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + }, { + name: "override memory but not CPU", + steps: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("100m"), + }, + }, + }}, + stepOverrides: []v1.TaskRunStepSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + want: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }}, + }, { + name: "override request but not limit", + steps: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + stepOverrides: []v1.TaskRunStepSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + }, + }}, + want: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + }, { + name: "override request and limit", + steps: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + stepOverrides: []v1.TaskRunStepSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}, + want: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}, + }, { + // We don't make any effort to reject overrides that would result in invalid pods; + // instead, we let k8s reject the resulting pod. + name: "new request > old limit", + steps: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + stepOverrides: []v1.TaskRunStepSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}, + want: []v1.Step{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + steps, err := v1.MergeStepsWithSpecs(tc.steps, tc.stepOverrides) + if err != nil { + t.Errorf("unexpected error merging steps with overrides: %s", err) + } + if d := cmp.Diff(tc.want, steps); d != "" { + t.Errorf("merged steps don't match, diff: %s", diff.PrintWantGot(d)) + } + }) + } +} + +func TestMergeSidecarSpec(t *testing.T) { + tcs := []struct { + name string + sidecars []v1.Sidecar + sidecarOverrides []v1.TaskRunSidecarSpec + want []v1.Sidecar + }{{ + name: "no overrides", + sidecars: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }}, + want: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }}, + }, { + name: "not all sidecars overridden", + sidecars: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }, { + Name: "bar", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }}, + sidecarOverrides: []v1.TaskRunSidecarSpec{{ + Name: "bar", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + want: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }, { + Name: "bar", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + }, { + name: "override memory but not CPU", + sidecars: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("100m"), + }, + }, + }}, + sidecarOverrides: []v1.TaskRunSidecarSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + want: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }}, + }, { + name: "override request but not limit", + sidecars: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + sidecarOverrides: []v1.TaskRunSidecarSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + }, + }}, + want: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + }, { + name: "override request and limit", + sidecars: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + sidecarOverrides: []v1.TaskRunSidecarSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}, + want: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1.5Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}, + }, { + // We don't make any effort to reject overrides that would result in invalid pods; + // instead, we let k8s reject the resulting pod. + name: "new request > old limit", + sidecars: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + sidecarOverrides: []v1.TaskRunSidecarSpec{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + }, + }}, + want: []v1.Sidecar{{ + Name: "foo", + ComputeResources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("3Gi")}, + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + sidecars, err := v1.MergeSidecarsWithSpecs(tc.sidecars, tc.sidecarOverrides) + if err != nil { + t.Errorf("unexpected error merging sidecars with overrides: %s", err) + } + if d := cmp.Diff(tc.want, sidecars); d != "" { + t.Errorf("merged sidecars don't match, diff: %s", diff.PrintWantGot(d)) + } + }) + } +}