Merge pull request #285 from perdasilva/sync_04_11

Sync 04 11

Author: openshift-merge-robot

staging/operator-lifecycle-manager/Makefile

@@ -279,10 +279,13 @@ uninstall:
 	- kubectl delete clusterroles.rbac.authorization.k8s.io "system:controller:operator-lifecycle-manager"
 	- kubectl delete clusterrolebindings.rbac.authorization.k8s.io "olm-operator-binding-openshift-operator-lifecycle-manager"
 
-.PHONY: run-local
-run-local: build-linux build-wait build-util-linux
+.PHONY: build-local
+build-local: build-linux build-wait build-util-linux
 	rm -rf build
 	. ./scripts/build_local.sh
+
+.PHONY: run-local
+run-local: build-local
 	mkdir -p build/resources
 	. ./scripts/package_release.sh 1.0.0 build/resources doc/install/local-values.yaml
 	. ./scripts/install_local.sh $(LOCAL_NAMESPACE) build/resources

staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go

@@ -727,10 +727,15 @@ 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.Status.Reason != v1alpha1.CatalogSourceIntervalInvalidError {
+				out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, fmt.Errorf(out.Spec.UpdateStrategy.RegistryPoll.ParsingError))
+			}
+			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 {

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go

@@ -128,14 +128,32 @@ func (o *Operator) ComponentLabelKey() (string, error) {
 		return o.componentLabelKey, nil
 	}
 
-	if o.GetName() == "" {
+	name := o.GetName()
+	if name == "" {
 		return "", fmt.Errorf(componentLabelKeyError, "empty name field")
 	}
 
-	name := o.GetName()
 	if len(name) > 63 {
 		// Truncate
 		name = name[0:63]
+
+		// Remove trailing illegal characters
+		idx := len(name) - 1
+		for ; idx >= 0; idx-- {
+			lastChar := name[idx]
+			if lastChar != '.' && lastChar != '_' && lastChar != '-' {
+				break
+			}
+		}
+
+		// Just being defensive. This is unlikely to happen since the operator name should
+		// be compatible kubernetes naming constraints
+		if idx < 0 {
+			return "", fmt.Errorf(componentLabelKeyError, "unsupported name field")
+		}
+
+		// Update Label
+		name = name[0 : idx+1]
 	}
 	o.componentLabelKey = ComponentLabelKeyPrefix + name
 

staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go

@@ -231,3 +231,61 @@ func TestAddComponents(t *testing.T) {
 		})
 	}
 }
+
+func TestComponentLabelKey(t *testing.T) {
+	tests := []struct {
+		description       string
+		componentLabelKey string
+		name              string
+		expectedLabel     string
+		returnsError      bool
+	}{
+		{
+			description:       "when componentLabelKey is set then return it",
+			componentLabelKey: "my-component-label",
+			name:              "my-operator",
+			expectedLabel:     "my-component-label",
+			returnsError:      false,
+		},
+		{
+			description:       "when componentLabelKey is not set and operator name is less than 63 characters then do not truncate",
+			componentLabelKey: "",
+			name:              "my-operator",
+			expectedLabel:     ComponentLabelKeyPrefix + "my-operator",
+			returnsError:      false,
+		},
+		{
+			description:   "when componentLabelKey is not set and operator name is more than 63 characters truncate",
+			name:          "this-is-my-operator-its-the-coolest-you-got-a-problem-with-that-come-at-me-bro",
+			expectedLabel: ComponentLabelKeyPrefix + "this-is-my-operator-its-the-coolest-you-got-a-problem-with-that",
+			returnsError:  false,
+		},
+		{
+			description:   "when componentLabelKey is not set and operator name is more than 63 characters truncate and drop trailing illegal characters",
+			name:          "this-is-my-operator-its-the-coolest-you-got-a-problem-with----...---___...---",
+			expectedLabel: ComponentLabelKeyPrefix + "this-is-my-operator-its-the-coolest-you-got-a-problem-with",
+			returnsError:  false,
+		},
+		{
+			description:   "when componentLabelKey is not set and operator name is more than 63 characters and is made up of illegal characters then return error",
+			name:          "----...---___...-------...---___...-------...---___...-------...---___...---",
+			expectedLabel: "",
+			returnsError:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		operator := &Operator{
+			Operator: &operatorsv1.Operator{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: tt.name,
+				},
+			},
+			componentLabelKey: tt.componentLabelKey,
+		}
+
+		actualLabel, actualErr := operator.ComponentLabelKey()
+		require.Equal(t, tt.returnsError, actualErr != nil, actualErr)
+		require.Equal(t, tt.expectedLabel, actualLabel)
+	}
+}

staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go

@@ -42,7 +42,7 @@ const (
 	catsrcImage           = "docker://quay.io/olmtest/catsrc-update-test:"
 )
 
-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
@@ -1074,76 +1074,156 @@ 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 polling 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: ns.GetName(),
-				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())
+
+			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
+				for _, p := range podList.Items {
+					if podReady(&p) {
+						return true, nil
+					}
+					return false, nil
 				}
+
 				return false, nil
-			}
+			}).Should(BeTrue())
+		})
 
-			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))
+		})
 
-		// 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 polling interval", func() {
+
+		var (
+			source     *v1alpha1.CatalogSource
+			sourceName string
+		)
+		const (
+			incorrectInterval = "45mError.code"
+			correctInterval   = "45m"
+		)
+		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: incorrectInterval,
+						},
+					},
+				},
+			}
+
+			_, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
+			Expect(err).ToNot(HaveOccurred())
+
+		})
+		AfterEach(func() {
+			err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Delete(context.TODO(), source.GetName(), metav1.DeleteOptions{})
+			Expect(err).ToNot(HaveOccurred())
+		})
+		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())
+		})
+		When("the catalogsource is updated with a valid polling interval", func() {
+			BeforeEach(func() {
+				catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+				catsrc.Spec.UpdateStrategy.RegistryPoll.RawInterval = correctInterval
+				_, err = crc.OperatorsV1alpha1().CatalogSources(catsrc.GetNamespace()).Update(context.TODO(), catsrc, metav1.UpdateOptions{})
+				Expect(err).ToNot(HaveOccurred())
+			})
+			It("the catalogsource spec shows the updated polling interval, and the error message in the status is cleared", func() {
+				Eventually(func() error {
+					catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
+					if err != nil {
+						return err
+					}
+					expectedTime, err := time.ParseDuration(correctInterval)
+					if err != nil {
+						return err
+					}
+					if catsrc.Status.Reason != "" || (catsrc.Spec.UpdateStrategy.Interval != &metav1.Duration{expectedTime}) {
+						return err
+					}
+					return nil
+				}).Should(Succeed())
+			})
+		})
 	})
 
 	It("adding catalog template adjusts image used", func() {