From 87b6978b8150f808496038d80610bc15fc2a55a7 Mon Sep 17 00:00:00 2001 From: Anik Bhattacharjee Date: Fri, 12 Nov 2021 17:33:03 -0500 Subject: [PATCH] (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. Signed-off-by: Anik Bhattacharjee --- pkg/controller/operators/catalog/operator.go | 17 +- test/e2e/catalog_e2e_test.go | 160 +++++++++++------- .../operators/v1alpha1/catalogsource_types.go | 35 +++- 3 files changed, 148 insertions(+), 64 deletions(-) diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go index a66712d8783..07de910e7dc 100644 --- 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 len(out.Spec.UpdateStrategy.RegistryPoll.ParsingError) != 0 { + 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 setting catalogsource 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 index 74cda7cebd4..2c67289782d 100644 --- a/test/e2e/catalog_e2e_test.go +++ b/test/e2e/catalog_e2e_test.go @@ -32,7 +32,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 @@ -973,76 +973,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{ - Interval: &metav1.Duration{Duration: 45 * time.Second}, + + 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.TODO(), 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.TODO(), 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() { diff --git a/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go b/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go index af38df203fa..02eeb18edea 100644 --- a/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go +++ b/vendor/github.com/operator-framework/api/pkg/operators/v1alpha1/catalogsource_types.go @@ -1,6 +1,7 @@ package v1alpha1 import ( + "encoding/json" "fmt" "time" @@ -10,8 +11,9 @@ import ( ) const ( - CatalogSourceCRDAPIVersion = GroupName + "/" + GroupVersion - CatalogSourceKind = "CatalogSource" + CatalogSourceCRDAPIVersion = GroupName + "/" + GroupVersion + CatalogSourceKind = "CatalogSource" + DefaultRegistryPollDuration = 15 * time.Minute ) // SourceType indicates the type of backing store for a CatalogSource @@ -36,6 +38,8 @@ const ( CatalogSourceConfigMapError ConditionReason = "ConfigMapError" // CatalogSourceRegistryServerError denotes when there is an issue querying the specified registry server. CatalogSourceRegistryServerError ConditionReason = "RegistryServerError" + // CatalogSourceIntervalInvalidError denotes if the registry polling interval is invalid. + CatalogSourceIntervalInvalidError ConditionReason = "InvalidIntervalError" ) type CatalogSourceSpec struct { @@ -96,7 +100,32 @@ type RegistryPoll struct { // Interval is used to determine the time interval between checks of the latest catalog source version. // The catalog operator polls to see if a new version of the catalog source is available. // If available, the latest image is pulled and gRPC traffic is directed to the latest catalog source. - Interval *metav1.Duration `json:"interval,omitempty"` + RawInterval string `json:"interval,omitempty"` + Interval *metav1.Duration `json:"-"` + ParsingError string `json:"-"` +} + +// UnmarshalJSON implements the encoding/json.Unmarshaler interface. +func (u *UpdateStrategy) UnmarshalJSON(data []byte) (err error) { + type alias struct { + *RegistryPoll `json:"registryPoll,omitempty"` + } + us := alias{} + if err = json.Unmarshal(data, &us); err != nil { + return err + } + registryPoll := &RegistryPoll{ + RawInterval: us.RegistryPoll.RawInterval, + } + duration, err := time.ParseDuration(registryPoll.RawInterval) + if err != nil { + registryPoll.ParsingError = fmt.Sprintf("error parsing spec.updateStrategy.registryPoll.interval. Using the default value of %s instead. Error: %s", DefaultRegistryPollDuration, err) + registryPoll.Interval = &metav1.Duration{Duration: DefaultRegistryPollDuration} + } else { + registryPoll.Interval = &metav1.Duration{Duration: duration} + } + u.RegistryPoll = registryPoll + return nil } type RegistryServiceStatus struct {