(chore): add e2e tests for workload resilience when catalog is deleted

camilamacedo86 · camilamacedo86 · commit bdf1b6323752 · 2026-01-20T18:57:02.000Z
diff --git a/internal/operator-controller/applier/boxcutter_test.go b/internal/operator-controller/applier/boxcutter_test.go
@@ -1190,10 +1190,12 @@ func Test_PreAuthorizer_Integration(t *testing.T) {
 				RevisionGenerator: dummyGenerator,
 				PreAuthorizer:     tc.preAuthorizer(t),
 			}
-			err := boxcutter.Apply(t.Context(), dummyBundleFs, ext, nil, revisionAnnotations)
+			completed, status, err := boxcutter.Apply(t.Context(), dummyBundleFs, ext, nil, revisionAnnotations)
 			if tc.validate != nil {
 				tc.validate(t, err)
 			}
+			_ = completed
+			_ = status
 		})
 	}
 }
diff --git a/test/e2e/features/recover.feature b/test/e2e/features/recover.feature
@@ -149,3 +149,127 @@ Feature: Recover cluster extension from errors that might occur during its lifet
     Then ClusterExtension is available
     And ClusterExtension reports Progressing as True with Reason Succeeded
     And ClusterExtension reports Installed as True
+
+  # CATALOG DELETION RESILIENCE SCENARIOS
+  
+  Scenario: Extension continues running after catalog deletion
+    Given ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+    And ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    And ClusterExtension is rolled out
+    And ClusterExtension is available
+    And resource "deployment/test-operator" is available
+    And resource "configmap/test-configmap" is available
+    When ClusterCatalog "test" is deleted
+    # Verify controller still maintains resources after catalog deletion by removing and restoring a resource.
+    # This approach avoids race conditions because:
+    # - We don't rely on status flags that might be unchanged (e.g., Installed=True before and after)
+    # - Resource restoration is an observable event that PROVES the controller reconciled after deletion
+    # - The controller must actively apply manifests to restore the removed resource
+    And resource "configmap/test-configmap" is removed
+    Then resource "configmap/test-configmap" is eventually restored
+    And resource "deployment/test-operator" is available
+
+  Scenario: Config changes are allowed even when the catalog does not exist anymore
+    Given ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+    And ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    And ClusterExtension is rolled out
+    And ClusterExtension is available
+    And ClusterCatalog "test" is deleted
+    When ClusterExtension is updated to add preflight config
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        install:
+          preflight:
+            crdUpgradeSafety:
+              enforcement: None
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    # Wait for reconciliation of the updated spec (config change should succeed without catalog)
+    # First ensure the controller has reconciled the new generation (spec update)
+    And ClusterExtension latest generation has been reconciled
+    # Config-only changes don't trigger resolution failure because the bundle version hasn't changed.
+    # The controller falls back to the installed bundle (via handleResolutionError), which allows
+    # Apply to run and successfully maintain resources. This results in Progressing=Succeeded.
+    And ClusterExtension reports Progressing as True with Reason Succeeded
+    Then ClusterExtension is available
+    And ClusterExtension reports Installed as True
+
+  Scenario: Version upgrade does not proceed when catalog does not exist
+    Given ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+    And ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            version: "1.0.0"
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    And ClusterExtension is rolled out
+    And ClusterExtension is available
+    And bundle "test-operator.1.0.0" is installed in version "1.0.0"
+    When ClusterCatalog "test" is deleted
+    And ClusterExtension is updated to version "1.0.1"
+    # Wait for reconciliation after the version change request
+    # Note: Retrying status means controller will auto-upgrade when catalog becomes available
+    Then ClusterExtension reports Progressing as True with Reason Retrying
+    # Verify upgrade did not proceed: version remains at 1.0.0 (not 1.0.1)
+    And bundle "test-operator.1.0.0" is installed in version "1.0.0"
+    And ClusterExtension reports Installed as True
diff --git a/test/e2e/steps/hooks.go b/test/e2e/steps/hooks.go
@@ -139,28 +139,33 @@ func stderrOutput(err error) string {
 	return ""
 }
 
-func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, err error) (context.Context, error) {
+func ScenarioCleanup(ctx context.Context, _ *godog.Scenario, scenarioErr error) (context.Context, error) {
 	sc := scenarioCtx(ctx)
 	for _, bgCmd := range sc.backGroundCmds {
 		if p := bgCmd.Process; p != nil {
 			_ = p.Kill()
 		}
 	}
-	if err != nil {
-		return ctx, err
-	}
 
+	// Run cleanup ALWAYS, even if scenario failed (to prevent resource leaks into next scenario)
 	forDeletion := []resource{}
 	if sc.clusterExtensionName != "" {
 		forDeletion = append(forDeletion, resource{name: sc.clusterExtensionName, kind: "clusterextension"})
 	}
 	forDeletion = append(forDeletion, resource{name: sc.namespace, kind: "namespace"})
-	go func() {
-		for _, r := range forDeletion {
-			if _, err := k8sClient("delete", r.kind, r.name, "--ignore-not-found=true"); err != nil {
-				logger.Info("Error deleting resource", "name", r.name, "namespace", sc.namespace, "stderr", stderrOutput(err))
+
+	// Cleanup must be synchronous to ensure proper test isolation.
+	// If cleanup runs in background, the next scenario may start before resources are deleted.
+	for _, r := range forDeletion {
+		// Try graceful deletion first (60s timeout), fall back to force if stuck
+		if _, err := k8sClient("delete", r.kind, r.name, "--ignore-not-found=true", "--wait=true", "--timeout=60s"); err != nil {
+			// Force delete if stuck on finalizers. Use grace-period=30s instead of 0
+			// to allow pods to shut down gracefully and avoid stale state in subsequent scenarios.
+			if _, forceErr := k8sClient("delete", r.kind, r.name, "--ignore-not-found=true", "--force", "--grace-period=30"); forceErr != nil {
+				logger.Info("Error force deleting resource", "kind", r.kind, "name", r.name, "stderr", stderrOutput(forceErr))
 			}
 		}
-	}()
-	return ctx, nil
+	}
+
+	return ctx, scenarioErr
 }
diff --git a/test/e2e/steps/steps.go b/test/e2e/steps/steps.go
@@ -56,6 +56,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 	sc.Step(`^(?i)ClusterExtension is updated(?:\s+.*)?$`, ResourceIsApplied)
 	sc.Step(`^(?i)ClusterExtension is available$`, ClusterExtensionIsAvailable)
 	sc.Step(`^(?i)ClusterExtension is rolled out$`, ClusterExtensionIsRolledOut)
+	sc.Step(`^(?i)ClusterExtension (?:latest generation )?has (?:been )?reconciled(?: the latest generation)?$`, ClusterExtensionReconciledLatestGeneration)
 	sc.Step(`^(?i)ClusterExtension reports "([^"]+)" as active revision(s?)$`, ClusterExtensionReportsActiveRevisions)
 	sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message:$`, ClusterExtensionReportsCondition)
 	sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) and Message includes:$`, ClusterExtensionReportsConditionWithMessageFragment)
@@ -89,6 +90,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 	sc.Step(`^(?i)ClusterCatalog "([^"]+)" serves bundles$`, CatalogServesBundles)
 	sc.Step(`^"([^"]+)" catalog image version "([^"]+)" is also tagged as "([^"]+)"$`, TagCatalogImage)
 	sc.Step(`^(?i)ClusterCatalog "([^"]+)" image version "([^"]+)" is also tagged as "([^"]+)"$`, TagCatalogImage)
+	sc.Step(`^(?i)ClusterCatalog "([^"]+)" is deleted$`, CatalogIsDeleted)
 
 	sc.Step(`^(?i)operator "([^"]+)" target namespace is "([^"]+)"$`, OperatorTargetNamespace)
 	sc.Step(`^(?i)Prometheus metrics are returned in the response$`, PrometheusMetricsAreReturned)
@@ -246,6 +248,25 @@ func ClusterExtensionIsAvailable(ctx context.Context) error {
 	return nil
 }
 
+func ClusterExtensionReconciledLatestGeneration(ctx context.Context) error {
+	sc := scenarioCtx(ctx)
+	waitFor(ctx, func() bool {
+		// Get both generation and observedGeneration in a single kubectl call
+		output, err := k8sClient("get", "clusterextension", sc.clusterExtensionName,
+			"-o", "jsonpath={.metadata.generation},{.status.conditions[?(@.type=='Progressing')].observedGeneration}")
+		if err != nil || output == "" {
+			return false
+		}
+		parts := strings.Split(output, ",")
+		if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+			return false
+		}
+		// Both exist and are equal means reconciliation happened
+		return parts[0] == parts[1]
+	})
+	return nil
+}
+
 func ClusterExtensionIsRolledOut(ctx context.Context) error {
 	sc := scenarioCtx(ctx)
 	require.Eventually(godog.T(ctx), func() bool {
@@ -727,6 +748,15 @@ func TagCatalogImage(name, oldTag, newTag string) error {
 	return crane.Tag(imageRef, newTag, crane.Insecure)
 }
 
+func CatalogIsDeleted(ctx context.Context, catalogName string) error {
+	catalogFullName := fmt.Sprintf("%s-catalog", catalogName)
+	_, err := k8sClient("delete", "clustercatalog", catalogFullName, "--ignore-not-found=true", "--wait=true")
+	if err != nil {
+		return fmt.Errorf("failed to delete catalog: %v", err)
+	}
+	return nil
+}
+
 func PrometheusMetricsAreReturned(ctx context.Context) error {
 	sc := scenarioCtx(ctx)
 	for podName, mr := range sc.metricsResponse {

Original file line number	Diff line number	Diff line change
`@@ -1190,10 +1190,12 @@ func Test_PreAuthorizer_Integration(t *testing.T) {`
`1190`	`1190`	`RevisionGenerator: dummyGenerator,`
`1191`	`1191`	`PreAuthorizer: tc.preAuthorizer(t),`
`1192`	`1192`	`}`
`1193`		`- err := boxcutter.Apply(t.Context(), dummyBundleFs, ext, nil, revisionAnnotations)`
	`1193`	`+ completed, status, err := boxcutter.Apply(t.Context(), dummyBundleFs, ext, nil, revisionAnnotations)`
`1194`	`1194`	`if tc.validate != nil {`
`1195`	`1195`	`tc.validate(t, err)`
`1196`	`1196`	`}`
	`1197`	`+ _ = completed`
	`1198`	`+ _ = status`
`1197`	`1199`	`})`
`1198`	`1200`	`}`
`1199`	`1201`	`}`