fix(olm): don't attempt to annotate deployments if the deployment hasn't

yet been created

this should prevent some churn and possible races

Author: ecordell

Makefile

@@ -71,7 +71,7 @@ setup-bare: clean e2e.namespace
 	. ./scripts/install_bare.sh $(shell cat ./e2e.namespace) test/e2e/resources
 
 e2e:
-	go test -v -failfast -timeout 50m ./test/e2e/... -namespace=openshift-operators -kubeconfig=${KUBECONFIG} -olmNamespace=openshift-operator-lifecycle-manager
+	go test -v -failfast -timeout 70m ./test/e2e/... -namespace=openshift-operators -kubeconfig=${KUBECONFIG} -olmNamespace=openshift-operator-lifecycle-manager
 
 e2e-local:
 	. ./scripts/build_local.sh

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

@@ -30,6 +30,20 @@ var uncopiableReasons = map[ConditionReason]struct{}{
 	CSVReasonCannotModifyStaticOperatorGroupProvidedAPIs: {},
 }
 
+// safeToAnnotateOperatorGroupReasons are the set of reasons that it's safe to attempt to update the operatorgroup
+// annotations
+var safeToAnnotateOperatorGroupReasons = map[ConditionReason]struct{}{
+	CSVReasonOwnerConflict:                               {},
+	CSVReasonInstallSuccessful:                           {},
+	CSVReasonInvalidInstallModes:                         {},
+	CSVReasonNoTargetNamespaces:                          {},
+	CSVReasonUnsupportedOperatorGroup:                    {},
+	CSVReasonNoOperatorGroup:                             {},
+	CSVReasonTooManyOperatorGroups:                       {},
+	CSVReasonInterOperatorGroupOwnerConflict:             {},
+	CSVReasonCannotModifyStaticOperatorGroupProvidedAPIs: {},
+}
+
 // SetPhaseWithEventIfChanged emits a Kubernetes event with details of a phase change and sets the current phase if phase, reason, or message would changed
 func (c *ClusterServiceVersion) SetPhaseWithEventIfChanged(phase ClusterServiceVersionPhase, reason ConditionReason, message string, now metav1.Time, recorder record.EventRecorder) {
 	if c.Status.Phase == phase && c.Status.Reason == reason && c.Status.Message == message {
@@ -120,6 +134,11 @@ func (c *ClusterServiceVersion) IsUncopiable() bool {
 	return ok
 }
 
+func (c *ClusterServiceVersion) IsSafeToUpdateOperatorGroupAnnotations() bool {
+	_, ok := safeToAnnotateOperatorGroupReasons[c.Status.Reason]
+	return ok
+}
+
 // NewInstallModeSet returns an InstallModeSet instantiated from the given list of InstallModes.
 // If the given list is not a set, an error is returned.
 func NewInstallModeSet(modes []InstallMode) (InstallModeSet, error) {

pkg/controller/operators/olm/operator.go

@@ -262,7 +262,7 @@ func NewOperator(logger *logrus.Logger, crClient versioned.Interface, opClient o
 	op.RegisterQueueIndexer(csvQueueIndexer)
 	op.copyQueueIndexer = csvQueueIndexer
 
-	// Register separate queue for copying csvs
+	// Register separate queue for gcing csvs
 	csvGCQueue := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "csvGC")
 	csvGCQueueIndexer := queueinformer.NewQueueIndexer(csvGCQueue, csvIndexes, op.syncGcCsv, "csvGC", logger, metrics.NewMetricsNil())
 	op.RegisterQueueIndexer(csvGCQueueIndexer)
@@ -513,7 +513,7 @@ func (a *Operator) removeDanglingChildCSVs(csv *v1alpha1.ClusterServiceVersion)
 
 func (a *Operator) deleteChild(csv *v1alpha1.ClusterServiceVersion, logger *logrus.Entry) error {
 	logger.Debug("gcing csv")
-	return a.client.OperatorsV1alpha1().ClusterServiceVersions(csv.GetNamespace()).Delete(csv.GetName(), &metav1.DeleteOptions{})
+	return a.client.OperatorsV1alpha1().ClusterServiceVersions(csv.GetNamespace()).Delete(csv.GetName(), metav1.NewDeleteOptions(0))
 }
 
 // syncClusterServiceVersion is the method that gets called when we see a CSV event in the cluster
@@ -753,7 +753,6 @@ func (a *Operator) transitionCSVState(in v1alpha1.ClusterServiceVersion) (out *v
 		return
 	}
 
-	logger.Info("updated annotations to match current operatorgroup")
 	if err := a.ensureDeploymentAnnotations(logger, out); err != nil {
 		return nil, err
 	}
@@ -1345,6 +1344,10 @@ func (a *Operator) cleanupCSVDeployments(logger *logrus.Entry, csv *v1alpha1.Clu
 }
 
 func (a *Operator) ensureDeploymentAnnotations(logger *logrus.Entry, csv *v1alpha1.ClusterServiceVersion) error {
+	if !csv.IsSafeToUpdateOperatorGroupAnnotations() {
+		return nil
+	}
+
 	// Get csv operatorgroup annotations
 	annotations := a.copyOperatorGroupAnnotations(&csv.ObjectMeta)
 
@@ -1376,7 +1379,6 @@ func (a *Operator) ensureDeploymentAnnotations(logger *logrus.Entry, csv *v1alph
 	for _, d := range existingDeployments {
 		existingMap[d.GetName()] = d
 	}
-
 	updateErrs := []error{}
 	for _, dep := range existingMap {
 		if dep.Spec.Template.Annotations == nil {
@@ -1389,5 +1391,7 @@ func (a *Operator) ensureDeploymentAnnotations(logger *logrus.Entry, csv *v1alph
 			updateErrs = append(updateErrs, err)
 		}
 	}
+	logger.Info("updated annotations to match current operatorgroup")
+
 	return utilerrors.NewAggregate(updateErrs)
 }

scripts/run_e2e_local.sh

@@ -58,4 +58,4 @@ else
 fi
 
 echo "${test_flags}"
-go test -mod=vendor -count=1 -failfast -tags=local -covermode=count -coverpkg ./pkg/controller/...  -test.v -test.timeout 20m ${test_flags} ./test/e2e/... -kubeconfig=${KUBECONFIG:-~/.kube/config} -namespace=${namespace}-operator -olmNamespace=${namespace}
+go test -mod=vendor -count=1 -failfast -tags=local -covermode=count -coverpkg ./pkg/controller/...  -test.v -test.timeout 30m ${test_flags} ./test/e2e/... -kubeconfig=${KUBECONFIG:-~/.kube/config} -namespace=${namespace}-operator -olmNamespace=${namespace}