Use step.ImageID instead of looking into status.TaskSpec

`tr.Status.TaskSpec.Steps` can be out-of-sync with `tr.Status.Steps`. As we already have the image information (through `ImageID`) in the struct be are getting from our iteration, we don't need to look into another array, with the risk of getting a panic. The same goes for sidecars. We managed to get multiple panics on the controller prior to this change. See #4952 for the initial implementation. Signed-off-by: Vincent Demeester <[email protected]>
tektoncd · Aug 1, 2022 · 03b9fde · 03b9fde
1 parent 19940f2
commit 03b9fde
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 4 deletions.
diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go
@@ -197,16 +197,16 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg
 }
 
 func (c *Reconciler) checkPodFailed(ctx context.Context, tr *v1beta1.TaskRun) (bool, v1beta1.TaskRunReason, string) {
-	for index, step := range tr.Status.Steps {
+	for _, step := range tr.Status.Steps {
 		if step.Waiting != nil && step.Waiting.Reason == "ImagePullBackOff" {
-			image := tr.Status.TaskSpec.Steps[index].Image
+			image := step.ImageID
 			message := fmt.Sprintf(`The step %q in TaskRun %q failed to pull the image %q. The pod errored with the message: "%s."`, step.Name, tr.Name, image, step.Waiting.Message)
 			return true, v1beta1.TaskRunReasonImagePullFailed, message
 		}
 	}
-	for index, sidecar := range tr.Status.Sidecars {
+	for _, sidecar := range tr.Status.Sidecars {
 		if sidecar.Waiting != nil && sidecar.Waiting.Reason == "ImagePullBackOff" {
-			image := tr.Status.TaskSpec.Sidecars[index].Image
+			image := sidecar.ImageID
 			message := fmt.Sprintf(`The sidecar %q in TaskRun %q failed to pull the image %q. The pod errored with the message: "%s."`, sidecar.Name, tr.Name, image, sidecar.Waiting.Message)
 			return true, v1beta1.TaskRunReasonImagePullFailed, message
 		}

diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -1988,6 +1988,7 @@ status:
   steps:
   - container: step-unnamed-0
     name: unnamed-0
+    imageID: whatever
     waiting:
       message: Back-off pulling image "whatever"
       reason: ImagePullBackOff
@@ -2051,6 +2052,7 @@ status:
       startedAt: "2022-06-09T10:13:41Z"
   - container: step-unnamed-1
     name: unnamed-1
+    imageID: whatever
     waiting:
       message: Back-off pulling image "whatever"
       reason: ImagePullBackOff