Skip to content

Commit

Permalink
Label 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 #7276
  • Loading branch information
JeromeJu committed Dec 10, 2023
1 parent a179226 commit 063d8e8
Show file tree
Hide file tree
Showing 12 changed files with 255 additions and 45 deletions.
31 changes: 31 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,36 @@ 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 reason, for example:

```yaml
# Failed PipelineRun with reason labeled "User error"
apiVersion: tekton.dev/v1
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: pipelineRun missing parameters:
[pl-param-x]'
reason: '[User error] ParameterMissing'
status: "False"
type: Succeeded
```

```console
~/pipeline$ tkn pr list
NAME STARTED DURATION STATUS
pipelinerun-with-params 5 seconds ago 0s Failed([User error] ParameterMissing)
```

## Cancelling a `PipelineRun`

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

const userErrorLabel = "[User error] "

type UserError struct {
Reason string
Original error
}

var _ error = &UserError{}

// Error returns the original error message. This implements the error.Error interface.
func (e *UserError) Error() string {
return e.Original.Error()
}

// Unwrap returns the original error without the Reason annotation. This is
// intended to support usage of errors.Is and errors.As with Errors.
func (e *UserError) Unwrap() error {
return e.Original
}

// newUserError returns a UserError with the given reason and underlying
// original error.
func newUserError(reason string, err error) *UserError {
return &UserError{
Reason: reason,
Original: err,
}
}

// WrapUserError wraps the original error with the user error label
func WrapUserError(err error) error {
return newUserError(userErrorLabel, err)
}

// LabelsUserErrorReason labels the failure RunStatus Reason if any of its error messages has been
// wrapped as an UserError. It indicates that the user is responsible for an error.
// See github.com/tektoncd/pipeline/blob/main/docs/pipelineruns.md#marking-off-user-errors
// for more details.
func LabelsUserErrorReason(reason string, messageA []interface{}) string {
for _, message := range messageA {
if ue, ok := message.(*UserError); ok {
return ue.Reason + reason
}
}
return reason
}
98 changes: 98 additions & 0 deletions pkg/apis/pipeline/errors/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
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"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
)

type TestError struct{}

var _ error = &TestError{}

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

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

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

func TestResolutionErrorMessage(t *testing.T) {
originalError := &TestError{}
expectedErrorMessage := originalError.Error()

userError := pipelineErrors.WrapUserError(originalError)

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

func TestLabelsUserError(t *testing.T) {
const hasUserError = true
tcs := []struct {
description string
reason string
messages []interface{}
expected string
}{{
description: "error messags with user error",
reason: v1.PipelineRunReasonInvalidGraph.String(),
messages: makeMessages(hasUserError),
expected: "[User error] " + v1.PipelineRunReasonInvalidGraph.String(),
}, {
description: "error messags without user error",
messages: makeMessages(!hasUserError),
reason: v1.PipelineRunReasonInvalidGraph.String(),
expected: v1.PipelineRunReasonInvalidGraph.String(),
}}
for _, tc := range tcs {
{
reason := pipelineErrors.LabelsUserErrorReason(tc.reason, tc.messages)

if reason != tc.expected {
t.Errorf("failure reason expected: %s; but got %s", tc.expected, reason)
}
}
}
}

func makeMessages(hasUserError bool) []interface{} {
msgs := []string{"foo error message", "bar error format"}
original := errors.New("orignal error")

var messages []interface{}
messages = make([]interface{}, len(msgs)+1)
for i, v := range msgs {
messages[i] = v
}

if hasUserError {
messages = append(messages, pipelineErrors.WrapUserError(original))
}
return messages
}
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
pod "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
runv1beta1 "github.com/tektoncd/pipeline/pkg/apis/run/v1beta1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -458,6 +459,7 @@ func (pr *PipelineRunStatus) MarkSucceeded(reason, messageFormat string, message

// MarkFailed changes the Succeeded condition to False with the provided reason and message.
func (pr *PipelineRunStatus) MarkFailed(reason, messageFormat string, messageA ...interface{}) {
reason = pipelineErrors.LabelsUserErrorReason(reason, messageA)
pipelineRunCondSet.Manage(pr).MarkFalse(apis.ConditionSucceeded, reason, messageFormat, messageA...)
succeeded := pr.GetCondition(apis.ConditionSucceeded)
pr.CompletionTime = &succeeded.LastTransitionTime.Inner
Expand Down
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.WrapUserError(ErrCouldntValidateObjectPermanent)
case apierrors.IsTimeout(err), apierrors.IsServerTimeout(err), apierrors.IsTooManyRequests(err):
errType = ErrCouldntValidateObjectRetryable
default:
Expand Down
46 changes: 29 additions & 17 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 @@ -375,7 +376,7 @@ func (c *Reconciler) resolvePipelineState(
} else {
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(),
"PipelineRun %s/%s can't be Run; couldn't resolve all references: %s",
pipelineMeta.Namespace, pr.Name, err)
pipelineMeta.Namespace, pr.Name, pipelineErrors.WrapUserError(err))
}
return nil, controller.NewPermanentError(err)
}
Expand Down Expand Up @@ -412,7 +413,10 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
pr.Status.MarkRunning(v1.PipelineRunReasonResolvingPipelineRef.String(), message)
return nil
case errors.Is(err, apiserver.ErrReferencedObjectValidationFailed), errors.Is(err, apiserver.ErrCouldntValidateObjectPermanent):
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(), err.Error())
logger.Errorf("Failed dryRunValidation for PipelineRun %s: %w", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(),
"Failed dryRunValidation for PipelineRun %s: %s",
pr.Name, pipelineErrors.WrapUserError(err))
return controller.NewPermanentError(err)
case errors.Is(err, apiserver.ErrCouldntValidateObjectRetryable):
return err
Expand Down Expand Up @@ -443,7 +447,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.WrapUserError(err))
return controller.NewPermanentError(err)
}

Expand All @@ -456,15 +460,15 @@ 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.WrapUserError(err))
return controller.NewPermanentError(err)
}

if err := pipelineSpec.Validate(ctx); err != nil {
// This Run has failed, so we need to mark it as failed and stop reconciling it
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(),
"Pipeline %s/%s can't be Run; it has an invalid spec: %s",
pipelineMeta.Namespace, pipelineMeta.Name, err)
pipelineMeta.Namespace, pipelineMeta.Name, pipelineErrors.WrapUserError(err))
return controller.NewPermanentError(err)
}

Expand Down Expand Up @@ -492,7 +496,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.WrapUserError(err))
return controller.NewPermanentError(err)
}
}
Expand Down Expand Up @@ -635,15 +639,19 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel
if !rpt.IsCustomTask() {
err := taskrun.ValidateResolvedTask(ctx, rpt.PipelineTask.Params, rpt.PipelineTask.Matrix, rpt.ResolvedTask)
if err != nil {
logger.Errorf("Failed to validate pipelinerun %q with error %v", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(), err.Error())
logger.Errorf("Failed to validate pipelinerun %s with error %w", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(),
"Failed to validate pipelinerun %s with error %s",
pr.Name, pipelineErrors.WrapUserError(err))
return controller.NewPermanentError(err)
}

if config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum {
if err := resources.ValidateParamEnumSubset(originalTasks[i].Params, pipelineSpec.Params, rpt.ResolvedTask); err != nil {
logger.Errorf("Failed to validate pipelinerun %q with error %v", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(), err.Error())
logger.Errorf("Failed to validate pipelinerun %q with error %w", pr.Name, err)
pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(),
"Failed to validate pipelinerun with error %s",
pipelineErrors.WrapUserError(err))
return controller.NewPermanentError(err)
}
}
Expand All @@ -655,7 +663,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.WrapUserError(err))
return controller.NewPermanentError(err)
}
}
Expand Down Expand Up @@ -684,8 +693,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.WrapUserError(err))
return controller.NewPermanentError(err)
}

Expand Down Expand Up @@ -854,8 +864,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.WrapUserError(err))
return controller.NewPermanentError(err)
}
}
Expand Down Expand Up @@ -906,8 +917,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.WrapUserError(err))
return nil, controller.NewPermanentError(err)
}
}
Expand Down
Loading

0 comments on commit 063d8e8

Please sign in to comment.