run bundle: OperatorGroup Install modes

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,10 +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.Channel = strings.Split(labels["operators.operatorframework.io.bundle.channels.v1"], ",")[0]
+	i.OperatorInstaller.SupportedInstallModes = operator.GetSupportedInstallModes(csv.Spec.InstallModes)
+	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}

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"
 )
 
@@ -31,6 +32,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 +43,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 == ""
 }
@@ -93,15 +99,44 @@ 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 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)
 		}
 	}
+
+	// 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 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 {
 			return fmt.Errorf("install mode type %q not supported in CSV %q", i.InstallModeType, csv.GetName())
 		}
 	}
 	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
+}

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())
+		})
+	})
+})

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")
+}

internal/olm/operator/packagemanifests/install.go

@@ -69,16 +69,19 @@ 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
 	}
 
 	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)
+
+	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

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,48 +124,66 @@ 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) error {
 	// 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{}
+
+	supported := o.SupportedInstallModes
+
+	// --install-mode was given
+	if !o.InstallMode.IsEmpty() {
+		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)
 		}
-		// 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)
+
+		supported = supported.Intersection(sets.NewString(string(o.InstallMode.InstallModeType)))
+		if supported.Len() == 0 {
+			return fmt.Errorf("operator %q does not support install mode %q", o.StartingCSV, o.InstallMode.InstallModeType)
 		}
-		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)
+	}
+
+	targetNamespaces, err := o.getTargetNamespaces(supported)
+	if err != nil {
+		return err
+	}
+
+	if !ogFound {
+		if og, err = o.createOperatorGroup(ctx, targetNamespaces); err != nil {
+			return fmt.Errorf("create operator group: %v", err)
 		}
-		log.Infof("Created OperatorGroup: %s", og.GetName())
+		log.Infof("OperatorGroup %q created", og.Name)
+	} else if err := o.isOperatorGroupCompatible(*og, targetNamespaces); err != nil {
+		return err
+	}
+
+	return nil
+}
 
+func (o *OperatorInstaller) createOperatorGroup(ctx context.Context, targetNamespaces []string) (*v1.OperatorGroup, error) {
+	og := newSDKOperatorGroup(o.cfg.Namespace, withTargetNamespaces(targetNamespaces...))
+	if err := o.cfg.Client.Create(ctx, og); err != nil {
+		return nil, err
+	}
+	return og, nil
+}
+
+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) {
+		return fmt.Errorf("existing operatorgroup %q is not compatible with install mode %q", og.Name, o.InstallMode)
+	}
+
 	return nil
 }
 
@@ -278,3 +298,16 @@ func (o OperatorInstaller) waitForInstallPlan(ctx context.Context, sub *v1alpha1
 	}
 	return nil
 }
+
+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)):
+		return o.InstallMode.TargetNamespaces, nil
+	default:
+		return nil, fmt.Errorf("no supported install modes")
+	}
+}