-
Notifications
You must be signed in to change notification settings - Fork 84
fix(e2e): improve MongoDB test application stability and resource configuration #2013
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
WalkthroughUpdated MongoDB container images to version 7.0 across three sample application configurations. Added memory resource requests (512Mi) and limits (1Gi). Replaced TCP socket–based probes with exec-based mongosh ping commands for readiness and liveness checks. Adjusted probe timing parameters and failure thresholds. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
Remove local must-gather directory and build process in favor of using the external quay.io/konveyor/oadp-must-gather:latest image via oc adm must-gather. This eliminates architecture mismatch issues and keeps must-gather code in its dedicated repository. Changes: - Updated RunMustGather() in tests/e2e/lib/apps.go to use oc adm must-gather - Added MUST_GATHER_IMAGE env var (defaults to quay.io/konveyor/oadp-must-gather:latest) - Removed build-must-gather target from Makefile - Removed entire must-gather/ directory (3,174 lines deleted) - Updated documentation in TESTING.md The SKIP_MUST_GATHER flag is preserved for skipping must-gather collection. Version-specific images can be used by setting MUST_GATHER_IMAGE env var. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Revert "refactor(e2e): migrate to external oadp-must-gather container image" This reverts commit 09a2a49. Revert "fix(e2e): derive must-gather directory pattern from image name" This reverts commit 2ae6d45. fix(e2e): update mongo image version and resource limits in mongo-persistent deployment fix(e2e): update mongo image version and resource limits in mongo-persistent deployment fix(e2e): derive must-gather directory pattern from image name The directory pattern was hardcoded to 'quay-io-konveyor-oadp-must-gather-*' which breaks when using custom images via MUST_GATHER_IMAGE env var. Now dynamically derives the pattern from the actual image name by replacing registry separators (. / :) with hyphens to match oc adm must-gather's directory naming convention. Examples: - quay.io/konveyor/oadp-must-gather:latest -> quay-io-konveyor-oadp-must-gather-latest-* - docker.io/myuser/custom:v1 -> docker-io-myuser-custom-v1-* 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yaml(4 hunks)tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yaml(2 hunks)tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yaml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yamltests/e2e/sample-applications/mongo-persistent/mongo-persistent.yamltests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yaml
🔇 Additional comments (1)
tests/e2e/sample-applications/mongo-persistent/mongo-persistent.yaml (1)
98-102: Resource configuration is appropriate for sample applications.The memory request (512Mi) and limit (1Gi) are reasonable defaults for a sample MongoDB deployment.
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | ||
| initialDelaySeconds: 30 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 3 | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | ||
| initialDelaySeconds: 60 | ||
| periodSeconds: 30 | ||
| timeoutSeconds: 10 | ||
| failureThreshold: 3 |
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.
Authentication mismatch between startup and readiness/liveness probes.
Same authentication inconsistency as the other two files: readinessProbe and livenessProbe lack credentials while startupProbe authenticates, causing probe failures once MongoDB enforces authentication.
Apply the authentication fix:
readinessProbe:
exec:
command:
- /bin/bash
- -c
- - "mongosh --eval 'db.runCommand(\"ping\")' --quiet"
+ - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet"
initialDelaySeconds: 30
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
livenessProbe:
exec:
command:
- /bin/bash
- -c
- - "mongosh --eval 'db.runCommand(\"ping\")' --quiet"
+ - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet"
initialDelaySeconds: 60
periodSeconds: 30
timeoutSeconds: 10
failureThreshold: 3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 30 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| failureThreshold: 3 | |
| livenessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 60 | |
| periodSeconds: 30 | |
| timeoutSeconds: 10 | |
| failureThreshold: 3 | |
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 30 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| failureThreshold: 3 | |
| livenessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 60 | |
| periodSeconds: 30 | |
| timeoutSeconds: 10 | |
| failureThreshold: 3 |
🤖 Prompt for AI Agents
In tests/e2e/sample-applications/mongo-persistent/mongo-persistent-block.yaml
around lines 136 to 155, the readinessProbe and livenessProbe call mongosh
without authentication while startupProbe uses credentials, causing failures
when MongoDB requires auth; update both probes to use the same authentication
parameters as startupProbe (e.g., add the same --username, --password and
--authenticationDatabase flags or use the identical authenticated connection
string used by startupProbe) so all probes authenticate consistently and succeed
once auth is enforced.
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | ||
| initialDelaySeconds: 30 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 3 | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | ||
| initialDelaySeconds: 60 | ||
| periodSeconds: 30 | ||
| timeoutSeconds: 10 | ||
| failureThreshold: 3 |
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.
Authentication mismatch between startup and readiness/liveness probes.
Same issue as in mongo-persistent.yaml: readinessProbe and livenessProbe lack credentials while startupProbe authenticates. This will cause readiness/liveness probe failures once MongoDB enforces authentication.
Apply the same fix as in mongo-persistent.yaml to add authentication:
readinessProbe:
exec:
command:
- /bin/bash
- -c
- - "mongosh --eval 'db.runCommand(\"ping\")' --quiet"
+ - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet"
initialDelaySeconds: 30
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
livenessProbe:
exec:
command:
- /bin/bash
- -c
- - "mongosh --eval 'db.runCommand(\"ping\")' --quiet"
+ - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet"
initialDelaySeconds: 60
periodSeconds: 30
timeoutSeconds: 10
failureThreshold: 3🤖 Prompt for AI Agents
In tests/e2e/sample-applications/mongo-persistent/mongo-persistent-csi.yaml
around lines 93 to 112, the readinessProbe and livenessProbe exec commands call
mongosh without credentials while the startupProbe uses authentication; update
both probes to use the same authentication options as the startupProbe (e.g.,
add --username, --password and --authenticationDatabase or switch to the same
authenticated connection string used by startupProbe) so the probes can
authenticate once MongoDB enforces auth; ensure shell quoting/escaping matches
the startupProbe invocation.
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | ||
| initialDelaySeconds: 30 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 3 | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | ||
| initialDelaySeconds: 60 | ||
| periodSeconds: 30 | ||
| timeoutSeconds: 10 | ||
| failureThreshold: 3 |
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.
Authentication mismatch between startup and readiness/liveness probes.
The readinessProbe and livenessProbe execute mongosh without credentials (lines 106-115, 116-125), while the startupProbe (line 127+) authenticates using MONGO_INITDB_ROOT_USERNAME and MONGO_INITDB_ROOT_PASSWORD. Since MongoDB is initialized with root credentials via environment variables, the readiness and liveness probes will fail with authentication errors, preventing the pod from becoming ready even after startup succeeds.
Apply this diff to add authentication to both probes:
readinessProbe:
exec:
command:
- /bin/bash
- -c
- - "mongosh --eval 'db.runCommand(\"ping\")' --quiet"
+ - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet"
initialDelaySeconds: 30
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
livenessProbe:
exec:
command:
- /bin/bash
- -c
- - "mongosh --eval 'db.runCommand(\"ping\")' --quiet"
+ - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet"
initialDelaySeconds: 60
periodSeconds: 30
timeoutSeconds: 10
failureThreshold: 3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 30 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| failureThreshold: 3 | |
| livenessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 60 | |
| periodSeconds: 30 | |
| timeoutSeconds: 10 | |
| failureThreshold: 3 | |
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 30 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| failureThreshold: 3 | |
| livenessProbe: | |
| exec: | |
| command: | |
| - /bin/bash | |
| - -c | |
| - "mongosh admin --authenticationDatabase admin -u \"$MONGO_INITDB_ROOT_USERNAME\" -p \"$MONGO_INITDB_ROOT_PASSWORD\" --eval 'db.runCommand(\"ping\")' --quiet" | |
| initialDelaySeconds: 60 | |
| periodSeconds: 30 | |
| timeoutSeconds: 10 | |
| failureThreshold: 3 |
|
/retest |
|
/test 4.19-e2e-test-aws |
|
/retest |
|
@kaovilai: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@mpryc @kaovilai @shubham-pampattiwar ha.. this worked :) health checks + pinning mongo to a more stable version. |
|
/retest ai-retester: The test The The test |
weshayutin
left a comment
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.
woot!
|
/LGTM |
@kaovilai PFFT AI |
mpryc
left a comment
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.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, mpryc, weshayutin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
This PR improves the reliability and stability of the MongoDB persistent test application used in E2E tests across all deployment variants (standard, CSI, and block storage).
Changes Made
1. Image Version Pinning
latestto7.0latestcan introduce unexpected breaking changes and make tests non-deterministic. Pinning to MongoDB 7.0 ensures consistent behavior across test runs.2. Resource Configuration Improvements
3. Enhanced Health Probes
Readiness Probe (new):
mongoshwith database ping commandLiveness Probe (improved):
mongoshpingStartup Probe (optimized):
Why These Changes Were Made
The MongoDB test pods were experiencing stability issues in CI environments, particularly:
How to Test
Deploy the MongoDB test application:
Verify pod startup and readiness:
Check probe status and resource usage:
Run E2E tests that use this application to verify backup/restore functionality still works correctly.
Impact
Note
Responses generated with Claude