From 61e9729b66d2a5c947e10931ddb054a24234d058 Mon Sep 17 00:00:00 2001 From: Christie Wilson Date: Mon, 19 Nov 2018 11:37:01 -0800 Subject: [PATCH] Allow Pipelines to be used with different Resources This change makes it possible to reuse a Pipeline with different Resources without having to change the Pipeline itself. It does this by moving the linking of the Resources a Task requires from the Pipeline into the PipelineRun. The relationship between Resources and the Tasks that are expected to be executed on them is still expressed in the Pipeline (via `providedBy`). ResourceBindings move from Pipeline to PipelineRun and become ResourceDependencies. No longer calling these "bindings" in the API or using the term "source" fixes + the additional docs in using.md fixes #139. The most difficult part of this change was updating validatePipelineTaskAndTask - hoping to refactor this in a follow up (which would also address #213). This interface is still probably not in its final form and hopefully we can iterate on it! Fixes #200 --- docs/using.md | 46 ++++- examples/README.md | 45 +++-- .../invocations/guestbook-pipeline-run.yaml | 69 +++++++ examples/invocations/kritis-pipeline-run.yaml | 31 +++ examples/pipelines/guestbook.yaml | 64 +----- examples/pipelines/kritis.yaml | 38 +--- pkg/apis/pipeline/v1alpha1/pipeline_types.go | 28 +-- .../pipeline/v1alpha1/pipeline_validation.go | 12 +- .../v1alpha1/pipeline_validation_test.go | 63 +++--- .../pipeline/v1alpha1/pipelinerun_types.go | 44 +++- pkg/apis/pipeline/v1alpha1/task_types.go | 6 +- .../v1alpha1/zz_generated.deepcopy.go | 108 +++++++--- .../v1alpha1/pipeline/resources/dag.go | 4 +- .../v1alpha1/pipeline/resources/dag_test.go | 32 +-- .../v1alpha1/pipelinerun/pipelinerun.go | 12 +- .../v1alpha1/pipelinerun/pipelinerun_test.go | 58 +++--- .../pipelinerun/resources/pipelinestate.go | 6 +- .../pipelinerun/resources/providedby_test.go | 13 +- .../v1alpha1/pipelinerun/validate.go | 90 +++++---- .../v1alpha1/pipelinerun/validate_test.go | 189 +++++++++++++----- test/helm_task_test.go | 30 ++- 21 files changed, 607 insertions(+), 381 deletions(-) create mode 100644 examples/invocations/guestbook-pipeline-run.yaml diff --git a/docs/using.md b/docs/using.md index a5d50a0f45a..17ba5a40e6c 100644 --- a/docs/using.md +++ b/docs/using.md @@ -3,7 +3,7 @@ * [How do I create a new Pipeline?](#creating-a-pipeline) * [How do I make a Task?](#creating-a-task) * [How do I make Resources?](#creating-resources) -* [How do I run a Pipeline?](#running-a-task) +* [How do I run a Pipeline?](#running-a-pipeline) * [How do I run a Task on its own?](#running-a-task) * [How do I troubleshoot a PipelineRun?](#troubleshooting) @@ -136,10 +136,46 @@ ${inputs.params.NAME} ## Running a Pipeline -1. To run your `Pipeline`, create a new `PipelineRun` which links your `Pipeline` to the - `PipelineParams` it will run with. -2. Creation of a `PipelineRun` will trigger the creation of [`TaskRuns`](#running-a-task) - for each `Task` in your pipeline. +In order to run a Pipeline, you will need to provide: + +1. A Pipeline to run (see [creating a Pipeline](#creating-a-pipeline)) +2. `PipelineParams` which specify the service account to use and where to put logs (see [the example `PipelineParams`](examples/pipelineparams.yaml)) +3. The `PipelineResources` to use with this Pipeline. + +On its own, a `Pipeline` declares what `Tasks` to run, and what order to run them in +(implied by [`providedBy`](#providedby)). When running a `Pipeline`, you will +need to specify the `PipelineResources` to use with it. One `Pipeline` may need to be run +with different `PipelineResources` in cases such as: + +* When triggering the run of a `Pipeline` against a pull request, the triggering system must + specify the commitish of a git `PipelineResource` to use +* When invoking a `Pipeline` manually against one's own setup, one will need to ensure that + one's own github fork (via the git `PipelineResource`), image registry (via the image + `PipelineResource`) and kubernetes cluster (via the cluster `PipelineResource`). + +Specify the `PipelineResources` in the PipelineRun using the `resources` section, for example: + +```yaml +resources: +- name: push-kritis + inputs: + - key: workspace + resourceRef: + name: kritis-resources-git + outputs: + - key: builtImage + resourceRef: + name: kritis-resources-image +``` + +This example section says: + +* For the `Task` in the `Pipeline` called `push-kritis` +* For the input called `workspace`, use the existing resource called `kritis-resources-git` +* For the output called `builtImage`, use the existing resource called `kritis-resources-image` + +Creation of a `PipelineRun` will trigger the creation of [`TaskRuns`](#running-a-task) +for each `Task` in your pipeline. See [the example PipelineRun](../examples/invocations/kritis-pipeline-run.yaml). diff --git a/examples/README.md b/examples/README.md index 01ff92979ce..9043323e860 100644 --- a/examples/README.md +++ b/examples/README.md @@ -15,24 +15,28 @@ kubectl apply -f examples/invocations We have 2 example [Pipelines](../README.md#pipeline) in [./pipelines](./pipelines) -1. [The Kritis Pipline](./pipelines/kritis.yaml): This example builds a Pipeline for the - [kritis project](https://github.com/grafeas/kritis), and demonstrates how to configure - a pipeline which: +### Kritis Pipeline - 1. Runs unit tests - 2. Build an image - 3. Deploys it to a test environment - 4. Runs integration tests +[The Kritis Pipline](./pipelines/kritis.yaml): This example builds a Pipeline for the +[kritis project](https://github.com/grafeas/kritis), and demonstrates how to configure +a pipeline which: - ![Pipeline Configuration](./pipelines/kritis-pipeline.png) +1. Runs unit tests +2. Build an image +3. Deploys it to a test environment +4. Runs integration tests -2. [Guestbook](./pipelines/guestbook.yaml): This Pipeline is based on example application in - [the Kubernetes example Repo](https://github.com/kubernetes/examples/tree/master/guestbook) - This pipeline demonstartes how to integrate frontend - [guestbook app code](https://github.com/kubernetes/examples/tree/master/guestbook-go) with - backend [redis-docker image](https://github.com/GoogleCloudPlatform/redis-docker/tree/master/4) provided by GCP. +![Pipeline Configuration](./pipelines/kritis-pipeline.png) - ![Pipeline Configuration](./pipelines/guestbook-pipeline.png) +### Guestbook Pipeline + +[Guestbook](./pipelines/guestbook.yaml): This Pipeline is based on example application in +[the Kubernetes example Repo](https://github.com/kubernetes/examples/tree/master/guestbook) +This pipeline demonstartes how to integrate frontend +[guestbook app code](https://github.com/kubernetes/examples/tree/master/guestbook-go) with +backend [redis-docker image](https://github.com/GoogleCloudPlatform/redis-docker/tree/master/4) provided by GCP. + +![Pipeline Configuration](./pipelines/guestbook-pipeline.png) ## Example Tasks @@ -52,14 +56,11 @@ This task then calls `docker run` which will run the test code. This follows the ### Example Runs -The [invocations](./invocations/) directory contains an example [TaskRun](../docs/Concepts.md#taskrun) and an example [PipelineRun](../docs/Concepts.md#pipelinerun). +The [invocations](./invocations/) directory contains an example [TaskRun](../docs/Concepts.md#taskrun) and two example [PipelineRuns](../docs/Concepts.md#pipelinerun). [run-kritis-test.yaml](./invocations/run-kritis-test.yaml) shows an example of how to manually run kritis unit test off your development branch. -[kritis-pipeline-run.yaml](./invocations/kritis-pipeline-run.yaml) shows an example of -what it would look like to invoke the [kritis example pipeline](#example-pipelines) -manually. In the `conditions` field for type `Successful` you can see that the status -is `False`, which indicates that the Pipeline was not run successfully. The field -`message` contains a human readable error indicating that one of the `TaskRuns` failed. -This `condition` (and everything else in the `status` section) would be populated by the -controller as it realized the PipelineRun (i.e. ran the Pipeline). +The example `PipelineRuns are`: + +* [kritis-pipeline-run.yaml](./invocations/kritis-pipeline-run.yaml) invokes [the kritis example pipeline](#kritis-pipeline) +* [guestbook-pipeline-run.yaml](./invocations/guestbook-pipeline-run.yaml) invokes [the guestbook example pipeline](#guestbook-pipeline) diff --git a/examples/invocations/guestbook-pipeline-run.yaml b/examples/invocations/guestbook-pipeline-run.yaml new file mode 100644 index 00000000000..ddc2fe46dfb --- /dev/null +++ b/examples/invocations/guestbook-pipeline-run.yaml @@ -0,0 +1,69 @@ +apiVersion: pipeline.knative.dev/v1alpha1 +kind: PipelineRun +metadata: + name: guestbook-pipeline-run-12321312984 + namespace: default +spec: + pipelineRef: + name: guestbook-example + pipelineParamsRef: + name: pipelineparams-sample + triggerRef: + type: manual + resources: + - name: build-guestbook + inputs: + - name: workspace + resourceRef: + name: guestbook-resources-git + outputs: + - name: builtImage + resourceRef: + name: guestbookstagingimage + - name: build-redis + inputs: + - name: workspace + resourceRef: + name: guestbook-resources-redis-docker + outputs: + - name: builtImage + resourceRef: + name: redisstagingimage + - name: deploy-bundle-test + inputs: + - name: imageToDeploy1 + resourceRef: + name: redisstagingimage + - name: imageToDeploy2 + resourceRef: + name: guestbookstagingimage + - name: workspace + resourceRef: + name: guestbook-resources-redis-docker + - name: testCluster + resourceRef: + name: testcluster + - name: int-test-osx + inputs: + - name: workspace + resourceRef: + name: guestbook-resources-git + - name: int-test-linux + inputs: + - name: workspace + resourceRef: + name: guestbook-resources-git + - name: deploy-bundle-test + inputs: + - name: redisImage + resourceRef: + name: redisstagingimage + - name: guestbookImage + resourceRef: + name: guestbookstagingimage + - name: workspace + resourceRef: + name: guestbook-resources-redis-docker + - name: testCluster + resourceRef: + name: testcluster diff --git a/examples/invocations/kritis-pipeline-run.yaml b/examples/invocations/kritis-pipeline-run.yaml index ffc6e4c6412..00e92d55389 100644 --- a/examples/invocations/kritis-pipeline-run.yaml +++ b/examples/invocations/kritis-pipeline-run.yaml @@ -10,3 +10,34 @@ spec: name: pipelineparams-sample triggerRef: type: manual + resources: + - name: unit-test-kritis + inputs: + - key: workspace + resourceRef: + name: kritis-resources-git + - name: push-kritis + inputs: + - key: workspace + resourceRef: + name: kritis-resources-git + outputs: + - key: builtImage + resourceRef: + name: kritis-resources-image + - name: deploy-test-env + inputs: + - name: workspace + resourceRef: + name: kritis-resources-git + - name: builtImage + resourceRef: + name: kritis-resources-image + - name: testCluster + resourceRef: + name: kritistestcluster + - name: integration-test + inputs: + - name: workspace + resourceRef: + name: kritis-resources-git diff --git a/examples/pipelines/guestbook.yaml b/examples/pipelines/guestbook.yaml index 3175b05eaae..a6e234f6cba 100644 --- a/examples/pipelines/guestbook.yaml +++ b/examples/pipelines/guestbook.yaml @@ -5,103 +5,61 @@ metadata: namespace: default spec: tasks: - - name: build-guestbook # 1.a Build guestbook go sample code. + - name: build-guestbook taskRef: name: build-push - inputSourceBindings: - - name: workspace - resourceRef: - name: guestbook-resources-git - outputSourceBindings: - - name: builtImage - resourceRef: - name: guestbookstagingimage params: - name: pathToDockerfile value: guestbook-go/Dockerfile - - name: build-redis # 1.b Build and push redis docker image. + - name: build-redis taskRef: name: build-push - inputSourceBindings: - - name: workspace - resourceRef: - name: guestbook-resources-redis-docker - outputSourceBindings: - - name: builtImage - resourceRef: - name: redisstagingimage params: - name: pathToDockerfile value: 4/debian9/4.0/Dockerfile - - name: deploy-bundle-test # 2. Deploy GuestBook and Redis to test cluster + - name: deploy-bundle-test taskRef: name: deploy-with-kubectl - inputSourceBindings: + resources: - name: imageToDeploy1 - resourceRef: - name: redisstagingimage providedBy: - build-redis - name: imageToDeploy2 - resourceRef: - name: guestbookstagingimage providedBy: - - build-guestbook - - name: workspace - resourceRef: - name: guestbook-resources-redis-docker - - name: testCluster - resourceRef: - name: testcluster + - build-guestbook params: - name: pathToFiles value: guestbook/all-in-one/guestbook-all-in-one.yaml - - name: int-test-osx # 3.a Run Integration tests for osx - taskRef: - name: integration-test-in-docker - inputSourceBindings: + - name: int-test-osx + resources: - name: workspace - resourceRef: - name: guestbook-resources-git providedBy: - deploy-bundle-test params: - name: dockerBuildFile value: guestbook-int/Dockerfile - - name: int-test-linux # 3.b Run Integration tests for linux + - name: int-test-linux taskRef: name: integration-test-in-docker - inputSourceBindings: + resources: - name: workspace - resourceRef: - name: guestbook-resources-git providedBy: - deploy-bundle-test params: - name: dockerBuildFile value: guestbook-int/Dockerfile - - name: deploy-bundle-test # 4. Deploy GuestBook and Redis to staging cluster + - name: deploy-bundle-test taskRef: name: deploy-with-kubectl - inputSourceBindings: + resources: - name: redisImage - resourceRef: - name: redisstagingimage providedBy: - int-test-osx - int-test-linux - name: guestbookImage - resourceRef: - name: guestbookstagingimage providedBy: - int-test-osx - int-test-linux - - name: workspace - resourceRef: - name: guestbook-resources-redis-docker - - name: testCluster - resourceRef: - name: testcluster params: - name: pathToFiles value: guestbook/all-in-one/guestbook-all-in-one.yaml \ No newline at end of file diff --git a/examples/pipelines/kritis.yaml b/examples/pipelines/kritis.yaml index fc66b7d629d..b916d9b5493 100644 --- a/examples/pipelines/kritis.yaml +++ b/examples/pipelines/kritis.yaml @@ -5,55 +5,35 @@ metadata: namespace: default spec: tasks: - - name: unit-test-kritis # 1. Run unit Tests + - name: unit-test-kritis taskRef: name: make - inputSourceBindings: - - name: workspace - resourceRef: - name: kritis-resources-git params: - name: makeTarget value: test - - name: push-kritis # 2. Build And Push Tests + - name: push-kritis taskRef: name: build-push - inputSourceBindings: - - name: workspace - resourceRef: - name: kritis-resources-git - providedBy: [unit-test-kritis] - outputSourceBindings: - - name: builtImage - resourceRef: - name: kritis-resources-image + resources: + - name: workspace + providedBy: [unit-test-kritis] params: - name: pathToDockerfile value: deploy/Dockerfile - - name: deploy-test-env # 3. Finally Deploy to Test environment + - name: deploy-test-env taskRef: name: deploy-with-helm - inputSourceBindings: - - name: workspace - resourceRef: - name: kritis-resources-git + resources: - name: builtImage - resourceRef: - name: kritis-resources-image providedBy: [push-kritis] - - name: testCluster - resourceRef: - name: kritistestcluster params: - name: pathToHelmCharts value: kritis-charts - - name: integration-test # 4. Run Integration Tests in test cluster + - name: integration-test taskRef: name: integration-test-in-docker - inputSourceBindings: + resources: - name: workspace - resourceRef: - name: kritis-resources-git providedBy: [deploy-test-env] params: - name: testArgs diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_types.go b/pkg/apis/pipeline/v1alpha1/pipeline_types.go index 6222de6d5dd..a8c8a421ab4 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_types.go @@ -27,10 +27,10 @@ type PipelineSpec struct { Generation int64 `json:"generation,omitempty"` } -// PipelineStatus defines the observed state of Pipeline +// PipelineStatus does not contain anything because Pipelines on their own +// do not have a status, they just hold data which is later used by a +// PipelineRun. type PipelineStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file } // Check that Pipeline may be validated and defaulted. @@ -64,9 +64,7 @@ type PipelineTask struct { Name string `json:"name"` TaskRef TaskRef `json:"taskRef"` // +optional - InputSourceBindings []SourceBinding `json:"inputSourceBindings,omitempty"` - // +optional - OutputSourceBindings []SourceBinding `json:"outputSourceBindings,omitempty"` + ResourceDependencies []ResourceDependency `json:"resources,omitempty"` // +optional Params []Param `json:"params,omitempty"` } @@ -77,13 +75,12 @@ type PipelineTaskParam struct { Value string `json:"value"` } -// SourceBinding is used to bind a Source from a PipelineParams to a source required -// as an input for a task. -type SourceBinding struct { +// ResourceDependency is used when a PipelineResource required by a Task is requird to be provided by +// a previous Task, i.e. that Task needs to operate on the PipelineResource before this Task can be +// executed. It is from this dependency that the Pipeline's DAG is constructed. +type ResourceDependency struct { // Name is the name of the Task's input that this Resource should be used for. Name string `json:"name"` - // The Resource this binding is referring to - ResourceRef PipelineResourceRef `json:"resourceRef"` // ProvidedBy is the list of Task names that the resource has to come from. // +optional ProvidedBy []string `json:"providedBy,omitempty"` @@ -99,15 +96,6 @@ type TaskRef struct { APIVersion string `json:"apiVersion,omitempty"` } -// PipelineResourceRef can be used to refer to a specific instance of a Resource -type PipelineResourceRef struct { - // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names - Name string `json:"name"` - // API version of the referent - // +optional - APIVersion string `json:"apiVersion,omitempty"` -} - // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // PipelineList contains a list of Pipeline diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go index 57c3a0d7283..b13d9dc650a 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go @@ -23,6 +23,8 @@ import ( "k8s.io/apimachinery/pkg/api/equality" ) +// Validate checks that the Pipeline structure is valid but does not validate +// that any references resources exist, that is done at run time. func (p *Pipeline) Validate() *apis.FieldError { if err := validateObjectMetadata(p.GetObjectMeta()); err != nil { return err.ViaField("metadata") @@ -30,6 +32,8 @@ func (p *Pipeline) Validate() *apis.FieldError { return nil } +// Validate checks that taskNames in the Pipeline are valid and that the graph +// of Tasks expressed in the Pipeline makes sense. func (ps *PipelineSpec) Validate() *apis.FieldError { if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) { return apis.ErrMissingField(apis.CurrentField) @@ -46,10 +50,10 @@ func (ps *PipelineSpec) Validate() *apis.FieldError { // providedBy should match other tasks. for _, t := range ps.Tasks { - for _, isb := range t.InputSourceBindings { - for _, pc := range isb.ProvidedBy { - if _, ok := taskNames[pc]; !ok { - return apis.ErrInvalidKeyName(pc, fmt.Sprintf("spec.tasks.inputSourceBindings.%s", pc)) + for _, rd := range t.ResourceDependencies { + for _, pb := range rd.ProvidedBy { + if _, ok := taskNames[pb]; !ok { + return apis.ErrInvalidKeyName(pb, fmt.Sprintf("spec.tasks.resources.%s", pb)) } } } diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go index 732db2ee06f..d7249dab921 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go @@ -32,29 +32,22 @@ func TestPipelineSpec_Validate_Error(t *testing.T) { { name: "duplicate tasks", fields: fields{ - Tasks: []PipelineTask{ - { - Name: "foo", - }, - { - Name: "foo", - }, - }, + Tasks: []PipelineTask{{ + Name: "foo", + }, { + Name: "foo", + }}, }, }, { name: "invalid constraint tasks", fields: fields{ - Tasks: []PipelineTask{ - { - Name: "foo", - InputSourceBindings: []SourceBinding{ - { - ProvidedBy: []string{"bar"}, - }, - }, - }, - }, + Tasks: []PipelineTask{{ + Name: "foo", + ResourceDependencies: []ResourceDependency{{ + ProvidedBy: []string{"bar"}, + }}, + }}, }, }, } @@ -83,32 +76,24 @@ func TestPipelineSpec_Validate_Valid(t *testing.T) { { name: "no duplicate tasks", fields: fields{ - Tasks: []PipelineTask{ - { - Name: "foo", - }, - { - Name: "baz", - }, - }, + Tasks: []PipelineTask{{ + Name: "foo", + }, { + Name: "baz", + }}, }, }, { name: "valid constraint tasks", fields: fields{ - Tasks: []PipelineTask{ - { - Name: "foo", - InputSourceBindings: []SourceBinding{ - { - ProvidedBy: []string{"bar"}, - }, - }, - }, - { - Name: "bar", - }, - }, + Tasks: []PipelineTask{{ + Name: "foo", + ResourceDependencies: []ResourceDependency{{ + ProvidedBy: []string{"bar"}, + }}, + }, { + Name: "bar", + }}, }, }, } diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index 181f8e20ebf..21fc7d694f1 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go @@ -28,10 +28,42 @@ var _ webhook.GenericCRD = (*TaskRun)(nil) // PipelineRunSpec defines the desired state of PipelineRun type PipelineRunSpec struct { - PipelineRef PipelineRef `json:"pipelineRef"` - PipelineParamsRef PipelineParamsRef `json:"pipelineParamsRef"` - PipelineTriggerRef PipelineTriggerRef `json:"triggerRef"` - Generation int64 `json:"generation,omitempty"` + PipelineRef PipelineRef `json:"pipelineRef"` + PipelineParamsRef PipelineParamsRef `json:"pipelineParamsRef"` + PipelineTriggerRef PipelineTriggerRef `json:"triggerRef"` + PipelineTaskResources []PipelineTaskResource `json:"resources"` + Generation int64 `json:"generation,omitempty"` +} + +// PipelineTaskResource maps Task inputs and outputs to existing PipelineResources by their names. +type PipelineTaskResource struct { + // Name is the name of the `PipelineTask` for which these PipelineResources are being provided. + Name string `json:"name"` + + // Inputs is a list containing mapping from the input Resources which the Task has declared it needs + // and the corresponding Resource instance in the system which should be used. + Inputs []TaskResourceBinding `json:"inputs"` + + // Outputs is a list containing mapping from the output Resources which the Task has declared it needs + // and the corresponding Resource instance in the system which should be used. + Outputs []TaskResourceBinding `json:"outputs"` +} + +// TaskResourceBinding is used to bind a PipelineResource to a PipelineResource required for a Task as an input or an output. +type TaskResourceBinding struct { + // Name is the name of the Task's input that this Resource should be used for. + Name string `json:"name"` + // The Resource that should be provided to the Task for the Resource it requires. + ResourceRef PipelineResourceRef `json:"resourceRef"` +} + +// PipelineResourceRef can be used to refer to a specific instance of a Resource +type PipelineResourceRef struct { + // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names + Name string `json:"name"` + // API version of the referent + // +optional + APIVersion string `json:"apiVersion,omitempty"` } // PipelineRef can be used to refer to a specific instance of a Pipeline. @@ -73,8 +105,8 @@ type PipelineTriggerRef struct { // PipelineRunStatus defines the observed state of PipelineRun type PipelineRunStatus struct { //+optional - TaskRuns []PipelineTaskRun `json:"taskRuns,omitempty"` - Conditions duckv1alpha1.Conditions `json:"conditions"` + TaskRuns []PipelineTaskRun `json:"taskRuns,omitempty"` + Conditions duckv1alpha1.Conditions `json:"conditions"` } var pipelineRunCondSet = duckv1alpha1.NewBatchConditionSet() diff --git a/pkg/apis/pipeline/v1alpha1/task_types.go b/pkg/apis/pipeline/v1alpha1/task_types.go index ca0e9362507..6d5e9935515 100644 --- a/pkg/apis/pipeline/v1alpha1/task_types.go +++ b/pkg/apis/pipeline/v1alpha1/task_types.go @@ -66,10 +66,10 @@ type TaskSpec struct { Affinity *corev1.Affinity `json:"affinity,omitempty"` } -// TaskStatus defines the observed state of Task +// TaskStatus does not contain anything because Tasks on their own +// do not have a status, they just hold data which is later used by a +// TaskRun. type TaskStatus struct { - // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster - // Important: Run "make" to regenerate code after modifying this file } // Check that Task may be validated and defaulted. diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 6be99e2fbc1..0b3011785e6 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -465,7 +465,7 @@ func (in *PipelineRun) DeepCopyInto(out *PipelineRun) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -527,6 +527,13 @@ func (in *PipelineRunSpec) DeepCopyInto(out *PipelineRunSpec) { out.PipelineRef = in.PipelineRef out.PipelineParamsRef = in.PipelineParamsRef out.PipelineTriggerRef = in.PipelineTriggerRef + if in.PipelineTaskResources != nil { + in, out := &in.PipelineTaskResources, &out.PipelineTaskResources + *out = make([]PipelineTaskResource, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -611,16 +618,9 @@ func (in *PipelineStatus) DeepCopy() *PipelineStatus { func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { *out = *in out.TaskRef = in.TaskRef - if in.InputSourceBindings != nil { - in, out := &in.InputSourceBindings, &out.InputSourceBindings - *out = make([]SourceBinding, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } - if in.OutputSourceBindings != nil { - in, out := &in.OutputSourceBindings, &out.OutputSourceBindings - *out = make([]SourceBinding, len(*in)) + if in.ResourceDependencies != nil { + in, out := &in.ResourceDependencies, &out.ResourceDependencies + *out = make([]ResourceDependency, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -659,6 +659,32 @@ func (in *PipelineTaskParam) DeepCopy() *PipelineTaskParam { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PipelineTaskResource) DeepCopyInto(out *PipelineTaskResource) { + *out = *in + if in.Inputs != nil { + in, out := &in.Inputs, &out.Inputs + *out = make([]TaskResourceBinding, len(*in)) + copy(*out, *in) + } + if in.Outputs != nil { + in, out := &in.Outputs, &out.Outputs + *out = make([]TaskResourceBinding, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineTaskResource. +func (in *PipelineTaskResource) DeepCopy() *PipelineTaskResource { + if in == nil { + return nil + } + out := new(PipelineTaskResource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineTaskRun) DeepCopyInto(out *PipelineTaskRun) { *out = *in @@ -691,6 +717,27 @@ func (in *PipelineTriggerRef) DeepCopy() *PipelineTriggerRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ResourceDependency) DeepCopyInto(out *ResourceDependency) { + *out = *in + if in.ProvidedBy != nil { + in, out := &in.ProvidedBy, &out.ProvidedBy + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ResourceDependency. +func (in *ResourceDependency) DeepCopy() *ResourceDependency { + if in == nil { + return nil + } + out := new(ResourceDependency) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResultTarget) DeepCopyInto(out *ResultTarget) { *out = *in @@ -750,28 +797,6 @@ func (in *SecretParam) DeepCopy() *SecretParam { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SourceBinding) DeepCopyInto(out *SourceBinding) { - *out = *in - out.ResourceRef = in.ResourceRef - if in.ProvidedBy != nil { - in, out := &in.ProvidedBy, &out.ProvidedBy - *out = make([]string, len(*in)) - copy(*out, *in) - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SourceBinding. -func (in *SourceBinding) DeepCopy() *SourceBinding { - if in == nil { - return nil - } - out := new(SourceBinding) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StepRun) DeepCopyInto(out *StepRun) { *out = *in @@ -897,6 +922,23 @@ func (in *TaskResource) DeepCopy() *TaskResource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *TaskResourceBinding) DeepCopyInto(out *TaskResourceBinding) { + *out = *in + out.ResourceRef = in.ResourceRef + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TaskResourceBinding. +func (in *TaskResourceBinding) DeepCopy() *TaskResourceBinding { + if in == nil { + return nil + } + out := new(TaskResourceBinding) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskRun) DeepCopyInto(out *TaskRun) { *out = *in diff --git a/pkg/reconciler/v1alpha1/pipeline/resources/dag.go b/pkg/reconciler/v1alpha1/pipeline/resources/dag.go index bb7f05d599f..76f027d1bfd 100644 --- a/pkg/reconciler/v1alpha1/pipeline/resources/dag.go +++ b/pkg/reconciler/v1alpha1/pipeline/resources/dag.go @@ -105,8 +105,8 @@ func Build(p *v1alpha1.Pipeline) (*DAG, error) { } // Process all providedBy constraints to add task dependency for _, pt := range p.Spec.Tasks { - for _, input := range pt.InputSourceBindings { - for _, constraint := range input.ProvidedBy { + for _, rd := range pt.ResourceDependencies { + for _, constraint := range rd.ProvidedBy { // We need to add dependency from constraint to node n prev, ok := d.Nodes[constraint] if !ok { diff --git a/pkg/reconciler/v1alpha1/pipeline/resources/dag_test.go b/pkg/reconciler/v1alpha1/pipeline/resources/dag_test.go index a46b5b83c1d..9a15072d136 100644 --- a/pkg/reconciler/v1alpha1/pipeline/resources/dag_test.go +++ b/pkg/reconciler/v1alpha1/pipeline/resources/dag_test.go @@ -29,28 +29,28 @@ func TestBuild(t *testing.T) { b := v1alpha1.PipelineTask{Name: "b"} c := v1alpha1.PipelineTask{Name: "c"} xDependsOnA := v1alpha1.PipelineTask{ - Name: "x", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"a"}}}, + Name: "x", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"a"}}}, } yDependsOnAB := v1alpha1.PipelineTask{ - Name: "y", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"b", "a"}}}, + Name: "y", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"b", "a"}}}, } zDependsOnX := v1alpha1.PipelineTask{ - Name: "z", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"x"}}}, + Name: "z", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"x"}}}, } aDependsOnZ := v1alpha1.PipelineTask{ - Name: "a", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"z"}}}, + Name: "a", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"z"}}}, } selfLink := v1alpha1.PipelineTask{ - Name: "a", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"a"}}}, + Name: "a", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"a"}}}, } invalidTask := v1alpha1.PipelineTask{ - Name: "a", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"none"}}}, + Name: "a", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"none"}}}, } nodeX := &Node{Task: xDependsOnA, Prev: []*Node{&Node{Task: a}}} @@ -135,12 +135,12 @@ func TestBuild(t *testing.T) { func TestGetPrevTasks(t *testing.T) { a := v1alpha1.PipelineTask{Name: "a"} x := v1alpha1.PipelineTask{ - Name: "x", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"a"}}}, + Name: "x", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"a"}}}, } y := v1alpha1.PipelineTask{ - Name: "y", - InputSourceBindings: []v1alpha1.SourceBinding{{ProvidedBy: []string{"x", "a"}}}, + Name: "y", + ResourceDependencies: []v1alpha1.ResourceDependency{{ProvidedBy: []string{"x", "a"}}}, } p := v1alpha1.Pipeline{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 3cfa1884ed4..7e2ffadf8d5 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -241,18 +241,22 @@ func (c *Reconciler) createTaskRun(t *v1alpha1.Task, trName string, pr *v1alpha1 ServiceAccount: sa, }, } - for _, isb := range pt.InputSourceBindings { + var resources v1alpha1.PipelineTaskResource + for _, ptr := range pr.Spec.PipelineTaskResources { + if ptr.Name == pt.Name { + resources = ptr + } + } + for _, isb := range resources.Inputs { tr.Spec.Inputs.Resources = append(tr.Spec.Inputs.Resources, v1alpha1.TaskRunResourceVersion{ ResourceRef: isb.ResourceRef, Name: isb.Name, - // TODO(#148) apply the correct version }) } - for _, osb := range pt.OutputSourceBindings { + for _, osb := range resources.Outputs { tr.Spec.Outputs.Resources = append(tr.Spec.Outputs.Resources, v1alpha1.TaskRunResourceVersion{ ResourceRef: osb.ResourceRef, Name: osb.Name, - // TODO(#148) apply the correct version }) } return c.PipelineClientSet.PipelineV1alpha1().TaskRuns(t.Namespace).Create(tr) diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 4801b1edab8..d5b4b44eb6a 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -46,6 +46,21 @@ func TestReconcile(t *testing.T) { PipelineParamsRef: v1alpha1.PipelineParamsRef{ Name: "unit-test-pp", }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + Inputs: []v1alpha1.TaskResourceBinding{{ + Name: "workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "some-repo", + }, + }}, + Outputs: []v1alpha1.TaskResourceBinding{{ + Name: "image-to-use", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "some-image", + }, + }}, + }}, }, }} ps := []*v1alpha1.Pipeline{{ @@ -54,37 +69,20 @@ func TestReconcile(t *testing.T) { Namespace: "foo", }, Spec: v1alpha1.PipelineSpec{ - Tasks: []v1alpha1.PipelineTask{ - { - Name: "unit-test-1", - TaskRef: v1alpha1.TaskRef{Name: "unit-test-task"}, - Params: []v1alpha1.Param{ - { - Name: "foo", - Value: "somethingfun", - }, - { - Name: "bar", - Value: "somethingmorefun", - }, - { - Name: "templatedparam", - Value: "${inputs.workspace.revision}", - }, - }, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "some-repo", - }, - }}, - OutputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "image-to-use", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "some-image", - }, - }}, + Tasks: []v1alpha1.PipelineTask{{ + Name: "unit-test-1", + TaskRef: v1alpha1.TaskRef{Name: "unit-test-task"}, + Params: []v1alpha1.Param{{ + Name: "foo", + Value: "somethingfun", + }, { + Name: "bar", + Value: "somethingmorefun", + }, { + Name: "templatedparam", + Value: "${inputs.workspace.revision}", }}, + }}, }}, } ts := []*v1alpha1.Task{{ diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinestate.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinestate.go index 7910ce6e54b..59223397447 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinestate.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinestate.go @@ -69,9 +69,9 @@ func GetNextTask(prName string, state []*PipelineRunTaskRun, logger *zap.Sugared func canTaskRun(pt *v1alpha1.PipelineTask, state []*PipelineRunTaskRun) bool { // Check if Task can run now. Go through all the input constraints - for _, input := range pt.InputSourceBindings { - if len(input.ProvidedBy) > 0 { - for _, constrainingTaskName := range input.ProvidedBy { + for _, rd := range pt.ResourceDependencies { + if len(rd.ProvidedBy) > 0 { + for _, constrainingTaskName := range rd.ProvidedBy { for _, prtr := range state { // the constraining task must have a successful task run to allow this task to run if prtr.PipelineTask.Name == constrainingTaskName { diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/providedby_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/providedby_test.go index 1ba7ac7215d..30e4891a912 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/providedby_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/providedby_test.go @@ -62,20 +62,11 @@ var mytask2 = &v1alpha1.Task{ var mypipelinetasks = []v1alpha1.PipelineTask{{ Name: "mypipelinetask1", TaskRef: v1alpha1.TaskRef{Name: "mytask1"}, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "myresource1", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "myresource1", - }, - }}, }, { Name: "mypipelinetask2", TaskRef: v1alpha1.TaskRef{Name: "mytask2"}, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "myresource1", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "myresource1", - }, + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "myresource1", ProvidedBy: []string{"mypipelinetask1"}, }}, }} diff --git a/pkg/reconciler/v1alpha1/pipelinerun/validate.go b/pkg/reconciler/v1alpha1/pipelinerun/validate.go index 4b896088983..33b5035cf39 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/validate.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/validate.go @@ -28,51 +28,54 @@ func validatePipelineRun(c *Reconciler, pr *v1alpha1.PipelineRun) (*v1alpha1.Pip // verify pipeline reference and all corresponding tasks exist p, err := c.pipelineLister.Pipelines(pr.Namespace).Get(pr.Spec.PipelineRef.Name) if err != nil { - return nil, sa, fmt.Errorf("Error listing pipeline ref %s: %v", pr.Spec.PipelineRef.Name, err) + return nil, sa, fmt.Errorf("error listing pipeline ref %s: %v", pr.Spec.PipelineRef.Name, err) } - if err := validatePipelineTask(c, p.Spec.Tasks, pr.Namespace); err != nil { - return nil, sa, fmt.Errorf("Error validating tasks in pipeline %s: %v", p.Name, err) + if err := validatePipelineTask(c, p.Spec.Tasks, pr.Spec.PipelineTaskResources, pr.Namespace); err != nil { + return nil, sa, fmt.Errorf("error validating tasks in pipeline %s: %v", p.Name, err) } if pr.Spec.PipelineParamsRef.Name != "" { pp, err := c.pipelineParamsLister.PipelineParamses(pr.Namespace).Get(pr.Spec.PipelineParamsRef.Name) if err != nil { - return nil, "", fmt.Errorf("Error listing pipelineparams %s for pipelinerun %s: %v", pr.Spec.PipelineParamsRef.Name, pr.Name, err) + return nil, "", fmt.Errorf("error listing pipelineparams %s for pipelinerun %s: %v", pr.Spec.PipelineParamsRef.Name, pr.Name, err) } sa = pp.Spec.ServiceAccount } return p, sa, nil } -//validatePipelineTaskAndTask validates pipelinetask inputs, params and output matches task -func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, task *v1alpha1.Task, ns string) error { - // stores all the input keys to validate with task input name - inputMapping := map[string]string{} - // stores all the output keys to validate with task output name - outMapping := map[string]string{} - // stores params to validate with task params +func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, task *v1alpha1.Task, inputs []v1alpha1.TaskResourceBinding, outputs []v1alpha1.TaskResourceBinding, ns string) error { + // a map from the name of the input resource to the type + inputMapping := map[string]v1alpha1.PipelineResourceType{} + // a map from the name of the output resource to the type + outputMapping := map[string]v1alpha1.PipelineResourceType{} + // a map that can be used to lookup if params have been specified paramsMapping := map[string]string{} for _, param := range ptask.Params { paramsMapping[param.Name] = "" } - for _, source := range ptask.InputSourceBindings { - inputMapping[source.Name] = "" - if source.ResourceRef.Name != "" { - rr, err := c.resourceLister.PipelineResources(ns).Get(source.ResourceRef.Name) + for _, r := range inputs { + inputMapping[r.Name] = "" + // TODO(#213): if this is the empty string should it be an error? or maybe let the lookup fail? + if r.ResourceRef.Name != "" { + rr, err := c.resourceLister.PipelineResources(ns).Get(r.ResourceRef.Name) if err != nil { - return fmt.Errorf("Error listing input pipeline resource for task %s: %v ", ptask.Name, err) + return fmt.Errorf("error listing input pipeline resource for task %s: %v ", ptask.Name, err) } - inputMapping[source.Name] = string(rr.Spec.Type) + inputMapping[r.Name] = rr.Spec.Type } } - for _, source := range ptask.OutputSourceBindings { - outMapping[source.Name] = "" - if source.ResourceRef.Name != "" { - if _, err := c.resourceLister.PipelineResources(ns).Get(source.ResourceRef.Name); err != nil { - return fmt.Errorf("Error listing output pipeline resource for task %s: %v ", ptask.Name, err) + for _, r := range outputs { + outputMapping[r.Name] = "" + // TODO(#213): if this is the empty string should it be an error? or maybe let the lookup fail? + if r.ResourceRef.Name != "" { + rr, err := c.resourceLister.PipelineResources(ns).Get(r.ResourceRef.Name) + if err != nil { + return fmt.Errorf("error listing output pipeline resource for task %s: %v ", ptask.Name, err) } + outputMapping[r.Name] = rr.Spec.Type } } @@ -80,38 +83,55 @@ func validatePipelineTaskAndTask(c *Reconciler, ptask v1alpha1.PipelineTask, tas for _, inputResource := range task.Spec.Inputs.Resources { inputResourceType, ok := inputMapping[inputResource.Name] if !ok { - return fmt.Errorf("Mismatch of input key %q between pipeline task %q and task %q", inputResource.Name, ptask.Name, task.Name) + return fmt.Errorf("resource %q not provided for pipeline task %q (task %q)", inputResource.Name, ptask.Name, task.Name) } // Validate the type of resource match - if string(inputResource.Type) != inputResourceType { - return fmt.Errorf("Mismatch of input resource type %q between pipeline task %q and task %q", inputResourceType, ptask.Name, task.Name) + if inputResource.Type != inputResourceType { + return fmt.Errorf("resource %q for pipeline task %q (task %q) should be type %q but was %q", inputResource.Name, ptask.Name, task.Name, inputResourceType, inputResource.Type) } } for _, inputResourceParam := range task.Spec.Inputs.Params { if _, ok := paramsMapping[inputResourceParam.Name]; !ok { - return fmt.Errorf("Mismatch of input params %q between pipeline task %q and task %q", inputResourceParam.Name, ptask.Name, task.Name) + return fmt.Errorf("input param %q not provided for pipeline task %q (task %q)", inputResourceParam.Name, ptask.Name, task.Name) } } } if task.Spec.Outputs != nil { for _, outputResource := range task.Spec.Outputs.Resources { - if _, ok := outMapping[outputResource.Name]; !ok { - return fmt.Errorf("Mismatch of output key %q between task %q and task ref %q", outputResource.Name, ptask.Name, task.Name) + outputResourceType, ok := outputMapping[outputResource.Name] + if !ok { + return fmt.Errorf("resource %q not provided for pipeline task %q (task %q)", outputResource.Name, ptask.Name, task.Name) + } + if outputResource.Type != outputResourceType { + return fmt.Errorf("resource %q for pipeline task %q (task %q) should be type %q but was %q", outputResource.Name, ptask.Name, task.Name, outputResourceType, outputResource.Type) } } } return nil } -func validatePipelineTask(c *Reconciler, tasks []v1alpha1.PipelineTask, ns string) error { - for _, t := range tasks { - tr, terr := c.taskLister.Tasks(ns).Get(t.TaskRef.Name) - if terr != nil { - return terr +func validatePipelineTask(c *Reconciler, tasks []v1alpha1.PipelineTask, resources []v1alpha1.PipelineTaskResource, ns string) error { + for _, pt := range tasks { + t, err := c.taskLister.Tasks(ns).Get(pt.TaskRef.Name) + if err != nil { + return fmt.Errorf("couldn't get task %q: %s", pt.TaskRef.Name, err) + } + + found := false + for _, tr := range resources { + if tr.Name == pt.Name { + if err := validatePipelineTaskAndTask(c, pt, t, tr.Inputs, tr.Outputs, ns); err != nil { + return fmt.Errorf("pipelineTask %q is invalid: %s", pt.Name, err) + } + found = true + } } - if verr := validatePipelineTaskAndTask(c, t, tr, ns); verr != nil { - return verr + + if !found && + ((t.Spec.Inputs != nil && len(t.Spec.Inputs.Resources) > 0) || + (t.Spec.Outputs != nil && len(t.Spec.Outputs.Resources) > 0)) { + return fmt.Errorf("resources required for pipeline task %q (task %q) but were missing", pt.Name, t.Name) } } return nil diff --git a/pkg/reconciler/v1alpha1/pipelinerun/validate_test.go b/pkg/reconciler/v1alpha1/pipelinerun/validate_test.go index 1c8b2272432..0696a618597 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/validate_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/validate_test.go @@ -21,12 +21,6 @@ func Test_InvalidPipelineTask(t *testing.T) { Tasks: []v1alpha1.PipelineTask{{ Name: "unit-test-1", TaskRef: v1alpha1.TaskRef{Name: "unit-test-task"}, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "test-resource-name", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "non-existent-resource1", - }, - }}, }}, }, }, { @@ -38,12 +32,6 @@ func Test_InvalidPipelineTask(t *testing.T) { Tasks: []v1alpha1.PipelineTask{{ Name: "unit-test-1", TaskRef: v1alpha1.TaskRef{Name: "unit-test-task"}, - OutputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "test-resource-name", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: "non-existent-resource", - }, - }}, }}, }, }, { @@ -55,9 +43,6 @@ func Test_InvalidPipelineTask(t *testing.T) { Tasks: []v1alpha1.PipelineTask{{ Name: "unit-test-1", TaskRef: v1alpha1.TaskRef{Name: "unit-task-wrong-input"}, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "non-existent", - }}, }}, }, }, { @@ -69,9 +54,6 @@ func Test_InvalidPipelineTask(t *testing.T) { Tasks: []v1alpha1.PipelineTask{{ Name: "unit-test-1", TaskRef: v1alpha1.TaskRef{Name: "unit-task-wrong-output"}, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "non-existent", - }}, }}, }, }, { @@ -98,7 +80,108 @@ func Test_InvalidPipelineTask(t *testing.T) { Tasks: []v1alpha1.PipelineTask{{ Name: "unit-test-1", TaskRef: v1alpha1.TaskRef{Name: "unit-task-bad-resourcetype"}, - InputSourceBindings: []v1alpha1.SourceBinding{{ + }}, + }, + }} + prs := []*v1alpha1.PipelineRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-1", + Namespace: "foo", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "test-pipeline-bad-inputbindings", + }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + Inputs: []v1alpha1.TaskResourceBinding{{ + // TODO(#213): these are currently covering both the case where the name of the + // resource is bad and the referred resource doesn't exist (the task has no inputs) + Name: "test-resource-name", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "non-existent-resource1", + }, + }}, + }}, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-2", + Namespace: "foo", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "test-pipeline-bad-outputbindings", + }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + Outputs: []v1alpha1.TaskResourceBinding{{ + // TODO(#213): these are currently covering both the case where the name of the + // resource is bad and the referred resource doesn't exist (the task has no outputs) + Name: "test-resource-name", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: "non-existent-resource1", + }, + }}, + }}, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-3", + Namespace: "foo", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "test-pipeline-bad-inputkey", + }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + Inputs: []v1alpha1.TaskResourceBinding{{ + Name: "non-existent", + }}, + }}, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-4", + Namespace: "foo", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "test-pipeline-bad-outputkey", + }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + Outputs: []v1alpha1.TaskResourceBinding{{ + Name: "non-existent", + }}, + }}, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-5", + Namespace: "foo", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "test-pipeline-param-mismatch", + }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + }}, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-6", + Namespace: "foo", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "test-pipeline-bad-resourcetype", + }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "unit-test-1", + Inputs: []v1alpha1.TaskResourceBinding{{ Name: "testimageinput", ResourceRef: v1alpha1.PipelineResourceRef{ Name: "git-test-resource", @@ -150,10 +233,10 @@ func Test_InvalidPipelineTask(t *testing.T) { Spec: v1alpha1.TaskSpec{ Inputs: &v1alpha1.Inputs{ Params: []v1alpha1.TaskParam{{ - Name: "foo", + Name: "foo", Description: "foo", }, { - Name: "bar", + Name: "bar", Description: "bar", }}, }, @@ -188,49 +271,45 @@ func Test_InvalidPipelineTask(t *testing.T) { }} tcs := []struct { - name string - pipeline *v1alpha1.Pipeline - reason string + name string + pipeline *v1alpha1.Pipeline + pipelineRun *v1alpha1.PipelineRun + reason string }{ { - name: "bad-input-source-bindings", - pipeline: ps[0], - reason: "input-source-binding-to-invalid-resource", + name: "bad-input-source-bindings", + pipeline: ps[0], + pipelineRun: prs[0], + reason: "input-source-binding-to-invalid-resource", }, { - name: "bad-output-source-bindings", - pipeline: ps[1], - reason: "output-source-binding-to-invalid-resource", + name: "bad-output-source-bindings", + pipeline: ps[1], + pipelineRun: prs[1], + reason: "output-source-binding-to-invalid-resource", }, { - name: "bad-inputkey", - pipeline: ps[2], - reason: "bad-input-mapping", + name: "bad-inputkey", + pipeline: ps[2], + pipelineRun: prs[2], + reason: "bad-input-mapping", }, { - name: "bad-outputkey", - pipeline: ps[3], - reason: "bad-output-mapping", + name: "bad-outputkey", + pipeline: ps[3], + pipelineRun: prs[3], + reason: "bad-output-mapping", }, { - name: "param-mismatch", - pipeline: ps[4], - reason: "input-param-mismatch", + name: "param-mismatch", + pipeline: ps[4], + pipelineRun: prs[4], + reason: "input-param-mismatch", }, { - name: "resource-mismatch", - pipeline: ps[5], - reason: "input-resource-mismatch", + name: "resource-mismatch", + pipeline: ps[5], + pipelineRun: prs[5], + reason: "input-resource-mismatch", }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - prs := []*v1alpha1.PipelineRun{{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pipeline-run", - Namespace: "foo", - }, - Spec: v1alpha1.PipelineRunSpec{ - PipelineRef: v1alpha1.PipelineRef{ - Name: tc.pipeline.Name, - }, - }, - }} d := test.Data{ PipelineRuns: prs, Pipelines: ps, @@ -239,12 +318,12 @@ func Test_InvalidPipelineTask(t *testing.T) { } c, _, _ := test.GetPipelineRunController(d) - err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run") + err := c.Reconciler.Reconcile(context.Background(), "foo/"+tc.pipelineRun.Name) if err != nil { t.Errorf("Did not expect to see error when reconciling invalid PipelineRun but saw %q", err) } - condition := prs[0].Status.GetCondition(duckv1alpha1.ConditionSucceeded) + condition := tc.pipelineRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded) if condition == nil || condition.Status != corev1.ConditionFalse { t.Errorf("Expected status to be failed on invalid PipelineRun %s but was: %v", tc.pipeline.Name, condition) } diff --git a/test/helm_task_test.go b/test/helm_task_test.go index 73c01cf69e9..a8a4b85b814 100644 --- a/test/helm_task_test.go +++ b/test/helm_task_test.go @@ -256,23 +256,14 @@ func getHelmDeployPipeline(namespace string) *v1alpha1.Pipeline { TaskRef: v1alpha1.TaskRef{ Name: createImageTaskName, }, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: sourceResourceName, - }, - }}, }, v1alpha1.PipelineTask{ Name: "helm-deploy", TaskRef: v1alpha1.TaskRef{ Name: helmDeployTaskName, }, - InputSourceBindings: []v1alpha1.SourceBinding{{ - Name: "workspace", - ResourceRef: v1alpha1.PipelineResourceRef{ - Name: sourceResourceName, - }, + ResourceDependencies: []v1alpha1.ResourceDependency{{ + Name: "workspace", ProvidedBy: []string{createImageTaskName}, }}, Params: []v1alpha1.Param{{ @@ -304,6 +295,23 @@ func getHelmDeployPipelineRun(namespace string) *v1alpha1.PipelineRun { PipelineTriggerRef: v1alpha1.PipelineTriggerRef{ Type: v1alpha1.PipelineTriggerTypeManual, }, + PipelineTaskResources: []v1alpha1.PipelineTaskResource{{ + Name: "helm-deploy", + Inputs: []v1alpha1.TaskResourceBinding{{ + Name: "workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: sourceResourceName, + }, + }}, + }, { + Name: "push-image", + Inputs: []v1alpha1.TaskResourceBinding{{ + Name: "workspace", + ResourceRef: v1alpha1.PipelineResourceRef{ + Name: sourceResourceName, + }, + }}, + }}, }, } }