fix(subscriptions): add check for csv manifest dif before applying new

installplans

Makefile

@@ -153,6 +153,7 @@ container-mockgen:
 	docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/fakes/client-go/listers/. ./pkg/fakes/client-go/listers
 	docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes/. ./pkg/lib/operatorlister/operatorlisterfakes
 	docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient/mock_client.go ./pkg/lib/operatorclient/mock_client.go
+	docker cp temp-mockgen:/go/src/github.com/operator-framework/operator-lifecycle-manager/pkg/fakes/. ./pkg/fakes/.
 	docker rm temp-mockgen
 
 # Must be run in gopath: https://github.com/kubernetes/kubernetes/issues/67566

go.mod

@@ -1,6 +1,7 @@
 module github.com/operator-framework/operator-lifecycle-manager
 
 require (
+	github.com/coreos/bbolt v1.3.2 // indirect
 	github.com/coreos/etcd v3.3.10+incompatible // indirect
 	github.com/coreos/go-semver v0.2.0
 	github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 // indirect
@@ -12,12 +13,13 @@ require (
 	github.com/go-openapi/strfmt v0.18.0 // indirect
 	github.com/go-openapi/validate v0.18.0 // indirect
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
+	github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
 	github.com/golang/mock v1.1.1
 	github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c // indirect
 	github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf // indirect
 	github.com/googleapis/gnostic v0.2.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.0.0 // indirect
-	github.com/grpc-ecosystem/grpc-gateway v1.6.3
+	github.com/grpc-ecosystem/grpc-gateway v1.6.3 // indirect
 	github.com/imdario/mergo v0.3.6 // indirect
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.5 // indirect
@@ -37,6 +39,7 @@ require (
 	github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5 // indirect
 	github.com/ugorji/go/codec v0.0.0-20181022190402-e5e69e061d4f // indirect
 	github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 // indirect
+	go.etcd.io/bbolt v1.3.2 // indirect
 	go.uber.org/atomic v1.3.2 // indirect
 	go.uber.org/multierr v1.1.0 // indirect
 	go.uber.org/zap v1.9.1 // indirect

go.sum

@@ -20,6 +20,8 @@ github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx2
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/coreos/bbolt v1.3.0 h1:HIgH5xUWXT914HCI671AxuTTqjj64UOFr7pHn48LUTI=
 github.com/coreos/bbolt v1.3.0/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
+github.com/coreos/bbolt v1.3.2 h1:wZwiHHUieZCquLkDL0B8UhzreNWsPHooDAG3q34zk0s=
+github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.9+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
 github.com/coreos/etcd v3.3.10+incompatible h1:KjVWqrZ5U0wa3CxY2AxlH6/UcB+PK2td1DcsYhA+HRs=
 github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
@@ -89,6 +91,8 @@ github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfU
 github.com/golang/groupcache v0.0.0-20180924190550-6f2cf27854a4/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/groupcache v0.0.0-20181024230925-c65c006176ff h1:kOkM9whyQYodu09SJ6W3NCsHG7crFaJILQ22Gozp3lg=
 github.com/golang/groupcache v0.0.0-20181024230925-c65c006176ff/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
+github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef h1:veQD95Isof8w9/WXiA+pa3tz3fJXkt5B7QaRBrM62gk=
+github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:tluoj9z5200jBnyusfRPU2LqT6J+DAorxEvtC7LHB+E=
 github.com/golang/mock v1.1.1 h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8=
 github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
@@ -206,6 +210,8 @@ github.com/ugorji/go/codec v0.0.0-20181022190402-e5e69e061d4f/go.mod h1:VFNgLljT
 github.com/xiang90/probing v0.0.0-20160813154853-07dd2e8dfe18/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
+go.etcd.io/bbolt v1.3.2 h1:Z/90sZLPOeCy2PwprqkFa25PdkusRzaj9P8zm/KNyvk=
+go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
 go.uber.org/atomic v1.3.2 h1:2Oa65PReHzfn29GpvgsYwloV9AVFHPDk8tYxt2c2tr4=
 go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/multierr v1.1.0 h1:HoEmRHQPVSqub6w2z2d2EOVs2fjyFRGyofhKuyDq0QI=

pkg/api/apis/operators/v1alpha1/installplan_types.go

@@ -145,6 +145,43 @@ type Step struct {
 	Status    StepStatus   `json:"status"`
 }
 
+// ManifestsMatch returns true if the CSV manifests in the StepResources of the given list of steps
+// matches those in the InstallPlanStatus.
+func (s *InstallPlanStatus) CSVManifestsMatch(steps []*Step) bool {
+	if s.Plan == nil && steps == nil {
+		return true
+	}
+	if s.Plan == nil || steps == nil {
+		return false
+	}
+
+	manifests := make(map[string]struct{})
+	for _, step := range s.Plan {
+		resource := step.Resource
+		if resource.Kind != ClusterServiceVersionKind {
+			continue
+		}
+		manifests[resource.Manifest] = struct{}{}
+	}
+
+	for _, step := range steps {
+		resource := step.Resource
+		if resource.Kind != ClusterServiceVersionKind {
+			continue
+		}
+		if _, ok := manifests[resource.Manifest]; !ok {
+			return false
+		}
+		delete(manifests, resource.Manifest)
+	}
+
+	if len(manifests) == 0 {
+		return true
+	}
+
+	return false
+}
+
 func (s *Step) String() string {
 	return fmt.Sprintf("%s: %s (%s)", s.Resolving, s.Resource, s.Status)
 }

pkg/controller/operators/catalog/operator.go

@@ -47,8 +47,7 @@ const (
 	serviceKind            = "Service"
 	roleKind               = "Role"
 	roleBindingKind        = "RoleBinding"
-
-	generatedByKey  = "olm/generated-by"
+	generatedByKey  	   = "olm.generated-by"
 )
 
 // for test stubbing and for ensuring standardization of timezones to UTC
@@ -580,9 +579,9 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error {
 			break
 		}
 	}
-	installplanReference, err := o.createInstallPlan(namespace, subs, installPlanApproval, steps)
+	installplanReference, err := o.ensureInstallPlan(logger, namespace, subs, installPlanApproval, steps)
 	if err != nil {
-		logger.WithError(err).Debug("error creating installplan")
+		logger.WithError(err).Debug("error ensuring installplan")
 		return err
 	}
 
@@ -679,8 +678,6 @@ func (o *Operator) ensureSubscriptionInstallPlanState(logger *logrus.Entry, sub
 	// this indicates it was newly resolved by another operator, and we should reference that installplan in the status
 	ipName, ok := sub.GetAnnotations()[generatedByKey]
 	if !ok {
-		// err := fmt.Errorf("no installplan reference or %s annotation found", generatedByKey)
-		// logger.WithField("err", err.Error()).Error("an error occurred while associating a subscription with an installplan")
 		return sub, nil
 	}
 
@@ -744,6 +741,28 @@ func (o *Operator) updateSubscriptionSetInstallPlanState(namespace string, subs
 	return nil
 }
 
+func (o *Operator) ensureInstallPlan(logger *logrus.Entry, namespace string, subs []*v1alpha1.Subscription, installPlanApproval v1alpha1.Approval, steps []*v1alpha1.Step) (*v1alpha1.InstallPlanReference, error) {
+	if len(steps) == 0 {
+		return nil, nil
+	}
+
+	// Check if any existing installplans are creating the same resources
+	installPlans, err := o.lister.OperatorsV1alpha1().InstallPlanLister().InstallPlans(namespace).List(labels.Everything())
+	if err != nil {
+		return nil, err
+	}
+
+	for _, installPlan := range installPlans {
+		if installPlan.Status.CSVManifestsMatch(steps) {
+			logger.Infof("found InstallPlan with matching manifests: %s", installPlan.GetName())
+			return o.referenceForInstallPlan(installPlan), nil
+		}
+	}
+	logger.Warn("no installplan found with matching manifests, creating new one")
+
+	return o.createInstallPlan(namespace, subs, installPlanApproval, steps)
+}
+
 func (o *Operator) createInstallPlan(namespace string, subs []*v1alpha1.Subscription, installPlanApproval v1alpha1.Approval, steps []*v1alpha1.Step) (*v1alpha1.InstallPlanReference, error) {
 	if len(steps) == 0 {
 		return nil, nil

pkg/controller/operators/catalog/operator_test.go

@@ -488,7 +488,8 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
 	podInformer := informerFactory.Core().V1().Pods()
 	configMapInformer := informerFactory.Core().V1().ConfigMaps()
 	subscriptionInformer := externalversions.NewSharedInformerFactoryWithOptions(clientFake, wakeupInterval, externalversions.WithNamespace(namespace)).Operators().V1alpha1().Subscriptions()
-
+	installPlanInformer := externalversions.NewSharedInformerFactoryWithOptions(clientFake,  wakeupInterval, externalversions.WithNamespace(namespace)).Operators().V1alpha1().InstallPlans()
+
 	// register informers
 	registryInformers := []cache.SharedIndexInformer{
 		roleInformer.Informer(),
@@ -498,6 +499,7 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
 		podInformer.Informer(),
 		configMapInformer.Informer(),
 		subscriptionInformer.Informer(),
+		installPlanInformer.Informer(),
 	}
 
 	// register listers
@@ -509,6 +511,7 @@ func NewFakeOperator(clientObjs []runtime.Object, k8sObjs []runtime.Object, extO
 	lister.CoreV1().RegisterPodLister(namespace, podInformer.Lister())
 	lister.CoreV1().RegisterConfigMapLister(namespace, configMapInformer.Lister())
 	lister.OperatorsV1alpha1().RegisterSubscriptionLister(namespace, subscriptionInformer.Lister())
+	lister.OperatorsV1alpha1().RegisterInstallPlanLister(namespace, installPlanInformer.Lister())
 
 	// Create the new operator
 	queueOperator, err := queueinformer.NewOperatorFromClient(opClientFake, logrus.New())

vendor/modules.txt

@@ -71,7 +71,7 @@ github.com/gogo/protobuf/gogoproto
 github.com/gogo/protobuf/protoc-gen-gogo/descriptor
 # github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 github.com/golang/glog
-# github.com/golang/groupcache v0.0.0-20181024230925-c65c006176ff
+# github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef
 github.com/golang/groupcache/lru
 # github.com/golang/mock v1.1.1
 github.com/golang/mock/mockgen
@@ -336,20 +336,21 @@ k8s.io/apimachinery/pkg/api/equality
 k8s.io/apimachinery/pkg/api/validation
 k8s.io/apimachinery/pkg/util/yaml
 k8s.io/apimachinery/pkg/util/framer
+k8s.io/apimachinery/pkg/util/rand
 k8s.io/apimachinery/pkg/apis/meta/v1beta1
 k8s.io/apimachinery/pkg/util/mergepatch
 k8s.io/apimachinery/third_party/forked/golang/json
 k8s.io/apimachinery/pkg/api/validation/path
 k8s.io/apimachinery/pkg/apis/meta/v1/validation
 k8s.io/apimachinery/pkg/util/uuid
 k8s.io/apimachinery/third_party/forked/golang/reflect
-k8s.io/apimachinery/pkg/util/rand
 # k8s.io/apiserver v0.0.0-20181026151315-13cfe3978170
 k8s.io/apiserver/pkg/server
 k8s.io/apiserver/pkg/util/logs
 k8s.io/apiserver/pkg/authentication/serviceaccount
 k8s.io/apiserver/pkg/authentication/user
 k8s.io/apiserver/pkg/authorization/authorizer
+k8s.io/apiserver/pkg/storage/names
 k8s.io/apiserver/pkg/endpoints/openapi
 k8s.io/apiserver/pkg/registry/rest
 k8s.io/apiserver/pkg/server/options
@@ -379,7 +380,6 @@ k8s.io/apiserver/pkg/server/mux
 k8s.io/apiserver/pkg/server/routes
 k8s.io/apiserver/pkg/server/storage
 k8s.io/apiserver/pkg/util/feature
-k8s.io/apiserver/pkg/storage/names
 k8s.io/apiserver/pkg/admission/initializer
 k8s.io/apiserver/pkg/admission/metrics
 k8s.io/apiserver/pkg/apis/apiserver