diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index a314760bcef..d5fb6fe9b8a 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -156,6 +156,8 @@ func (trs *TaskRunStatus) MarkResourceFailed(reason TaskRunReason, err error) { Reason: reason.String(), Message: err.Error(), }) + succeeded := trs.GetCondition(apis.ConditionSucceeded) + trs.CompletionTime = &succeeded.LastTransitionTime.Inner } // TaskRunStatusFields holds the fields of TaskRun's status. This is defined diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 19c29f51b17..048c2962f08 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -24,6 +24,8 @@ import ( "strings" "time" + "go.uber.org/zap" + "github.com/ghodss/yaml" "github.com/hashicorp/go-multierror" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -103,54 +105,46 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg // on the event to perform user facing initialisations, such has reset a CI check status afterCondition := tr.Status.GetCondition(apis.ConditionSucceeded) events.Emit(ctx, nil, afterCondition, tr) + + // We already sent an event for start, so update `before` with the current status + before = tr.Status.GetCondition(apis.ConditionSucceeded) } // If the TaskRun is complete, run some post run fixtures when applicable if tr.IsDone() { logger.Infof("taskrun done : %s \n", tr.Name) - var merr *multierror.Error + + // We may be reading a version of the object that was stored at an older version + // and may not have had all of the assumed default specified. + tr.SetDefaults(contexts.WithUpgradeViaDefaulting(ctx)) + // Try to send cloud events first cloudEventErr := cloudevent.SendCloudEvents(tr, c.cloudEventClient, logger) // Regardless of `err`, we must write back any status update that may have // been generated by `sendCloudEvents` - _, updateErr := c.updateLabelsAndAnnotations(tr) - merr = multierror.Append(cloudEventErr, updateErr) if cloudEventErr != nil { // Let's keep timeouts and sidecars running as long as we're trying to // send cloud events. So we stop here an return errors encountered this far. - return merr.ErrorOrNil() + return cloudEventErr } + c.timeoutHandler.Release(tr) - pod, err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{}) - if err == nil { - err = podconvert.StopSidecars(c.Images.NopImage, c.KubeClientSet, *pod) - if err == nil { - // Check if any SidecarStatuses are still shown as Running after stopping - // Sidecars. If any Running, update SidecarStatuses based on Pod ContainerStatuses. - if podconvert.IsSidecarStatusRunning(tr) { - err = updateStoppedSidecarStatus(ctx, pod, tr, c) - } - } - } else if k8serrors.IsNotFound(err) { - return merr.ErrorOrNil() - } - if err != nil { - logger.Errorf("Error stopping sidecars for TaskRun %q: %v", tr.Name, err) - merr = multierror.Append(merr, err) - } + + pod := c.stopSidecars(ctx, tr) go func(metrics *Recorder) { err := metrics.DurationAndCount(tr) if err != nil { logger.Warnf("Failed to log the metrics : %v", err) } - err = metrics.RecordPodLatency(pod, tr) - if err != nil { - logger.Warnf("Failed to log the metrics : %v", err) + if pod != nil { + err = metrics.RecordPodLatency(pod, tr) + if err != nil { + logger.Warnf("Failed to log the metrics : %v", err) + } } }(c.metrics) - - return merr.ErrorOrNil() + return c.finishReconcileUpdateEmitEvents(ctx, tr, before, nil) } // If the TaskRun is cancelled, kill resources and update status @@ -176,12 +170,9 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg logger.Errorf("TaskRun prepare error: %v", err.Error()) // We only return an error if update failed, otherwise we don't want to // reconcile an invalid TaskRun anymore - return c.finishReconcileUpdateEmitEvents(ctx, tr, nil, err) + return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err) } - // Store the condition before reconcile - before = tr.Status.GetCondition(apis.ConditionSucceeded) - // Reconcile this copy of the task run and then write back any status // updates regardless of whether the reconciliation errored out. if err = c.reconcile(ctx, tr, taskSpec, rtr); err != nil { @@ -190,8 +181,32 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg // Emit events (only when ConditionSucceeded was changed) return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err) } +func (c *Reconciler) stopSidecars(ctx context.Context, tr *v1beta1.TaskRun) *corev1.Pod { + logger := logging.FromContext(ctx) + if tr.Status.PodName == "" { + return nil + } + pod, err := c.KubeClientSet.CoreV1().Pods(tr.Namespace).Get(tr.Status.PodName, metav1.GetOptions{}) + if err == nil { + err = podconvert.StopSidecars(c.Images.NopImage, c.KubeClientSet, *pod) + if err == nil { + // Check if any SidecarStatuses are still shown as Running after stopping + // Sidecars. If any Running, update SidecarStatuses based on Pod ContainerStatuses. + if podconvert.IsSidecarStatusRunning(tr) { + err = updateStoppedSidecarStatus(ctx, pod, tr, c) + } + } + } + if err != nil { + logger.Errorf("Error stopping sidecars for TaskRun %q: %v", tr.Name, err) + tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) + } + return pod +} func (c *Reconciler) finishReconcileUpdateEmitEvents(ctx context.Context, tr *v1beta1.TaskRun, beforeCondition *apis.Condition, previousError error) error { + logger := logging.FromContext(ctx) + afterCondition := tr.Status.GetCondition(apis.ConditionSucceeded) // Send k8s events and cloud events (when configured) @@ -199,12 +214,15 @@ func (c *Reconciler) finishReconcileUpdateEmitEvents(ctx context.Context, tr *v1 _, err := c.updateLabelsAndAnnotations(tr) if err != nil { + logger.Warn("Failed to update PipelineRun labels/annotations", zap.Error(err)) events.EmitError(controller.GetEventRecorder(ctx), err, tr) } + + merr := multierror.Append(previousError, err).ErrorOrNil() if controller.IsPermanentError(previousError) { - return controller.NewPermanentError(multierror.Append(previousError, err)) + return controller.NewPermanentError(merr) } - return multierror.Append(previousError, err).ErrorOrNil() + return merr } func (c *Reconciler) getTaskResolver(tr *v1beta1.TaskRun) (*resources.LocalTaskRefResolver, v1beta1.TaskKind) { diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 7ad27d2f3f3..2f5a21a3f1f 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -39,7 +39,7 @@ import ( "github.com/tektoncd/pipeline/pkg/system" "github.com/tektoncd/pipeline/pkg/timeout" "github.com/tektoncd/pipeline/pkg/workspace" - test "github.com/tektoncd/pipeline/test" + "github.com/tektoncd/pipeline/test" "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" @@ -1733,6 +1733,57 @@ func TestReconcileInvalidTaskRuns(t *testing.T) { } +func TestReconcileTaskRunWithPermanentError(t *testing.T) { + noTaskRun := tb.TaskRun("notaskrun", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(tb.TaskRunTaskRef("notask")), + tb.TaskRunStatus(tb.TaskRunStartTime(time.Now()), + tb.StatusCondition(apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: podconvert.ReasonFailedResolution, + Message: "error when listing tasks for taskRun taskrun-failure: tasks.tekton.dev \"notask\" not found", + }))) + + taskRuns := []*v1beta1.TaskRun{noTaskRun} + + d := test.Data{ + TaskRuns: taskRuns, + } + + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + reconcileErr := c.Reconciler.Reconcile(context.Background(), getRunName(noTaskRun)) + + // When a TaskRun was rejected with a permanent error, reconciler must stop and forget about the run + // Such TaskRun enters Reconciler and from within the isDone block, marks the run success so that + // reconciler does not keep trying to reconcile + if reconcileErr != nil { + t.Fatalf("Expected to see no error when reconciling TaskRun with Permanent Error but was not none") + } + + // Check actions + actions := clients.Kube.Actions() + if len(actions) != 3 || actions[0].Matches("namespaces", "list") { + t.Errorf("expected 3 actions (list namespaces, list configmaps, and watch configmaps) created by the reconciler,"+ + " got %d. Actions: %#v", len(actions), actions) + } + + newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(noTaskRun.Namespace).Get(noTaskRun.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Expected TaskRun %s to exist but instead got error when getting it: %v", noTaskRun.Name, err) + } + + // Since the TaskRun is invalid, the status should say it has failed + condition := newTr.Status.GetCondition(apis.ConditionSucceeded) + if condition == nil || condition.Status != corev1.ConditionFalse { + t.Errorf("Expected invalid TaskRun to have failed status, but had %v", condition) + } + if condition != nil && condition.Reason != podconvert.ReasonFailedResolution { + t.Errorf("Expected failure to be because of reason %q but was %s", podconvert.ReasonFailedResolution, condition.Reason) + } +} + func TestReconcilePodFetchError(t *testing.T) { taskRun := tb.TaskRun("test-taskrun-run-success", tb.TaskRunNamespace("foo"),