(catsrc) set status reason/message on incorrect polling interval

This PR sets the status reason as InvalidIntervalError and a status message if updateStrategy.RegistryPoll.ParsingError is set for the catsrc.
operator-framework · Mar 7, 2022 · d5c43b1 · d5c43b1
1 parent 2d649b0
commit d5c43b1
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 61 deletions.
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
@@ -727,10 +727,19 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
 
 	// requeue the catalog sync based on the polling interval, for accurate syncs of catalogs with polling enabled
 	if out.Spec.UpdateStrategy != nil {
-		logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
-		resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
-		o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
-		return
+		if out.Spec.UpdateStrategy.RegistryPoll != nil {
+			if out.Spec.UpdateStrategy.RegistryPoll.ParsingError != "" {
+				out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, fmt.Errorf(out.Spec.UpdateStrategy.RegistryPoll.ParsingError))
+				if _, err := o.client.OperatorsV1alpha1().CatalogSources(out.GetNamespace()).UpdateStatus(context.TODO(), out, metav1.UpdateOptions{}); err != nil {
+					logger.Errorf("error while updating catalogsource status for an invalid interval - %v", err)
+					return
+				}
+			}
+			logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
+			resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
+			o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
+			return
+		}
 	}
 
 	if err := o.sources.Remove(sourceKey); err != nil {

diff --git a/test/e2e/catalog_e2e_test.go b/test/e2e/catalog_e2e_test.go
@@ -33,7 +33,7 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx"
 )
 
-var _ = Describe("Catalog represents a store of bundles which OLM can use to install Operators", func() {
+var _ = Describe("Starting CatalogSource e2e tests", func() {
 	var (
 		c   operatorclient.ClientInterface
 		crc versioned.Interface
@@ -987,76 +987,122 @@ var _ = Describe("Catalog represents a store of bundles which OLM can use to ins
 		Expect(err).ShouldNot(HaveOccurred())
 		Expect(csv.Spec.Replaces).To(Equal("busybox-dependency.v1.0.0"))
 	})
-	It("registry polls on the correct interval", func() {
-		// Create a catalog source with polling enabled
-		// Confirm the following
-		//   a) the new update pod is spun up roughly in line with the registry polling interval
-		//   b) the update pod is removed quickly when the image is found to not have changed
-		// This is more of a behavioral test that ensures the feature is working as designed.
-
-		c := newKubeClient()
-		crc := newCRClient()
+	When("A catalogSource is created with correct interval", func() {
 
+		var source *v1alpha1.CatalogSource
+		singlePod := podCount(1)
 		sourceName := genName("catalog-")
-		source := &v1alpha1.CatalogSource{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       v1alpha1.CatalogSourceKind,
-				APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      sourceName,
-				Namespace: testNamespace,
-				Labels:    map[string]string{"olm.catalogSource": sourceName},
-			},
-			Spec: v1alpha1.CatalogSourceSpec{
-				SourceType: v1alpha1.SourceTypeGrpc,
-				Image:      "quay.io/olmtest/catsrc-update-test:new",
-				UpdateStrategy: &v1alpha1.UpdateStrategy{
-					RegistryPoll: &v1alpha1.RegistryPoll{
-						RawInterval: "45s",
+
+		BeforeEach(func() {
+			source = &v1alpha1.CatalogSource{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       v1alpha1.CatalogSourceKind,
+					APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      sourceName,
+					Namespace: testNamespace,
+					Labels:    map[string]string{"olm.catalogSource": sourceName},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					SourceType: v1alpha1.SourceTypeGrpc,
+					Image:      "quay.io/olmtest/catsrc-update-test:new",
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							RawInterval: "45s",
+						},
 					},
 				},
-			},
-		}
+			}
 
-		source, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.Background(), source, metav1.CreateOptions{})
-		Expect(err).ToNot(HaveOccurred())
+			source, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
 
-		// wait for new catalog source pod to be created and report ready
-		selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
-		singlePod := podCount(1)
-		catalogPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(catalogPods).ToNot(BeNil())
+			// wait for new catalog source pod to be created and report ready
+			selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
 
-		Eventually(func() (bool, error) {
-			podList, err := c.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).List(context.Background(), metav1.ListOptions{LabelSelector: selector.String()})
-			if err != nil {
-				return false, err
-			}
+			catalogPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(catalogPods).ToNot(BeNil())
 
-			for _, p := range podList.Items {
-				if podReady(&p) {
-					return true, nil
+			Eventually(func() (bool, error) {
+				podList, err := c.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).List(context.TODO(), metav1.ListOptions{LabelSelector: selector.String()})
+				if err != nil {
+					return false, err
 				}
+
+				for _, p := range podList.Items {
+					if podReady(&p) {
+						return true, nil
+					}
+					return false, nil
+				}
+
 				return false, nil
+			}).Should(BeTrue())
+		})
+
+		It("registry polls on the correct interval", func() {
+			// Wait roughly the polling interval for update pod to show up
+			updateSelector := labels.SelectorFromSet(map[string]string{"catalogsource.operators.coreos.com/update": source.GetName()})
+			updatePods, err := awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 5*time.Second, 2*time.Minute, singlePod)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(updatePods).ToNot(BeNil())
+			Expect(updatePods.Items).To(HaveLen(1))
+
+			// No update to image: update pod should be deleted quickly
+			noPod := podCount(0)
+			updatePods, err = awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 1*time.Second, 30*time.Second, noPod)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(updatePods.Items).To(HaveLen(0))
+		})
+
+	})
+
+	When("A catalogSource is created with incorrect interval", func() {
+
+		var source *v1alpha1.CatalogSource
+
+		BeforeEach(func() {
+			sourceName := genName("catalog-")
+			source = &v1alpha1.CatalogSource{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       v1alpha1.CatalogSourceKind,
+					APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      sourceName,
+					Namespace: testNamespace,
+					Labels:    map[string]string{"olm.catalogSource": sourceName},
+				},
+				Spec: v1alpha1.CatalogSourceSpec{
+					SourceType: v1alpha1.SourceTypeGrpc,
+					Image:      "quay.io/olmtest/catsrc-update-test:new",
+					UpdateStrategy: &v1alpha1.UpdateStrategy{
+						RegistryPoll: &v1alpha1.RegistryPoll{
+							RawInterval: "45mError.code",
+						},
+					},
+				},
 			}
 
-			return false, nil
-		}).Should(BeTrue())
+			_, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
 
-		// Wait roughly the polling interval for update pod to show up
-		updateSelector := labels.SelectorFromSet(map[string]string{"catalogsource.operators.coreos.com/update": source.GetName()})
-		updatePods, err := awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 5*time.Second, 2*time.Minute, singlePod)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(updatePods).ToNot(BeNil())
-		Expect(updatePods.Items).To(HaveLen(1))
+		})
 
-		// No update to image: update pod should be deleted quickly
-		noPod := podCount(0)
-		updatePods, err = awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 1*time.Second, 30*time.Second, noPod)
-		Expect(err).ToNot(HaveOccurred())
-		Expect(updatePods.Items).To(HaveLen(0))
+		It("the catalogsource status communicates that a default interval time is being used instead", func() {
+			Eventually(func() bool {
+				catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+				if catsrc.Status.Reason == v1alpha1.CatalogSourceIntervalInvalidError {
+					if catsrc.Status.Message == "error parsing spec.updateStrategy.registryPoll.interval. Using the default value of 15m0s instead. Error: time: unknown unit \"mError\" in duration \"45mError.code\"" {
+						return true
+					}
+				}
+				return false
+			}).Should(BeTrue())
+		})
 	})
 
 	It("adding catalog template adjusts image used", func() {