feat: E2E Test suite using ginkgo#1081
feat: E2E Test suite using ginkgo#1081TheiLLeniumStudios wants to merge 15 commits intostakater:masterfrom
Conversation
…or pod annotations
rasheedamir
requested review from
Felix-Stakater,
faizanahmad055 and
msafwankarim
msafwankarim
left a comment
msafwankarim
left a comment
There was a problem hiding this comment.
Is it possible to generate list of specs titles and add it in description maybe. Just for the overview and see if there is redundant or missing stuff
Rest looks good to me
|
@msafwankarim added a list of all tests in the description |
| var secretControllerInitialized bool = false | ||
| var configmapControllerInitialized bool = false | ||
| var secretControllerInitialized = false | ||
| var configmapControllerInitialized = false |
There was a problem hiding this comment.
Should we add some kind of synchronization here with atomic or mutex? Not sure but since multiple threads might try to access this, it could cause race condition or sync issues.
|
Rest it looks okay. I ran the e2e and unit tests. It works great. |
There was a problem hiding this comment.
Pull request overview
Overhauls the end-to-end testing setup by introducing a Ginkgo v2-based e2e suite with shared utilities (workload adapters + watch-based waits), plus Makefile/CI integration for provisioning infra and running the suite.
Changes:
- Added a new Ginkgo v2 e2e test hierarchy (
core,annotations,flags,advanced,csi,argo) and a comprehensivetest/e2e/utilshelper library. - Introduced watch-based waiting utilities and a workload adapter/registry pattern to run the same specs across multiple workload types.
- Added Makefile targets, CI workflow updates, and cluster cleanup scripting; plus minor refactors/formatting across existing code.
Reviewed changes
Copilot reviewed 83 out of 89 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/loadtest/internal/cmd/run.go | Import ordering / minor formatting cleanup. |
| test/loadtest/internal/cmd/report.go | Struct field alignment / formatting. |
| test/e2e/utils/workload_statefulset.go | Adds StatefulSet workload adapter. |
| test/e2e/utils/workload_openshift.go | Adds OpenShift DeploymentConfig workload adapter. |
| test/e2e/utils/workload_job.go | Adds Job workload adapter (recreation-oriented). |
| test/e2e/utils/workload_deployment.go | Adds Deployment workload adapter (incl. pause/unpause checks). |
| test/e2e/utils/workload_daemonset.go | Adds DaemonSet workload adapter. |
| test/e2e/utils/workload_cronjob.go | Adds CronJob workload adapter (job-trigger checks). |
| test/e2e/utils/workload_argo.go | Adds Argo Rollout workload adapter. |
| test/e2e/utils/workload_adapter.go | Defines WorkloadAdapter interface + registry + capability interfaces. |
| test/e2e/utils/watch.go | Adds generic watch-based waiting helpers and timeouts. |
| test/e2e/utils/utils.go | Adds generic command runner + project-dir/kubeconfig helpers. |
| test/e2e/utils/testenv.go | Adds TestEnvironment for wiring Kubernetes clients + per-suite namespaces/releases. |
| test/e2e/utils/test_helpers_test.go | Unit tests for MergeAnnotations helper. |
| test/e2e/utils/test_helpers.go | Adds MergeAnnotations helper. |
| test/e2e/utils/rand_test.go | Unit tests for random name utilities. |
| test/e2e/utils/rand.go | Adds RandSeq/RandName helper functions. |
| test/e2e/utils/podspec.go | Adds PodSpec mutation helpers + ApplyWorkloadConfig. |
| test/e2e/utils/openshift.go | Adds OpenShift API detection helper. |
| test/e2e/utils/helm_test.go | Unit tests for Helm helper functions. |
| test/e2e/utils/helm.go | Adds Helm-based deploy/undeploy helpers for Reloader + image parsing. |
| test/e2e/utils/conditions.go | Adds generic watch conditions for readiness/annotations/envvars/CSI SPCPS checks. |
| test/e2e/utils/argo.go | Adds typed helpers for Argo Rollouts creation/deletion and installation detection. |
| test/e2e/utils/annotations.go | Adds annotation constants + builder helpers used across e2e tests. |
| test/e2e/utils/accessors.go | Adds typed accessors used by generic conditions across workload kinds. |
| test/e2e/flags/watch_globally_test.go | Adds watchGlobally flag coverage. |
| test/e2e/flags/resource_selector_test.go | Adds resourceLabelSelector flag coverage. |
| test/e2e/flags/reload_on_delete_test.go | Adds reloadOnDelete flag coverage. |
| test/e2e/flags/reload_on_create_test.go | Adds reloadOnCreate flag coverage. |
| test/e2e/flags/namespace_selector_test.go | Adds namespaceSelector flag coverage. |
| test/e2e/flags/namespace_ignore_test.go | Adds ignoreNamespaces flag coverage. |
| test/e2e/flags/ignored_workloads_test.go | Adds ignoreCronJobs/ignoreJobs flag coverage. |
| test/e2e/flags/ignore_resources_test.go | Adds ignoreConfigMaps/ignoreSecrets flag coverage. |
| test/e2e/flags/flags_suite_test.go | Flags suite bootstrapping (SetupTestEnvironment, deploy helpers). |
| test/e2e/flags/auto_reload_all_test.go | Adds autoReloadAll flag coverage. |
| test/e2e/csi/csi_suite_test.go | CSI suite bootstrapping + preflight skips. |
| test/e2e/core/core_suite_test.go | Core suite bootstrapping + optional adapter registration (Argo/OpenShift). |
| test/e2e/argo/rollout_test.go | Adds Argo-specific rollout strategy tests. |
| test/e2e/argo/argo_suite_test.go | Argo suite bootstrapping + preflight skip. |
| test/e2e/annotations/search_match_test.go | Adds search/match annotation behavior tests (incl. pending pod-template case). |
| test/e2e/annotations/resource_ignore_test.go | Adds resource ignore annotation tests. |
| test/e2e/annotations/pause_period_test.go | Adds pause-period behavior tests (incl. pending pod-template case). |
| test/e2e/annotations/annotations_suite_test.go | Annotations suite bootstrapping + optional adapter registration. |
| test/e2e/advanced/regex_test.go | Adds regex-based reload tests. |
| test/e2e/advanced/multi_container_test.go | Adds multi-container and init-container CSI tests. |
| test/e2e/advanced/advanced_suite_test.go | Advanced suite bootstrapping. |
| test/e2e/README.md | Documents e2e setup/run workflow and suite structure. |
| scripts/e2e-cluster-cleanup.sh | Adds cluster cleanup automation (Vault/CSI/Argo + test resources). |
| pkg/kube/client.go | Comment spacing cleanup. |
| pkg/common/config.go | Import ordering cleanup. |
| pkg/common/common.go | Import ordering cleanup + parameter rename in ShouldReload. |
| internal/pkg/util/util_test.go | Import ordering cleanup. |
| internal/pkg/util/util.go | Import ordering cleanup. |
| internal/pkg/util/interface.go | Changes type assertions to non-panicking style. |
| internal/pkg/testutil/kube.go | Import ordering cleanup. |
| internal/pkg/leadership/leadership_test.go | Import spacing cleanup. |
| internal/pkg/leadership/leadership.go | Import ordering cleanup + goroutine loop minor tweak. |
| internal/pkg/handler/upgrade.go | Import ordering cleanup + small naming/comment cleanups. |
| internal/pkg/handler/update.go | Import ordering cleanup. |
| internal/pkg/handler/pause_deployment_test.go | Improves error wrapping with %w + import ordering. |
| internal/pkg/handler/pause_deployment.go | Import ordering cleanup. |
| internal/pkg/handler/handlers_test.go | Adds unit tests for resource handler config/SHA behavior. |
| internal/pkg/handler/delete.go | Minor refactor for safer type assertion usage + comment cleanup. |
| internal/pkg/handler/create.go | Minor refactor for safer type assertion usage. |
| internal/pkg/controller/controller.go | Refactors controller construction, list option modifier naming, and handler type assertion safety. |
| internal/pkg/app/app.go | Minor variable rename for clarity. |
| Makefile | Adds e2e targets + updates lint/fmt/test targets. |
| .golangci.yml | Adds golangci-lint configuration (including ginkgo linter). |
| .gitignore | Ignores *.test artifacts. |
| .github/workflows/pull_request.yaml | Updates Kind version and wires e2e setup/run into PR workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| opts := metav1.ListOptions{Watch: true} | ||
| if name != "" { | ||
| opts.FieldSelector = fields.OneTermEqualSelector("metadata.name", name).String() | ||
| } |
There was a problem hiding this comment.
WatchUntil starts a watch without first checking current state (e.g., via GET/LIST). If the condition is already satisfied before the watch begins (common for fast updates like reload annotations), no event may be delivered and this will timeout/flap. Consider: (1) do an initial GET/LIST + condition check, and (2) start the watch from the returned ResourceVersion (or use client-go's watchtools.UntilWithSync-style list+watch).
| watcher, err := watchFunc(ctx, opts) | ||
| if err != nil { | ||
| return zero, false, nil | ||
| } |
There was a problem hiding this comment.
watchOnce discards errors from watchFunc (return zero, false, nil). This turns real failures (RBAC, missing API, bad selector) into silent retries until ErrWatchTimeout, which makes debugging very hard. Prefer returning the error (possibly wrapped) or at least propagating the last error when timing out.
| result, done, err := watchOnce(ctx, watchFunc, opts, condition) | ||
| if done { | ||
| return result, err | ||
| } |
There was a problem hiding this comment.
WatchUntil ignores any non-nil err from watchOnce when done is false. As written, ErrWatchError returned by watchOnce on watch.Error events is never surfaced to callers (it just loops until timeout). Either treat ErrWatchError as terminal (return it) or explicitly decide which errors are retryable and carry the last error forward.
| if cfg.UseInitContainerCSI && cfg.SPCName != "" { | ||
| AddCSIVolume(spec, 0, cfg.SPCName) | ||
| } |
There was a problem hiding this comment.
UseInitContainerCSI calls AddCSIVolume(spec, 0, ...), which mounts the CSI volume onto the first main container, not an init container. That doesn't match the flag name and will lead to incorrect workload manifests if this config is used. Either implement init-container CSI mounting (volume + mount on spec.InitContainers[...]) or remove this option to avoid misleading behavior.
| if err := os.Chdir(cmd.Dir); err != nil { | ||
| _, _ = fmt.Fprintf(GinkgoWriter, "chdir dir: %q\n", err) | ||
| } |
There was a problem hiding this comment.
Run sets cmd.Dir (good) but also calls os.Chdir(cmd.Dir), which mutates process-wide working directory and can introduce test flakiness if commands run concurrently in the same process. Since exec.Cmd already respects cmd.Dir, consider removing the os.Chdir call (or restoring the original cwd via defer).
|
|
||
| test: | ||
| "$(GOCMD)" test -timeout 1800s -v ./... | ||
| "$(GOCMD)" test -timeout 1800s -v -short -count=1 ./internal/... ./test/e2e/utils/... |
There was a problem hiding this comment.
make test now runs only ./internal/... and ./test/e2e/utils/..., which skips existing unit tests under ./pkg/... (e.g., pkg/common/common_test.go). If this is the CI entrypoint, it will silently drop coverage. Consider including ./pkg/... (or reverting to ./...) and controlling duration with -short/package exclusions instead.
3 participants
This PR introduces a complete overhaul of the e2e testing infrastructure, replacing the previous test approach with a modern, maintainable, and faster test suite built on Ginkgo v2
Key Changes
Test Framework Migration
core,annotations,flags,advanced,csi,argoRuntime Image Building
make e2e)Real Infrastructure Integration
Watch-Based Waiting Infrastructure
time.Sleep()and polling-based waits with Kubernetes watch APIWatchUntil[T runtime.Object]function for type-safe resource watchingDeploymentReady,DaemonSetReady, etc.) for clean assertionsWorkload Adapter Pattern
WorkloadAdapterinterface for all workload typesPausable,Recreatable,JobTriggerer) for workload-specific featuresTest Coverage
auto=true, typed auto (configmap.auto,secret.auto,secretproviderclass.auto), search/match, exclude, pause-period--watch-globally,--namespaces-to-ignore,--resources-to-ignore,--reload-on-create,--reload-on-delete,--auto-reload-allSTAKATER_*environment variables)Infrastructure Scripts
scripts/e2e-cluster-setup.sh- Installs Argo Rollouts, CSI Secrets Store Driver, Vault (with Kubernetes auth configured)scripts/e2e-cluster-cleanup.sh- Cleans up test resourcesmake e2e-setup,make e2e,make e2e-cleanup,make e2e-ciCode Quality
.golangci.ymlwith comprehensive linter configuration (errcheck, govet, staticcheck, ginkgolinter, etc.)List of all tests
advanced/job_reload_test.go
JobJobJobJobJobJobadvanced/multi_container_test.go
advanced/regex_test.go
annotations/auto_reload_test.go
DeploymentDeploymentDeploymentDeploymentDeploymentannotations/combination_test.go
annotations/exclude_test.go
Deployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigannotations/pause_period_test.go
DeploymentDeploymentDeploymentannotations/resource_ignore_test.go
annotations/search_match_test.go
DeploymentDeploymentDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigargo/rollout_test.go
ArgoRolloutArgoRolloutcore/reference_methods_test.go
Deployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigcore/workloads_test.go
Deployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSetDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigCronJobCronJobCronJobDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSetDeployment, DaemonSet, StatefulSetDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigDeployment, DaemonSet, StatefulSet, ArgoRollout, DeploymentConfigcsi/csi_test.go
flags/auto_reload_all_test.go
flags/ignored_workloads_test.go
CronJobCronJobCronJobflags/ignore_resources_test.go
flags/namespace_ignore_test.go
flags/namespace_selector_test.go
flags/reload_on_create_test.go
flags/reload_on_delete_test.go
flags/resource_selector_test.go
flags/watch_globally_test.go