Repository: operator-framework/operator-lifecycle-manager
Pull request #1596 (2020-06-24 15:33:23)

Replace assertions within async assertions.
Functions passed to Gomega's async assertions (Eventually/Consistently) are called repeatedly until the first return value satisfies the given matcher and any other return values are their respective type's zero value.

In several cases, functions passed to async assertions are themselves directly making assertions. If one of those fails, it can cause the entire test to fail immediately rather than continue retrying the async condition.


openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/1596#" title="Author self-approved">benluddy</a>*

The full list of commands accepted by this bot can be found [here](?repo=operator-framework%2Foperator-lifecycle-manager).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/OWNERS)~~ [benluddy]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/controller/bundle/bundle_unpacker.go b/pkg/controller/bundle/bundle_unpacker.go
index bfca2f92d..a76427a03 100644
--- a/pkg/controller/bundle/bundle_unpacker.go
+++ b/pkg/controller/bundle/bundle_unpacker.go
@@ -34,27 +34,6 @@ func (b *BundleUnpackResult) Bundle() *api.Bundle {
 	return b.bundle
 }
 
-func (b *BundleUnpackResult) Name() string {
-	return b.name
-}
-
-// SetCondition replaces the existing BundleLookupCondition of the same type, or adds it if it was not found.
-func (b *BundleUnpackResult) SetCondition(cond operatorsv1alpha1.BundleLookupCondition) operatorsv1alpha1.BundleLookupCondition {
-	for i, existing := range b.Conditions {
-		if existing.Type != cond.Type {
-			continue
-		}
-		if existing.Status == cond.Status && existing.Reason == cond.Reason {
-			cond.LastTransitionTime = existing.LastTransitionTime
-		}
-		b.Conditions[i] = cond
-		return cond
-	}
-	b.Conditions = append(b.Conditions, cond)
-
-	return cond
-}
-
 var catalogSourceGVK = operatorsv1alpha1.SchemeGroupVersion.WithKind(operatorsv1alpha1.CatalogSourceKind)
 
 func newBundleUnpackResult(lookup *operatorsv1alpha1.BundleLookup) *BundleUnpackResult {
@@ -272,26 +251,16 @@ const (
 	CatalogSourceMissingMessage = "referenced catalogsource not found"
 	JobIncompleteReason         = "JobIncomplete"
 	JobIncompleteMessage        = "unpack job not completed"
-	JobNotStartedReason         = "JobNotStarted"
-	JobNotStartedMessage        = "unpack job not yet started"
-	NotUnpackedReason           = "BundleNotUnpacked"
-	NotUnpackedMessage          = "bundle contents have not yet been persisted to installplan status"
 )
 
 func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup) (result *BundleUnpackResult, err error) {
 	result = newBundleUnpackResult(lookup)
-
-	// if pending condition is missing, bundle has already been unpacked
 	cond := result.GetCondition(operatorsv1alpha1.BundleLookupPending)
-	if cond.Status == corev1.ConditionUnknown {
-		return result, nil
-	}
-
 	now := c.now()
 
 	var cs *operatorsv1alpha1.CatalogSource
 	if cs, err = c.csLister.CatalogSources(result.CatalogSourceRef.Namespace).Get(result.CatalogSourceRef.Name); err != nil {
-		if apierrors.IsNotFound(err) && cond.Reason != CatalogSourceMissingReason {
+		if apierrors.IsNotFound(err) && cond.Status != corev1.ConditionTrue && cond.Reason != CatalogSourceMissingReason {
 			cond.Status = corev1.ConditionTrue
 			cond.Reason = CatalogSourceMissingReason
 			cond.Message = CatalogSourceMissingMessage
@@ -335,7 +304,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
 		return
 	}
 
-	if !jobConditionTrue(job, batchv1.JobComplete) && (cond.Status != corev1.ConditionTrue || cond.Reason != JobIncompleteReason) {
+	if !jobConditionTrue(job, batchv1.JobComplete) && cond.Status != corev1.ConditionTrue && cond.Reason != JobIncompleteReason {
 		cond.Status = corev1.ConditionTrue
 		cond.Reason = JobIncompleteReason
 		cond.Message = JobIncompleteMessage
@@ -350,10 +319,6 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup)
 		return
 	}
 
-	if result.Bundle() == nil || len(result.Bundle().GetObject()) == 0 {
-		return
-	}
-
 	// A successful load should remove the pending condition
 	result.RemoveCondition(operatorsv1alpha1.BundleLookupPending)
 
diff --git a/pkg/controller/operators/catalog/manifests.go b/pkg/controller/operators/catalog/manifests.go
deleted file mode 100644
index d3b4e03b2..000000000
--- a/pkg/controller/operators/catalog/manifests.go
+++ /dev/null
@@ -1,109 +0,0 @@
-package catalog
-
-import (
-	"encoding/json"
-	"fmt"
-
-	"github.com/operator-framework/api/pkg/operators/v1alpha1"
-	"github.com/operator-framework/operator-registry/pkg/configmap"
-	errorwrap "github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
-	v1 "k8s.io/client-go/listers/core/v1"
-
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
-)
-
-// ManifestResolver can dereference a manifest for a step. Steps may embed manifests directly or reference content
-// in configmaps
-type ManifestResolver interface {
-	ManifestForStep(step *v1alpha1.Step) (string, error)
-}
-
-// manifestResolver caches manifest from unpacked bundles (via configmaps)
-type manifestResolver struct {
-	configMapLister v1.ConfigMapLister
-	unpackedSteps   map[string][]v1alpha1.StepResource
-	namespace       string
-	logger          logrus.FieldLogger
-}
-
-func newManifestResolver(namespace string, configMapLister v1.ConfigMapLister, logger logrus.FieldLogger) *manifestResolver {
-	return &manifestResolver{
-		namespace:       namespace,
-		configMapLister: configMapLister,
-		unpackedSteps:   map[string][]v1alpha1.StepResource{},
-		logger:          logger,
-	}
-}
-
-// ManifestForStep always returns the manifest that should be applied to the cluster for a given step
-// the manifest field in the installplan status can contain a reference to a configmap instead
-func (r *manifestResolver) ManifestForStep(step *v1alpha1.Step) (string, error) {
-	manifest := step.Resource.Manifest
-	ref := refForStep(step, r.logger)
-	if ref == nil {
-		return manifest, nil
-	}
-
-	log := r.logger.WithFields(logrus.Fields{"resolving": step.Resolving, "step": step.Resource.Name})
-	log.WithField("ref", ref).Debug("step is a reference to configmap")
-
-	usteps, err := r.unpackedStepsForBundle(step.Resolving, ref)
-	if err != nil  {
-		return "", err
-	}
-
-	log.Debugf("checking cache for unpacked step")
-	// need to find the real manifest from the unpacked steps
-	for _, u := range usteps {
-		if u.Name == step.Resource.Name &&
-			u.Kind == step.Resource.Kind &&
-			u.Version == step.Resource.Version &&
-			u.Group == step.Resource.Group {
-			manifest = u.Manifest
-			log.WithField("manifest", manifest).Debug("step replaced with unpacked value")
-			break
-		}
-	}
-	if manifest == step.Resource.Manifest {
-		return "", fmt.Errorf("couldn't find unpacked step for %v", step)
-	}
-	return manifest, nil
-}
-
-func (r *manifestResolver) unpackedStepsForBundle(bundleName string, ref *UnpackedBundleReference) ([]v1alpha1.StepResource, error) {
-	usteps, ok := r.unpackedSteps[bundleName]
-	if ok {
-		return usteps, nil
-	}
-	cm, err := r.configMapLister.ConfigMaps(ref.Namespace).Get(ref.Name)
-	if err != nil {
-		return nil, errorwrap.Wrapf(err, "error finding unpacked bundle configmap for ref %v", *ref)
-	}
-	loader := configmap.NewBundleLoader()
-	bundle, err := loader.Load(cm)
-	if err != nil {
-		return nil, errorwrap.Wrapf(err, "error loading unpacked bundle configmap for ref %v", *ref)
-	}
-	steps, err := resolver.NewStepResourceFromBundle(bundle, r.namespace, ref.Replaces, ref.CatalogSourceName, ref.CatalogSourceNamespace)
-	if err != nil {
-		return nil, errorwrap.Wrapf(err, "error calculating steps for ref %v", *ref)
-	}
-	r.unpackedSteps[bundleName] = steps
-	return steps, nil
-}
-
-func refForStep(step *v1alpha1.Step, log logrus.FieldLogger) *UnpackedBundleReference {
-	log = log.WithFields(logrus.Fields{"resolving": step.Resolving, "step": step.Resource.Name})
-	var ref UnpackedBundleReference
-	if err := json.Unmarshal([]byte(step.Resource.Manifest), &ref); err != nil {
-		log.Debug("step is not a reference to an unpacked bundle (this is not an error if the step is a manifest)")
-		return nil
-	}
-	log = log.WithField("ref", ref)
-	if ref.Kind != "ConfigMap" || ref.Name == "" || ref.Namespace == "" || ref.CatalogSourceName == "" || ref.CatalogSourceNamespace == "" {
-		log.Debug("step is not a reference to an unpacked bundle (this is not an error if the step is a manifest)")
-		return nil
-	}
-	return &ref
-}
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
index c2d5ea533..dedbef13b 100644
--- a/pkg/controller/operators/catalog/operator.go
+++ b/pkg/controller/operators/catalog/operator.go
@@ -1105,16 +1105,6 @@ func (o *Operator) createInstallPlan(namespace string, gen int, subs []*v1alpha1
 	return reference.GetReference(res)
 }
 
-type UnpackedBundleReference struct {
-	Kind                   string `json:"kind"`
-	Name                   string `json:"name"`
-	Namespace              string `json:"namespace"`
-	CatalogSourceName      string `json:"catalogSourceName"`
-	CatalogSourceNamespace string `json:"catalogSourceNamespace"`
-	Replaces               string `json:"replaces"`
-}
-
-// unpackBundles makes one walk through the bundlelookups and attempts to progress them
 func (o *Operator) unpackBundles(plan *v1alpha1.InstallPlan) (bool, *v1alpha1.InstallPlan, error) {
 	out := plan.DeepCopy()
 	unpacked := true
@@ -1127,53 +1117,32 @@ func (o *Operator) unpackBundles(plan *v1alpha1.InstallPlan) (bool, *v1alpha1.In
 			errs = append(errs, err)
 			continue
 		}
-		out.Status.BundleLookups[i] = *res.BundleLookup
 
-		// if pending condition is present, still waiting for the job to unpack to configmap
-		if res.GetCondition(v1alpha1.BundleLookupPending).Status == corev1.ConditionTrue {
+		if res == nil {
 			unpacked = false
 			continue
 		}
 
-		// if packed condition is missing, bundle has already been unpacked into steps, continue
-		if res.GetCondition(resolver.BundleLookupConditionPacked).Status == corev1.ConditionUnknown {
+		out.Status.BundleLookups[i] = *res.BundleLookup
+		if res.Bundle() == nil || len(res.Bundle().GetObject()) == 0 {
+			unpacked = false
 			continue
 		}
 
-		// Ensure that bundle can be applied by the current version of OLM by converting to steps
 		steps, err := resolver.NewStepsFromBundle(res.Bundle(), out.GetNamespace(), res.Replaces, res.CatalogSourceRef.Name, res.CatalogSourceRef.Namespace)
 		if err != nil {
-			errs = append(errs, fmt.Errorf("failed to turn bundle into steps: %v", err))
+			errs = append(errs, fmt.Errorf("failed to turn bundle into steps: %s", err.Error()))
 			unpacked = false
 			continue
 		}
 
-		// step manifests are replaced with references to the configmap containing them
-		for i, s := range steps {
-			ref := UnpackedBundleReference{
-				Kind:                   "ConfigMap",
-				Namespace:              res.CatalogSourceRef.Namespace,
-				Name:                   res.Name(),
-				CatalogSourceName:      res.CatalogSourceRef.Name,
-				CatalogSourceNamespace: res.CatalogSourceRef.Namespace,
-				Replaces:               res.Replaces,
-			}
-			r, err := json.Marshal(&ref)
-			if err != nil {
-				errs = append(errs, fmt.Errorf("failed to generate reference for configmap: %v", err))
-				unpacked = false
-				continue
-			}
-			s.Resource.Manifest = string(r)
-			steps[i] = s
-		}
-		res.RemoveCondition(resolver.BundleLookupConditionPacked)
-		out.Status.BundleLookups[i] = *res.BundleLookup
+		// Add steps and remove resolved bundle lookup
 		out.Status.Plan = append(out.Status.Plan, steps...)
+		out.Status.BundleLookups = append(out.Status.BundleLookups[:i], out.Status.BundleLookups[i+1:]...)
+		i--
 	}
 
 	if err := utilerrors.NewAggregate(errs); err != nil {
-		o.logger.Debugf("failed to unpack bundles: %v", err)
 		return false, nil, err
 	}
 
@@ -1447,8 +1416,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 	}
 
 	ensurer := newStepEnsurer(kubeclient, crclient, dynamicClient)
-	r := newManifestResolver(plan.GetNamespace(), o.lister.CoreV1().ConfigMapLister(), o.logger)
-	b := newBuilder(kubeclient, dynamicClient, o.csvProvidedAPIsIndexer, r, o.logger)
+	b := newBuilder(kubeclient, dynamicClient, o.csvProvidedAPIsIndexer, o.logger)
 
 	for i, step := range plan.Status.Plan {
 		doStep := true
@@ -1475,16 +1443,12 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 		case v1alpha1.StepStatusPresent, v1alpha1.StepStatusCreated, v1alpha1.StepStatusWaitingForAPI:
 			continue
 		case v1alpha1.StepStatusUnknown, v1alpha1.StepStatusNotPresent:
-			manifest, err := r.ManifestForStep(step)
-			if err != nil {
-				return err
-			}
 			o.logger.WithFields(logrus.Fields{"kind": step.Resource.Kind, "name": step.Resource.Name}).Debug("execute resource")
 			switch step.Resource.Kind {
 			case v1alpha1.ClusterServiceVersionKind:
 				// Marshal the manifest into a CSV instance.
 				var csv v1alpha1.ClusterServiceVersion
-				err := json.Unmarshal([]byte(manifest), &csv)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &csv)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1517,7 +1481,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case v1alpha1.SubscriptionKind:
 				// Marshal the manifest into a subscription instance.
 				var sub v1alpha1.Subscription
-				err := json.Unmarshal([]byte(manifest), &sub)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &sub)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1541,7 +1505,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 
 			case resolver.BundleSecretKind:
 				var s corev1.Secret
-				err := json.Unmarshal([]byte(manifest), &s)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &s)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1580,7 +1544,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case clusterRoleKind:
 				// Marshal the manifest into a ClusterRole instance.
 				var cr rbacv1.ClusterRole
-				err := json.Unmarshal([]byte(manifest), &cr)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &cr)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1595,7 +1559,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case clusterRoleBindingKind:
 				// Marshal the manifest into a RoleBinding instance.
 				var rb rbacv1.ClusterRoleBinding
-				err := json.Unmarshal([]byte(manifest), &rb)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &rb)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1610,7 +1574,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case roleKind:
 				// Marshal the manifest into a Role instance.
 				var r rbacv1.Role
-				err := json.Unmarshal([]byte(manifest), &r)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &r)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1633,7 +1597,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case roleBindingKind:
 				// Marshal the manifest into a RoleBinding instance.
 				var rb rbacv1.RoleBinding
-				err := json.Unmarshal([]byte(manifest), &rb)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &rb)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1656,7 +1620,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case serviceAccountKind:
 				// Marshal the manifest into a ServiceAccount instance.
 				var sa corev1.ServiceAccount
-				err := json.Unmarshal([]byte(manifest), &sa)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &sa)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1679,7 +1643,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 			case serviceKind:
 				// Marshal the manifest into a Service instance
 				var s corev1.Service
-				err := json.Unmarshal([]byte(manifest), &s)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &s)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1709,7 +1673,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 
 			case configMapKind:
 				var cfg corev1.ConfigMap
-				err := json.Unmarshal([]byte(manifest), &cfg)
+				err := json.Unmarshal([]byte(step.Resource.Manifest), &cfg)
 				if err != nil {
 					return errorwrap.Wrapf(err, "error parsing step manifest: %s", step.Resource.Name)
 				}
@@ -1745,7 +1709,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 				}
 
 				// Marshal the manifest into an unstructured object
-				dec := yaml.NewYAMLOrJSONDecoder(strings.NewReader(manifest), 10)
+				dec := yaml.NewYAMLOrJSONDecoder(strings.NewReader(step.Resource.Manifest), 10)
 				unstructuredObject := &unstructured.Unstructured{}
 				if err := dec.Decode(unstructuredObject); err != nil {
 					return errorwrap.Wrapf(err, "error decoding %s object to an unstructured object", step.Resource.Name)
diff --git a/pkg/controller/operators/catalog/step.go b/pkg/controller/operators/catalog/step.go
index 99e2910e7..958db78fa 100644
--- a/pkg/controller/operators/catalog/step.go
+++ b/pkg/controller/operators/catalog/step.go
@@ -38,17 +38,15 @@ func (s StepperFunc) Status() (v1alpha1.StepStatus, error) {
 type builder struct {
 	opclient          operatorclient.ClientInterface
 	dynamicClient     dynamic.Interface
-	manifestResolver  ManifestResolver
 	csvToProvidedAPIs map[string]cache.Indexer
 	logger            logger.FieldLogger
 }
 
 func newBuilder(opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, csvToProvidedAPIs map[string]cache.Indexer,
-	manifestResolver ManifestResolver, logger logger.FieldLogger) *builder {
+	logger logger.FieldLogger) *builder {
 	return &builder{
 		opclient:          opclient,
 		dynamicClient:     dynamicClient,
-		manifestResolver:  manifestResolver,
 		csvToProvidedAPIs: csvToProvidedAPIs,
 		logger:            logger,
 	}
@@ -64,15 +62,9 @@ func (n notSupportedStepperErr) Error() string {
 
 // step is a factory that creates StepperFuncs based on the install plan step Kind.
 func (b *builder) create(step *v1alpha1.Step) (Stepper, error) {
-	manifest, err := b.manifestResolver.ManifestForStep(step)
-	if err != nil {
-		return nil, err
-	}
-	step.Resource.Manifest = manifest
-
 	switch step.Resource.Kind {
 	case crdKind:
-		version, err := crdlib.Version(&manifest)
+		version, err := crdlib.Version(&step.Resource.Manifest)
 		if err != nil {
 			return nil, err
 		}
diff --git a/pkg/controller/registry/resolver/rbac.go b/pkg/controller/registry/resolver/rbac.go
index d29c80a09..957626235 100644
--- a/pkg/controller/registry/resolver/rbac.go
+++ b/pkg/controller/registry/resolver/rbac.go
@@ -2,30 +2,19 @@ package resolver
 
 import (
 	"fmt"
-	"hash/fnv"
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	utilrand "k8s.io/apimachinery/pkg/util/rand"
+	"k8s.io/apiserver/pkg/storage/names"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
-	hashutil "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubernetes/pkg/util/hash"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
 )
 
-const maxNameLength = 63
-
-func generateName(base string, o interface{}) string {
-	hasher := fnv.New32a()
-	hashutil.DeepHashObject(hasher, o)
-	hash := utilrand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
-	if len(base)+len(hash) > maxNameLength {
-		base = base[:maxNameLength - len(hash) - 1]
-	}
-
-	return fmt.Sprintf("%s-%s", base, hash)
+var generateName = func(base string) string {
+	return names.SimpleNameGenerator.GenerateName(base + "-")
 }
 
 type OperatorPermissions struct {
@@ -93,7 +82,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 		// Create Role
 		role := &rbacv1.Role{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:            generateName(csv.GetName(), permission.Rules),
+				Name:            generateName(csv.GetName()),
 				Namespace:       csv.GetNamespace(),
 				OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)},
 				Labels:          ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind),
@@ -105,7 +94,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 		// Create RoleBinding
 		roleBinding := &rbacv1.RoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:     		 generateName(role.GetName(), role),
+				Name:            generateName(fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName)),
 				Namespace:       csv.GetNamespace(),
 				OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(csv)},
 				Labels:          ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind),
@@ -137,7 +126,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 		// Create ClusterRole
 		role := &rbacv1.ClusterRole{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:   generateName(csv.GetName(), permission.Rules),
+				Name:   generateName(csv.GetName()),
 				Labels: ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind),
 			},
 			Rules: permission.Rules,
@@ -147,7 +136,7 @@ func RBACForClusterServiceVersion(csv *v1alpha1.ClusterServiceVersion) (map[stri
 		// Create ClusterRoleBinding
 		roleBinding := &rbacv1.ClusterRoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      generateName(role.GetName(), role),
+				Name:      generateName(fmt.Sprintf("%s-%s", role.GetName(), permission.ServiceAccountName)),
 				Namespace: csv.GetNamespace(),
 				Labels:    ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind),
 			},
diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go
index 0f70de9c6..ea551ee25 100644
--- a/pkg/controller/registry/resolver/resolver.go
+++ b/pkg/controller/registry/resolver/resolver.go
@@ -16,14 +16,9 @@ import (
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
 	v1alpha1listers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
-	controllerbundle "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
 )
 
-const (
-	BundleLookupConditionPacked v1alpha1.BundleLookupConditionType = "BundleLookupNotPersisted"
-)
-
 var timeNow = func() metav1.Time { return metav1.NewTime(time.Now().UTC()) }
 
 type Resolver interface {
@@ -122,20 +117,6 @@ func (r *OperatorsV1alpha1Resolver) ResolveSteps(namespace string, sourceQuerier
 						Namespace: op.SourceInfo().Catalog.Namespace,
 						Name:      op.SourceInfo().Catalog.Name,
 					},
-					Conditions: []v1alpha1.BundleLookupCondition{
-						{
-							Type:    BundleLookupConditionPacked,
-							Status:  corev1.ConditionTrue,
-							Reason:  controllerbundle.NotUnpackedReason,
-							Message: controllerbundle.NotUnpackedMessage,
-						},
-						{
-							Type:    v1alpha1.BundleLookupPending,
-							Status:  corev1.ConditionTrue,
-							Reason:  controllerbundle.JobNotStartedReason,
-							Message: controllerbundle.JobNotStartedMessage,
-						},
-					},
 				})
 			}
 


Test patch:
diff --git a/pkg/controller/bundle/bundle_unpacker_test.go b/pkg/controller/bundle/bundle_unpacker_test.go
index e81653182..f0643ef92 100644
--- a/pkg/controller/bundle/bundle_unpacker_test.go
+++ b/pkg/controller/bundle/bundle_unpacker_test.go
@@ -71,14 +71,6 @@ func TestConfigMapUnpacker(t *testing.T) {
 						Namespace: "ns-a",
 						Name:      "src-a",
 					},
-					Conditions: []operatorsv1alpha1.BundleLookupCondition{
-						{
-							Type:    operatorsv1alpha1.BundleLookupPending,
-							Status:  corev1.ConditionTrue,
-							Reason:  JobNotStartedReason,
-							Message: JobNotStartedMessage,
-						},
-					},
 				},
 			},
 			expected: expected{
@@ -124,14 +116,6 @@ func TestConfigMapUnpacker(t *testing.T) {
 						Namespace: "ns-a",
 						Name:      "src-a",
 					},
-					Conditions: []operatorsv1alpha1.BundleLookupCondition{
-						{
-							Type:    operatorsv1alpha1.BundleLookupPending,
-							Status:  corev1.ConditionTrue,
-							Reason:  JobNotStartedReason,
-							Message: JobNotStartedMessage,
-						},
-					},
 				},
 			},
 			expected: expected{
diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go
index 7902572a7..ee883c508 100644
--- a/pkg/controller/operators/catalog/operator_test.go
+++ b/pkg/controller/operators/catalog/operator_test.go
@@ -269,13 +269,13 @@ func TestExecutePlan(t *testing.T) {
 							Version:                "v1",
 							Kind:                   "ConfigMap",
 							Name:                   "cfg",
-							Manifest:               toManifest(t, configMap("cfg", namespace)),
+							Manifest:               toManifest(t, configmap("cfg", namespace)),
 						},
 						Status: v1alpha1.StepStatusUnknown,
 					},
 				},
 			),
-			want: []runtime.Object{configMap("cfg", namespace)},
+			want: []runtime.Object{configmap("cfg", namespace)},
 			err:  nil,
 		},
 		{
@@ -1307,7 +1307,7 @@ func serviceAccount(name, namespace, generateName string, secretRef *corev1.Obje
 	}
 }
 
-func configMap(name, namespace string) *corev1.ConfigMap {
+func configmap(name, namespace string) *corev1.ConfigMap {
 	return &corev1.ConfigMap{
 		TypeMeta:   metav1.TypeMeta{Kind: configMapKind},
 		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
diff --git a/pkg/controller/registry/resolver/rbac_test.go b/pkg/controller/registry/resolver/rbac_test.go
deleted file mode 100644
index 060f99d75..000000000
--- a/pkg/controller/registry/resolver/rbac_test.go
+++ /dev/null
@@ -1,65 +0,0 @@
-package resolver
-
-import (
-	"github.com/stretchr/testify/require"
-	"math/rand"
-	"reflect"
-	"strings"
-	"testing"
-	"testing/quick"
-)
-
-func TestGenerateName(t *testing.T) {
-	type args struct {
-		base string
-		o    interface{}
-	}
-	tests := []struct {
-		name string
-		args args
-		want string
-	}{
-		{
-			name: "generate",
-			args: args{
-				base: "myname",
-				o: []string{"something"},
-			},
-			want: "myname-9c895f74f",
-		},
-		{
-			name: "truncated",
-			args: args{
-				base: strings.Repeat("name", 100),
-				o: []string{"something", "else"},
-			},
-			want: "namenamenamenamenamenamenamenamenamenamenamenamename-78fd8b4d6b",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := generateName(tt.args.base, tt.args.o)
-			require.Equal(t, tt.want, got)
-			require.LessOrEqual(t, len(got), maxNameLength)
-		})
-	}
-}
-
-var runeSet = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-")
-
-type validKubeName string
-
-func (validKubeName) Generate(rand *rand.Rand, size int) reflect.Value {
-	b := make([]rune, size)
-	for i := range b {
-		b[i] = runeSet[rand.Intn(len(runeSet))]
-	}
-	return reflect.ValueOf(validKubeName(b))
-}
-
-func TestGeneratesWithinRange(t *testing.T) {
-	f := func(base validKubeName, o string) bool {
-		return len(generateName(string(base), o)) <= maxNameLength
-	}
-	require.NoError(t, quick.Check(f, nil))
-}
diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go
index df0195f09..a4d11ce6f 100644
--- a/pkg/controller/registry/resolver/resolver_test.go
+++ b/pkg/controller/registry/resolver/resolver_test.go
@@ -20,7 +20,6 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/informers/externalversions"
-	controllerbundle "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/bundle"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
 )
 
@@ -127,20 +126,6 @@ func TestNamespaceResolver(t *testing.T) {
 							Namespace: catalog.Namespace,
 							Name:      catalog.Name,
 						},
-						Conditions: []v1alpha1.BundleLookupCondition{
-							{
-								Type:    BundleLookupConditionPacked,
-								Status:  corev1.ConditionTrue,
-								Reason:  controllerbundle.NotUnpackedReason,
-								Message: controllerbundle.NotUnpackedMessage,
-							},
-							{
-								Type:    v1alpha1.BundleLookupPending,
-								Status:  corev1.ConditionTrue,
-								Reason:  controllerbundle.JobNotStartedReason,
-								Message: controllerbundle.JobNotStartedMessage,
-							},
-						},
 					},
 				},
 				subs: []*v1alpha1.Subscription{
@@ -256,20 +241,6 @@ func TestNamespaceResolver(t *testing.T) {
 							Namespace: catalog.Namespace,
 							Name:      catalog.Name,
 						},
-						Conditions: []v1alpha1.BundleLookupCondition{
-							{
-								Type:    BundleLookupConditionPacked,
-								Status:  corev1.ConditionTrue,
-								Reason:  controllerbundle.NotUnpackedReason,
-								Message: controllerbundle.NotUnpackedMessage,
-							},
-							{
-								Type:    v1alpha1.BundleLookupPending,
-								Status:  corev1.ConditionTrue,
-								Reason:  controllerbundle.JobNotStartedReason,
-								Message: controllerbundle.JobNotStartedMessage,
-							},
-						},
 					},
 				},
 				subs: []*v1alpha1.Subscription{
@@ -569,6 +540,10 @@ func TestNamespaceResolver(t *testing.T) {
 }
 
 func TestNamespaceResolverRBAC(t *testing.T) {
+	generateName = func(base string) string {
+		return "a"
+	}
+
 	namespace := "catsrc-namespace"
 	catalog := CatalogKey{"catsrc", namespace}
 
diff --git a/test/e2e/crd_e2e_test.go b/test/e2e/crd_e2e_test.go
index da7257e86..47f76ef20 100644
--- a/test/e2e/crd_e2e_test.go
+++ b/test/e2e/crd_e2e_test.go
@@ -3,6 +3,8 @@ package e2e
 import (
 	"context"
 	"fmt"
+	"time"
+
 	"github.com/blang/semver"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -13,7 +15,6 @@ import (
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"time"
 )
 
 var _ = Describe("CRD Versions", func() {
@@ -306,21 +307,33 @@ var _ = Describe("CRD Versions", func() {
 		_, err = crc.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
 		Expect(err).ToNot(HaveOccurred(), "could not update subscription")
 
+		subscriptionAtLatestWithDifferentInstallPlan := func(v *operatorsv1alpha1.Subscription) bool {
+			return subscriptionStateAtLatestChecker(v) && v.Status.InstallPlanRef != nil && v.Status.InstallPlanRef.Name != fetchedInstallPlan.Name
+		}
+
 		// fetch new subscription
-		s, err := fetchSubscription(crc, testNamespace, subscriptionName, subscriptionStateAtLatestChecker)
+		s, err := fetchSubscription(crc, testNamespace, subscriptionName, subscriptionAtLatestWithDifferentInstallPlan)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(s).ToNot(BeNil())
 		Expect(s.Status.InstallPlanRef).ToNot(Equal(nil))
 
 		// Check the error on the installplan - should be related to data loss and the CRD upgrade missing a stored version
-		Eventually(func() bool {
-			ip, err := crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), s.Status.InstallPlanRef.Name, metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred(), "could not get installplan")
-
-			Expect(ip.Status.Phase).To(Equal(operatorsv1alpha1.InstallPlanPhaseFailed))
-			Expect(ip.Status.Conditions[len(ip.Status.Conditions)-1].Message).To(ContainSubstring("risk of data loss"))
-			return true
-		}).Should(BeTrue())
+		Eventually(func() (*operatorsv1alpha1.InstallPlan, error) {
+			return crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), s.Status.InstallPlanRef.Name, metav1.GetOptions{})
+		}).Should(And(
+			WithTransform(
+				func(v *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase {
+					return v.Status.Phase
+				},
+				Equal(operatorsv1alpha1.InstallPlanPhaseFailed),
+			),
+			WithTransform(
+				func(v *operatorsv1alpha1.InstallPlan) string {
+					return v.Status.Conditions[len(v.Status.Conditions)-1].Message
+				},
+				ContainSubstring("risk of data loss"),
+			),
+		))
 	})
 
 	// Create a CRD on cluster with v1alpha1 (storage)
@@ -369,7 +382,9 @@ var _ = Describe("CRD Versions", func() {
 		// wrap CRD update in a poll because of the object has been modified related errors
 		Eventually(func() error {
 			oldCRD, err = c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(context.TODO(), oldCRD.GetName(), metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred(), "error getting old CRD")
+			if err != nil {
+				return err
+			}
 			GinkgoT().Logf("old crd status stored versions: %#v", oldCRD.Status.StoredVersions)
 
 			// set v1alpha1 to no longer served
@@ -448,12 +463,12 @@ var _ = Describe("CRD Versions", func() {
 		Expect(catalogCSV.GetName()).To(Equal(subscription.Status.CurrentCSV))
 
 		// Check the error on the installplan - should be related to data loss and the CRD upgrade missing a stored version (v1alpha1)
-		Eventually(func() bool {
-			ip, err := crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), subscription.Status.InstallPlanRef.Name, metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred(), "could not get installplan")
-
-			return ip.Status.Phase == operatorsv1alpha1.InstallPlanPhaseFailed
-		}).Should(BeTrue())
+		Eventually(func() (*operatorsv1alpha1.InstallPlan, error) {
+			return crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), subscription.Status.InstallPlanRef.Name, metav1.GetOptions{})
+		}).Should(WithTransform(
+			func(v *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase { return v.Status.Phase },
+			Equal(operatorsv1alpha1.InstallPlanPhaseFailed),
+		))
 
 		// update CRD status to remove the v1alpha1 stored version
 		newCRD, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(context.TODO(), oldCRD.GetName(), metav1.GetOptions{})
diff --git a/test/e2e/gc_e2e_test.go b/test/e2e/gc_e2e_test.go
index 18430b89e..f2a12a7a9 100644
--- a/test/e2e/gc_e2e_test.go
+++ b/test/e2e/gc_e2e_test.go
@@ -458,16 +458,15 @@ var _ = Describe("Garbage collection for dependent resources", func() {
 			})
 
 			It("OLM should have upgraded associated configmap in place", func() {
-				Eventually(func() bool {
+				Eventually(func() (string, error) {
 					cfg, err := kubeClient.GetConfigMap(testNamespace, configmapName)
 					if err != nil {
-						return false
+						return "", err
 					}
 					// check data in configmap to ensure it is the new data (configmap was updated in the newer bundle)
 					// new value in the configmap is "updated-very-much"
-					data := cfg.Data["special.how"]
-					return data == "updated-very-much"
-				}).Should(BeTrue())
+					return cfg.Data["special.how"], nil
+				}).Should(Equal("updated-very-much"))
 				ctx.Ctx().Logf("dependent successfully updated after csv owner was updated")
 			})
 		})
diff --git a/test/e2e/packagemanifest_e2e_test.go b/test/e2e/packagemanifest_e2e_test.go
index f62945f6d..f2602d32d 100644
--- a/test/e2e/packagemanifest_e2e_test.go
+++ b/test/e2e/packagemanifest_e2e_test.go
@@ -9,7 +9,6 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
 
@@ -264,11 +263,13 @@ var _ = Describe("Package Manifest API lists available Operators from Catalog So
 			})
 			It("should successfully update the CatalogSource field", func() {
 
-				Eventually(func() string {
+				Eventually(func() (string, error) {
 					pm, err := fetchPackageManifest(pmc, testNamespace, packageName,
 						packageManifestHasStatus)
-					Expect(err).NotTo(HaveOccurred(), "error getting package manifest after updating catsrc")
-					return pm.Status.CatalogSourceDisplayName
+					if err != nil {
+						return "", err
+					}
+					return pm.Status.CatalogSourceDisplayName, nil
 				}).Should(Equal(displayName))
 			})
 		})
@@ -286,11 +287,11 @@ func fetchPackageManifest(pmc pmversioned.Interface, namespace, name string, che
 	var fetched *packagev1.PackageManifest
 	var err error
 
-	Eventually(func() (bool, error) {
+	EventuallyWithOffset(1, func() (bool, error) {
 		ctx.Ctx().Logf("Polling...")
 		fetched, err = pmc.OperatorsV1().PackageManifests(namespace).Get(context.TODO(), name, metav1.GetOptions{})
-		if err != nil && !errors.IsNotFound(err) {
-			return true, err
+		if err != nil {
+			return false, err
 		}
 		return check(fetched), nil
 	}).Should(BeTrue())