Fix: testCatalogName conflict and Explicitly set CollisionProtection to Prevent in GenerateRevision

Author: jianzhangbjz

internal/operator-controller/applier/boxcutter.go

@@ -120,7 +120,8 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
 		sanitizedUnstructured(ctx, &unstr)
 
 		objs = append(objs, ocv1.ClusterExtensionRevisionObject{
-			Object: unstr,
+			Object:              unstr,
+			CollisionProtection: ocv1.CollisionProtectionPrevent,
 		})
 	}
 

internal/operator-controller/applier/boxcutter_test.go

@@ -199,6 +199,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
 							"spec": map[string]interface{}{},
 						},
 					},
+					CollisionProtection: ocv1.CollisionProtectionPrevent,
 				},
 				{
 					Object: unstructured.Unstructured{
@@ -227,6 +228,7 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
 							},
 						},
 					},
+					CollisionProtection: ocv1.CollisionProtectionPrevent,
 				},
 			},
 		},

internal/shared/util/testutils/artifacts.go

@@ -13,6 +13,7 @@ import (
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	kubeclient "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/utils/env"
@@ -71,9 +72,13 @@ func CollectTestArtifacts(t *testing.T, artifactName string, c client.Client, cf
 	}
 
 	// get all cluster extension revisions save them to the artifact path.
+	// Note: ClusterExtensionRevision is an experimental API and may not be available in all environments.
 	clusterExtensionRevisions := ocv1.ClusterExtensionRevisionList{}
 	if err := c.List(context.Background(), &clusterExtensionRevisions); err != nil {
-		fmt.Printf("Failed to list cluster extensions: %v", err)
+		// If the CRD doesn't exist (e.g., in non-experimental environments), skip this step silently
+		if !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
+			fmt.Printf("Failed to list cluster extension revisions: %v\n", err)
+		}
 	}
 	for _, cer := range clusterExtensionRevisions.Items {
 		// Save cluster extension to artifact path

test/e2e/cluster_extension_install_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"slices"
 	"testing"
 	"time"
 
@@ -197,7 +198,8 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) {
 	t.Log("When a cluster extension is installed from a catalog")
 
 	clusterExtension, extensionCatalog, sa, ns := TestInit(t)
-	extraCatalog, err := CreateTestCatalog(context.Background(), "extra-test-catalog", os.Getenv(testCatalogRefEnvVar))
+	extraCatalogName := fmt.Sprintf("extra-test-catalog-%s", rand.String(8))
+	extraCatalog, err := CreateTestCatalog(context.Background(), extraCatalogName, os.Getenv(testCatalogRefEnvVar))
 	require.NoError(t, err)
 
 	defer TestCleanup(t, extensionCatalog, clusterExtension, sa, ns)
@@ -238,7 +240,11 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) {
 		require.NotNil(ct, cond)
 		require.Equal(ct, metav1.ConditionTrue, cond.Status)
 		require.Equal(ct, ocv1.ReasonRetrying, cond.Reason)
-		require.Contains(ct, cond.Message, "in multiple catalogs with the same priority [extra-test-catalog test-catalog]")
+		// Catalog names are sorted alphabetically in the error message
+		catalogs := []string{extensionCatalog.Name, extraCatalog.Name}
+		slices.Sort(catalogs)
+		expectedMessage := fmt.Sprintf("in multiple catalogs with the same priority %v", catalogs)
+		require.Contains(ct, cond.Message, expectedMessage)
 	}, pollDuration, pollInterval)
 }
 
@@ -441,7 +447,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
 	// patch imageRef tag on test-catalog image with v2 image
 	t.Log("By patching the catalog ImageRef to point to the v2 catalog")
 	updatedCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:v2", os.Getenv("CLUSTER_REGISTRY_HOST"))
-	err := patchTestCatalog(context.Background(), testCatalogName, updatedCatalogImage)
+	err := patchTestCatalog(context.Background(), extensionCatalog.Name, updatedCatalogImage)
 	require.NoError(t, err)
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
 		require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: extensionCatalog.Name}, extensionCatalog))
@@ -474,8 +480,9 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
 	require.NoError(t, err)
 
 	// create a test-catalog with latest image tag
+	catalogName := fmt.Sprintf("test-catalog-%s", rand.String(8))
 	latestCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:latest", os.Getenv("CLUSTER_REGISTRY_HOST"))
-	extensionCatalog, err := CreateTestCatalog(context.Background(), testCatalogName, latestCatalogImage)
+	extensionCatalog, err := CreateTestCatalog(context.Background(), catalogName, latestCatalogImage)
 	require.NoError(t, err)
 	clusterExtensionName := fmt.Sprintf("clusterextension-%s", rand.String(8))
 	clusterExtension := &ocv1.ClusterExtension{

test/helpers/helpers.go

@@ -218,17 +218,18 @@ func TestInit(t *testing.T) (*ocv1.ClusterExtension, *ocv1.ClusterCatalog, *core
 
 func TestInitClusterExtensionClusterCatalog(t *testing.T) (*ocv1.ClusterExtension, *ocv1.ClusterCatalog) {
 	ceName := fmt.Sprintf("clusterextension-%s", rand.String(8))
+	catalogName := fmt.Sprintf("test-catalog-%s", rand.String(8))
 
 	ce := &ocv1.ClusterExtension{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: ceName,
 		},
 	}
 
-	cc, err := CreateTestCatalog(context.Background(), testCatalogName, os.Getenv(testCatalogRefEnvVar))
+	cc, err := CreateTestCatalog(context.Background(), catalogName, os.Getenv(testCatalogRefEnvVar))
 	require.NoError(t, err)
 
-	ValidateCatalogUnpack(t)
+	ValidateCatalogUnpackWithName(t, catalogName)
 
 	return ce, cc
 }
@@ -250,11 +251,18 @@ func TestInitServiceAccountNamespace(t *testing.T, clusterExtensionName string)
 	return sa, ns
 }
 
+// ValidateCatalogUnpack validates that the test catalog with the default name has unpacked successfully.
+// Deprecated: Use ValidateCatalogUnpackWithName for tests that use unique catalog names.
 func ValidateCatalogUnpack(t *testing.T) {
+	ValidateCatalogUnpackWithName(t, testCatalogName)
+}
+
+// ValidateCatalogUnpackWithName validates that a catalog with the given name has unpacked successfully.
+func ValidateCatalogUnpackWithName(t *testing.T, catalogName string) {
 	catalog := &ocv1.ClusterCatalog{}
 	t.Log("Ensuring ClusterCatalog has Status.Condition of Progressing with a status == True and reason == Succeeded")
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
-		err := c.Get(context.Background(), types.NamespacedName{Name: testCatalogName}, catalog)
+		err := c.Get(context.Background(), types.NamespacedName{Name: catalogName}, catalog)
 		require.NoError(ct, err)
 		cond := apimeta.FindStatusCondition(catalog.Status.Conditions, ocv1.TypeProgressing)
 		require.NotNil(ct, cond)
@@ -265,11 +273,11 @@ func ValidateCatalogUnpack(t *testing.T) {
 	t.Log("Checking that catalog has the expected metadata label")
 	require.NotNil(t, catalog.Labels)
 	require.Contains(t, catalog.Labels, "olm.operatorframework.io/metadata.name")
-	require.Equal(t, testCatalogName, catalog.Labels["olm.operatorframework.io/metadata.name"])
+	require.Equal(t, catalogName, catalog.Labels["olm.operatorframework.io/metadata.name"])
 
 	t.Log("Ensuring ClusterCatalog has Status.Condition of Type = Serving with status == True")
 	require.EventuallyWithT(t, func(ct *assert.CollectT) {
-		err := c.Get(context.Background(), types.NamespacedName{Name: testCatalogName}, catalog)
+		err := c.Get(context.Background(), types.NamespacedName{Name: catalogName}, catalog)
 		require.NoError(ct, err)
 		cond := apimeta.FindStatusCondition(catalog.Status.Conditions, ocv1.TypeServing)
 		require.NotNil(ct, cond)