fix(csv): CSV update process optimization regression

Makefile

@@ -42,7 +42,7 @@ build-coverage: $(CMDS)
 $(CMDS): .FORCE
 	@if [ cover-$(GENCOVER) = cover-true ]; then \
 		echo "building bin/$(shell basename $@)" with coverage; \
-		GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go test -o $@ -c -covermode=count -coverpkg ./pkg/... $(PKG)/cmd/$(shell basename $@); \
+		GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go test -o $@ -c -covermode=count -coverpkg ./pkg/controller/... $(PKG)/cmd/$(shell basename $@); \
 	else \
 		echo "building bin/$(shell basename $@)"; \
 		GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o $@ $(PKG)/cmd/$(shell basename $@); \

cmd/olm/main.go

@@ -10,7 +10,10 @@ import (
 
 	log "github.com/sirupsen/logrus"
 
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/signals"
 )
 
@@ -73,8 +76,16 @@ func main() {
 	// the empty string, the resulting array will be `[]string{""}`.
 	namespaces := strings.Split(*watchedNamespaces, ",")
 
+	// Create a client for OLM
+	crClient, err := client.NewClient(*kubeConfigPath)
+	if err != nil {
+		log.Fatalf("error configuring client: %s", err.Error())
+	}
+
+	opClient := operatorclient.NewClient(*kubeConfigPath)
+
 	// Create a new instance of the operator.
-	operator, err := olm.NewOperator(*kubeConfigPath, *wakeupInterval, annotation, namespaces)
+	operator, err := olm.NewOperator(crClient, opClient, &install.StrategyResolver{}, *wakeupInterval, annotation, namespaces)
 
 	if err != nil {
 		log.Fatalf("error configuring operator: %s", err.Error())

deploy/chart/catalog_resources/ocs/etcdoperator.v0.9.0.clusterserviceversion.yaml

@@ -1,6 +1,6 @@
 #! validate-crd: ./deploy/chart/templates/03-clusterserviceversion.crd.yaml
 #! parse-kind: ClusterServiceVersion
-apiVersion: app.coreos.com/v1alpha1
+apiVersion: operators.coreos.com/v1alpha1
 kind: ClusterServiceVersion
 metadata:
   name: etcdoperator.v0.9.0

deploy/chart/catalog_resources/ocs/etcdoperator.v0.9.2.clusterserviceversion.yaml

@@ -1,6 +1,6 @@
 #! validate-crd: ./deploy/chart/templates/03-clusterserviceversion.crd.yaml
 #! parse-kind: ClusterServiceVersion
-apiVersion: app.coreos.com/v1alpha1
+apiVersion: operators.coreos.com/v1alpha1
 kind: ClusterServiceVersion
 metadata:
   name: etcdoperator.v0.9.2

pkg/controller/operators/catalog/operator.go

@@ -169,7 +169,7 @@ func (o *Operator) syncSubscriptions(obj interface{}) (syncError error) {
 	var updatedSub *v1alpha1.Subscription
 	updatedSub, syncError = o.syncSubscription(sub)
 
-	if updatedSub == nil {
+	if updatedSub == nil || updatedSub.Status.State == sub.Status.State {
 		return
 	}
 	if syncError != nil {

pkg/controller/operators/catalog/subscriptions.go

@@ -23,116 +23,116 @@ const (
 )
 
 // FIXME(alecmerdler): Rewrite this whole block to be more clear
-func (o *Operator) syncSubscription(sub *v1alpha1.Subscription) (*v1alpha1.Subscription, error) {
-	if sub == nil || sub.Spec == nil {
+func (o *Operator) syncSubscription(in *v1alpha1.Subscription) (*v1alpha1.Subscription, error) {
+	if in == nil || in.Spec == nil {
 		return nil, ErrNilSubscription
 	}
-
-	sub = ensureLabels(sub)
+	out := in.DeepCopy()
+	out = ensureLabels(out)
 
 	// Only sync if catalog has been updated since last sync time
-	if o.sourcesLastUpdate.Before(&sub.Status.LastUpdated) && sub.Status.State == v1alpha1.SubscriptionStateAtLatest {
+	if o.sourcesLastUpdate.Before(&out.Status.LastUpdated) && out.Status.State == v1alpha1.SubscriptionStateAtLatest {
 		log.Infof("skipping sync: no new updates to catalog since last sync at %s",
-			sub.Status.LastUpdated.String())
+			out.Status.LastUpdated.String())
 		return nil, nil
 	}
 
 	o.sourcesLock.Lock()
 	defer o.sourcesLock.Unlock()
 
-	catalogNamespace := sub.Spec.CatalogSourceNamespace
+	catalogNamespace := out.Spec.CatalogSourceNamespace
 	if catalogNamespace == "" {
 		catalogNamespace = o.namespace
 	}
-	catalog, ok := o.sources[registry.SourceKey{Name: sub.Spec.CatalogSource, Namespace: catalogNamespace}]
+	catalog, ok := o.sources[registry.SourceKey{Name: out.Spec.CatalogSource, Namespace: catalogNamespace}]
 	if !ok {
-		return sub, fmt.Errorf("unknown catalog source %s in namespace %s", sub.Spec.CatalogSource, catalogNamespace)
+		return out, fmt.Errorf("unknown catalog source %s in namespace %s", out.Spec.CatalogSource, catalogNamespace)
 	}
 
 	// Find latest CSV if no CSVs are installed already
-	if sub.Status.CurrentCSV == "" {
-		if sub.Spec.StartingCSV != "" {
-			sub.Status.CurrentCSV = sub.Spec.StartingCSV
+	if out.Status.CurrentCSV == "" {
+		if out.Spec.StartingCSV != "" {
+			out.Status.CurrentCSV = out.Spec.StartingCSV
 		} else {
-			csv, err := catalog.FindCSVForPackageNameUnderChannel(sub.Spec.Package, sub.Spec.Channel)
+			csv, err := catalog.FindCSVForPackageNameUnderChannel(out.Spec.Package, out.Spec.Channel)
 			if err != nil {
-				return sub, fmt.Errorf("failed to find CSV for package %s in channel %s: %v", sub.Spec.Package, sub.Spec.Channel, err)
+				return out, fmt.Errorf("failed to find CSV for package %s in channel %s: %v", out.Spec.Package, out.Spec.Channel, err)
 			}
 			if csv == nil {
-				return sub, fmt.Errorf("failed to find CSV for package %s in channel %s: nil CSV", sub.Spec.Package, sub.Spec.Channel)
+				return out, fmt.Errorf("failed to find CSV for package %s in channel %s: nil CSV", out.Spec.Package, out.Spec.Channel)
 			}
-			sub.Status.CurrentCSV = csv.GetName()
+			out.Status.CurrentCSV = csv.GetName()
 		}
-		sub.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable
-		return sub, nil
+		out.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable
+		return out, nil
 	}
 
 	// Check that desired CSV has been installed
-	csv, err := o.client.OperatorsV1alpha1().ClusterServiceVersions(sub.GetNamespace()).Get(sub.Status.CurrentCSV, metav1.GetOptions{})
+	csv, err := o.client.OperatorsV1alpha1().ClusterServiceVersions(out.GetNamespace()).Get(out.Status.CurrentCSV, metav1.GetOptions{})
 	if err != nil || csv == nil {
-		log.Infof("error fetching CSV %s via k8s api: %v", sub.Status.CurrentCSV, err)
-		if sub.Status.Install != nil && sub.Status.Install.Name != "" {
-			ip, err := o.client.OperatorsV1alpha1().InstallPlans(sub.GetNamespace()).Get(sub.Status.Install.Name, metav1.GetOptions{})
+		log.Infof("error fetching CSV %s via k8s api: %v", out.Status.CurrentCSV, err)
+		if out.Status.Install != nil && out.Status.Install.Name != "" {
+			ip, err := o.client.OperatorsV1alpha1().InstallPlans(out.GetNamespace()).Get(out.Status.Install.Name, metav1.GetOptions{})
 			if err != nil {
-				log.Errorf("get installplan %s error: %v", sub.Status.Install.Name, err)
+				log.Errorf("get installplan %s error: %v", out.Status.Install.Name, err)
 			}
 			if err == nil && ip != nil {
-				log.Infof("installplan for %s already exists", sub.Status.CurrentCSV)
-				return sub, nil
+				log.Infof("installplan for %s already exists", out.Status.CurrentCSV)
+				return out, nil
 			}
-			log.Infof("installplan %s not found: creating new plan", sub.Status.Install.Name)
-			sub.Status.Install = nil
+			log.Infof("installplan %s not found: creating new plan", out.Status.Install.Name)
+			out.Status.Install = nil
 		}
 
 		// Install CSV if doesn't exist
-		sub.Status.State = v1alpha1.SubscriptionStateUpgradePending
+		out.Status.State = v1alpha1.SubscriptionStateUpgradePending
 		ip := &v1alpha1.InstallPlan{
 			ObjectMeta: metav1.ObjectMeta{},
 			Spec: v1alpha1.InstallPlanSpec{
-				ClusterServiceVersionNames: []string{sub.Status.CurrentCSV},
-				Approval:                   sub.GetInstallPlanApproval(),
+				ClusterServiceVersionNames: []string{out.Status.CurrentCSV},
+				Approval:                   out.GetInstallPlanApproval(),
 			},
 		}
-		ownerutil.AddNonBlockingOwner(ip, sub)
-		ip.SetGenerateName(fmt.Sprintf("install-%s-", sub.Status.CurrentCSV))
-		ip.SetNamespace(sub.GetNamespace())
+		ownerutil.AddNonBlockingOwner(ip, out)
+		ip.SetGenerateName(fmt.Sprintf("install-%s-", out.Status.CurrentCSV))
+		ip.SetNamespace(out.GetNamespace())
 
 		// Inherit the subscription's catalog source
-		ip.Spec.CatalogSource = sub.Spec.CatalogSource
-		ip.Spec.CatalogSourceNamespace = sub.Spec.CatalogSourceNamespace
+		ip.Spec.CatalogSource = out.Spec.CatalogSource
+		ip.Spec.CatalogSourceNamespace = out.Spec.CatalogSourceNamespace
 
-		res, err := o.client.OperatorsV1alpha1().InstallPlans(sub.GetNamespace()).Create(ip)
+		res, err := o.client.OperatorsV1alpha1().InstallPlans(out.GetNamespace()).Create(ip)
 		if err != nil {
-			return sub, fmt.Errorf("failed to ensure current CSV %s installed: %v", sub.Status.CurrentCSV, err)
+			return out, fmt.Errorf("failed to ensure current CSV %s installed: %v", out.Status.CurrentCSV, err)
 		}
 		if res == nil {
-			return sub, errors.New("unexpected installplan returned by k8s api on create: <nil>")
+			return out, errors.New("unexpected installplan returned by k8s api on create: <nil>")
 		}
-		sub.Status.Install = &v1alpha1.InstallPlanReference{
+		out.Status.Install = &v1alpha1.InstallPlanReference{
 			UID:        res.GetUID(),
 			Name:       res.GetName(),
 			APIVersion: v1alpha1.SchemeGroupVersion.String(),
 			Kind:       v1alpha1.InstallPlanKind,
 		}
-		return sub, nil
+		return out, nil
 	}
 
 	// Poll catalog for an update
-	repl, err := catalog.FindReplacementCSVForPackageNameUnderChannel(sub.Spec.Package, sub.Spec.Channel, sub.Status.CurrentCSV)
+	repl, err := catalog.FindReplacementCSVForPackageNameUnderChannel(out.Spec.Package, out.Spec.Channel, out.Status.CurrentCSV)
 	if err != nil {
-		sub.Status.State = v1alpha1.SubscriptionStateAtLatest
-		return sub, fmt.Errorf("failed to lookup replacement CSV for %s: %v", sub.Status.CurrentCSV, err)
+		out.Status.State = v1alpha1.SubscriptionStateAtLatest
+		return out, fmt.Errorf("failed to lookup replacement CSV for %s: %v", out.Status.CurrentCSV, err)
 	}
 	if repl == nil {
-		sub.Status.State = v1alpha1.SubscriptionStateAtLatest
-		return sub, fmt.Errorf("nil replacement CSV for %s returned from catalog", sub.Status.CurrentCSV)
+		out.Status.State = v1alpha1.SubscriptionStateAtLatest
+		return out, fmt.Errorf("nil replacement CSV for %s returned from catalog", out.Status.CurrentCSV)
 	}
 
 	// Update subscription with new latest
-	sub.Status.CurrentCSV = repl.GetName()
-	sub.Status.Install = nil
-	sub.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable
-	return sub, nil
+	out.Status.CurrentCSV = repl.GetName()
+	out.Status.Install = nil
+	out.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable
+	return out, nil
 }
 
 func ensureLabels(sub *v1alpha1.Subscription) *v1alpha1.Subscription {