Replace solver with simple catalog filtering

Signed-off-by: Mikalai Radchuk <[email protected]>

Author: Mikalai Radchuk

cmd/manager/main.go

@@ -35,7 +35,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
-	"github.com/operator-framework/deppy/pkg/deppy/solver"
 	rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
 
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
@@ -111,17 +110,10 @@ func main() {
 	cl := mgr.GetClient()
 	catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(cachePath, &http.Client{Timeout: 10 * time.Second}))
 
-	resolver, err := solver.New()
-	if err != nil {
-		setupLog.Error(err, "unable to create a solver")
-		os.Exit(1)
-	}
-
 	if err = (&controllers.ClusterExtensionReconciler{
 		Client:         cl,
 		BundleProvider: catalogClient,
 		Scheme:         mgr.GetScheme(),
-		Resolver:       resolver,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension")
 		os.Exit(1)

internal/controllers/clusterextension_controller.go

@@ -19,8 +19,10 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"sort"
 	"strings"
 
+	mmsemver "github.com/Masterminds/semver/v3"
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
@@ -37,22 +39,21 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
-	"github.com/operator-framework/deppy/pkg/deppy"
-	"github.com/operator-framework/deppy/pkg/deppy/solver"
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
 
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/catalogmetadata"
-	olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
+	catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
+	catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
+	"github.com/operator-framework/operator-controller/pkg/features"
 )
 
 // ClusterExtensionReconciler reconciles a ClusterExtension object
 type ClusterExtensionReconciler struct {
 	client.Client
 	BundleProvider BundleProvider
 	Scheme         *runtime.Scheme
-	Resolver       *solver.Solver
 }
 
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch
@@ -116,33 +117,8 @@ func checkForUnexpectedFieldChange(a, b ocv1alpha1.ClusterExtension) bool {
 //
 //nolint:unparam
 func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (ctrl.Result, error) {
-	// gather vars for resolution
-	vars, err := r.variables(ctx)
-	if err != nil {
-		ext.Status.InstalledBundle = nil
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted due to failure to gather data for resolution", ext.GetGeneration())
-		ext.Status.ResolvedBundle = nil
-		setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
-
-		setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted due to failure to gather data for resolution", ext.GetGeneration())
-		return ctrl.Result{}, err
-	}
-
-	// run resolution
-	selection, err := r.Resolver.Solve(vars)
-	if err != nil {
-		ext.Status.InstalledBundle = nil
-		setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as resolution failed", ext.GetGeneration())
-		ext.Status.ResolvedBundle = nil
-		setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
-
-		setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as resolution failed", ext.GetGeneration())
-		return ctrl.Result{}, err
-	}
-
-	// lookup the bundle in the solution that corresponds to the
-	// ClusterExtension's desired package name.
-	bundle, err := r.bundleFromSolution(selection, ext.Spec.PackageName)
+	// Lookup the bundle that corresponds to the ClusterExtension's desired package.
+	bundle, err := r.resolve(ctx, ext)
 	if err != nil {
 		ext.Status.InstalledBundle = nil
 		setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as resolution failed", ext.GetGeneration())
@@ -202,21 +178,133 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	return ctrl.Result{}, nil
 }
 
-func (r *ClusterExtensionReconciler) variables(ctx context.Context) ([]deppy.Variable, error) {
+func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) {
 	allBundles, err := r.BundleProvider.Bundles(ctx)
 	if err != nil {
 		return nil, err
 	}
-	clusterExtensionList := ocv1alpha1.ClusterExtensionList{}
-	if err := r.Client.List(ctx, &clusterExtensionList); err != nil {
+
+	installedBundle, err := r.installedBundle(ctx, allBundles, ext)
+	if err != nil {
+		return nil, err
+	}
+
+	packageName := ext.Spec.PackageName
+	channelName := ext.Spec.Channel
+	versionRange := ext.Spec.Version
+
+	predicates := []catalogfilter.Predicate[catalogmetadata.Bundle]{
+		catalogfilter.WithPackageName(packageName),
+	}
+
+	if channelName != "" {
+		predicates = append(predicates, catalogfilter.InChannel(channelName))
+	}
+
+	if versionRange != "" {
+		vr, err := mmsemver.NewConstraint(versionRange)
+		if err != nil {
+			return nil, fmt.Errorf("invalid version range %q: %w", versionRange, err)
+		}
+		predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr))
+	}
+
+	if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil {
+		upgradePredicate, err := upgradePredicate(installedBundle)
+		if err != nil {
+			return nil, err
+		}
+
+		predicates = append(predicates, upgradePredicate)
+	}
+
+	resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))
+
+	var upgradeErrorPrefix string
+	if installedBundle != nil {
+		installedBundleVersion, err := installedBundle.Version()
+		if err != nil {
+			return nil, err
+		}
+		upgradeErrorPrefix = fmt.Sprintf("error upgrading from currently installed version %q: ", installedBundleVersion.String())
+	}
+	if len(resultSet) == 0 {
+		if versionRange != "" && channelName != "" {
+			return nil, fmt.Errorf("%sno package %q matching version %q found in channel %q", upgradeErrorPrefix, packageName, versionRange, channelName)
+		}
+		if versionRange != "" {
+			return nil, fmt.Errorf("%sno package %q matching version %q found", upgradeErrorPrefix, packageName, versionRange)
+		}
+		if channelName != "" {
+			return nil, fmt.Errorf("%sno package %q found in channel %q", upgradeErrorPrefix, packageName, channelName)
+		}
+		return nil, fmt.Errorf("%sno package %q found", upgradeErrorPrefix, packageName)
+	}
+	sort.SliceStable(resultSet, func(i, j int) bool {
+		return catalogsort.ByVersion(resultSet[i], resultSet[j])
+	})
+	sort.SliceStable(resultSet, func(i, j int) bool {
+		return catalogsort.ByDeprecated(resultSet[i], resultSet[j])
+	})
+
+	return resultSet[0], nil
+}
+
+func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) {
+	bd := &rukpakv1alpha2.BundleDeployment{}
+	err := r.Client.Get(ctx, types.NamespacedName{Name: ext.GetName()}, bd)
+	if client.IgnoreNotFound(err) != nil {
+		return nil, err
+	}
+
+	if bd.Spec.Source.Image == nil || bd.Spec.Source.Image.Ref == "" {
+		// Bundle not yet installed
+		return nil, nil
+	}
+
+	bundleImage := bd.Spec.Source.Image.Ref
+	// find corresponding bundle for the installed content
+	resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(
+		catalogfilter.WithPackageName(ext.Spec.PackageName),
+		catalogfilter.WithBundleImage(bundleImage),
+	))
+	if len(resultSet) == 0 {
+		return nil, fmt.Errorf("bundle with image %q for package %q not found in available catalogs but is currently installed via BundleDeployment %q", bundleImage, ext.Spec.PackageName, bd.Name)
+	}
+
+	sort.SliceStable(resultSet, func(i, j int) bool {
+		return catalogsort.ByVersion(resultSet[i], resultSet[j])
+	})
+
+	return resultSet[0], nil
+}
+
+func upgradePredicate(installedBundle *catalogmetadata.Bundle) (catalogfilter.Predicate[catalogmetadata.Bundle], error) {
+	var successors SuccessorsPredicateFunc = LegacySemanticsSuccessorsPredicate
+	if features.OperatorControllerFeatureGate.Enabled(features.ForceSemverUpgradeConstraints) {
+		successors = SemverSuccessorsPredicate
+	}
+
+	installedBundleVersion, err := installedBundle.Version()
+	if err != nil {
+		return nil, err
+	}
+
+	installedVersionConstraint, err := mmsemver.NewConstraint(installedBundleVersion.String())
+	if err != nil {
 		return nil, err
 	}
-	bundleDeploymentList := rukpakv1alpha2.BundleDeploymentList{}
-	if err := r.Client.List(ctx, &bundleDeploymentList); err != nil {
+
+	successorsPredicate, err := successors(installedBundle)
+	if err != nil {
 		return nil, err
 	}
 
-	return GenerateVariables(allBundles, clusterExtensionList.Items, bundleDeploymentList.Items)
+	// We need either successors or current version (no upgrade)
+	return catalogfilter.Or(
+		successorsPredicate,
+		catalogfilter.InMastermindsSemverRange(installedVersionConstraint),
+	), nil
 }
 
 func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alpha2.BundleDeployment, ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
@@ -346,19 +434,6 @@ func SetDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetad
 	}
 }
 
-func (r *ClusterExtensionReconciler) bundleFromSolution(selection []deppy.Variable, packageName string) (*catalogmetadata.Bundle, error) {
-	for _, variable := range selection {
-		switch v := variable.(type) {
-		case *olmvariables.BundleVariable:
-			bundlePkgName := v.Bundle().Package
-			if packageName == bundlePkgName {
-				return v.Bundle(), nil
-			}
-		}
-	}
-	return nil, fmt.Errorf("bundle for package %q not found in solution", packageName)
-}
-
 func (r *ClusterExtensionReconciler) GenerateExpectedBundleDeployment(o ocv1alpha1.ClusterExtension, bundlePath string, bundleProvisioner string) *unstructured.Unstructured {
 	// We use unstructured here to avoid problems of serializing default values when sending patches to the apiserver.
 	// If you use a typed object, any default values from that struct get serialized into the JSON patch, which could

internal/controllers/clusterextension_controller_test.go

@@ -122,7 +122,7 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
 	require.NotNil(t, cond)
 	require.Equal(t, metav1.ConditionUnknown, cond.Status)
 	require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason)
-	require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message)
+	require.Equal(t, "installation has not been attempted as resolution failed", cond.Message)
 
 	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
@@ -608,57 +608,6 @@ func TestClusterExtensionExpectedBundleDeployment(t *testing.T) {
 	require.NoError(t, cl.DeleteAllOf(ctx, &rukpakv1alpha2.BundleDeployment{}))
 }
 
-func TestClusterExtensionDuplicatePackage(t *testing.T) {
-	cl, reconciler := newClientAndReconciler(t)
-	ctx := context.Background()
-	extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
-	const pkgName = "prometheus"
-
-	t.Log("When the cluster extension specifies a duplicate package")
-	t.Log("By initializing cluster state")
-	dupClusterExtension := &ocv1alpha1.ClusterExtension{
-		ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("orig-%s", extKey.Name)},
-		Spec:       ocv1alpha1.ClusterExtensionSpec{PackageName: pkgName},
-	}
-	require.NoError(t, cl.Create(ctx, dupClusterExtension))
-
-	clusterExtension := &ocv1alpha1.ClusterExtension{
-		ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
-		Spec:       ocv1alpha1.ClusterExtensionSpec{PackageName: pkgName},
-	}
-	require.NoError(t, cl.Create(ctx, clusterExtension))
-
-	t.Log("It sets resolution failure status")
-	t.Log("By running reconcile")
-	res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
-	require.Equal(t, ctrl.Result{}, res)
-	require.EqualError(t, err, `duplicate identifier "required package prometheus" in input`)
-
-	t.Log("By fetching updated cluster extension after reconcile")
-	require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
-
-	t.Log("By checking the status fields")
-	require.Empty(t, clusterExtension.Status.ResolvedBundle)
-	require.Empty(t, clusterExtension.Status.InstalledBundle)
-
-	t.Log("By checking the expected conditions")
-	cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
-	require.NotNil(t, cond)
-	require.Equal(t, metav1.ConditionFalse, cond.Status)
-	require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
-	require.Equal(t, `duplicate identifier "required package prometheus" in input`, cond.Message)
-
-	cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
-	require.NotNil(t, cond)
-	require.Equal(t, metav1.ConditionUnknown, cond.Status)
-	require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason)
-	require.Equal(t, "installation has not been attempted as resolution failed", cond.Message)
-
-	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
-	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
-	require.NoError(t, cl.DeleteAllOf(ctx, &rukpakv1alpha2.BundleDeployment{}))
-}
-
 func TestClusterExtensionChannelVersionExists(t *testing.T) {
 	cl, reconciler := newClientAndReconciler(t)
 	ctx := context.Background()
@@ -819,7 +768,7 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
 	require.NotNil(t, cond)
 	require.Equal(t, metav1.ConditionUnknown, cond.Status)
 	require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason)
-	require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message)
+	require.Equal(t, "installation has not been attempted as resolution failed", cond.Message)
 
 	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
@@ -867,7 +816,7 @@ func TestClusterExtensionNoChannel(t *testing.T) {
 	require.NotNil(t, cond)
 	require.Equal(t, metav1.ConditionUnknown, cond.Status)
 	require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason)
-	require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message)
+	require.Equal(t, "installation has not been attempted as resolution failed", cond.Message)
 
 	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
@@ -917,7 +866,7 @@ func TestClusterExtensionNoVersion(t *testing.T) {
 	require.NotNil(t, cond)
 	require.Equal(t, metav1.ConditionUnknown, cond.Status)
 	require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason)
-	require.Equal(t, "installation has not been attempted due to failure to gather data for resolution", cond.Message)
+	require.Equal(t, "installation has not been attempted as resolution failed", cond.Message)
 
 	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
@@ -1173,8 +1122,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 		require.NotNil(t, cond)
 		assert.Equal(t, metav1.ConditionFalse, cond.Status)
 		assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
-		assert.Contains(t, cond.Message, "constraints not satisfiable")
-		assert.Regexp(t, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.2.0, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.0$", cond.Message)
+		assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"2.0.0\" found in channel \"beta\"", cond.Message)
 
 		// Valid update skipping one version
 		clusterExtension.Spec.Version = "1.2.0"
@@ -1266,8 +1214,7 @@ func TestClusterExtensionUpgrade(t *testing.T) {
 		require.NotNil(t, cond)
 		assert.Equal(t, metav1.ConditionFalse, cond.Status)
 		assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
-		assert.Contains(t, cond.Message, "constraints not satisfiable")
-		assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.0\n")
+		assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"1.2.0\" found in channel \"beta\"", cond.Message)
 
 		// Valid update skipping one version
 		clusterExtension.Spec.Version = "1.0.1"
@@ -1458,8 +1405,7 @@ func TestClusterExtensionDowngrade(t *testing.T) {
 				require.NotNil(t, cond)
 				assert.Equal(t, metav1.ConditionFalse, cond.Status)
 				assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
-				assert.Contains(t, cond.Message, "constraints not satisfiable")
-				assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.2.0, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1\n")
+				assert.Equal(t, "error upgrading from currently installed version \"1.0.1\": no package \"prometheus\" matching version \"1.0.0\" found in channel \"beta\"", cond.Message)
 			})
 		}
 	})