-
Notifications
You must be signed in to change notification settings - Fork 84
Add readiness and liveness probes to MySQL and TodoList deployments; create MySQL Minikube test configuration #2017
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
base: oadp-dev
Are you sure you want to change the base?
Conversation
WalkthroughKubernetes health probe configurations are updated across three MySQL persistent deployment manifests. MySQL container livenessProbes transition from tcpSocket to exec-based checks executing SELECT 1 queries, readinessProbes are added for MySQL containers, and httpGet-based livenessProbe and readinessProbe are added to todolist containers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai 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 |
…create MySQL Minikube test configuration Signed-off-by: Tiger Kaovilai <tkaovila@redhat.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
♻️ Duplicate comments (3)
tests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yaml (1)
124-140: Critical: Duplicate issue—environment variables will not expand in exec probe command array.Same variable expansion issue as the readinessProbe (lines 107-123). The livenessProbe must also be wrapped in a shell.
Apply the same fix pattern:
livenessProbe: - exec: - command: - - /usr/bin/timeout - - 1s - - /usr/bin/mysql - - $(MYSQL_DATABASE) - - -h - - 127.0.0.1 - - -u$(MYSQL_USER) - - -p$(MYSQL_PASSWORD) - - -e - - SELECT 1 + exec: + command: + - /bin/sh + - -c + - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1" initialDelaySeconds: 30 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml (1)
116-132: Critical: Duplicate issue—environment variables will not expand in exec probe command array.Same variable expansion issue affecting the livenessProbe. Must wrap in shell for variable substitution to work.
Apply the same fix pattern:
livenessProbe: - exec: - command: - - /usr/bin/timeout - - 1s - - /usr/bin/mysql - - $(MYSQL_DATABASE) - - -h - - 127.0.0.1 - - -u$(MYSQL_USER) - - -p$(MYSQL_PASSWORD) - - -e - - SELECT 1 + exec: + command: + - /bin/sh + - -c + - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1" initialDelaySeconds: 30 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml (1)
137-153: Critical: Duplicate issue—environment variables will not expand in exec probe command array.The livenessProbe has the same variable expansion issue. Additionally, note that the pre-existing startupProbe (lines 154-170) uses the same problematic command pattern and should be fixed as well.
Apply the same fix pattern:
livenessProbe: - exec: - command: - - /usr/bin/timeout - - 1s - - /usr/bin/mysql - - $(MYSQL_DATABASE) - - -h - - 127.0.0.1 - - -u$(MYSQL_USER) - - -p$(MYSQL_PASSWORD) - - -e - - SELECT 1 + exec: + command: + - /bin/sh + - -c + - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1" initialDelaySeconds: 30 periodSeconds: 10 timeoutSeconds: 5 failureThreshold: 3Consider also fixing the startupProbe at lines 154-170 to use the same shell-wrapped approach.
📜 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/mysql-persistent/mysql-persistent-csi.yaml(2 hunks)tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml(2 hunks)tests/e2e/sample-applications/mysql-persistent/mysql-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/mysql-persistent/mysql-persistent-csi.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml
🔇 Additional comments (3)
tests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yaml (1)
214-225: TodoList health probes look good.The httpGet-based probes for the todolist container are correctly formatted and have appropriate timing (10s initialDelay for readiness, 30s for liveness).
tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml (1)
205-216: TodoList health probes look good.The httpGet-based probes for the todolist container are correctly formatted and have appropriate timing.
tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml (1)
227-238: TodoList health probes look good.The httpGet-based probes for the todolist container are correctly formatted with appropriate timing.
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /usr/bin/timeout | ||
| - 1s | ||
| - /usr/bin/mysql | ||
| - $(MYSQL_DATABASE) | ||
| - -h | ||
| - 127.0.0.1 | ||
| - -u$(MYSQL_USER) | ||
| - -p$(MYSQL_PASSWORD) | ||
| - -e | ||
| - SELECT 1 | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 5 | ||
| timeoutSeconds: 2 | ||
| 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.
Critical: Environment variables will not expand in exec probe command array.
Kubernetes exec probes execute commands directly without shell interpretation. The shell variables $(MYSQL_DATABASE), $(MYSQL_USER), and $(MYSQL_PASSWORD) in the command array will be passed as literal strings to the mysql binary, causing the health check to fail. For example, the mysql command will receive the literal argument "$(MYSQL_DATABASE)" instead of the actual database name.
The exec command must be wrapped in a shell to enable variable expansion. Apply this fix:
readinessProbe:
- exec:
- command:
- - /usr/bin/timeout
- - 1s
- - /usr/bin/mysql
- - $(MYSQL_DATABASE)
- - -h
- - 127.0.0.1
- - -u$(MYSQL_USER)
- - -p$(MYSQL_PASSWORD)
- - -e
- - SELECT 1
+ exec:
+ command:
+ - /bin/sh
+ - -c
+ - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1"
initialDelaySeconds: 10
periodSeconds: 5
timeoutSeconds: 2
failureThreshold: 3🤖 Prompt for AI Agents
In tests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yaml
around lines 107 to 123, the exec readinessProbe uses literal array entries like
$(MYSQL_DATABASE) which will not be expanded; wrap the whole command in a shell
so environment variables are expanded by replacing the array with a shell
invocation (e.g. /bin/sh -c) and pass the full timeout + mysql invocation as a
single string so the shell expands MYSQL_DATABASE, MYSQL_USER and MYSQL_PASSWORD
at runtime; ensure proper quoting of the SELECT statement and timeout invocation
within that single string.
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /usr/bin/timeout | ||
| - 1s | ||
| - /usr/bin/mysql | ||
| - $(MYSQL_DATABASE) | ||
| - -h | ||
| - 127.0.0.1 | ||
| - -u$(MYSQL_USER) | ||
| - -p$(MYSQL_PASSWORD) | ||
| - -e | ||
| - SELECT 1 | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 5 | ||
| timeoutSeconds: 2 | ||
| 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.
Critical: Environment variables will not expand in exec probe command array.
Same critical variable expansion issue as identified in mysql-persistent-csi.yaml. The readinessProbe shell variables must be wrapped in /bin/sh -c for expansion to occur.
Apply this fix:
readinessProbe:
- exec:
- command:
- - /usr/bin/timeout
- - 1s
- - /usr/bin/mysql
- - $(MYSQL_DATABASE)
- - -h
- - 127.0.0.1
- - -u$(MYSQL_USER)
- - -p$(MYSQL_PASSWORD)
- - -e
- - SELECT 1
+ exec:
+ command:
+ - /bin/sh
+ - -c
+ - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1"
initialDelaySeconds: 10
periodSeconds: 5
timeoutSeconds: 2
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: | |
| - /usr/bin/timeout | |
| - 1s | |
| - /usr/bin/mysql | |
| - $(MYSQL_DATABASE) | |
| - -h | |
| - 127.0.0.1 | |
| - -u$(MYSQL_USER) | |
| - -p$(MYSQL_PASSWORD) | |
| - -e | |
| - SELECT 1 | |
| initialDelaySeconds: 10 | |
| periodSeconds: 5 | |
| timeoutSeconds: 2 | |
| failureThreshold: 3 | |
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/sh | |
| - -c | |
| - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1" | |
| initialDelaySeconds: 10 | |
| periodSeconds: 5 | |
| timeoutSeconds: 2 | |
| failureThreshold: 3 |
🤖 Prompt for AI Agents
In
tests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yaml
around lines 99 to 115, the readinessProbe uses an exec command array containing
shell variables (e.g. $(MYSQL_DATABASE), $(MYSQL_USER), $(MYSQL_PASSWORD)) which
will not be expanded; change the exec to run a shell so variables expand by
replacing the command array with a single shell invocation like /bin/sh -c and
pass the full command as one string (e.g. use /usr/bin/timeout 1s /usr/bin/mysql
"$(MYSQL_DATABASE)" -h 127.0.0.1 -u"$(MYSQL_USER)" -p"$(MYSQL_PASSWORD)" -e
'SELECT 1'), keeping the existing probe timings intact.
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /usr/bin/timeout | ||
| - 1s | ||
| - /usr/bin/mysql | ||
| - $(MYSQL_DATABASE) | ||
| - -h | ||
| - 127.0.0.1 | ||
| - -u$(MYSQL_USER) | ||
| - -p$(MYSQL_PASSWORD) | ||
| - -e | ||
| - SELECT 1 | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 5 | ||
| timeoutSeconds: 2 | ||
| 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.
Critical: Environment variables will not expand in exec probe command array.
Same critical variable expansion issue identified across all three YAML files. The readinessProbe command array contains literal $(MYSQL_DATABASE), $(MYSQL_USER), and $(MYSQL_PASSWORD) strings that Kubernetes will not expand. These must be wrapped in a shell for variable substitution.
Apply this fix:
readinessProbe:
- exec:
- command:
- - /usr/bin/timeout
- - 1s
- - /usr/bin/mysql
- - $(MYSQL_DATABASE)
- - -h
- - 127.0.0.1
- - -u$(MYSQL_USER)
- - -p$(MYSQL_PASSWORD)
- - -e
- - SELECT 1
+ exec:
+ command:
+ - /bin/sh
+ - -c
+ - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1"
initialDelaySeconds: 10
- periodSeconds: 5
- timeoutSeconds: 2
- failureThreshold: 3
+ periodSeconds: 5
+ timeoutSeconds: 2
+ 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: | |
| - /usr/bin/timeout | |
| - 1s | |
| - /usr/bin/mysql | |
| - $(MYSQL_DATABASE) | |
| - -h | |
| - 127.0.0.1 | |
| - -u$(MYSQL_USER) | |
| - -p$(MYSQL_PASSWORD) | |
| - -e | |
| - SELECT 1 | |
| initialDelaySeconds: 10 | |
| periodSeconds: 5 | |
| timeoutSeconds: 2 | |
| failureThreshold: 3 | |
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/sh | |
| - -c | |
| - /usr/bin/timeout 1s /usr/bin/mysql $(MYSQL_DATABASE) -h 127.0.0.1 -u$(MYSQL_USER) -p$(MYSQL_PASSWORD) -e "SELECT 1" | |
| initialDelaySeconds: 10 | |
| periodSeconds: 5 | |
| timeoutSeconds: 2 | |
| failureThreshold: 3 |
🤖 Prompt for AI Agents
In tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yaml around
lines 120-136, the readinessProbe exec command uses literal $(MYSQL_...) tokens
which Kubernetes will not expand; change the command to run a shell so
environment variables are expanded (e.g. replace the command array with a shell
invocation like /bin/sh -c and provide a single string that runs timeout and
mysql using $MYSQL_DATABASE, $MYSQL_USER and $MYSQL_PASSWORD with proper
quoting), and apply the same change to the other two YAML files that share this
probe.
|
/retest ai-retester: The The underlying issue seems to be that 'todolist' container in pod remains waiting to start (PodInitializing), and that external AWS EBS volumes were not being successfully provisioned on time preventing the test from proceeding, leading to Pod remaining on "Pending" phase. |
|
@kaovilai: The following tests failed, say
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. |
Signed-off-by: Tiger Kaovilai tkaovila@redhat.com
Why the changes were made
This PR enhances the reliability of MySQL-based E2E test applications by adding comprehensive health probes to prevent race conditions and improve startup coordination during backup/restore operations.
Problem
MySQL E2E tests (particularly "MySQL application two Vol CSI") were experiencing intermittent failures with symptoms including:
Root Cause
The MySQL deployments only had:
The TodoList deployments had no health probes at all.
Changes Made
MySQL Container Enhancements
All three MySQL deployment files updated with:
Added readinessProbe:
Enhanced livenessProbe:
Changed from TCP socket to MySQL exec command for better failure detection:
TodoList Container Enhancements
Added HTTP-based health probes:
Files Modified
tests/e2e/sample-applications/mysql-persistent/mysql-persistent.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-csi.yamltests/e2e/sample-applications/mysql-persistent/mysql-persistent-twovol-csi.yamlExpected Benefits
How to test the changes made
Minikube Testing (Verified)
E2E Testing
The enhanced probes should improve reliability of MySQL-related E2E tests, particularly:
Probe Behavior Verification
Note
Responses generated with Claude