-
Notifications
You must be signed in to change notification settings - Fork 0
Add functional test CI: Docker MySQL topology + ProxySQL + failover tests #51
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
Changes from all commits
4e3df8a
607b618
53a9bc4
8c2c783
9a15a4b
b12a4c5
94a5098
ef5cc19
a0ec5e9
0ca14bd
0533931
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| name: Functional Tests | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - master | ||
| pull_request: | ||
| branches: | ||
| - master | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| functional: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 20 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.25.7' | ||
| cache: true | ||
|
|
||
| - name: Build orchestrator (for CLI tests on host) | ||
| run: go build -o bin/orchestrator ./go/cmd/orchestrator | ||
|
|
||
| - name: Start test infrastructure (MySQL + ProxySQL + Orchestrator) | ||
| working-directory: tests/functional | ||
| run: | | ||
| docker compose up -d mysql1 mysql2 mysql3 proxysql | ||
| echo "Waiting for MySQL and ProxySQL to be healthy..." | ||
| timeout 120 bash -c ' | ||
| while true; do | ||
| HEALTHY=$(docker compose ps --format json 2>/dev/null | python3 -c " | ||
| import json, sys | ||
| healthy = 0 | ||
| for line in sys.stdin: | ||
| svc = json.loads(line) | ||
| if \"healthy\" in svc.get(\"Status\",\"\").lower(): | ||
| healthy += 1 | ||
| print(healthy) | ||
| " 2>/dev/null || echo "0") | ||
| if [ "$HEALTHY" -ge 4 ]; then | ||
| echo "All 4 services healthy" | ||
| exit 0 | ||
| fi | ||
| sleep 2 | ||
| done | ||
| ' || { echo "Timeout"; docker compose ps; docker compose logs --tail=30; exit 1; } | ||
|
|
||
| - name: Setup replication | ||
| run: bash tests/functional/setup-replication.sh | ||
|
|
||
| - name: Start orchestrator in Docker network | ||
| working-directory: tests/functional | ||
| run: | | ||
| docker compose up -d orchestrator | ||
| echo "Waiting for orchestrator to be ready..." | ||
| timeout 120 bash -c ' | ||
| while true; do | ||
| if curl -sf http://localhost:3099/api/clusters > /dev/null 2>&1; then | ||
| echo "Orchestrator ready" | ||
| exit 0 | ||
| fi | ||
| sleep 2 | ||
| done | ||
| ' || { echo "Orchestrator not ready"; docker compose logs orchestrator --tail=50; exit 1; } | ||
|
|
||
| - name: Run smoke tests | ||
| run: bash tests/functional/test-smoke.sh | ||
|
|
||
| - name: Run regression tests | ||
| run: bash tests/functional/test-regression.sh | ||
|
|
||
| - name: Run failover tests | ||
| run: bash tests/functional/test-failover.sh | ||
|
|
||
| - name: Collect orchestrator logs | ||
| if: always() | ||
| working-directory: tests/functional | ||
| run: docker compose logs orchestrator > /tmp/orchestrator-test.log 2>&1 || true | ||
|
|
||
| - name: Upload orchestrator logs | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: orchestrator-test-logs | ||
| path: /tmp/orchestrator-test.log | ||
|
|
||
| - name: Collect all docker logs on failure | ||
| if: failure() | ||
| working-directory: tests/functional | ||
| run: docker compose logs > /tmp/docker-compose-logs.txt 2>&1 || true | ||
|
|
||
| - name: Upload docker logs on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: docker-compose-logs | ||
| path: /tmp/docker-compose-logs.txt | ||
|
|
||
| - name: Cleanup | ||
| if: always() | ||
| working-directory: tests/functional | ||
| run: docker compose down -v --remove-orphans 2>/dev/null || true |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,127 @@ | ||||||
| version: "3.8" | ||||||
|
|
||||||
| services: | ||||||
| mysql1: | ||||||
| image: mysql:8.4 | ||||||
| hostname: mysql1 | ||||||
| environment: | ||||||
| MYSQL_ROOT_PASSWORD: testpass | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded credentials are used here and throughout the new test files. While this is a test environment, it's a security risk and a bad practice that could be copied elsewhere. Consider using environment variables to manage secrets. You can define them in a This issue is also present in:
Comment on lines
+4
to
+8
|
||||||
| ports: | ||||||
| - "13306:3306" | ||||||
| volumes: | ||||||
| - ./mysql/master.cnf:/etc/mysql/conf.d/repl.cnf | ||||||
| - ./mysql/init-master.sql:/docker-entrypoint-initdb.d/init.sql | ||||||
| healthcheck: | ||||||
| test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-uroot", "-ptestpass"] | ||||||
| interval: 5s | ||||||
| timeout: 3s | ||||||
| retries: 30 | ||||||
| networks: | ||||||
| orchnet: | ||||||
| aliases: | ||||||
| - mysql1 | ||||||
|
|
||||||
| mysql2: | ||||||
| image: mysql:8.4 | ||||||
| hostname: mysql2 | ||||||
| environment: | ||||||
| MYSQL_ROOT_PASSWORD: testpass | ||||||
| ports: | ||||||
| - "13307:3306" | ||||||
| volumes: | ||||||
| - ./mysql/replica.cnf:/etc/mysql/conf.d/repl.cnf | ||||||
| - ./mysql/init-replica.sql:/docker-entrypoint-initdb.d/init.sql | ||||||
| depends_on: | ||||||
| mysql1: | ||||||
| condition: service_healthy | ||||||
| healthcheck: | ||||||
| test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-uroot", "-ptestpass"] | ||||||
| interval: 5s | ||||||
| timeout: 3s | ||||||
| retries: 30 | ||||||
| networks: | ||||||
| orchnet: | ||||||
| aliases: | ||||||
| - mysql2 | ||||||
|
|
||||||
| mysql3: | ||||||
| image: mysql:8.4 | ||||||
| hostname: mysql3 | ||||||
| environment: | ||||||
| MYSQL_ROOT_PASSWORD: testpass | ||||||
| ports: | ||||||
| - "13308:3306" | ||||||
| volumes: | ||||||
| - ./mysql/replica2.cnf:/etc/mysql/conf.d/repl.cnf | ||||||
| - ./mysql/init-replica.sql:/docker-entrypoint-initdb.d/init.sql | ||||||
| depends_on: | ||||||
| mysql1: | ||||||
| condition: service_healthy | ||||||
| healthcheck: | ||||||
| test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-uroot", "-ptestpass"] | ||||||
| interval: 5s | ||||||
| timeout: 3s | ||||||
| retries: 30 | ||||||
| networks: | ||||||
| orchnet: | ||||||
| aliases: | ||||||
| - mysql3 | ||||||
|
|
||||||
| proxysql: | ||||||
| image: proxysql/proxysql:latest | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
| image: proxysql/proxysql:latest | |
| image: proxysql/proxysql:2.6.3 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,125 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/bin/bash | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Shared test helpers for functional tests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PASS_COUNT=0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FAIL_COUNT=0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SKIP_COUNT=0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ORC_URL="http://localhost:3099" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pass() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo " ✅ PASS: $1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ((PASS_COUNT++)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fail() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo " ❌ FAIL: $1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ -n "$2" ] && echo " $2" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [ -n "$2" ] && echo " $2" | |
| [ -n "${2-}" ] && echo " ${2-}" |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
bash -euo pipefail -c 'count=0; ((count++)); echo "unreached"'
bash -euo pipefail -c 'f(){ [ -n "$2" ] && echo "$2"; }; f only-one-arg'Repository: ProxySQL/orchestrator
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Check lines 1-8 to see if counters are initialized
head -n 30 tests/functional/lib.shRepository: ProxySQL/orchestrator
Length of output: 566
🏁 Script executed:
#!/bin/bash
# Test 1: Post-increment at 0 returns old value (0), causing exit status 1 under set -e
bash -euo pipefail -c 'count=0; ((count++)); echo "reached: $count"'
echo "Test 1 exit code: $?"
# Test 2: Post-increment at 1+ returns old value, causing exit status 0 (truthy)
bash -euo pipefail -c 'count=1; ((count++)); echo "reached: $count"'
echo "Test 2 exit code: $?"
# Test 3: Assignment form always succeeds
bash -euo pipefail -c 'count=0; count=$((count+1)); echo "reached: $count"'
echo "Test 3 exit code: $?"Repository: ProxySQL/orchestrator
Length of output: 146
pass/fail/skip are not safe under the strict mode the suites enable.
The post-increment ((...++)) evaluates to the old value and returns exit status 1 when the counter is 0, causing the script to exit on the first call to any of these functions. Additionally, fail() dereferences $2 without guarding for missing positional parameters, which causes an unbound variable error under set -u when called with a single argument.
Because every suite does set -euo pipefail before sourcing tests/functional/lib.sh, the first PASS, the first SKIP, or any one-argument FAIL exits the script instead of recording the result.
Suggested fix
pass() {
echo " ✅ PASS: $1"
- ((PASS_COUNT++))
+ PASS_COUNT=$((PASS_COUNT + 1))
}
fail() {
echo " ❌ FAIL: $1"
- [ -n "$2" ] && echo " $2"
- ((FAIL_COUNT++))
+ if [ $# -ge 2 ] && [ -n "$2" ]; then
+ echo " $2"
+ fi
+ FAIL_COUNT=$((FAIL_COUNT + 1))
}
skip() {
echo " ⚠️ SKIP: $1"
- ((SKIP_COUNT++))
+ SKIP_COUNT=$((SKIP_COUNT + 1))
}📝 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.
| pass() { | |
| echo " ✅ PASS: $1" | |
| ((PASS_COUNT++)) | |
| } | |
| fail() { | |
| echo " ❌ FAIL: $1" | |
| [ -n "$2" ] && echo " $2" | |
| ((FAIL_COUNT++)) | |
| } | |
| skip() { | |
| echo " ⚠️ SKIP: $1" | |
| ((SKIP_COUNT++)) | |
| pass() { | |
| echo " ✅ PASS: $1" | |
| PASS_COUNT=$((PASS_COUNT + 1)) | |
| } | |
| fail() { | |
| echo " ❌ FAIL: $1" | |
| if [ $# -ge 2 ] && [ -n "$2" ]; then | |
| echo " $2" | |
| fi | |
| FAIL_COUNT=$((FAIL_COUNT + 1)) | |
| } | |
| skip() { | |
| echo " ⚠️ SKIP: $1" | |
| SKIP_COUNT=$((SKIP_COUNT + 1)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/functional/lib.sh` around lines 9 - 22, The three reporting functions
(pass, fail, skip) are unsafe under set -euo pipefail: change the post-increment
arithmetic ((...++)) to a safe assignment form (e.g.
PASS_COUNT=$((PASS_COUNT+1)), SKIP_COUNT=$((SKIP_COUNT+1)),
FAIL_COUNT=$((FAIL_COUNT+1))) so the increment never produces a failing exit
status, and make fail() guard the optional second parameter by using a parameter
expansion fallback (e.g. "${2:-}") or testing with "${2+set}" before echoing so
referencing $2 cannot trigger an unbound variable error; update the functions
pass, fail, and skip accordingly (ensuring their echo lines remain) to preserve
behavior under strict mode.
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 use of 2>&1 can lead to incorrect behavior if curl fails to connect. In such cases, the error message from curl is redirected to stdout and gets assigned to the CODE variable, which will then contain more than just the HTTP status code. A more robust approach is to remove the redirection and check curl's exit code separately.
| test_endpoint() { | |
| local NAME="$1" URL="$2" EXPECT="$3" | |
| local CODE | |
| CODE=$(curl -s -o /dev/null -w "%{http_code}" "$URL" 2>&1) | |
| if [ "$CODE" = "$EXPECT" ]; then | |
| pass "$NAME (HTTP $CODE)" | |
| else | |
| fail "$NAME (HTTP $CODE, expected $EXPECT)" | |
| fi | |
| } | |
| test_endpoint() { | |
| local NAME="$1" URL="$2" EXPECT="$3" | |
| local CODE | |
| CODE=$(curl -s -o /dev/null -w "%{http_code}" "$URL") | |
| if [ $? -ne 0 ]; then | |
| fail "$NAME" "curl command failed for $URL" | |
| return | |
| fi | |
| if [ "$CODE" = "$EXPECT" ]; then | |
| pass "$NAME (HTTP $CODE)" | |
| else | |
| fail "$NAME (HTTP $CODE, expected $EXPECT)" | |
| fi | |
| } |
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
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.
This function is not fully robust. If curl fails with a network error, the error message is captured in BODY, and the test might fail with a generic message. It's better to explicitly check curl's exit code. Additionally, using grep -F is safer for matching fixed strings, as it prevents misinterpretation of special characters as regex.
| test_body_contains() { | |
| local NAME="$1" URL="$2" EXPECT="$3" | |
| local BODY | |
| BODY=$(curl -s "$URL" 2>&1) | |
| if echo "$BODY" | grep -q "$EXPECT"; then | |
| pass "$NAME" | |
| else | |
| fail "$NAME" "Response does not contain '$EXPECT'" | |
| fi | |
| } | |
| test_body_contains() { | |
| local NAME="$1" URL="$2" EXPECT="$3" | |
| local BODY | |
| BODY=$(curl -s "$URL") | |
| if [ $? -ne 0 ]; then | |
| fail "$NAME" "curl command failed for $URL" | |
| return | |
| fi | |
| if echo "$BODY" | grep -qF -- "$EXPECT"; then | |
| pass "$NAME" | |
| else | |
| fail "$NAME" "Response does not contain '$EXPECT'" | |
| fi | |
| } |
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.
🧩 Analysis chain
🌐 Web query:
Does curlreturn a zero exit status for HTTP 404/500 responses unless-f/--fail is used?
💡 Result:
Yes, curl returns a zero exit status for HTTP 404 and 500 responses unless the -f or --fail option is used. By default, curl considers the transfer successful as long as the HTTP response is received, regardless of the status code (4xx or 5xx). The --fail option makes curl return a non-zero exit code (typically 22) for HTTP responses with status codes >= 400.
Citations:
- 1: https://curl.se/docs/manpage.html
- 2: https://man7.org/linux/man-pages/man1/curl.1.html
- 3: HTTP error codes not reflected in curl exit value curl/curl#3789
- 4: https://superuser.com/questions/590099/can-i-make-curl-fail-with-an-exitcode-different-than-0-if-the-http-status-code-i
- 5: https://stackoverflow.com/questions/22506024/get-page-output-with-curl-fail
- 6: https://everything.curl.dev/http/response.html
Fix readiness check to require HTTP success response.
curl -s -o /dev/null exits with code 0 for HTTP 404/500 responses, allowing wait_for_orchestrator() to incorrectly report success when the endpoint is failing. Use curl -fsS to make curl fail on HTTP error status codes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/functional/lib.sh` around lines 57 - 63, The readiness check in
wait_for_orchestrator currently treats any HTTP response (including 4xx/5xx) as
success because curl is invoked with -s -o /dev/null; change the curl invocation
in wait_for_orchestrator (the call hitting "$ORC_URL/api/clusters") to use flags
that cause non-2xx responses to exit nonzero (e.g., replace -s -o /dev/null with
-fsS -o /dev/null or equivalent) so the loop only reports "Orchestrator ready"
on actual HTTP success.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| -- Orchestrator user with full privileges | ||
| CREATE USER IF NOT EXISTS 'orchestrator'@'%' IDENTIFIED BY 'orch_pass'; | ||
| GRANT ALL PRIVILEGES ON *.* TO 'orchestrator'@'%' WITH GRANT OPTION; | ||
|
|
||
| -- Replication user | ||
| CREATE USER IF NOT EXISTS 'repl'@'%' IDENTIFIED BY 'repl_pass'; | ||
| GRANT REPLICATION SLAVE ON *.* TO 'repl'@'%'; | ||
|
|
||
| FLUSH PRIVILEGES; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| -- Orchestrator user | ||
| CREATE USER IF NOT EXISTS 'orchestrator'@'%' IDENTIFIED BY 'orch_pass'; | ||
| GRANT ALL PRIVILEGES ON *.* TO 'orchestrator'@'%' WITH GRANT OPTION; | ||
| -- Replication user (needed if this replica gets promoted) | ||
| CREATE USER IF NOT EXISTS 'repl'@'%' IDENTIFIED BY 'repl_pass'; | ||
| GRANT REPLICATION SLAVE ON *.* TO 'repl'@'%'; | ||
| FLUSH PRIVILEGES; | ||
| -- NOTE: Replication is configured by setup-replication.sh after all containers are up |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| [mysqld] | ||
| server-id=1 | ||
| log-bin=mysql-bin | ||
| binlog-format=ROW | ||
| gtid-mode=ON | ||
| enforce-gtid-consistency=ON | ||
| log-replica-updates=ON | ||
| binlog-row-image=MINIMAL | ||
| report-host=mysql1 |
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.
🧩 Analysis chain
🏁 Script executed:
Repository: ProxySQL/orchestrator
Length of output: 355
Pin container images to immutable versions (ideally digest-pinned).
Floating tags (
mysql:8.4andproxysql/proxysql:latest) make the functional test suite non-reproducible and vulnerable to upstream image drift. Each pull of the image may yield different contents, causing tests to break without code changes.Apply to lines 5, 21, 40 (all
mysql:8.4), and line 59 (proxysql/proxysql:latest).🔧 Proposed fix
🤖 Prompt for AI Agents