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

Update resource path to be prefixed with resource name #393

Merged
merged 1 commit into from
Jan 17, 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
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.
Copy link
Member

Choose a reason for hiding this comment

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

In the different examples here, my understanding is that the resource will be copied to the PVC in the path /pvc/task_name/resource_name in all of the cases, if this is correct why do we need 3 examples?

Copy link
Contributor Author

@shashwathi shashwathi Jan 16, 2019

Choose a reason for hiding this comment

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

We need all the examples to show that source path when copied to PVC can change depending on

  1. resource only in output
  2. resource in input and output (no target path)
  3. resource in input and output( with target path)


```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