Helm POC e2e fixes

Fixes the e2e tests in helm-poc branch.

Signed-off-by: dtfranz <[email protected]>

Author: dtfranz

internal/catalogmetadata/filter/bundle_predicates.go

@@ -60,6 +60,12 @@ func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] {
 	}
 }
 
+func WithBundleName(bundleName string) Predicate[catalogmetadata.Bundle] {
+	return func(bundle *catalogmetadata.Bundle) bool {
+		return bundle.Name == bundleName
+	}
+}
+
 func LegacySuccessor(installedBundle *catalogmetadata.Bundle) Predicate[catalogmetadata.Bundle] {
 	isSuccessor := func(candidateBundleEntry declcfg.ChannelEntry) bool {
 		if candidateBundleEntry.Replaces == installedBundle.Name {

internal/catalogmetadata/filter/bundle_predicates_test.go

@@ -201,3 +201,16 @@ func TestWithDeprecation(t *testing.T) {
 	assert.True(t, f(b1))
 	assert.False(t, f(b2))
 }
+
+func TestWithBundleName(t *testing.T) {
+	b1 := &catalogmetadata.Bundle{
+		Bundle: declcfg.Bundle{Name: "package1.v0.0.1"},
+	}
+	b2 := &catalogmetadata.Bundle{
+		Bundle: declcfg.Bundle{Name: "package1.v0.0.2"},
+	}
+
+	f := filter.WithBundleName("package1.v0.0.1")
+	assert.True(t, f(b1))
+	assert.False(t, f(b2))
+}

internal/controllers/clusterextension_controller.go

@@ -411,6 +411,11 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
 	channelName := ext.Spec.Channel
 	versionRange := ext.Spec.Version
 
+	installedBundle, err := r.installedBundle(ctx, allBundles, &ext)
+	if err != nil {
+		return nil, err
+	}
+
 	predicates := []catalogfilter.Predicate[catalogmetadata.Bundle]{
 		catalogfilter.WithPackageName(packageName),
 	}
@@ -427,35 +432,35 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
 		predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr))
 	}
 
-	var installedVersion string
-	var upgradeErrorPrefix string
-	if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore {
-		installedBundle, err := r.getInstalledVersion(ctx, ext)
+	if ext.Spec.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore && installedBundle != nil {
+		upgradePredicate, err := SuccessorsPredicate(installedBundle)
 		if err != nil {
-			return nil, utilerrors.NewAggregate([]error{fmt.Errorf("error fetching installed version: %w", err)})
-		}
-		if installedBundle != nil {
-			installedVersion = installedBundle.String()
-			upgradeErrorPrefix = fmt.Sprintf("error upgrading from currently installed version %q: ", installedVersion)
-			wantedVersionRangeConstraint, err := mmsemver.NewConstraint(fmt.Sprintf("^%s", installedVersion))
-			if err != nil {
-				return nil, utilerrors.NewAggregate([]error{fmt.Errorf("%serror creating version constraint: %w", upgradeErrorPrefix, err)})
-			}
-			predicates = append(predicates, catalogfilter.InMastermindsSemverRange(wantedVersionRangeConstraint))
+			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 {
 		switch {
 		case versionRange != "" && channelName != "":
-			return nil, fmt.Errorf("no package %q matching version %q in channel %q found", packageName, versionRange, channelName)
+			return nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", upgradeErrorPrefix, packageName, versionRange, channelName)
 		case versionRange != "":
-			return nil, fmt.Errorf("no package %q matching version %q found", packageName, versionRange)
+			return nil, fmt.Errorf("%sno package %q matching version %q found", upgradeErrorPrefix, packageName, versionRange)
 		case channelName != "":
-			return nil, fmt.Errorf("no package %q in channel %q found", packageName, channelName)
+			return nil, fmt.Errorf("%sno package %q in channel %q found", upgradeErrorPrefix, packageName, channelName)
 		default:
-			return nil, fmt.Errorf("no package %q found", packageName)
+			return nil, fmt.Errorf("%sno package %q found", upgradeErrorPrefix, packageName)
 		}
 	}
 
@@ -682,32 +687,43 @@ func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crh
 	}
 }
 
-// getInstalledVersion fetches the installed version of a particular bundle from the cluster. To do so, we read the release label
-// which are added during install.
-func (r *ClusterExtensionReconciler) getInstalledVersion(ctx context.Context, clusterExtension ocv1alpha1.ClusterExtension) (*bsemver.Version, error) {
-	cl, err := r.ActionClientGetter.ActionClientFor(ctx, &clusterExtension)
+func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) {
+	cl, err := r.ActionClientGetter.ActionClientFor(ctx, ext)
 	if err != nil {
 		return nil, err
 	}
 
-	release, err := cl.Get(clusterExtension.GetName())
+	release, err := cl.Get(ext.GetName())
 	if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
 		return nil, err
 	}
 	if release == nil {
 		return nil, nil
 	}
 
-	existingVersion, ok := release.Labels[labels.BundleVersionKey]
-	if !ok {
-		return nil, fmt.Errorf("release %q: missing bundle version", release.Name)
+	// Bundle must match installed version exactly
+	vr, err := mmsemver.NewConstraint(release.Labels[labels.BundleVersionKey])
+	if err != nil {
+		return nil, err
 	}
 
-	existingVersionSemver, err := bsemver.New(existingVersion)
-	if err != nil {
-		return nil, fmt.Errorf("could not determine bundle version for the chart %q: %w", release.Name, err)
+	// TODO (dfranz) Are the PackageName + BundleName + BundleVersion enough to
+	// be 100% sure we have the correct bundle?
+	// find corresponding bundle for the installed content
+	resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(
+		catalogfilter.WithPackageName(release.Labels[labels.PackageNameKey]),
+		catalogfilter.WithBundleName(release.Labels[labels.BundleNameKey]),
+		catalogfilter.InMastermindsSemverRange(vr),
+	))
+	if len(resultSet) == 0 {
+		return nil, fmt.Errorf("bundle %q for package %q not found in available catalogs but is currently installed via Helm Chart %q", release.Labels[labels.BundleNameKey], ext.Spec.PackageName, release.Labels[labels.BundleNameKey])
 	}
-	return existingVersionSemver, nil
+
+	sort.SliceStable(resultSet, func(i, j int) bool {
+		return catalogsort.ByVersion(resultSet[i], resultSet[j])
+	})
+
+	return resultSet[0], nil
 }
 
 type releaseState string

test/e2e/cluster_extension_install_test.go

@@ -183,7 +183,6 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
 }
 
 func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
-	t.Skip("Skipping tests related to upgrades")
 	t.Log("When a cluster extension is installed from a catalog")
 	t.Log("When resolving upgrade edges")
 
@@ -224,12 +223,12 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
 		}
 		assert.Equal(ct, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
 		assert.Equal(ct, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"1.2.0\" found", cond.Message)
-		assert.Empty(ct, clusterExtension.Status.ResolvedBundle)
+		// TODO (dfranz) do we still need to be clearing this status field when this error is reached?
+		//assert.Empty(ct, clusterExtension.Status.ResolvedBundle)
 	}, pollDuration, pollInterval)
 }
 
 func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
-	t.Skip("Skipping tests related to upgrades")
 	t.Log("When a cluster extension is installed from a catalog")
 	t.Log("When resolving upgrade edges")
 
@@ -276,7 +275,6 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
 }
 
 func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
-	t.Skip("Skipping tests related to upgrades")
 	t.Log("When a cluster extension is installed from a catalog")
 	t.Log("When resolving upgrade edges")
 	clusterExtension, extensionCatalog := testInit(t)

test/e2e/cluster_extension_registryV1_limitations_test.go

@@ -2,60 +2,36 @@ package e2e
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
-	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
-
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 )
 
 func TestClusterExtensionPackagesWithWebhooksAreNotAllowed(t *testing.T) {
 	ctx := context.Background()
-	catalog, err := createTestCatalog(ctx, testCatalogName, os.Getenv(testCatalogRefEnvVar))
-	defer func(cat *catalogd.Catalog) {
-		require.NoError(t, c.Delete(context.Background(), cat))
-		require.Eventually(t, func() bool {
-			err := c.Get(context.Background(), types.NamespacedName{Name: cat.Name}, &catalogd.Catalog{})
-			return errors.IsNotFound(err)
-		}, pollDuration, pollInterval)
-	}(catalog)
-	require.NoError(t, err)
-
-	deleteClusterExtension := func(clusterExtension *ocv1alpha1.ClusterExtension) {
-		require.NoError(t, c.Delete(ctx, clusterExtension))
-		require.Eventually(t, func() bool {
-			err := c.Get(ctx, types.NamespacedName{Name: clusterExtension.Name}, &ocv1alpha1.ClusterExtension{})
-			return errors.IsNotFound(err)
-		}, pollDuration, pollInterval)
-	}
+	clusterExtension, catalog := testInit(t)
 
-	clusterExtension := &ocv1alpha1.ClusterExtension{
-		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: "package-with-webhooks-",
-		},
-		Spec: ocv1alpha1.ClusterExtensionSpec{
-			PackageName:      "package-with-webhooks",
-			Version:          "1.0.0",
-			InstallNamespace: "default",
-		},
+	clusterExtension.Spec = ocv1alpha1.ClusterExtensionSpec{
+		PackageName:      "package-with-webhooks",
+		Version:          "1.0.0",
+		InstallNamespace: "default",
 	}
-	err = c.Create(ctx, clusterExtension)
-	defer deleteClusterExtension(clusterExtension)
-	require.NoError(t, err)
+	require.NoError(t, c.Create(ctx, clusterExtension))
+	defer testCleanup(t, catalog, clusterExtension)
+	defer getArtifactsOutput(t)
 
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
 		assert.NoError(ct, c.Get(ctx, types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
-
 		cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
-		assert.NotNil(ct, cond)
+		if !assert.NotNil(ct, cond) {
+			return
+		}
 		assert.Equal(ct, metav1.ConditionFalse, cond.Status)
 		assert.Equal(ct, ocv1alpha1.ReasonInstallationFailed, cond.Reason)
 		assert.Contains(ct, cond.Message, "webhookDefinitions are not supported")