Skip to content

Commit

Permalink
Label user error for failure PipelineRun Status
Browse files Browse the repository at this point in the history
This commit labels the user errors for failed PipelineRun status. This aims to
communicate explicitly with users of whether the run failed could be
attributed to users' responsibility.

/kind misc
part of tektoncd#7276
  • Loading branch information
JeromeJu committed Dec 8, 2023
1 parent a179226 commit 27b35f7
Show file tree
Hide file tree
Showing 14 changed files with 176 additions and 41 deletions.
25 changes: 25 additions & 0 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ weight: 204
- [<code>PipelineRun</code> status](#pipelinerun-status)
- [The <code>status</code> field](#the-status-field)
- [Monitoring execution status](#monitoring-execution-status)
- [Marking off user errors](#marking-off-user-errors)
- [Cancelling a <code>PipelineRun</code>](#cancelling-a-pipelinerun)
- [Gracefully cancelling a <code>PipelineRun</code>](#gracefully-cancelling-a-pipelinerun)
- [Gracefully stopping a <code>PipelineRun</code>](#gracefully-stopping-a-pipelinerun)
Expand Down Expand Up @@ -1538,6 +1539,30 @@ Some examples:
| pipeline-run-0123456789-0123456789-0123456789-0123456789 | task2-0123456789-0123456789-0123456789-0123456789-0123456789 | pipeline-run-0123456789-012345607ad8c7aac5873cdfabe472a68996b5c |
| pipeline-run | task4 (with 2x2 `Matrix`) | pipeline-run-task1-0, pipeline-run-task1-2, pipeline-run-task1-3, pipeline-run-task1-4 |

### Marking off user errors

A user error in Tekton is any mistake made by user, such as a syntax error when specifying pipelines, tasks. User errors can occur in various stages of the Tekton pipeline, from authoring the pipeline configuration to executing the pipelines. They are currently explicitly labeled in the Run's conditions message, for example:

```yaml
# Failed PipelineRun with "user error" labeled
apiVersion: tekton.dev/v1 # or tekton.dev/v1beta1
kind: PipelineRun
metadata:
...
spec:
...
status:
...
conditions:
- lastTransitionTime: "2022-06-02T19:02:58Z"
message: |-
PipelineRun default parameters is missing some parameters required by Pipeline pipelinerun-with-params's parameters: user error
pipelineRun missing parameters: [param foo]
reason: ParameterMissing
status: "False"
type: Succeeded
```

## Cancelling a `PipelineRun`

To cancel a `PipelineRun` that's currently executing, update its definition
Expand Down
35 changes: 35 additions & 0 deletions pkg/apis/pipeline/errors/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
Copyright 2023 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 errors

import (
"errors"
)

var (
// ErrUser is used to indicate that the user is responsible for an aerror.
// See github.com/tektoncd/pipeline/blob/main/docs/pipelineruns.md#marking-off-user-errors
// for more details.
ErrUser = errors.New("user error")
)

// LabelUserError adds the "user error" mark of the error message
func LabelUserError(err error) error {
err = errors.Join(ErrUser, err)
return err
}

func IsUserError(err error) bool {
return errors.Is(err, ErrUser)
}
64 changes: 64 additions & 0 deletions pkg/apis/pipeline/errors/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
Copyright 2023 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 errors_test

import (
"errors"
"testing"

pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
)

type TestError struct{}

var _ error = &TestError{}

func (*TestError) Error() string {
return "test error"
}

func TestUserErrorUnwrap(t *testing.T) {
originalError := &TestError{}
userError := pipelineErrors.LabelUserError(originalError)

if !errors.Is(userError, pipelineErrors.ErrUser) {
t.Errorf("user error expected to unwrap successfully")
}
}

func TestResolutionErrorMessage(t *testing.T) {
originalError := &TestError{}
expectedErrorMessage := pipelineErrors.ErrUser.Error() + "\n" + originalError.Error()

userError := pipelineErrors.LabelUserError(originalError)

if userError.Error() != expectedErrorMessage {
t.Errorf("user error message expected to equal to %s, got: %s", expectedErrorMessage, userError.Error())
}
}

func TestIsUserError(t *testing.T) {
originalError := &TestError{}
if pipelineErrors.IsUserError(originalError) {
t.Errorf("IsUserError fails to unwrap non-user error")
}

userError := pipelineErrors.LabelUserError(originalError)
if !pipelineErrors.IsUserError(userError) {
t.Errorf("IsUserError fails to unwrap user error")
}
}
3 changes: 2 additions & 1 deletion pkg/reconciler/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"

"github.com/google/uuid"
pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
Expand Down Expand Up @@ -76,7 +77,7 @@ func handleDryRunCreateErr(err error, objectName string) error {
case apierrors.IsBadRequest(err): // Object rejected by validating webhook
errType = ErrReferencedObjectValidationFailed
case apierrors.IsInvalid(err), apierrors.IsMethodNotSupported(err):
errType = ErrCouldntValidateObjectPermanent
errType = pipelineErrors.LabelUserError(ErrCouldntValidateObjectPermanent)
case apierrors.IsTimeout(err), apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err):
errType = ErrCouldntValidateObjectRetryable
default:
Expand Down
25 changes: 15 additions & 10 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/hashicorp/go-multierror"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
Expand Down Expand Up @@ -443,7 +444,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
// This Run has failed, so we need to mark it as failed and stop reconciling it
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidGraph.String(),
"PipelineRun %s/%s's Pipeline DAG is invalid: %s",
pr.Namespace, pr.Name, err)
pr.Namespace, pr.Name, pipelineErrors.LabelUserError(err))
return controller.NewPermanentError(err)
}

Expand All @@ -456,7 +457,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
// This Run has failed, so we need to mark it as failed and stop reconciling it
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidGraph.String(),
"PipelineRun %s's Pipeline DAG is invalid for finally clause: %s",
pr.Namespace, pr.Name, err)
pr.Namespace, pr.Name, pipelineErrors.LabelUserError(err))
return controller.NewPermanentError(err)
}

Expand Down Expand Up @@ -492,7 +493,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
logger.Errorf("PipelineRun %q Param Enum validation failed: %v", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidParamValue.String(),
"PipelineRun %s/%s parameters have invalid value: %s",
pr.Namespace, pr.Name, err)
pr.Namespace, pr.Name, pipelineErrors.LabelUserError(err))
return controller.NewPermanentError(err)
}
}
Expand Down Expand Up @@ -655,7 +656,8 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
err := rpt.EvaluateCEL()
if err != nil {
logger.Errorf("Error evaluating CEL %s: %v", pr.Name, err)
pr.Status.MarkFailed(string(v1.PipelineRunReasonCELEvaluationFailed), err.Error())
pr.Status.MarkFailed(string(v1.PipelineRunReasonCELEvaluationFailed),
"Error evaluating CEL %s: %w", pr.Name, pipelineErrors.LabelUserError(err))
return controller.NewPermanentError(err)
}
}
Expand Down Expand Up @@ -684,8 +686,9 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
}

if err := resources.ValidateOptionalWorkspaces(pipelineSpec.Workspaces, pipelineRunFacts.State); err != nil {
logger.Errorf("Optional workspace not supported by task: %v", err)
pr.Status.MarkFailed(v1.PipelineRunReasonRequiredWorkspaceMarkedOptional.String(), err.Error())
logger.Errorf("Optional workspace not supported by task: %w", err)
pr.Status.MarkFailed(v1.PipelineRunReasonRequiredWorkspaceMarkedOptional.String(),
"Optional workspace not supported by task: %w", pipelineErrors.LabelUserError(err))
return controller.NewPermanentError(err)
}

Expand Down Expand Up @@ -854,8 +857,9 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline
// Validate parameter types in matrix after apply substitutions from Task Results
if rpt.PipelineTask.IsMatrixed() {
if err := resources.ValidateParameterTypesInMatrix(pipelineRunFacts.State); err != nil {
logger.Errorf("Failed to validate matrix %q with error %v", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidMatrixParameterTypes.String(), err.Error())
logger.Errorf("Failed to validate matrix %q with error %w", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidMatrixParameterTypes.String(),
"Failed to validate matrix %q with error %w", pipelineErrors.LabelUserError(err))
return controller.NewPermanentError(err)
}
}
Expand Down Expand Up @@ -906,8 +910,9 @@ func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.Resolved
}
params = append(params, rpt.PipelineTask.Params...)
if err := taskrun.ValidateEnumParam(ctx, params, rpt.ResolvedTask.TaskSpec.Params); err != nil {
err = fmt.Errorf("Invalid param value from PipelineTask \"%s\": %w", rpt.PipelineTask.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidParamValue.String(), err.Error())
pr.Status.MarkFailed(v1.PipelineRunReasonInvalidParamValue.String(),
"Invalid param value from PipelineTask \"%s\": %w",
rpt.PipelineTask.Name, pipelineErrors.LabelUserError(err))
return nil, controller.NewPermanentError(err)
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ spec:
permanentError: true,
wantEvents: []string{
"Normal Started",
"Warning Failed PipelineRun foo/pipeline-missing-object-param-keys parameters is missing object keys required by Pipeline foo/a-pipeline-with-object-params's parameters: pipelineRun missing object keys for parameters",
"Warning Failed PipelineRun foo/pipeline-missing-object-param-keys parameters is missing object keys required by Pipeline foo/a-pipeline-with-object-params's parameters: user error\npipelineRun missing object keys for parameters: map[some-param:[key2]]",
},
}, {
name: "invalid-pipeline-array-index-out-of-bound",
Expand All @@ -900,7 +900,7 @@ spec:
permanentError: true,
wantEvents: []string{
"Normal Started",
"Warning Failed PipelineRun foo/pipeline-param-array-out-of-bound failed validation: failed to validate Pipeline foo/a-pipeline-with-array-indexing-params's parameter which has an invalid index while referring to an array: non-existent param references:[$(params.some-param[2]",
"Warning Failed PipelineRun foo/pipeline-param-array-out-of-bound failed validation: failed to validate Pipeline foo/a-pipeline-with-array-indexing-params's parameter which has an invalid index while referring to an array: user error\nnon-existent param references:[$(params.some-param[2]",
},
}, {
name: "invalid-embedded-pipeline-bad-name-shd-stop-reconciling",
Expand Down Expand Up @@ -1190,7 +1190,7 @@ status:
image: busybox
script: 'exit 0'
conditions:
- message: "Invalid task result reference: Could not find result with name result2 for task task1"
- message: "user error\nInvalid task result reference: Could not find result with name result2 for task task1"
reason: InvalidTaskResultReference
status: "False"
type: Succeeded
Expand Down
5 changes: 3 additions & 2 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/google/cel-go/cel"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
Expand Down Expand Up @@ -507,7 +508,7 @@ func ValidateWorkspaceBindings(p *v1.PipelineSpec, pr *v1.PipelineRun) error {
continue
}
if _, ok := pipelineRunWorkspaces[ws.Name]; !ok {
return fmt.Errorf("pipeline requires workspace with name %q be provided by pipelinerun", ws.Name)
return pipelineErrors.LabelUserError(fmt.Errorf("pipeline requires workspace with name %q be provided by pipelinerun", ws.Name))
}
}
return nil
Expand All @@ -526,7 +527,7 @@ func ValidateTaskRunSpecs(p *v1.PipelineSpec, pr *v1.PipelineRun) error {

for _, taskrunSpec := range pr.Spec.TaskRunSpecs {
if _, ok := pipelineTasks[taskrunSpec.PipelineTaskName]; !ok {
return fmt.Errorf("pipelineRun's taskrunSpecs defined wrong taskName: %q, does not exist in Pipeline", taskrunSpec.PipelineTaskName)
return pipelineErrors.LabelUserError(fmt.Errorf("pipelineRun's taskrunSpecs defined wrong taskName: %q, does not exist in Pipeline", taskrunSpec.PipelineTaskName))
}
}
return nil
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/pipelinerun/resources/resultrefresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ import (
"fmt"
"sort"

pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

var (
// ErrInvalidTaskResultReference indicates that the reason for the failure status is that there
// is an invalid task result reference
ErrInvalidTaskResultReference = errors.New("Invalid task result reference")
ErrInvalidTaskResultReference = pipelineErrors.LabelUserError(errors.New("Invalid task result reference"))
)

// ResolvedResultRefs represents all of the ResolvedResultRef for a pipeline task
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,14 +672,14 @@ func TestCheckMissingResultReferences(t *testing.T) {
targets: PipelineRunState{
pipelineRunState[3],
},
wantErr: "Invalid task result reference: Could not find result with name missingResult for task aTask",
wantErr: "user error\nInvalid task result reference: Could not find result with name missingResult for task aTask",
}, {
name: "Test unsuccessful result references resolution - params",
pipelineRunState: pipelineRunState,
targets: PipelineRunState{
pipelineRunState[4],
},
wantErr: "Invalid task result reference: Could not find result with name missingResult for task aTask",
wantErr: "user error\nInvalid task result reference: Could not find result with name missingResult for task aTask",
}, {
name: "Valid: Test successful result references resolution - params - Run",
pipelineRunState: pipelineRunState,
Expand Down Expand Up @@ -716,14 +716,14 @@ func TestCheckMissingResultReferences(t *testing.T) {
targets: PipelineRunState{
pipelineRunState[13],
},
wantErr: "Invalid task result reference: Could not find result with name iDoNotExist for task dTask",
wantErr: "user error\nInvalid task result reference: Could not find result with name iDoNotExist for task dTask",
}, {
name: "Invalid: Test result references resolution - matrix custom task - missing references to string replacements",
pipelineRunState: pipelineRunState,
targets: PipelineRunState{
pipelineRunState[14],
},
wantErr: "Invalid task result reference: Could not find result with name iDoNotExist for task aCustomPipelineTask",
wantErr: "user error\nInvalid task result reference: Could not find result with name iDoNotExist for task aCustomPipelineTask",
}} {
t.Run(tt.name, func(t *testing.T) {
err := CheckMissingResultReferences(tt.pipelineRunState, tt.targets)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package resources
import (
"fmt"

pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"k8s.io/apimachinery/pkg/util/sets"
)
Expand All @@ -32,7 +33,7 @@ func ValidatePipelineTaskResults(state PipelineRunState) error {
for _, rpt := range state {
for _, ref := range v1.PipelineTaskResultRefs(rpt.PipelineTask) {
if err := validateResultRef(ref, ptMap); err != nil {
return fmt.Errorf("invalid result reference in pipeline task %q: %w", rpt.PipelineTask.Name, err)
return pipelineErrors.LabelUserError(fmt.Errorf("invalid result reference in pipeline task %q: %w", rpt.PipelineTask.Name, err))
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/reconciler/pipelinerun/resources/validate_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package resources
import (
"fmt"

pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/list"
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun"
Expand All @@ -45,7 +46,7 @@ func ValidateParamTypesMatching(p *v1.PipelineSpec, pr *v1.PipelineRun) error {

// Return an error with the misconfigured parameters' names, or return nil if there are none.
if len(wrongTypeParamNames) != 0 {
return fmt.Errorf("parameters have inconsistent types : %s", wrongTypeParamNames)
return pipelineErrors.LabelUserError(fmt.Errorf("parameters have inconsistent types : %s", wrongTypeParamNames))
}
return nil
}
Expand All @@ -71,7 +72,7 @@ func ValidateRequiredParametersProvided(pipelineParameters *v1.ParamSpecs, pipel

// Return an error with the missing parameters' names, or return nil if there are none.
if len(missingParams) != 0 {
return fmt.Errorf("pipelineRun missing parameters: %s", missingParams)
return pipelineErrors.LabelUserError(fmt.Errorf("pipelineRun missing parameters: %s", missingParams))
}
return nil
}
Expand All @@ -80,7 +81,7 @@ func ValidateRequiredParametersProvided(pipelineParameters *v1.ParamSpecs, pipel
func ValidateObjectParamRequiredKeys(pipelineParameters []v1.ParamSpec, pipelineRunParameters []v1.Param) error {
missings := taskrun.MissingKeysObjectParamNames(pipelineParameters, pipelineRunParameters)
if len(missings) != 0 {
return fmt.Errorf("pipelineRun missing object keys for parameters: %v", missings)
return pipelineErrors.LabelUserError(fmt.Errorf("pipelineRun missing object keys for parameters: %v", missings))
}

return nil
Expand Down
Loading

0 comments on commit 27b35f7

Please sign in to comment.