Error sweep: Move TaskRun Reasons in pkg/pod to pkg/apis #7406
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -340,7 +340,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| tr.Status.MarkResourceOngoing(v1.TaskRunReasonResolvingTaskRef, message) | ||
| return nil, nil, err | ||
| case errors.Is(err, apiserver.ErrReferencedObjectValidationFailed), errors.Is(err, apiserver.ErrCouldntValidateObjectPermanent): | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonTaskFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| case errors.Is(err, apiserver.ErrCouldntValidateObjectRetryable): | ||
| return nil, nil, err | ||
|
|
@@ -349,7 +349,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| if resources.IsErrTransient(err) { | ||
| return nil, nil, err | ||
| } | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedResolution, err) | ||
|
There was a problem hiding this comment. Should we avoid using There was a problem hiding this comment. Yes that is correct. Those reasons were once living in There was a problem hiding this comment. I've also updated all reference to podconvert.*reasons to use v1.*TaskRunReasons instead. |
||
| return nil, nil, controller.NewPermanentError(err) | ||
| default: | ||
| // Store the fetched TaskSpec on the TaskRun for auditing | ||
|
|
@@ -365,7 +365,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| tr.Status.MarkResourceOngoing(v1.TaskRunReasonResolvingStepActionRef, message) | ||
| return nil, nil, err | ||
| case errors.Is(err, apiserver.ErrReferencedObjectValidationFailed), errors.Is(err, apiserver.ErrCouldntValidateObjectPermanent): | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonTaskFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| case errors.Is(err, apiserver.ErrCouldntValidateObjectRetryable): | ||
| return nil, nil, err | ||
|
|
@@ -374,7 +374,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| if resources.IsErrTransient(err) { | ||
| return nil, nil, err | ||
| } | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedResolution, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| default: | ||
| // Store the fetched StepActions to TaskSpec, and update the stored TaskSpec again | ||
|
|
@@ -388,7 +388,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| switch taskMeta.VerificationResult.VerificationResultType { | ||
| case trustedresources.VerificationError: | ||
| logger.Errorf("TaskRun %s/%s referred task failed signature verification", tr.Namespace, tr.Name) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonResourceVerificationFailed, taskMeta.VerificationResult.Err) | ||
| tr.Status.SetCondition(&apis.Condition{ | ||
| Type: trustedresources.ConditionTrustedResourcesVerified, | ||
| Status: corev1.ConditionFalse, | ||
|
|
@@ -419,13 +419,13 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
|
|
||
| if err := validateTaskSpecRequestResources(taskSpec); err != nil { | ||
| logger.Errorf("TaskRun %s taskSpec request resources are invalid: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
|
|
||
| if err := ValidateResolvedTask(ctx, tr.Spec.Params, &v1.Matrix{}, rtr); err != nil { | ||
| logger.Errorf("TaskRun %q resources are invalid: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
|
|
||
|
|
@@ -439,13 +439,13 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
|
|
||
| if err := resources.ValidateParamArrayIndex(rtr.TaskSpec, tr.Spec.Params); err != nil { | ||
| logger.Errorf("TaskRun %q Param references are invalid: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
|
|
||
| if err := c.updateTaskRunWithDefaultWorkspaces(ctx, tr, taskSpec); err != nil { | ||
| logger.Errorf("Failed to update taskrun %s with default workspace: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedResolution, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
|
|
||
|
|
@@ -464,7 +464,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| } | ||
| if err := workspace.ValidateBindings(ctx, workspaceDeclarations, tr.Spec.Workspaces); err != nil { | ||
| logger.Errorf("TaskRun %q workspaces are invalid: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
|
|
||
|
|
@@ -475,14 +475,14 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, | |
| if aaBehavior == affinityassistant.AffinityAssistantPerWorkspace { | ||
| if err := workspace.ValidateOnlyOnePVCIsUsed(tr.Spec.Workspaces); err != nil { | ||
| logger.Errorf("TaskRun %q workspaces incompatible with Affinity Assistant: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
| } | ||
|
|
||
| if err := validateOverrides(taskSpec, &tr.Spec); err != nil { | ||
| logger.Errorf("TaskRun %q step or sidecar overrides are invalid: %v", tr.Name, err) | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return nil, nil, controller.NewPermanentError(err) | ||
| } | ||
|
|
||
|
|
@@ -596,7 +596,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1.TaskRun, rtr *resourc | |
| } | ||
|
|
||
| if err := validateTaskRunResults(tr, rtr.TaskSpec); err != nil { | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| return err | ||
| } | ||
|
|
||
|
|
@@ -671,17 +671,17 @@ func (c *Reconciler) handlePodCreationError(tr *v1.TaskRun, err error) error { | |
| case isResourceQuotaConflictError(err): | ||
| // Requeue if it runs into ResourceQuotaConflictError Error i.e https://github.com/kubernetes/kubernetes/issues/67761 | ||
| tr.Status.StartTime = nil | ||
| tr.Status.MarkResourceOngoing(podconvert.ReasonPodPending, "tried to create pod, but it failed with ResourceQuotaConflictError") | ||
| return controller.NewRequeueAfter(time.Second) | ||
| case isExceededResourceQuotaError(err): | ||
| // If we are struggling to create the pod, then it hasn't started. | ||
| tr.Status.StartTime = nil | ||
| tr.Status.MarkResourceOngoing(podconvert.ReasonExceededResourceQuota, fmt.Sprint("TaskRun Pod exceeded available resources: ", err)) | ||
| return controller.NewRequeueAfter(time.Minute) | ||
| case isTaskRunValidationFailed(err): | ||
| tr.Status.MarkResourceFailed(v1.TaskRunReasonFailedValidation, err) | ||
| case k8serrors.IsAlreadyExists(err): | ||
| tr.Status.MarkResourceOngoing(podconvert.ReasonPodPending, "tried to create pod, but it already exists") | ||
| case isPodAdmissionFailed(err): | ||
| tr.Status.MarkResourceFailed(podconvert.ReasonPodAdmissionFailed, err) | ||
| default: | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need a .String() here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reasons previously were just strings while the v1.TaskRunReasons* are of type TaskRunReason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it looks like only
ReasonFailedResolutionis attached with.String()before. Now it is consistent. Thanks.