Workarounds for OLM e2e tests

tests/e2e/addons_test.go

@@ -11,6 +11,7 @@ import (
 )
 
 func TestHelmChartAddOns(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	configDir := t.TempDir()

tests/e2e/basic_test.go

@@ -18,6 +18,7 @@ import (
 
 // TestDeployBothSimple tests deploying both components together (simplest scenario)
 func TestDeployBothSimple(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	// Create temporary envrc file
@@ -41,21 +42,11 @@ func TestDeployBothSimple(t *testing.T) {
 	verifyNamespaceExists(t, sensorNamespace)
 	verifyNamespaceHasLabel(t, sensorNamespace, "app.kubernetes.io/managed-by", "roxie")
 
-	// Brief pause before cleanup
-	time.Sleep(5 * time.Second)
-
-	// Cleanup
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "both"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	t.Log("Verifying components are removed")
-	verifyCentralNotInstalled(t, centralNamespace)
-	verifySecuredClusterNotInstalled(t, sensorNamespace)
 }
 
 // TestDetachedPortForwarding tests the detached port-forwarding mode for central.
 func TestDetachedPortForwarding(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*")
@@ -89,12 +80,14 @@ func TestDetachedPortForwarding(t *testing.T) {
 
 	testCentralAPI(t, endpoint, caCertFile)
 
+	// Verify port-forward cleanup by checking the port is free after teardown.
+	// This assertion depends on teardown having run, so we keep it explicit
+	// rather than relying on t.Cleanup ordering.
 	t.Log("=== Cleaning up ===")
 	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"}
 	runCommand(t, teardownTimeout, env, teardownArgs...)
 
-	// Verify port-forward cleanup by checking the port is free. We can't use
-	// kill(pid, 0) because CI containers often lack a proper init to reap
+	// We can't use kill(pid, 0) because CI containers often lack a proper init to reap
 	// zombies, causing the check to pass for dead processes. Binding to the
 	// port works because even zombies release their file descriptors.
 	assert.Eventually(t, func() bool {

tests/e2e/custom_registry_test.go

@@ -14,6 +14,7 @@ import (
 // TestCentralDeployWithStackroxIORegistry verifies that roxie can deploy Central using
 // the public quay.io/stackrox-io registry instead of the default quay.io/rhacs-eng.
 func TestCentralDeployWithStackroxIORegistry(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	const stackroxIORegistry = "quay.io/stackrox-io"
@@ -30,12 +31,6 @@ func TestCentralDeployWithStackroxIORegistry(t *testing.T) {
 	verifyCentralInstalled(t, centralNamespace)
 	verifyContainerImageRegistry(t, operatorSystemNamespace, operatorDeploymentName, "manager", stackroxIORegistry)
 	verifyContainerImageRegistry(t, centralNamespace, "central", "central", stackroxIORegistry)
-
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "all"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	verifyCentralNotInstalled(t, centralNamespace)
 }
 
 // verifyContainerImageRegistry asserts that the given container's image in the

tests/e2e/custom_tls_test.go

@@ -22,6 +22,7 @@ import (
 )
 
 func TestDeployWithCustomTLSCert(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	const secretName = "custom-central-tls"
@@ -54,12 +55,6 @@ func TestDeployWithCustomTLSCert(t *testing.T) {
 
 	verifyCACertFileContainsCustomCA(t, env["ROX_CA_CERT_FILE"], caCertPEM)
 
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--single-namespace", "--skip-user-config", "both"}
-	runCommand(t, teardownTimeout, env, teardownArgs...)
-
-	verifyCentralNotInstalled(t, sharedNamespace)
-	verifySecuredClusterNotInstalled(t, sharedNamespace)
 }
 
 // generateCentralTLSCert creates a self-signed CA and a leaf certificate for

tests/e2e/e2e_test.go

@@ -58,6 +58,7 @@ func lookupLatestTag() (string, error) {
 }
 
 func TestDeployBothComponentsTogetherInSingleNamespace(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	// Create temporary envrc file.
@@ -74,11 +75,4 @@ func TestDeployBothComponentsTogetherInSingleNamespace(t *testing.T) {
 
 	verifyCentralInstalled(t, sharedNamespace)
 	verifySecuredClusterInstalled(t, sharedNamespace)
-
-	t.Log("=== Tearing down both components in single namespace ===")
-	args = []string{roxieBinary, "teardown", "--skip-user-config", "--single-namespace"}
-	runCommand(t, teardownTimeout, nil, args...)
-
-	verifyCentralNotInstalled(t, sharedNamespace)
-	verifySecuredClusterNotInstalled(t, sharedNamespace)
 }

tests/e2e/helpers.go

@@ -43,6 +43,32 @@ var (
 	roxieBinary = "roxie"
 )
 
+// Callers must register dumpClusterStateOnFailure after this function so that it runs before
+// teardown (LIFO) and can capture cluster state while resources still exist.
+func ensureCleanCluster(t *testing.T) {
+	t.Helper()
+	t.Cleanup(func() {
+		assert.NoError(t, teardownAllDeployments(), "Failed to tear down deployments")
+
+		assert.False(t, doesDeploymentExist(t, centralNamespace, "central"),
+			"Central should not exist in %s after teardown", centralNamespace)
+		assert.False(t, doesDeploymentExist(t, sharedNamespace, "central"),
+			"Central should not exist in %s after teardown", sharedNamespace)
+
+		assert.False(t, doesDeploymentExist(t, sensorNamespace, "sensor"),
+			"SecuredCluster should not exist in %s after teardown", sensorNamespace)
+		assert.False(t, doesDeploymentExist(t, sharedNamespace, "sensor"),
+			"SecuredCluster should not exist in %s after teardown", sharedNamespace)
+
+		assert.False(t, doesDeploymentExist(t, operatorSystemNamespace, operatorDeploymentName),
+			"Operator should not exist in %s after teardown", operatorSystemNamespace)
+		assert.False(t, doesDeploymentExist(t, operatorCentralNamespace, operatorDeploymentName),
+			"Operator should not exist in %s after teardown", operatorCentralNamespace)
+		assert.False(t, doesDeploymentExist(t, operatorSensorNamespace, operatorDeploymentName),
+			"Operator should not exist in %s after teardown", operatorSensorNamespace)
+	})
+}
+
 func teardownAllDeployments() error {
 	fmt.Println("=== Tearing down all deployments before running tests ===")
 
@@ -71,6 +97,13 @@ func teardownAllDeployments() error {
 	return nil
 }
 
+func skipUnlessOLM(t *testing.T) {
+	t.Helper()
+	if os.Getenv("SKIP_OLM_TESTS") != "" {
+		t.Skip("SKIP_OLM_TESTS is set")
+	}
+}
+
 func requireBinary(name string) error {
 	_, err := exec.LookPath(name)
 	return err
@@ -233,22 +266,6 @@ func verifySecuredClusterInstalled(t *testing.T, namespace string) {
 	}
 }
 
-func verifyCentralNotInstalled(t *testing.T, namespace string) {
-	t.Helper()
-
-	if doesDeploymentExist(t, namespace, "central") {
-		t.Fatalf("Central is installed in namespace %s", namespace)
-	}
-}
-
-func verifySecuredClusterNotInstalled(t *testing.T, namespace string) {
-	t.Helper()
-
-	if doesDeploymentExist(t, namespace, "sensor") {
-		t.Fatalf("Secured cluster is installed in namespace %s", namespace)
-	}
-}
-
 var clusterDumpNamespaces = []string{
 	operatorSystemNamespace,
 	operatorCentralNamespace,

tests/e2e/mixed_versions_test.go

@@ -50,6 +50,7 @@ func lookupTwoReleasedTags(t *testing.T) (string, string) {
 // TestMixedVersionOperatorDeploy tests deploying two operators with different versions
 // (mixed-version mode) and then switching back to a single operator.
 func TestMixedVersionOperatorDeploy(t *testing.T) {
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
 	mainTag, centralTag := lookupTwoReleasedTags(t)
@@ -99,13 +100,4 @@ func TestMixedVersionOperatorDeploy(t *testing.T) {
 	verifyOperatorDeploymentExists(t, operatorCentralNamespace)
 	verifyOperatorDeploymentExists(t, operatorSensorNamespace)
 	verifyOperatorNotInNamespace(t, operatorSystemNamespace)
-
-	// Cleanup
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	verifyOperatorNotInNamespace(t, operatorCentralNamespace)
-	verifyOperatorNotInNamespace(t, operatorSensorNamespace)
-	verifyOperatorNotInNamespace(t, operatorSystemNamespace)
 }

tests/e2e/olm_switch_test.go

@@ -38,12 +38,10 @@ func verifyOperatorMode(t *testing.T, expectOLM bool) {
 
 // TestOLMToNonOLMSwitch tests switching from OLM operator to non-OLM operator
 func TestOLMToNonOLMSwitch(t *testing.T) {
+	skipUnlessOLM(t)
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
-	if os.Getenv("SKIP_OLM_TESTS") != "" {
-		t.Skip("SKIP_OLM_TESTS is set")
-	}
-
 	// Create temporary envrc file
 	envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*")
 	if err != nil {
@@ -67,7 +65,9 @@ func TestOLMToNonOLMSwitch(t *testing.T) {
 
 	// Step 2: Deploy central again without OLM (should switch modes)
 	t.Log("=== Step 2: Redeploy central without OLM (triggering mode switch) ===")
-	args = append([]string{roxieBinary, "deploy", "central", "--envrc", envrcPath}, commonDeployArgs...)
+	// Note: we need to *explicitly* set OLM because of an pre-existing issue where the on-cluster manifest influences
+	// the effective deployment configuration on a re-deployment.
+	args = append([]string{roxieBinary, "deploy", "central", "--olm=false", "--envrc", envrcPath}, commonDeployArgs...)
 	runCommand(t, deployTimeout, nil, args...)
 
 	// Verify operator switched to non-OLM mode
@@ -77,23 +77,14 @@ func TestOLMToNonOLMSwitch(t *testing.T) {
 
 	// Verify central namespace still exists
 	verifyNamespaceExists(t, centralNamespace)
-
-	// Cleanup
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	verifyCentralNotInstalled(t, centralNamespace)
 }
 
 // TestNonOLMToOLMSwitch tests switching from non-OLM operator to OLM operator
 func TestNonOLMToOLMSwitch(t *testing.T) {
+	skipUnlessOLM(t)
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
-	if os.Getenv("SKIP_OLM_TESTS") != "" {
-		t.Skip("SKIP_OLM_TESTS is set")
-	}
-
 	// Create temporary envrc file
 	envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*")
 	if err != nil {
@@ -104,7 +95,9 @@ func TestNonOLMToOLMSwitch(t *testing.T) {
 
 	// Step 1: Deploy central without OLM (non-OLM operator)
 	t.Log("=== Step 1: Deploy central with non-OLM operator ===")
-	args := append([]string{roxieBinary, "deploy", "central", "--envrc", envrcPath}, commonDeployArgs...)
+	// Note: we need to *explicitly* set OLM because of an pre-existing issue where the on-cluster manifest influences
+	// the effective deployment configuration on a re-deployment.
+	args := append([]string{roxieBinary, "deploy", "central", "--olm=false", "--envrc", envrcPath}, commonDeployArgs...)
 	runCommand(t, deployTimeout, nil, args...)
 
 	// Verify operator is in non-OLM mode
@@ -127,24 +120,15 @@ func TestNonOLMToOLMSwitch(t *testing.T) {
 
 	// Verify central namespace still exists
 	verifyNamespaceExists(t, centralNamespace)
-
-	// Cleanup
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	verifyCentralNotInstalled(t, centralNamespace)
 }
 
 // TestOLMOperatorVersionUpgrade tests that OLM operator version mismatches trigger teardown and redeploy,
 // and that operand tag overrides propagate RELATED_IMAGE_* env vars to the OLM Subscription.
 func TestOLMOperatorVersionUpgrade(t *testing.T) {
+	skipUnlessOLM(t)
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
-	if os.Getenv("SKIP_OLM_TESTS") != "" {
-		t.Skip("SKIP_OLM_TESTS is set")
-	}
-
 	// Look up two release tags different from the main tag. We use one as a blanket
 	// operand tag override and the other as a per-image override for central-db.
 	tag1, tag2 := lookupTwoReleasedTags(t)
@@ -210,23 +194,14 @@ func TestOLMOperatorVersionUpgrade(t *testing.T) {
 	// Note: In a real version upgrade test, we would set a different MAIN_IMAGE_TAG
 	// and verify that the operator was torn down and redeployed with the new version.
 	// For now, this test validates the basic flow.
-
-	// Cleanup
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "central"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	verifyCentralNotInstalled(t, centralNamespace)
 }
 
 // TestSecuredClusterWithOLMSwitch tests that secured-cluster deployment also respects OLM mode switches
 func TestSecuredClusterWithOLMSwitch(t *testing.T) {
+	skipUnlessOLM(t)
+	ensureCleanCluster(t)
 	dumpClusterStateOnFailure(t)
 
-	if os.Getenv("SKIP_OLM_TESTS") != "" {
-		t.Skip("SKIP_OLM_TESTS is set")
-	}
-
 	// Create temporary envrc file
 	envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*")
 	if err != nil {
@@ -260,20 +235,12 @@ func TestSecuredClusterWithOLMSwitch(t *testing.T) {
 
 	// Step 3: Switch to non-OLM by redeploying secured-cluster without --olm
 	t.Log("=== Step 3: Redeploy secured-cluster without OLM (triggering mode switch) ===")
-	// Note: we need to *explicitly* disable OLM because of an pre-existing issue where the on-cluster manifest influences
+	// Note: we need to *explicitly* set OLM because of an pre-existing issue where the on-cluster manifest influences
 	// the effective deployment configuration on a re-deployment.
-	args = append([]string{roxieBinary, "deploy", "--early-readiness", "secured-cluster", "--olm=false"}, commonDeployArgs...)
+	args = append([]string{roxieBinary, "deploy", "--early-readiness", "--olm=false", "secured-cluster"}, commonDeployArgs...)
 	runCommand(t, deployTimeout, envrcEnv, args...)
 
 	// Verify operator switched to non-OLM mode
 	verifyOperatorMode(t, false)
 	verifyNamespaceExists(t, sensorNamespace)
-
-	// Cleanup
-	t.Log("=== Cleaning up ===")
-	teardownArgs := []string{roxieBinary, "teardown", "--skip-user-config", "both"}
-	runCommand(t, teardownTimeout, nil, teardownArgs...)
-
-	verifyCentralNotInstalled(t, centralNamespace)
-	verifySecuredClusterNotInstalled(t, sensorNamespace)
 }