From a5f83d7db1d878bd2dc83e5b5079d527609478ee Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Fri, 10 Nov 2023 16:18:13 -0500 Subject: [PATCH] TEP-0142: Surface step results via termination message This PR surfaces step results (i.e results written to $(step.results..path)) via termination messages. A followup PR will handle surfacing the results via sidecar logs. --- cmd/entrypoint/main.go | 2 + docs/pipeline-api.md | 30 ++- docs/stepactions.md | 83 ++++++- hack/ignored-openapi-violations.list | 1 + pkg/apis/pipeline/v1/openapi_generated.go | 15 +- pkg/apis/pipeline/v1/result_types.go | 3 + pkg/apis/pipeline/v1/swagger.json | 7 + pkg/apis/pipeline/v1/taskrun_types.go | 7 +- pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 7 + .../pipeline/v1alpha1/openapi_generated.go | 2 +- pkg/apis/pipeline/v1alpha1/result_types.go | 2 +- pkg/apis/pipeline/v1alpha1/swagger.json | 2 +- .../pipeline/v1beta1/openapi_generated.go | 15 +- pkg/apis/pipeline/v1beta1/result_types.go | 3 + pkg/apis/pipeline/v1beta1/swagger.json | 7 + .../pipeline/v1beta1/taskrun_conversion.go | 12 + pkg/apis/pipeline/v1beta1/taskrun_types.go | 7 +- .../pipeline/v1beta1/zz_generated.deepcopy.go | 7 + pkg/entrypoint/entrypointer.go | 27 +- pkg/entrypoint/entrypointer_test.go | 41 ++- pkg/pod/entrypoint.go | 53 +++- pkg/pod/entrypoint_test.go | 151 +++++++++++- pkg/pod/pod.go | 4 +- pkg/pod/status.go | 87 ++++++- pkg/pod/status_test.go | 233 ++++++++++++++++++ pkg/reconciler/taskrun/resources/apply.go | 15 +- pkg/reconciler/taskrun/resources/taskspec.go | 6 +- .../taskrun/resources/taskspec_test.go | 70 ++++++ pkg/result/result.go | 4 + pkg/result/result_test.go | 4 + 30 files changed, 863 insertions(+), 44 deletions(-) diff --git a/cmd/entrypoint/main.go b/cmd/entrypoint/main.go index 54422d789e5..dead6fa6581 100644 --- a/cmd/entrypoint/main.go +++ b/cmd/entrypoint/main.go @@ -49,6 +49,7 @@ var ( postFile = flag.String("post_file", "", "If specified, file to write upon completion") terminationPath = flag.String("termination_path", "/tekton/termination", "If specified, file to write upon termination") results = flag.String("results", "", "If specified, list of file names that might contain task results") + stepResults = flag.String("step_results", "", "step results if specified") timeout = flag.Duration("timeout", time.Duration(0), "If specified, sets timeout for step") stdoutPath = flag.String("stdout_path", "", "If specified, file to copy stdout to") stderrPath = flag.String("stderr_path", "", "If specified, file to copy stderr to") @@ -159,6 +160,7 @@ func main() { }, PostWriter: &realPostWriter{}, Results: strings.Split(*results, ","), + StepResults: strings.Split(*stepResults, ","), Timeout: timeout, BreakpointOnFailure: *breakpointOnFailure, OnError: *onError, diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 5705f9ced47..c9f912f3ca7 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -4534,6 +4534,18 @@ string + + +results
+ + +[]TaskRunResult + + + + + +

StepTemplate @@ -5052,7 +5064,7 @@ reasons that emerge from underlying resources are not included here

TaskRunResult

-(Appears on:TaskRunStatusFields) +(Appears on:StepState, TaskRunStatusFields)

TaskRunResult used to describe the results of a task

@@ -7329,7 +7341,7 @@ Refer Go’s ParseDuration documentation for expected format: StepActionSpec)

-

StepActionResult used to describe the results of a task

+

StepActionResult used to describe the results of a StepAction

@@ -13426,6 +13438,18 @@ string + + + +
+results
+ + +[]TaskRunResult + + +
+

StepTemplate @@ -14328,7 +14352,7 @@ reasons that emerge from underlying resources are not included here

TaskRunResult

-(Appears on:TaskRunStatusFields) +(Appears on:StepState, TaskRunStatusFields)

TaskRunResult used to describe the results of a task

diff --git a/docs/stepactions.md b/docs/stepactions.md index 88af514d4c5..854bbecc840 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -39,7 +39,7 @@ A `StepAction` definition supports the following fields: - cannot be used at the same time as using `command`. - `env` - [`params`](#declaring-params) - - [`results`](#declaring-results) + - [`results`](#emitting-results) - [`securityContext`](#declaring-securitycontext) - [`volumeMounts`](#declaring-volumemounts) @@ -93,7 +93,7 @@ spec: ] ``` -### Declaring Results +### Emitting Results A `StepAction` also declares the results that it will emit. @@ -115,6 +115,85 @@ spec: date | tee $(results.current-date-human-readable.path) ``` +In the above example, it is possible that the same `StepAction` is used multiple times in the same `Task` or multiple `StepActions` in the same `Task` produce a result that has the same name, resolving the names becomes critical otherwise there could be unexpected outcomes. The `Task` needs to be able to resolve these `Result` names clashes by mapping it to a different result name. For this reason, we introduce the capability to store results on a `Step` level. + +The above `StepAction` can also emit results to `$(step.results..path)`. + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: stepaction-declaring-results +spec: + results: + - name: current-date-unix-timestamp + description: The current date in unix timestamp format + - name: current-date-human-readable + description: The current date in human readable format + image: bash:latest + script: | + #!/usr/bin/env bash + date +%s | tee $(step.results.current-date-unix-timestamp.path) + date | tee $(step.results.current-date-human-readable.path) +``` + +`Results` from the above `StepAciton` can be [fetched by the `Task`](#fetching-emitted-results-from-step-actions) using this `StepAction` via `$(steps..results.)`. + +#### Fetching Emitted Results from StepActions + +A `Task` can fetch `Results` produced by the `StepActions` (i.e. only results emitted to `$(step.results..path)`, `NOT` $(results..path)) using variable replacement syntax. We introduce a field to [`Task Results`](./tasks.md#emitting-results) called `Value` whose value can be set to the variable `$(steps..results.)`. + +```yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-fetching-results +spec: + results: + - name: git-url + description: "url of git repo" + value: $(steps.git-clone.results.url) + - name: registry-url + description: "url of docker registry" + value: $(steps.kaniko.results.url) + steps: + - name: git-clone + ref: + name: clone-step-action + - name: kaniko + ref: + name: kaniko-step-action +``` + +`Results` emitted to `$(step.results..path)` are not automatically available as `TaskRun Results`. The `Task` must explicitly fetch it from the underlying `Step` referencing `StepActions`. + +For example, lets assume that in the previous example, the "kaniko" `StepAction` also produced a result called "digest". In that case, the `Task` should also fetch the "digest" from "kaniko" `Step`. + +```yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-fetching-results +spec: + results: + - name: git-url + description: "url of git repo" + value: $(steps.git-clone.results.url) + - name: registry-url + description: "url of docker registry" + value: $(steps.kaniko.results.url) + - name: digest + description: "digest of the image" + value: $(steps.kaniko.results.digest) + steps: + - name: git-clone + ref: + name: clone-step-action + - name: kaniko + ref: + name: kaniko-step-action +``` + ### Declaring SecurityContext You can declare `securityContext` in a `StepAction`: diff --git a/hack/ignored-openapi-violations.list b/hack/ignored-openapi-violations.list index 43afc5a82d9..89435f934f7 100644 --- a/hack/ignored-openapi-violations.list +++ b/hack/ignored-openapi-violations.list @@ -49,3 +49,4 @@ API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipe API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Resources API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1,ParamSpec,Enum API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1,ParamSpec,Enum +API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1,StepState,Results diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index e16eee08c82..4995e8cc42c 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -3057,11 +3057,24 @@ func schema_pkg_apis_pipeline_v1_StepState(ref common.ReferenceCallback) common. Format: "", }, }, + "results": { + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskRunResult"), + }, + }, + }, + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/api/core/v1.ContainerStateRunning", "k8s.io/api/core/v1.ContainerStateTerminated", "k8s.io/api/core/v1.ContainerStateWaiting"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskRunResult", "k8s.io/api/core/v1.ContainerStateRunning", "k8s.io/api/core/v1.ContainerStateTerminated", "k8s.io/api/core/v1.ContainerStateWaiting"}, } } diff --git a/pkg/apis/pipeline/v1/result_types.go b/pkg/apis/pipeline/v1/result_types.go index 59083ede906..0db7707c04b 100644 --- a/pkg/apis/pipeline/v1/result_types.go +++ b/pkg/apis/pipeline/v1/result_types.go @@ -52,6 +52,9 @@ type TaskRunResult struct { Value ResultValue `json:"value"` } +// StepResult is a type alias of TaskRunResult +type StepResult = TaskRunResult + // ResultValue is a type alias of ParamValue type ResultValue = ParamValue diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 23b19292f3b..b6c6dafa3fa 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1556,6 +1556,13 @@ "name": { "type": "string" }, + "results": { + "type": "array", + "items": { + "default": {}, + "$ref": "#/definitions/v1.TaskRunResult" + } + }, "running": { "description": "Details about a running container", "$ref": "#/definitions/v1.ContainerStateRunning" diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go index 0b8e9451975..cd8dab39f9a 100644 --- a/pkg/apis/pipeline/v1/taskrun_types.go +++ b/pkg/apis/pipeline/v1/taskrun_types.go @@ -338,9 +338,10 @@ func (trs *TaskRunStatus) SetCondition(newCond *apis.Condition) { // StepState reports the results of running a step in a Task. type StepState struct { corev1.ContainerState `json:",inline"` - Name string `json:"name,omitempty"` - Container string `json:"container,omitempty"` - ImageID string `json:"imageID,omitempty"` + Name string `json:"name,omitempty"` + Container string `json:"container,omitempty"` + ImageID string `json:"imageID,omitempty"` + Results []StepResult `json:"results,omitempty"` } // SidecarState reports the results of running a sidecar in a Task. diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 628630cf446..f2f38b23236 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1330,6 +1330,13 @@ func (in *StepOutputConfig) DeepCopy() *StepOutputConfig { func (in *StepState) DeepCopyInto(out *StepState) { *out = *in in.ContainerState.DeepCopyInto(&out.ContainerState) + if in.Results != nil { + in, out := &in.Results, &out.Results + *out = make([]TaskRunResult, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/apis/pipeline/v1alpha1/openapi_generated.go b/pkg/apis/pipeline/v1alpha1/openapi_generated.go index cb18896855e..1e2b64f08ff 100644 --- a/pkg/apis/pipeline/v1alpha1/openapi_generated.go +++ b/pkg/apis/pipeline/v1alpha1/openapi_generated.go @@ -752,7 +752,7 @@ func schema_pkg_apis_pipeline_v1alpha1_StepActionResult(ref common.ReferenceCall return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Description: "StepActionResult used to describe the results of a task", + Description: "StepActionResult used to describe the results of a StepAction", Type: []string{"object"}, Properties: map[string]spec.Schema{ "name": { diff --git a/pkg/apis/pipeline/v1alpha1/result_types.go b/pkg/apis/pipeline/v1alpha1/result_types.go index df2214875bd..fd6da15b78f 100644 --- a/pkg/apis/pipeline/v1alpha1/result_types.go +++ b/pkg/apis/pipeline/v1alpha1/result_types.go @@ -19,7 +19,7 @@ import ( v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" ) -// StepActionResult used to describe the results of a task +// StepActionResult used to describe the results of a StepAction type StepActionResult struct { // Name the given name Name string `json:"name"` diff --git a/pkg/apis/pipeline/v1alpha1/swagger.json b/pkg/apis/pipeline/v1alpha1/swagger.json index eea6d5d9a67..6e0e4ee9664 100644 --- a/pkg/apis/pipeline/v1alpha1/swagger.json +++ b/pkg/apis/pipeline/v1alpha1/swagger.json @@ -385,7 +385,7 @@ } }, "v1alpha1.StepActionResult": { - "description": "StepActionResult used to describe the results of a task", + "description": "StepActionResult used to describe the results of a StepAction", "type": "object", "required": [ "name" diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 6408a47d891..48e14b34ff2 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4011,11 +4011,24 @@ func schema_pkg_apis_pipeline_v1beta1_StepState(ref common.ReferenceCallback) co Format: "", }, }, + "results": { + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunResult"), + }, + }, + }, + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/api/core/v1.ContainerStateRunning", "k8s.io/api/core/v1.ContainerStateTerminated", "k8s.io/api/core/v1.ContainerStateWaiting"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunResult", "k8s.io/api/core/v1.ContainerStateRunning", "k8s.io/api/core/v1.ContainerStateTerminated", "k8s.io/api/core/v1.ContainerStateWaiting"}, } } diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index 2f484f47ddd..ad9c5e0efa7 100644 --- a/pkg/apis/pipeline/v1beta1/result_types.go +++ b/pkg/apis/pipeline/v1beta1/result_types.go @@ -52,6 +52,9 @@ type TaskRunResult struct { Value ResultValue `json:"value"` } +// StepResult is a type alias of TaskRunResult +type StepResult = TaskRunResult + // ResultValue is a type alias of ParamValue type ResultValue = ParamValue diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 6df62472fbe..2faf4d4595c 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2211,6 +2211,13 @@ "name": { "type": "string" }, + "results": { + "type": "array", + "items": { + "default": {}, + "$ref": "#/definitions/v1beta1.TaskRunResult" + } + }, "running": { "description": "Details about a running container", "$ref": "#/definitions/v1.ContainerStateRunning" diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go index ed3fcc5856e..6773d39254e 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go @@ -330,6 +330,12 @@ func (ss StepState) convertTo(ctx context.Context, sink *v1.StepState) { sink.Name = ss.Name sink.Container = ss.ContainerName sink.ImageID = ss.ImageID + sink.Results = nil + for _, r := range ss.Results { + new := v1.StepResult{} + r.convertTo(ctx, &new) + sink.Results = append(sink.Results, new) + } } func (ss *StepState) convertFrom(ctx context.Context, source v1.StepState) { @@ -337,6 +343,12 @@ func (ss *StepState) convertFrom(ctx context.Context, source v1.StepState) { ss.Name = source.Name ss.ContainerName = source.Container ss.ImageID = source.ImageID + ss.Results = nil + for _, r := range source.Results { + new := StepResult{} + new.convertFrom(ctx, r) + ss.Results = append(ss.Results, new) + } } func (trr TaskRunResult) convertTo(ctx context.Context, sink *v1.TaskRunResult) { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 2b869121d2c..dedc7ef037d 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -367,9 +367,10 @@ func (trs *TaskRunStatus) SetCondition(newCond *apis.Condition) { // StepState reports the results of running a step in a Task. type StepState struct { corev1.ContainerState `json:",inline"` - Name string `json:"name,omitempty"` - ContainerName string `json:"container,omitempty"` - ImageID string `json:"imageID,omitempty"` + Name string `json:"name,omitempty"` + ContainerName string `json:"container,omitempty"` + ImageID string `json:"imageID,omitempty"` + Results []StepResult `json:"results,omitempty"` } // SidecarState reports the results of running a sidecar in a Task. diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index ca255ddee6c..0ef5494ef20 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1802,6 +1802,13 @@ func (in *StepOutputConfig) DeepCopy() *StepOutputConfig { func (in *StepState) DeepCopyInto(out *StepState) { *out = *in in.ContainerState.DeepCopyInto(&out.ContainerState) + if in.Results != nil { + in, out := &in.Results, &out.Results + *out = make([]TaskRunResult, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index ac7baf16dfb..c90b6d0960c 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -96,6 +96,8 @@ type Entrypointer struct { // PostWriter encapsulates writing files when complete. PostWriter PostWriter + // StepResults is the set of files that might contain step results + StepResults []string // Results is the set of files that might contain task results Results []string // Timeout is an optional user-specified duration within which the Step must complete @@ -147,6 +149,9 @@ func (e Entrypointer) Go() error { _ = logger.Sync() }() + if err := os.MkdirAll(filepath.Join(e.StepMetadataDir, "results"), os.ModePerm); err != nil { + return err + } for _, f := range e.WaitFiles { if err := e.Waiter.Wait(context.Background(), f, e.WaitFileContent, e.BreakpointOnFailure); err != nil { // An error happened while waiting, so we bail @@ -237,17 +242,30 @@ func (e Entrypointer) Go() error { if e.ResultsDirectory != "" { resultPath = e.ResultsDirectory } - if err := e.readResultsFromDisk(ctx, resultPath); err != nil { + if err := e.readResultsFromDisk(ctx, resultPath, result.TaskRunResultType); err != nil { logger.Fatalf("Error while handling results: %s", err) } } + if len(e.StepResults) >= 1 && e.StepResults[0] != "" { + stepResultPath := filepath.Join(e.StepMetadataDir, "results") + if e.ResultsDirectory != "" { + stepResultPath = e.ResultsDirectory + } + if err := e.readResultsFromDisk(ctx, stepResultPath, result.StepResultType); err != nil { + logger.Fatalf("Error while handling step results: %s", err) + } + } return err } -func (e Entrypointer) readResultsFromDisk(ctx context.Context, resultDir string) error { +func (e Entrypointer) readResultsFromDisk(ctx context.Context, resultDir string, resultType result.ResultType) error { output := []result.RunResult{} - for _, resultFile := range e.Results { + results := e.Results + if resultType == result.StepResultType { + results = e.StepResults + } + for _, resultFile := range results { if resultFile == "" { continue } @@ -261,9 +279,10 @@ func (e Entrypointer) readResultsFromDisk(ctx context.Context, resultDir string) output = append(output, result.RunResult{ Key: resultFile, Value: string(fileContents), - ResultType: result.TaskRunResultType, + ResultType: resultType, }) } + if e.SpireWorkloadAPI != nil { signed, err := e.SpireWorkloadAPI.Sign(ctx, output) if err != nil { diff --git a/pkg/entrypoint/entrypointer_test.go b/pkg/entrypoint/entrypointer_test.go index 84731daeb24..127d70b992d 100644 --- a/pkg/entrypoint/entrypointer_test.go +++ b/pkg/entrypoint/entrypointer_test.go @@ -262,11 +262,13 @@ func TestReadResultsFromDisk(t *testing.T) { desc string results []string resultContent []v1beta1.ResultValue + resultType result.ResultType want []result.RunResult }{{ desc: "read string result file", results: []string{"results"}, resultContent: []v1beta1.ResultValue{*v1beta1.NewStructuredValues("hello world")}, + resultType: result.TaskRunResultType, want: []result.RunResult{ {Value: `"hello world"`, ResultType: 1}}, @@ -274,6 +276,7 @@ func TestReadResultsFromDisk(t *testing.T) { desc: "read array result file", results: []string{"results"}, resultContent: []v1beta1.ResultValue{*v1beta1.NewStructuredValues("hello", "world")}, + resultType: result.TaskRunResultType, want: []result.RunResult{ {Value: `["hello","world"]`, ResultType: 1}}, @@ -281,12 +284,40 @@ func TestReadResultsFromDisk(t *testing.T) { desc: "read string and array result files", results: []string{"resultsArray", "resultsString"}, resultContent: []v1beta1.ResultValue{*v1beta1.NewStructuredValues("hello", "world"), *v1beta1.NewStructuredValues("hello world")}, + resultType: result.TaskRunResultType, want: []result.RunResult{ {Value: `["hello","world"]`, ResultType: 1}, {Value: `"hello world"`, ResultType: 1}, }, + }, { + desc: "read string step result file", + results: []string{"results"}, + resultContent: []v1beta1.ResultValue{*v1beta1.NewStructuredValues("hello world")}, + resultType: result.StepResultType, + want: []result.RunResult{ + {Value: `"hello world"`, + ResultType: 4}}, + }, { + desc: "read array step result file", + results: []string{"results"}, + resultContent: []v1beta1.ResultValue{*v1beta1.NewStructuredValues("hello", "world")}, + resultType: result.StepResultType, + want: []result.RunResult{ + {Value: `["hello","world"]`, + ResultType: 4}}, + }, { + desc: "read string and array step result files", + results: []string{"resultsArray", "resultsString"}, + resultContent: []v1beta1.ResultValue{*v1beta1.NewStructuredValues("hello", "world"), *v1beta1.NewStructuredValues("hello world")}, + resultType: result.StepResultType, + want: []result.RunResult{ + {Value: `["hello","world"]`, + ResultType: 4}, + {Value: `"hello world"`, + ResultType: 4}, + }, }, } { t.Run(c.desc, func(t *testing.T) { @@ -319,10 +350,11 @@ func TestReadResultsFromDisk(t *testing.T) { e := Entrypointer{ Results: resultsFilePath, + StepResults: resultsFilePath, TerminationPath: terminationPath, ResultExtractionMethod: config.ResultExtractionMethodTerminationMessage, } - if err := e.readResultsFromDisk(ctx, ""); err != nil { + if err := e.readResultsFromDisk(ctx, "", c.resultType); err != nil { t.Fatal(err) } msg, err := os.ReadFile(terminationPath) @@ -462,6 +494,12 @@ func TestEntrypointerResults(t *testing.T) { resultsToWrite: map[string]string{ "foo": "abc", }, + }, { + desc: "write single step result", + entrypoint: "echo", + resultsToWrite: map[string]string{ + "foo": "abc", + }, }, { desc: "write multiple result", entrypoint: "echo", @@ -555,6 +593,7 @@ func TestEntrypointerResults(t *testing.T) { Runner: fr, PostWriter: fpw, Results: results, + StepResults: results, ResultsDirectory: resultsDir, ResultExtractionMethod: config.ResultExtractionMethodTerminationMessage, TerminationPath: terminationPath, diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index d7cd3506405..404a62efcd9 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -57,6 +57,7 @@ const ( readyAnnotation = "tekton.dev/ready" readyAnnotationValue = "READY" + // stepPrefix is the prefix applied to the container name of a step container. stepPrefix = "step-" sidecarPrefix = "sidecar-" @@ -124,7 +125,7 @@ var ( // command, we must have fetched the image's ENTRYPOINT before calling this // method, using entrypoint_lookup.go. // Additionally, Step timeouts are added as entrypoint flag. -func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1.TaskSpec, breakpointConfig *v1.TaskRunDebug, waitForReadyAnnotation, enableKeepPodOnCancel bool) ([]corev1.Container, error) { +func orderContainers(ctx context.Context, commonExtraEntrypointArgs []string, steps []corev1.Container, taskSpec *v1.TaskSpec, breakpointConfig *v1.TaskRunDebug, waitForReadyAnnotation, enableKeepPodOnCancel bool) ([]corev1.Container, error) { if len(steps) == 0 { return nil, errors.New("No steps specified") } @@ -149,6 +150,7 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe "-termination_path", terminationPath, "-step_metadata_dir", filepath.Join(RunDir, idx, "status"), ) + argsForEntrypoint = append(argsForEntrypoint, commonExtraEntrypointArgs...) if taskSpec != nil { if taskSpec.Steps != nil && len(taskSpec.Steps) >= i+1 { @@ -170,6 +172,13 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe } } argsForEntrypoint = append(argsForEntrypoint, resultArgument(steps, taskSpec.Results)...) + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions { + stepResultArgs, err := stepResultArgument(taskSpec.Results, s.Name) + if err != nil { + return nil, err + } + argsForEntrypoint = append(argsForEntrypoint, stepResultArgs...) + } } if breakpointConfig != nil && breakpointConfig.NeedsDebugOnFailure() { @@ -199,6 +208,38 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe return steps, nil } +// stepResultArgument creates the cli arguments for step results to the entrypointer. +// It takes in the slice of Task Results and the StepName as input and if it finds +// that the task result needs a result from the specified step then it adds it to the +// list of arguments. +func stepResultArgument(results []v1.TaskResult, stepName string) ([]string, error) { + if len(results) == 0 { + return nil, nil + } + if stepName == "" { + return nil, nil + } + res := []string{} + for _, r := range results { + if r.Value != nil { + if r.Value.StringVal != "" { + sName, resultName, err := v1.ExtractStepResultName(r.Value.StringVal) + if err != nil { + return nil, err + } + if stepName == sName { + res = append(res, resultName) + } + } + } + } + // No step results need fetching + if len(res) == 0 { + return nil, nil + } + return []string{"-step_results", strings.Join(res, ",")}, nil +} + func resultArgument(steps []corev1.Container, results []v1.TaskResult) []string { if len(results) == 0 { return nil @@ -209,7 +250,9 @@ func resultArgument(steps []corev1.Container, results []v1.TaskResult) []string func collectResultsName(results []v1.TaskResult) string { var resultNames []string for _, r := range results { - resultNames = append(resultNames, r.Name) + if r.Value == nil { + resultNames = append(resultNames, r.Name) + } } return strings.Join(resultNames, ",") } @@ -335,7 +378,11 @@ func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sid // returns "step-unnamed-" if not specified func StepName(name string, i int) string { if name != "" { - return fmt.Sprintf("%s%s", stepPrefix, name) + return getContainerName(name) } return fmt.Sprintf("%sunnamed-%d", stepPrefix, i) } + +func getContainerName(name string) string { + return fmt.Sprintf("%s%s", stepPrefix, name) +} diff --git a/pkg/pod/entrypoint_test.go b/pkg/pod/entrypoint_test.go index 56d57c8a146..0eefc6fbe40 100644 --- a/pkg/pod/entrypoint_test.go +++ b/pkg/pod/entrypoint_test.go @@ -19,6 +19,7 @@ package pod import ( "context" "errors" + "fmt" "testing" "time" @@ -95,7 +96,7 @@ func TestOrderContainers(t *testing.T) { }, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, nil, nil, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, nil, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -163,7 +164,7 @@ func TestOrderContainersWithResultsSidecarLogs(t *testing.T) { }, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{"-dont_send_results_to_termination_path"}, steps, nil, nil, true, false) + got, err := orderContainers(context.Background(), []string{"-dont_send_results_to_termination_path"}, steps, nil, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -209,7 +210,7 @@ func TestOrderContainersWithNoWait(t *testing.T) { VolumeMounts: []corev1.VolumeMount{volumeMount}, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, nil, nil, false, false) + got, err := orderContainers(context.Background(), []string{}, steps, nil, nil, false, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -245,7 +246,7 @@ func TestOrderContainersWithDebugOnFailure(t *testing.T) { OnFailure: "enabled", }, } - got, err := orderContainers([]string{}, steps, nil, taskRunDebugConfig, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, nil, taskRunDebugConfig, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -273,7 +274,137 @@ func TestOrderContainersWithEnabelKeepPodOnCancel(t *testing.T) { VolumeMounts: []corev1.VolumeMount{downwardMount}, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, nil, nil, false, true) + got, err := orderContainers(context.Background(), []string{}, steps, nil, nil, false, true) + if err != nil { + t.Fatalf("orderContainers: %v", err) + } + if d := cmp.Diff(want, got); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } +} + +func TestStepResultArgument(t *testing.T) { + for _, tc := range []struct { + name string + results []v1.TaskResult + stepName string + want []string + }{{ + name: "no results", + stepName: "named-step", + }, { + name: "no stepName", + results: []v1.TaskResult{{ + Name: "sum", + Description: "This is the sum result of the task", + }}, + }, { + name: "fetched step result", + results: []v1.TaskResult{{ + Name: "sum", + Description: "This is the sum result of the task", + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.unnamed-step.results.sum)", + }, + }, { + Name: "sub", + Description: "This is the sub result of the task", + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.named-step.results.sub)", + }, + }}, + stepName: "named-step", + want: []string{"-step_results", "sub"}, + }} { + t.Run(tc.name, func(t *testing.T) { + got, err := stepResultArgument(tc.results, tc.stepName) + if err != nil { + t.Fatalf("stepResultArgument(): Did not expect an error but got %v", err) + } + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + }) + } +} + +func TestStepResultArgumentError(t *testing.T) { + for _, tc := range []struct { + name string + results []v1.TaskResult + stepName string + err error + }{{ + name: "invalid result", + results: []v1.TaskResult{{ + Name: "subNad", + Description: "invalid result", + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "invalid", + }, + }}, + stepName: "named-step", + err: fmt.Errorf(`Could not extract step name and result name. Expected value to look like $(steps..results.) but got "invalid"`), + }} { + t.Run(tc.name, func(t *testing.T) { + got, err := stepResultArgument(tc.results, tc.stepName) + if got != nil { + t.Fatalf("stepResultArgument(): Did not expect an output but got %v", got) + } + if d := cmp.Diff(tc.err.Error(), err.Error()); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + }) + } +} +func TestEntryPointStepActionResults(t *testing.T) { + taskSpec := v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "sum", + Description: "This is the sum result of the task", + }, { + Name: "sub", + Description: "This is the sub result of the task", + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.named-step.results.sub)", + }, + }}, + } + + steps := []corev1.Container{{ + Name: "named-step", + Image: "step-1", + Command: []string{"cmd"}, + Args: []string{"arg1", "arg2"}, + }} + want := []corev1.Container{{ + Name: "named-step", + Image: "step-1", + Command: []string{entrypointBinary}, + Args: []string{ + "-wait_file", "/tekton/downward/ready", + "-wait_file_content", + "-post_file", "/tekton/run/0/out", + "-termination_path", "/tekton/termination", + "-step_metadata_dir", "/tekton/run/0/status", + "-results", "sum", + "-step_results", "sub", + "-entrypoint", "cmd", "--", + "arg1", "arg2", + }, + VolumeMounts: []corev1.VolumeMount{downwardMount}, + TerminationMessagePath: "/tekton/termination", + }} + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + got, err := orderContainers(ctx, []string{}, steps, &taskSpec, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -351,7 +482,7 @@ func TestEntryPointResults(t *testing.T) { }, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, &taskSpec, nil, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, &taskSpec, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -392,7 +523,7 @@ func TestEntryPointResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{downwardMount}, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, &taskSpec, nil, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, &taskSpec, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -429,7 +560,7 @@ func TestEntryPointSingleResultsSingleStep(t *testing.T) { VolumeMounts: []corev1.VolumeMount{downwardMount}, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, &taskSpec, nil, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, &taskSpec, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } @@ -500,7 +631,7 @@ func TestEntryPointOnError(t *testing.T) { err: errors.New("task step onError must be either \"continue\" or \"stopAndFail\" but it is set to an invalid value \"invalid-on-error\""), }} { t.Run(tc.desc, func(t *testing.T) { - got, err := orderContainers([]string{}, steps, &tc.taskSpec, nil, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, &tc.taskSpec, nil, true, false) if len(tc.wantContainers) == 0 { if err == nil { t.Fatalf("expected an error for an invalid value for onError but received none") @@ -599,7 +730,7 @@ func TestEntryPointStepOutputConfigs(t *testing.T) { }, TerminationMessagePath: "/tekton/termination", }} - got, err := orderContainers([]string{}, steps, &taskSpec, nil, true, false) + got, err := orderContainers(context.Background(), []string{}, steps, &taskSpec, nil, true, false) if err != nil { t.Fatalf("orderContainers: %v", err) } diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index cfc7d69d683..04a0a2dec0e 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -239,9 +239,9 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1.TaskRun, taskSpec v1.Ta readyImmediately := isPodReadyImmediately(*featureFlags, taskSpec.Sidecars) if alphaAPIEnabled { - stepContainers, err = orderContainers(commonExtraEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug, !readyImmediately, enableKeepPodOnCancel) + stepContainers, err = orderContainers(ctx, commonExtraEntrypointArgs, stepContainers, &taskSpec, taskRun.Spec.Debug, !readyImmediately, enableKeepPodOnCancel) } else { - stepContainers, err = orderContainers(commonExtraEntrypointArgs, stepContainers, &taskSpec, nil, !readyImmediately, enableKeepPodOnCancel) + stepContainers, err = orderContainers(ctx, commonExtraEntrypointArgs, stepContainers, &taskSpec, nil, !readyImmediately, enableKeepPodOnCancel) } if err != nil { return nil, err diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 7c52ca4ce32..1921ed5b755 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -186,7 +186,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL } // populate task run CRD with results from sidecar logs - taskResults, _ := filterResults(sidecarLogResults, specResults) + taskResults, _, _ := filterResults(sidecarLogResults, specResults, nil) if tr.IsDone() { trs.Results = append(trs.Results, taskResults...) } @@ -195,6 +195,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL for _, s := range stepStatuses { // Avoid changing the original value by modifying the pointer value. state := s.State.DeepCopy() + stepResults := []v1.TaskRunResult{} if state.Terminated != nil && len(state.Terminated.Message) != 0 { msg := state.Terminated.Message @@ -214,9 +215,14 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL merr = multierror.Append(merr, err) } - taskResults, filteredResults := filterResults(results, specResults) + neededStepResults, err := findStepResultsFetchedByTask(s.Name, specResults) + if err != nil { + merr = multierror.Append(merr, err) + } + taskResults, stepRes, filteredResults := filterResults(results, specResults, neededStepResults) if tr.IsDone() { trs.Results = append(trs.Results, taskResults...) + stepResults = append(stepResults, stepRes...) } msg, err = createMessageFromResults(filteredResults) if err != nil { @@ -238,6 +244,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL Name: trimStepPrefix(s.Name), Container: s.Name, ImageID: s.ImageID, + Results: stepResults, }) } @@ -266,15 +273,46 @@ func createMessageFromResults(results []result.RunResult) (string, error) { return string(bytes), nil } +// findStepResultsFetchedByTask fetches step results that the Task needs. +// It accepts a container name and the TaskResults as input and outputs +// a map with the name of the step result as the key and the name of the task result that is fetching it as value. +func findStepResultsFetchedByTask(containerName string, specResults []v1.TaskResult) (map[string]string, error) { + neededStepResults := map[string]string{} + for _, r := range specResults { + if r.Value != nil { + if r.Value.StringVal != "" { + sName, resultName, err := v1.ExtractStepResultName(r.Value.StringVal) + if err != nil { + return nil, err + } + // Only look at named results - referencing unnamed steps is unsupported. + if getContainerName(sName) == containerName { + neededStepResults[resultName] = r.Name + } + } + } + } + return neededStepResults, nil +} + // filterResults filters the RunResults and TaskResults based on the results declared in the task spec. // It returns a slice of any of the input results that are defined in the task spec, converted to TaskRunResults, // and a slice of any of the RunResults that don't represent internal values (i.e. those that should not be displayed in the TaskRun status. -func filterResults(results []result.RunResult, specResults []v1.TaskResult) ([]v1.TaskRunResult, []result.RunResult) { +func filterResults(results []result.RunResult, specResults []v1.TaskResult, neededStepResults map[string]string) ([]v1.TaskRunResult, []v1.StepResult, []result.RunResult) { var taskResults []v1.TaskRunResult + var stepResults []v1.TaskRunResult var filteredResults []result.RunResult neededTypes := make(map[string]v1.ResultsType) + neededStepTypes := make(map[string]v1.ResultsType) for _, r := range specResults { neededTypes[r.Name] = r.Type + // identify the type of the task result that is fetching step result + // this way we can either render it as a json string or unmarshal it to an array/object + for k, v := range neededStepResults { + if v == r.Name { + neededStepTypes[k] = r.Type + } + } } for _, r := range results { switch r.ResultType { @@ -300,6 +338,46 @@ func filterResults(results []result.RunResult, specResults []v1.TaskResult) ([]v } taskResults = append(taskResults, taskRunResult) filteredResults = append(filteredResults, r) + case result.StepResultType: + if neededStepTypes[r.Key] == v1.ResultsTypeString { + stepResult := v1.StepResult{ + Name: r.Key, + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues(r.Value), + } + // this result was requested by the Task + if _, ok := neededStepResults[r.Key]; ok { + taskRunResult := v1.TaskRunResult{ + Name: neededStepResults[r.Key], + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues(r.Value), + } + taskResults = append(taskResults, taskRunResult) + } + stepResults = append(stepResults, stepResult) + } else { + v := v1.ResultValue{} + err := v.UnmarshalJSON([]byte(r.Value)) + if err != nil { + continue + } + stepResult := v1.StepResult{ + Name: r.Key, + Type: v1.ResultsType(v.Type), + Value: v, + } + stepResults = append(stepResults, stepResult) + // this result was requested by the Task + if _, ok := neededStepResults[r.Key]; ok { + taskRunResult := v1.TaskRunResult{ + Name: neededStepResults[r.Key], + Type: v1.ResultsType(v.Type), + Value: v, + } + taskResults = append(taskResults, taskRunResult) + } + } + filteredResults = append(filteredResults, r) case result.InternalTektonResultType: // Internal messages are ignored because they're not used as external result continue @@ -307,8 +385,7 @@ func filterResults(results []result.RunResult, specResults []v1.TaskResult) ([]v filteredResults = append(filteredResults, r) } } - - return taskResults, filteredResults + return taskResults, stepResults, filteredResults } func removeDuplicateResults(taskRunResult []v1.TaskRunResult) []v1.TaskRunResult { diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index 77c0176399a..8fa594235c1 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -189,6 +189,225 @@ func TestSetTaskRunStatusBasedOnStepStatus_sidecar_logs(t *testing.T) { } } +func TestMakeTaskRunStatus_StepResults(t *testing.T) { + for _, c := range []struct { + desc string + podStatus corev1.PodStatus + pod corev1.Pod + tr v1.TaskRun + want v1.TaskRunStatus + }{{ + desc: "step results string type", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-one", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"uri","value":"https://foo.bar\n","type":4}]`, + }, + }, + }}, + }, + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "task-result", + Type: v1.ResultsTypeString, + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.one.results.uri)", + }, + }}, + }, + }, + }, + want: v1.TaskRunStatus{ + Status: statusSuccess(), + TaskRunStatusFields: v1.TaskRunStatusFields{ + Steps: []v1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"uri","value":"https://foo.bar\n","type":4}]`, + }}, + Name: "one", + Container: "step-one", + Results: []v1.TaskRunResult{{ + Name: "uri", + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues("https://foo.bar\n"), + }}, + }}, + Sidecars: []v1.SidecarState{}, + Results: []v1.TaskRunResult{{ + Name: "task-result", + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues("https://foo.bar\n"), + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "step results array type", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-one", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"array","value":"[\"hello\",\"world\"]","type":4}]`, + }, + }, + }}, + }, + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "resultName", + Type: v1.ResultsTypeArray, + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.one.results.array)", + }, + }}, + }, + }, + }, + want: v1.TaskRunStatus{ + Status: statusSuccess(), + TaskRunStatusFields: v1.TaskRunStatusFields{ + Steps: []v1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"array","value":"[\"hello\",\"world\"]","type":4}]`, + }}, + Name: "one", + Container: "step-one", + Results: []v1.TaskRunResult{{ + Name: "array", + Type: v1.ResultsTypeArray, + Value: *v1.NewStructuredValues("hello", "world"), + }}, + }}, + Sidecars: []v1.SidecarState{}, + Results: []v1.TaskRunResult{{ + Name: "resultName", + Type: v1.ResultsTypeArray, + Value: *v1.NewStructuredValues("hello", "world"), + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "filter task results from step results", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-one", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:1234","type":4},{"key":"resultName","value":"resultValue","type":4}]`, + }, + }, + }}, + }, + tr: v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Results: []v1.TaskResult{{ + Name: "resultDigest", + Type: v1.ResultsTypeString, + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.one.results.digest)", + }, + }}, + }, + }, + }, + want: v1.TaskRunStatus{ + Status: statusSuccess(), + TaskRunStatusFields: v1.TaskRunStatusFields{ + Steps: []v1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:1234","type":4},{"key":"resultName","value":"resultValue","type":4}]`, + }}, + Name: "one", + Container: "step-one", + Results: []v1.TaskRunResult{{ + Name: "digest", + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues("sha256:1234"), + }, { + Name: "resultName", + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues("resultValue"), + }}, + }}, + Sidecars: []v1.SidecarState{}, + Results: []v1.TaskRunResult{{ + Name: "resultDigest", + Type: v1.ResultsTypeString, + Value: *v1.NewStructuredValues("sha256:1234"), + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }} { + t.Run(c.desc, func(t *testing.T) { + now := metav1.Now() + if cmp.Diff(c.pod, corev1.Pod{}) == "" { + c.pod = corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Namespace: "foo", + CreationTimestamp: now, + }, + Status: c.podStatus, + } + } + + logger, _ := logging.NewLogger("", "status") + kubeclient := fakek8s.NewSimpleClientset() + got, err := MakeTaskRunStatus(context.Background(), logger, c.tr, &c.pod, kubeclient, &v1.TaskSpec{}) + if err != nil { + t.Errorf("MakeTaskRunResult: %s", err) + } + + // Common traits, set for test case brevity. + c.want.PodName = "pod" + + ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool { + if x == nil { + return y == nil + } + return y != nil + }) + if d := cmp.Diff(c.want, got, ignoreVolatileTime, ensureTimeNotNil); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestMakeTaskRunStatus(t *testing.T) { for _, c := range []struct { desc string @@ -1276,6 +1495,11 @@ func TestMakeTaskRunStatus(t *testing.T) { // Common traits, set for test case brevity. c.want.PodName = "pod" c.want.StartTime = &metav1.Time{Time: startTime} + for i := range c.want.TaskRunStatusFields.Steps { + if c.want.TaskRunStatusFields.Steps[i].Results == nil { + c.want.TaskRunStatusFields.Steps[i].Results = []v1.TaskRunResult{} + } + } ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool { if x == nil { @@ -1554,6 +1778,11 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) { // Common traits, set for test case brevity. c.want.PodName = "pod" c.want.StartTime = &metav1.Time{Time: startTime} + for i := range c.want.TaskRunStatusFields.Steps { + if c.want.TaskRunStatusFields.Steps[i].Results == nil { + c.want.TaskRunStatusFields.Steps[i].Results = []v1.TaskRunResult{} + } + } ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool { if x == nil { @@ -1632,24 +1861,28 @@ func TestMakeRunStatusJSONError(t *testing.T) { }}, Name: "non-json", Container: "step-non-json", + Results: []v1.TaskRunResult{}, ImageID: "image", }, { ContainerState: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{}}, Name: "after-non-json", Container: "step-after-non-json", + Results: []v1.TaskRunResult{}, ImageID: "image", }, { ContainerState: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{}}, Name: "this-step-might-panic", Container: "step-this-step-might-panic", + Results: []v1.TaskRunResult{}, ImageID: "image", }, { ContainerState: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{}}, Name: "foo", Container: "step-foo", + Results: []v1.TaskRunResult{}, ImageID: "image", }}, Sidecars: []v1.SidecarState{}, diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index 0af13115fda..5f67bead047 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -24,6 +24,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/container" "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/substitution" @@ -44,14 +45,20 @@ var ( // FIXME(vdemeester) Remove that with deprecating v1beta1 "inputs.params.%s", } + stepActionResultPatterns = []string{ + "step.results.%s.path", + "step.results[%q].path", + "step.results['%s'].path", + } ) // applyStepActionParameters applies the params from the Task and the underlying Step to the referenced StepAction. -func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, stepParams v1.Params, defaults []v1.ParamSpec) *v1.Step { +func applyStepActionParametersAndResults(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, stepParams v1.Params, stepActionSpec v1alpha1.StepActionSpec, stepName string) *v1.Step { if stepParams != nil { stringR, arrayR, objectR := getTaskParameters(spec, tr, spec.Params...) stepParams = stepParams.ReplaceVariables(stringR, arrayR, objectR) } + defaults := stepActionSpec.Params // Set params from StepAction defaults stringReplacements, arrayReplacements, _ := replacementsFromDefaultParams(defaults) @@ -63,6 +70,12 @@ func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, for k, v := range stepArrays { arrayReplacements[k] = v } + // Overwrite step result path reference + for _, result := range stepActionSpec.Results { + for _, pattern := range stepActionResultPatterns { + stringReplacements[fmt.Sprintf(pattern, result.Name)] = filepath.Join(pipeline.StepsDir, stepName, "results", result.Name) + } + } container.ApplyStepReplacements(step, stringReplacements, arrayReplacements) return step diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index cf24b19b977..4dc2587eea3 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -25,6 +25,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned" resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution" + "github.com/tektoncd/pipeline/pkg/pod" remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource" "github.com/tektoncd/pipeline/pkg/trustedresources" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -103,7 +104,7 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask) (*re // GetStepActionsData extracts the StepActions and merges them with the inlined Step specification. func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.TaskRun, tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester) ([]v1.Step, error) { steps := []v1.Step{} - for _, step := range taskSpec.Steps { + for i, step := range taskSpec.Steps { s := step.DeepCopy() if step.Ref != nil { getStepAction := GetStepActionFunc(tekton, k8s, requester, taskRun, s) @@ -134,7 +135,8 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T if err := validateStepHasStepActionParameters(s.Params, stepActionSpec.Params); err != nil { return nil, err } - s = applyStepActionParameters(s, &taskSpec, taskRun, s.Params, stepActionSpec.Params) + stepName := pod.StepName(s.Name, i) + s = applyStepActionParametersAndResults(s, &taskSpec, taskRun, s.Params, stepActionSpec, stepName) s.Params = nil s.Ref = nil steps = append(steps, *s) diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index 33e11ae9e9b..73d71c9183a 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -786,6 +786,76 @@ func TestGetStepActionsData(t *testing.T) { Image: "myimage", Args: []string{"taskrun string param", "taskspec", "array", "taskspec", "array", "param", "taskrun key", "taskspec key2", "step action key3"}, }}, + }, { + name: "an unnamed step emitting results to step result path", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + }, + }, + }, + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepAction", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Results: []v1alpha1.StepActionResult{{ + Name: "resultName", + }}, + Script: "echo hello | tee $(step.results.resultName.path)", + }, + }, + want: []v1.Step{{ + Image: "myimage", + Script: "echo hello | tee /tekton/steps/step-unnamed-0/results/resultName", + }}, + }, { + name: "a named step emitting results to step result path", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "mystep", + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + }, + }, + }, + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepAction", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Results: []v1alpha1.StepActionResult{{ + Name: "resultName", + }}, + Script: "echo hello | tee $(step.results.resultName.path)", + }, + }, + want: []v1.Step{{ + Name: "mystep", + Image: "myimage", + Script: "echo hello | tee /tekton/steps/step-mystep/results/resultName", + }}, }} for _, tt := range tests { ctx := context.Background() diff --git a/pkg/result/result.go b/pkg/result/result.go index cfcbc3e90a2..515fe9a6025 100644 --- a/pkg/result/result.go +++ b/pkg/result/result.go @@ -33,6 +33,8 @@ const ( InternalTektonResultType = 3 // UnknownResultType default unknown result type value UnknownResultType = 10 + // StepResultType default step result value + StepResultType ResultType = 4 ) // RunResult is used to write key/value pairs to TaskRun pod termination messages. @@ -80,6 +82,8 @@ func (r *ResultType) UnmarshalJSON(data []byte) error { } switch asString { + case "StepResult": + *r = StepResultType case "TaskRunResult": *r = TaskRunResultType case "InternalTektonResult": diff --git a/pkg/result/result_test.go b/pkg/result/result_test.go index ee2e5c366c4..41006aed079 100644 --- a/pkg/result/result_test.go +++ b/pkg/result/result_test.go @@ -33,6 +33,10 @@ func TestRunResult_UnmarshalJSON(t *testing.T) { name: "type defined as string - TaskRunResult", data: "{\"key\":\"resultName\",\"value\":\"resultValue\", \"type\": \"TaskRunResult\"}", pr: RunResult{Key: "resultName", Value: "resultValue", ResultType: TaskRunResultType}, + }, { + name: "type defined as string - StepResult", + data: "{\"key\":\"resultName\",\"value\":\"resultValue\", \"type\": \"StepResult\"}", + pr: RunResult{Key: "resultName", Value: "resultValue", ResultType: StepResultType}, }, { name: "type defined as string - InternalTektonResult",