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 ConfigMap volume source support to workspaces #1800

Merged
merged 1 commit into from Jan 14, 2020
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
19 changes: 19 additions & 0 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ at runtime you need to map the `workspaces` to actual physical volumes with

* [`emptyDir`](https://kubernetes.io/docs/concepts/storage/volumes/#emptydir)
* [`persistentVolumeClaim`](https://kubernetes.io/docs/concepts/storage/volumes/#persistentvolumeclaim)
* [`configMap`](https://kubernetes.io/docs/concepts/storage/volumes/#configmap)

_If you need support for a `VolumeSource` not listed here
[please open an issue](https://github.com/tektoncd/pipeline/issues) or feel free to
Expand Down Expand Up @@ -260,6 +261,24 @@ workspaces:
emptyDir: {}
```

A ConfigMap can also be used as a workspace with the following caveats:

1. ConfigMap volume sources are always mounted as read-only inside a task's
containers - tasks cannot write content to them and a step may error out
and fail the task if a write is attempted.
2. The ConfigMap you want to use as a workspace must already exist prior
to the TaskRun being submitted.

To use a [`configMap`](https://kubernetes.io/docs/concepts/storage/volumes/#configmap)
as a `workspace`:

```yaml
workspaces:
- name: myworkspace
configmap:
name: my-configmap
```

Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉

_For a complete example see [workspace.yaml](../examples/taskruns/workspace.yaml)._

## Status
Expand Down
47 changes: 25 additions & 22 deletions examples/taskruns/workspace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ spec:
accessModes:
- ReadWriteOnce
---
apiVersion: v1
kind: ConfigMap
metadata:
name: my-configmap
data:
message: hello world
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉

apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
Expand All @@ -26,43 +33,39 @@ spec:
- name: custom3
emptyDir: {}
subPath: testing
- name: custom4
configMap:
name: my-configmap
items:
- key: message
path: my-message.txt
taskSpec:
steps:
- name: write
image: ubuntu
script: |
#!/usr/bin/env bash
set -xe
echo $(workspaces.custom.volume) > $(workspaces.custom.path)/foo
script: echo $(workspaces.custom.volume) > $(workspaces.custom.path)/foo
- name: read
image: ubuntu
script: |
#!/usr/bin/env bash
set -xe
cat $(workspaces.custom.path)/foo | grep $(workspaces.custom.volume)
script: cat $(workspaces.custom.path)/foo | grep $(workspaces.custom.volume)
- name: write2
image: ubuntu
script: |
#!/usr/bin/env bash
set -xe
echo $(workspaces.custom2.path) > $(workspaces.custom2.path)/foo
script: echo $(workspaces.custom2.path) > $(workspaces.custom2.path)/foo
- name: read2
image: ubuntu
script: |
#!/usr/bin/env bash
cat $(workspaces.custom2.path)/foo | grep $(workspaces.custom2.path)
script: cat $(workspaces.custom2.path)/foo | grep $(workspaces.custom2.path)
- name: write3
image: ubuntu
script: |
#!/usr/bin/env bash
echo $(workspaces.custom3.path) > $(workspaces.custom3.path)/foo
script: echo $(workspaces.custom3.path) > $(workspaces.custom3.path)/foo
- name: read3
image: ubuntu
script: |
#!/usr/bin/env bash
cat $(workspaces.custom3.path)/foo | grep $(workspaces.custom3.path)
script: cat $(workspaces.custom3.path)/foo | grep $(workspaces.custom3.path)
- name: readconfigmap
image: ubuntu
script: cat $(workspaces.custom4.path)/my-message.txt | grep "hello world"
workspaces:
- name: custom
- name: custom2
mountPath: /foo/bar/baz
- name: custom3
- name: custom3
- name: custom4
mountPath: /baz/bar/quux
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1alpha1/workspace_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ import (
type WorkspaceDeclaration = v1alpha2.WorkspaceDeclaration

// WorkspaceBinding maps a Task's declared workspace to a Volume.
// Currently we only support PersistentVolumeClaims and EmptyDir.
// Currently we only support PersistentVolumeClaims, EmptyDir and ConfigMap.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we'd be better off removing this comment so we don't have it fall out of date later 🤔

type WorkspaceBinding = v1alpha2.WorkspaceBinding
16 changes: 16 additions & 0 deletions pkg/apis/pipeline/v1alpha1/workspace_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ func TestWorkspaceBindingValidateValid(t *testing.T) {
Name: "beth",
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
}, {
name: "Valid configMap",
binding: &WorkspaceBinding{
Name: "beth",
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "a-configmap-name",
},
},
},
}} {
t.Run(tc.name, func(t *testing.T) {
if err := tc.binding.Validate(context.Background()); err != nil {
Expand Down Expand Up @@ -78,6 +88,12 @@ func TestWorkspaceBindingValidateInvalid(t *testing.T) {
Name: "beth",
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{},
},
}, {
name: "Provide configmap without a name",
binding: &WorkspaceBinding{
Name: "beth",
ConfigMap: &corev1.ConfigMapVolumeSource{},
},
}} {
t.Run(tc.name, func(t *testing.T) {
if err := tc.binding.Validate(context.Background()); err == nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1alpha2/workspace_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,7 @@ type WorkspaceBinding struct {
// Either this OR PersistentVolumeClaim can be used.
// +optional
EmptyDir *corev1.EmptyDirVolumeSource `json:"emptyDir,omitempty"`
// ConfigMap represents a configMap that should populate this workspace.
// +optional
ConfigMap *corev1.ConfigMapVolumeSource `json:"configMap,omitempty"`
}
42 changes: 36 additions & 6 deletions pkg/apis/pipeline/v1alpha2/workspace_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ import (
"knative.dev/pkg/apis"
)

// allVolumeSourceFields is a list of all the volume source field paths that a
// WorkspaceBinding may include.
var allVolumeSourceFields []string = []string{
"workspace.persistentvolumeclaim",
"workspace.emptydir",
"workspace.configmap",
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉


// Validate looks at the Volume provided in wb and makes sure that it is valid.
// This means that only one VolumeSource can be specified, and also that the
// supported VolumeSource is itself valid.
Expand All @@ -31,19 +39,41 @@ func (b *WorkspaceBinding) Validate(ctx context.Context) *apis.FieldError {
return apis.ErrMissingField(apis.CurrentField)
}

// Users should only provide one supported VolumeSource.
if b.PersistentVolumeClaim != nil && b.EmptyDir != nil {
return apis.ErrMultipleOneOf("workspace.persistentvolumeclaim", "workspace.emptydir")
numSources := b.numSources()

if numSources > 1 {
return apis.ErrMultipleOneOf(allVolumeSourceFields...)
}

// Users must provide at least one supported VolumeSource.
if b.PersistentVolumeClaim == nil && b.EmptyDir == nil {
return apis.ErrMissingOneOf("workspace.persistentvolumeclaim", "workspace.emptydir")
if numSources == 0 {
return apis.ErrMissingOneOf(allVolumeSourceFields...)
}

// For a PersistentVolumeClaim to work, you must at least provide the name of the PVC to use.
if b.PersistentVolumeClaim != nil && b.PersistentVolumeClaim.ClaimName == "" {
return apis.ErrMissingField("workspace.persistentvolumeclaim.claimname")
}

// For a ConfigMap to work, you must provide the name of the ConfigMap to use.
if b.ConfigMap != nil && b.ConfigMap.LocalObjectReference.Name == "" {
return apis.ErrMissingField("workspace.configmap.name")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we have a unit test that tries to set a configmap + something else?

that's quickly starting to balloon out of control unit test permutation wise - i wonder if we can isolate the logic for determining this somehow such that the tests wont need to be updated with every permutation 🤔


return nil
}

// numSources returns the total number of volume sources that this WorkspaceBinding
// has been configured with.
func (b *WorkspaceBinding) numSources() int {
n := 0
if b.PersistentVolumeClaim != nil {
n++
}
if b.EmptyDir != nil {
n++
}
if b.ConfigMap != nil {
n++
}
return n
}
16 changes: 16 additions & 0 deletions pkg/apis/pipeline/v1alpha2/workspace_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ func TestWorkspaceBindingValidateValid(t *testing.T) {
Name: "beth",
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
}, {
name: "Valid configMap",
binding: &WorkspaceBinding{
Name: "beth",
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "a-configmap-name",
},
},
},
}} {
t.Run(tc.name, func(t *testing.T) {
if err := tc.binding.Validate(context.Background()); err != nil {
Expand Down Expand Up @@ -78,6 +88,12 @@ func TestWorkspaceBindingValidateInvalid(t *testing.T) {
Name: "beth",
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{},
},
}, {
name: "Provide configmap without a name",
binding: &WorkspaceBinding{
Name: "beth",
ConfigMap: &corev1.ConfigMapVolumeSource{},
},
}} {
t.Run(tc.name, func(t *testing.T) {
if err := tc.binding.Validate(context.Background()); err == nil {
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1alpha2/zz_generated.deepcopy.go

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

13 changes: 11 additions & 2 deletions pkg/workspace/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ func GetVolumes(wb []v1alpha1.WorkspaceBinding) map[string]corev1.Volume {
v := map[string]corev1.Volume{}
for _, w := range wb {
name := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(volumeNameBase)
if w.PersistentVolumeClaim != nil {
switch {
case w.PersistentVolumeClaim != nil:
// If it's a PVC, we need to check if we've encountered it before so we avoid mounting it twice
if vv, ok := pvcs[w.PersistentVolumeClaim.ClaimName]; ok {
v[w.Name] = vv
Expand All @@ -34,14 +35,22 @@ func GetVolumes(wb []v1alpha1.WorkspaceBinding) map[string]corev1.Volume {
}
pvcs[pvc.ClaimName] = v[w.Name]
}
} else if w.EmptyDir != nil {
case w.EmptyDir != nil:
ed := *w.EmptyDir
v[w.Name] = corev1.Volume{
Name: name,
VolumeSource: corev1.VolumeSource{
EmptyDir: &ed,
},
}
case w.ConfigMap != nil:
cm := *w.ConfigMap
v[w.Name] = corev1.Volume{
Name: name,
VolumeSource: corev1.VolumeSource{
ConfigMap: &cm,
},
}
}
}
return v
Expand Down
39 changes: 35 additions & 4 deletions pkg/workspace/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,37 @@ func TestGetVolumes(t *testing.T) {
},
},
},
}, {
name: "binding a single workspace with configMap",
workspaces: []v1alpha1.WorkspaceBinding{{
Name: "custom",
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "foobarconfigmap",
},
Items: []corev1.KeyToPath{{
Key: "foobar",
Path: "foobar.txt",
}},
},
SubPath: "/foo/bar/baz",
}},
expectedVolumes: map[string]corev1.Volume{
"custom": {
Name: "ws-mssqb",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: "foobarconfigmap",
},
Items: []corev1.KeyToPath{{
Key: "foobar",
Path: "foobar.txt",
}},
},
},
},
},
}, {
name: "0 workspace bindings",
workspaces: []v1alpha1.WorkspaceBinding{},
Expand All @@ -74,15 +105,15 @@ func TestGetVolumes(t *testing.T) {
}},
expectedVolumes: map[string]corev1.Volume{
"custom": {
Name: "ws-mssqb",
Name: "ws-78c5n",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
},
},
},
"even-more-custom": {
Name: "ws-78c5n",
Name: "ws-6nl7g",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "myotherpvc",
Expand All @@ -107,7 +138,7 @@ func TestGetVolumes(t *testing.T) {
}},
expectedVolumes: map[string]corev1.Volume{
"custom": {
Name: "ws-6nl7g",
Name: "ws-j2tds",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand All @@ -116,7 +147,7 @@ func TestGetVolumes(t *testing.T) {
},
"custom2": {
// Since it is the same PVC source, it can't be added twice with two different names
Name: "ws-6nl7g",
Name: "ws-j2tds",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand Down