diff --git a/staging/operator-lifecycle-manager/Makefile b/staging/operator-lifecycle-manager/Makefile index 6220e2f24d..43e319fb0e 100644 --- a/staging/operator-lifecycle-manager/Makefile +++ b/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 diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 703d76d284..455d9f4646 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/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 { diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go index 612a8a6244..f85d9ef790 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go +++ b/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 diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go index 3c6ee8dbf9..b438680ce1 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/decorators/operator_test.go +++ b/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) + } +} diff --git a/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go index ac14002c4a..997e07c54d 100644 --- a/staging/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go +++ b/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() { diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 703d76d284..455d9f4646 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/vendor/github.com/operator-framework/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 { diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go index 612a8a6244..f85d9ef790 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/decorators/operator.go +++ b/vendor/github.com/operator-framework/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