From 1d4829040302d5aad9e68791b44e68412af525e1 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 31 Aug 2020 16:20:46 -0400 Subject: [PATCH 01/24] Add getSupportedInstallModes --- internal/olm/operator/bundle/install.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/olm/operator/bundle/install.go b/internal/olm/operator/bundle/install.go index a2ab4c7319..b55bdcb132 100644 --- a/internal/olm/operator/bundle/install.go +++ b/internal/olm/operator/bundle/install.go @@ -24,6 +24,7 @@ import ( apimanifests "github.com/operator-framework/api/pkg/manifests" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/operator-sdk/internal/olm/operator" "github.com/operator-framework/operator-sdk/internal/olm/operator/registry" @@ -78,6 +79,7 @@ func (i *Install) setup(ctx context.Context) error { i.OperatorInstaller.PackageName = labels["operators.operatorframework.io.bundle.package.v1"] i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = csv.Name + i.OperatorInstaller.SupportedInstallModes = getSupportedInstallModes(csv.Spec.InstallModes) i.OperatorInstaller.Channel = strings.Split(labels["operators.operatorframework.io.bundle.channels.v1"], ",")[0] i.IndexImageCatalogCreator.BundleImage = i.BundleImage i.IndexImageCatalogCreator.PackageName = i.OperatorInstaller.PackageName @@ -116,3 +118,13 @@ func loadBundle(ctx context.Context, bundleImage string) (registryutil.Labels, * return labels, bundle.CSV, nil } + +func getSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { + supported := sets.NewString() + for _, im := range csvInstallModes { + if im.Supported { + supported.Insert(string(im.Type)) + } + } + return supported +} From 876bc00f23d734a52a62ffc1afe24f3fa6ec3043 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 31 Aug 2020 16:21:19 -0400 Subject: [PATCH 02/24] Add Godoc & fix nil pointer --- internal/olm/operator/install_mode.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/olm/operator/install_mode.go b/internal/olm/operator/install_mode.go index 0be3cb3589..46c235efd3 100644 --- a/internal/olm/operator/install_mode.go +++ b/internal/olm/operator/install_mode.go @@ -31,6 +31,8 @@ type InstallMode struct { var _ flag.Value = &InstallMode{} +// Set is called when the --install-mode flag is passed to the CLI. It will +// configure the InstallMode based on the values passed in. func (i *InstallMode) Set(str string) error { split := strings.SplitN(str, "=", 2) i.InstallModeType = v1alpha1.InstallModeType(split[0]) @@ -40,10 +42,13 @@ func (i *InstallMode) Set(str string) error { i.TargetNamespaces = append(i.TargetNamespaces, strings.TrimSpace(ns)) } sort.Strings(i.TargetNamespaces) + } else { + i.TargetNamespaces = []string{} } return i.Validate() } +// IsEmpty returns true if the InstallModeType is empty. func (i InstallMode) IsEmpty() bool { return i.InstallModeType == "" } @@ -94,7 +99,7 @@ func (i InstallMode) Validate() error { // CheckCompatibility checks if an InstallMode is compatible with the operator's namespace and is supported by csv. func (i InstallMode) CheckCompatibility(csv *v1alpha1.ClusterServiceVersion, operatorNamespace string) error { if i.InstallModeType == v1alpha1.InstallModeTypeOwnNamespace { - if i.TargetNamespaces[0] != operatorNamespace { + if len(i.TargetNamespaces) > 0 && i.TargetNamespaces[0] != operatorNamespace { return fmt.Errorf("install mode %s must match operator namespace %q", i, operatorNamespace) } } From 5870e8708800d5bbd5ca8f3adb258760de2b9cb0 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 31 Aug 2020 16:22:49 -0400 Subject: [PATCH 03/24] Add ensureOperatorGroup --- .../operator/registry/operator_installer.go | 192 +++++++++++++----- 1 file changed, 145 insertions(+), 47 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 44c3868aba..577cb235df 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -17,14 +17,13 @@ package registry import ( "context" "fmt" - "reflect" - "sort" "time" v1 "github.com/operator-framework/api/pkg/operators/v1" "github.com/operator-framework/api/pkg/operators/v1alpha1" log "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,12 +33,13 @@ import ( ) type OperatorInstaller struct { - CatalogSourceName string - PackageName string - StartingCSV string - Channel string - InstallMode operator.InstallMode - CatalogCreator CatalogCreator + CatalogSourceName string + PackageName string + StartingCSV string + Channel string + InstallMode operator.InstallMode + CatalogCreator CatalogCreator + SupportedInstallModes sets.String cfg *operator.Configuration } @@ -55,16 +55,18 @@ func (o OperatorInstaller) InstallOperator(ctx context.Context) (*v1alpha1.Clust } log.Infof("Created CatalogSource: %s", cs.GetName()) - // TODO: OLM doesn't appear to propagate the "READY" connection status to the catalogsource in a timely manner - // even though its catalog-operator reports a connection almost immediately. This condition either needs - // to be propagated more quickly by OLM or we need to find a different resource to probe for readiness. - // wait for catalog source to be ready + // TODO: OLM doesn't appear to propagate the "READY" connection status to the + // catalogsource in a timely manner even though its catalog-operator reports + // a connection almost immediately. This condition either needs to be + // propagated more quickly by OLM or we need to find a different resource to + // probe for readiness. + // // if err := o.waitForCatalogSource(ctx, cs); err != nil { // return nil, err // } // Ensure Operator Group - if err = o.createOperatorGroup(ctx); err != nil { + if _, err = o.ensureOperatorGroup(ctx); err != nil { return nil, err } @@ -122,51 +124,116 @@ func (o OperatorInstaller) waitForCatalogSource(ctx context.Context, cs *v1alpha return nil } -// createOperatorGroup creates an OperatorGroup using package name if an OperatorGroup does not exist. -// If one exists in the desired namespace and it's target namespaces do not match the desired set, -// createOperatorGroup will return an error. -func (o OperatorInstaller) createOperatorGroup(ctx context.Context) error { - targetNamespaces := make([]string, len(o.InstallMode.TargetNamespaces), cap(o.InstallMode.TargetNamespaces)) - copy(targetNamespaces, o.InstallMode.TargetNamespaces) +func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.OperatorGroup, error) { // Check OperatorGroup existence, since we cannot create a second OperatorGroup in namespace. og, ogFound, err := o.getOperatorGroup(ctx) if err != nil { - return err + return nil, err } - // TODO: we may need to poll for status updates, since status.namespaces may not be updated immediately. - if ogFound { - // targetNamespaces will always be initialized, but the operator group's namespaces may not be - // (required for comparison). - if og.Status.Namespaces == nil { - og.Status.Namespaces = []string{} - } - // Simple check for OperatorGroup compatibility: if namespaces are not an exact match, - // the user must manage the resource themselves. - sort.Strings(og.Status.Namespaces) - sort.Strings(targetNamespaces) - if !reflect.DeepEqual(og.Status.Namespaces, targetNamespaces) { - msg := fmt.Sprintf("namespaces %+q do not match desired namespaces %+q", og.Status.Namespaces, targetNamespaces) - if og.GetName() == operator.SDKOperatorGroupName { - return fmt.Errorf("existing SDK-managed operator group's %s, "+ - "please clean up existing operators `operator-sdk cleanup` before running package %q", msg, o.PackageName) + fmt.Printf("XXX OperatorGroup found? %v\n", ogFound) + + supported := o.SupportedInstallModes + + if supported.Len() == 0 { + return nil, fmt.Errorf("operator %q is not installable: no supported install modes", o.StartingCSV) + } + + // --install-mode was given + if !o.InstallMode.IsEmpty() { + // TODO: probably remove multinamespace + if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace || + o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeMultiNamespace { + + targetNsSet := sets.NewString(o.InstallMode.TargetNamespaces...) + if !supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNsSet.Has(o.cfg.Namespace) { + return nil, fmt.Errorf("cannot watch namespace %q: operator %q does not support install mode %q", o.cfg.Namespace, o.StartingCSV, v1alpha1.InstallModeTypeOwnNamespace) } - return fmt.Errorf("existing operator group %q's %s, "+ - "please ensure it has the exact namespace set before running package %q", og.GetName(), msg, o.PackageName) } - log.Infof("Using existing operator group %q", og.GetName()) - } else { - // New SDK-managed OperatorGroup. - og = newSDKOperatorGroup(o.cfg.Namespace, - withTargetNamespaces(targetNamespaces...)) - if err = o.cfg.Client.Create(ctx, og); err != nil { - return fmt.Errorf("error creating OperatorGroup: %w", err) + if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace && + o.InstallMode.TargetNamespaces[0] == o.cfg.Namespace { + return nil, fmt.Errorf("use install mode %q to watch operator's namespace %q", v1alpha1.InstallModeTypeOwnNamespace, o.cfg.Namespace) } - log.Infof("Created OperatorGroup: %s", og.GetName()) + supported = supported.Intersection(sets.NewString(string(o.InstallMode.InstallModeType))) + if supported.Len() == 0 { + return nil, fmt.Errorf("operator %q does not support install mode %q", o.StartingCSV, o.InstallMode.InstallModeType) + } + + if og == nil { + targetNamespaces, err := o.getTargetNamespaces(supported) + if err != nil { + return nil, err + } + if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil { + return nil, fmt.Errorf("create operator group: %v", err) + } + log.Infof("operatorgroup %q created", og.Name) + } + // else if err := o.validateOperatorGroup(*og, supported); err != nil { + // return nil, err + // } } - return nil + + return og, nil } +func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNamespaces []string) (*v1.OperatorGroup, error) { + og := &v1.OperatorGroup{} + og.SetName(o.cfg.Namespace) + og.SetNamespace(o.cfg.Namespace) + og.Spec.TargetNamespaces = targetNamespaces + + if err := o.cfg.Client.Create(ctx, og); err != nil { + return nil, err + } + return og, nil +} + +// createOperatorGroup creates an OperatorGroup using package name if an OperatorGroup does not exist. +// If one exists in the desired namespace and it's target namespaces do not match the desired set, +// createOperatorGroup will return an error. +// func (o OperatorInstaller) createOperatorGroup(ctx context.Context) error { +// fmt.Printf("XXX targetnamespaces: %v\n", o.InstallMode.TargetNamespaces) +// targetNamespaces := make([]string, len(o.InstallMode.TargetNamespaces), cap(o.InstallMode.TargetNamespaces)) +// copy(targetNamespaces, o.InstallMode.TargetNamespaces) +// // Check OperatorGroup existence, since we cannot create a second OperatorGroup in namespace. +// og, ogFound, err := o.getOperatorGroup(ctx) +// if err != nil { +// return err +// } +// // TODO: we may need to poll for status updates, since status.namespaces may not be updated immediately. +// if ogFound { +// // targetNamespaces will always be initialized, but the operator group's namespaces may not be +// // (required for comparison). +// if og.Status.Namespaces == nil { +// og.Status.Namespaces = []string{} +// } +// // Simple check for OperatorGroup compatibility: if namespaces are not an exact match, +// // the user must manage the resource themselves. +// sort.Strings(og.Status.Namespaces) +// sort.Strings(targetNamespaces) +// if !reflect.DeepEqual(og.Status.Namespaces, targetNamespaces) { +// msg := fmt.Sprintf("namespaces %+q do not match desired namespaces %+q", og.Status.Namespaces, targetNamespaces) +// if og.GetName() == operator.SDKOperatorGroupName { +// return fmt.Errorf("existing SDK-managed operator group's %s, "+ +// "please clean up existing operators `operator-sdk cleanup` before running package %q", msg, o.PackageName) +// } +// return fmt.Errorf("existing operator group %q's %s, "+ +// "please ensure it has the exact namespace set before running package %q", og.GetName(), msg, o.PackageName) +// } +// log.Infof("Using existing operator group %q", og.GetName()) +// } else { +// // New SDK-managed OperatorGroup. +// og = newSDKOperatorGroup(o.cfg.Namespace, +// withTargetNamespaces(targetNamespaces...)) +// log.Info("Creating OperatorGroup") +// if err = o.cfg.Client.Create(ctx, og); err != nil { +// return fmt.Errorf("error creating OperatorGroup: %w", err) +// } +// } +// return nil +// } + // getOperatorGroup returns true if an OperatorGroup in the desired namespace was found. // If more than one operator group exists in namespace, this function will return an error // since CSVs in namespace will have an error status in that case. @@ -278,3 +345,34 @@ func (o OperatorInstaller) waitForInstallPlan(ctx context.Context, sub *v1alpha1 } return nil } + +func getSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { + supported := sets.NewString() + for _, im := range csvInstallModes { + if im.Supported { + supported.Insert(string(im.Type)) + } + } + return supported +} + +func (o *OperatorInstaller) getTargetNamespaces(supported sets.String) ([]string, error) { + switch { + case supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces)): + return nil, nil + case supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)): + return []string{o.cfg.Namespace}, nil + case supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace)): + if len(o.InstallMode.TargetNamespaces) != 1 { + return nil, fmt.Errorf("install mode %q requires explicit target namespace", v1alpha1.InstallModeTypeSingleNamespace) + } + return o.InstallMode.TargetNamespaces, nil + case supported.Has(string(v1alpha1.InstallModeTypeMultiNamespace)): + if len(o.InstallMode.TargetNamespaces) == 0 { + return nil, fmt.Errorf("install mode %q requires explicit target namespaces", v1alpha1.InstallModeTypeMultiNamespace) + } + return o.InstallMode.TargetNamespaces, nil + default: + return nil, fmt.Errorf("no supported install modes") + } +} From ee725b44f1811d79e30400bea3272d052aea1d88 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 4 Sep 2020 18:46:26 -0400 Subject: [PATCH 04/24] put getSupportedInstallModes at the bottom --- .../operator/registry/operator_installer.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 577cb235df..4ced80068e 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -346,16 +346,6 @@ func (o OperatorInstaller) waitForInstallPlan(ctx context.Context, sub *v1alpha1 return nil } -func getSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { - supported := sets.NewString() - for _, im := range csvInstallModes { - if im.Supported { - supported.Insert(string(im.Type)) - } - } - return supported -} - func (o *OperatorInstaller) getTargetNamespaces(supported sets.String) ([]string, error) { switch { case supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces)): @@ -376,3 +366,13 @@ func (o *OperatorInstaller) getTargetNamespaces(supported sets.String) ([]string return nil, fmt.Errorf("no supported install modes") } } + +func getSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { + supported := sets.NewString() + for _, im := range csvInstallModes { + if im.Supported { + supported.Insert(string(im.Type)) + } + } + return supported +} From cc606b06b045b24dc164e59e0120ef6bac90e73e Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 4 Sep 2020 22:43:10 -0400 Subject: [PATCH 05/24] Add validation and createOperatorGroup; Fix OperatorGroup creation --- .../operator/registry/operator_installer.go | 58 +++++++++++++------ 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 4ced80068e..dcb8a14b37 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -66,9 +66,12 @@ func (o OperatorInstaller) InstallOperator(ctx context.Context) (*v1alpha1.Clust // } // Ensure Operator Group + fmt.Println("XXX enter ensureOperatorGroup") if _, err = o.ensureOperatorGroup(ctx); err != nil { + fmt.Printf("XXX error from ensureOpreatorGroup: %v\n", err) return nil, err } + fmt.Println("XXX returned ensureOperatorGroup") var subscription *v1alpha1.Subscription // Create Subscription @@ -159,36 +162,60 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.Operato return nil, fmt.Errorf("operator %q does not support install mode %q", o.StartingCSV, o.InstallMode.InstallModeType) } - if og == nil { - targetNamespaces, err := o.getTargetNamespaces(supported) - if err != nil { - return nil, err - } - if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil { - return nil, fmt.Errorf("create operator group: %v", err) - } - log.Infof("operatorgroup %q created", og.Name) + } + if !ogFound { + targetNamespaces, err := o.getTargetNamespaces(supported) + if err != nil { + return nil, err + } + if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil { + return nil, fmt.Errorf("create operator group: %v", err) } - // else if err := o.validateOperatorGroup(*og, supported); err != nil { - // return nil, err - // } + log.Infof("operatorgroup %q created", og.Name) + } else if err := o.validateOperatorGroup(*og, supported); err != nil { + return nil, err } return og, nil } func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNamespaces []string) (*v1.OperatorGroup, error) { + fmt.Printf("XXX name %v - namespaces %v = namespaces: %v\n", o.cfg.Namespace, o.cfg.Namespace, targetNamespaces) og := &v1.OperatorGroup{} - og.SetName(o.cfg.Namespace) + og.SetName(operator.SDKOperatorGroupName) og.SetNamespace(o.cfg.Namespace) og.Spec.TargetNamespaces = targetNamespaces if err := o.cfg.Client.Create(ctx, og); err != nil { + fmt.Printf("XXX failed to create og: %v\n", err) return nil, err } return og, nil } +func (o *OperatorInstaller) validateOperatorGroup(og v1.OperatorGroup, supported sets.String) error { + ogTargetNs := sets.NewString(og.Spec.TargetNamespaces...) + imTargetNs := sets.NewString(o.InstallMode.TargetNamespaces...) + ownNamespaceNs := sets.NewString(o.cfg.Namespace) + + if supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) && len(og.Spec.TargetNamespaces) == 0 || + supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && ogTargetNs.Equal(ownNamespaceNs) || + supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) && ogTargetNs.Equal(imTargetNs) || + supported.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && ogTargetNs.Equal(imTargetNs) { + return nil + } + + switch o.InstallMode.InstallModeType { + case v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, + v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace: + return fmt.Errorf("existing operatorgroup %q is not compatible with install mode %q", og.Name, o.InstallMode) + case "": + return fmt.Errorf("existing operatorgroup %q is not compatible with any supported package install modes", og.Name) + } + + return fmt.Errorf("unknown install mode %q", o.InstallMode.InstallModeType) +} + // createOperatorGroup creates an OperatorGroup using package name if an OperatorGroup does not exist. // If one exists in the desired namespace and it's target namespaces do not match the desired set, // createOperatorGroup will return an error. @@ -357,11 +384,6 @@ func (o *OperatorInstaller) getTargetNamespaces(supported sets.String) ([]string return nil, fmt.Errorf("install mode %q requires explicit target namespace", v1alpha1.InstallModeTypeSingleNamespace) } return o.InstallMode.TargetNamespaces, nil - case supported.Has(string(v1alpha1.InstallModeTypeMultiNamespace)): - if len(o.InstallMode.TargetNamespaces) == 0 { - return nil, fmt.Errorf("install mode %q requires explicit target namespaces", v1alpha1.InstallModeTypeMultiNamespace) - } - return o.InstallMode.TargetNamespaces, nil default: return nil, fmt.Errorf("no supported install modes") } From 7bc92422e81d1df88070298a61e27c0196903ce6 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sat, 5 Sep 2020 00:02:16 -0400 Subject: [PATCH 06/24] Test getSupportedInstallModes --- .../registry/operator_installer_test.go | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 internal/olm/operator/registry/operator_installer_test.go diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go new file mode 100644 index 0000000000..a06c0eaf67 --- /dev/null +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -0,0 +1,186 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package registry + +import ( + // "context" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/operator-framework/api/pkg/operators/v1alpha1" + // v1 "github.com/operator-framework/api/pkg/operators/v1" + // "k8s.io/apimachinery/pkg/runtime" + // "sigs.k8s.io/controller-runtime/pkg/client" + // "sigs.k8s.io/controller-runtime/pkg/client/fake" + // + // "github.com/operator-framework/operator-sdk/internal/olm/operator" +) + +var _ = Describe("OperatorInstaller", func() { + Describe("InstallOperator", func() { + }) + + Describe("ensureOperatorGroup", func() { + }) + + Describe("getOperatorGroup", func() { + }) + + Describe("createSubscription", func() { + }) + + Describe("getTargetNamespaces", func() { + }) + + Describe("getSupportedInstallModes", func() { + It("should return empty set if empty installmodes", func() { + supported := getSupportedInstallModes([]v1alpha1.InstallMode{}) + Expect(supported.Len()).To(Equal(0)) + }) + It("should return empty set if no installmodes are supported", func() { + installModes := []v1alpha1.InstallMode{ + { + Type: v1alpha1.InstallModeTypeSingleNamespace, + Supported: false, + }, + { + Type: v1alpha1.InstallModeTypeOwnNamespace, + Supported: false, + }, + } + supported := getSupportedInstallModes(installModes) + Expect(supported.Len()).To(Equal(0)) + }) + It("should return set with supported installmodes", func() { + installModes := []v1alpha1.InstallMode{ + { + Type: v1alpha1.InstallModeTypeSingleNamespace, + Supported: true, + }, + { + Type: v1alpha1.InstallModeTypeOwnNamespace, + Supported: true, + }, + { + Type: v1alpha1.InstallModeTypeAllNamespaces, + Supported: false, + }, + } + supported := getSupportedInstallModes(installModes) + Expect(supported.Len()).To(Equal(2)) + Expect(supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace))).Should(BeTrue()) + Expect(supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace))).Should(BeTrue()) + Expect(supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces))).Should(BeFalse()) + }) + }) + + // Describe("createOperatorGroup", func() { + // var ( + // o *OperatorInstaller + // ctx context.Context + // err error + // + // packageName = "test-operator" + // namespace = "default" + // nonSDKOperatorGroupName = "my-og" + // ) + // + // BeforeEach(func() { + // sch := runtime.NewScheme() + // Expect(v1.AddToScheme(sch)).To(Succeed()) + // o = &OperatorInstaller{ + // PackageName: packageName, + // cfg: &operator.Configuration{ + // Scheme: sch, + // Namespace: namespace, + // Client: fake.NewFakeClientWithScheme(sch), + // }, + // } + // ctx = context.TODO() + // }) + // + // Context("with no existing OperatorGroup", func() { + // It("creates one successfully", func() { + // Expect(o.createOperatorGroup(ctx)).To(Succeed()) + // og, ogExists, err := o.getOperatorGroup(ctx) + // Expect(err).To(BeNil()) + // Expect(ogExists).To(BeTrue()) + // Expect(og.GetName()).To(Equal(operator.SDKOperatorGroupName)) + // }) + // }) + // + // Context("with an existing, valid OperatorGroup", func() { + // It("returns no error and the existing SDK OperatorGroup with no target namespaces is unchanged", func() { + // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace) + // Expect(o.createOperatorGroup(ctx)).To(Succeed()) + // og, ogExists, err := o.getOperatorGroup(ctx) + // Expect(err).To(BeNil()) + // Expect(ogExists).To(BeTrue()) + // Expect(og.GetName()).To(Equal(existingOG.GetName())) + // }) + // It("returns no error and the existing SDK OperatorGroup with the same set of target namespaces is unchanged", func() { + // targetNamespaces := []string{"foo", "bar"} + // o.InstallMode.TargetNamespaces = targetNamespaces + // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace, targetNamespaces...) + // Expect(o.createOperatorGroup(ctx)).To(Succeed()) + // og, ogExists, err := o.getOperatorGroup(ctx) + // Expect(err).To(BeNil()) + // Expect(ogExists).To(BeTrue()) + // Expect(og.GetName()).To(Equal(existingOG.GetName())) + // }) + // It("returns no error and the existing non-SDK OperatorGroup is unchanged", func() { + // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, nonSDKOperatorGroupName, namespace) + // Expect(o.createOperatorGroup(ctx)).To(Succeed()) + // og, ogExists, err := o.getOperatorGroup(ctx) + // Expect(err).To(BeNil()) + // Expect(ogExists).To(BeTrue()) + // Expect(og.GetName()).To(Equal(existingOG.GetName())) + // }) + // It("returns no error and the existing OperatorGroup in another namespace is unchanged", func() { + // otherNS := "my-ns" + // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, otherNS) + // Expect(o.createOperatorGroup(ctx)).To(Succeed()) + // og, ogExists, err := o.getOperatorGroup(ctx) + // Expect(err).To(BeNil()) + // Expect(ogExists).To(BeTrue()) + // Expect(og.GetName()).To(Equal(existingOG.GetName())) + // Expect(og.GetNamespace()).NotTo(Equal(existingOG.GetNamespace())) + // }) + // }) + // + // Context("with an existing, invalid OperatorGroup", func() { + // It("returns an error for an SDK OperatorGroup", func() { + // _ = createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace, "foo") + // err = o.createOperatorGroup(ctx) + // Expect(err.Error()).To(ContainSubstring(`existing SDK-managed operator group's namespaces ["foo"] do not match desired namespaces []`)) + // }) + // It("returns an error for a non-SDK OperatorGroup", func() { + // _ = createOperatorGroupHelper(ctx, o.cfg.Client, nonSDKOperatorGroupName, namespace, "foo") + // err = o.createOperatorGroup(ctx) + // Expect(err.Error()).To(ContainSubstring(`existing operator group "my-og"'s namespaces ["foo"] do not match desired namespaces []`)) + // }) + // }) + // }) + +}) + +// func createOperatorGroupHelper(ctx context.Context, c client.Client, name, namespace string, targetNamespaces ...string) (og v1.OperatorGroup) { +// og.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("OperatorGroup")) +// og.SetName(name) +// og.SetNamespace(namespace) +// og.Status.Namespaces = targetNamespaces +// ExpectWithOffset(1, c.Create(ctx, &og)).Should(Succeed()) +// return +// } From 56f4cf855e8325c6fb1b2c81ec5f93ae3e827388 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Tue, 8 Sep 2020 09:51:09 -0400 Subject: [PATCH 07/24] Add operator_installer_test * Add more tests * Test InstallModes * Remove operatorgroup_test in favor of operator_installer_test --- .../registry/operator_installer_test.go | 436 +++++++++++++----- .../operator/registry/operatorgroup_test.go | 128 ----- 2 files changed, 333 insertions(+), 231 deletions(-) delete mode 100644 internal/olm/operator/registry/operatorgroup_test.go diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index a06c0eaf67..ad84ce184e 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -17,31 +17,346 @@ package registry import ( // "context" + "context" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + v1 "github.com/operator-framework/api/pkg/operators/v1" "github.com/operator-framework/api/pkg/operators/v1alpha1" - // v1 "github.com/operator-framework/api/pkg/operators/v1" - // "k8s.io/apimachinery/pkg/runtime" - // "sigs.k8s.io/controller-runtime/pkg/client" - // "sigs.k8s.io/controller-runtime/pkg/client/fake" - // - // "github.com/operator-framework/operator-sdk/internal/olm/operator" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" + crclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/operator-framework/operator-sdk/internal/olm/operator" ) +var _ runtime.Object = (*v1.OperatorGroup)(nil) + var _ = Describe("OperatorInstaller", func() { Describe("InstallOperator", func() { + // TODO: fill this in once run bundle is done }) Describe("ensureOperatorGroup", func() { + var ( + oi OperatorInstaller + client crclient.Client + ) + BeforeEach(func() { + sch := runtime.NewScheme() + Expect(v1.AddToScheme(sch)).To(Succeed()) + client = fake.NewFakeClientWithScheme(sch) + oi = OperatorInstaller{ + cfg: &operator.Configuration{ + Scheme: sch, + Client: client, + Namespace: "testns", + }, + } + + // setup supported install modes + modes := []v1alpha1.InstallMode{ + { + Type: v1alpha1.InstallModeTypeSingleNamespace, + Supported: true, + }, + { + Type: v1alpha1.InstallModeTypeOwnNamespace, + Supported: true, + }, + { + Type: v1alpha1.InstallModeTypeAllNamespaces, + Supported: true, + }, + } + oi.SupportedInstallModes = getSupportedInstallModes(modes) + }) + It("should return an error when problems finding OperatorGroup", func() { + oi.cfg.Client = fake.NewFakeClient() + grp, err := oi.ensureOperatorGroup(context.TODO()) + Expect(grp).To(BeNil()) + Expect(err).To(HaveOccurred()) + }) + It("should return an error if there are no supported modes", func() { + oi.SupportedInstallModes = getSupportedInstallModes([]v1alpha1.InstallMode{}) + grp, err := oi.ensureOperatorGroup(context.TODO()) + Expect(grp).To(BeNil()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("no supported install modes")) + }) + // It("should return an error if OwnNamespace is used and target does not match", func() { + // oi.InstallMode.Set(string(v1alpha1.InstallModeTypeOwnNamespace)) + // oi.InstallMode.TargetNamespaces = []string{"notownns"} + // og, err := oi.ensureOperatorGroup(context.TODO()) + // Expect(og).To(BeNil()) + // Expect(err).ToNot(BeNil()) + // Expect(err.Error()).Should(ContainSubstring("use install mode \"OwnNamespace\"")) + // }) + Context("with no existing OperatorGroup", func() { + Context("given SingleNamespace", func() { + It("should create one with the given target namespaces", func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeSingleNamespace)) + oi.InstallMode.TargetNamespaces = []string{"anotherns"} + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(og).ToNot(BeNil()) + Expect(err).To(BeNil()) + Expect(og.Name).To(Equal("operator-sdk-og")) + Expect(og.Namespace).To(Equal("testns")) + Expect(og.Spec.TargetNamespaces).To(Equal([]string{"anotherns"})) + }) + It("should return an error if target matches operator ns", func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeSingleNamespace)) + oi.InstallMode.TargetNamespaces = []string{"testns"} + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).ToNot(BeNil()) + Expect(og).To(BeNil()) + Expect(err.Error()).Should(ContainSubstring("use install mode \"OwnNamespace\"")) + }) + }) + Context("given OwnNamespace", func() { + It("should create one with the given target namespaces", func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeOwnNamespace)) + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(og).ToNot(BeNil()) + Expect(err).To(BeNil()) + Expect(og.Name).To(Equal("operator-sdk-og")) + Expect(og.Namespace).To(Equal("testns")) + Expect(len(og.Spec.TargetNamespaces)).To(Equal(1)) + }) + }) + Context("given AllNamespaces", func() { + It("should create one with the given target namespaces", func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeAllNamespaces)) + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(og).ToNot(BeNil()) + Expect(err).To(BeNil()) + Expect(og.Name).To(Equal("operator-sdk-og")) + Expect(og.Namespace).To(Equal("testns")) + Expect(len(og.Spec.TargetNamespaces)).To(Equal(0)) + }) + }) + }) + Context("with an existing OperatorGroup", func() { + Context("given AllNamespaces", func() { + BeforeEach(func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeAllNamespaces)) + }) + It("should return nil for AllNamespaces with empty targets", func() { + // context, client, name, ns, targets + oog := createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns") + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + Expect(og.Name).To(Equal(oog.Name)) + Expect(og.Namespace).To(Equal(oog.Namespace)) + }) + It("should return an error for AllNamespaces when target is not empty", func() { + // context, client, name, ns, targets + _ = createOperatorGroupHelper(context.TODO(), client, "existing-og", + "testns", "incompatiblens") + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("is not compatible")) + Expect(og).To(BeNil()) + }) + }) + Context("given OwnNamespace", func() { + BeforeEach(func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeOwnNamespace)) + }) + It("should return nil for OwnNamespace when target matches operator", func() { + oog := createOperatorGroupHelper(context.TODO(), client, "existing-og", + "testns", "testns") + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + Expect(og.Name).To(Equal(oog.Name)) + Expect(og.Namespace).To(Equal(oog.Namespace)) + }) + It("should return an error for OwnNamespace when target does not match operator", func() { + _ = createOperatorGroupHelper(context.TODO(), client, "existing-og", + "testns", "incompatiblens") + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("is not compatible")) + Expect(og).To(BeNil()) + }) + }) + Context("given SingleNamespace", func() { + BeforeEach(func() { + _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeSingleNamespace)) + }) + It("should return nil for SingleNamespace when target differs from operator", func() { + oi.InstallMode.TargetNamespaces = []string{"anotherns"} + oog := createOperatorGroupHelper(context.TODO(), client, "existing-og", + "testns", "anotherns") + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + Expect(og.Name).To(Equal(oog.Name)) + Expect(og.Namespace).To(Equal(oog.Namespace)) + }) + It("should return an error for SingleNamespace when target matches operator", func() { + oi.InstallMode.TargetNamespaces = []string{"testns"} + _ = createOperatorGroupHelper(context.TODO(), client, "existing-og", + "testns", "testns") + og, err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).To(ContainSubstring("use install mode \"OwnNamespace\"")) + Expect(og).To(BeNil()) + }) + }) + }) + }) + + Describe("createOperatorGroup", func() { + var ( + oi OperatorInstaller + client crclient.Client + ) + BeforeEach(func() { + sch := runtime.NewScheme() + Expect(v1.AddToScheme(sch)).To(Succeed()) + client = fake.NewFakeClientWithScheme(sch) + oi = OperatorInstaller{ + cfg: &operator.Configuration{ + Scheme: sch, + Client: client, + Namespace: "testnamespace", + }, + } + }) + It("should return an error if OperatorGroup already exists", func() { + _ = createOperatorGroupHelper(context.TODO(), client, + operator.SDKOperatorGroupName, "testnamespace") + + og, err := oi.createOperatorGroup(context.TODO(), nil) + Expect(og).Should(BeNil()) + Expect(err).To(HaveOccurred()) + }) + It("should create the OperatorGroup", func() { + og, err := oi.createOperatorGroup(context.TODO(), nil) + Expect(og).ShouldNot(BeNil()) + Expect(og.Name).To(Equal(operator.SDKOperatorGroupName)) + Expect(og.Namespace).To(Equal("testnamespace")) + Expect(err).To(BeNil()) + }) }) Describe("getOperatorGroup", func() { + var ( + oi OperatorInstaller + client crclient.Client + ) + BeforeEach(func() { + sch := runtime.NewScheme() + Expect(v1.AddToScheme(sch)).To(Succeed()) + client = fake.NewFakeClientWithScheme(sch) + oi = OperatorInstaller{ + cfg: &operator.Configuration{ + Scheme: sch, + Client: client, + Namespace: "atestns", + }, + } + }) + It("should return an error if no OperatorGroups exist", func() { + oi.cfg.Client = fake.NewFakeClient() + grp, found, err := oi.getOperatorGroup(context.TODO()) + Expect(grp).To(BeNil()) + Expect(found).To(BeFalse()) + Expect(err).To(HaveOccurred()) + }) + It("should return nothing if namespace does not match", func() { + oi.cfg.Namespace = "fakens" + _ = createOperatorGroupHelper(context.TODO(), client, "og1", "atestns") + grp, found, err := oi.getOperatorGroup(context.TODO()) + Expect(grp).To(BeNil()) + Expect(found).To(BeFalse()) + Expect(err).Should(BeNil()) + }) + It("should return an error when more than OperatorGroup found", func() { + _ = createOperatorGroupHelper(context.TODO(), client, "og1", "atestns") + _ = createOperatorGroupHelper(context.TODO(), client, "og2", "atestns") + grp, found, err := oi.getOperatorGroup(context.TODO()) + Expect(grp).To(BeNil()) + Expect(found).To(BeTrue()) + Expect(err).Should(HaveOccurred()) + }) + It("should return list of OperatorGroups", func() { + og := createOperatorGroupHelper(context.TODO(), client, "og1", "atestns") + grp, found, err := oi.getOperatorGroup(context.TODO()) + Expect(grp).ShouldNot(BeNil()) + Expect(grp.Name).To(Equal(og.Name)) + Expect(grp.Namespace).To(Equal(og.Namespace)) + Expect(found).To(BeTrue()) + Expect(err).Should(BeNil()) + }) }) Describe("createSubscription", func() { }) Describe("getTargetNamespaces", func() { + var ( + oi OperatorInstaller + supported sets.String + ) + BeforeEach(func() { + oi = OperatorInstaller{ + cfg: &operator.Configuration{}, + } + supported = sets.NewString() + }) + It("should return an error when nothing is supported", func() { + target, err := oi.getTargetNamespaces(supported) + Expect(target).To(BeNil()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("no supported install modes")) + }) + It("should return nothing when AllNamespaces is supported", func() { + supported.Insert(string(v1alpha1.InstallModeTypeAllNamespaces)) + target, err := oi.getTargetNamespaces(supported) + Expect(target).To(BeNil()) + Expect(err).To(BeNil()) + }) + It("should return operator's namespace when OwnNamespace is supported", func() { + oi.cfg.Namespace = "test-ns" + supported.Insert(string(v1alpha1.InstallModeTypeOwnNamespace)) + target, err := oi.getTargetNamespaces(supported) + Expect(len(target)).To(Equal(1)) + Expect(target[0]).To(Equal("test-ns")) + Expect(err).To(BeNil()) + }) + It("should return an error when SingleNamespace has no target namespaces", func() { + + supported.Insert(string(v1alpha1.InstallModeTypeSingleNamespace)) + target, err := oi.getTargetNamespaces(supported) + Expect(target).To(BeNil()) + Expect(err).To(HaveOccurred()) + }) + It("should return an error when SingleNamespace has more than one target namespace", func() { + oi.InstallMode = operator.InstallMode{ + InstallModeType: v1alpha1.InstallModeTypeSingleNamespace, + TargetNamespaces: []string{"test-ns", "test2", "default"}, + } + + supported.Insert(string(v1alpha1.InstallModeTypeSingleNamespace)) + target, err := oi.getTargetNamespaces(supported) + Expect(target).To(BeNil()) + Expect(err).To(HaveOccurred()) + }) + It("should return configured namespace when SingleNamespace is passed in", func() { + + oi.InstallMode = operator.InstallMode{ + InstallModeType: v1alpha1.InstallModeTypeSingleNamespace, + TargetNamespaces: []string{"test-ns"}, + } + + supported.Insert(string(v1alpha1.InstallModeTypeSingleNamespace)) + target, err := oi.getTargetNamespaces(supported) + Expect(len(target)).To(Equal(1)) + Expect(target[0]).To(Equal("test-ns")) + Expect(err).To(BeNil()) + }) }) Describe("getSupportedInstallModes", func() { @@ -85,102 +400,17 @@ var _ = Describe("OperatorInstaller", func() { Expect(supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces))).Should(BeFalse()) }) }) - - // Describe("createOperatorGroup", func() { - // var ( - // o *OperatorInstaller - // ctx context.Context - // err error - // - // packageName = "test-operator" - // namespace = "default" - // nonSDKOperatorGroupName = "my-og" - // ) - // - // BeforeEach(func() { - // sch := runtime.NewScheme() - // Expect(v1.AddToScheme(sch)).To(Succeed()) - // o = &OperatorInstaller{ - // PackageName: packageName, - // cfg: &operator.Configuration{ - // Scheme: sch, - // Namespace: namespace, - // Client: fake.NewFakeClientWithScheme(sch), - // }, - // } - // ctx = context.TODO() - // }) - // - // Context("with no existing OperatorGroup", func() { - // It("creates one successfully", func() { - // Expect(o.createOperatorGroup(ctx)).To(Succeed()) - // og, ogExists, err := o.getOperatorGroup(ctx) - // Expect(err).To(BeNil()) - // Expect(ogExists).To(BeTrue()) - // Expect(og.GetName()).To(Equal(operator.SDKOperatorGroupName)) - // }) - // }) - // - // Context("with an existing, valid OperatorGroup", func() { - // It("returns no error and the existing SDK OperatorGroup with no target namespaces is unchanged", func() { - // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace) - // Expect(o.createOperatorGroup(ctx)).To(Succeed()) - // og, ogExists, err := o.getOperatorGroup(ctx) - // Expect(err).To(BeNil()) - // Expect(ogExists).To(BeTrue()) - // Expect(og.GetName()).To(Equal(existingOG.GetName())) - // }) - // It("returns no error and the existing SDK OperatorGroup with the same set of target namespaces is unchanged", func() { - // targetNamespaces := []string{"foo", "bar"} - // o.InstallMode.TargetNamespaces = targetNamespaces - // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace, targetNamespaces...) - // Expect(o.createOperatorGroup(ctx)).To(Succeed()) - // og, ogExists, err := o.getOperatorGroup(ctx) - // Expect(err).To(BeNil()) - // Expect(ogExists).To(BeTrue()) - // Expect(og.GetName()).To(Equal(existingOG.GetName())) - // }) - // It("returns no error and the existing non-SDK OperatorGroup is unchanged", func() { - // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, nonSDKOperatorGroupName, namespace) - // Expect(o.createOperatorGroup(ctx)).To(Succeed()) - // og, ogExists, err := o.getOperatorGroup(ctx) - // Expect(err).To(BeNil()) - // Expect(ogExists).To(BeTrue()) - // Expect(og.GetName()).To(Equal(existingOG.GetName())) - // }) - // It("returns no error and the existing OperatorGroup in another namespace is unchanged", func() { - // otherNS := "my-ns" - // existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, otherNS) - // Expect(o.createOperatorGroup(ctx)).To(Succeed()) - // og, ogExists, err := o.getOperatorGroup(ctx) - // Expect(err).To(BeNil()) - // Expect(ogExists).To(BeTrue()) - // Expect(og.GetName()).To(Equal(existingOG.GetName())) - // Expect(og.GetNamespace()).NotTo(Equal(existingOG.GetNamespace())) - // }) - // }) - // - // Context("with an existing, invalid OperatorGroup", func() { - // It("returns an error for an SDK OperatorGroup", func() { - // _ = createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace, "foo") - // err = o.createOperatorGroup(ctx) - // Expect(err.Error()).To(ContainSubstring(`existing SDK-managed operator group's namespaces ["foo"] do not match desired namespaces []`)) - // }) - // It("returns an error for a non-SDK OperatorGroup", func() { - // _ = createOperatorGroupHelper(ctx, o.cfg.Client, nonSDKOperatorGroupName, namespace, "foo") - // err = o.createOperatorGroup(ctx) - // Expect(err.Error()).To(ContainSubstring(`existing operator group "my-og"'s namespaces ["foo"] do not match desired namespaces []`)) - // }) - // }) - // }) - }) -// func createOperatorGroupHelper(ctx context.Context, c client.Client, name, namespace string, targetNamespaces ...string) (og v1.OperatorGroup) { -// og.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("OperatorGroup")) -// og.SetName(name) -// og.SetNamespace(namespace) -// og.Status.Namespaces = targetNamespaces -// ExpectWithOffset(1, c.Create(ctx, &og)).Should(Succeed()) -// return -// } +func createOperatorGroupHelper(ctx context.Context, c crclient.Client, name, namespace string, targetNamespaces ...string) v1.OperatorGroup { + og := v1.OperatorGroup{} + og.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("OperatorGroup")) + og.SetName(name) + og.SetNamespace(namespace) + og.Spec.TargetNamespaces = targetNamespaces + og.Status.Namespaces = targetNamespaces + if c != nil { + ExpectWithOffset(1, c.Create(ctx, &og)).Should(Succeed()) + } + return og +} diff --git a/internal/olm/operator/registry/operatorgroup_test.go b/internal/olm/operator/registry/operatorgroup_test.go deleted file mode 100644 index 1a7aa8403d..0000000000 --- a/internal/olm/operator/registry/operatorgroup_test.go +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright 2020 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package registry - -import ( - "context" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - v1 "github.com/operator-framework/api/pkg/operators/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - "github.com/operator-framework/operator-sdk/internal/olm/operator" -) - -var _ = Describe("Tenancy", func() { - Describe("createOperatorGroup", func() { - var ( - o *OperatorInstaller - ctx context.Context - err error - - packageName = "test-operator" - namespace = "default" - nonSDKOperatorGroupName = "my-og" - ) - - BeforeEach(func() { - sch := runtime.NewScheme() - Expect(v1.AddToScheme(sch)).To(Succeed()) - o = &OperatorInstaller{ - PackageName: packageName, - cfg: &operator.Configuration{ - Scheme: sch, - Namespace: namespace, - Client: fake.NewFakeClientWithScheme(sch), - }, - } - ctx = context.TODO() - }) - - Context("with no existing OperatorGroup", func() { - It("creates one successfully", func() { - Expect(o.createOperatorGroup(ctx)).To(Succeed()) - og, ogExists, err := o.getOperatorGroup(ctx) - Expect(err).To(BeNil()) - Expect(ogExists).To(BeTrue()) - Expect(og.GetName()).To(Equal(operator.SDKOperatorGroupName)) - }) - }) - - Context("with an existing, valid OperatorGroup", func() { - It("returns no error and the existing SDK OperatorGroup with no target namespaces is unchanged", func() { - existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace) - Expect(o.createOperatorGroup(ctx)).To(Succeed()) - og, ogExists, err := o.getOperatorGroup(ctx) - Expect(err).To(BeNil()) - Expect(ogExists).To(BeTrue()) - Expect(og.GetName()).To(Equal(existingOG.GetName())) - }) - It("returns no error and the existing SDK OperatorGroup with the same set of target namespaces is unchanged", func() { - targetNamespaces := []string{"foo", "bar"} - o.InstallMode.TargetNamespaces = targetNamespaces - existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace, targetNamespaces...) - Expect(o.createOperatorGroup(ctx)).To(Succeed()) - og, ogExists, err := o.getOperatorGroup(ctx) - Expect(err).To(BeNil()) - Expect(ogExists).To(BeTrue()) - Expect(og.GetName()).To(Equal(existingOG.GetName())) - }) - It("returns no error and the existing non-SDK OperatorGroup is unchanged", func() { - existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, nonSDKOperatorGroupName, namespace) - Expect(o.createOperatorGroup(ctx)).To(Succeed()) - og, ogExists, err := o.getOperatorGroup(ctx) - Expect(err).To(BeNil()) - Expect(ogExists).To(BeTrue()) - Expect(og.GetName()).To(Equal(existingOG.GetName())) - }) - It("returns no error and the existing OperatorGroup in another namespace is unchanged", func() { - otherNS := "my-ns" - existingOG := createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, otherNS) - Expect(o.createOperatorGroup(ctx)).To(Succeed()) - og, ogExists, err := o.getOperatorGroup(ctx) - Expect(err).To(BeNil()) - Expect(ogExists).To(BeTrue()) - Expect(og.GetName()).To(Equal(existingOG.GetName())) - Expect(og.GetNamespace()).NotTo(Equal(existingOG.GetNamespace())) - }) - }) - - Context("with an existing, invalid OperatorGroup", func() { - It("returns an error for an SDK OperatorGroup", func() { - _ = createOperatorGroupHelper(ctx, o.cfg.Client, operator.SDKOperatorGroupName, namespace, "foo") - err = o.createOperatorGroup(ctx) - Expect(err.Error()).To(ContainSubstring(`existing SDK-managed operator group's namespaces ["foo"] do not match desired namespaces []`)) - }) - It("returns an error for a non-SDK OperatorGroup", func() { - _ = createOperatorGroupHelper(ctx, o.cfg.Client, nonSDKOperatorGroupName, namespace, "foo") - err = o.createOperatorGroup(ctx) - Expect(err.Error()).To(ContainSubstring(`existing operator group "my-og"'s namespaces ["foo"] do not match desired namespaces []`)) - }) - }) - }) - -}) - -func createOperatorGroupHelper(ctx context.Context, c client.Client, name, namespace string, targetNamespaces ...string) (og v1.OperatorGroup) { - og.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("OperatorGroup")) - og.SetName(name) - og.SetNamespace(namespace) - og.Status.Namespaces = targetNamespaces - ExpectWithOffset(1, c.Create(ctx, &og)).Should(Succeed()) - return -} From 53b7b0d7d3f9ccd2aefb9ec6f78cbd41e9f381ae Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Tue, 8 Sep 2020 18:43:15 -0400 Subject: [PATCH 08/24] Remove MultiNamespace --- internal/olm/operator/registry/operator_installer.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index dcb8a14b37..0671b7e25a 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -144,9 +144,7 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.Operato // --install-mode was given if !o.InstallMode.IsEmpty() { // TODO: probably remove multinamespace - if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace || - o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeMultiNamespace { - + if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace { targetNsSet := sets.NewString(o.InstallMode.TargetNamespaces...) if !supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNsSet.Has(o.cfg.Namespace) { return nil, fmt.Errorf("cannot watch namespace %q: operator %q does not support install mode %q", o.cfg.Namespace, o.StartingCSV, v1alpha1.InstallModeTypeOwnNamespace) @@ -200,14 +198,13 @@ func (o *OperatorInstaller) validateOperatorGroup(og v1.OperatorGroup, supported if supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) && len(og.Spec.TargetNamespaces) == 0 || supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && ogTargetNs.Equal(ownNamespaceNs) || - supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) && ogTargetNs.Equal(imTargetNs) || - supported.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && ogTargetNs.Equal(imTargetNs) { + supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) && ogTargetNs.Equal(imTargetNs) { return nil } switch o.InstallMode.InstallModeType { case v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, - v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeMultiNamespace: + v1alpha1.InstallModeTypeSingleNamespace: return fmt.Errorf("existing operatorgroup %q is not compatible with install mode %q", og.Name, o.InstallMode) case "": return fmt.Errorf("existing operatorgroup %q is not compatible with any supported package install modes", og.Name) From 321df13759db3e65fbe8cc676b983caf3813bbd3 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Tue, 8 Sep 2020 23:50:25 -0400 Subject: [PATCH 09/24] Remove debug logging; commented out code; use newSDKOperatorGroup --- .../operator/registry/operator_installer.go | 57 +------------------ 1 file changed, 1 insertion(+), 56 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 0671b7e25a..3dfc0bfd68 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -66,12 +66,9 @@ func (o OperatorInstaller) InstallOperator(ctx context.Context) (*v1alpha1.Clust // } // Ensure Operator Group - fmt.Println("XXX enter ensureOperatorGroup") if _, err = o.ensureOperatorGroup(ctx); err != nil { - fmt.Printf("XXX error from ensureOpreatorGroup: %v\n", err) return nil, err } - fmt.Println("XXX returned ensureOperatorGroup") var subscription *v1alpha1.Subscription // Create Subscription @@ -133,7 +130,6 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.Operato if err != nil { return nil, err } - fmt.Printf("XXX OperatorGroup found? %v\n", ogFound) supported := o.SupportedInstallModes @@ -178,14 +174,8 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.Operato } func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNamespaces []string) (*v1.OperatorGroup, error) { - fmt.Printf("XXX name %v - namespaces %v = namespaces: %v\n", o.cfg.Namespace, o.cfg.Namespace, targetNamespaces) - og := &v1.OperatorGroup{} - og.SetName(operator.SDKOperatorGroupName) - og.SetNamespace(o.cfg.Namespace) - og.Spec.TargetNamespaces = targetNamespaces - + og := newSDKOperatorGroup(o.cfg.Namespace, withTargetNamespaces(targetNamespaces...)) if err := o.cfg.Client.Create(ctx, og); err != nil { - fmt.Printf("XXX failed to create og: %v\n", err) return nil, err } return og, nil @@ -213,51 +203,6 @@ func (o *OperatorInstaller) validateOperatorGroup(og v1.OperatorGroup, supported return fmt.Errorf("unknown install mode %q", o.InstallMode.InstallModeType) } -// createOperatorGroup creates an OperatorGroup using package name if an OperatorGroup does not exist. -// If one exists in the desired namespace and it's target namespaces do not match the desired set, -// createOperatorGroup will return an error. -// func (o OperatorInstaller) createOperatorGroup(ctx context.Context) error { -// fmt.Printf("XXX targetnamespaces: %v\n", o.InstallMode.TargetNamespaces) -// targetNamespaces := make([]string, len(o.InstallMode.TargetNamespaces), cap(o.InstallMode.TargetNamespaces)) -// copy(targetNamespaces, o.InstallMode.TargetNamespaces) -// // Check OperatorGroup existence, since we cannot create a second OperatorGroup in namespace. -// og, ogFound, err := o.getOperatorGroup(ctx) -// if err != nil { -// return err -// } -// // TODO: we may need to poll for status updates, since status.namespaces may not be updated immediately. -// if ogFound { -// // targetNamespaces will always be initialized, but the operator group's namespaces may not be -// // (required for comparison). -// if og.Status.Namespaces == nil { -// og.Status.Namespaces = []string{} -// } -// // Simple check for OperatorGroup compatibility: if namespaces are not an exact match, -// // the user must manage the resource themselves. -// sort.Strings(og.Status.Namespaces) -// sort.Strings(targetNamespaces) -// if !reflect.DeepEqual(og.Status.Namespaces, targetNamespaces) { -// msg := fmt.Sprintf("namespaces %+q do not match desired namespaces %+q", og.Status.Namespaces, targetNamespaces) -// if og.GetName() == operator.SDKOperatorGroupName { -// return fmt.Errorf("existing SDK-managed operator group's %s, "+ -// "please clean up existing operators `operator-sdk cleanup` before running package %q", msg, o.PackageName) -// } -// return fmt.Errorf("existing operator group %q's %s, "+ -// "please ensure it has the exact namespace set before running package %q", og.GetName(), msg, o.PackageName) -// } -// log.Infof("Using existing operator group %q", og.GetName()) -// } else { -// // New SDK-managed OperatorGroup. -// og = newSDKOperatorGroup(o.cfg.Namespace, -// withTargetNamespaces(targetNamespaces...)) -// log.Info("Creating OperatorGroup") -// if err = o.cfg.Client.Create(ctx, og); err != nil { -// return fmt.Errorf("error creating OperatorGroup: %w", err) -// } -// } -// return nil -// } - // getOperatorGroup returns true if an OperatorGroup in the desired namespace was found. // If more than one operator group exists in namespace, this function will return an error // since CSVs in namespace will have an error status in that case. From eb26a89d2a597bfd4da7fbd067703477fba6a639 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 10 Sep 2020 11:20:10 -0400 Subject: [PATCH 10/24] Fix log message --- internal/olm/operator/registry/operator_installer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 3dfc0bfd68..23f62dac3b 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -165,7 +165,7 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.Operato if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil { return nil, fmt.Errorf("create operator group: %v", err) } - log.Infof("operatorgroup %q created", og.Name) + log.Infof("OperatorGroup %q created", og.Name) } else if err := o.validateOperatorGroup(*og, supported); err != nil { return nil, err } From c51f8e98d4e501a8842de9e8772eb8919ae1f8b9 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Fri, 11 Sep 2020 15:06:36 -0400 Subject: [PATCH 11/24] packagemanifest should use same precedence logic as bundle --- internal/olm/operator/packagemanifests/install.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/olm/operator/packagemanifests/install.go b/internal/olm/operator/packagemanifests/install.go index 259f9a2ea0..360b65064f 100644 --- a/internal/olm/operator/packagemanifests/install.go +++ b/internal/olm/operator/packagemanifests/install.go @@ -69,9 +69,6 @@ func (i *Install) setup() error { return err } - if i.InstallMode.IsEmpty() { - i.InstallMode.InstallModeType = v1alpha1.InstallModeTypeAllNamespaces - } if err := i.InstallMode.CheckCompatibility(bundle.CSV, i.cfg.Namespace); err != nil { return err } From d113e2f56192c786b05ede7172ab88fac756decb Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sat, 12 Sep 2020 16:11:48 -0400 Subject: [PATCH 12/24] Do not return error from validateOperatorGroup --- internal/olm/operator/registry/operator_installer.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 23f62dac3b..aa7156971e 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -322,9 +322,6 @@ func (o *OperatorInstaller) getTargetNamespaces(supported sets.String) ([]string case supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)): return []string{o.cfg.Namespace}, nil case supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace)): - if len(o.InstallMode.TargetNamespaces) != 1 { - return nil, fmt.Errorf("install mode %q requires explicit target namespace", v1alpha1.InstallModeTypeSingleNamespace) - } return o.InstallMode.TargetNamespaces, nil default: return nil, fmt.Errorf("no supported install modes") From 6d2ebd629717fc80314a7e721691c1c9897137be Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sat, 12 Sep 2020 23:39:20 -0400 Subject: [PATCH 13/24] Remove outdated tests --- .../registry/operator_installer_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index ad84ce184e..3e9ffc3dd4 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -326,24 +326,6 @@ var _ = Describe("OperatorInstaller", func() { Expect(target[0]).To(Equal("test-ns")) Expect(err).To(BeNil()) }) - It("should return an error when SingleNamespace has no target namespaces", func() { - - supported.Insert(string(v1alpha1.InstallModeTypeSingleNamespace)) - target, err := oi.getTargetNamespaces(supported) - Expect(target).To(BeNil()) - Expect(err).To(HaveOccurred()) - }) - It("should return an error when SingleNamespace has more than one target namespace", func() { - oi.InstallMode = operator.InstallMode{ - InstallModeType: v1alpha1.InstallModeTypeSingleNamespace, - TargetNamespaces: []string{"test-ns", "test2", "default"}, - } - - supported.Insert(string(v1alpha1.InstallModeTypeSingleNamespace)) - target, err := oi.getTargetNamespaces(supported) - Expect(target).To(BeNil()) - Expect(err).To(HaveOccurred()) - }) It("should return configured namespace when SingleNamespace is passed in", func() { oi.InstallMode = operator.InstallMode{ From 83e787c3b30e09037d46001e074857abdaf8d399 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sat, 12 Sep 2020 23:55:40 -0400 Subject: [PATCH 14/24] Removed unused code --- internal/olm/operator/registry/operator_installer_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index 3e9ffc3dd4..8a761cc112 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -31,8 +31,6 @@ import ( "github.com/operator-framework/operator-sdk/internal/olm/operator" ) -var _ runtime.Object = (*v1.OperatorGroup)(nil) - var _ = Describe("OperatorInstaller", func() { Describe("InstallOperator", func() { // TODO: fill this in once run bundle is done From 4d702e41400fbad02e23370c4adca484258ac438 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sun, 13 Sep 2020 17:23:37 -0400 Subject: [PATCH 15/24] Move getSupportedInstallModes to InstallMode and export it --- internal/olm/operator/bundle/install.go | 13 +--- internal/olm/operator/install_mode.go | 13 ++++ internal/olm/operator/install_mode_test.go | 66 +++++++++++++++++++ internal/olm/operator/operator_suite_test.go | 27 ++++++++ .../operator/registry/operator_installer.go | 10 --- .../registry/operator_installer_test.go | 46 +------------ 6 files changed, 109 insertions(+), 66 deletions(-) create mode 100644 internal/olm/operator/install_mode_test.go create mode 100644 internal/olm/operator/operator_suite_test.go diff --git a/internal/olm/operator/bundle/install.go b/internal/olm/operator/bundle/install.go index b55bdcb132..5cf039c57c 100644 --- a/internal/olm/operator/bundle/install.go +++ b/internal/olm/operator/bundle/install.go @@ -24,7 +24,6 @@ import ( apimanifests "github.com/operator-framework/api/pkg/manifests" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/spf13/pflag" - "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/operator-sdk/internal/olm/operator" "github.com/operator-framework/operator-sdk/internal/olm/operator/registry" @@ -79,7 +78,7 @@ func (i *Install) setup(ctx context.Context) error { i.OperatorInstaller.PackageName = labels["operators.operatorframework.io.bundle.package.v1"] i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = csv.Name - i.OperatorInstaller.SupportedInstallModes = getSupportedInstallModes(csv.Spec.InstallModes) + i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(csv.Spec.InstallModes) i.OperatorInstaller.Channel = strings.Split(labels["operators.operatorframework.io.bundle.channels.v1"], ",")[0] i.IndexImageCatalogCreator.BundleImage = i.BundleImage i.IndexImageCatalogCreator.PackageName = i.OperatorInstaller.PackageName @@ -118,13 +117,3 @@ func loadBundle(ctx context.Context, bundleImage string) (registryutil.Labels, * return labels, bundle.CSV, nil } - -func getSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { - supported := sets.NewString() - for _, im := range csvInstallModes { - if im.Supported { - supported.Insert(string(im.Type)) - } - } - return supported -} diff --git a/internal/olm/operator/install_mode.go b/internal/olm/operator/install_mode.go index 46c235efd3..e2a6694473 100644 --- a/internal/olm/operator/install_mode.go +++ b/internal/olm/operator/install_mode.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/operator-framework/api/pkg/operators/v1alpha1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" ) @@ -110,3 +111,15 @@ func (i InstallMode) CheckCompatibility(csv *v1alpha1.ClusterServiceVersion, ope } return nil } + +// GetSupportedInstallModes returns the given slice of InstallModes as a +// String set. +func GetSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { + supported := sets.NewString() + for _, im := range csvInstallModes { + if im.Supported { + supported.Insert(string(im.Type)) + } + } + return supported +} diff --git a/internal/olm/operator/install_mode_test.go b/internal/olm/operator/install_mode_test.go new file mode 100644 index 0000000000..acd490a3a7 --- /dev/null +++ b/internal/olm/operator/install_mode_test.go @@ -0,0 +1,66 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package operator + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/operator-framework/api/pkg/operators/v1alpha1" +) + +var _ = Describe("InstallMode", func() { + + Describe("GetSupportedInstallModes", func() { + It("should return empty set if empty installmodes", func() { + supported := GetSupportedInstallModes([]v1alpha1.InstallMode{}) + Expect(supported.Len()).To(Equal(0)) + }) + It("should return empty set if no installmodes are supported", func() { + installModes := []v1alpha1.InstallMode{ + { + Type: v1alpha1.InstallModeTypeSingleNamespace, + Supported: false, + }, + { + Type: v1alpha1.InstallModeTypeOwnNamespace, + Supported: false, + }, + } + supported := GetSupportedInstallModes(installModes) + Expect(supported.Len()).To(Equal(0)) + }) + It("should return set with supported installmodes", func() { + installModes := []v1alpha1.InstallMode{ + { + Type: v1alpha1.InstallModeTypeSingleNamespace, + Supported: true, + }, + { + Type: v1alpha1.InstallModeTypeOwnNamespace, + Supported: true, + }, + { + Type: v1alpha1.InstallModeTypeAllNamespaces, + Supported: false, + }, + } + supported := GetSupportedInstallModes(installModes) + Expect(supported.Len()).To(Equal(2)) + Expect(supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace))).Should(BeTrue()) + Expect(supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace))).Should(BeTrue()) + Expect(supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces))).Should(BeFalse()) + }) + }) +}) diff --git a/internal/olm/operator/operator_suite_test.go b/internal/olm/operator/operator_suite_test.go new file mode 100644 index 0000000000..79f492273b --- /dev/null +++ b/internal/olm/operator/operator_suite_test.go @@ -0,0 +1,27 @@ +// Copyright 2020 The Operator-SDK Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package operator + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestOperator(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Operator Suite") +} diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index aa7156971e..09a23a38a6 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -327,13 +327,3 @@ func (o *OperatorInstaller) getTargetNamespaces(supported sets.String) ([]string return nil, fmt.Errorf("no supported install modes") } } - -func getSupportedInstallModes(csvInstallModes []v1alpha1.InstallMode) sets.String { - supported := sets.NewString() - for _, im := range csvInstallModes { - if im.Supported { - supported.Insert(string(im.Type)) - } - } - return supported -} diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index 8a761cc112..56c1dc48fe 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -68,7 +68,7 @@ var _ = Describe("OperatorInstaller", func() { Supported: true, }, } - oi.SupportedInstallModes = getSupportedInstallModes(modes) + oi.SupportedInstallModes = operator.GetSupportedInstallModes(modes) }) It("should return an error when problems finding OperatorGroup", func() { oi.cfg.Client = fake.NewFakeClient() @@ -77,7 +77,7 @@ var _ = Describe("OperatorInstaller", func() { Expect(err).To(HaveOccurred()) }) It("should return an error if there are no supported modes", func() { - oi.SupportedInstallModes = getSupportedInstallModes([]v1alpha1.InstallMode{}) + oi.SupportedInstallModes = operator.GetSupportedInstallModes([]v1alpha1.InstallMode{}) grp, err := oi.ensureOperatorGroup(context.TODO()) Expect(grp).To(BeNil()) Expect(err).To(HaveOccurred()) @@ -338,48 +338,6 @@ var _ = Describe("OperatorInstaller", func() { Expect(err).To(BeNil()) }) }) - - Describe("getSupportedInstallModes", func() { - It("should return empty set if empty installmodes", func() { - supported := getSupportedInstallModes([]v1alpha1.InstallMode{}) - Expect(supported.Len()).To(Equal(0)) - }) - It("should return empty set if no installmodes are supported", func() { - installModes := []v1alpha1.InstallMode{ - { - Type: v1alpha1.InstallModeTypeSingleNamespace, - Supported: false, - }, - { - Type: v1alpha1.InstallModeTypeOwnNamespace, - Supported: false, - }, - } - supported := getSupportedInstallModes(installModes) - Expect(supported.Len()).To(Equal(0)) - }) - It("should return set with supported installmodes", func() { - installModes := []v1alpha1.InstallMode{ - { - Type: v1alpha1.InstallModeTypeSingleNamespace, - Supported: true, - }, - { - Type: v1alpha1.InstallModeTypeOwnNamespace, - Supported: true, - }, - { - Type: v1alpha1.InstallModeTypeAllNamespaces, - Supported: false, - }, - } - supported := getSupportedInstallModes(installModes) - Expect(supported.Len()).To(Equal(2)) - Expect(supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace))).Should(BeTrue()) - Expect(supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace))).Should(BeTrue()) - Expect(supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces))).Should(BeFalse()) - }) - }) }) func createOperatorGroupHelper(ctx context.Context, c crclient.Client, name, namespace string, targetNamespaces ...string) v1.OperatorGroup { From 30f345118532a77f4c525c4a0d36ea4e8ed6a8f2 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Sun, 13 Sep 2020 17:41:58 -0400 Subject: [PATCH 16/24] Do not return OperatorGroup from ensureOpratorGroup --- .../operator/registry/operator_installer.go | 22 +++---- .../registry/operator_installer_test.go | 60 ++++++++++++------- 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 09a23a38a6..700168ef04 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -66,7 +66,7 @@ func (o OperatorInstaller) InstallOperator(ctx context.Context) (*v1alpha1.Clust // } // Ensure Operator Group - if _, err = o.ensureOperatorGroup(ctx); err != nil { + if err = o.ensureOperatorGroup(ctx); err != nil { return nil, err } @@ -124,17 +124,17 @@ func (o OperatorInstaller) waitForCatalogSource(ctx context.Context, cs *v1alpha return nil } -func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.OperatorGroup, error) { +func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) error { // Check OperatorGroup existence, since we cannot create a second OperatorGroup in namespace. og, ogFound, err := o.getOperatorGroup(ctx) if err != nil { - return nil, err + return err } supported := o.SupportedInstallModes if supported.Len() == 0 { - return nil, fmt.Errorf("operator %q is not installable: no supported install modes", o.StartingCSV) + return fmt.Errorf("operator %q is not installable: no supported install modes", o.StartingCSV) } // --install-mode was given @@ -143,34 +143,34 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) (*v1.Operato if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace { targetNsSet := sets.NewString(o.InstallMode.TargetNamespaces...) if !supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNsSet.Has(o.cfg.Namespace) { - return nil, fmt.Errorf("cannot watch namespace %q: operator %q does not support install mode %q", o.cfg.Namespace, o.StartingCSV, v1alpha1.InstallModeTypeOwnNamespace) + return fmt.Errorf("cannot watch namespace %q: operator %q does not support install mode %q", o.cfg.Namespace, o.StartingCSV, v1alpha1.InstallModeTypeOwnNamespace) } } if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace && o.InstallMode.TargetNamespaces[0] == o.cfg.Namespace { - return nil, fmt.Errorf("use install mode %q to watch operator's namespace %q", v1alpha1.InstallModeTypeOwnNamespace, o.cfg.Namespace) + return fmt.Errorf("use install mode %q to watch operator's namespace %q", v1alpha1.InstallModeTypeOwnNamespace, o.cfg.Namespace) } supported = supported.Intersection(sets.NewString(string(o.InstallMode.InstallModeType))) if supported.Len() == 0 { - return nil, fmt.Errorf("operator %q does not support install mode %q", o.StartingCSV, o.InstallMode.InstallModeType) + return fmt.Errorf("operator %q does not support install mode %q", o.StartingCSV, o.InstallMode.InstallModeType) } } if !ogFound { targetNamespaces, err := o.getTargetNamespaces(supported) if err != nil { - return nil, err + return err } if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil { - return nil, fmt.Errorf("create operator group: %v", err) + return fmt.Errorf("create operator group: %v", err) } log.Infof("OperatorGroup %q created", og.Name) } else if err := o.validateOperatorGroup(*og, supported); err != nil { - return nil, err + return err } - return og, nil + return nil } func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNamespaces []string) (*v1.OperatorGroup, error) { diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index 56c1dc48fe..5d972f53f8 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -72,14 +72,12 @@ var _ = Describe("OperatorInstaller", func() { }) It("should return an error when problems finding OperatorGroup", func() { oi.cfg.Client = fake.NewFakeClient() - grp, err := oi.ensureOperatorGroup(context.TODO()) - Expect(grp).To(BeNil()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).To(HaveOccurred()) }) It("should return an error if there are no supported modes", func() { oi.SupportedInstallModes = operator.GetSupportedInstallModes([]v1alpha1.InstallMode{}) - grp, err := oi.ensureOperatorGroup(context.TODO()) - Expect(grp).To(BeNil()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring("no supported install modes")) }) @@ -96,9 +94,13 @@ var _ = Describe("OperatorInstaller", func() { It("should create one with the given target namespaces", func() { _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeSingleNamespace)) oi.InstallMode.TargetNamespaces = []string{"anotherns"} - og, err := oi.ensureOperatorGroup(context.TODO()) - Expect(og).ToNot(BeNil()) + err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + + og, found, err := oi.getOperatorGroup(context.TODO()) Expect(err).To(BeNil()) + Expect(found).To(BeTrue()) + Expect(og).ToNot(BeNil()) Expect(og.Name).To(Equal("operator-sdk-og")) Expect(og.Namespace).To(Equal("testns")) Expect(og.Spec.TargetNamespaces).To(Equal([]string{"anotherns"})) @@ -106,18 +108,21 @@ var _ = Describe("OperatorInstaller", func() { It("should return an error if target matches operator ns", func() { _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeSingleNamespace)) oi.InstallMode.TargetNamespaces = []string{"testns"} - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).ToNot(BeNil()) - Expect(og).To(BeNil()) Expect(err.Error()).Should(ContainSubstring("use install mode \"OwnNamespace\"")) }) }) Context("given OwnNamespace", func() { It("should create one with the given target namespaces", func() { _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeOwnNamespace)) - og, err := oi.ensureOperatorGroup(context.TODO()) - Expect(og).ToNot(BeNil()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).To(BeNil()) + + og, found, err := oi.getOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + Expect(found).To(BeTrue()) + Expect(og).ToNot(BeNil()) Expect(og.Name).To(Equal("operator-sdk-og")) Expect(og.Namespace).To(Equal("testns")) Expect(len(og.Spec.TargetNamespaces)).To(Equal(1)) @@ -126,9 +131,13 @@ var _ = Describe("OperatorInstaller", func() { Context("given AllNamespaces", func() { It("should create one with the given target namespaces", func() { _ = oi.InstallMode.Set(string(v1alpha1.InstallModeTypeAllNamespaces)) - og, err := oi.ensureOperatorGroup(context.TODO()) - Expect(og).ToNot(BeNil()) + err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + + og, found, err := oi.getOperatorGroup(context.TODO()) Expect(err).To(BeNil()) + Expect(found).To(BeTrue()) + Expect(og).ToNot(BeNil()) Expect(og.Name).To(Equal("operator-sdk-og")) Expect(og.Namespace).To(Equal("testns")) Expect(len(og.Spec.TargetNamespaces)).To(Equal(0)) @@ -143,8 +152,12 @@ var _ = Describe("OperatorInstaller", func() { It("should return nil for AllNamespaces with empty targets", func() { // context, client, name, ns, targets oog := createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns") - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + + og, found, err := oi.getOperatorGroup(context.TODO()) Expect(err).To(BeNil()) + Expect(found).To(BeTrue()) Expect(og.Name).To(Equal(oog.Name)) Expect(og.Namespace).To(Equal(oog.Namespace)) }) @@ -152,10 +165,9 @@ var _ = Describe("OperatorInstaller", func() { // context, client, name, ns, targets _ = createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns", "incompatiblens") - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).ShouldNot(BeNil()) Expect(err.Error()).To(ContainSubstring("is not compatible")) - Expect(og).To(BeNil()) }) }) Context("given OwnNamespace", func() { @@ -165,18 +177,21 @@ var _ = Describe("OperatorInstaller", func() { It("should return nil for OwnNamespace when target matches operator", func() { oog := createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns", "testns") - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + + og, found, err := oi.getOperatorGroup(context.TODO()) Expect(err).To(BeNil()) + Expect(found).To(BeTrue()) Expect(og.Name).To(Equal(oog.Name)) Expect(og.Namespace).To(Equal(oog.Namespace)) }) It("should return an error for OwnNamespace when target does not match operator", func() { _ = createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns", "incompatiblens") - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).ShouldNot(BeNil()) Expect(err.Error()).To(ContainSubstring("is not compatible")) - Expect(og).To(BeNil()) }) }) Context("given SingleNamespace", func() { @@ -187,8 +202,12 @@ var _ = Describe("OperatorInstaller", func() { oi.InstallMode.TargetNamespaces = []string{"anotherns"} oog := createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns", "anotherns") - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) + Expect(err).To(BeNil()) + + og, found, err := oi.getOperatorGroup(context.TODO()) Expect(err).To(BeNil()) + Expect(found).To(BeTrue()) Expect(og.Name).To(Equal(oog.Name)) Expect(og.Namespace).To(Equal(oog.Namespace)) }) @@ -196,10 +215,9 @@ var _ = Describe("OperatorInstaller", func() { oi.InstallMode.TargetNamespaces = []string{"testns"} _ = createOperatorGroupHelper(context.TODO(), client, "existing-og", "testns", "testns") - og, err := oi.ensureOperatorGroup(context.TODO()) + err := oi.ensureOperatorGroup(context.TODO()) Expect(err).ShouldNot(BeNil()) Expect(err.Error()).To(ContainSubstring("use install mode \"OwnNamespace\"")) - Expect(og).To(BeNil()) }) }) }) From 47f87ef3ee95b7446589d6bdff7b63c5c3b242f4 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 14 Sep 2020 10:55:42 -0400 Subject: [PATCH 17/24] Set SupportedInstallModes on packagemanifests as well --- internal/olm/operator/packagemanifests/install.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/olm/operator/packagemanifests/install.go b/internal/olm/operator/packagemanifests/install.go index 360b65064f..5a2ee120f0 100644 --- a/internal/olm/operator/packagemanifests/install.go +++ b/internal/olm/operator/packagemanifests/install.go @@ -76,6 +76,7 @@ func (i *Install) setup() error { i.OperatorInstaller.PackageName = pkg.PackageName i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = bundle.CSV.GetName() + i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(bundle.CSV.Spec.InstallModes) i.OperatorInstaller.Channel, err = getChannelForCSVName(pkg, i.OperatorInstaller.StartingCSV) if err != nil { return err From d0ce5686f74642f00d4096182b4a66ec80e92df5 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 14 Sep 2020 21:34:27 -0400 Subject: [PATCH 18/24] Replace validateOG with isOGCompatible --- .../operator/registry/operator_installer.go | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 700168ef04..6c0ad7e726 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -157,16 +157,18 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) error { } } + + targetNamespaces, err := o.getTargetNamespaces(supported) + if err != nil { + return err + } + if !ogFound { - targetNamespaces, err := o.getTargetNamespaces(supported) - if err != nil { - return err - } if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil { return fmt.Errorf("create operator group: %v", err) } log.Infof("OperatorGroup %q created", og.Name) - } else if err := o.validateOperatorGroup(*og, supported); err != nil { + } else if err := o.isOperatorGroupCompatible(*og, targetNamespaces); err != nil { return err } @@ -181,26 +183,14 @@ func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNames return og, nil } -func (o *OperatorInstaller) validateOperatorGroup(og v1.OperatorGroup, supported sets.String) error { - ogTargetNs := sets.NewString(og.Spec.TargetNamespaces...) - imTargetNs := sets.NewString(o.InstallMode.TargetNamespaces...) - ownNamespaceNs := sets.NewString(o.cfg.Namespace) - - if supported.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) && len(og.Spec.TargetNamespaces) == 0 || - supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && ogTargetNs.Equal(ownNamespaceNs) || - supported.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) && ogTargetNs.Equal(imTargetNs) { - return nil - } - - switch o.InstallMode.InstallModeType { - case v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeOwnNamespace, - v1alpha1.InstallModeTypeSingleNamespace: +func (o *OperatorInstaller) isOperatorGroupCompatible(og v1.OperatorGroup, targetNamespaces []string) error { + targets := sets.NewString(targetNamespaces...) + ogtargets := sets.NewString(og.Spec.TargetNamespaces...) + if !ogtargets.Equal(targets) { return fmt.Errorf("existing operatorgroup %q is not compatible with install mode %q", og.Name, o.InstallMode) - case "": - return fmt.Errorf("existing operatorgroup %q is not compatible with any supported package install modes", og.Name) } - return fmt.Errorf("unknown install mode %q", o.InstallMode.InstallModeType) + return nil } // getOperatorGroup returns true if an OperatorGroup in the desired namespace was found. From edf5b30b2852a951de56263513c321cdb7995f8b Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 14 Sep 2020 22:09:55 -0400 Subject: [PATCH 19/24] Validate supported install modes earlier --- internal/olm/operator/bundle/install.go | 5 +++++ internal/olm/operator/packagemanifests/install.go | 5 +++++ internal/olm/operator/registry/operator_installer.go | 6 ------ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/olm/operator/bundle/install.go b/internal/olm/operator/bundle/install.go index 5cf039c57c..595bc9da5f 100644 --- a/internal/olm/operator/bundle/install.go +++ b/internal/olm/operator/bundle/install.go @@ -79,6 +79,11 @@ func (i *Install) setup(ctx context.Context) error { i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = csv.Name i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(csv.Spec.InstallModes) + + if i.OperatorInstaller.SupportedInstallModes.Len() == 0 { + return fmt.Errorf("operator %q is not installable: no supported install modes", csv.Name) + } + i.OperatorInstaller.Channel = strings.Split(labels["operators.operatorframework.io.bundle.channels.v1"], ",")[0] i.IndexImageCatalogCreator.BundleImage = i.BundleImage i.IndexImageCatalogCreator.PackageName = i.OperatorInstaller.PackageName diff --git a/internal/olm/operator/packagemanifests/install.go b/internal/olm/operator/packagemanifests/install.go index 5a2ee120f0..f62767f573 100644 --- a/internal/olm/operator/packagemanifests/install.go +++ b/internal/olm/operator/packagemanifests/install.go @@ -77,6 +77,11 @@ func (i *Install) setup() error { i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = bundle.CSV.GetName() i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(bundle.CSV.Spec.InstallModes) + + if i.OperatorInstaller.SupportedInstallModes.Len() == 0 { + return fmt.Errorf("operator %q is not installable: no supported install modes", bundle.CSV.GetName()) + } + i.OperatorInstaller.Channel, err = getChannelForCSVName(pkg, i.OperatorInstaller.StartingCSV) if err != nil { return err diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 6c0ad7e726..17020cf678 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -133,13 +133,8 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) error { supported := o.SupportedInstallModes - if supported.Len() == 0 { - return fmt.Errorf("operator %q is not installable: no supported install modes", o.StartingCSV) - } - // --install-mode was given if !o.InstallMode.IsEmpty() { - // TODO: probably remove multinamespace if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace { targetNsSet := sets.NewString(o.InstallMode.TargetNamespaces...) if !supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNsSet.Has(o.cfg.Namespace) { @@ -155,7 +150,6 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) error { if supported.Len() == 0 { return fmt.Errorf("operator %q does not support install mode %q", o.StartingCSV, o.InstallMode.InstallModeType) } - } targetNamespaces, err := o.getTargetNamespaces(supported) From 2ed2b9156802325db8b66efc86d558149fff4327 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 17 Sep 2020 09:52:36 -0400 Subject: [PATCH 20/24] Rework the install mode verification --- internal/olm/operator/install_mode.go | 12 ++++++++++++ internal/olm/operator/registry/operator_installer.go | 6 ------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/olm/operator/install_mode.go b/internal/olm/operator/install_mode.go index e2a6694473..61e5b91b79 100644 --- a/internal/olm/operator/install_mode.go +++ b/internal/olm/operator/install_mode.go @@ -99,11 +99,23 @@ func (i InstallMode) Validate() error { // CheckCompatibility checks if an InstallMode is compatible with the operator's namespace and is supported by csv. func (i InstallMode) CheckCompatibility(csv *v1alpha1.ClusterServiceVersion, operatorNamespace string) error { + // allnamespace was validated in Validate() + + // own namespace and targetns != opname if i.InstallModeType == v1alpha1.InstallModeTypeOwnNamespace { if len(i.TargetNamespaces) > 0 && i.TargetNamespaces[0] != operatorNamespace { return fmt.Errorf("install mode %s must match operator namespace %q", i, operatorNamespace) } } + + // single namespace and targetns == opname + if i.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace { + if len(i.TargetNamespaces) < 1 || i.TargetNamespaces[0] == operatorNamespace { + return fmt.Errorf("use install mode %q to watch operator's namespace %q", v1alpha1.InstallModeTypeOwnNamespace, i.TargetNamespaces[0]) + } + } + + // ensure the CSV supports the given installmode for _, mode := range csv.Spec.InstallModes { if mode.Type == i.InstallModeType && !mode.Supported { return fmt.Errorf("install mode type %q not supported in CSV %q", i.InstallModeType, csv.GetName()) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 17020cf678..31f50414dd 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -135,12 +135,6 @@ func (o OperatorInstaller) ensureOperatorGroup(ctx context.Context) error { // --install-mode was given if !o.InstallMode.IsEmpty() { - if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace { - targetNsSet := sets.NewString(o.InstallMode.TargetNamespaces...) - if !supported.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNsSet.Has(o.cfg.Namespace) { - return fmt.Errorf("cannot watch namespace %q: operator %q does not support install mode %q", o.cfg.Namespace, o.StartingCSV, v1alpha1.InstallModeTypeOwnNamespace) - } - } if o.InstallMode.InstallModeType == v1alpha1.InstallModeTypeSingleNamespace && o.InstallMode.TargetNamespaces[0] == o.cfg.Namespace { return fmt.Errorf("use install mode %q to watch operator's namespace %q", v1alpha1.InstallModeTypeOwnNamespace, o.cfg.Namespace) From 45ba1d0f07ebea5f333df60017e324568a1ce394 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 17 Sep 2020 11:01:42 -0400 Subject: [PATCH 21/24] add TODO for createSubscription & deleted unused test --- .../olm/operator/registry/operator_installer_test.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index 5d972f53f8..238193743b 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -15,8 +15,6 @@ package registry import ( - // "context" - "context" . "github.com/onsi/ginkgo" @@ -81,14 +79,6 @@ var _ = Describe("OperatorInstaller", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring("no supported install modes")) }) - // It("should return an error if OwnNamespace is used and target does not match", func() { - // oi.InstallMode.Set(string(v1alpha1.InstallModeTypeOwnNamespace)) - // oi.InstallMode.TargetNamespaces = []string{"notownns"} - // og, err := oi.ensureOperatorGroup(context.TODO()) - // Expect(og).To(BeNil()) - // Expect(err).ToNot(BeNil()) - // Expect(err.Error()).Should(ContainSubstring("use install mode \"OwnNamespace\"")) - // }) Context("with no existing OperatorGroup", func() { Context("given SingleNamespace", func() { It("should create one with the given target namespaces", func() { @@ -309,6 +299,7 @@ var _ = Describe("OperatorInstaller", func() { }) Describe("createSubscription", func() { + // TODO: add them as part of a different story }) Describe("getTargetNamespaces", func() { From b5f0e32461f48d5904b37024178a3d3934b835e4 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 17 Sep 2020 11:20:58 -0400 Subject: [PATCH 22/24] Move support install modes validation to CheckCompatiblity method --- internal/olm/operator/bundle/install.go | 5 ----- internal/olm/operator/install_mode.go | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/olm/operator/bundle/install.go b/internal/olm/operator/bundle/install.go index 595bc9da5f..5cf039c57c 100644 --- a/internal/olm/operator/bundle/install.go +++ b/internal/olm/operator/bundle/install.go @@ -79,11 +79,6 @@ func (i *Install) setup(ctx context.Context) error { i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = csv.Name i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(csv.Spec.InstallModes) - - if i.OperatorInstaller.SupportedInstallModes.Len() == 0 { - return fmt.Errorf("operator %q is not installable: no supported install modes", csv.Name) - } - i.OperatorInstaller.Channel = strings.Split(labels["operators.operatorframework.io.bundle.channels.v1"], ",")[0] i.IndexImageCatalogCreator.BundleImage = i.BundleImage i.IndexImageCatalogCreator.PackageName = i.OperatorInstaller.PackageName diff --git a/internal/olm/operator/install_mode.go b/internal/olm/operator/install_mode.go index 61e5b91b79..56399afce8 100644 --- a/internal/olm/operator/install_mode.go +++ b/internal/olm/operator/install_mode.go @@ -115,6 +115,11 @@ func (i InstallMode) CheckCompatibility(csv *v1alpha1.ClusterServiceVersion, ope } } + // ensure the CSV has an installmode + if len(csv.Spec.InstallModes) == 0 { + return fmt.Errorf("operator %q is not installable: no supported install modes", csv.Name) + } + // ensure the CSV supports the given installmode for _, mode := range csv.Spec.InstallModes { if mode.Type == i.InstallModeType && !mode.Supported { From 59889fac231bb73185fd53aea4e18c37802e9be9 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Thu, 17 Sep 2020 11:49:35 -0400 Subject: [PATCH 23/24] Switch to constants for the label keys --- internal/olm/operator/bundle/install.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/olm/operator/bundle/install.go b/internal/olm/operator/bundle/install.go index 5cf039c57c..54c812e22f 100644 --- a/internal/olm/operator/bundle/install.go +++ b/internal/olm/operator/bundle/install.go @@ -23,6 +23,7 @@ import ( apimanifests "github.com/operator-framework/api/pkg/manifests" "github.com/operator-framework/api/pkg/operators/v1alpha1" + registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" "github.com/spf13/pflag" "github.com/operator-framework/operator-sdk/internal/olm/operator" @@ -75,11 +76,11 @@ func (i *Install) setup(ctx context.Context) error { return err } - i.OperatorInstaller.PackageName = labels["operators.operatorframework.io.bundle.package.v1"] + i.OperatorInstaller.PackageName = labels[registrybundle.PackageLabel] i.OperatorInstaller.CatalogSourceName = fmt.Sprintf("%s-catalog", i.OperatorInstaller.PackageName) i.OperatorInstaller.StartingCSV = csv.Name i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(csv.Spec.InstallModes) - i.OperatorInstaller.Channel = strings.Split(labels["operators.operatorframework.io.bundle.channels.v1"], ",")[0] + i.OperatorInstaller.Channel = strings.Split(labels[registrybundle.ChannelsLabel], ",")[0] i.IndexImageCatalogCreator.BundleImage = i.BundleImage i.IndexImageCatalogCreator.PackageName = i.OperatorInstaller.PackageName i.IndexImageCatalogCreator.InjectBundles = []string{i.BundleImage} From 4efad9d06a1c860445930d85da4a163cb646c703 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Tue, 22 Sep 2020 13:54:36 -0400 Subject: [PATCH 24/24] Default to existing operatorgroup if no installmode present --- .../operator/registry/operator_installer.go | 6 +++ .../registry/operator_installer_test.go | 37 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/internal/olm/operator/registry/operator_installer.go b/internal/olm/operator/registry/operator_installer.go index 31f50414dd..4d5448ba58 100644 --- a/internal/olm/operator/registry/operator_installer.go +++ b/internal/olm/operator/registry/operator_installer.go @@ -172,6 +172,12 @@ func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNames } func (o *OperatorInstaller) isOperatorGroupCompatible(og v1.OperatorGroup, targetNamespaces []string) error { + // no install mode use the existing operator group + if o.InstallMode.IsEmpty() { + return nil + } + + // otherwise, check that the target namespaces match targets := sets.NewString(targetNamespaces...) ogtargets := sets.NewString(og.Spec.TargetNamespaces...) if !ogtargets.Equal(targets) { diff --git a/internal/olm/operator/registry/operator_installer_test.go b/internal/olm/operator/registry/operator_installer_test.go index 238193743b..c6a48b3836 100644 --- a/internal/olm/operator/registry/operator_installer_test.go +++ b/internal/olm/operator/registry/operator_installer_test.go @@ -247,6 +247,43 @@ var _ = Describe("OperatorInstaller", func() { }) }) + Describe("isOperatorGroupCompatible", func() { + var ( + oi OperatorInstaller + og v1.OperatorGroup + ) + BeforeEach(func() { + oi = OperatorInstaller{} + og = createOperatorGroupHelper(context.TODO(), nil, "existing-og", "default", "default") + }) + It("should return an error if namespaces do not match", func() { + oi.InstallMode = operator.InstallMode{ + InstallModeType: v1alpha1.InstallModeTypeOwnNamespace, + TargetNamespaces: []string{"wontmatchns"}, + } + + err := oi.isOperatorGroupCompatible(og, oi.InstallMode.TargetNamespaces) + Expect(err).ShouldNot(BeNil()) + Expect(err.Error()).Should(ContainSubstring("is not compatible")) + }) + It("should return nil if no installmode is empty", func() { + // empty install mode + oi.InstallMode = operator.InstallMode{} + Expect(oi.InstallMode.IsEmpty()).To(BeTrue()) + err := oi.isOperatorGroupCompatible(og, oi.InstallMode.TargetNamespaces) + Expect(err).Should(BeNil()) + }) + It("should return nil if namespaces match", func() { + oi.InstallMode = operator.InstallMode{ + InstallModeType: v1alpha1.InstallModeTypeOwnNamespace, + TargetNamespaces: []string{"matchingns"}, + } + aog := createOperatorGroupHelper(context.TODO(), nil, "existing-og", "testns", "matchingns") + err := oi.isOperatorGroupCompatible(aog, oi.InstallMode.TargetNamespaces) + Expect(err).Should(BeNil()) + }) + }) + Describe("getOperatorGroup", func() { var ( oi OperatorInstaller