feat(resolver): treat the registry as the source of truth for

replaces fields

this lets the registry tell olm which operators replace which others

Author: ecordell

pkg/controller/operators/olm/operator.go

@@ -1379,7 +1379,7 @@ func (a *Operator) isReplacing(in *v1alpha1.ClusterServiceVersion) *v1alpha1.Clu
 	}
 
 	// using the client instead of a lister; missing an object because of a cache sync can cause upgrades to fail
-	previous, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(in.GetNamespace()).Get(in.Spec.Replaces)
+	previous, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(in.GetNamespace()).Get(in.Spec.Replaces, metav1.GetOptions{})
 	if err != nil {
 		a.Log.WithField("replacing", in.Spec.Replaces).WithError(err).Debugf("unable to get previous csv")
 		return nil

pkg/controller/registry/resolver/evolver.go

@@ -62,14 +62,12 @@ func (e *NamespaceGenerationEvolver) checkForUpdates() error {
 			continue
 		}
 
-		o, err := NewOperatorFromBundle(bundle, op.SourceInfo().StartingCSV, *key)
+		o, err := NewOperatorFromBundle(bundle, op.Identifier(), op.SourceInfo().StartingCSV, *key)
 		if err != nil {
 			return errors.Wrap(err, "error parsing bundle")
 		}
 		if err := e.gen.AddOperator(o); err != nil {
-			if err != nil {
-				return errors.Wrap(err, "error calculating generation changes due to new bundle")
-			}
+			return errors.Wrap(err, "error calculating generation changes due to new bundle")
 		}
 		e.gen.RemoveOperator(op)
 	}
@@ -91,7 +89,7 @@ func (e *NamespaceGenerationEvolver) addNewOperators(add map[OperatorSourceInfo]
 			return errors.Wrapf(err, "%s not found", s)
 		}
 
-		o, err := NewOperatorFromBundle(bundle, s.StartingCSV, *key)
+		o, err := NewOperatorFromBundle(bundle, "", s.StartingCSV, *key)
 		if err != nil {
 			return errors.Wrap(err, "error parsing bundle")
 		}
@@ -117,7 +115,7 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
 		// attempt to find a bundle that provides that api
 		if bundle, key, err := e.querier.FindProvider(*api); err == nil {
 			// add a bundle that provides the api to the generation
-			o, err := NewOperatorFromBundle(bundle, "", *key)
+			o, err := NewOperatorFromBundle(bundle, "", "", *key)
 			if err != nil {
 				return errors.Wrap(err, "error parsing bundle")
 			}

pkg/controller/registry/resolver/evolver_test.go

@@ -394,6 +394,22 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
 				NewFakeOperatorSurface("updated", "o", "c", "original", "catsrc", "", nil, nil, nil, nil),
 			),
 		},
+		{
+			// an existing operator has an update available and skips previous versions via channel head annotations
+			name: "UpdateRequired/SkipVersions",
+			fields: fields{
+				querier: NewFakeSourceQuerierCustomReplacement(CatalogKey{"catsrc", "catsrc-namespace"}, bundle("updated.v3", "o", "c", "updated.v2", nil, nil, nil, nil)),
+				gen: NewGenerationFromOperators(
+					NewFakeOperatorSurface("original", "o", "c", "", "catsrc", "", nil, nil, nil, nil),
+				),
+			},
+			args: args{},
+			wantGen: NewGenerationFromOperators(
+				// the csv in the bundle still has the original replaces field, but the surface has the value overridden
+				withReplaces(NewFakeOperatorSurface("updated.v3", "o", "c", "updated.v2", "catsrc", "", nil, nil, nil, nil),
+					"original"),
+			),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/controller/registry/resolver/operators.go

@@ -234,7 +234,7 @@ type Operator struct {
 
 var _ OperatorSurface = &Operator{}
 
-func NewOperatorFromBundle(bundle *opregistry.Bundle, startingCSV string, sourceKey CatalogKey) (*Operator, error) {
+func NewOperatorFromBundle(bundle *opregistry.Bundle, replaces string, startingCSV string, sourceKey CatalogKey) (*Operator, error) {
 	csv, err := bundle.ClusterServiceVersion()
 	if err != nil {
 		return nil, err
@@ -247,9 +247,13 @@ func NewOperatorFromBundle(bundle *opregistry.Bundle, startingCSV string, source
 	if err != nil {
 		return nil, err
 	}
+	r := replaces
+	if r == "" {
+		r = csv.Spec.Replaces
+	}
 	return &Operator{
 		name:         csv.GetName(),
-		replaces:     csv.Spec.Replaces,
+		replaces:     r,
 		version:      &csv.Spec.Version.Version,
 		providedAPIs: providedAPIs,
 		requiredAPIs: requiredAPIs,

pkg/controller/registry/resolver/operators_test.go

@@ -898,6 +898,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
 			Namespace: "testNamespace",
 		},
 		Spec: v1alpha1.ClusterServiceVersionSpec{
+			Replaces: "v1",
 			CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{
 				Owned:    []v1alpha1.CRDDescription{},
 				Required: []v1alpha1.CRDDescription{},
@@ -982,6 +983,7 @@ func TestNewOperatorFromBundle(t *testing.T) {
 	type args struct {
 		bundle    *opregistry.Bundle
 		sourceKey CatalogKey
+		replaces  string
 	}
 	tests := []struct {
 		name    string
@@ -994,10 +996,12 @@ func TestNewOperatorFromBundle(t *testing.T) {
 			args: args{
 				bundle:    bundleNoAPIs,
 				sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
+				replaces:  "",
 			},
 			want: &Operator{
 				name:         "testCSV",
 				version:      &version.Version,
+				replaces:     "v1",
 				providedAPIs: EmptyAPISet(),
 				requiredAPIs: EmptyAPISet(),
 				bundle:       bundleNoAPIs,
@@ -1013,10 +1017,12 @@ func TestNewOperatorFromBundle(t *testing.T) {
 			args: args{
 				bundle:    bundleWithAPIs,
 				sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
+				replaces:  "",
 			},
 			want: &Operator{
-				name:    "testCSV",
-				version: &version.Version,
+				name:     "testCSV",
+				version:  &version.Version,
+				replaces: "v1",
 				providedAPIs: APISet{
 					opregistry.APIKey{
 						Group:   "crd.group.com",
@@ -1053,10 +1059,31 @@ func TestNewOperatorFromBundle(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "BundleReplaceOverrides",
+			args: args{
+				bundle:    bundleNoAPIs,
+				sourceKey: CatalogKey{Name: "source", Namespace: "testNamespace"},
+				replaces:  "replaced",
+			},
+			want: &Operator{
+				name:         "testCSV",
+				providedAPIs: EmptyAPISet(),
+				requiredAPIs: EmptyAPISet(),
+				bundle:       bundleNoAPIs,
+				replaces:     "replaced",
+				version:      &version.Version,
+				sourceInfo: &OperatorSourceInfo{
+					Package: "testPackage",
+					Channel: "testChannel",
+					Catalog: CatalogKey{"source", "testNamespace"},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := NewOperatorFromBundle(tt.args.bundle, "", tt.args.sourceKey)
+			got, err := NewOperatorFromBundle(tt.args.bundle, tt.args.replaces, "", tt.args.sourceKey)
 			require.Equal(t, tt.wantErr, err)
 			require.Equal(t, tt.want, got)
 		})

pkg/controller/registry/resolver/resolver.go

@@ -91,7 +91,7 @@ func (r *OperatorsV1alpha1Resolver) ResolveSteps(namespace string, sourceQuerier
 
 		// add steps for any new bundle
 		if op.Bundle() != nil {
-			bundleSteps, err := NewStepResourceFromBundle(op.Bundle(), namespace, op.SourceInfo().Catalog.Name, op.SourceInfo().Catalog.Namespace)
+			bundleSteps, err := NewStepResourceFromBundle(op.Bundle(), namespace, op.Replaces(), op.SourceInfo().Catalog.Name, op.SourceInfo().Catalog.Namespace)
 			if err != nil {
 				return nil, nil, fmt.Errorf("failed to turn bundle into steps")
 			}