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 #7276
  • Loading branch information
JeromeJu committed Dec 19, 2023
1 parent a80af95 commit 990189e
Show file tree
Hide file tree
Showing 13 changed files with 320 additions and 36 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 message, for example:

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

```console
~/pipeline$ tkn pr list
NAME STARTED DURATION STATUS
pipelinerun-with-params 5 seconds ago 0s Failed(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)
}

// LabelUserError labels the failure RunStatus message 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 LabelUserError(messageFormat string, messageA []interface{}) string {
for _, message := range messageA {
if ue, ok := message.(*UserError); ok {
return ue.Reason + messageFormat
}
}
return messageFormat
}
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

makeMessages := func(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
}

tcs := []struct {
description string
messageFormat string
messages []interface{}
expected string
}{{
description: "error messags with user error",
messageFormat: v1.PipelineRunReasonInvalidGraph.String(),
messages: makeMessages(hasUserError),
expected: "[User error] " + v1.PipelineRunReasonInvalidGraph.String(),
}, {
description: "error messags without user error",
messages: makeMessages(!hasUserError),
messageFormat: v1.PipelineRunReasonInvalidGraph.String(),
expected: v1.PipelineRunReasonInvalidGraph.String(),
}}
for _, tc := range tcs {
{
messageFormat := pipelineErrors.LabelUserError(tc.messageFormat, tc.messages)

if messageFormat != tc.expected {
t.Errorf("failure messageFormat expected: %s; but got %s", tc.expected, messageFormat)
}
}
}
}
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 @@ -465,6 +466,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{}) {
messageFormat = pipelineErrors.LabelUserError(messageFormat, messageA)
pipelineRunCondSet.Manage(pr).MarkFalse(apis.ConditionSucceeded, reason, messageFormat, messageA...)
succeeded := pr.GetCondition(apis.ConditionSucceeded)
pr.CompletionTime = &succeeded.LastTransitionTime.Inner
Expand Down
83 changes: 83 additions & 0 deletions pkg/apis/pipeline/v1/pipelinerun_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ package v1_test

import (
"context"
"errors"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/pipeline/pkg/apis/config"
pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/test/diff"
Expand Down Expand Up @@ -699,3 +701,84 @@ func TestPipelineRun_GetTaskRunSpec(t *testing.T) {
}
}
}

func TestPipelineRunMarkFailedCondition(t *testing.T) {
failedRunReason := v1.PipelineRunReasonFailed
messageFormat := "error bar occurred %s"

makeMessages := func(hasUserError bool) []interface{} {
errorMsg := "baz error message"
original := errors.New("orignal error")

messages := make([]interface{}, 0)
if hasUserError {
messages = append(messages, pipelineErrors.WrapUserError(original))
} else {
messages = append(messages, errorMsg)
}

return messages
}

tcs := []struct {
name string
hasUserError bool
prStatus v1.PipelineRunStatus
expectedConditions duckv1.Conditions
}{{
name: "mark pipelinerun status failed with user error",
hasUserError: true,
prStatus: v1.PipelineRunStatus{
PipelineRunStatusFields: v1.PipelineRunStatusFields{
StartTime: &metav1.Time{Time: now},
},
Status: duckv1.Status{
Conditions: duckv1.Conditions{},
},
},
expectedConditions: duckv1.Conditions{
apis.Condition{
Type: "Succeeded",
Status: "False",
Reason: "Failed",
Message: "[User error] error bar occurred orignal error",
},
},
}, {
name: "mark pipelinerun status failed non user error",
hasUserError: false,
prStatus: v1.PipelineRunStatus{
PipelineRunStatusFields: v1.PipelineRunStatusFields{
StartTime: &metav1.Time{Time: now},
},
Status: duckv1.Status{
Conditions: duckv1.Conditions{},
},
},
expectedConditions: duckv1.Conditions{
apis.Condition{
Type: "Succeeded",
Status: "False",
Reason: "Failed",
Message: "error bar occurred baz error message",
},
},
}}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
pr := &v1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Status: tc.prStatus,
}
pr.Status.MarkFailed(failedRunReason.String(), messageFormat, makeMessages(tc.hasUserError)...)
updatedCondition := pr.Status.Status.Conditions

if d := cmp.Diff(tc.expectedConditions, updatedCondition, cmpopts.IgnoreFields(apis.Condition{}, "LastTransitionTime")); d != "" {
t.Error(diff.PrintWantGot(d))
}
})
}
}
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
Loading

0 comments on commit 990189e

Please sign in to comment.