From 3e6e4dd516bb43d796334be4634647cfc91c4c97 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Wed, 28 Sep 2022 14:16:58 -0400 Subject: [PATCH] Write TaskRun.Status.TaskSpec with replaced spec on every reconcile run fixes #5574 With #5537, the call to `applyParamsContextsResultsAndWorkspaces` and setting of `tr.Status.TaskSpec` to the output of that call was moved from happening with every call to `reconcile` to only happening if there wasn't already a pod created. This didn't cause any problems with execution, but it sometimes resulted in `tr.Status.TaskSpec` not getting set properly or getting reset to `nil` on a subsequent `reconcile` run, with the end result that, sometimes, `tr.Status.TaskSpec` would contain the original `TaskSpec` without parameter, result, context, and workspace references being replaced with the corresponding value. This caused flaky failures in the `TestPipelineRunStatusSpec/pipeline_status_spec_updated` e2e test, and integration test failures in Chains (https://github.com/tektoncd/chains/issues/577). This change moves the PVC creation out of the `if pod == nil {` block that creates the pod if needed, while still checking if `pod` is `nil` before creating the PVC, and brings the `applyParamsContextsResultsAndWorkspaces` call and setting of `tr.Status.TaskSpec` back out of the pod creation block, but after the possible PVC creation. Signed-off-by: Andrew Bayer --- pkg/reconciler/taskrun/taskrun.go | 46 +++++++++++++++++-------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index d19fef28578..7e45a0136d2 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -481,31 +481,35 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *re } } - if pod == nil { - if tr.HasVolumeClaimTemplate() { - if err := c.pvcHandler.CreatePersistentVolumeClaimsForWorkspaces(ctx, tr.Spec.Workspaces, *kmeta.NewControllerRef(tr), tr.Namespace); err != nil { - logger.Errorf("Failed to create PVC for TaskRun %s: %v", tr.Name, err) - tr.Status.MarkResourceFailed(volumeclaim.ReasonCouldntCreateWorkspacePVC, - fmt.Errorf("Failed to create PVC for TaskRun %s workspaces correctly: %s", - fmt.Sprintf("%s/%s", tr.Namespace, tr.Name), err)) - return controller.NewPermanentError(err) - } - - taskRunWorkspaces := applyVolumeClaimTemplates(tr.Spec.Workspaces, *kmeta.NewControllerRef(tr)) - // This is used by createPod below. Changes to the Spec are not updated. - tr.Spec.Workspaces = taskRunWorkspaces + if pod == nil && tr.HasVolumeClaimTemplate() { + if err := c.pvcHandler.CreatePersistentVolumeClaimsForWorkspaces(ctx, tr.Spec.Workspaces, *kmeta.NewControllerRef(tr), tr.Namespace); err != nil { + logger.Errorf("Failed to create PVC for TaskRun %s: %v", tr.Name, err) + tr.Status.MarkResourceFailed(volumeclaim.ReasonCouldntCreateWorkspacePVC, + fmt.Errorf("Failed to create PVC for TaskRun %s workspaces correctly: %s", + fmt.Sprintf("%s/%s", tr.Namespace, tr.Name), err)) + return controller.NewPermanentError(err) } - // Get the randomized volume names assigned to workspace bindings - workspaceVolumes := workspace.CreateVolumes(tr.Spec.Workspaces) + taskRunWorkspaces := applyVolumeClaimTemplates(tr.Spec.Workspaces, *kmeta.NewControllerRef(tr)) + // This is used by createPod below. Changes to the Spec are not updated. + tr.Spec.Workspaces = taskRunWorkspaces + } - ts, err := applyParamsContextsResultsAndWorkspaces(ctx, tr, rtr, workspaceVolumes) - if err != nil { - logger.Errorf("Error updating task spec parameters, contexts, results and workspaces: %s", err) - return err - } - tr.Status.TaskSpec = ts + // Get the randomized volume names assigned to workspace bindings + workspaceVolumes := workspace.CreateVolumes(tr.Spec.Workspaces) + ts, err := applyParamsContextsResultsAndWorkspaces(ctx, tr, rtr, workspaceVolumes) + if err != nil { + logger.Errorf("Error updating task spec parameters, contexts, results and workspaces: %s", err) + return err + } + tr.Status.TaskSpec = ts + + if len(tr.Status.TaskSpec.Steps) > 0 { + logger.Infof("set taskspec for %s/%s - script: %s", tr.Namespace, tr.Name, tr.Status.TaskSpec.Steps[0].Script) + } + + if pod == nil { pod, err = c.createPod(ctx, ts, tr, rtr, workspaceVolumes) if err != nil { newErr := c.handlePodCreationError(tr, err)