diff --git a/.github/workflows/create-dev-cluster.yml b/.github/workflows/create-dev-cluster.yml index 3961416..e172cb0 100644 --- a/.github/workflows/create-dev-cluster.yml +++ b/.github/workflows/create-dev-cluster.yml @@ -13,6 +13,9 @@ on: required: false type: string default: '' + lifespan: + type: string + default: '3h' outputs: cluster-name: description: "Name of the created cluster" @@ -29,6 +32,6 @@ jobs: flavor: ${{ inputs.flavor }} name: ${{ inputs.cluster-name }} args: ${{ inputs.args }} - lifespan: "2h" + lifespan: ${{ inputs.lifespan }} wait: true token: ${{ secrets.INFRA_CI_TOKEN }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 126016b..5c542cb 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -34,6 +34,7 @@ jobs: with: cluster-name: infra-roxie-pr-${{ github.event.pull_request.number }}-gke flavor: gke-default + lifespan: 2h args: machine-type=e2-standard-4,nodes=3,gcp-image-type=ubuntu_containerd secrets: inherit @@ -44,6 +45,7 @@ jobs: with: cluster-name: infra-roxie-pr-${{ github.event.pull_request.number }}-openshift flavor: ocp-4 + lifespan: 3h args: master-node-type=e2-standard-4,worker-node-type=e2-standard-8,master-node-count=3,worker-node-count=3 secrets: inherit diff --git a/internal/deployer/crs.go b/internal/deployer/crs.go index dd9e491..6938ff9 100644 --- a/internal/deployer/crs.go +++ b/internal/deployer/crs.go @@ -49,8 +49,8 @@ type crsGenResponse struct { // generateCRS is a retrying wrapper on top of generateCRSOnce. func (d *Deployer) generateCRS(ctx context.Context, clusterName string) (string, error) { - const maxAttempts = 5 - const baseRetryDelay = 10 + const maxAttempts = 20 + const retryWaitTime = 10 * time.Second client, err := d.centralHTTPClient() if err != nil { @@ -60,12 +60,11 @@ func (d *Deployer) generateCRS(ctx context.Context, clusterName string) (string, var lastErr error for attempt := 1; attempt <= maxAttempts; attempt++ { if attempt > 1 { - waitTime := time.Duration(attempt*baseRetryDelay) * time.Second - log.Infof("Retrying CRS generation (attempt %d/%d) after %v...", attempt, maxAttempts, waitTime) + log.Infof("Retrying CRS generation (attempt %d/%d) after %v...", attempt, maxAttempts, retryWaitTime) select { case <-ctx.Done(): return "", ctx.Err() - case <-time.After(waitTime): + case <-time.After(retryWaitTime): } } diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index 8541dc6..8be40e1 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -22,6 +22,10 @@ import ( "github.com/stackrox/roxie/internal/types" ) +const ( + loadBalancerWaitTime = 10 * time.Minute +) + var ( smallScale = map[string]interface{}{ "autoScaling": "Enabled", @@ -614,11 +618,11 @@ func (d *Deployer) waitForComponentReady(ctx context.Context, comp component.Com // waitForLoadBalancer waits for a LoadBalancer service to get an external IP. // Returns the endpoint as "host:port", with no https:// prefix. -func (d *Deployer) waitForLoadBalancer(ctx context.Context, namespace, serviceName string, timeout int) (string, error) { - log.Infof("⏳ Waiting for LoadBalancer %s to get external IP...", serviceName) +func (d *Deployer) waitForLoadBalancer(ctx context.Context, namespace, serviceName string, waitTime time.Duration) (string, error) { + log.Infof("⏳ Waiting %s for LoadBalancer %s to get external IP...", waitTime, serviceName) start := time.Now() - for time.Since(start) < time.Duration(timeout)*time.Second { + for time.Since(start) < waitTime { result, err := k8s.RunKubectl(ctx, k8s.KubectlOptions{ Args: []string{"get", "svc", serviceName, "-n", namespace, "-o", "jsonpath={.status.loadBalancer.ingress[0].ip}"}, }) @@ -768,7 +772,7 @@ func (d *Deployer) configureCentralEndpoint(ctx context.Context) error { // Start port-forward for CLI tool access via localhost:8443 serviceName := "central" if exposure == types.ExposureLoadBalancer { - _, err := d.waitForLoadBalancer(ctx, d.config.Central.Namespace, "central-loadbalancer", 300) + _, err := d.waitForLoadBalancer(ctx, d.config.Central.Namespace, "central-loadbalancer", loadBalancerWaitTime) if err != nil { log.Warningf("LoadBalancer not ready: %v", err) } else { @@ -791,7 +795,7 @@ func (d *Deployer) configureCentralEndpoint(ctx context.Context) error { d.centralEndpoint = endpoint } } else if exposure == types.ExposureLoadBalancer { - endpoint, err := d.waitForLoadBalancer(ctx, d.config.Central.Namespace, "central-loadbalancer", 300) + endpoint, err := d.waitForLoadBalancer(ctx, d.config.Central.Namespace, "central-loadbalancer", loadBalancerWaitTime) if err != nil { return fmt.Errorf("failed to get LoadBalancer endpoint: %w", err) } diff --git a/tests/e2e/addons_test.go b/tests/e2e/addons_test.go index 6ed9bdc..2977cda 100644 --- a/tests/e2e/addons_test.go +++ b/tests/e2e/addons_test.go @@ -11,7 +11,7 @@ import ( ) func TestHelmChartAddOns(t *testing.T) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) configDir := t.TempDir() diff --git a/tests/e2e/basic_test.go b/tests/e2e/basic_test.go index 9e6e630..99eed02 100644 --- a/tests/e2e/basic_test.go +++ b/tests/e2e/basic_test.go @@ -18,7 +18,7 @@ import ( // TestDeployBothSimple tests deploying both components together (simplest scenario) func TestDeployBothSimple(t *testing.T) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) // Create temporary envrc file envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") @@ -41,22 +41,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) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") if err != nil { @@ -89,12 +78,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 { diff --git a/tests/e2e/custom_registry_test.go b/tests/e2e/custom_registry_test.go index 9972a6c..9635aae 100644 --- a/tests/e2e/custom_registry_test.go +++ b/tests/e2e/custom_registry_test.go @@ -14,7 +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) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) const stackroxIORegistry = "quay.io/stackrox-io" @@ -30,12 +30,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 diff --git a/tests/e2e/custom_tls_test.go b/tests/e2e/custom_tls_test.go index 62fe413..3bb9fff 100644 --- a/tests/e2e/custom_tls_test.go +++ b/tests/e2e/custom_tls_test.go @@ -22,7 +22,7 @@ import ( ) func TestDeployWithCustomTLSCert(t *testing.T) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) const secretName = "custom-central-tls" @@ -54,12 +54,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 diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index d5f70bd..75c39a9 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -58,7 +58,7 @@ func lookupLatestTag() (string, error) { } func TestDeployBothComponentsTogetherInSingleNamespace(t *testing.T) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) // Create temporary envrc file. envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") @@ -74,11 +74,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) } diff --git a/tests/e2e/helpers.go b/tests/e2e/helpers.go index db5312b..05fb226 100644 --- a/tests/e2e/helpers.go +++ b/tests/e2e/helpers.go @@ -43,14 +43,46 @@ var ( roxieBinary = "roxie" ) +func registerClusterDumpAndCleanup(t *testing.T) { + t.Helper() + ensureCleanCluster(t) + dumpClusterStateOnFailure(t) +} + +func ensureCleanCluster(t *testing.T) { + t.Helper() + t.Cleanup(func() { + assert.NoError(t, teardownAllDeployments(), "Failed to tear down deployments") + + for _, tc := range []struct { + component string + namespace string + deployment string + }{ + {"Central", centralNamespace, "central"}, + {"Central", sharedNamespace, "central"}, + {"SecuredCluster", sensorNamespace, "sensor"}, + {"SecuredCluster", sharedNamespace, "sensor"}, + {"Operator", operatorSystemNamespace, operatorDeploymentName}, + {"Operator", operatorCentralNamespace, operatorDeploymentName}, + {"Operator", operatorSensorNamespace, operatorDeploymentName}, + } { + assert.False(t, doesDeploymentExist(t, tc.namespace, tc.deployment), + "%s should not exist in %s after teardown", tc.component, tc.namespace) + } + }) +} + func teardownAllDeployments() error { - fmt.Println("=== Tearing down all deployments before running tests ===") + fmt.Println("=== Tearing down all deployments ===") ctx, cancel := context.WithTimeout(context.Background(), teardownTimeout) defer cancel() // Teardown standard deployments - cmd := exec.CommandContext(ctx, roxieBinary, "teardown", "--skip-user-config") + // Teardown "both" components to keep operator running. + // This will be improved once the automatic discovery for teardown lands. + cmd := exec.CommandContext(ctx, roxieBinary, "teardown", "--skip-user-config", "both") cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if err := cmd.Run(); err != nil { @@ -71,6 +103,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 @@ -153,6 +192,8 @@ func verifyNamespaceExists(t *testing.T, namespace string) { if err := cmd.Run(); err != nil { t.Fatalf("Namespace %s does not exist", namespace) } + + t.Logf("✓ Namespace %s exists", namespace) } func verifyNamespaceHasLabel(t *testing.T, namespace, key, value string) { @@ -233,22 +274,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, diff --git a/tests/e2e/mixed_versions_test.go b/tests/e2e/mixed_versions_test.go index b3a4c67..f7a9310 100644 --- a/tests/e2e/mixed_versions_test.go +++ b/tests/e2e/mixed_versions_test.go @@ -50,7 +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) { - dumpClusterStateOnFailure(t) + registerClusterDumpAndCleanup(t) mainTag, centralTag := lookupTwoReleasedTags(t) @@ -99,13 +99,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) } diff --git a/tests/e2e/olm_switch_test.go b/tests/e2e/olm_switch_test.go index 37821cc..850ad75 100644 --- a/tests/e2e/olm_switch_test.go +++ b/tests/e2e/olm_switch_test.go @@ -38,11 +38,8 @@ func verifyOperatorMode(t *testing.T, expectOLM bool) { // TestOLMToNonOLMSwitch tests switching from OLM operator to non-OLM operator func TestOLMToNonOLMSwitch(t *testing.T) { - dumpClusterStateOnFailure(t) - - if os.Getenv("SKIP_OLM_TESTS") != "" { - t.Skip("SKIP_OLM_TESTS is set") - } + skipUnlessOLM(t) + registerClusterDumpAndCleanup(t) // Create temporary envrc file envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") @@ -67,7 +64,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,22 +76,12 @@ 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) { - dumpClusterStateOnFailure(t) - - if os.Getenv("SKIP_OLM_TESTS") != "" { - t.Skip("SKIP_OLM_TESTS is set") - } + skipUnlessOLM(t) + registerClusterDumpAndCleanup(t) // Create temporary envrc file envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") @@ -104,7 +93,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,23 +118,13 @@ 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) { - dumpClusterStateOnFailure(t) - - if os.Getenv("SKIP_OLM_TESTS") != "" { - t.Skip("SKIP_OLM_TESTS is set") - } + skipUnlessOLM(t) + registerClusterDumpAndCleanup(t) // 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. @@ -210,22 +191,12 @@ 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) { - dumpClusterStateOnFailure(t) - - if os.Getenv("SKIP_OLM_TESTS") != "" { - t.Skip("SKIP_OLM_TESTS is set") - } + skipUnlessOLM(t) + registerClusterDumpAndCleanup(t) // Create temporary envrc file envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") @@ -260,20 +231,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) }