diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index b57375c288e..df022763d28 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -82,3 +82,8 @@ data: # This is an experimental feature and thus should still be considered # an alpha feature. enable-tekton-oci-bundles: "false" + # Setting this flag to "true" enables the use of custom tasks from + # within pipelines. + # This is an experimental feature and thus should still be considered + # an alpha feature. + enable-custom-tasks: "false" diff --git a/docs/install.md b/docs/install.md index 7da605118b6..71f2bb2b2bd 100644 --- a/docs/install.md +++ b/docs/install.md @@ -353,6 +353,9 @@ not running. and use Workspaces to mount credentials from Secrets instead. The default is `false`. For more information, see the [associated issue](https://github.com/tektoncd/pipeline/issues/3399). +- `enable-custom-tasks`: set this flag to `"true"` to enable the +use of custom tasks in pipelines. + For example: ```yaml diff --git a/docs/pipelines.md b/docs/pipelines.md index 080fae22c5e..ea158df174e 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -25,6 +25,7 @@ weight: 3 - [Configuring the `Task` execution order](#configuring-the-task-execution-order) - [Adding a description](#adding-a-description) - [Adding `Finally` to the `Pipeline`](#adding-finally-to-the-pipeline) + - [Using Custom Tasks](#using-custom-tasks) - [Code examples](#code-examples) ## Overview @@ -888,6 +889,90 @@ In this example, `PipelineResults` is set to: ], ``` +## Using Custom Tasks + +**Note: This is only allowed if `enable-custom-tasks` is set to +`"true"` in the `feature-flags` configmap, see [`install.md`](./install.md#customizing-the-pipelines-controller-behavior)** + +[Custom Tasks](https://github.com/tektoncd/community/blob/master/teps/0002-custom-tasks.md) +can implement behavior that doesn't correspond directly to running a workload in a `Pod` on the cluster. +For example, a custom task might execute some operation outside of the cluster and wait for its execution to complete. + +A PipelineRun starts a custom task by creating a [`Run`](https://github.com/tektoncd/pipeline/blob/master/docs/runs.md) instead of a `TaskRun`. +In order for a custom task to execute, there must be a custom task controller running on the cluster +that is responsible for watching and updating `Run`s which reference their type. +If no such controller is running, those `Run`s will never complete and Pipelines using them will time out. + +Custom tasks are an **_experimental alpha feature_** and should be expected to change +in breaking ways or even be removed. + +### Specifying the target Custom Task + +To specify the custom task type you want to execute, the `taskRef` field +must include the custom task's `apiVersion` and `kind` as shown below: + +```yaml +spec: + tasks: + - name: run-custom-task + taskRef: + apiVersion: example.dev/v1alpha1 + kind: Example +``` + +This creates a `Run` of a custom task of type `Example` in the `example.dev` API group with the version `v1alpha1`. + +You can also specify the `name` of a custom task resource object previously defined in the cluster. + +```yaml +spec: + tasks: + - name: run-custom-task + taskRef: + apiVersion: example.dev/v1alpha1 + kind: Example + name: myexample +``` + +If the `taskRef` specifies a name, the custom task controller should look up the +`Example` resource with that name and use that object to configure the execution. + +If the `taskRef` does not specify a name, the custom task controller might support +some default behavior for executing unnamed tasks. + +### Specifying `Parameters` + +If a custom task supports [`parameters`](tasks.md#parameters), you can use the +`params` field to specify their values: + +```yaml +spec: +spec: + tasks: + - name: run-custom-task + taskRef: + apiVersion: example.dev/v1alpha1 + kind: Example + name: myexample + params: + - name: foo + value: bah +``` + +### Using `Results` + +If the custom task produces results, you can reference them in a Pipeline using the normal syntax, +`$(tasks..results.)`. + +### Limitations + +Pipelines do not directly support passing the following items to custom tasks: +* Pipeline Resources +* Workspaces +* Service account name +* Pod templates + + ## Code examples For a better understanding of `Pipelines`, study [our code examples](https://github.com/tektoncd/pipeline/tree/master/examples). diff --git a/docs/runs.md b/docs/runs.md index 9ce4a08f387..87611eb9ea5 100644 --- a/docs/runs.md +++ b/docs/runs.md @@ -35,10 +35,6 @@ will have no `.status` value and no further action will be taken. `Run`s are an **_experimental alpha feature_** and should be expected to change in breaking ways or even be removed. -`Run`s are not currently integrated with Pipelines, and require a running -third-party controller to actually perform any work. Without a third-party -controller, `Run`s will just exist without a status indefinitely. - ## Configuring a `Run` A `Run` definition supports the following fields: diff --git a/go.sum b/go.sum index 2bff5c80813..a99b6f09904 100644 --- a/go.sum +++ b/go.sum @@ -343,8 +343,8 @@ github.com/docker/cli v0.0.0-20190925022749-754388324470/go.mod h1:JLrzqnKDaYBop github.com/docker/cli v0.0.0-20191017083524-a8ff7f821017 h1:2HQmlpI3yI9deH18Q6xiSOIjXD4sLI55Y/gfpa8/558= github.com/docker/cli v0.0.0-20191017083524-a8ff7f821017/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/cli v0.0.0-20200130152716-5d0cf8839492/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= -github.com/docker/cli v0.0.0-20200303162255-7d407207c304 h1:A7SYzidcyuQ/yS4wezWGYeUioUFJQk8HYWY9aMYTF4I= -github.com/docker/cli v0.0.0-20200303162255-7d407207c304/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/cli v0.0.0-20200210162036-a4bedce16568 h1:AbI1uj9w4yt6TvfKHfRu7G55KuQe7NCvWPQRKDoXggE= +github.com/docker/cli v0.0.0-20200210162036-a4bedce16568/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/distribution v0.0.0-20191216044856-a8371794149d/go.mod h1:0+TTO4EOBfRPhZXAeF1Vu+W3hHZ8eLp8PgKVZlcvtFY= github.com/docker/distribution v2.6.0-rc.1.0.20180327202408-83389a148052+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/distribution v2.7.1+incompatible h1:a5mlkVzth6W5A4fOsS3D2EO5BUmsJpcB+cRlLU7cSug= @@ -1541,7 +1541,6 @@ golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgw golang.org/x/tools v0.0.0-20190614205625-5aca471b1d59/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190617190820-da514acc4774/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190621195816-6e04913cbbac/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= -golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc= golang.org/x/tools v0.0.0-20190706070813-72ffa07ba3db/go.mod h1:jcCCGcm9btYwXyDqrUWc6MKQKKGJCWEQ3AfLSRIbEuI= golang.org/x/tools v0.0.0-20190719005602-e377ae9d6386/go.mod h1:jcCCGcm9btYwXyDqrUWc6MKQKKGJCWEQ3AfLSRIbEuI= @@ -1807,8 +1806,6 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20190709130402-674ba3eaed22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= -gotest.tools/v3 v3.0.2 h1:kG1BFyqVHuQoVQiR1bWGnfz/fmHvvuiSPIV7rvl360E= -gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= helm.sh/helm/v3 v3.1.1/go.mod h1:WYsFJuMASa/4XUqLyv54s0U/f3mlAaRErGmyy4z921g= honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 022a7111fe3..7ccfaa0279f 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -58,6 +58,11 @@ ${PREFIX}/deepcopy-gen \ --go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt \ -i github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1/storage +${PREFIX}/deepcopy-gen \ + -O zz_generated.deepcopy \ + --go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt \ +-i github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1 + # Knative Injection # This generates the knative injection packages for the resource package (v1alpha1). # This is separate from the pipeline package for the same reason as client and all (see above). diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index b1d18201b9f..007b2a869eb 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -32,6 +32,7 @@ const ( runningInEnvWithInjectedSidecarsKey = "running-in-environment-with-injected-sidecars" requireGitSSHSecretKnownHostsKey = "require-git-ssh-secret-known-hosts" // nolint: gosec enableTektonOCIBundles = "enable-tekton-oci-bundles" + enableCustomTasks = "enable-custom-tasks" DefaultDisableHomeEnvOverwrite = false DefaultDisableWorkingDirOverwrite = false DefaultDisableAffinityAssistant = false @@ -39,6 +40,7 @@ const ( DefaultRunningInEnvWithInjectedSidecars = true DefaultRequireGitSSHSecretKnownHosts = false DefaultEnableTektonOciBundles = false + DefaultEnableCustomTasks = false ) // FeatureFlags holds the features configurations @@ -51,6 +53,7 @@ type FeatureFlags struct { RunningInEnvWithInjectedSidecars bool RequireGitSSHSecretKnownHosts bool EnableTektonOCIBundles bool + EnableCustomTasks bool } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -99,6 +102,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setFeature(enableTektonOCIBundles, DefaultEnableTektonOciBundles, &tc.EnableTektonOCIBundles); err != nil { return nil, err } + if err := setFeature(enableCustomTasks, DefaultEnableCustomTasks, &tc.EnableCustomTasks); err != nil { + return nil, err + } return &tc, nil } diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 775fd07d008..31730963c2a 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -47,6 +47,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { RunningInEnvWithInjectedSidecars: false, RequireGitSSHSecretKnownHosts: true, EnableTektonOCIBundles: true, + EnableCustomTasks: true, }, fileName: "feature-flags-all-flags-set", }, diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index 0eb165c7134..1f09d87dba9 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -24,3 +24,4 @@ data: running-in-environment-with-injected-sidecars: "false" require-git-ssh-secret-known-hosts: "true" enable-tekton-oci-bundles: "true" + enable-custom-tasks: "true" diff --git a/pkg/apis/pipeline/v1alpha1/run_types.go b/pkg/apis/pipeline/v1alpha1/run_types.go index 9f07eb4ba72..6cb820ef5b6 100644 --- a/pkg/apis/pipeline/v1alpha1/run_types.go +++ b/pkg/apis/pipeline/v1alpha1/run_types.go @@ -18,14 +18,12 @@ package v1alpha1 import ( "fmt" - "time" "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + runv1alpha1 "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/json" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" ) @@ -75,85 +73,16 @@ func (rs RunSpec) GetParam(name string) *v1beta1.Param { return nil } -type RunStatus struct { - duckv1.Status `json:",inline"` +const ( + // RunReasonCancelled must be used in the Condition Reason to indicate that a Run was cancelled. + RunReasonCancelled = "RunCancelled" +) - // RunStatusFields inlines the status fields. - RunStatusFields `json:",inline"` -} +// RunStatus defines the observed state of Run. +type RunStatus = runv1alpha1.RunStatus var runCondSet = apis.NewBatchConditionSet() -// GetCondition returns the Condition matching the given type. -func (r *RunStatus) GetCondition(t apis.ConditionType) *apis.Condition { - return runCondSet.Manage(r).GetCondition(t) -} - -// InitializeConditions will set all conditions in runCondSet to unknown for the PipelineRun -// and set the started time to the current time -func (r *RunStatus) InitializeConditions() { - started := false - if r.StartTime.IsZero() { - r.StartTime = &metav1.Time{Time: time.Now()} - started = true - } - conditionManager := runCondSet.Manage(r) - conditionManager.InitializeConditions() - // Ensure the started reason is set for the "Succeeded" condition - if started { - initialCondition := conditionManager.GetCondition(apis.ConditionSucceeded) - initialCondition.Reason = "Started" - conditionManager.SetCondition(*initialCondition) - } -} - -// SetCondition sets the condition, unsetting previous conditions with the same -// type as necessary. -func (r *RunStatus) SetCondition(newCond *apis.Condition) { - if newCond != nil { - runCondSet.Manage(r).SetCondition(*newCond) - } -} - -// MarkRunSucceeded changes the Succeeded condition to True with the provided reason and message. -func (r *RunStatus) MarkRunSucceeded(reason, messageFormat string, messageA ...interface{}) { - runCondSet.Manage(r).MarkTrueWithReason(apis.ConditionSucceeded, reason, messageFormat, messageA...) - succeeded := r.GetCondition(apis.ConditionSucceeded) - r.CompletionTime = &succeeded.LastTransitionTime.Inner -} - -// MarkRunFailed changes the Succeeded condition to False with the provided reason and message. -func (r *RunStatus) MarkRunFailed(reason, messageFormat string, messageA ...interface{}) { - runCondSet.Manage(r).MarkFalse(apis.ConditionSucceeded, reason, messageFormat, messageA...) - succeeded := r.GetCondition(apis.ConditionSucceeded) - r.CompletionTime = &succeeded.LastTransitionTime.Inner -} - -// MarkRunRunning changes the Succeeded condition to Unknown with the provided reason and message. -func (r *RunStatus) MarkRunRunning(reason, messageFormat string, messageA ...interface{}) { - runCondSet.Manage(r).MarkUnknown(apis.ConditionSucceeded, reason, messageFormat, messageA...) -} - -// DecodeExtraFields deserializes the extra fields in the Run status. -func (r *RunStatus) DecodeExtraFields(into interface{}) error { - if len(r.ExtraFields.Raw) == 0 { - return nil - } - return json.Unmarshal(r.ExtraFields.Raw, into) -} - -// EncodeExtraFields serializes the extra fields in the Run status. -func (r *RunStatus) EncodeExtraFields(from interface{}) error { - data, err := json.Marshal(from) - if err != nil { - return err - } - r.ExtraFields = runtime.RawExtension{ - Raw: data, - } - return nil -} - // GetConditionSet retrieves the condition set for this resource. Implements // the KRShaped interface. func (r *Run) GetConditionSet() apis.ConditionSet { return runCondSet } @@ -165,24 +94,10 @@ func (r *Run) GetStatus() *duckv1.Status { return &r.Status.Status } // RunStatusFields holds the fields of Run's status. This is defined // separately and inlined so that other types can readily consume these fields // via duck typing. -type RunStatusFields struct { - // StartTime is the time the build is actually started. - // +optional - StartTime *metav1.Time `json:"startTime,omitempty"` - - // CompletionTime is the time the build completed. - // +optional - CompletionTime *metav1.Time `json:"completionTime,omitempty"` - - // Results reports any output result values to be consumed by later - // tasks in a pipeline. - // +optional - Results []v1beta1.TaskRunResult `json:"results,omitempty"` +type RunStatusFields = runv1alpha1.RunStatusFields - // ExtraFields holds arbitrary fields provided by the custom task - // controller. - ExtraFields runtime.RawExtension `json:"extraFields,omitempty"` -} +// RunResult used to describe the results of a task +type RunResult = runv1alpha1.RunResult // +genclient // +genreconciler diff --git a/pkg/apis/pipeline/v1alpha1/run_types_test.go b/pkg/apis/pipeline/v1alpha1/run_types_test.go index 4e713fbe13a..fd4280a0f61 100644 --- a/pkg/apis/pipeline/v1alpha1/run_types_test.go +++ b/pkg/apis/pipeline/v1alpha1/run_types_test.go @@ -18,6 +18,7 @@ package v1alpha1_test import ( "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -102,6 +103,71 @@ func TestGetParams(t *testing.T) { } } +func TestRunHasStarted(t *testing.T) { + params := []struct { + name string + runStatus v1alpha1.RunStatus + expectedValue bool + }{{ + name: "runWithNoStartTime", + runStatus: v1alpha1.RunStatus{}, + expectedValue: false, + }, { + name: "runWithStartTime", + runStatus: v1alpha1.RunStatus{ + RunStatusFields: v1alpha1.RunStatusFields{ + StartTime: &metav1.Time{Time: time.Now()}, + }, + }, + expectedValue: true, + }, { + name: "runWithZeroStartTime", + runStatus: v1alpha1.RunStatus{ + RunStatusFields: v1alpha1.RunStatusFields{ + StartTime: &metav1.Time{}, + }, + }, + expectedValue: false, + }} + for _, tc := range params { + t.Run(tc.name, func(t *testing.T) { + run := v1alpha1.Run{} + run.Status = tc.runStatus + if run.HasStarted() != tc.expectedValue { + t.Fatalf("Expected run HasStarted() to return %t but got %t", tc.expectedValue, run.HasStarted()) + } + }) + } +} + +func TestRunIsDone(t *testing.T) { + run := v1alpha1.Run{ + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }}, + }, + }, + } + + if !run.IsDone() { + t.Fatal("Expected run status to be done") + } +} + +func TestRunIsCancelled(t *testing.T) { + run := v1alpha1.Run{ + Spec: v1alpha1.RunSpec{ + Status: v1alpha1.RunSpecStatusCancelled, + }, + } + if !run.IsCancelled() { + t.Fatal("Expected run status to be cancelled") + } +} + // TestRunStatusExtraFields tests that extraFields in a RunStatus can be parsed // from YAML. func TestRunStatus(t *testing.T) { @@ -153,7 +219,7 @@ status: }, RunStatusFields: v1alpha1.RunStatusFields{ // Results are parsed correctly. - Results: []v1beta1.TaskRunResult{{ + Results: []v1alpha1.RunResult{{ Name: "foo", Value: "bar", }}, diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index cf2dbdfb958..b0b06c95bcf 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -699,54 +699,6 @@ func (in *RunSpec) DeepCopy() *RunSpec { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RunStatus) DeepCopyInto(out *RunStatus) { - *out = *in - in.Status.DeepCopyInto(&out.Status) - in.RunStatusFields.DeepCopyInto(&out.RunStatusFields) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunStatus. -func (in *RunStatus) DeepCopy() *RunStatus { - if in == nil { - return nil - } - out := new(RunStatus) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RunStatusFields) DeepCopyInto(out *RunStatusFields) { - *out = *in - if in.StartTime != nil { - in, out := &in.StartTime, &out.StartTime - *out = (*in).DeepCopy() - } - if in.CompletionTime != nil { - in, out := &in.CompletionTime, &out.CompletionTime - *out = (*in).DeepCopy() - } - if in.Results != nil { - in, out := &in.Results, &out.Results - *out = make([]v1beta1.TaskRunResult, len(*in)) - copy(*out, *in) - } - in.ExtraFields.DeepCopyInto(&out.ExtraFields) - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunStatusFields. -func (in *RunStatusFields) DeepCopy() *RunStatusFields { - if in == nil { - return nil - } - out := new(RunStatusFields) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Task) DeepCopyInto(out *Task) { *out = *in diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 7ff7525a0cb..e06d6844bd2 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -55,6 +55,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunConditionCheckStatus": schema_pkg_apis_pipeline_v1beta1_PipelineRunConditionCheckStatus(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunList": schema_pkg_apis_pipeline_v1beta1_PipelineRunList(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunResult": schema_pkg_apis_pipeline_v1beta1_PipelineRunResult(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunRunStatus": schema_pkg_apis_pipeline_v1beta1_PipelineRunRunStatus(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunSpec": schema_pkg_apis_pipeline_v1beta1_PipelineRunSpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunSpecServiceAccountName": schema_pkg_apis_pipeline_v1beta1_PipelineRunSpecServiceAccountName(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunStatus": schema_pkg_apis_pipeline_v1beta1_PipelineRunStatus(ref), @@ -1322,6 +1323,47 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunResult(ref common.ReferenceCall } } +func schema_pkg_apis_pipeline_v1beta1_PipelineRunRunStatus(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "PipelineRunRunStatus contains the name of the PipelineTask for this Run and the Run's Status", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "pipelineTaskName": { + SchemaProps: spec.SchemaProps{ + Description: "PipelineTaskName is the name of the PipelineTask.", + Type: []string{"string"}, + Format: "", + }, + }, + "status": { + SchemaProps: spec.SchemaProps{ + Description: "Status is the RunStatus for the corresponding Run", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1.RunStatus"), + }, + }, + "whenExpressions": { + SchemaProps: spec.SchemaProps{ + Description: "WhenExpressions is the list of checks guarding the execution of the PipelineTask", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WhenExpression"), + }, + }, + }, + }, + }, + }, + }, + }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WhenExpression", "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1.RunStatus"}, + } +} + func schema_pkg_apis_pipeline_v1beta1_PipelineRunSpec(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -1536,6 +1578,20 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunStatus(ref common.ReferenceCall }, }, }, + "runs": { + SchemaProps: spec.SchemaProps{ + Description: "map of PipelineRunRunStatus with the run name as the key", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunRunStatus"), + }, + }, + }, + }, + }, "pipelineResults": { SchemaProps: spec.SchemaProps{ Description: "PipelineResults are the list of results written out by the pipeline task's containers", @@ -1572,7 +1628,7 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunStatus(ref common.ReferenceCall }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunTaskRunStatus", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.SkippedTask", "k8s.io/apimachinery/pkg/apis/meta/v1.Time", "knative.dev/pkg/apis.Condition"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunRunStatus", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunTaskRunStatus", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.SkippedTask", "k8s.io/apimachinery/pkg/apis/meta/v1.Time", "knative.dev/pkg/apis.Condition"}, } } @@ -1609,6 +1665,20 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunStatusFields(ref common.Referen }, }, }, + "runs": { + SchemaProps: spec.SchemaProps{ + Description: "map of PipelineRunRunStatus with the run name as the key", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunRunStatus"), + }, + }, + }, + }, + }, "pipelineResults": { SchemaProps: spec.SchemaProps{ Description: "PipelineResults are the list of results written out by the pipeline task's containers", @@ -1645,7 +1715,7 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunStatusFields(ref common.Referen }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunTaskRunStatus", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.SkippedTask", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunRunStatus", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineRunTaskRunStatus", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.SkippedTask", "k8s.io/apimachinery/pkg/apis/meta/v1.Time"}, } } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 511f8fdc7ac..982dc7ce3ca 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -105,7 +105,7 @@ func validatePipelineTask(ctx context.Context, t PipelineTask, taskNames sets.St hasTaskRef := t.TaskRef != nil hasTaskSpec := t.TaskSpec != nil - isCustomTask := hasTaskRef && t.TaskRef.APIVersion != "" + isCustomTask := cfg.FeatureFlags.EnableCustomTasks && hasTaskRef && t.TaskRef.APIVersion != "" // can't have both taskRef and taskSpec at the same time if hasTaskRef && hasTaskSpec { @@ -126,34 +126,36 @@ func validatePipelineTask(ctx context.Context, t PipelineTask, taskNames sets.St } taskNames[t.Name] = struct{}{} - // Custom Task refs are allowed to have no name. - if hasTaskRef && t.TaskRef.Name != "" { - // TaskRef name must be a valid k8s name - if errSlice := validation.IsQualifiedName(t.TaskRef.Name); len(errSlice) != 0 { - errs = errs.Also(apis.ErrInvalidValue(strings.Join(errSlice, ","), "name")) + if hasTaskRef { + if t.TaskRef.Name != "" { + // TaskRef name must be a valid k8s name + if errSlice := validation.IsQualifiedName(t.TaskRef.Name); len(errSlice) != 0 { + errs = errs.Also(apis.ErrInvalidValue(strings.Join(errSlice, ","), "name")) + } + } else { + // Custom Task refs are allowed to have no name. + if !isCustomTask { + errs = errs.Also(apis.ErrInvalidValue("taskRef must specify name", "taskRef.name")) + } } } - if hasTaskRef && !isCustomTask && t.TaskRef.Name == "" { - return apis.ErrInvalidValue(t.TaskRef, "taskRef must specify name") - } - if isCustomTask && t.TaskRef.Kind == "" { - return apis.ErrInvalidValue(t.TaskRef, "custom task ref must specify apiVersion and kind") - } - - // TODO(#3133): Support these features if possible. if isCustomTask { + if t.TaskRef.Kind == "" { + errs = errs.Also(apis.ErrInvalidValue("custom task ref must specify kind", "taskRef.kind")) + } + // TODO(#3133): Support these features if possible. if t.Retries > 0 { - return apis.ErrInvalidValue(t.Retries, "custom tasks do not support Retries") + errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support retries", "retries")) } if t.Resources != nil { - return apis.ErrInvalidValue(t.Resources, "custom tasks do not support PipelineResources") + errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support PipelineResources", "resources")) } if len(t.Workspaces) > 0 { - return apis.ErrInvalidValue(t.Workspaces, "custom tasks do not support Workspaces") + errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support Workspaces", "workspaces")) } if t.Timeout != nil { - return apis.ErrInvalidValue(t.Timeout, "custom tasks do not support Timeout") + errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support timeout", "timeout")) } } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 89cca574080..afa9dea95db 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -36,6 +36,7 @@ func TestPipeline_Validate_Success(t *testing.T) { tests := []struct { name string p *Pipeline + wc func(context.Context) context.Context }{{ name: "valid metadata", p: &Pipeline{ @@ -51,6 +52,7 @@ func TestPipeline_Validate_Success(t *testing.T) { Tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example", Name: ""}}}, }, }, + wc: enableFeature(t, "enable-custom-tasks"), }, { name: "valid pipeline with params, resources, workspaces, task results, and pipeline results", p: &Pipeline{ @@ -113,7 +115,11 @@ func TestPipeline_Validate_Success(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.p.Validate(context.Background()) + ctx := context.Background() + if tt.wc != nil { + ctx = tt.wc(ctx) + } + err := tt.p.Validate(ctx) if err != nil { t.Errorf("Pipeline.Validate() returned error for valid Pipeline: %v", err) } @@ -190,7 +196,7 @@ func TestPipeline_Validate_Failure(t *testing.T) { }, }, expectedError: *apis.ErrInvalidValue("invalid bundle reference (could not parse reference: invalid reference)", "spec.tasks[0].taskref.bundle"), - wc: enableTektonOCIBundles(t), + wc: enableFeature(t, "enable-tekton-oci-bundles"), }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -482,10 +488,11 @@ func TestValidatePipelineTasks_Success(t *testing.T) { } func TestValidatePipelineTasks_Failure(t *testing.T) { - for _, tc := range []struct { + tests := []struct { name string tasks []PipelineTask expectedError apis.FieldError + wc func(context.Context) context.Context }{{ name: "pipeline task missing taskref and taskspec", tasks: []PipelineTask{{ @@ -520,7 +527,7 @@ func TestValidatePipelineTasks_Failure(t *testing.T) { name: "pipeline tasks invalid (duplicate tasks)", tasks: []PipelineTask{ {Name: "foo", TaskRef: &TaskRef{Name: "foo-task"}}, - {Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}}, + {Name: "foo", TaskRef: &TaskRef{Name: "foo-task"}}, }, expectedError: apis.FieldError{ Message: `expected exactly one, got both`, @@ -564,16 +571,17 @@ func TestValidatePipelineTasks_Failure(t *testing.T) { name: "pipelinetask taskRef without name", tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{Name: ""}}}, expectedError: apis.FieldError{ - Message: `GMD FIX ME`, - Paths: []string{"tasks[0].taskSpec.steps"}, + Message: `invalid value: taskRef must specify name`, + Paths: []string{"tasks[0].taskRef.name"}, }, }, { - name: "pipelinetask custom task taskRef without name", + name: "pipelinetask custom task taskRef without kind", tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "", Name: ""}}}, expectedError: apis.FieldError{ - Message: `GMD FIX ME -- i think this one is not an error though`, - Paths: []string{"tasks[0].taskSpec.steps"}, + Message: `invalid value: custom task ref must specify kind`, + Paths: []string{"tasks[0].taskRef.kind"}, }, + wc: enableFeature(t, "enable-custom-tasks"), }, { name: "pipelinetask custom task doesn't support retries", tasks: []PipelineTask{{ @@ -582,9 +590,10 @@ func TestValidatePipelineTasks_Failure(t *testing.T) { TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}, }}, expectedError: apis.FieldError{ - Message: `GMD FIX ME`, - Paths: []string{"tasks[0].taskSpec.steps"}, + Message: `invalid value: custom tasks do not support retries`, + Paths: []string{"tasks[0].retries"}, }, + wc: enableFeature(t, "enable-custom-tasks"), }, { name: "pipelinetask custom task doesn't support pipeline resources", tasks: []PipelineTask{{ @@ -593,9 +602,10 @@ func TestValidatePipelineTasks_Failure(t *testing.T) { TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}, }}, expectedError: apis.FieldError{ - Message: `GMD FIX ME`, - Paths: []string{"tasks[0].taskSpec.steps"}, + Message: `invalid value: custom tasks do not support PipelineResources`, + Paths: []string{"tasks[0].resources"}, }, + wc: enableFeature(t, "enable-custom-tasks"), }, { name: "pipelinetask custom task doesn't support workspaces", tasks: []PipelineTask{{ @@ -603,20 +613,31 @@ func TestValidatePipelineTasks_Failure(t *testing.T) { Workspaces: []WorkspacePipelineTaskBinding{{}}, TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}, }}, + expectedError: apis.FieldError{ + Message: `invalid value: custom tasks do not support Workspaces`, + Paths: []string{"tasks[0].workspaces"}, + }, + wc: enableFeature(t, "enable-custom-tasks"), }, { name: "pipelinetask custom task doesn't support timeout", tasks: []PipelineTask{{ Name: "foo", - Timeout: &metav1.Duration{time.Duration(3)}, + Timeout: &metav1.Duration{Duration: time.Duration(3)}, TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}, }}, expectedError: apis.FieldError{ - Message: `GMD FIX ME`, - Paths: []string{"tasks[0].taskSpec.steps"}, + Message: `invalid value: custom tasks do not support timeout`, + Paths: []string{"tasks[0].timeout"}, }, - }} { - t.Run(tc.name, func(t *testing.T) { - err := validatePipelineTasks(context.Background(), tc.tasks, []PipelineTask{}) + wc: enableFeature(t, "enable-custom-tasks"), + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + if tt.wc != nil { + ctx = tt.wc(ctx) + } + err := validatePipelineTasks(ctx, tt.tasks, []PipelineTask{}) if err == nil { t.Error("Pipeline.validatePipelineTasks() did not return error for invalid pipeline tasks") } @@ -2101,13 +2122,13 @@ func getTaskSpec() TaskSpec { } } -func enableTektonOCIBundles(t *testing.T) func(context.Context) context.Context { +func enableFeature(t *testing.T, feature string) func(context.Context) context.Context { return func(ctx context.Context) context.Context { s := config.NewStore(logtesting.TestLogger(t)) s.OnConfigChanged(&corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName()}, Data: map[string]string{ - "enable-tekton-oci-bundles": "true", + feature: "true", }, }) return s.ToContext(ctx) diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index c43a3a6a599..fa212a27781 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -22,6 +22,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" + runv1alpha1 "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -266,6 +267,9 @@ func (pr *PipelineRunStatus) InitializeConditions() { if pr.TaskRuns == nil { pr.TaskRuns = make(map[string]*PipelineRunTaskRunStatus) } + if pr.Runs == nil { + pr.Runs = make(map[string]*PipelineRunRunStatus) + } if pr.StartTime.IsZero() { pr.StartTime = &metav1.Time{Time: time.Now()} started = true @@ -390,8 +394,12 @@ type PipelineRunTaskRunStatus struct { type PipelineRunRunStatus struct { // PipelineTaskName is the name of the PipelineTask. PipelineTaskName string `json:"pipelineTaskName,omitempty"` - - // TODO(#3133): Add v1alpha1.RunStatus here, without introducing an import cycle. + // Status is the RunStatus for the corresponding Run + // +optional + Status *runv1alpha1.RunStatus `json:"status,omitempty"` + // WhenExpressions is the list of checks guarding the execution of the PipelineTask + // +optional + WhenExpressions []WhenExpression `json:"whenExpressions,omitempty"` } // PipelineRunConditionCheckStatus returns the condition check status diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go index 679628362ed..85df2841592 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go @@ -92,7 +92,11 @@ func TestInitializePipelineRunConditions(t *testing.T) { p.Status.InitializeConditions() if p.Status.TaskRuns == nil { - t.Fatalf("PipelineRun status not initialized correctly") + t.Fatalf("PipelineRun TaskRun status not initialized correctly") + } + + if p.Status.Runs == nil { + t.Fatalf("PipelineRun Run status not initialized correctly") } if p.Status.StartTime.IsZero() { @@ -104,6 +108,7 @@ func TestInitializePipelineRunConditions(t *testing.T) { t.Fatalf("PipelineRun initialize reason should be %s, got %s instead", v1beta1.PipelineRunReasonStarted.String(), condition.Reason) } p.Status.TaskRuns["fooTask"] = &v1beta1.PipelineRunTaskRunStatus{} + p.Status.Runs["bahTask"] = &v1beta1.PipelineRunRunStatus{} // Change the reason before we initialize again p.Status.SetCondition(&apis.Condition{ @@ -115,7 +120,10 @@ func TestInitializePipelineRunConditions(t *testing.T) { p.Status.InitializeConditions() if len(p.Status.TaskRuns) != 1 { - t.Fatalf("PipelineRun status getting reset") + t.Fatalf("PipelineRun TaskRun status getting reset") + } + if len(p.Status.Runs) != 1 { + t.Fatalf("PipelineRun Run status getting reset") } newCondition := p.Status.GetCondition(apis.ConditionSucceeded) diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 46c3df3030e..e2156137759 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -832,6 +832,27 @@ } } }, + "v1beta1.PipelineRunRunStatus": { + "description": "PipelineRunRunStatus contains the name of the PipelineTask for this Run and the Run's Status", + "type": "object", + "properties": { + "pipelineTaskName": { + "description": "PipelineTaskName is the name of the PipelineTask.", + "type": "string" + }, + "status": { + "description": "Status is the RunStatus for the corresponding Run", + "$ref": "#/definitions/github.com.tektoncd.pipeline.pkg.apis.run.v1alpha1.RunStatus" + }, + "whenExpressions": { + "description": "WhenExpressions is the list of checks guarding the execution of the PipelineTask", + "type": "array", + "items": { + "$ref": "#/definitions/v1beta1.WhenExpression" + } + } + } + }, "v1beta1.PipelineRunSpec": { "description": "PipelineRunSpec defines the desired state of PipelineRun", "type": "object", @@ -946,6 +967,13 @@ "description": "PipelineRunSpec contains the exact spec used to instantiate the run", "$ref": "#/definitions/v1beta1.PipelineSpec" }, + "runs": { + "description": "map of PipelineRunRunStatus with the run name as the key", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/v1beta1.PipelineRunRunStatus" + } + }, "skippedTasks": { "description": "list of tasks that were skipped due to when expressions evaluating to false", "type": "array", @@ -985,6 +1013,13 @@ "description": "PipelineRunSpec contains the exact spec used to instantiate the run", "$ref": "#/definitions/v1beta1.PipelineSpec" }, + "runs": { + "description": "map of PipelineRunRunStatus with the run name as the key", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/v1beta1.PipelineRunRunStatus" + } + }, "skippedTasks": { "description": "list of tasks that were skipped due to when expressions evaluating to false", "type": "array", diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 8e9fbb7178d..2905500acbe 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package v1beta1 import ( pod "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" + runv1alpha1 "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" @@ -590,6 +591,18 @@ func (in *PipelineRunResult) DeepCopy() *PipelineRunResult { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineRunRunStatus) DeepCopyInto(out *PipelineRunRunStatus) { *out = *in + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(runv1alpha1.RunStatus) + (*in).DeepCopyInto(*out) + } + if in.WhenExpressions != nil { + in, out := &in.WhenExpressions, &out.WhenExpressions + *out = make([]WhenExpression, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -742,7 +755,7 @@ func (in *PipelineRunStatusFields) DeepCopyInto(out *PipelineRunStatusFields) { } else { in, out := &val, &outVal *out = new(PipelineRunRunStatus) - **out = **in + (*in).DeepCopyInto(*out) } (*out)[key] = outVal } diff --git a/pkg/apis/run/v1alpha1/runstatus_types.go b/pkg/apis/run/v1alpha1/runstatus_types.go new file mode 100644 index 00000000000..caab02e4deb --- /dev/null +++ b/pkg/apis/run/v1alpha1/runstatus_types.go @@ -0,0 +1,144 @@ +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "encoding/json" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" +) + +// This package exists to avoid an import cycle between v1alpha1 and v1beta1. +// It contains common definitions needed by v1alpha1.Run and v1beta1.PipelineRun. + +// +k8s:deepcopy-gen=true + +// RunStatus defines the observed state of Run +type RunStatus struct { + duckv1.Status `json:",inline"` + + // RunStatusFields inlines the status fields. + RunStatusFields `json:",inline"` +} + +// +k8s:deepcopy-gen=true + +// RunStatusFields holds the fields of Run's status. This is defined +// separately and inlined so that other types can readily consume these fields +// via duck typing. +type RunStatusFields struct { + // StartTime is the time the build is actually started. + // +optional + StartTime *metav1.Time `json:"startTime,omitempty"` + + // CompletionTime is the time the build completed. + // +optional + CompletionTime *metav1.Time `json:"completionTime,omitempty"` + + // Results reports any output result values to be consumed by later + // tasks in a pipeline. + // +optional + Results []RunResult `json:"results,omitempty"` + + // ExtraFields holds arbitrary fields provided by the custom task + // controller. + ExtraFields runtime.RawExtension `json:"extraFields,omitempty"` +} + +// RunResult used to describe the results of a task +type RunResult struct { + // Name the given name + Name string `json:"name"` + // Value the given value of the result + Value string `json:"value"` +} + +var runCondSet = apis.NewBatchConditionSet() + +// GetCondition returns the Condition matching the given type. +func (r *RunStatus) GetCondition(t apis.ConditionType) *apis.Condition { + return runCondSet.Manage(r).GetCondition(t) +} + +// InitializeConditions will set all conditions in runCondSet to unknown for the PipelineRun +// and set the started time to the current time +func (r *RunStatus) InitializeConditions() { + started := false + if r.StartTime.IsZero() { + r.StartTime = &metav1.Time{Time: time.Now()} + started = true + } + conditionManager := runCondSet.Manage(r) + conditionManager.InitializeConditions() + // Ensure the started reason is set for the "Succeeded" condition + if started { + initialCondition := conditionManager.GetCondition(apis.ConditionSucceeded) + initialCondition.Reason = "Started" + conditionManager.SetCondition(*initialCondition) + } +} + +// SetCondition sets the condition, unsetting previous conditions with the same +// type as necessary. +func (r *RunStatus) SetCondition(newCond *apis.Condition) { + if newCond != nil { + runCondSet.Manage(r).SetCondition(*newCond) + } +} + +// MarkRunSucceeded changes the Succeeded condition to True with the provided reason and message. +func (r *RunStatus) MarkRunSucceeded(reason, messageFormat string, messageA ...interface{}) { + runCondSet.Manage(r).MarkTrueWithReason(apis.ConditionSucceeded, reason, messageFormat, messageA...) + succeeded := r.GetCondition(apis.ConditionSucceeded) + r.CompletionTime = &succeeded.LastTransitionTime.Inner +} + +// MarkRunFailed changes the Succeeded condition to False with the provided reason and message. +func (r *RunStatus) MarkRunFailed(reason, messageFormat string, messageA ...interface{}) { + runCondSet.Manage(r).MarkFalse(apis.ConditionSucceeded, reason, messageFormat, messageA...) + succeeded := r.GetCondition(apis.ConditionSucceeded) + r.CompletionTime = &succeeded.LastTransitionTime.Inner +} + +// MarkRunRunning changes the Succeeded condition to Unknown with the provided reason and message. +func (r *RunStatus) MarkRunRunning(reason, messageFormat string, messageA ...interface{}) { + runCondSet.Manage(r).MarkUnknown(apis.ConditionSucceeded, reason, messageFormat, messageA...) +} + +// DecodeExtraFields deserializes the extra fields in the Run status. +func (r *RunStatus) DecodeExtraFields(into interface{}) error { + if len(r.ExtraFields.Raw) == 0 { + return nil + } + return json.Unmarshal(r.ExtraFields.Raw, into) +} + +// EncodeExtraFields serializes the extra fields in the Run status. +func (r *RunStatus) EncodeExtraFields(from interface{}) error { + data, err := json.Marshal(from) + if err != nil { + return err + } + r.ExtraFields = runtime.RawExtension{ + Raw: data, + } + return nil +} diff --git a/pkg/apis/run/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/run/v1alpha1/zz_generated.deepcopy.go new file mode 100644 index 00000000000..b4eb0a2c4c6 --- /dev/null +++ b/pkg/apis/run/v1alpha1/zz_generated.deepcopy.go @@ -0,0 +1,69 @@ +// +build !ignore_autogenerated + +/* +Copyright 2020 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by deepcopy-gen. DO NOT EDIT. + +package v1alpha1 + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RunStatus) DeepCopyInto(out *RunStatus) { + *out = *in + in.Status.DeepCopyInto(&out.Status) + in.RunStatusFields.DeepCopyInto(&out.RunStatusFields) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunStatus. +func (in *RunStatus) DeepCopy() *RunStatus { + if in == nil { + return nil + } + out := new(RunStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RunStatusFields) DeepCopyInto(out *RunStatusFields) { + *out = *in + if in.StartTime != nil { + in, out := &in.StartTime, &out.StartTime + *out = (*in).DeepCopy() + } + if in.CompletionTime != nil { + in, out := &in.CompletionTime, &out.CompletionTime + *out = (*in).DeepCopy() + } + if in.Results != nil { + in, out := &in.Results, &out.Results + *out = make([]RunResult, len(*in)) + copy(*out, *in) + } + in.ExtraFields.DeepCopyInto(&out.ExtraFields) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunStatusFields. +func (in *RunStatusFields) DeepCopy() *RunStatusFields { + if in == nil { + return nil + } + out := new(RunStatusFields) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/reconciler/pipelinerun/cancel.go b/pkg/reconciler/pipelinerun/cancel.go index b7d18651974..d3226046ffb 100644 --- a/pkg/reconciler/pipelinerun/cancel.go +++ b/pkg/reconciler/pipelinerun/cancel.go @@ -24,6 +24,7 @@ import ( "strings" "time" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned" "go.uber.org/zap" @@ -34,17 +35,25 @@ import ( "knative.dev/pkg/apis" ) -var cancelPatchBytes []byte +var cancelTaskRunPatchBytes, cancelRunPatchBytes []byte func init() { var err error - cancelPatchBytes, err = json.Marshal([]jsonpatch.JsonPatchOperation{{ + cancelTaskRunPatchBytes, err = json.Marshal([]jsonpatch.JsonPatchOperation{{ Operation: "add", Path: "/spec/status", Value: v1beta1.TaskRunSpecStatusCancelled, }}) if err != nil { - log.Fatalf("failed to marshal cancel patch bytes: %v", err) + log.Fatalf("failed to marshal TaskRun cancel patch bytes: %v", err) + } + cancelRunPatchBytes, err = json.Marshal([]jsonpatch.JsonPatchOperation{{ + Operation: "add", + Path: "/spec/status", + Value: v1alpha1.RunSpecStatusCancelled, + }}) + if err != nil { + log.Fatalf("failed to marshal Run cancel patch bytes: %v", err) } } @@ -57,12 +66,21 @@ func cancelPipelineRun(ctx context.Context, logger *zap.SugaredLogger, pr *v1bet for taskRunName := range pr.Status.TaskRuns { logger.Infof("cancelling TaskRun %s", taskRunName) - if _, err := clientSet.TektonV1beta1().TaskRuns(pr.Namespace).Patch(ctx, taskRunName, types.JSONPatchType, cancelPatchBytes, metav1.PatchOptions{}, ""); err != nil { + if _, err := clientSet.TektonV1beta1().TaskRuns(pr.Namespace).Patch(ctx, taskRunName, types.JSONPatchType, cancelTaskRunPatchBytes, metav1.PatchOptions{}, ""); err != nil { errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %s", taskRunName, err).Error()) continue } } - // If we successfully cancelled all the TaskRuns, we can consider the PipelineRun cancelled. + // Loop over the Runs in the PipelineRun status. + for runName := range pr.Status.Runs { + logger.Infof("cancelling Run %s", runName) + + if _, err := clientSet.TektonV1alpha1().Runs(pr.Namespace).Patch(ctx, runName, types.JSONPatchType, cancelRunPatchBytes, metav1.PatchOptions{}, ""); err != nil { + errs = append(errs, fmt.Errorf("Failed to patch Run `%s` with cancellation: %s", runName, err).Error()) + continue + } + } + // If we successfully cancelled all the TaskRuns and Runs, we can consider the PipelineRun cancelled. if len(errs) == 0 { pr.Status.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, @@ -79,7 +97,7 @@ func cancelPipelineRun(ctx context.Context, logger *zap.SugaredLogger, pr *v1bet Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, Reason: ReasonCouldntCancel, - Message: fmt.Sprintf("PipelineRun %q was cancelled but had errors trying to cancel TaskRuns: %s", pr.Name, e), + Message: fmt.Sprintf("PipelineRun %q was cancelled but had errors trying to cancel TaskRuns and/or Runs: %s", pr.Name, e), }) return fmt.Errorf("error(s) from cancelling TaskRun(s) from PipelineRun %s: %s", pr.Name, e) } diff --git a/pkg/reconciler/pipelinerun/cancel_test.go b/pkg/reconciler/pipelinerun/cancel_test.go index c5d1d0bf1a1..4f786effd29 100644 --- a/pkg/reconciler/pipelinerun/cancel_test.go +++ b/pkg/reconciler/pipelinerun/cancel_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/test" @@ -33,6 +34,7 @@ func TestCancelPipelineRun(t *testing.T) { name string pipelineRun *v1beta1.PipelineRun taskRuns []*v1beta1.TaskRun + runs []*v1alpha1.Run }{{ name: "no-resolved-taskrun", pipelineRun: &v1beta1.PipelineRun{ @@ -75,6 +77,24 @@ func TestCancelPipelineRun(t *testing.T) { {ObjectMeta: metav1.ObjectMeta{Name: "t1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "t2"}}, }, + }, { + name: "multiple-runs", + pipelineRun: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"}, + Spec: v1beta1.PipelineRunSpec{ + Status: v1beta1.PipelineRunSpecStatusCancelled, + }, + Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + Runs: map[string]*v1beta1.PipelineRunRunStatus{ + "t1": {PipelineTaskName: "task-1"}, + "t2": {PipelineTaskName: "task-2"}, + }, + }}, + }, + runs: []*v1alpha1.Run{ + {ObjectMeta: metav1.ObjectMeta{Name: "t1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "t2"}}, + }, }} for _, tc := range testCases { tc := tc @@ -82,6 +102,7 @@ func TestCancelPipelineRun(t *testing.T) { d := test.Data{ PipelineRuns: []*v1beta1.PipelineRun{tc.pipelineRun}, TaskRuns: tc.taskRuns, + Runs: tc.runs, } ctx, _ := ttesting.SetupFakeContext(t) ctx, cancel := context.WithCancel(ctx) @@ -95,13 +116,26 @@ func TestCancelPipelineRun(t *testing.T) { if cond.IsTrue() { t.Errorf("Expected PipelineRun status to be complete and false, but was %v", cond) } - l, err := c.Pipeline.TektonV1beta1().TaskRuns("").List(ctx, metav1.ListOptions{}) - if err != nil { - t.Fatal(err) + if tc.taskRuns != nil { + l, err := c.Pipeline.TektonV1beta1().TaskRuns("").List(ctx, metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + for _, tr := range l.Items { + if tr.Spec.Status != v1beta1.TaskRunSpecStatusCancelled { + t.Errorf("expected task %q to be marked as cancelled, was %q", tr.Name, tr.Spec.Status) + } + } } - for _, tr := range l.Items { - if tr.Spec.Status != v1beta1.TaskRunSpecStatusCancelled { - t.Errorf("expected task %q to be marked as cancelled, was %q", tr.Name, tr.Spec.Status) + if tc.runs != nil { + l, err := c.Pipeline.TektonV1alpha1().Runs("").List(ctx, metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + for _, r := range l.Items { + if r.Spec.Status != v1alpha1.RunSpecStatusCancelled { + t.Errorf("expected Run %q to be marked as cancelled, was %q", r.Name, r.Spec.Status) + } } } }) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index ac954081c3f..4a2af6f1ea8 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -189,6 +189,10 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) logger.Errorf("Failed to update TaskRun status for PipelineRun %s: %v", pr.Name, err) return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) } + if err := c.updateRunsStatusDirectly(pr); err != nil { + logger.Errorf("Failed to update Run status for PipelineRun %s: %v", pr.Name, err) + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) + } go func(metrics *Recorder) { err := metrics.DurationAndCount(pr) if err != nil { @@ -473,7 +477,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get tasks = append(tasks, pipelineSpec.Finally...) } pipelineRunState, err := c.resolvePipelineState(ctx, tasks, pipelineMeta, pr, providedResources) - if err != nil { return err } @@ -547,7 +550,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get after = pr.Status.GetCondition(apis.ConditionSucceeded) pr.Status.StartTime = pipelineRunFacts.State.AdjustStartTime(pr.Status.StartTime) pr.Status.TaskRuns = pipelineRunFacts.State.GetTaskRunsStatus(pr) - pr.Status.Runs = pipelineRunFacts.getRunsStatus(pr) + pr.Status.Runs = pipelineRunFacts.State.GetRunsStatus(pr) pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks() logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after) return nil @@ -557,6 +560,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get // pipeline run state, and starts them // after all DAG tasks are done, it's responsible for scheduling final tasks and start executing them func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.PipelineRun, pipelineRunFacts *resources.PipelineRunFacts, as artifacts.ArtifactStorageInterface) error { + logger := logging.FromContext(ctx) recorder := controller.GetEventRecorder(ctx) @@ -587,18 +591,18 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip continue } if rprt.ResolvedConditionChecks == nil || rprt.ResolvedConditionChecks.IsSuccess() { - if !rprt.IsCustomTask() { - rprt.TaskRun, err = c.createTaskRun(ctx, rprt, pr, as.StorageBasePath(pr)) - if err != nil { - recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunCreationFailed", "Failed to create TaskRun %q: %v", rprt.TaskRunName, err) - return fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rprt.TaskRunName, rprt.PipelineTask.Name, pr.Name, err) - } - } else { + if rprt.IsCustomTask() { rprt.Run, err = c.createRun(ctx, rprt, pr) if err != nil { recorder.Eventf(pr, corev1.EventTypeWarning, "RunCreationFailed", "Failed to create Run %q: %v", rprt.RunName, err) return fmt.Errorf("error creating Run called %s for PipelineTask %s from PipelineRun %s: %w", rprt.RunName, rprt.PipelineTask.Name, pr.Name, err) } + } else { + rprt.TaskRun, err = c.createTaskRun(ctx, rprt, pr, as.StorageBasePath(pr)) + if err != nil { + recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunCreationFailed", "Failed to create TaskRun %q: %v", rprt.TaskRunName, err) + return fmt.Errorf("error creating TaskRun called %s for PipelineTask %s from PipelineRun %s: %w", rprt.TaskRunName, rprt.PipelineTask.Name, pr.Name, err) + } } } else if !rprt.ResolvedConditionChecks.HasStarted() { for _, rcc := range rprt.ResolvedConditionChecks { @@ -639,11 +643,29 @@ func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1beta1.PipelineRun) error // TODO(dibyom): Add conditionCheck statuses here prtrs := pr.Status.TaskRuns[taskRunName] tr, err := c.taskRunLister.TaskRuns(pr.Namespace).Get(taskRunName) - if err != nil && !errors.IsNotFound(err) { + if err != nil { // If the TaskRun isn't found, it just means it won't be run - return fmt.Errorf("error retrieving TaskRun %s: %w", taskRunName, err) + if !errors.IsNotFound(err) { + return fmt.Errorf("error retrieving TaskRun %s: %w", taskRunName, err) + } + } else { + prtrs.Status = &tr.Status + } + } + return nil +} + +func (c *Reconciler) updateRunsStatusDirectly(pr *v1beta1.PipelineRun) error { + for runName := range pr.Status.Runs { + prRunStatus := pr.Status.Runs[runName] + run, err := c.runLister.Runs(pr.Namespace).Get(runName) + if err != nil { + if !errors.IsNotFound(err) { + return fmt.Errorf("error retrieving Run %s: %w", runName, err) + } + } else { + prRunStatus.Status = &run.Status } - prtrs.Status = &tr.Status } return nil } @@ -676,9 +698,8 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved Params: rprt.PipelineTask.Params, ServiceAccountName: taskRunSpec.TaskServiceAccountName, Timeout: getTaskRunTimeout(ctx, pr, rprt), - PodTemplate: podTemplate, - }, - } + PodTemplate: taskRunSpec.TaskPodTemplate, + }} if rprt.ResolvedTaskResources.TaskName != "" { // We pass the entire, original task ref because it may contain additional references like a Bundle url. @@ -714,12 +735,13 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved } func (c *Reconciler) createRun(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, pr *v1beta1.PipelineRun) (*v1alpha1.Run, error) { + logger := logging.FromContext(ctx) r := &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ Name: rprt.RunName, Namespace: pr.Namespace, OwnerReferences: []metav1.OwnerReference{pr.GetOwnerReference()}, - Labels: getTaskrunLabels(pr, rprt.PipelineTask.Name), + Labels: getTaskrunLabels(pr, rprt.PipelineTask.Name, true), Annotations: getTaskrunAnnotations(pr), }, Spec: v1alpha1.RunSpec{ @@ -727,7 +749,8 @@ func (c *Reconciler) createRun(ctx context.Context, rprt *resources.ResolvedPipe Params: rprt.PipelineTask.Params, }, } - return c.PipelineClientSet.TektonV1alpha1().Runs(pr.Namespace).Create(r) + logger.Infof("Creating a new Run object %s", rprt.RunName) + return c.PipelineClientSet.TektonV1alpha1().Runs(pr.Namespace).Create(ctx, r, metav1.CreateOptions{}) } // taskWorkspaceByWorkspaceVolumeSource is returning the WorkspaceBinding with the TaskRun specified name. @@ -965,31 +988,40 @@ func (c *Reconciler) updatePipelineRunStatusFromInformer(ctx context.Context, pr logger.Errorf("could not list TaskRuns %#v", err) return err } - pr.Status = updatePipelineRunStatusFromTaskRuns(logger, pr, pr.Status, taskRuns) + updatePipelineRunStatusFromTaskRuns(logger, pr, taskRuns) + + runs, err := c.runLister.Runs(pr.Namespace).List(labels.SelectorFromSet(pipelineRunLabels)) + if err != nil { + logger.Errorf("could not list Runs %#v", err) + return err + } + updatePipelineRunStatusFromRuns(logger, pr, runs) + return nil } -func updatePipelineRunStatusFromTaskRuns(logger *zap.SugaredLogger, pr *v1beta1.PipelineRun, prStatus v1beta1.PipelineRunStatus, trs []*v1beta1.TaskRun) v1beta1.PipelineRunStatus { +func updatePipelineRunStatusFromTaskRuns(logger *zap.SugaredLogger, pr *v1beta1.PipelineRun, trs []*v1beta1.TaskRun) { // If no TaskRun was found, nothing to be done. We never remove taskruns from the status if trs == nil || len(trs) == 0 { - return prStatus + return } // Store a list of Condition TaskRuns for each PipelineTask (by name) conditionTaskRuns := make(map[string][]*v1beta1.TaskRun) // Map PipelineTask names to TaskRun names that were already in the status taskRunByPipelineTask := make(map[string]string) - if prStatus.TaskRuns != nil { - for taskRunName, pipelineRunTaskRunStatus := range prStatus.TaskRuns { + if pr.Status.TaskRuns != nil { + for taskRunName, pipelineRunTaskRunStatus := range pr.Status.TaskRuns { taskRunByPipelineTask[pipelineRunTaskRunStatus.PipelineTaskName] = taskRunName } } else { - prStatus.TaskRuns = make(map[string]*v1beta1.PipelineRunTaskRunStatus) + pr.Status.TaskRuns = make(map[string]*v1beta1.PipelineRunTaskRunStatus) } // Loop over all the TaskRuns associated to Tasks for _, taskrun := range trs { // Only process TaskRuns that are owned by this PipelineRun. + // This skips TaskRuns that are indirectly created by the PipelineRun (e.g. by custom tasks). if len(taskrun.OwnerReferences) < 1 || taskrun.OwnerReferences[0].UID != pr.ObjectMeta.UID { - logger.Infof("Found a TaskRun %s that is not owned by this PipelineRun", taskrun.Name) + logger.Debugf("Found a TaskRun %s that is not owned by this PipelineRun", taskrun.Name) continue } lbls := taskrun.GetLabels() @@ -1003,11 +1035,11 @@ func updatePipelineRunStatusFromTaskRuns(logger *zap.SugaredLogger, pr *v1beta1. conditionTaskRuns[pipelineTaskName] = append(conditionTaskRuns[pipelineTaskName], taskrun) continue } - if _, ok := prStatus.TaskRuns[taskrun.Name]; !ok { + if _, ok := pr.Status.TaskRuns[taskrun.Name]; !ok { // This taskrun was missing from the status. // Add it without conditions, which are handled in the next loop logger.Infof("Found a TaskRun %s that was missing from the PipelineRun status", taskrun.Name) - prStatus.TaskRuns[taskrun.Name] = &v1beta1.PipelineRunTaskRunStatus{ + pr.Status.TaskRuns[taskrun.Name] = &v1beta1.PipelineRunTaskRunStatus{ PipelineTaskName: pipelineTaskName, Status: &taskrun.Status, ConditionChecks: nil, @@ -1024,8 +1056,8 @@ func updatePipelineRunStatusFromTaskRuns(logger *zap.SugaredLogger, pr *v1beta1. // status. This means that the conditions were orphaned, and never added to the // status. In this case we need to generate a new TaskRun name, that will be used // to run the TaskRun if the conditions are passed. - taskRunName = resources.GetTaskRunName(prStatus.TaskRuns, pipelineTaskName, pr.Name) - prStatus.TaskRuns[taskRunName] = &v1beta1.PipelineRunTaskRunStatus{ + taskRunName = resources.GetTaskRunName(pr.Status.TaskRuns, pipelineTaskName, pr.Name) + pr.Status.TaskRuns[taskRunName] = &v1beta1.PipelineRunTaskRunStatus{ PipelineTaskName: pipelineTaskName, Status: nil, ConditionChecks: nil, @@ -1033,7 +1065,7 @@ func updatePipelineRunStatusFromTaskRuns(logger *zap.SugaredLogger, pr *v1beta1. } // Build the map of condition checks for the taskrun // If there were no other condition, initialise the map - conditionChecks := prStatus.TaskRuns[taskRunName].ConditionChecks + conditionChecks := pr.Status.TaskRuns[taskRunName].ConditionChecks if conditionChecks == nil { conditionChecks = make(map[string]*v1beta1.PipelineRunConditionCheckStatus) } @@ -1054,7 +1086,34 @@ func updatePipelineRunStatusFromTaskRuns(logger *zap.SugaredLogger, pr *v1beta1. } } } - prStatus.TaskRuns[taskRunName].ConditionChecks = conditionChecks + pr.Status.TaskRuns[taskRunName].ConditionChecks = conditionChecks + } +} + +func updatePipelineRunStatusFromRuns(logger *zap.SugaredLogger, pr *v1beta1.PipelineRun, runs []*v1alpha1.Run) { + // If no Run was found, nothing to be done. We never remove runs from the status + if runs == nil || len(runs) == 0 { + return + } + if pr.Status.Runs == nil { + pr.Status.Runs = make(map[string]*v1beta1.PipelineRunRunStatus) + } + // Loop over all the Runs associated to Tasks + for _, run := range runs { + // Only process Runs that are owned by this PipelineRun. + // This skips Runs that are indirectly created by the PipelineRun (e.g. by custom tasks). + if len(run.OwnerReferences) < 1 && run.OwnerReferences[0].UID != pr.ObjectMeta.UID { + logger.Debugf("Found a Run %s that is not owned by this PipelineRun", run.Name) + continue + } + lbls := run.GetLabels() + pipelineTaskName := lbls[pipeline.GroupName+pipeline.PipelineTaskLabelKey] + if _, ok := pr.Status.Runs[run.Name]; !ok { + // This run was missing from the status. + pr.Status.Runs[run.Name] = &v1beta1.PipelineRunRunStatus{ + PipelineTaskName: pipelineTaskName, + Status: &run.Status, + } + } } - return prStatus } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 3f7760b86aa..8ecc4d8513a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -56,6 +56,7 @@ import ( ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" "knative.dev/pkg/configmap" "knative.dev/pkg/controller" @@ -464,23 +465,36 @@ func TestReconcile_CustomTask(t *testing.T) { names.TestingSeed() const pipelineRunName = "test-pipelinerun-custom-task" const namespace = "namespace" - prt := NewPipelineRunTest(test.Data{PipelineRuns: []*v1beta1.PipelineRun{{ - ObjectMeta: metav1.ObjectMeta{ - Name: pipelineRunName, - Namespace: namespace, - }, - Spec: v1beta1.PipelineRunSpec{ - PipelineSpec: &v1beta1.PipelineSpec{ - Tasks: []v1beta1.PipelineTask{{ - Name: "custom-task", - TaskRef: &v1beta1.TaskRef{ - APIVersion: "example.dev/v0", - Kind: "Example", - }, - }}, + prt := NewPipelineRunTest(test.Data{ + PipelineRuns: []*v1beta1.PipelineRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelineRunName, + Namespace: namespace, + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineSpec: &v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "custom-task", + Params: []v1beta1.Param{{ + Name: "param1", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "value1"}, + }}, + TaskRef: &v1beta1.TaskRef{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + }}, + }, + }, + }}, + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{ + "enable-custom-tasks": "true", + }, }, }, - }}, }, t) defer prt.Cancel() @@ -517,6 +531,10 @@ func TestReconcile_CustomTask(t *testing.T) { Annotations: map[string]string{}, }, Spec: v1alpha1.RunSpec{ + Params: []v1beta1.Param{{ + Name: "param1", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "value1"}, + }}, Ref: &v1beta1.TaskRef{ APIVersion: "example.dev/v0", Kind: "Example", @@ -959,6 +977,79 @@ func TestUpdateTaskRunsState(t *testing.T) { } +func TestUpdateRunsState(t *testing.T) { + // TestUpdateRunsState runs "getRunsStatus" and verifies how it updates a PipelineRun status + // from a Run associated to the PipelineRun + pr := tb.PipelineRun("test-pipeline-run", tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec("test-pipeline")) + pipelineTask := v1beta1.PipelineTask{ + Name: "unit-test-1", + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }}, + TaskRef: &v1beta1.TaskRef{ + APIVersion: "example.dev/v0", + Kind: "Example", + Name: "unit-test-run", + }, + } + run := v1alpha1.Run{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "unit-test-run", + Namespace: "foo", + }, + Spec: v1alpha1.RunSpec{}, + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}, + }, + }, + } + expectedRunsStatus := make(map[string]*v1beta1.PipelineRunRunStatus) + expectedRunsStatus["test-pipeline-run-success-unit-test-1"] = &v1beta1.PipelineRunRunStatus{ + PipelineTaskName: "unit-test-1", + Status: &v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}, + }, + }, + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "foo", + Operator: selection.In, + Values: []string{"foo", "bar"}, + }}, + } + expectedPipelineRunStatus := v1beta1.PipelineRunStatus{ + PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{ + Runs: expectedRunsStatus, + }, + } + + state := resources.PipelineRunState{{ + PipelineTask: &pipelineTask, + CustomTask: true, + RunName: "test-pipeline-run-success-unit-test-1", + Run: &run, + }} + pr.Status.InitializeConditions() + status := state.GetRunsStatus(pr) + if d := cmp.Diff(expectedPipelineRunStatus.Runs, status); d != "" { + t.Fatalf("Expected PipelineRun status to match Run(s) status, but got a mismatch: %s", diff.PrintWantGot(d)) + } + +} + func TestUpdateTaskRunStateWithConditionChecks(t *testing.T) { // TestUpdateTaskRunsState runs "getTaskRunsStatus" and verifies how it updates a PipelineRun status // from several different TaskRun with Conditions associated to the PipelineRun @@ -2962,7 +3053,12 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { tb.PipelineTask("hello-world-1", helloWorldTask.Name), tb.PipelineTask("hello-world-2", helloWorldTask.Name), tb.PipelineTask("hello-world-3", helloWorldTask.Name, tb.PipelineTaskCondition("always-true")), - tb.PipelineTask("hello-world-4", helloWorldTask.Name, tb.PipelineTaskCondition("always-true")))) + tb.PipelineTask("hello-world-4", helloWorldTask.Name, tb.PipelineTaskCondition("always-true")), + tb.PipelineTask("hello-world-5", helloWorldTask.Name), // custom task (corrected below) + )) + + // Update the fifth pipeline task to be a custom task (builder does not support this case). + testPipeline.Spec.Tasks[4].TaskRef = &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example"} // This taskrun is in the pipelinerun status. It completed successfully. taskRunDone := tb.TaskRun("test-pipeline-run-out-of-sync-hello-world-1", @@ -3051,6 +3147,39 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { ), ) + orphanedRun := &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipeline-run-out-of-sync-hello-world-5", + Namespace: "foo", + OwnerReferences: []metav1.OwnerReference{{ + Kind: "PipelineRun", + Name: prOutOfSyncName, + }}, + Labels: map[string]string{ + pipeline.GroupName + pipeline.PipelineLabelKey: testPipeline.Name, + pipeline.GroupName + pipeline.PipelineRunLabelKey: prOutOfSyncName, + pipeline.GroupName + pipeline.PipelineTaskLabelKey: "hello-world-5", + }, + Annotations: map[string]string{}, + }, + Spec: v1alpha1.RunSpec{ + Ref: &v1beta1.TaskRef{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + }, + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + }, + }, + }, + }, + } + prOutOfSync := tb.PipelineRun(prOutOfSyncName, tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec(testPipeline.Name, tb.PipelineRunServiceAccountName("test-sa")), @@ -3076,18 +3205,30 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { ts := []*v1beta1.Task{helloWorldTask} trs := []*v1beta1.TaskRun{taskRunDone, taskRunOrphaned, taskRunWithCondition, taskRunForOrphanedCondition, taskRunForConditionOfOrphanedTaskRun} + runs := []*v1alpha1.Run{orphanedRun} cs := []*v1alpha1.Condition{ tbv1alpha1.Condition("always-true", tbv1alpha1.ConditionNamespace("foo"), tbv1alpha1.ConditionSpec( tbv1alpha1.ConditionSpecCheck("", "foo", tbv1alpha1.Args("bar")), )), } + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.GetNamespace()}, + Data: map[string]string{ + "enable-custom-tasks": "true", + }, + }, + } + d := test.Data{ PipelineRuns: prs, Pipelines: ps, Tasks: ts, TaskRuns: trs, Conditions: cs, + Runs: runs, + ConfigMaps: cms, } prt := NewPipelineRunTest(d, t) defer prt.Cancel() @@ -3106,6 +3247,8 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { switch { case action.Matches("create", "taskruns"): t.Errorf("Expected client to not have created a TaskRun, but it did") + case action.Matches("create", "runs"): + t.Errorf("Expected client to not have created a Run, but it did") case action.Matches("update", "pipelineruns"): pipelineUpdates++ case action.Matches("patch", "pipelineruns"): @@ -3130,6 +3273,7 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { } expectedTaskRunsStatus := make(map[string]*v1beta1.PipelineRunTaskRunStatus) + expectedRunsStatus := make(map[string]*v1beta1.PipelineRunRunStatus) // taskRunDone did not change expectedTaskRunsStatus[taskRunDone.Name] = &v1beta1.PipelineRunTaskRunStatus{ PipelineTaskName: "hello-world-1", @@ -3178,6 +3322,20 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { PipelineTaskName: "hello-world-4", ConditionChecks: prccs4, } + // orphanedRun was recovered into the status + expectedRunsStatus[orphanedRun.Name] = &v1beta1.PipelineRunRunStatus{ + PipelineTaskName: "hello-world-5", + Status: &v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + }, + }, + }, + }, + } // We cannot just diff status directly because the taskrun name for the orphaned condition // is dynamically generated, but we can change the name to allow us to then diff. @@ -3191,6 +3349,9 @@ func TestReconcileOutOfSyncPipelineRun(t *testing.T) { if d := cmp.Diff(reconciledRun.Status.TaskRuns, expectedTaskRunsStatus); d != "" { t.Fatalf("Expected PipelineRun status to match TaskRun(s) status, but got a mismatch: %s", d) } + if d := cmp.Diff(reconciledRun.Status.Runs, expectedRunsStatus); d != "" { + t.Fatalf("Expected PipelineRun status to match Run(s) status, but got a mismatch: %s", d) + } } func TestUpdatePipelineRunStatusFromInformer(t *testing.T) { @@ -3582,7 +3743,9 @@ func TestUpdatePipelineRunStatusFromTaskRuns(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: tc.prName, UID: prUID}, Status: tc.prStatus, } - actualPrStatus := updatePipelineRunStatusFromTaskRuns(logger, pr, tc.prStatus, tc.trs) + + updatePipelineRunStatusFromTaskRuns(logger, pr, tc.trs) + actualPrStatus := pr.Status // The TaskRun keys for recovered taskruns will contain a new random key, appended to the // base name that we expect. Replace the random part so we can diff the whole structure diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index c89f24effe7..911893ccb47 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -21,6 +21,7 @@ import ( "fmt" "strconv" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" @@ -28,10 +29,7 @@ import ( "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/names" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" - "go.uber.org/zap" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" ) @@ -66,11 +64,10 @@ func (e *ConditionNotFoundError) Error() string { type ResolvedPipelineRunTask struct { TaskRunName string TaskRun *v1beta1.TaskRun - // If the PipelineTask is a Custom Task, RunName and Run will be set. - RunName string - Run *v1alpha1.Run - + CustomTask bool + RunName string + Run *v1alpha1.Run PipelineTask *v1beta1.PipelineTask ResolvedTaskResources *resources.ResolvedTaskResources // ConditionChecks ~~TaskRuns but for evaling conditions @@ -84,77 +81,55 @@ func (t ResolvedPipelineRunTask) IsDone(facts *PipelineRunFacts) bool { // IsCustomTask returns true if the PipelineTask references a Custom Task. func (t ResolvedPipelineRunTask) IsCustomTask() bool { - return t.PipelineTask.TaskRef != nil && - t.PipelineTask.TaskRef.APIVersion != "" + return t.CustomTask } -func (t ResolvedPipelineRunTask) IsDone() bool { - if t.PipelineTask == nil { - return false - } - - if !t.IsCustomTask() { - if t.TaskRun == nil { - return false - } - status := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) - retriesDone := len(t.TaskRun.Status.RetriesStatus) - retries := t.PipelineTask.Retries - return status.IsTrue() || status.IsFalse() && retriesDone >= retries - } - return t.Run != nil && t.Run.IsDone() -} - -// IsSuccessful returns true only if the taskrun itself has completed successfully +// IsSuccessful returns true only if the run has completed successfully func (t ResolvedPipelineRunTask) IsSuccessful() bool { - if !t.IsCustomTask() { - if t.TaskRun == nil { - return false - } - return t.TaskRun.IsSuccessful() + if t.IsCustomTask() { + return t.Run != nil && t.Run.IsSuccessful() } - return t.Run != nil && t.Run.IsSuccessful() + return t.TaskRun != nil && t.TaskRun.IsSuccessful() } // IsFailure returns true only if the run has failed and will not be retried. func (t ResolvedPipelineRunTask) IsFailure() bool { - if !t.IsCustomTask() { - if t.TaskRun == nil { - return false - } - c := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) - retriesDone := len(t.TaskRun.Status.RetriesStatus) - retries := t.PipelineTask.Retries - return c.IsFalse() && retriesDone >= retries + if t.IsCustomTask() { + return t.Run != nil && t.Run.IsDone() && !t.Run.IsSuccessful() + } + if t.TaskRun == nil { + return false } - return t.Run != nil && t.Run.IsDone() && !t.Run.IsSuccessful() + c := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) + retriesDone := len(t.TaskRun.Status.RetriesStatus) + retries := t.PipelineTask.Retries + return c.IsFalse() && retriesDone >= retries } -// IsCancelled returns true only if the taskrun itself has cancelled +// IsCancelled returns true only if the run is cancelled func (t ResolvedPipelineRunTask) IsCancelled() bool { - if !t.IsCustomTask() { - if t.TaskRun == nil { - return false - } - - c := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) - if c == nil { + if t.IsCustomTask() { + if t.Run == nil { return false } - - return c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String() + c := t.Run.Status.GetCondition(apis.ConditionSucceeded) + return c != nil && c.IsFalse() && c.Reason == v1alpha1.RunReasonCancelled } - // TODO(#3133): Support cancellation of Custom Task runs. - return false + if t.TaskRun == nil { + return false + } + c := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) + return c != nil && c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String() } // IsStarted returns true only if the PipelineRunTask itself has a TaskRun or // Run associated that has a Succeeded-type condition. func (t ResolvedPipelineRunTask) IsStarted() bool { - if !t.IsCustomTask() { - return t.TaskRun != nil && t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) != nil + if t.IsCustomTask() { + return t.Run != nil && t.Run.Status.GetCondition(apis.ConditionSucceeded) != nil + } - return t.Run != nil && t.Run.Status.GetCondition(apis.ConditionSucceeded) != nil + return t.TaskRun != nil && t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) != nil } func (t *ResolvedPipelineRunTask) checkParentsDone(facts *PipelineRunFacts) bool { @@ -333,15 +308,10 @@ func ValidateServiceaccountMapping(p *v1beta1.PipelineSpec, pr *v1beta1.Pipeline return nil } -<<<<<<< HEAD // ResolvePipelineRunTask retrieves a single Task's instance using the getTask to fetch // the spec. If it is unable to retrieve an instance of a referenced Task, it will return // an error, otherwise it returns a list of all of the Tasks retrieved. It will retrieve // the Resources needed for the TaskRun using the mapping of providedResources. -// -// TODO: The proliferation of Get* methods here is a smell. This method should -// take a versioned clientset instead which includes methods to retrieve these -// resources, with generated fakes for testing. func ResolvePipelineRunTask( ctx context.Context, pipelineRun v1beta1.PipelineRun, @@ -357,6 +327,10 @@ func ResolvePipelineRunTask( PipelineTask: &task, } + cfg := config.FromContextOrDefaults(ctx) + rprt.CustomTask = cfg.FeatureFlags.EnableCustomTasks && rprt.PipelineTask.TaskRef != nil && + rprt.PipelineTask.TaskRef.APIVersion != "" && rprt.PipelineTask.TaskRef.Kind != "" + if rprt.IsCustomTask() { rprt.RunName = GetRunName(pipelineRun.Status.Runs, task.Name, pipelineRun.Name) run, err := getRun(rprt.RunName) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 42a6e45f593..3a25242ae7c 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -18,6 +18,7 @@ package resources import ( "context" + "errors" "fmt" "testing" @@ -25,6 +26,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" tbv1alpha1 "github.com/tektoncd/pipeline/internal/builder/v1alpha1" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" @@ -39,18 +41,18 @@ import ( "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" + logtesting "knative.dev/pkg/logging/testing" ) -func nopGetRun(string) (*v1alpha1.Run, error) { return nil, errors.New("GetRun should not be called") } -func nopGetTask(string) (v1beta1.TaskInterface, error) { +func nopGetRun(string) (*v1alpha1.Run, error) { + return nil, errors.New("GetRun should not be called") +} +func nopGetTask(context.Context, string) (v1beta1.TaskInterface, error) { return nil, errors.New("GetTask should not be called") } func nopGetTaskRun(string) (*v1beta1.TaskRun, error) { return nil, errors.New("GetTaskRun should not be called") } -func nopGetClusterTask(string) (v1beta1.TaskInterface, error) { - return nil, errors.New("GetClusterTask should not be called") -} var pts = []v1beta1.PipelineTask{{ Name: "mytask1", @@ -113,7 +115,10 @@ var pts = []v1beta1.PipelineTask{{ }}, RunAfter: []string{"mytask1"}, }, { - Name: "mytask10-Jason FIXME", + Name: "mytask13", + TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example", Name: "customtask"}, +}, { + Name: "mytask14", TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example", Name: "customtask"}, }} @@ -152,6 +157,20 @@ var trs = []v1beta1.TaskRun{{ Spec: v1beta1.TaskRunSpec{}, }} +var runs = []v1alpha1.Run{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "pipelinerun-mytask13", + }, + Spec: v1alpha1.RunSpec{}, +}, { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "pipelinerun-mytask14", + }, + Spec: v1alpha1.RunSpec{}, +}} + var condition = v1alpha1.Condition{ ObjectMeta: metav1.ObjectMeta{ Name: "always-true", @@ -175,23 +194,47 @@ func makeStarted(tr v1beta1.TaskRun) *v1beta1.TaskRun { return newTr } +func makeRunStarted(run v1alpha1.Run) *v1alpha1.Run { + newRun := newRun(run) + newRun.Status.Conditions[0].Status = corev1.ConditionUnknown + return newRun +} + func makeSucceeded(tr v1beta1.TaskRun) *v1beta1.TaskRun { newTr := newTaskRun(tr) newTr.Status.Conditions[0].Status = corev1.ConditionTrue return newTr } +func makeRunSucceeded(run v1alpha1.Run) *v1alpha1.Run { + newRun := newRun(run) + newRun.Status.Conditions[0].Status = corev1.ConditionTrue + return newRun +} + func makeFailed(tr v1beta1.TaskRun) *v1beta1.TaskRun { newTr := newTaskRun(tr) newTr.Status.Conditions[0].Status = corev1.ConditionFalse return newTr } +func makeRunFailed(run v1alpha1.Run) *v1alpha1.Run { + newRun := newRun(run) + newRun.Status.Conditions[0].Status = corev1.ConditionFalse + return newRun +} + func withCancelled(tr *v1beta1.TaskRun) *v1beta1.TaskRun { tr.Status.Conditions[0].Reason = v1beta1.TaskRunSpecStatusCancelled return tr } +func withRunCancelled(run v1alpha1.Run) *v1alpha1.Run { + newRun := newRun(run) + newRun.Status.Conditions[0].Reason = v1alpha1.RunReasonCancelled + return newRun +} + func withCancelledBySpec(tr *v1beta1.TaskRun) *v1beta1.TaskRun { tr.Spec.Status = v1beta1.TaskRunSpecStatusCancelled return tr @@ -236,6 +279,21 @@ func newTaskRun(tr v1beta1.TaskRun) *v1beta1.TaskRun { } } +func newRun(run v1alpha1.Run) *v1alpha1.Run { + return &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: run.Namespace, + Name: run.Name, + }, + Spec: run.Spec, + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{{Type: apis.ConditionSucceeded}}, + }, + }, + } +} + var noneStartedState = PipelineRunState{{ PipelineTask: &pts[0], TaskRunName: "pipelinerun-mytask1", @@ -316,6 +374,54 @@ var allFinishedState = PipelineRunState{{ }, }} +var noRunStartedState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: nil, +}, { + PipelineTask: &pts[13], + CustomTask: true, + RunName: "pipelinerun-mytask14", + Run: nil, +}} + +var oneRunStartedState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: makeRunStarted(runs[0]), +}, { + PipelineTask: &pts[13], + CustomTask: true, + RunName: "pipelinerun-mytask14", + Run: nil, +}} + +var oneRunFinishedState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: makeRunSucceeded(runs[0]), +}, { + PipelineTask: &pts[13], + CustomTask: true, + RunName: "pipelinerun-mytask14", + Run: nil, +}} + +var oneRunFailedState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: makeRunFailed(runs[0]), +}, { + PipelineTask: &pts[13], + CustomTask: true, + RunName: "pipelinerun-mytask14", + Run: nil, +}} + var successTaskConditionCheckState = TaskConditionCheckState{{ ConditionCheckName: "myconditionCheck", Condition: &condition, @@ -485,6 +591,12 @@ var taskCancelled = PipelineRunState{{ }, }} +var runCancelled = PipelineRunState{{ + PipelineTask: &pts[12], + RunName: "pipelinerun-mytask13", + Run: withRunCancelled(runs[0]), +}} + var taskWithOptionalResourcesDeprecated = &v1beta1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: "task", @@ -855,20 +967,20 @@ func TestIsSkipped(t *testing.T) { "mytask12": false, }, }, { - name: "run-started", - state: oneRunStartedState, + name: "run-started", + state: oneRunStartedState, expected: map[string]bool{ "mytask13": false, }, }, { - name: "run-cancelled", - state: runCancelled, + name: "run-cancelled", + state: runCancelled, expected: map[string]bool{ "mytask13": false, }, }} { t.Run(tc.name, func(t *testing.T) { - dag, err := dagFromState(tc.state) + d, err := dagFromState(tc.state) if err != nil { t.Fatalf("Could not get a dag from the TC state %#v: %v", tc.state, err) } @@ -1098,22 +1210,37 @@ func TestResolvePipelineRun_CustomTask(t *testing.T) { return nil, kerrors.NewNotFound(v1beta1.Resource("run"), name) } nopGetCondition := func(string) (*v1alpha1.Condition, error) { return nil, errors.New("GetCondition should not be called") } - pipelineState, err := ResolvePipelineRun(context.Background(), pr, nopGetTask, nopGetTaskRun, getRun, nopGetClusterTask, nopGetCondition, pts, nil) - if err != nil { - t.Fatalf("ResolvePipelineRun: %v", err) + pipelineState := PipelineRunState{} + ctx := context.Background() + cfg := config.NewStore(logtesting.TestLogger(t)) + cfg.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-custom-tasks": "true", + }, + }) + ctx = cfg.ToContext(ctx) + for _, task := range pts { + ps, err := ResolvePipelineRunTask(ctx, pr, nopGetTask, nopGetTaskRun, getRun, nopGetCondition, task, nil) + if err != nil { + t.Fatalf("ResolvePipelineRunTask: %v", err) + } + pipelineState = append(pipelineState, ps) } expectedState := PipelineRunState{{ PipelineTask: &pts[0], + CustomTask: true, RunName: "pipelinerun-customtask-9l9zj", Run: nil, }, { PipelineTask: &pts[1], + CustomTask: true, RunName: "pipelinerun-run-exists-mz4c7", Run: run, }} if d := cmp.Diff(expectedState, pipelineState); d != "" { - t.Errorf("Unexpected pipelien state: %s", diff.PrintWantGot(d)) + t.Errorf("Unexpected pipeline state: %s", diff.PrintWantGot(d)) } } @@ -1522,18 +1649,14 @@ func TestResolveConditionChecks_MultipleConditions(t *testing.T) { ResolvedResources: providedResources, }} - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - ps, err := ResolvePipelineRunTask(context.Background(), pr, getTask, tc.getTaskRun, nopGetRun, getCondition, pt, providedResources) - if err != nil { - t.Fatalf("Did not expect error when resolving PipelineRun without Conditions: %v", err) - } - pipelineState := PipelineRunState{ps} + ps, err := ResolvePipelineRunTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, getCondition, pt, providedResources) + if err != nil { + t.Fatalf("Did not expect error when resolving PipelineRun without Conditions: %v", err) + } + pipelineState := PipelineRunState{ps} - if d := cmp.Diff(expectedConditionCheck, pipelineState[0].ResolvedConditionChecks, cmpopts.IgnoreUnexported(v1beta1.TaskRunSpec{}, ResolvedConditionCheck{})); d != "" { - t.Fatalf("ConditionChecks did not resolve as expected: %s", diff.PrintWantGot(d)) - } - }) + if d := cmp.Diff(expectedConditionCheck, pipelineState[0].ResolvedConditionChecks, cmpopts.IgnoreUnexported(v1beta1.TaskRunSpec{}, ResolvedConditionCheck{})); d != "" { + t.Fatalf("ConditionChecks did not resolve as expected: %s", diff.PrintWantGot(d)) } } func TestResolveConditionChecks_ConditionDoesNotExist(t *testing.T) { @@ -2015,7 +2138,7 @@ func TestResolvePipeline_WhenExpressions(t *testing.T) { } t.Run("When Expressions exist", func(t *testing.T) { - _, err := ResolvePipelineRunTask(context.Background(), pr, getTask, getTaskRun, getCondition, pt, providedResources) + _, err := ResolvePipelineRunTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, getCondition, pt, providedResources) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) } @@ -2023,41 +2146,59 @@ func TestResolvePipeline_WhenExpressions(t *testing.T) { } func TestIsCustomTask(t *testing.T) { + pr := v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinerun", + }, + } + getTask := func(ctx context.Context, name string) (v1beta1.TaskInterface, error) { return task, nil } + getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil } + getRun := func(name string) (*v1alpha1.Run, error) { return nil, nil } + getCondition := func(name string) (*v1alpha1.Condition, error) { return nil, nil } + for _, tc := range []struct { name string - rprt ResolvedPipelineRunTask + pt v1beta1.PipelineTask want bool }{{ name: "custom", - rprt: ResolvedPipelineRunTask{ - PipelineTask: &v1beta1.PipelineTask{ - TaskRef: &v1beta1.TaskRef{ - APIVersion: "example.dev/v0", - }, + pt: v1beta1.PipelineTask{ + TaskRef: &v1beta1.TaskRef{ + APIVersion: "example.dev/v0", + Kind: "Sample", }, }, want: true, }, { name: "non-custom taskref", - rprt: ResolvedPipelineRunTask{ - PipelineTask: &v1beta1.PipelineTask{ - TaskRef: &v1beta1.TaskRef{ - Name: "task", - }, + pt: v1beta1.PipelineTask{ + TaskRef: &v1beta1.TaskRef{ + Name: "task", }, }, want: false, }, { name: "non-custom taskspec", - rprt: ResolvedPipelineRunTask{ - PipelineTask: &v1beta1.PipelineTask{ - TaskSpec: &v1beta1.EmbeddedTask{}, - }, + pt: v1beta1.PipelineTask{ + TaskSpec: &v1beta1.EmbeddedTask{}, }, want: false, }} { t.Run(tc.name, func(t *testing.T) { - got := tc.rprt.IsCustomTask() + ctx := context.Background() + cfg := config.NewStore(logtesting.TestLogger(t)) + cfg.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-custom-tasks": "true", + }, + }) + ctx = cfg.ToContext(ctx) + rprt, err := ResolvePipelineRunTask(ctx, pr, getTask, getTaskRun, getRun, getCondition, tc.pt, nil) + if err != nil { + t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) + } + got := rprt.IsCustomTask() if d := cmp.Diff(tc.want, got); d != "" { t.Errorf("IsCustomTask: %s", diff.PrintWantGot(d)) } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 80a483d1268..10382dc1cbc 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -80,7 +80,9 @@ func (state PipelineRunState) ToMap() map[string]*ResolvedPipelineRunTask { // IsBeforeFirstTaskRun returns true if the PipelineRun has not yet started its first TaskRun func (state PipelineRunState) IsBeforeFirstTaskRun() bool { for _, t := range state { - if t.TaskRun != nil { + if t.IsCustomTask() && t.Run != nil { + return false + } else if t.TaskRun != nil { return false } } @@ -99,10 +101,15 @@ func (state PipelineRunState) AdjustStartTime(unadjustedStartTime *metav1.Time) adjustedStartTime := unadjustedStartTime for _, rprt := range state { if rprt.TaskRun == nil { - continue - } - if rprt.TaskRun.CreationTimestamp.Time.Before(adjustedStartTime.Time) { - adjustedStartTime = &rprt.TaskRun.CreationTimestamp + if rprt.Run != nil { + if rprt.Run.CreationTimestamp.Time.Before(adjustedStartTime.Time) { + adjustedStartTime = &rprt.Run.CreationTimestamp + } + } + } else { + if rprt.TaskRun.CreationTimestamp.Time.Before(adjustedStartTime.Time) { + adjustedStartTime = &rprt.TaskRun.CreationTimestamp + } } } return adjustedStartTime.DeepCopy() @@ -114,6 +121,9 @@ func (state PipelineRunState) AdjustStartTime(unadjustedStartTime *metav1.Time) func (state PipelineRunState) GetTaskRunsStatus(pr *v1beta1.PipelineRun) map[string]*v1beta1.PipelineRunTaskRunStatus { status := make(map[string]*v1beta1.PipelineRunTaskRunStatus) for _, rprt := range state { + if rprt.IsCustomTask() { + continue + } if rprt.TaskRun == nil && rprt.ResolvedConditionChecks == nil { continue } @@ -161,21 +171,57 @@ func (state PipelineRunState) GetTaskRunsStatus(pr *v1beta1.PipelineRun) map[str return status } +// GetRunsStatus returns a map of run name and the run. +// Ignore a nil run in pipelineRunState, otherwise, capture run object from PipelineRun Status. +// Update run status based on the pipelineRunState before returning it in the map. +func (state PipelineRunState) GetRunsStatus(pr *v1beta1.PipelineRun) map[string]*v1beta1.PipelineRunRunStatus { + status := map[string]*v1beta1.PipelineRunRunStatus{} + for _, rprt := range state { + if !rprt.IsCustomTask() { + continue + } + if rprt.Run == nil && rprt.ResolvedConditionChecks == nil { + continue + } + + var prrs *v1beta1.PipelineRunRunStatus + if rprt.Run != nil { + prrs = pr.Status.Runs[rprt.RunName] + } + + if prrs == nil { + prrs = &v1beta1.PipelineRunRunStatus{ + PipelineTaskName: rprt.PipelineTask.Name, + WhenExpressions: rprt.PipelineTask.WhenExpressions, + } + } + + if rprt.Run != nil { + prrs.Status = &rprt.Run.Status + } + + // TODO(#3133): Include any condition check statuses here too. + status[rprt.RunName] = prrs + } + return status +} + // getNextTasks returns a list of tasks which should be executed next i.e. // a list of tasks from candidateTasks which aren't yet indicated in state to be running and // a list of cancelled/failed tasks from candidateTasks which haven't exhausted their retries func (state PipelineRunState) getNextTasks(candidateTasks sets.String) []*ResolvedPipelineRunTask { tasks := []*ResolvedPipelineRunTask{} for _, t := range state { - if _, ok := candidateTasks[t.PipelineTask.Name]; ok && t.TaskRun == nil { - tasks = append(tasks, t) - } - if _, ok := candidateTasks[t.PipelineTask.Name]; ok && t.TaskRun != nil { - status := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) - if status != nil && status.IsFalse() { - if !(t.TaskRun.IsCancelled() || status.Reason == v1beta1.TaskRunReasonCancelled.String() || status.Reason == ReasonConditionCheckFailed) { - if len(t.TaskRun.Status.RetriesStatus) < t.PipelineTask.Retries { - tasks = append(tasks, t) + if _, ok := candidateTasks[t.PipelineTask.Name]; ok { + if t.TaskRun == nil && t.Run == nil { + tasks = append(tasks, t) + } else if t.TaskRun != nil { // only TaskRun currently supports retry + status := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) + if status != nil && status.IsFalse() { + if !(t.TaskRun.IsCancelled() || status.Reason == v1beta1.TaskRunReasonCancelled.String() || status.Reason == ReasonConditionCheckFailed) { + if len(t.TaskRun.Status.RetriesStatus) < t.PipelineTask.Retries { + tasks = append(tasks, t) + } } } } @@ -375,12 +421,12 @@ func (facts *PipelineRunFacts) getPipelineTasksCount() pipelineRunStatusCount { // increment success counter since the task is successful case t.IsSuccessful(): s.Succeeded++ - // increment failure counter since the task has failed - case t.IsFailure(): - s.Failed++ // increment cancelled counter since the task is cancelled case t.IsCancelled(): s.Cancelled++ + // increment failure counter since the task has failed + case t.IsFailure(): + s.Failed++ // increment skip counter since the task is skipped case t.Skip(facts): s.Skipped++ diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index 498b437049d..06fc000cb86 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" @@ -32,6 +33,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) @@ -99,6 +101,27 @@ func TestPipelineRunFacts_CheckDAGTasksDoneDone(t *testing.T) { }, }} + var runRunningState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: makeRunStarted(runs[0]), + }} + + var runSucceededState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: makeRunSucceeded(runs[0]), + }} + + var runFailedState = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: makeRunFailed(runs[0]), + }} + tcs := []struct { name string state PipelineRunState @@ -139,11 +162,26 @@ func TestPipelineRunFacts_CheckDAGTasksDoneDone(t *testing.T) { state: noTaskRunState, expected: false, ptExpected: []bool{false}, + }, { + name: "run-running-no-candidates", + state: runRunningState, + expected: false, + ptExpected: []bool{false}, + }, { + name: "run-succeeded-no-candidates", + state: runSucceededState, + expected: true, + ptExpected: []bool{true}, + }, { + name: "run-failed-no-candidates", + state: runFailedState, + expected: true, + ptExpected: []bool{true}, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - d, err := DagFromState(tc.state) + d, err := dagFromState(tc.state) if err != nil { t.Fatalf("Unexpected error while buildig DAG for state %v: %v", tc.state, err) } @@ -174,12 +212,24 @@ func TestIsBeforeFirstTaskRun_WithNotStartedTask(t *testing.T) { } } +func TestIsBeforeFirstTaskRun_WithNotStartedRun(t *testing.T) { + if !noRunStartedState.IsBeforeFirstTaskRun() { + t.Fatalf("Expected state to be before first taskrun (Run test)") + } +} + func TestIsBeforeFirstTaskRun_WithStartedTask(t *testing.T) { if oneStartedState.IsBeforeFirstTaskRun() { t.Fatalf("Expected state to be after first taskrun") } } +func TestIsBeforeFirstTaskRun_WithStartedRun(t *testing.T) { + if oneRunStartedState.IsBeforeFirstTaskRun() { + t.Fatalf("Expected state to be after first taskrun (Run test)") + } +} + func TestGetNextTasks(t *testing.T) { tcs := []struct { name string @@ -291,6 +341,21 @@ func TestGetNextTasks(t *testing.T) { state: taskCancelled, candidates: sets.NewString("mytask5"), expectedNext: []*ResolvedPipelineRunTask{}, + }, { + name: "no-runs-started-both-candidates", + state: noRunStartedState, + candidates: sets.NewString("mytask13", "mytask14"), + expectedNext: []*ResolvedPipelineRunTask{noRunStartedState[0], noRunStartedState[1]}, + }, { + name: "one-run-started-both-candidates", + state: oneRunStartedState, + candidates: sets.NewString("mytask13", "mytask14"), + expectedNext: []*ResolvedPipelineRunTask{oneRunStartedState[1]}, + }, { + name: "one-run-failed-both-candidates", + state: oneRunFailedState, + candidates: sets.NewString("mytask13", "mytask14"), + expectedNext: []*ResolvedPipelineRunTask{oneRunFailedState[1]}, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { @@ -460,10 +525,22 @@ func TestPipelineRunState_SuccessfulOrSkippedDAGTasks(t *testing.T) { name: "large deps, not started", state: largePipelineState, expectedNames: []string{}, + }, { + name: "one-run-started", + state: oneRunStartedState, + expectedNames: []string{}, + }, { + name: "one-run-finished", + state: oneRunFinishedState, + expectedNames: []string{pts[12].Name}, + }, { + name: "one-run-failed", + state: oneRunFailedState, + expectedNames: []string{pts[13].Name}, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - d, err := DagFromState(tc.state) + d, err := dagFromState(tc.state) if err != nil { t.Fatalf("Unexpected error while buildig DAG for state %v: %v", tc.state, err) } @@ -743,6 +820,21 @@ func TestGetPipelineConditionStatus(t *testing.T) { }, }} + var cancelledRun = PipelineRunState{{ + PipelineTask: &pts[12], + CustomTask: true, + RunName: "pipelinerun-mytask13", + Run: &v1alpha1.Run{ + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{Conditions: []apis.Condition{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: v1alpha1.RunReasonCancelled, + }}}, + }, + }, + }} + // 6 Tasks, 4 that run in parallel in the beginning // Of the 4, 1 passed, 1 cancelled, 2 failed // 1 runAfter the passed one, currently running @@ -943,11 +1035,17 @@ func TestGetPipelineConditionStatus(t *testing.T) { expectedStatus: corev1.ConditionFalse, expectedReason: v1beta1.PipelineRunReasonCancelled.String(), expectedCancelled: 1, + }, { + name: "cancelled run should result in cancelled pipeline", + state: cancelledRun, + expectedStatus: corev1.ConditionFalse, + expectedReason: v1beta1.PipelineRunReasonCancelled.String(), + expectedCancelled: 1, }} for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { pr := tb.PipelineRun("somepipelinerun") - d, err := DagFromState(tc.state) + d, err := dagFromState(tc.state) if err != nil { t.Fatalf("Unexpected error while buildig DAG for state %v: %v", tc.state, err) } @@ -1089,7 +1187,7 @@ func TestGetPipelineConditionStatus_WithFinalTasks(t *testing.T) { // pipeline should result in timeout if its runtime exceeds its spec.Timeout based on its status.Timeout func TestGetPipelineConditionStatus_PipelineTimeouts(t *testing.T) { - d, err := DagFromState(oneFinishedState) + d, err := dagFromState(oneFinishedState) if err != nil { t.Fatalf("Unexpected error while buildig DAG for state %v: %v", oneFinishedState, err) } @@ -1185,6 +1283,30 @@ func TestAdjustStartTime(t *testing.T) { }}, // We expect this to adjust to the earlier time. want: baseline.Time.Add(-2 * time.Second), + }, { + name: "run starts later", + prs: PipelineRunState{{ + Run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "blah", + CreationTimestamp: metav1.Time{Time: baseline.Time.Add(1 * time.Second)}, + }, + }, + }}, + // Stay where you are, you are before the Run. + want: baseline.Time, + }, { + name: "run starts earlier", + prs: PipelineRunState{{ + Run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "blah", + CreationTimestamp: metav1.Time{Time: baseline.Time.Add(-1 * time.Second)}, + }, + }, + }}, + // We expect this to adjust to the earlier time. + want: baseline.Time.Add(-1 * time.Second), }} for _, test := range tests { diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index 1a43afca6f2..44a088ae546 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -20,6 +20,7 @@ import ( "fmt" "sort" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "knative.dev/pkg/apis" ) @@ -34,6 +35,7 @@ type ResolvedResultRef struct { Value v1beta1.ArrayOrString ResultReference v1beta1.ResultRef FromTaskRun string + FromRun string } // ResolveResultRefs resolves any ResultReference that are found in the target ResolvedPipelineRunTask @@ -206,17 +208,35 @@ func convertPipelineResultToResultRefs(pipelineStatus v1beta1.PipelineRunStatus, } func resolveResultRef(pipelineState PipelineRunState, resultRef *v1beta1.ResultRef) (*ResolvedResultRef, error) { - referencedTaskRun, err := getReferencedTaskRun(pipelineState, resultRef) - if err != nil { - return nil, err + + referencedPipelineTask := pipelineState.ToMap()[resultRef.PipelineTask] + if referencedPipelineTask == nil { + return nil, fmt.Errorf("could not find task %q referenced by result", resultRef.PipelineTask) } - result, err := findTaskResultForParam(referencedTaskRun, resultRef) - if err != nil { - return nil, err + if !referencedPipelineTask.IsSuccessful() { + return nil, fmt.Errorf("task %q referenced by result was not successful", referencedPipelineTask.PipelineTask.Name) + } + + var runName, taskRunName, resultValue string + var err error + if referencedPipelineTask.IsCustomTask() { + runName = referencedPipelineTask.Run.Name + resultValue, err = findRunResultForParam(referencedPipelineTask.Run, resultRef) + if err != nil { + return nil, err + } + } else { + taskRunName = referencedPipelineTask.TaskRun.Name + resultValue, err = findTaskResultForParam(referencedPipelineTask.TaskRun, resultRef) + if err != nil { + return nil, err + } } + return &ResolvedResultRef{ - Value: *v1beta1.NewArrayOrString(result.Value), - FromTaskRun: referencedTaskRun.Name, + Value: *v1beta1.NewArrayOrString(resultValue), + FromTaskRun: taskRunName, + FromRun: runName, ResultReference: *resultRef, }, nil } @@ -238,18 +258,6 @@ func resolveResultRefForPipelineResult(pipelineStatus v1beta1.PipelineRunStatus, }, nil } -func getReferencedTaskRun(pipelineState PipelineRunState, reference *v1beta1.ResultRef) (*v1beta1.TaskRun, error) { - referencedPipelineTask := pipelineState.ToMap()[reference.PipelineTask] - - if referencedPipelineTask == nil { - return nil, fmt.Errorf("could not find task %q referenced by result", reference.PipelineTask) - } - if referencedPipelineTask.TaskRun == nil || referencedPipelineTask.IsFailure() { - return nil, fmt.Errorf("could not find successful taskrun for task %q", referencedPipelineTask.PipelineTask.Name) - } - return referencedPipelineTask.TaskRun, nil -} - func getTaskRunStatus(pipelineStatus v1beta1.PipelineRunStatus, pipelineTaskName string) (*v1beta1.TaskRunStatus, string, error) { for key, taskRun := range pipelineStatus.PipelineRunStatusFields.TaskRuns { // check if the task run was successful @@ -274,14 +282,24 @@ func findTaskResultForPipelineResult(taskStatus *v1beta1.TaskRunStatus, referenc return nil, fmt.Errorf("Could not find result with name %s for task run %s", reference.Result, reference.PipelineTask) } -func findTaskResultForParam(taskRun *v1beta1.TaskRun, reference *v1beta1.ResultRef) (*v1beta1.TaskRunResult, error) { +func findRunResultForParam(run *v1alpha1.Run, reference *v1beta1.ResultRef) (string, error) { + results := run.Status.Results + for _, result := range results { + if result.Name == reference.Result { + return result.Value, nil + } + } + return "", fmt.Errorf("Could not find result with name %s for task %s", reference.Result, reference.PipelineTask) +} + +func findTaskResultForParam(taskRun *v1beta1.TaskRun, reference *v1beta1.ResultRef) (string, error) { results := taskRun.Status.TaskRunStatusFields.TaskRunResults for _, result := range results { if result.Name == reference.Result { - return &result, nil + return result.Value, nil } } - return nil, fmt.Errorf("Could not find result with name %s for task run %s", reference.Result, reference.PipelineTask) + return "", fmt.Errorf("Could not find result with name %s for task %s", reference.Result, reference.PipelineTask) } func (rs ResolvedResultRefs) getStringReplacements() map[string]string { diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go index bf191744bf0..ebe4380e412 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go @@ -8,15 +8,30 @@ import ( "github.com/google/go-cmp/cmp" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/selection" "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) +var ( + successCondition = apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + } + failedCondition = apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + } +) + func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { + for _, tt := range []struct { name string pipelineRunState PipelineRunState @@ -44,6 +59,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { pipelineRunState: PipelineRunState{{ TaskRunName: "aTaskRun", TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus( + tb.StatusCondition(successCondition), tb.TaskRunResult("aResult", "aResultValue"), )), PipelineTask: &v1beta1.PipelineTask{ @@ -69,6 +85,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { pipelineRunState: PipelineRunState{{ TaskRunName: "aTaskRun", TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus( + tb.StatusCondition(successCondition), tb.TaskRunResult("aResult", "aResultValue"), )), PipelineTask: &v1beta1.PipelineTask{ @@ -78,6 +95,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { }, { TaskRunName: "bTaskRun", TaskRun: tb.TaskRun("bTaskRun", tb.TaskRunStatus( + tb.StatusCondition(successCondition), tb.TaskRunResult("bResult", "bResultValue"), )), PipelineTask: &v1beta1.PipelineTask{ @@ -110,6 +128,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { pipelineRunState: PipelineRunState{{ TaskRunName: "aTaskRun", TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus( + tb.StatusCondition(successCondition), tb.TaskRunResult("aResult", "aResultValue"), )), PipelineTask: &v1beta1.PipelineTask{ @@ -134,7 +153,9 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { name: "unsuccessful resolution: referenced result doesn't exist in referenced task", pipelineRunState: PipelineRunState{{ TaskRunName: "aTaskRun", - TaskRun: tb.TaskRun("aTaskRun"), + TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus( + tb.StatusCondition(successCondition), + )), PipelineTask: &v1beta1.PipelineTask{ Name: "aTask", TaskRef: &v1beta1.TaskRef{Name: "aTask"}, @@ -169,13 +190,100 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { }, want: nil, wantErr: true, + }, { + name: "failed resolution: using result reference to a failed task", + pipelineRunState: PipelineRunState{{ + TaskRunName: "aTaskRun", + TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus( + tb.StatusCondition(failedCondition), + )), + PipelineTask: &v1beta1.PipelineTask{ + Name: "aTask", + TaskRef: &v1beta1.TaskRef{Name: "aTask"}, + }, + }}, + param: v1beta1.Param{ + Name: "targetParam", + Value: *v1beta1.NewArrayOrString("$(tasks.aTask.results.aResult)"), + }, + want: nil, + wantErr: true, + }, { + name: "successful resolution: using result reference to a Run", + pipelineRunState: PipelineRunState{{ + CustomTask: true, + RunName: "aRun", + Run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{Name: "aRun"}, + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{successCondition}, + }, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "aResult", + Value: "aResultValue", + }}, + }, + }, + }, + PipelineTask: &v1beta1.PipelineTask{ + Name: "aCustomPipelineTask", + TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example", Name: "aTask"}, + }, + }}, + param: v1beta1.Param{ + Name: "targetParam", + Value: *v1beta1.NewArrayOrString("$(tasks.aCustomPipelineTask.results.aResult)"), + }, + want: ResolvedResultRefs{{ + Value: *v1beta1.NewArrayOrString("aResultValue"), + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aCustomPipelineTask", + Result: "aResult", + }, + FromRun: "aRun", + }}, + wantErr: false, + }, { + name: "failed resolution: using result reference to a failed Run", + pipelineRunState: PipelineRunState{{ + CustomTask: true, + RunName: "aRun", + Run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{Name: "aRun"}, + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{failedCondition}, + }, + }, + }, + PipelineTask: &v1beta1.PipelineTask{ + Name: "aCustomPipelineTask", + TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example", Name: "aTask"}, + }, + }}, + param: v1beta1.Param{ + Name: "targetParam", + Value: *v1beta1.NewArrayOrString("$(tasks.aCustomPipelineTask.results.aResult)"), + }, + want: nil, + wantErr: true, }} { t.Run(tt.name, func(t *testing.T) { t.Logf("test name: %s\n", tt.name) got, err := extractResultRefsForParam(tt.pipelineRunState, tt.param) // sort result ref based on task name to guarantee an certain order sort.SliceStable(got, func(i, j int) bool { - return strings.Compare(got[i].FromTaskRun, got[j].FromTaskRun) < 0 + fromI := got[i].FromTaskRun + if fromI == "" { + fromI = got[i].FromRun + } + fromJ := got[j].FromTaskRun + if fromJ == "" { + fromJ = got[j].FromRun + } + return strings.Compare(fromI, fromJ) < 0 }) if (err != nil) != tt.wantErr { t.Fatalf("ResolveResultRef() error = %v, wantErr %v", err, tt.wantErr) @@ -210,6 +318,7 @@ func TestResolveResultRefs(t *testing.T) { pipelineRunState := PipelineRunState{{ TaskRunName: "aTaskRun", TaskRun: tb.TaskRun("aTaskRun", tb.TaskRunStatus( + tb.StatusCondition(successCondition), tb.TaskRunResult("aResult", "aResultValue"), )), PipelineTask: &v1beta1.PipelineTask{ @@ -254,6 +363,36 @@ func TestResolveResultRefs(t *testing.T) { Value: *v1beta1.NewArrayOrString("$(tasks.aTask.results.missingResult)"), }}, }, + }, { + CustomTask: true, + RunName: "aRun", + Run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{Name: "aRun"}, + Status: v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: []apis.Condition{successCondition}, + }, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "aResult", + Value: "aResultValue", + }}, + }, + }, + }, + PipelineTask: &v1beta1.PipelineTask{ + Name: "aCustomPipelineTask", + TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example", Name: "aTask"}, + }, + }, { + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + Params: []v1beta1.Param{{ + Name: "bParam", + Value: *v1beta1.NewArrayOrString("$(tasks.aCustomPipelineTask.results.aResult)"), + }}, + }, }} for _, tt := range []struct { @@ -316,11 +455,34 @@ func TestResolveResultRefs(t *testing.T) { }, want: nil, wantErr: true, + }, { + name: "Test successful result references resolution - params - Run", + pipelineRunState: pipelineRunState, + targets: PipelineRunState{ + pipelineRunState[6], + }, + want: ResolvedResultRefs{{ + Value: *v1beta1.NewArrayOrString("aResultValue"), + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aCustomPipelineTask", + Result: "aResult", + }, + FromRun: "aRun", + }}, + wantErr: false, }} { t.Run(tt.name, func(t *testing.T) { got, err := ResolveResultRefs(tt.pipelineRunState, tt.targets) sort.SliceStable(got, func(i, j int) bool { - return strings.Compare(got[i].FromTaskRun, got[j].FromTaskRun) < 0 + fromI := got[i].FromTaskRun + if fromI == "" { + fromI = got[i].FromRun + } + fromJ := got[j].FromTaskRun + if fromJ == "" { + fromJ = got[j].FromRun + } + return strings.Compare(fromI, fromJ) < 0 }) if (err != nil) != tt.wantErr { t.Errorf("ResolveResultRefs() error = %v, wantErr %v", err, tt.wantErr) diff --git a/test/controller.go b/test/controller.go index eff94b881b1..2ec9d2e5f16 100644 --- a/test/controller.go +++ b/test/controller.go @@ -20,8 +20,9 @@ import ( "context" "fmt" "sync/atomic" - "testing" // Link in the fakes so they get injected into injection.Fake + "testing" + // Link in the fakes so they get injected into injection.Fake "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" fakepipelineclientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake" @@ -68,6 +69,7 @@ type Data struct { ClusterTasks []*v1beta1.ClusterTask PipelineResources []*v1alpha1.PipelineResource Conditions []*v1alpha1.Condition + Runs []*v1alpha1.Run Pods []*corev1.Pod Namespaces []*corev1.Namespace ConfigMaps []*corev1.ConfigMap @@ -229,6 +231,13 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers t.Fatal(err) } } + c.Pipeline.PrependReactor("*", "runs", AddToInformer(t, i.Run.Informer().GetIndexer())) + for _, run := range d.Runs { + run := run.DeepCopy() // Avoid assumptions that the informer's copy is modified. + if _, err := c.Pipeline.TektonV1alpha1().Runs(run.Namespace).Create(ctx, run, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + } c.Kube.PrependReactor("*", "pods", AddToInformer(t, i.Pod.Informer().GetIndexer())) for _, p := range d.Pods { p := p.DeepCopy() // Avoid assumptions that the informer's copy is modified. diff --git a/test/custom_task_test.go b/test/custom_task_test.go index c1bacae8be8..4cce972cb05 100644 --- a/test/custom_task_test.go +++ b/test/custom_task_test.go @@ -19,11 +19,16 @@ limitations under the License. package test import ( + "context" "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/system" + "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -37,36 +42,61 @@ const ( ) func TestCustomTask(t *testing.T) { - c, namespace := setup(t) - knativetest.CleanupOnInterrupt(func() { tearDown(t, c, namespace) }, t.Logf) - defer tearDown(t, c, namespace) + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + c, namespace := setup(ctx, t, skipIfCustomTasksDisabled) + knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) + defer tearDown(ctx, t, c, namespace) // Create a PipelineRun that runs a Custom Task. pipelineRunName := "custom-task-pipeline" - if _, err := c.PipelineRunClient.Create(&v1beta1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{Name: pipelineRunName}, - Spec: v1beta1.PipelineRunSpec{ - PipelineSpec: &v1beta1.PipelineSpec{ - Tasks: []v1beta1.PipelineTask{{ - Name: "example", - TaskRef: &v1beta1.TaskRef{ - APIVersion: apiVersion, - Kind: kind, - }, - }}, + if _, err := c.PipelineRunClient.Create( + ctx, + &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{Name: pipelineRunName}, + Spec: v1beta1.PipelineRunSpec{ + PipelineSpec: &v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "custom-task", + TaskRef: &v1beta1.TaskRef{ + APIVersion: apiVersion, + Kind: kind, + }, + }, { + Name: "result-consumer", + Params: []v1beta1.Param{{ + Name: "input-result-from-custom-task", Value: *v1beta1.NewArrayOrString("$(tasks.custom-task.results.runResult)"), + }}, + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "input-result-from-custom-task", Type: v1beta1.ParamTypeString, + }}, + Steps: []v1beta1.Step{{Container: corev1.Container{ + Image: "ubuntu", + Command: []string{"/bin/bash"}, + Args: []string{"-c", "echo $(input-result-from-custom-task)"}, + }}}, + }}, + }}, + Results: []v1beta1.PipelineResult{{ + Name: "prResult", + Value: "$(tasks.custom-task.results.runResult)", + }}, + }, }, }, - }); err != nil { + metav1.CreateOptions{}); err != nil { t.Fatalf("Failed to create PipelineRun %q: %v", pipelineRunName, err) } // Wait for the PipelineRun to start. - if err := WaitForPipelineRunState(c, pipelineRunName, time.Minute, Running(pipelineRunName), "PipelineRunRunning"); err != nil { + if err := WaitForPipelineRunState(ctx, c, pipelineRunName, time.Minute, Running(pipelineRunName), "PipelineRunRunning"); err != nil { t.Fatalf("Waiting for PipelineRun to start running: %v", err) } // Get the status of the PipelineRun. - pr, err := c.PipelineRunClient.Get(pipelineRunName, metav1.GetOptions{}) + pr, err := c.PipelineRunClient.Get(ctx, pipelineRunName, metav1.GetOptions{}) if err != nil { t.Fatalf("Failed to get PipelineRun %q: %v", pipelineRunName, err) } @@ -82,7 +112,7 @@ func TestCustomTask(t *testing.T) { } // Get the Run. - r, err := c.RunClient.Get(runName, metav1.GetOptions{}) + r, err := c.RunClient.Get(ctx, runName, metav1.GetOptions{}) if err != nil { t.Fatalf("Failed to get Run %q: %v", runName, err) } @@ -90,20 +120,28 @@ func TestCustomTask(t *testing.T) { t.Fatalf("Run unexpectedly done: %v", r.Status.GetCondition(apis.ConditionSucceeded)) } - // Simulate a Custom Task controller updating the Run to - // done/successful. - r.Status = v1alpha1.RunStatus{Status: duckv1.Status{ - Conditions: duckv1.Conditions{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }} - if _, err := c.RunClient.UpdateStatus(r); err != nil { + // Simulate a Custom Task controller updating the Run to done/successful. + r.Status = v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}, + }, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "runResult", + Value: "aResultValue", + }}, + }, + } + + if _, err := c.RunClient.UpdateStatus(ctx, r, metav1.UpdateOptions{}); err != nil { t.Fatalf("Failed to update Run to successful: %v", err) } // Get the Run. - r, err = c.RunClient.Get(runName, metav1.GetOptions{}) + r, err = c.RunClient.Get(ctx, runName, metav1.GetOptions{}) if err != nil { t.Fatalf("Failed to get Run %q: %v", runName, err) } @@ -112,7 +150,68 @@ func TestCustomTask(t *testing.T) { } // Wait for the PipelineRun to become done/successful. - if err := WaitForPipelineRunState(c, pipelineRunName, time.Minute, PipelineRunSucceed(pipelineRunName), "PipelineRunCompleted"); err != nil { + if err := WaitForPipelineRunState(ctx, c, pipelineRunName, time.Minute, PipelineRunSucceed(pipelineRunName), "PipelineRunCompleted"); err != nil { t.Fatalf("Waiting for PipelineRun to complete successfully: %v", err) } + + // Get the updated status of the PipelineRun. + pr, err = c.PipelineRunClient.Get(ctx, pipelineRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get PipelineRun %q after it completed: %v", pipelineRunName, err) + } + + // Get the TaskRun name. + if len(pr.Status.TaskRuns) != 1 { + t.Fatalf("PipelineRun had unexpected .status.taskRuns; got %d, want 1", len(pr.Status.TaskRuns)) + } + var taskRunName string + for k := range pr.Status.TaskRuns { + taskRunName = k + break + } + + // Get the TaskRun. + taskRun, err := c.TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get TaskRun %q: %v", taskRunName, err) + } + + // Validate the task's result reference to the custom task's result was resolved. + expectedTaskRunParams := []v1beta1.Param{{ + Name: "input-result-from-custom-task", Value: *v1beta1.NewArrayOrString("aResultValue"), + }} + + if d := cmp.Diff(expectedTaskRunParams, taskRun.Spec.Params); d != "" { + t.Fatalf("Unexpected TaskRun Params: %s", diff.PrintWantGot(d)) + } + + // Validate that the pipeline's result reference to the custom task's result was resolved. + // This validation has been commented out because of a race condition. + // The pipelinerun reconciler updates the pipelinerun results AFTER the pipelinerun is marked DONE. + // So when the test case sees the pipelinerun is done, the results may or may not have been + // updated yet. The FUBAR logic in the pipelinerun reconciler needs to be fixed to not mark + // the PR done before all the status updates are made. + + // expectedPipelineResults := []v1beta1.PipelineRunResult{{ + // Name: "prResult", + // Value: "aResultValue", + // }} + + // if len(pr.Status.PipelineResults) == 0 { + // t.Fatalf("Expected PipelineResults but there are none") + // } + // if d := cmp.Diff(expectedPipelineResults, pr.Status.PipelineResults); d != "" { + // t.Fatalf("Unexpected PipelineResults: %s", diff.PrintWantGot(d)) + // } +} + +func skipIfCustomTasksDisabled(ctx context.Context, t *testing.T, c *clients, namespace string) { + featureFlagsCM, err := c.KubeClient.Kube.CoreV1().ConfigMaps(system.GetNamespace()).Get(ctx, config.GetFeatureFlagsConfigName(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get ConfigMap `%s`: %s", config.GetFeatureFlagsConfigName(), err) + } + enableCustomTasks, ok := featureFlagsCM.Data["enable-custom-tasks"] + if !ok || enableCustomTasks != "true" { + t.Skip("Skip because enable-custom-tasks != true") + } } diff --git a/third_party/github.com/docker/cli/cli/config/NOTICE b/third_party/github.com/docker/cli/cli/config/NOTICE index 58b19b6d15b..0c74e15b057 100644 --- a/third_party/github.com/docker/cli/cli/config/NOTICE +++ b/third_party/github.com/docker/cli/cli/config/NOTICE @@ -3,7 +3,7 @@ Copyright 2012-2017 Docker, Inc. This product includes software developed at Docker, Inc. (https://www.docker.com). -This product contains software (https://github.com/creack/pty) developed +This product contains software (https://github.com/kr/pty) developed by Keith Rarick, licensed under the MIT License. The following is courtesy of our legal counsel: diff --git a/vendor/github.com/docker/cli/AUTHORS b/vendor/github.com/docker/cli/AUTHORS index ecb6251ba0d..04edcf794e2 100644 --- a/vendor/github.com/docker/cli/AUTHORS +++ b/vendor/github.com/docker/cli/AUTHORS @@ -11,7 +11,6 @@ Abin Shahab Ace Tang Addam Hardy Adolfo Ochagavía -Adrian Plata Adrien Duermael Adrien Folie Ahmet Alp Balkan @@ -137,7 +136,6 @@ Dafydd Crosby dalanlan Damien Nadé Dan Cotora -Daniel Cassidy Daniel Dao Daniel Farrell Daniel Gasienica @@ -217,7 +215,6 @@ Felix Rabe Filip Jareš Flavio Crisciani Florian Klein -Forest Johnson Foysal Iqbal François Scala Fred Lifton @@ -234,7 +231,6 @@ George MacRorie George Xie Gianluca Borello Gildas Cuisinier -Goksu Toprak Gou Rao Grant Reaber Greg Pflaum @@ -355,7 +351,6 @@ Kara Alexandra Kareem Khazem Karthik Nayak Kat Samperi -Kathryn Spiers Katie McLaughlin Ke Xu Kei Ohmura @@ -377,6 +372,7 @@ Krasi Georgiev Kris-Mikael Krister Kun Zhang Kunal Kushwaha +Kyle Spiers Lachlan Cooper Lai Jiangshan Lars Kellogg-Stedman @@ -541,7 +537,6 @@ Qiang Huang Qinglan Peng qudongfang Raghavendra K T -Ravi Shekhar Jethani Ray Tsang Reficul Remy Suen @@ -558,7 +553,6 @@ Robin Naundorf Robin Speekenbrink Rodolfo Ortiz Rogelio Canedo -Rohan Verma Roland Kammerer Roman Dudin Rory Hunter @@ -707,7 +701,6 @@ Yuan Sun Yue Zhang Yunxiang Huang Zachary Romero -Zander Mackie zebrilee Zhang Kun Zhang Wei diff --git a/vendor/github.com/docker/cli/NOTICE b/vendor/github.com/docker/cli/NOTICE index 58b19b6d15b..0c74e15b057 100644 --- a/vendor/github.com/docker/cli/NOTICE +++ b/vendor/github.com/docker/cli/NOTICE @@ -3,7 +3,7 @@ Copyright 2012-2017 Docker, Inc. This product includes software developed at Docker, Inc. (https://www.docker.com). -This product contains software (https://github.com/creack/pty) developed +This product contains software (https://github.com/kr/pty) developed by Keith Rarick, licensed under the MIT License. The following is courtesy of our legal counsel: diff --git a/vendor/github.com/docker/cli/cli/config/config.go b/vendor/github.com/docker/cli/cli/config/config.go index 6e4d73dfad7..f5e33f2b3e9 100644 --- a/vendor/github.com/docker/cli/cli/config/config.go +++ b/vendor/github.com/docker/cli/cli/config/config.go @@ -106,13 +106,9 @@ func Load(configDir string) (*configfile.ConfigFile, error) { } // Can't find latest config file so check for the old one - homedir, err := os.UserHomeDir() - if err != nil { - return configFile, errors.Wrap(err, oldConfigfile) - } - confFile := filepath.Join(homedir, oldConfigfile) + confFile := filepath.Join(homedir.Get(), oldConfigfile) if _, err := os.Stat(confFile); err != nil { - return configFile, nil // missing file is not an error + return configFile, nil //missing file is not an error } file, err := os.Open(confFile) if err != nil { diff --git a/vendor/github.com/docker/cli/cli/config/configfile/file.go b/vendor/github.com/docker/cli/cli/config/configfile/file.go index a4e97a5caa6..388a5d54d69 100644 --- a/vendor/github.com/docker/cli/cli/config/configfile/file.go +++ b/vendor/github.com/docker/cli/cli/config/configfile/file.go @@ -196,9 +196,6 @@ func (configFile *ConfigFile) Save() error { os.Remove(temp.Name()) return err } - // Try copying the current config file (if any) ownership and permissions - copyFilePermissions(configFile.Filename, temp.Name()) - return os.Rename(temp.Name(), configFile.Filename) } diff --git a/vendor/github.com/docker/cli/cli/config/configfile/file_unix.go b/vendor/github.com/docker/cli/cli/config/configfile/file_unix.go deleted file mode 100644 index 3ca65c6140d..00000000000 --- a/vendor/github.com/docker/cli/cli/config/configfile/file_unix.go +++ /dev/null @@ -1,35 +0,0 @@ -// +build !windows - -package configfile - -import ( - "os" - "syscall" -) - -// copyFilePermissions copies file ownership and permissions from "src" to "dst", -// ignoring any error during the process. -func copyFilePermissions(src, dst string) { - var ( - mode os.FileMode = 0600 - uid, gid int - ) - - fi, err := os.Stat(src) - if err != nil { - return - } - if fi.Mode().IsRegular() { - mode = fi.Mode() - } - if err := os.Chmod(dst, mode); err != nil { - return - } - - uid = int(fi.Sys().(*syscall.Stat_t).Uid) - gid = int(fi.Sys().(*syscall.Stat_t).Gid) - - if uid > 0 && gid > 0 { - _ = os.Chown(dst, uid, gid) - } -} diff --git a/vendor/github.com/docker/cli/cli/config/configfile/file_windows.go b/vendor/github.com/docker/cli/cli/config/configfile/file_windows.go deleted file mode 100644 index 42fffc39ad2..00000000000 --- a/vendor/github.com/docker/cli/cli/config/configfile/file_windows.go +++ /dev/null @@ -1,5 +0,0 @@ -package configfile - -func copyFilePermissions(src, dst string) { - // TODO implement for Windows -} diff --git a/vendor/modules.txt b/vendor/modules.txt index 3d7c4fd2b7a..283e6859ab6 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -123,7 +123,7 @@ github.com/cloudevents/sdk-go/v2/types github.com/davecgh/go-spew/spew # github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/dgrijalva/jwt-go -# github.com/docker/cli v0.0.0-20200303162255-7d407207c304 +# github.com/docker/cli v0.0.0-20200210162036-a4bedce16568 github.com/docker/cli/cli/config github.com/docker/cli/cli/config/configfile github.com/docker/cli/cli/config/credentials