Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Label user error for failure PipelineRun Status #7470

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
97 changes: 97 additions & 0 deletions pkg/apis/pipeline/errors/errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
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")

messages := make([]interface{}, 0)
for _, msg := range msgs {
messages = append(messages, msg)
}

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(),
"Validation failed for 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(),
"Validation failed for 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
Loading