-
Notifications
You must be signed in to change notification settings - Fork 0
Add E2E tests with GitHub Actions CI integration #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Migrate e2e tests from Ginkgo/Gomega to sigs.k8s.io/e2e-framework - Add automatic Kind cluster creation/destruction via envfuncs - Update Makefile test-e2e target to use native Go testing - Add basic cluster connection test to verify framework setup
- Introduced a new test for verifying CRD installation and establishment. - Added utility functions for CRD setup and teardown in the e2e framework. - Created a scheme setup function to register CRDs with the testing environment.
- Implemented a new smoke test that verifies the full deployment chain of ScalityUI, ScalityUIComponent, and ScalityUIComponentExposer. - Introduced utility functions for loading and applying YAML configurations with templating support. - Updated mock server configuration to use port 80 for consistency across services. - Added test data YAML files for ScalityUI, ScalityUIComponent, and ScalityUIComponentExposer to facilitate the smoke test.
… logic - Added ForceRefreshAnnotation constant in the new annotations.go file for better organization. - Updated references to ForceRefreshAnnotation in the ScalityUIComponent controller and tests to use the new constant. - Introduced new condition types and reasons for configuration retrieval status in the controller logic. - Enhanced the handling of the force-refresh annotation during configuration processing and status updates.
…ion and error handling - Introduced a new test file for end-to-end testing of ScalityUIComponent, covering scenarios such as storm prevention, timeout handling, recovery after server failure, and ensuring no fetch without a trigger. - Implemented a builder pattern for ScalityUIComponent creation to streamline test setup. - Enhanced mock server client functionality to support testing various response scenarios. - Added utility functions for managing namespaces and mock server interactions during tests.
- Introduced a new test file for comprehensive end-to-end testing of pod lifecycle scenarios, including rolling updates, operator crash recovery, and prevention of spurious updates after restarts. - Implemented builders for ScalityUI and ScalityUIComponentExposer to streamline resource creation during tests. - Enhanced the framework with utility functions for managing deployments, replica sets, and resource version tracking. - Added detailed assessments to verify the stability and recovery of components under various conditions.
…t for ScalityUIComponentExposer - Added a new deletion handling mechanism to clean up resources associated with ScalityUIComponentExposer during deletion. - Introduced methods to remove volumes from deployments and clean up ConfigMaps, ensuring proper resource management. - Implemented finalizer management to prevent premature deletion of resources and ensure cleanup operations are completed. - Enhanced the Reconcile function to handle deletion scenarios and ensure finalizers are added when necessary.
- Introduced a new test file for end-to-end testing of cascade garbage collection scenarios, including component updates and deletion cleanup. - Implemented tests to verify the creation and configuration of ScalityUI and its components, ensuring proper volume management and finalizer handling. - Enhanced the framework with utility functions for waiting on deployment states, volume mounts, and ConfigMap finalizers. - Added assessments to validate the stability and cleanup of resources during the lifecycle of ScalityUI components.
- Introduced a new end-to-end test for verifying the cascade deletion of namespaces and associated ScalityUI components. - Implemented assessments to ensure proper cleanup of resources, including verification of component removal from deployed applications. - Enhanced the testing framework with utility functions for managing namespace lifecycle and validating deployment states during deletion scenarios.
- Introduced a new test file for end-to-end testing of multi-namespace scenarios involving ScalityUI and its components. - Implemented tests to verify the creation, configuration, and aggregation of components across multiple namespaces. - Added assessments to ensure proper resource management and validation of deployed applications during multi-namespace operations. - Enhanced the framework with utility functions for managing namespaces and verifying component states in a multi-namespace context.
- Reduced the timeout for e2e tests from 30 minutes to 10 minutes in the Makefile. - Added a new GitHub Actions workflow for running E2E tests, including steps for code checkout, Go setup with private modules, Kind installation, and executing the E2E tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive E2E testing framework using sigs.k8s.io/e2e-framework with GitHub Actions CI integration. The changes include new test infrastructure, mock server implementation, controller enhancements for proper resource cleanup, and CI workflow updates.
Changes:
- Added comprehensive E2E test framework with helper utilities and builders for testing full resource lifecycle
- Implemented mock HTTP server for testing configuration fetching behavior
- Added deletion logic with finalizers to ScalityUIComponentExposer controller for proper cascade cleanup
- Integrated E2E tests into GitHub Actions CI workflow
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
test/e2e/*.go |
New E2E test files covering smoke tests, pod lifecycle, namespace deletion, multi-namespace scenarios, HTTP behavior, and cascade GC |
test/e2e/framework/*.go |
Test framework infrastructure including builders, wait utilities, mock server client, and configuration |
test/e2e/mock-server/* |
Mock HTTP server implementation for testing configuration fetching |
test/e2e/testdata/smoke/*.yaml |
Test fixture YAML files for smoke tests |
internal/controller/scalityuicomponentexposer/deletion.go |
New deletion handler with finalizer logic for proper cleanup |
internal/controller/scalityuicomponentexposer/controller.go |
Updated controller to handle deletion and finalizers |
internal/controller/scalityuicomponent/controller.go |
Moved ForceRefreshAnnotation constant to API package |
api/v1alpha1/annotations.go |
New file defining ForceRefreshAnnotation constant |
.github/workflows/tests.yml |
Added E2E test job to CI workflow |
go.mod, go.sum |
Added e2e-framework and updated dependencies |
Makefile |
Updated test-e2e target to use new framework |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…C tests - Replaced hardcoded volume and config map suffixes with constants from the framework for consistency. - Updated test assertions to reflect the new constant usage, enhancing maintainability. - Reduced wait time in pod lifecycle tests to optimize test execution duration. - Adjusted long timeout duration in the wait framework for improved performance.
de2748b to
e96a7e5
Compare
- Increased the timeout for e2e tests from 10 minutes to 15 minutes in the Makefile. - Added steps in the GitHub Actions workflow to output Docker image information to the summary after building. - Modified the tests workflow to include a build job that pushes the Docker image and pulls it for e2e tests. - Updated the Dockerfile.e2e comment to clarify its usage for local testing versus CI.
e96a7e5 to
c817b82
Compare
- Updated the environment variable for skipping operator image builds in the GitHub Actions workflow. - Introduced a new function to determine if only the operator image build should be skipped, improving clarity and maintainability. - Modified the operator deployment logic to utilize the new skipping function, allowing for more flexible build configurations.
| return nil | ||
| } | ||
|
|
||
| func (c *MockServerClient) execCurlCommand(ctx context.Context, fromNamespace string, curlArgs []string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of os/exec, you can use the SPDY executor provided by Kubernetes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kubectl exec is simpler and works well for E2E tests. SPDY executor adds boilerplate without much benefit here
| return strings.TrimSpace(stdout.String()), nil | ||
| } | ||
|
|
||
| func (c *MockServerClient) CleanupCurlPods(ctx context.Context) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do this have been called in your test teardowns ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CleanupCurlPods is called in teardown (real_http_test.go:572)
| # restart the operator pod and should not run in parallel with other tests. | ||
| .PHONY: test-e2e | ||
| test-e2e: ## Run all e2e tests against a Kind cluster (auto-created) | ||
| go test ./test/e2e/... -v -timeout 20m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20m ? this is quite long no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the max timeout, not actual duration. E2E tests need time for pod scheduling, image pulls, and operator recovery tests. Actual runs complete much faster(10m-11m).
| "time" | ||
| ) | ||
|
|
||
| const curlImage = "curlimages/curl:8.6.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making these configurable via environment variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image version rarely changes. Keeping it simple for now.
|
Have you consider running independent test suites in parallel to reduce CI time ? |
|
The namespace setup/teardown pattern is repeated - consider a helper |
| uses: ./.github/workflows/docker.yml | ||
| with: | ||
| push-image: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} | ||
| push-image: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to push image on every PR ? I think is ok !
You can also allow running specific test suites with build tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if we want to test the image which didn't merge yet
…d timeout adjustments - Introduced new context keys for managing ScalityUI, component, and exposer names across various tests, improving clarity and maintainability. - Updated e2e test timeouts in the Makefile for better performance during test execution. - Enhanced existing tests to utilize the new context keys, ensuring consistent resource management and validation across different scenarios. - Added assessments to verify the stability and cleanup of resources during the lifecycle of ScalityUI components.
09613f4 to
7bee8c9
Compare
Each test file has different setup needs (1 vs 2 namespaces, different context keys). real_http_test.go already has helpers. Will keep as-is for now |
Already implemented - see WithParallelTestEnabled() in e2e_suite_test.go:34 and the test-e2e-safe/test-e2e-disruptive Makefile targets. |
925f37c to
d33a15d
Compare
- Removed unnecessary sleep duration in the operator crash recovery test to improve test execution speed. - Adjusted the sleep command in the mock client to reduce the wait time from 3600 seconds to 300 seconds, enhancing overall test efficiency.
d33a15d to
1c96247
Compare
E2E Test Coverage
Basic Tests (
e2e_test.go)scalityuis,scalityuicomponents,scalityuicomponentexposersare in Established conditionCascade GC Tests (
cascade_gc_test.go)2. Create Exposer
3. Verify volume added to Deployment
4. Verify new ReplicaSet created (rolling update occurred)
2. Delete Exposer
3. Verify volume removed from Deployment
4. Verify finalizer removed, ConfigMap deleted by GC
Multi-Namespace Tests (
multi_namespace_test.go)2. ns-a: 2 Components + 2 Exposers
3. ns-b: 1 Component + 1 Exposer
4. Verify deployed-apps ConfigMap contains all 3 components
5. Verify each component has correct AppHistoryBasePath
2. Delete ns-a
3. Verify deployed-apps now has only 1 component (from ns-b)
4. Verify ns-b Deployment/Exposer/ConfigMap are unaffected
Namespace Deletion Tests (
namespace_deletion_test.go)2. Verify component exists in deployed-apps
3. Delete entire namespace
4. Verify component removed from deployed-apps, count=0
Pod Lifecycle Tests (
pod_lifecycle_test.go)2. Modify Exposer's appHistoryBasePath
3. Verify hash annotation changed
4. Verify new ReplicaSet created, old RS scaled down to 0
2. Delete operator pod to simulate crash
3. Wait for new pod to start
4. Verify all conditions still True, resources intact
2. Restart operator
3. Trigger reconcile (via label change)
4. Verify all ResourceVersions unchanged
Real HTTP Tests (
real_http_test.go)2. Trigger 10 reconciles (without changing image)
3. Verify counter still=1 (LastFetchedImage mechanism works)
2. Configure mock server with 15s delay
3. Trigger refetch via force-refresh
4. Verify ConfigurationRetrieved=False, Reason=FetchFailed
2. Configure mock server to return 500, verify condition becomes False
3. Reset mock server (200)
4. Use force-refresh, verify condition becomes True
2. Reset counter=0, configure 500
3. Trigger 5 reconciles (via label change, no force-refresh)
4. Verify counter=0, condition still True
5. Add force-refresh, verify counter=1, condition becomes False