diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index a4f06d4adef..7c974ac73e5 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -129,9 +129,8 @@ var ( // resource with the current status of the resource. func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) pkgreconciler.Event { logger := logging.FromContext(ctx) - recorder := controller.GetEventRecorder(ctx) - // Snapshot original for the label/annotation check below. - original := pr.DeepCopy() + // Read the initial condition + before := pr.Status.GetCondition(apis.ConditionSucceeded) if !pr.HasStarted() { pr.Status.InitializeConditions() @@ -142,16 +141,9 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) } // start goroutine to track pipelinerun timeout only startTime is not set go c.timeoutHandler.WaitPipelineRun(pr, pr.Status.StartTime) - } else { - pr.Status.InitializeConditions() } - // In case of reconcile errors, we store the error in a multierror, attempt - // to update, and return the original error combined with any update error - var merr *multierror.Error - - switch { - case pr.IsDone(): + if pr.IsDone() { // 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. pr.SetDefaults(contexts.WithUpgradeViaDefaulting(ctx)) @@ -159,16 +151,16 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) c.updatePipelineResults(ctx, pr) if err := artifacts.CleanupArtifactStorage(pr, c.KubeClientSet, logger); err != nil { logger.Errorf("Failed to delete PVC for PipelineRun %s: %v", pr.Name, err) - return err + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) } if err := c.cleanupAffinityAssistants(pr); err != nil { logger.Errorf("Failed to delete StatefulSet for PipelineRun %s: %v", pr.Name, err) - return err + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) } c.timeoutHandler.Release(pr) if err := c.updateTaskRunsStatusDirectly(pr); err != nil { logger.Errorf("Failed to update TaskRun status for PipelineRun %s: %v", pr.Name, err) - return err + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) } go func(metrics *Recorder) { err := metrics.DurationAndCount(pr) @@ -176,46 +168,50 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) logger.Warnf("Failed to log the metrics : %v", err) } }(c.metrics) - case pr.IsCancelled(): + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, nil) + } + + if pr.IsCancelled() { // If the pipelinerun is cancelled, cancel tasks and update status - before := pr.Status.GetCondition(apis.ConditionSucceeded) - merr = multierror.Append(merr, cancelPipelineRun(logger, pr, c.PipelineClientSet)) - after := pr.Status.GetCondition(apis.ConditionSucceeded) - events.Emit(recorder, before, after, pr) - default: - if err := c.tracker.Track(pr.GetTaskRunRef(), pr); err != nil { - logger.Errorf("Failed to create tracker for TaskRuns for PipelineRun %s: %v", pr.Name, err) - recorder.Event(pr, corev1.EventTypeWarning, v1beta1.PipelineRunReasonFailed.String(), "Failed to create tracker for TaskRuns for PipelineRun") - return err - } + err := cancelPipelineRun(logger, pr, c.PipelineClientSet) + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) + } - // Make sure that the PipelineRun status is in sync with the actual TaskRuns - err := c.updatePipelineRunStatusFromInformer(ctx, pr) - if err != nil { - // This should not fail. Return the error so we can re-try later. - logger.Errorf("Error while syncing the pipelinerun status: %v", err.Error()) - return err - } + if err := c.tracker.Track(pr.GetTaskRunRef(), pr); err != nil { + logger.Errorf("Failed to create tracker for TaskRuns for PipelineRun %s: %v", pr.Name, err) + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) + } - // Reconcile this copy of the pipelinerun and then write back any status or label - // updates regardless of whether the reconciliation errored out. - if err = c.reconcile(ctx, pr); err != nil { - logger.Errorf("Reconcile error: %v", err.Error()) - merr = multierror.Append(merr, err) - } + // Make sure that the PipelineRun status is in sync with the actual TaskRuns + err := c.updatePipelineRunStatusFromInformer(ctx, pr) + if err != nil { + // This should not fail. Return the error so we can re-try later. + logger.Errorf("Error while syncing the pipelinerun status: %v", err.Error()) + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) } - // If we need to update the labels or annotations, we need to call Update with these - // changes explicitly. - if !reflect.DeepEqual(original.ObjectMeta.Labels, pr.ObjectMeta.Labels) || !reflect.DeepEqual(original.ObjectMeta.Annotations, pr.ObjectMeta.Annotations) { - if _, err := c.updateLabelsAndAnnotations(pr); err != nil { - logger.Warn("Failed to update PipelineRun labels/annotations", zap.Error(err)) - recorder.Event(pr, corev1.EventTypeWarning, "Error", "PipelineRun failed to update labels/annotations") - return multierror.Append(merr, err) - } + // Reconcile this copy of the pipelinerun and then write back any status or label + // updates regardless of whether the reconciliation errored out. + if err = c.reconcile(ctx, pr); err != nil { + logger.Errorf("Reconcile error: %v", err.Error()) + } + + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) +} + +func (c *Reconciler) finishReconcileUpdateEmitEvents(ctx context.Context, pr *v1beta1.PipelineRun, beforeCondition *apis.Condition, previousError error) error { + recorder := controller.GetEventRecorder(ctx) + logger := logging.FromContext(ctx) + + afterCondition := pr.Status.GetCondition(apis.ConditionSucceeded) + events.Emit(recorder, beforeCondition, afterCondition, pr) + _, err := c.updateLabelsAndAnnotations(pr) + if err != nil { + logger.Warn("Failed to update PipelineRun labels/annotations", zap.Error(err)) + events.EmitError(recorder, err, pr) } - return merr.ErrorOrNil() + return multierror.Append(previousError, err).ErrorOrNil() } func (c *Reconciler) updatePipelineResults(ctx context.Context, pr *v1beta1.PipelineRun) { diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 98156e40e69..6a0a1a90c19 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -88,6 +88,8 @@ var _ taskrunreconciler.Interface = (*Reconciler)(nil) func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkgreconciler.Event { logger := logging.FromContext(ctx) recorder := controller.GetEventRecorder(ctx) + // Read the initial condition + before := tr.Status.GetCondition(apis.ConditionSucceeded) // If the TaskRun is just starting, this will also set the starttime, // from which the timeout will immediately begin counting down. @@ -157,7 +159,6 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg // If the TaskRun is cancelled, kill resources and update status if tr.IsCancelled() { - before := tr.Status.GetCondition(apis.ConditionSucceeded) message := fmt.Sprintf("TaskRun %q was cancelled", tr.Name) err := c.failTaskRun(ctx, tr, v1beta1.TaskRunReasonCancelled, message) return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err) @@ -166,7 +167,6 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg // Check if the TaskRun has timed out; if it is, this will set its status // accordingly. if tr.HasTimedOut() { - before := tr.Status.GetCondition(apis.ConditionSucceeded) message := fmt.Sprintf("TaskRun %q failed to finish within %q", tr.Name, tr.GetTimeout()) err := c.failTaskRun(ctx, tr, v1beta1.TaskRunReasonTimedOut, message) return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err) @@ -184,7 +184,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg } // Store the condition before reconcile - before := tr.Status.GetCondition(apis.ConditionSucceeded) + 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.