Skip to content

Commit

Permalink
Sync v1 TaskRun StepSpecs and SidecarSpecs Merge with v1beta1 Overrid…
Browse files Browse the repository at this point in the history 
…es Merge
  • Loading branch information
@JeromeJu @tekton-robot
JeromeJu authored and tekton-robot committed Apr 6, 2023
1 parent a91cff0 commit 333b0d4
Show file tree
Hide file tree
Showing 2 changed files with 378 additions and 0 deletions.
60 changes: 60 additions & 0 deletions pkg/apis/pipeline/v1/merge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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.
Expand Down
318 changes: 318 additions & 0 deletions pkg/apis/pipeline/v1/merge_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
})
}
}

0 comments on commit 333b0d4

Please sign in to comment.