diff --git a/AGENTS.md b/AGENTS.md index b712030d1..13ddb169b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -109,6 +109,9 @@ Use this as a quick reference for common decision points. When you encounter a s - **IF** testing internal/private functions **THEN** use `package controllers` (internal) - **IF** test needs async wait **THEN** use `Eventually()`, not `time.Sleep()` - **IF** documenting test steps **THEN** use `By("step description")` +- **IF** e2e test creates DevWorkspace **THEN** MUST use `DeleteDevWorkspaceAndWait` in cleanup (AfterAll or AfterEach) +- **IF** e2e test suite is `ginkgo.Ordered` **THEN** use `AfterAll` for cleanup +- **IF** e2e test runs multiple times **THEN** use `AfterEach` for cleanup ### Workspace Bootstrapping Decisions @@ -430,6 +433,70 @@ var _ = Describe("DevWorkspace Controller", func() { }) ``` +### E2E Test Cleanup Pattern + +**AI Agent Note**: ALWAYS add PVC cleanup to e2e tests to prevent conflicts between test runs, especially in CI environments. + +**Critical**: DevWorkspaces use a shared PVC (`claim-devworkspace`) that persists after workspace deletion. Without proper cleanup, subsequent tests can fail due to PVC conflicts or stale data. + +**Pattern**: Use `DeleteDevWorkspaceAndWait` in cleanup blocks: + +```go +var _ = ginkgo.Describe("[Test Suite Name]", ginkgo.Ordered, func() { + defer ginkgo.GinkgoRecover() + + const workspaceName = "test-workspace" + + ginkgo.AfterAll(func() { + // Cleanup workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) + }) + + ginkgo.It("Test case", func() { + // Test implementation + }) +}) +``` + +**Decision Tree for Cleanup**: +- **IF** test suite runs multiple times with different workspaces **THEN** use `AfterEach` +- **IF** test suite uses `ginkgo.Ordered` (sequential tests on same workspace) **THEN** use `AfterAll` +- **IF** test creates workspace **THEN** MUST include cleanup with `DeleteDevWorkspaceAndWait` + +**Available Helper Functions** (in `test/e2e/pkg/client/devws.go`): +- `DeleteDevWorkspace(name, namespace)` - Deletes workspace only (fast, may leave PVC) +- `WaitForPVCDeleted(pvcName, namespace, timeout)` - Waits for PVC deletion +- `DeleteDevWorkspaceAndWait(name, namespace)` - Deletes workspace and waits for PVC cleanup (RECOMMENDED) + +**Example: AfterEach Pattern** (for tests running multiple times): + +```go +ginkgo.Context("Test context", func() { + const workspaceName = "test-workspace" + + ginkgo.BeforeEach(func() { + // Setup + }) + + ginkgo.It("Test case", func() { + // Test implementation + }) + + ginkgo.AfterEach(func() { + // Cleanup workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) + }) +}) +``` + +**Why This Matters**: +- **CI Flakiness**: Without PVC cleanup, tests can fail intermittently in CI with "PVC already exists" errors +- **Stale Data**: Old PVC data can affect test results and cause false positives/negatives +- **Cloud Environments**: PVC deletion can be slow (30-60+ seconds), requiring explicit wait +- **Test Isolation**: Each test should start with a clean state + ### Deep Copy Pattern **AI Agent Note**: Always DeepCopy objects from cache before modifying to avoid race conditions. @@ -540,6 +607,8 @@ if err := r.Update(ctx, workspaceCopy); err != nil { - ❌ Don't commit disabled tests without tracking issues - ❌ Don't write tests that depend on timing (use Eventually/Consistently) - ❌ Don't leave test resources unmanaged (always clean up) +- ❌ Don't forget PVC cleanup in e2e tests (use `DeleteDevWorkspaceAndWait`) +- ❌ Don't use `DeleteDevWorkspace` alone in e2e tests (PVC may persist and cause conflicts) ## Debugging diff --git a/test/e2e/pkg/client/client.go b/test/e2e/pkg/client/client.go index da92bd118..c8e644070 100644 --- a/test/e2e/pkg/client/client.go +++ b/test/e2e/pkg/client/client.go @@ -17,21 +17,21 @@ package client import ( "fmt" - "os" - - dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "k8s.io/apimachinery/pkg/runtime" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" - "log" + "os" "os/exec" "strconv" "time" + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/clientcmd" crclient "sigs.k8s.io/controller-runtime/pkg/client" + + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" ) var ( @@ -41,6 +41,7 @@ var ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(dw.AddToScheme(scheme)) + utilruntime.Must(controllerv1alpha1.AddToScheme(scheme)) } type K8sClient struct { @@ -124,6 +125,11 @@ func (c *K8sClient) Kube() kubernetes.Interface { return c.kubeClient } +// ControllerRuntimeClient returns the controller-runtime client for accessing custom resources. +func (c *K8sClient) ControllerRuntimeClient() crclient.Client { + return c.crClient +} + // read a source file and copy to the selected path func copyFile(sourceFile string, destinationFile string) error { input, err := os.ReadFile(sourceFile) diff --git a/test/e2e/pkg/client/devws.go b/test/e2e/pkg/client/devws.go index ab7d48020..6db93f82e 100644 --- a/test/e2e/pkg/client/devws.go +++ b/test/e2e/pkg/client/devws.go @@ -1,5 +1,5 @@ // -// Copyright (c) 2019-2025 Red Hat, Inc. +// Copyright (c) 2019-2026 Red Hat, Inc. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -26,7 +26,9 @@ import ( dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" k8sErrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" ) func (w *K8sClient) UpdateDevWorkspaceStarted(name, namespace string, started bool) error { @@ -105,3 +107,35 @@ func (w *K8sClient) DeleteDevWorkspace(name, namespace string) error { } return nil } + +// WaitForPVCDeleted waits for a PVC to be fully deleted from the cluster. +// Returns true if deleted successfully, false if timeout occurred. +func (w *K8sClient) WaitForPVCDeleted(pvcName, namespace string, timeout time.Duration) (bool, error) { + deleted := false + err := wait.PollImmediate(2*time.Second, timeout, func() (bool, error) { + _, err := w.Kube().CoreV1().PersistentVolumeClaims(namespace). + Get(context.TODO(), pvcName, metav1.GetOptions{}) + + if k8sErrors.IsNotFound(err) { + deleted = true + return true, nil + } + if err != nil { + return false, err + } + return false, nil + }) + return deleted, err +} + +// DeleteDevWorkspaceAndWait deletes a workspace and waits for its PVC to be fully removed. +// This ensures proper cleanup and prevents PVC conflicts in subsequent tests. +func (w *K8sClient) DeleteDevWorkspaceAndWait(name, namespace string) error { + if err := w.DeleteDevWorkspace(name, namespace); err != nil { + return err + } + + // Wait for shared PVC to be deleted (may take time in cloud environments) + _, err := w.WaitForPVCDeleted("claim-devworkspace", namespace, 2*time.Minute) + return err +} diff --git a/test/e2e/pkg/tests/custom_init_container_tests.go b/test/e2e/pkg/tests/custom_init_container_tests.go index 8f18d0a40..01c3bae32 100644 --- a/test/e2e/pkg/tests/custom_init_container_tests.go +++ b/test/e2e/pkg/tests/custom_init_container_tests.go @@ -1,5 +1,5 @@ // -// Copyright (c) 2019-2025 Red Hat, Inc. +// Copyright (c) 2019-2026 Red Hat, Inc. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -23,10 +23,13 @@ import ( "runtime" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/test/e2e/pkg/config" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/test/e2e/pkg/config" ) // getProjectRoot returns the project root directory by navigating up from this file. @@ -35,9 +38,49 @@ func getProjectRoot() string { return filepath.Join(filepath.Dir(filename), "..", "..", "..", "..") } -var _ = ginkgo.Describe("[Custom Init Container Tests]", func() { +var _ = ginkgo.Describe("[Custom Init Container Tests]", ginkgo.Ordered, func() { defer ginkgo.GinkgoRecover() + var originalConfig *controllerv1alpha1.OperatorConfiguration + + ginkgo.BeforeAll(func() { + // Save original DWOC configuration to restore after tests + ctx := context.Background() + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{} + err := config.AdminK8sClient.ControllerRuntimeClient().Get(ctx, types.NamespacedName{ + Name: "devworkspace-operator-config", + Namespace: config.OperatorNamespace, + }, dwoc) + if err != nil { + ginkgo.Fail(fmt.Sprintf("Failed to get original DWOC: %s", err)) + } + // Deep copy the config to save it + if dwoc.Config != nil { + originalConfig = dwoc.Config.DeepCopy() + } + }) + + ginkgo.AfterAll(func() { + // Restore original DWOC configuration to prevent config leaks between test runs + ctx := context.Background() + dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{} + err := config.AdminK8sClient.ControllerRuntimeClient().Get(ctx, types.NamespacedName{ + Name: "devworkspace-operator-config", + Namespace: config.OperatorNamespace, + }, dwoc) + if err != nil { + ginkgo.Fail(fmt.Sprintf("Failed to get current DWOC for restoration: %s", err)) + } + + // Restore the original config + dwoc.Config = originalConfig + + err = config.AdminK8sClient.ControllerRuntimeClient().Update(ctx, dwoc) + if err != nil { + ginkgo.Fail(fmt.Sprintf("Failed to restore original DWOC configuration: %s", err)) + } + }) + ginkgo.It("Wait DevWorkspace Webhook Server Pod", func() { controllerLabel := "app.kubernetes.io/name=devworkspace-webhook-server" @@ -95,8 +138,9 @@ var _ = ginkgo.Describe("[Custom Init Container Tests]", func() { }) ginkgo.AfterEach(func() { - // Cleanup workspace - _ = config.DevK8sClient.DeleteDevWorkspace(workspaceName, config.DevWorkspaceNamespace) + // Clean up workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) }) }) @@ -149,8 +193,9 @@ var _ = ginkgo.Describe("[Custom Init Container Tests]", func() { }) ginkgo.AfterEach(func() { - // Cleanup workspace - _ = config.DevK8sClient.DeleteDevWorkspace(workspaceName, config.DevWorkspaceNamespace) + // Clean up workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) }) }) }) diff --git a/test/e2e/pkg/tests/devworkspace_debug_poststart_tests.go b/test/e2e/pkg/tests/devworkspace_debug_poststart_tests.go index 06edf0fed..e3a1fdb61 100644 --- a/test/e2e/pkg/tests/devworkspace_debug_poststart_tests.go +++ b/test/e2e/pkg/tests/devworkspace_debug_poststart_tests.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2025 Red Hat, Inc. +// Copyright (c) 2019-2026 Red Hat, Inc. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -22,9 +22,17 @@ import ( "github.com/onsi/ginkgo/v2" ) -var _ = ginkgo.Describe("[DevWorkspace Debug Start Mode]", func() { +var _ = ginkgo.Describe("[DevWorkspace Debug Start Mode]", ginkgo.Ordered, func() { defer ginkgo.GinkgoRecover() + const workspaceName = "code-latest-with-debug-start" + + ginkgo.AfterAll(func() { + // Clean up workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) + }) + ginkgo.It("Wait DevWorkspace Webhook Server Pod", func() { controllerLabel := "app.kubernetes.io/name=devworkspace-webhook-server" @@ -39,26 +47,26 @@ var _ = ginkgo.Describe("[DevWorkspace Debug Start Mode]", func() { } }) - ginkgo.It("Add Debug DevWorkspace to cluster and wait starting status", func() { + ginkgo.It("Add Debug DevWorkspace to cluster and wait running status", func() { commandResult, err := config.DevK8sClient.OcApplyWorkspace(config.DevWorkspaceNamespace, "test/resources/simple-devworkspace-debug-start-annotation.yaml") if err != nil { ginkgo.Fail(fmt.Sprintf("Failed to create DevWorkspace: %s %s", err.Error(), commandResult)) return } - deploy, err := config.DevK8sClient.WaitDevWsStatus("code-latest-with-debug-start", config.DevWorkspaceNamespace, dw.DevWorkspaceStatusStarting) + deploy, err := config.DevK8sClient.WaitDevWsStatus(workspaceName, config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) if !deploy { ginkgo.Fail(fmt.Sprintf("DevWorkspace didn't start properly. Error: %s", err)) } }) ginkgo.It("Check DevWorkspace Conditions for Debug Start message", func() { - devWorkspaceStatus, err := config.DevK8sClient.GetDevWsStatus("code-latest-with-debug-start", config.DevWorkspaceNamespace) + devWorkspaceStatus, err := config.DevK8sClient.GetDevWsStatus(workspaceName, config.DevWorkspaceNamespace) if err != nil { ginkgo.Fail(fmt.Sprintf("Failure in fetching DevWorkspace status. Error: %s", err)) } - expectedSubstring := "Debug mode: failed postStart commands will be trapped; inspect logs/exec to debug" + expectedSubstring := "DevWorkspace is starting in debug mode" found := false for _, cond := range devWorkspaceStatus.Conditions { diff --git a/test/e2e/pkg/tests/devworkspace_restart_tests.go b/test/e2e/pkg/tests/devworkspace_restart_tests.go index 2e45eee40..1d9325a7c 100644 --- a/test/e2e/pkg/tests/devworkspace_restart_tests.go +++ b/test/e2e/pkg/tests/devworkspace_restart_tests.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2025 Red Hat, Inc. +// Copyright (c) 2019-2026 Red Hat, Inc. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -21,9 +21,17 @@ import ( "github.com/onsi/gomega" ) -var _ = ginkgo.Describe("[Create DevWorkspace and ensure data is persisted during restarts]", func() { +var _ = ginkgo.Describe("[Create DevWorkspace and ensure data is persisted during restarts]", ginkgo.Ordered, func() { defer ginkgo.GinkgoRecover() + const workspaceName = "code-latest" + + ginkgo.AfterAll(func() { + // Cleanup workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) + }) + ginkgo.It("Wait DevWorkspace Webhook Server Pod", func() { controllerLabel := "app.kubernetes.io/name=devworkspace-webhook-server" @@ -45,7 +53,7 @@ var _ = ginkgo.Describe("[Create DevWorkspace and ensure data is persisted durin return } - deploy, err := config.DevK8sClient.WaitDevWsStatus("code-latest", config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) + deploy, err := config.DevK8sClient.WaitDevWsStatus(workspaceName, config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) if !deploy { ginkgo.Fail(fmt.Sprintf("DevWorkspace didn't start properly. Error: %s", err)) } @@ -74,22 +82,22 @@ var _ = ginkgo.Describe("[Create DevWorkspace and ensure data is persisted durin }) ginkgo.It("Stop DevWorkspace", func() { - err := config.AdminK8sClient.UpdateDevWorkspaceStarted("code-latest", config.DevWorkspaceNamespace, false) + err := config.AdminK8sClient.UpdateDevWorkspaceStarted(workspaceName, config.DevWorkspaceNamespace, false) if err != nil { ginkgo.Fail(fmt.Sprintf("failed to stop DevWorkspace container, returned: %s", err)) } - deploy, err := config.DevK8sClient.WaitDevWsStatus("code-latest", config.DevWorkspaceNamespace, dw.DevWorkspaceStatusStopped) + deploy, err := config.DevK8sClient.WaitDevWsStatus(workspaceName, config.DevWorkspaceNamespace, dw.DevWorkspaceStatusStopped) if !deploy { ginkgo.Fail(fmt.Sprintf("DevWorkspace didn't start properly. Error: %s", err)) } }) ginkgo.It("Start DevWorkspace", func() { - err := config.AdminK8sClient.UpdateDevWorkspaceStarted("code-latest", config.DevWorkspaceNamespace, true) + err := config.AdminK8sClient.UpdateDevWorkspaceStarted(workspaceName, config.DevWorkspaceNamespace, true) if err != nil { ginkgo.Fail(fmt.Sprintf("failed to start DevWorkspace container")) } - deploy, err := config.DevK8sClient.WaitDevWsStatus("code-latest", config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) + deploy, err := config.DevK8sClient.WaitDevWsStatus(workspaceName, config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) if !deploy { ginkgo.Fail(fmt.Sprintf("DevWorkspace didn't start properly. Error: %s", err)) } diff --git a/test/e2e/pkg/tests/devworkspaces_tests.go b/test/e2e/pkg/tests/devworkspaces_tests.go index 1c1b36cf8..647235ae3 100644 --- a/test/e2e/pkg/tests/devworkspaces_tests.go +++ b/test/e2e/pkg/tests/devworkspaces_tests.go @@ -1,5 +1,5 @@ // -// Copyright (c) 2019-2025 Red Hat, Inc. +// Copyright (c) 2019-2026 Red Hat, Inc. // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -25,9 +25,17 @@ import ( "github.com/onsi/gomega" ) -var _ = ginkgo.Describe("[Create OpenShift Web Terminal Workspace]", func() { +var _ = ginkgo.Describe("[Create OpenShift Web Terminal Workspace]", ginkgo.Ordered, func() { defer ginkgo.GinkgoRecover() + const workspaceName = "restricted-access" + + ginkgo.AfterAll(func() { + // Cleanup workspace and wait for PVC to be fully deleted + // This prevents PVC conflicts in subsequent tests, especially in CI environments + _ = config.DevK8sClient.DeleteDevWorkspaceAndWait(workspaceName, config.DevWorkspaceNamespace) + }) + ginkgo.It("Wait DewWorkspace Webhook Server Pod", func() { controllerLabel := "app.kubernetes.io/name=devworkspace-webhook-server" @@ -49,7 +57,7 @@ var _ = ginkgo.Describe("[Create OpenShift Web Terminal Workspace]", func() { return } - deploy, err := config.DevK8sClient.WaitDevWsStatus("restricted-access", config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) + deploy, err := config.DevK8sClient.WaitDevWsStatus(workspaceName, config.DevWorkspaceNamespace, dw.DevWorkspaceStatusRunning) if !deploy { ginkgo.Fail(fmt.Sprintf("OpenShift Web terminal workspace didn't start properly. Error: %s", err)) }