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 failed TaskRunStatus message #7543

Merged
merged 1 commit into from
Jan 30, 2024
Merged
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
16 changes: 14 additions & 2 deletions pkg/apis/pipeline/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ limitations under the License.

package errors

const userErrorLabel = "[User error] "
import "errors"

const UserErrorLabel = "[User error] "

type UserError struct {
Reason string
Expand Down Expand Up @@ -44,7 +46,7 @@ func newUserError(reason string, err error) *UserError {

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

// LabelUserError labels the failure RunStatus message if any of its error messages has been
Expand All @@ -59,3 +61,13 @@ func LabelUserError(messageFormat string, messageA []interface{}) string {
}
return messageFormat
}

// GetErrorMessage returns the error message with the user error label if it is of type user
// error
func GetErrorMessage(err error) string {
var ue *UserError
if errors.As(err, &ue) {
return ue.Reason + err.Error()
}
return err.Error()
}
26 changes: 26 additions & 0 deletions pkg/apis/pipeline/errors/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,29 @@ func TestLabelsUserError(t *testing.T) {
}
}
}

func TestGetErrorMess(t *testing.T) {
original := errors.New("orignal error")
tcs := []struct {
description string
err error
expected string
}{{
description: "error messages with user error",
err: pipelineErrors.WrapUserError(original),
expected: "[User error] " + original.Error(),
}, {
description: "error messages without user error",
err: original,
expected: original.Error(),
}}
for _, tc := range tcs {
{
msg := pipelineErrors.GetErrorMessage(tc.err)

if msg != tc.expected {
t.Errorf("failure messageFormat expected: %s; but got %s", tc.expected, msg)
}
}
}
}
3 changes: 2 additions & 1 deletion pkg/apis/pipeline/v1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/config"
apisconfig "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"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -242,7 +243,7 @@ func (trs *TaskRunStatus) MarkResourceFailed(reason TaskRunReason, err error) {
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: reason.String(),
Message: err.Error(),
Message: pipelineErrors.GetErrorMessage(err),
})
succeeded := trs.GetCondition(apis.ConditionSucceeded)
trs.CompletionTime = &succeeded.LastTransitionTime.Inner
Expand Down
3 changes: 2 additions & 1 deletion pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/tektoncd/pipeline/internal/sidecarlogresults"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
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"
clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
Expand Down Expand Up @@ -196,7 +197,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon
logger.Errorf("Reconcile: %v", err.Error())
if errors.Is(err, sidecarlogresults.ErrSizeExceeded) {
cfg := config.FromContextOrDefaults(ctx)
message := fmt.Sprintf("TaskRun \"%q\" failed: results exceeded size limit %d bytes", tr.Name, cfg.FeatureFlags.MaxResultSize)
message := fmt.Sprintf("%s TaskRun \"%q\" failed: results exceeded size limit %d bytes", pipelineErrors.UserErrorLabel, tr.Name, cfg.FeatureFlags.MaxResultSize)
err := c.failTaskRun(ctx, tr, v1.TaskRunReasonResultLargerThanAllowedLimit, message)
return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err)
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/tektoncd/pipeline/pkg/apis/config"
cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing"
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
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/pkg/apis/pipeline/v1alpha1"
Expand Down Expand Up @@ -1666,7 +1667,7 @@ status:
type: string
value: aResultValue
`)
failedOnReconcileFailureTaskRun = parse.MustParseV1TaskRun(t, `
failedOnReconcileFailureTaskRun = parse.MustParseV1TaskRun(t, fmt.Sprintf(`
metadata:
name: test-taskrun-results-type-mismatched
namespace: foo
Expand All @@ -1679,14 +1680,14 @@ status:
- reason: ToBeRetried
status: Unknown
type: Succeeded
message: "Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\""
message: "%sProvided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\""
sideCars:
retriesStatus:
- conditions:
- reason: TaskRunValidationFailed
status: "False"
type: "Succeeded"
message: "Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\""
message: "%sProvided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\""
startTime: "2021-12-31T23:59:59Z"
completionTime: "2022-01-01T00:00:00Z"
podName: "test-taskrun-results-type-mismatched-pod"
Expand Down Expand Up @@ -1714,7 +1715,7 @@ status:
ResultExtractionMethod: "termination-message"
MaxResultSize: 4096
Coschedule: "workspaces"
`)
`, pipelineErrors.UserErrorLabel, pipelineErrors.UserErrorLabel))
reconciliatonError = fmt.Errorf("1 error occurred:\n\t* Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"")
toBeRetriedTaskRun = parse.MustParseV1TaskRun(t, `
metadata:
Expand Down
17 changes: 9 additions & 8 deletions pkg/reconciler/taskrun/validate_taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"strings"

"github.com/hashicorp/go-multierror"
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/resources"
Expand Down Expand Up @@ -170,7 +171,7 @@ func ValidateResolvedTask(ctx context.Context, params []v1.Param, matrix *v1.Mat
paramSpecs = rtr.TaskSpec.Params
}
if err := validateParams(ctx, paramSpecs, params, matrix.GetAllParams()); err != nil {
return fmt.Errorf("invalid input params for task %s: %w", rtr.TaskName, err)
return pipelineErrors.WrapUserError(fmt.Errorf("invalid input params for task %s: %w", rtr.TaskName, err))
}
return nil
}
Expand Down Expand Up @@ -198,7 +199,7 @@ func ValidateEnumParam(ctx context.Context, params []v1.Param, paramSpecs v1.Par
}

if !slices.Contains(paramSpecNameToEnum[p.Name], p.Value.StringVal) {
return fmt.Errorf("param `%s` value: %s is not in the enum list", p.Name, p.Value.StringVal)
return pipelineErrors.WrapUserError(fmt.Errorf("param `%s` value: %s is not in the enum list", p.Name, p.Value.StringVal))
}
}
return nil
Expand All @@ -211,13 +212,13 @@ func validateTaskSpecRequestResources(taskSpec *v1.TaskSpec) error {
// First validate the limit in step
if limit, ok := step.ComputeResources.Limits[k]; ok {
if (&limit).Cmp(request) == -1 {
return fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String())
return pipelineErrors.WrapUserError(fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String()))
}
} else if taskSpec.StepTemplate != nil {
// If step doesn't configure the limit, validate the limit in stepTemplate
if limit, ok := taskSpec.StepTemplate.ComputeResources.Limits[k]; ok {
if (&limit).Cmp(request) == -1 {
return fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String())
return pipelineErrors.WrapUserError(fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String()))
}
}
}
Expand All @@ -243,7 +244,7 @@ func validateStepOverrides(ts *v1.TaskSpec, trs *v1.TaskRunSpec) error {
}
for _, stepOverride := range trs.StepSpecs {
if !stepNames.Has(stepOverride.Name) {
err = multierror.Append(err, fmt.Errorf("invalid StepOverride: No Step named %s", stepOverride.Name))
err = multierror.Append(err, pipelineErrors.WrapUserError(fmt.Errorf("invalid StepOverride: No Step named %s", stepOverride.Name)))
}
}
return err
Expand All @@ -257,7 +258,7 @@ func validateSidecarOverrides(ts *v1.TaskSpec, trs *v1.TaskRunSpec) error {
}
for _, sidecarOverride := range trs.SidecarSpecs {
if !sidecarNames.Has(sidecarOverride.Name) {
err = multierror.Append(err, fmt.Errorf("invalid SidecarOverride: No Sidecar named %s", sidecarOverride.Name))
err = multierror.Append(err, pipelineErrors.WrapUserError(fmt.Errorf("invalid SidecarOverride: No Sidecar named %s", sidecarOverride.Name)))
}
}
return err
Expand All @@ -281,12 +282,12 @@ func validateTaskRunResults(tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) error
s = append(s, fmt.Sprintf(" \"%v\": %v", k, v))
}
sort.Strings(s)
return fmt.Errorf("Provided results don't match declared results; may be invalid JSON or missing result declaration: %v", strings.Join(s, ","))
return pipelineErrors.WrapUserError(fmt.Errorf("Provided results don't match declared results; may be invalid JSON or missing result declaration: %v", strings.Join(s, ",")))
}

// When get the results, for object value need to check if they have missing keys.
if missingKeysObjectNames := missingKeysofObjectResults(tr, specResults); len(missingKeysObjectNames) != 0 {
return fmt.Errorf("missing keys for these results which are required in TaskResult's properties %v", missingKeysObjectNames)
return pipelineErrors.WrapUserError(fmt.Errorf("missing keys for these results which are required in TaskResult's properties %v", missingKeysObjectNames))
}
return nil
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/workspace/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"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 @@ -31,7 +32,7 @@ func ValidateBindings(ctx context.Context, decls []v1.WorkspaceDeclaration, bind
// reason we'll invoke the same validation here.
for _, b := range binds {
if err := b.Validate(ctx); err != nil {
return fmt.Errorf("binding %q is invalid: %w", b.Name, err)
return pipelineErrors.WrapUserError(fmt.Errorf("binding %q is invalid: %w", b.Name, err))
}
}

Expand All @@ -49,12 +50,12 @@ func ValidateBindings(ctx context.Context, decls []v1.WorkspaceDeclaration, bind
continue
}
if !bindNames.Has(decl.Name) {
return fmt.Errorf("declared workspace %q is required but has not been bound", decl.Name)
return pipelineErrors.WrapUserError(fmt.Errorf("declared workspace %q is required but has not been bound", decl.Name))
}
}
for _, bind := range binds {
if !declNames.Has(bind.Name) {
return fmt.Errorf("workspace binding %q does not match any declared workspace", bind.Name)
return pipelineErrors.WrapUserError(fmt.Errorf("workspace binding %q does not match any declared workspace", bind.Name))
}
}

Expand All @@ -78,7 +79,7 @@ func ValidateOnlyOnePVCIsUsed(wb []v1.WorkspaceBinding) error {
}

if len(workspaceVolumes) > 1 {
return fmt.Errorf("more than one PersistentVolumeClaim is bound")
return pipelineErrors.WrapUserError(fmt.Errorf("more than one PersistentVolumeClaim is bound"))
}
return nil
}