Skip to content

Commit

Permalink
Update resource path to be prefixed with resource name
Browse files Browse the repository at this point in the history
Why: If task has multiple input resources defined without target path then all
of them would be copied into /workspace folder and would overlap files.

What: With this PR input resources will be placed under file path
`/workspace/task_resource_name` . This will provide specific filepath
for each resource and there should be no clash.
  • Loading branch information
Shash Reddy authored and knative-prow-robot committed Jan 17, 2019
1 parent 39828e2 commit 8557df4
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 46 deletions.
33 changes: 24 additions & 9 deletions docs/developers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ path `/pvc` by Pipelinerun.
### How are inputs handled?

Input resources like source code(git) or artifacts are dumped at path
`/workspace/`. Resource definition in task can have custom target directory. If
`targetPath` is mentioned then controllers have to be responsible for adding
container definition to create directories and pulling down the versioned
`/workspace/task_resource_name`. Resource definition in task can have custom target directory. If
`targetPath` is mentioned in task input then controllers have to be responsible for adding
container definitions to create directories and also to fetch versioned
artifacts into that directory.

### How are outputs handled?
Expand All @@ -32,9 +32,7 @@ expected in directory path `/workspace/output/resource_name`.
- If there is PVC volume present(taskrun holds owner reference to pipelinerun)
then copy step is added as well.

- Copy step includes resource being copied to PVC to path
`/pvc/task_name/resource_name` from `/workspace/output/resource_name` if
resource is not declared in input resource like below example.
- If resource is declared only in output but not in input for task then copy step includes resource being copied to PVC to path `/pvc/task_name/resource_name` from `/workspace/output/resource_name` like the following example.

```yaml
kind: Task
Expand All @@ -48,9 +46,7 @@ expected in directory path `/workspace/output/resource_name`.
type: storage
```
- Copy step includes resource being copied to PVC to path
`/pvc/task_name/resource_name` from `/workspace/random-space/` if resource
is declared in input resource like below example.
- If resource is declared both in input and output for task then copy step includes resource being copied to PVC to path `/pvc/task_name/resource_name` from `/workspace/random-space/` if input resource has custom target directory(`random-space`) declared like the following example.

```yaml
kind: Task
Expand All @@ -68,3 +64,22 @@ expected in directory path `/workspace/output/resource_name`.
- name: gcs-workspace
type: storage
```

- If resource is declared both in input and output for task without custom target directory then copy step includes resource being copied to PVC to path `/pvc/task_name/resource_name` from `/workspace/random-space/` like the following example.

```yaml
kind: Task
metadata:
name: get-gcs-task
namespace: default
spec:
inputs:
resources:
- name: gcs-workspace
type: storage
name: storage
outputs:
resources:
- name: gcs-workspace
type: storage
```
14 changes: 7 additions & 7 deletions docs/tutorial.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,17 @@ metadata:
spec:
inputs:
resources:
- name: workspace
- name: docker-source
type: git
params:
- name: pathToDockerFile
description: The path to the dockerfile to build
default: /workspace/Dockerfile
default: /workspace/docker-source/Dockerfile
- name: pathToContext
description:
The build context used by Kaniko
(https://github.com/GoogleContainerTools/kaniko#kaniko-build-contexts)
default: /workspace
default: /workspace/docker-source
outputs:
resources:
- name: builtImage
Expand Down Expand Up @@ -199,14 +199,14 @@ spec:
type: manual
inputs:
resources:
- name: workspace
- name: gitspace
resourceRef:
name: skaffold-git
params:
- name: pathToDockerFile
value: Dockerfile
- name: pathToContext
value: /workspace/examples/microservices/leeroy-web
value: /workspace/gitspace/examples/microservices/leeroy-web
outputs:
resources:
- name: builtImage
Expand Down Expand Up @@ -271,9 +271,9 @@ spec:
- name: pathToDockerFile
value: Dockerfile
- name: pathToContext
value: /workspace/examples/microservices/leeroy-web
value: /workspace/git-source/examples/microservices/leeroy-web
resources:
- name: workspace
- name: git-source
paths: null
resourceRef:
name: skaffold-git
Expand Down
1 change: 1 addition & 0 deletions docs/using.md
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ We current support these `PipelineResources`:
- [Git resource](#git-resource)
- [Image resource](#image-resource)
- [Cluster resource](#cluster-resource)
- [Storage resource](#storage-resource)

When used as inputs, these resources will be made available in a mounted
directory called `/workspace` at the path /workspace/<resource-name>`.
Expand Down
66 changes: 61 additions & 5 deletions pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestAddResourceToBuild(t *testing.T) {
Spec: v1alpha1.TaskSpec{
Inputs: &v1alpha1.Inputs{
Resources: []v1alpha1.TaskResource{{
Name: "workspace",
Name: "gitspace",
Type: "git",
}},
},
Expand Down Expand Up @@ -230,7 +230,7 @@ func TestAddResourceToBuild(t *testing.T) {
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "the-git",
},
Name: "workspace",
Name: "gitspace",
}},
},
},
Expand Down Expand Up @@ -291,7 +291,7 @@ func TestAddResourceToBuild(t *testing.T) {
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "the-git-with-branch",
},
Name: "workspace",
Name: "gitspace",
}},
},
},
Expand Down Expand Up @@ -342,7 +342,7 @@ func TestAddResourceToBuild(t *testing.T) {
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "the-git",
},
Name: "workspace",
Name: "gitspace",
}},
},
},
Expand Down Expand Up @@ -391,7 +391,7 @@ func TestAddResourceToBuild(t *testing.T) {
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "the-git-with-branch",
},
Name: "workspace",
Name: "gitspace",
}},
},
},
Expand Down Expand Up @@ -424,6 +424,62 @@ func TestAddResourceToBuild(t *testing.T) {
}},
},
},
}, {
desc: "git resource as input from previous task",
task: task,
taskRun: &v1alpha1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "get-from-git",
Namespace: "marshmallow",
OwnerReferences: []metav1.OwnerReference{{
Kind: "PipelineRun",
Name: "pipelinerun",
}},
},
Spec: v1alpha1.TaskRunSpec{
Inputs: v1alpha1.TaskRunInputs{
Resources: []v1alpha1.TaskResourceBinding{{
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "the-git",
},
Name: "gitspace",
Paths: []string{"prev-task-path"},
}},
},
},
},
build: build(),
wantErr: false,
want: &buildv1alpha1.Build{
TypeMeta: metav1.TypeMeta{
Kind: "Build",
APIVersion: "build.knative.dev/v1alpha1"},
ObjectMeta: metav1.ObjectMeta{
Name: "build-from-repo",
Namespace: "marshmallow",
OwnerReferences: []metav1.OwnerReference{{
APIVersion: "pipeline.knative.dev/v1alpha1",
Kind: "TaskRun",
Name: "build-from-repo-run",
Controller: &boolTrue,
BlockOwnerDeletion: &boolTrue,
}},
},
Spec: buildv1alpha1.BuildSpec{
Steps: []corev1.Container{{
Name: "source-copy-gitspace-0",
Image: "override-with-bash-noop:latest",
Args: []string{"-args", "cp -r prev-task-path/. /workspace/gitspace"},
VolumeMounts: []corev1.VolumeMount{{MountPath: "/pvc", Name: "pipelinerun-pvc"}},
}},
Volumes: []corev1.Volume{{
Name: "pipelinerun-pvc",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: "pipelinerun-pvc"},
},
}},
},
},
}, {
desc: "storage resource as input",
task: taskWithTargetPath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func AddInputResource(
for i, path := range boundResource.Paths {
var dPath string
if input.TargetPath == "" {
dPath = workspaceDir
dPath = filepath.Join(workspaceDir, input.Name)
} else {
dPath = filepath.Join(workspaceDir, input.TargetPath)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func AddOutputResources(

if taskSpec.Inputs != nil {
for _, input := range taskSpec.Inputs.Resources {
var targetPath = workspaceDir
var targetPath = filepath.Join(workspaceDir, input.Name)
if input.TargetPath != "" {
targetPath = filepath.Join(workspaceDir, input.TargetPath)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func Test_Valid_OutputResources(t *testing.T) {
}, {
Name: "source-copy-source-git",
Image: "override-with-bash-noop:latest",
Args: []string{"-args", "cp -r /workspace/. pipeline-task-name"},
Args: []string{"-args", "cp -r /workspace/source-workspace/. pipeline-task-name"},
VolumeMounts: []corev1.VolumeMount{{
Name: "pipelinerun-pvc",
MountPath: "/pvc",
Expand Down
8 changes: 6 additions & 2 deletions pkg/reconciler/v1alpha1/taskrun/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ func gitToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, e
}

if source.TargetPath != "" {
args = append(args, []string{"-path", source.TargetPath}...)
args = append(args, []string{"-path", filepath.Join(source.Name, source.TargetPath)}...)
} else {
args = append(args, []string{"-path", source.Name}...)
}

containerName := initContainerPrefix + gitSource + "-"
Expand Down Expand Up @@ -148,7 +150,9 @@ func gcsToContainer(source v1alpha1.SourceSpec, index int) (*corev1.Container, e
args := []string{"--type", string(gcs.Type), "--location", gcs.Location}
// dest_dir is the destination directory for GCS files to be copies"
if source.TargetPath != "" {
args = append(args, "--dest_dir", filepath.Join(workspaceDir, source.TargetPath))
args = append(args, "--dest_dir", filepath.Join(workspaceDir, source.Name, source.TargetPath))
} else {
args = append(args, "--dest_dir", filepath.Join(workspaceDir, source.Name))
}

// source name is empty then use `build-step-gcs-source` name
Expand Down
41 changes: 25 additions & 16 deletions pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ func TestMakePod(t *testing.T) {
desc: "source",
b: v1alpha1.BuildSpec{
Source: &v1alpha1.SourceSpec{
Name: "myrepo",
Git: &v1alpha1.GitSourceSpec{
Url: "github.com/my/repo",
Revision: "master",
Expand All @@ -131,9 +132,9 @@ func TestMakePod(t *testing.T) {
VolumeMounts: implicitVolumeMounts,
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gitSource + "-0",
Name: initContainerPrefix + gitSource + "-myrepo",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo", "-revision", "master"},
Args: []string{"-url", "github.com/my/repo", "-revision", "master", "-path", "myrepo"},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts,
WorkingDir: workspaceDir,
Expand Down Expand Up @@ -178,16 +179,22 @@ func TestMakePod(t *testing.T) {
VolumeMounts: implicitVolumeMounts,
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gitSource + "-" + "repo1",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo", "-revision", "master"},
Name: initContainerPrefix + gitSource + "-" + "repo1",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo",
"-revision", "master",
"-path", "repo1",
},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts,
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gitSource + "-" + "repo2",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo", "-revision", "master"},
Name: initContainerPrefix + gitSource + "-" + "repo2",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo",
"-revision", "master",
"-path", "repo2",
},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts,
WorkingDir: workspaceDir,
Expand All @@ -205,6 +212,7 @@ func TestMakePod(t *testing.T) {
desc: "git-source-with-subpath",
b: v1alpha1.BuildSpec{
Source: &v1alpha1.SourceSpec{
Name: "myrepo",
Git: &v1alpha1.GitSourceSpec{
Url: "github.com/my/repo",
Revision: "master",
Expand All @@ -226,9 +234,9 @@ func TestMakePod(t *testing.T) {
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gitSource + "-0",
Name: initContainerPrefix + gitSource + "-myrepo",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo", "-revision", "master"},
Args: []string{"-url", "github.com/my/repo", "-revision", "master", "-path", "myrepo"},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
Expand Down Expand Up @@ -275,16 +283,16 @@ func TestMakePod(t *testing.T) {
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gitSource + "-" + "myrepo",
Name: initContainerPrefix + gitSource + "-myrepo",
Image: *gitImage,
Args: []string{"-url", "github.com/my/repo", "-revision", "master"},
Args: []string{"-url", "github.com/my/repo", "-revision", "master", "-path", "myrepo"},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gitSource + "-" + "ownrepo",
Image: *gitImage,
Args: []string{"-url", "github.com/own/repo", "-revision", "master"},
Args: []string{"-url", "github.com/own/repo", "-revision", "master", "-path", "ownrepo"},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
Expand Down Expand Up @@ -325,7 +333,7 @@ func TestMakePod(t *testing.T) {
}, {
Name: initContainerPrefix + gcsSource + "-0",
Image: *gcsFetcherImage,
Args: []string{"--type", "Manifest", "--location", "gs://foo/bar"},
Args: []string{"--type", "Manifest", "--location", "gs://foo/bar", "--dest_dir", "/workspace"},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
Expand All @@ -343,6 +351,7 @@ func TestMakePod(t *testing.T) {
desc: "gcs-source-with-targetPath",
b: v1alpha1.BuildSpec{
Source: &v1alpha1.SourceSpec{
Name: "gcs-foo-bar",
GCS: &v1alpha1.GCSSourceSpec{
Type: v1alpha1.GCSManifest,
Location: "gs://foo/bar",
Expand All @@ -360,9 +369,9 @@ func TestMakePod(t *testing.T) {
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
}, {
Name: initContainerPrefix + gcsSource + "-0",
Name: initContainerPrefix + gcsSource + "-gcs-foo-bar",
Image: *gcsFetcherImage,
Args: []string{"--type", "Manifest", "--location", "gs://foo/bar", "--dest_dir", "/workspace/path/foo"},
Args: []string{"--type", "Manifest", "--location", "gs://foo/bar", "--dest_dir", "/workspace/gcs-foo-bar/path/foo"},
Env: implicitEnvVars,
VolumeMounts: implicitVolumeMounts, // without subpath
WorkingDir: workspaceDir,
Expand Down
Loading

0 comments on commit 8557df4

Please sign in to comment.