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.

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

Add container template configuration to Task #767

Merged
merged 1 commit into from
Apr 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 34 additions & 1 deletion docs/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ entire Kubernetes cluster.
- [Outputs](#outputs)
- [Controlling where resources are mounted](#controlling-where-resources-are-mounted)
- [Volumes](#volumes)
- [Container Template](#container-template)
- [Templating](#templating)
- [Examples](#examples)

Expand Down Expand Up @@ -72,7 +73,9 @@ following fields:
- [`outputs`](#outputs) - Specifies [`PipelineResources`](resources.md) needed
by your `Task`
- [`volumes`](#volumes) - Specifies one or more volumes that you want to make
available to your build.
available to your `Task`'s steps.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, thanks for fixing! ❤️

- [`containerTemplate`](#container-template) - Specifies a `Container`
definition to use as the basis for all steps within your `Task`.

[kubernetes-overview]:
https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields
Expand Down Expand Up @@ -322,6 +325,36 @@ For example, use volumes to accomplish one of the following common tasks:
unsafe_. Use [kaniko](https://github.com/GoogleContainerTools/kaniko) instead.
This is used only for the purposes of demonstration.

### Container Template

Specifies a [`Container`](https://kubernetes.io/docs/concepts/containers/)
configuration that will be used as the basis for all [`steps`](#steps) in your
`Task`. Configuration in an individual step will override or merge with the
container template's configuration.

In the example below, the `Task` specifies a `containerTemplate` with the
environment variable `FOO` set to `bar`. The first step will use that value for
`FOO`, but in the second step, `FOO` is overridden and set to `baz`.

```yaml
containerTemplate:
env:
- name: "FOO"
value: "bar"
steps:
- image: ubuntu
command: [echo]
args:
["FOO is ${FOO}"]
- image: ubuntu
command: [echo]
args:
["FOO is ${FOO}"]
env:
- name: "FOO"
value: "baz"
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wooooot great docs 🙇‍♀️ :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Albeit ones I now have to rewrite...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the new ones are great too!!

### Templating

`Tasks` support templating using values from all [`inputs`](#inputs) and
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/task_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ type TaskSpec struct {
// Volumes is a collection of volumes that are available to mount into the
// steps of the build.
Volumes []corev1.Volume `json:"volumes,omitempty"`

// ContainerTemplate can be used as the basis for all step containers within the
// Task, so that the steps inherit settings on the base container.
ContainerTemplate *corev1.Container `json:"containerTemplate,omitempty"`
}

// Check that Task may be validated and defaulted.
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 84 additions & 0 deletions pkg/merge/merge.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
Copyright 2019 Knative Authors LLC
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package merge

import (
"encoding/json"
"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/strategicpatch"
)

// CombineStepsWithContainerTemplate takes a possibly nil container template and a list of step containers, merging each
// of the step containers onto the container template, if it's not nil, and returning the resulting list.
func CombineStepsWithContainerTemplate(template *v1.Container, steps []v1.Container) ([]v1.Container, error) {
if template == nil {
return steps, nil
}

// We need JSON bytes to generate a patch to merge the step containers onto the template container, so marshal the template.
templateAsJSON, err := json.Marshal(template)
if err != nil {
return nil, err
}
// We need to do a three-way merge to actually combine the template and step containers, so we need an empty container
// as the "original"
emptyAsJSON, err := json.Marshal(&v1.Container{})
if err != nil {
return nil, err
}

for i, s := range steps {
// Marshal the step to JSON
stepAsJSON, err := json.Marshal(s)
if err != nil {
return nil, err
}

// Get the patch meta for Container, which is needed for generating and applying the merge patch.
patchSchema, err := strategicpatch.NewPatchMetaFromStruct(template)

if err != nil {
return nil, err
}

// Create a merge patch, with the empty JSON as the original, the step JSON as the modified, and the template
// JSON as the current - this lets us do a deep merge of the template and step containers, with awareness of
// the "patchMerge" tags.
patch, err := strategicpatch.CreateThreeWayMergePatch(emptyAsJSON, stepAsJSON, templateAsJSON, patchSchema, true)
if err != nil {
return nil, err
}

// Actually apply the merge patch to the template JSON.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks so much for these detailed comments, I think it really clears up what's going on! 🙏 🎉

mergedAsJSON, err := strategicpatch.StrategicMergePatchUsingLookupPatchMeta(templateAsJSON, patch, patchSchema)
if err != nil {
return nil, err
}

// Unmarshal the merged JSON to a Container pointer, and return it.
merged := &v1.Container{}
err = json.Unmarshal(mergedAsJSON, merged)
if err != nil {
return nil, err
}

// If the container's args is nil, reset it to empty instead
if merged.Args == nil && s.Args != nil {
merged.Args = []string{}
}

steps[i] = *merged
}
return steps, nil
}
118 changes: 118 additions & 0 deletions pkg/merge/merge_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
Copyright 2019 Knative Authors LLC
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package merge

import (
"testing"

"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
)

func TestCombineStepsWithContainerTemplate(t *testing.T) {
resourceQuantityCmp := cmp.Comparer(func(x, y resource.Quantity) bool {
return x.Cmp(y) == 0
})

for _, tc := range []struct {
name string
template *corev1.Container
steps []corev1.Container
expected []corev1.Container
}{{
name: "nil-template",
template: nil,
steps: []corev1.Container{{
Image: "some-image",
}},
expected: []corev1.Container{{
Image: "some-image",
}},
}, {
name: "not-overlapping",
template: &corev1.Container{
Command: []string{"/somecmd"},
},
steps: []corev1.Container{{
Image: "some-image",
}},
expected: []corev1.Container{{
Command: []string{"/somecmd"},
Image: "some-image",
}},
}, {
name: "overwriting-one-field",
template: &corev1.Container{
Image: "some-image",
Command: []string{"/somecmd"},
},
steps: []corev1.Container{{
Image: "some-other-image",
}},
expected: []corev1.Container{{
Command: []string{"/somecmd"},
Image: "some-other-image",
}},
}, {
name: "merge-and-overwrite-slice",
template: &corev1.Container{
Env: []corev1.EnvVar{
{
Name: "KEEP_THIS",
Value: "A_VALUE",
}, {
Name: "SOME_KEY",
Value: "ORIGINAL_VALUE",
},
},
},
steps: []corev1.Container{{
Env: []corev1.EnvVar{
{
Name: "NEW_KEY",
Value: "A_VALUE",
}, {
Name: "SOME_KEY",
Value: "NEW_VALUE",
},
},
}},
expected: []corev1.Container{{
Env: []corev1.EnvVar{
{
Name: "NEW_KEY",
Value: "A_VALUE",
}, {
Name: "KEEP_THIS",
Value: "A_VALUE",
}, {
Name: "SOME_KEY",
Value: "NEW_VALUE",
},
},
}},
}} {
t.Run(tc.name, func(t *testing.T) {
result, err := CombineStepsWithContainerTemplate(tc.template, tc.steps)
if err != nil {
t.Errorf("expected no error. Got error %v", err)
}

if d := cmp.Diff(tc.expected, result, resourceQuantityCmp); d != "" {
t.Errorf("Combined steps don't match, diff: %s", d)
}
})
}
}
25 changes: 17 additions & 8 deletions pkg/reconciler/v1alpha1/taskrun/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/tektoncd/pipeline/pkg/credentials"
"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
"github.com/tektoncd/pipeline/pkg/merge"
"github.com/tektoncd/pipeline/pkg/names"
"github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/taskrun/entrypoint"
)
Expand Down Expand Up @@ -174,13 +175,13 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
if err != nil {
return nil, err
}

initContainers := []corev1.Container{*cred}
podContainers := []corev1.Container{}

maxIndicesByResource := findMaxResourceRequest(taskSpec.Steps, corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage)

for i, step := range taskSpec.Steps {
for i := range taskSpec.Steps {
step := &taskSpec.Steps[i]
step.Env = append(implicitEnvVars, step.Env...)
// TODO(mattmoor): Check that volumeMounts match volumes.

Expand All @@ -206,10 +207,10 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
}
// use the step name to add the entrypoint biary as an init container
if step.Name == names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%v%v", containerPrefix, entrypoint.InitContainerName)) {
initContainers = append(initContainers, step)
initContainers = append(initContainers, *step)
} else {
zeroNonMaxResourceRequests(&step, i, maxIndicesByResource)
podContainers = append(podContainers, step)
zeroNonMaxResourceRequests(step, i, maxIndicesByResource)
podContainers = append(podContainers, *step)
}
}
// Add our implicit volumes and any volumes needed for secrets to the explicitly
Expand All @@ -231,6 +232,15 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
entrypoint.RedirectStep(cache, len(podContainers), nopContainer, kubeclient, taskRun, logger)
podContainers = append(podContainers, *nopContainer)

mergedInitContainers, err := merge.CombineStepsWithContainerTemplate(taskSpec.ContainerTemplate, initContainers)
if err != nil {
return nil, err
}
mergedPodContainers, err := merge.CombineStepsWithContainerTemplate(taskSpec.ContainerTemplate, podContainers)
if err != nil {
return nil, err
}

return &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
// We execute the build's pod in the same namespace as where the build was
Expand All @@ -249,10 +259,9 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
Labels: makeLabels(taskRun),
},
Spec: corev1.PodSpec{
// If the build fails, don't restart it.
RestartPolicy: corev1.RestartPolicyNever,
InitContainers: initContainers,
Containers: podContainers,
InitContainers: mergedInitContainers,
Containers: mergedPodContainers,
ServiceAccountName: taskRun.Spec.ServiceAccount,
Volumes: volumes,
NodeSelector: taskRun.Spec.NodeSelector,
Expand Down
Loading