From a007e516c58368226d5da575342654a6e681bf5f Mon Sep 17 00:00:00 2001
From: Ricardo Maraschini <ricardo.maraschini@gmail.com>
Date: Fri, 22 May 2020 15:30:11 +0200
Subject: [PATCH] Allow scheduled retry of failed ImageStream import

If an ImageStream tag refers to an image by SHA and we fail to import it
we need to allow the schedule process to take over an try again. With this
patch we now retry to import image stream tags based on SHA until it succeeds
at least once.
---
 .../controller/scheduled_image_controller.go  | 20 ++++++++---
 .../scheduled_image_controller_test.go        | 35 +++++++++++++++++++
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/pkg/image/controller/scheduled_image_controller.go b/pkg/image/controller/scheduled_image_controller.go
index 11e5e126e..a8b793f30 100644
--- a/pkg/image/controller/scheduled_image_controller.go
+++ b/pkg/image/controller/scheduled_image_controller.go
@@ -13,6 +13,7 @@ import (
 
 	imagev1 "github.com/openshift/api/image/v1"
 	imagev1lister "github.com/openshift/client-go/image/listers/image/v1"
+	"github.com/openshift/library-go/pkg/image/imageutil"
 	imageref "github.com/openshift/library-go/pkg/image/reference"
 	metrics "github.com/openshift/openshift-controller-manager/pkg/image/metrics/prometheus"
 )
@@ -182,9 +183,10 @@ func resetScheduledTags(stream *imagev1.ImageStream) {
 		if tagImportable(tagRef) && tagRef.ImportPolicy.Scheduled {
 			if ref, err := imageref.Parse(tagRef.From.Name); err != nil && len(tagRef.From.Name) > 0 {
 				continue
-			} else if len(ref.ID) > 0 && !tagNeedsImport(stream, tagRef, true) {
+			} else if len(ref.ID) > 0 && tagImported(stream, tagRef) {
 				// ref.ID is set if this is a canonical, sha/digest ref;
-				// we allow scheduled import of those, but only until the initially succeed
+				// we allow scheduled import of those, but only until it succeeds
+				// at least once.
 				continue
 			}
 			tagRef.Generation = &next
@@ -193,15 +195,25 @@ func resetScheduledTags(stream *imagev1.ImageStream) {
 	}
 }
 
+// tagImported returns if a tag has been imported and is on the last generation.
+func tagImported(stream *imagev1.ImageStream, tag imagev1.TagReference) bool {
+	tagEvents, hasTag := imageutil.StatusHasTag(stream, tag.Name)
+	if !hasTag || tag.Generation == nil || len(tagEvents.Items) == 0 {
+		return false
+	}
+	return tagEvents.Items[0].Generation == *tag.Generation
+}
+
 // needsScheduling returns true if this image stream has any scheduled tags
 func needsScheduling(stream *imagev1.ImageStream) bool {
 	for _, tagRef := range stream.Spec.Tags {
 		if tagImportable(tagRef) && tagRef.ImportPolicy.Scheduled {
 			if ref, err := imageref.Parse(tagRef.From.Name); err != nil && len(tagRef.From.Name) > 0 {
 				continue
-			} else if len(ref.ID) > 0 && !tagNeedsImport(stream, tagRef, true) {
+			} else if len(ref.ID) > 0 && tagImported(stream, tagRef) {
 				// ref.ID is set if this is a canonical, sha/digest ref;
-				// we allow scheduled import of those, but only until the initially succeed
+				// we allow scheduled import of those, but only until it succeeds
+				// at least once.
 				continue
 			}
 			return true
diff --git a/pkg/image/controller/scheduled_image_controller_test.go b/pkg/image/controller/scheduled_image_controller_test.go
index c8c5d7c33..6469bde75 100644
--- a/pkg/image/controller/scheduled_image_controller_test.go
+++ b/pkg/image/controller/scheduled_image_controller_test.go
@@ -160,6 +160,41 @@ func TestSchedulingChecks(t *testing.T) {
 			},
 			expectedResult: false,
 		},
+		"sha ref that failed will import": {
+			is: &imagev1.ImageStream{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test", Namespace: "other", UID: "1", ResourceVersion: "1",
+					Annotations: map[string]string{imagev1.DockerImageRepositoryCheckAnnotation: "done"},
+					Generation:  1,
+				},
+				Spec: imagev1.ImageStreamSpec{
+					Tags: []imagev1.TagReference{
+						{
+							Name:         "default1",
+							From:         &corev1.ObjectReference{Kind: "DockerImage", Name: "abc@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25"},
+							Generation:   &one,
+							ImportPolicy: imagev1.TagImportPolicy{Scheduled: true},
+						},
+					},
+				},
+				Status: imagev1.ImageStreamStatus{
+					Tags: []imagev1.NamedTagEventList{
+						{
+							Tag: "default1",
+							Conditions: []imagev1.TagEventCondition{
+								{
+									Type:       imagev1.ImportSuccess,
+									Status:     corev1.ConditionFalse,
+									Generation: 1,
+									Message:    "transient error",
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedResult: true,
+		},
 		"sha ref not imported": {
 			is: &imagev1.ImageStream{
 				ObjectMeta: metav1.ObjectMeta{