unpack retry e2e tests

Signed-off-by: Ankita Thomas <ankithom@redhat.com>

pkg/controller/bundle/bundle_unpacker.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8slabels "k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/client-go/kubernetes"
 	listersbatchv1 "k8s.io/client-go/listers/batch/v1"
 	listerscorev1 "k8s.io/client-go/listers/core/v1"
@@ -96,6 +97,7 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
 		ObjectMeta: metav1.ObjectMeta{
 			Labels: map[string]string{
 				install.OLMManagedLabelKey: install.OLMManagedLabelValue,
+				bundleUnpackRefLabel:       cmRef.Name,
 			},
 		},
 		Spec: batchv1.JobSpec{
@@ -254,7 +256,6 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
 	}
 	job.SetNamespace(cmRef.Namespace)
 	job.SetName(cmRef.Name)
-	job.SetLabels(map[string]string{bundleUnpackRefLabel: cmRef.Name})
 	job.SetOwnerReferences([]metav1.OwnerReference{ownerRef(cmRef)})
 	if c.runAsUser > 0 {
 		job.Spec.Template.Spec.SecurityContext.RunAsUser = &c.runAsUser
@@ -695,10 +696,10 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
 
 			if failed {
 				if time.Now().After(lastFailureTime.Add(unpackRetryInterval)) {
-					fresh.SetName(fmt.Sprintf("%s-%d", fresh.GetName(), len(jobs)))
+					fresh.SetName(names.SimpleNameGenerator.GenerateName(fresh.GetName()))
 					job, err = c.client.BatchV1().Jobs(fresh.GetNamespace()).Create(context.TODO(), fresh, metav1.CreateOptions{})
-					return
 				}
+				return
 			}
 		}
 	}
@@ -890,7 +891,7 @@ func OperatorGroupBundleUnpackRetryInterval(ogLister v1listers.OperatorGroupName
 
 	d, err := time.ParseDuration(timeoutStr)
 	if err != nil {
-		return 0, fmt.Errorf("failed to parse unpack timeout annotation(%s: %s): %w", BundleUnpackRetryMinimumIntervalAnnotationKey, timeoutStr, err)
+		return 0, fmt.Errorf("failed to parse unpack retry annotation(%s: %s): %w", BundleUnpackRetryMinimumIntervalAnnotationKey, timeoutStr, err)
 	}
 
 	return d, nil

pkg/controller/bundle/bundle_unpacker_test.go

@@ -208,7 +208,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 						ObjectMeta: metav1.ObjectMeta{
 							Name:      pathHash,
 							Namespace: "ns-a",
-							Labels:    map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue},
+							Labels:    map[string]string{install.OLMManagedLabelKey: install.OLMManagedLabelValue, bundleUnpackRefLabel: pathHash},
 							OwnerReferences: []metav1.OwnerReference{
 								{
 									APIVersion:         "v1",
@@ -1839,3 +1839,115 @@ func TestOperatorGroupBundleUnpackTimeout(t *testing.T) {
 		})
 	}
 }
+
+func TestOperatorGroupBundleUnpackRetryInterval(t *testing.T) {
+	nsName := "fake-ns"
+
+	for _, tc := range []struct {
+		name            string
+		operatorGroups  []*operatorsv1.OperatorGroup
+		expectedTimeout time.Duration
+		expectedError   error
+	}{
+		{
+			name:            "No operator groups exist",
+			expectedTimeout: 0,
+			expectedError:   errors.New("found 0 operatorGroups, expected 1"),
+		},
+		{
+			name: "Multiple operator groups exist",
+			operatorGroups: []*operatorsv1.OperatorGroup{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       operatorsv1.OperatorGroupKind,
+						APIVersion: operatorsv1.GroupVersion.String(),
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "og1",
+						Namespace: nsName,
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       operatorsv1.OperatorGroupKind,
+						APIVersion: operatorsv1.GroupVersion.String(),
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "og2",
+						Namespace: nsName,
+					},
+				},
+			},
+			expectedTimeout: 0,
+			expectedError:   errors.New("found 2 operatorGroups, expected 1"),
+		},
+		{
+			name: "One operator group exists with valid unpack retry annotation",
+			operatorGroups: []*operatorsv1.OperatorGroup{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       operatorsv1.OperatorGroupKind,
+						APIVersion: operatorsv1.GroupVersion.String(),
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:        "og",
+						Namespace:   nsName,
+						Annotations: map[string]string{BundleUnpackRetryMinimumIntervalAnnotationKey: "1m"},
+					},
+				},
+			},
+			expectedTimeout: 1 * time.Minute,
+			expectedError:   nil,
+		},
+		{
+			name: "One operator group exists with no unpack retry annotation",
+			operatorGroups: []*operatorsv1.OperatorGroup{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       operatorsv1.OperatorGroupKind,
+						APIVersion: operatorsv1.GroupVersion.String(),
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "og",
+						Namespace: nsName,
+					},
+				},
+			},
+			expectedTimeout: 0,
+			expectedError:   nil,
+		},
+		{
+			name: "One operator group exists with invalid unpack retry annotation",
+			operatorGroups: []*operatorsv1.OperatorGroup{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       operatorsv1.OperatorGroupKind,
+						APIVersion: operatorsv1.GroupVersion.String(),
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:        "og",
+						Namespace:   nsName,
+						Annotations: map[string]string{BundleUnpackRetryMinimumIntervalAnnotationKey: "invalid"},
+					},
+				},
+			},
+			expectedTimeout: 0,
+			expectedError:   fmt.Errorf("failed to parse unpack retry annotation(operatorframework.io/bundle-unpack-min-retry-interval: invalid): %w", errors.New("time: invalid duration \"invalid\"")),
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ogIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
+			ogLister := v1listers.NewOperatorGroupLister(ogIndexer).OperatorGroups(nsName)
+
+			for _, og := range tc.operatorGroups {
+				err := ogIndexer.Add(og)
+				assert.NoError(t, err)
+			}
+
+			timeout, err := OperatorGroupBundleUnpackRetryInterval(ogLister)
+
+			assert.Equal(t, tc.expectedTimeout, timeout)
+			assert.Equal(t, tc.expectedError, err)
+		})
+	}
+}

pkg/controller/operators/catalog/subscriptions_test.go

@@ -1172,7 +1172,7 @@ func TestSyncSubscriptions(t *testing.T) {
 			defer cancel()
 
 			fakeBundleUnpacker := &bundlefakes.FakeUnpacker{
-				UnpackBundleStub: func(lookup *v1alpha1.BundleLookup, timeout time.Duration) (*bundle.BundleUnpackResult, error) {
+				UnpackBundleStub: func(lookup *v1alpha1.BundleLookup, timeout, retryInterval time.Duration) (*bundle.BundleUnpackResult, error) {
 					return &bundle.BundleUnpackResult{BundleLookup: lookup.DeepCopy()}, tt.fields.unpackBundleErr
 				},
 			}

test/e2e/fbc_provider.go

@@ -31,3 +31,9 @@ func NewFileBasedFiledBasedCatalogProvider(path string) (FileBasedCatalogProvide
 func (f *fileBasedFileBasedCatalogProvider) GetCatalog() string {
 	return f.fbc
 }
+
+func NewRawFileBasedCatalogProvider(data string) (FileBasedCatalogProvider, error) {
+	return &fileBasedFileBasedCatalogProvider{
+		fbc: string(data),
+	}, nil
+}

test/e2e/registry.go

@@ -54,6 +54,7 @@ func createDockerRegistry(client operatorclient.ClientInterface, namespace strin
 					Port: int32(5000),
 				},
 			},
+			Type: corev1.ServiceTypeNodePort,
 		},
 	}