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 a PodTemplate field for PipelineRun and TaskRun #1004

Merged
merged 1 commit into from
Jul 12, 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
60 changes: 60 additions & 0 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ A `TaskRun` runs until all `steps` have completed or until a failure occurs.
- [Providing resources](#providing-resources)
- [Overriding where resources are copied from](#overriding-where-resources-are-copied-from)
- [Service Account](#service-account)
- [Pod Template](#pod-template)
- [Steps](#steps)
- [Cancelling a TaskRun](#cancelling-a-taskrun)
- [Examples](#examples)
Expand Down Expand Up @@ -159,6 +160,65 @@ of the `TaskRun` resource object.
For examples and more information about specifying service accounts, see the
[`ServiceAccount`](./auth.md) reference topic.

## Pod Template

Specifies a subset of
[`PodSpec`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.15/#pod-v1-core)
configuration that will be used as the basis for the `Task` pod. This
allows to customize some Pod specific field per `Task` execution, aka
`TaskRun`. The current field supported are

- `nodeSelector`: a selector which must be true for the pod to fit on
a node, see [here](https://kubernetes.io/docs/concepts/configuration/assign-pod-node/).
- `tolerations`: allow (but do not require) the pods to schedule onto
nodes with matching taints.
- `affinity`: allow to constrain which nodes your pod is eligible to
be scheduled on, based on labels on the node.
- `securityContext`: pod-level security attributes and common
container settings, like `runAsUser` or `selinux`.
- `volumes`: list of volumes that can be mounted by containers
belonging to the pod. This lets the user of a Task define which type
of volume to use for a Task `volumeMount`

In the following example, the Task is defined with a `volumeMount`
(`my-cache`), that is provided by the TaskRun, using a
PersistenceVolumeClaim. The Pod will also run as a non-root user.

```yaml
apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
name: myTask
namespace: default
spec:
steps:
- name: write something
image: ubuntu
command: ["bash", "-c"]
args: ["echo 'foo' > /my-cache/bar"]
volumeMounts:
- name: my-cache
mountPath: /my-cache
```

```yaml
apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
name: myTaskRun
namespace: default
spec:
taskRef:
name: myTask
podTemplate:
securityContext:
runAsNonRoot: true
volumes:
- name: my-cache
persistentVolumeClaim:
claimName: my-volume-claim
```

### Overriding where resources are copied from

When specifying input and output `PipelineResources`, you can optionally specify
Expand Down
8 changes: 4 additions & 4 deletions examples/taskruns/task-volume.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ spec:
volumeMounts:
- name: custom
mountPath: /short/and/stout

volumes:
- name: custom
emptyDir: {}
---
apiVersion: tekton.dev/v1alpha1
kind: TaskRun
Expand All @@ -31,3 +27,7 @@ metadata:
spec:
taskRef:
name: task-volume
podTemplate:
volumes:
- name: custom
emptyDir: {}
vdemeester marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ type PipelineRunSpec struct {
// Refer to Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
// +optional
Timeout *metav1.Duration `json:"timeout,omitempty"`

// PodTemplate holds pod specific configuration
PodTemplate PodTemplate `json:"podTemplate,omitempty"`

// FIXME(vdemeester) Deprecated
// NodeSelector is a selector which must be true for the pod to fit on a node.
// Selector which must match a node's labels for the pod to be scheduled on that node.
// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
Expand Down
62 changes: 62 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pod.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2019 The Tekton Authors.
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 v1alpha1

import (
corev1 "k8s.io/api/core/v1"
)

// PodTemplate holds pod specific configuration
type PodTemplate struct {
vdemeester marked this conversation as resolved.
Show resolved Hide resolved
// NodeSelector is a selector which must be true for the pod to fit on a node.
// Selector which must match a node's labels for the pod to be scheduled on that node.
// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`

// If specified, the pod's tolerations.
// +optional
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`

// If specified, the pod's scheduling constraints
// +optional
Affinity *corev1.Affinity `json:"affinity,omitempty"`

// SecurityContext holds pod-level security attributes and common container settings.
// Optional: Defaults to empty. See type description for default values of each field.
// +optional
SecurityContext *corev1.PodSecurityContext `json:"securityContext,omitempty"`

// List of volumes that can be mounted by containers belonging to the pod.
// More info: https://kubernetes.io/docs/concepts/storage/volumes
// +optional
Volumes []corev1.Volume `json:"volumes,omitempty" patchStrategy:"merge,retainKeys" patchMergeKey:"name" protobuf:"bytes,1,rep,name=volumes"`
}

// CombinePodTemplate takes a PodTemplate (either from TaskRun or PipelineRun) and merge it with deprecated field that were inlined.
func CombinedPodTemplate(template PodTemplate, deprecatedNodeSelector map[string]string, deprecatedTolerations []corev1.Toleration, deprecatedAffinity *corev1.Affinity) PodTemplate {
if len(template.NodeSelector) == 0 && len(deprecatedNodeSelector) != 0 {
template.NodeSelector = deprecatedNodeSelector
}
if len(template.Tolerations) == 0 && len(deprecatedTolerations) != 0 {
template.Tolerations = deprecatedTolerations
}
if template.Affinity == nil && deprecatedAffinity != nil {
template.Affinity = deprecatedAffinity
}
return template
}
139 changes: 139 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pod_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
Copyright 2019 The Tekton Authors.

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 v1alpha1_test

import (
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
corev1 "k8s.io/api/core/v1"
)

func TestCombinedPodTemplateTakesPrecedence(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

affinity := &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{},
}
nodeSelector := map[string]string{
"banana": "chocolat",
}
tolerations := []corev1.Toleration{{
Key: "banana",
Value: "chocolat",
}}

template := v1alpha1.PodTemplate{
NodeSelector: map[string]string{
"foo": "bar",
"bar": "baz",
},
Tolerations: []corev1.Toleration{{
Key: "foo",
Value: "bar",
}},
Affinity: &corev1.Affinity{
PodAffinity: &corev1.PodAffinity{},
},
}
// Same as template above
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm a bit confused! wouldn't we want "banana": "chocolat", etc. to show up in the expected (got) template?

Copy link
Member Author

Choose a reason for hiding this comment

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

I should change the names of the variable to make it clearer, good point. So affinity, nodeSelector and tolerations are coming from the deprecated fields, so PodTemplate takes precedence over them. I choose to not merge NodSelector and Tolerations to make it simpler (especially as those deprecated field are going away rather shortly anyway) ; that's why "banana": "chocolat" is not in the got template 👼

Copy link
Collaborator

Choose a reason for hiding this comment

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

OOOOOOOOH okay i get it, the point is to override them - maybe we should include a deprecated field we don't override as well in one of the tests? (to make sure they can be combined - unless it's supposed to be all or nothing, all depcreated or all new)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah right, I see what you mean, yeah I'll update the tests

want := v1alpha1.PodTemplate{
NodeSelector: map[string]string{
"foo": "bar",
"bar": "baz",
},
Tolerations: []corev1.Toleration{{
Key: "foo",
Value: "bar",
}},
Affinity: &corev1.Affinity{
PodAffinity: &corev1.PodAffinity{},
},
}

got := v1alpha1.CombinedPodTemplate(template, nodeSelector, tolerations, affinity)
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
}
func TestCombinedPodTemplate(t *testing.T) {
nodeSelector := map[string]string{
"banana": "chocolat",
}
tolerations := []corev1.Toleration{{
Key: "banana",
Value: "chocolat",
}}

template := v1alpha1.PodTemplate{
Tolerations: []corev1.Toleration{{
Key: "foo",
Value: "bar",
}},
Affinity: &corev1.Affinity{
PodAffinity: &corev1.PodAffinity{},
},
}
// Same as template above
want := v1alpha1.PodTemplate{
NodeSelector: map[string]string{
"banana": "chocolat",
Copy link
Collaborator

Choose a reason for hiding this comment

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

woot 👍

},
Tolerations: []corev1.Toleration{{
Key: "foo",
Value: "bar",
}},
Affinity: &corev1.Affinity{
PodAffinity: &corev1.PodAffinity{},
},
}

got := v1alpha1.CombinedPodTemplate(template, nodeSelector, tolerations, nil)
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
}

func TestCombinedPodTemplateOnlyDeprecated(t *testing.T) {
template := v1alpha1.PodTemplate{}
affinity := &corev1.Affinity{
NodeAffinity: &corev1.NodeAffinity{},
}

nodeSelector := map[string]string{
"foo": "bar",
}
tolerations := []corev1.Toleration{{
Key: "foo",
Value: "bar",
}}

want := v1alpha1.PodTemplate{
NodeSelector: map[string]string{
"foo": "bar",
},
Tolerations: []corev1.Toleration{{
Key: "foo",
Value: "bar",
}},
Affinity: affinity,
}

got := v1alpha1.CombinedPodTemplate(template, nodeSelector, tolerations, affinity)
if d := cmp.Diff(got, want); d != "" {
t.Errorf("Diff:\n%s", d)
}
}
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1alpha1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ type TaskRunSpec struct {
// Refer Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
// +optional
Timeout *metav1.Duration `json:"timeout,omitempty"`

// PodTemplate holds pod specific configuration
PodTemplate PodTemplate `json:"podTemplate,omitempty"`

// FIXME(vdemeester) Deprecated
// NodeSelector is a selector which must be true for the pod to fit on a node.
// Selector which must match a node's labels for the pod to be scheduled on that node.
// More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
Expand Down
49 changes: 49 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.

Loading