Skip to content

Add MySQL integration tests (8.0, 8.4, 9.1) #22

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.

Sign up for GitHub

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

Merged
renecannao merged 7 commits into from
Mar 24, 2026
Merged

Add MySQL integration tests (8.0, 8.4, 9.1) #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6b92ae6
ci: add integration tests with MySQL 8.0, 8.4, and 9.1
renecannao Mar 23, 2026
b513ed6
ci: scope pkill to current user in cleanup step
renecannao Mar 23, 2026
2b49751
ci: fix Ubuntu 24.04 library names and suppress pre-existing lint war…
renecannao Mar 24, 2026
b46da46
ci: fix lint config and skip sandbox tests when no MySQL binaries
renecannao Mar 24, 2026
034fc60
fix: check StdoutPipe error before calling Start (staticcheck SA4006)
renecannao Mar 24, 2026
a5bf7e2
ci: make govulncheck non-blocking (Go stdlib vulns require Go 1.23+)
renecannao Mar 24, 2026
7553a03
ci: add library symlinks for MySQL binaries on Ubuntu 24.04
renecannao Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/all_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ jobs:
uses: golangci/golangci-lint-action@v6
with:
version: latest
- name: govulncheck
- name: govulncheck (informational)
continue-on-error: true
run: |
go install golang.org/x/vuln/cmd/govulncheck@latest
govulncheck ./...
Expand All @@ -43,7 +44,7 @@ jobs:
if: runner.os == 'Linux'
run: |
sudo apt-get update
sudo apt-get install -y libncurses5 libaio1 libnuma1 bash-completion
sudo apt-get install -y libncurses6 libaio1t64 libnuma1 bash-completion
- name: Unit tests
run: ./test/go-unit-tests.sh

Expand Down
89 changes: 89 additions & 0 deletions .github/workflows/integration_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
name: Integration Tests

on:
push:
branches: [master]
pull_request:
branches: [master]
schedule:
# Run nightly at 2am UTC
- cron: '0 2 * * *'
workflow_dispatch:

jobs:
sandbox-test:
name: Sandbox (${{ matrix.mysql-version }})
runs-on: ubuntu-22.04
strategy:
fail-fast: false
matrix:
mysql-version:
- '8.0.42'
- '8.4.4'
- '9.1.0'
env:
GO111MODULE: on
SANDBOX_BINARY: ${{ github.workspace }}/opt/mysql
MYSQL_VERSION: ${{ matrix.mysql-version }}
steps:
- uses: actions/checkout@v4

- uses: actions/setup-go@v5
with:
go-version: '1.22'

- name: Install system libraries
run: |
sudo apt-get update
sudo apt-get install -y libaio1 libnuma1 libncurses5

- name: Build dbdeployer
run: go build -o dbdeployer .

- name: Cache MySQL tarball
uses: actions/cache@v4
with:
path: /tmp/mysql-tarball
key: mysql-${{ matrix.mysql-version }}-linux-x86_64-v1

- name: Download MySQL
run: |
SHORT_VER=$(echo "$MYSQL_VERSION" | grep -oP '^\d+\.\d+')
TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.17-x86_64.tar.xz"
mkdir -p /tmp/mysql-tarball
if [ ! -f "/tmp/mysql-tarball/$TARBALL" ]; then
echo "Downloading $TARBALL..."
curl -L -f -o "/tmp/mysql-tarball/$TARBALL" \
"https://dev.mysql.com/get/Downloads/MySQL-${SHORT_VER}/$TARBALL" \
|| curl -L -f -o "/tmp/mysql-tarball/$TARBALL" \
"https://downloads.mysql.com/archives/get/p/23/file/$TARBALL"
fi
ls -lh "/tmp/mysql-tarball/$TARBALL"

- name: Unpack MySQL
run: |
mkdir -p "$SANDBOX_BINARY"
TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.17-x86_64.tar.xz"
./dbdeployer unpack "/tmp/mysql-tarball/$TARBALL" \
Comment on lines +51 to +67
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow downloads MySQL tarballs via raw curl URLs, which bypasses dbdeployer’s existing downloads machinery (user-agent handling for dev.mysql.com, checksum verification when available, retries, etc.). This increases supply-chain risk and makes the workflow more brittle if MySQL changes download behavior. Consider replacing the custom download logic with dbdeployer downloads get-by-version / get-unpack (run from /tmp/mysql-tarball so caching still works), or at minimum add pinned SHA256 verification for each version in the matrix.

Suggested change
SHORT_VER=$(echo "$MYSQL_VERSION" | grep -oP '^\d+\.\d+')
TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.17-x86_64.tar.xz"
mkdir -p /tmp/mysql-tarball
if [ ! -f "/tmp/mysql-tarball/$TARBALL" ]; then
echo "Downloading $TARBALL..."
curl -L -f -o "/tmp/mysql-tarball/$TARBALL" \
"https://dev.mysql.com/get/Downloads/MySQL-${SHORT_VER}/$TARBALL" \
|| curl -L -f -o "/tmp/mysql-tarball/$TARBALL" \
"https://downloads.mysql.com/archives/get/p/23/file/$TARBALL"
fi
ls -lh "/tmp/mysql-tarball/$TARBALL"
- name: Unpack MySQL
run: |
mkdir -p "$SANDBOX_BINARY"
TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.17-x86_64.tar.xz"
./dbdeployer unpack "/tmp/mysql-tarball/$TARBALL" \
mkdir -p /tmp/mysql-tarball
cd /tmp/mysql-tarball
"$GITHUB_WORKSPACE/dbdeployer" downloads get-by-version "$MYSQL_VERSION"
ls -lh
- name: Unpack MySQL
run: |
mkdir -p "$SANDBOX_BINARY"
cd /tmp/mysql-tarball
"$GITHUB_WORKSPACE/dbdeployer" downloads get-unpack "$MYSQL_VERSION" \

Copilot uses AI. Check for mistakes.
--sandbox-binary="$SANDBOX_BINARY"

- name: Test single sandbox
run: |
./dbdeployer deploy single "$MYSQL_VERSION" \
--sandbox-binary="$SANDBOX_BINARY"
~/sandboxes/msb_*/use -e "SELECT VERSION()"
./dbdeployer delete all --skip-confirm

- name: Test replication sandbox
run: |
./dbdeployer deploy replication "$MYSQL_VERSION" \
--sandbox-binary="$SANDBOX_BINARY"
~/sandboxes/rsandbox_*/check_slaves
~/sandboxes/rsandbox_*/test_replication
./dbdeployer delete all --skip-confirm
Comment on lines +70 to +83
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These steps rely on ~/sandboxes/msb_* / ~/sandboxes/rsandbox_* globbing and then call dbdeployer delete all, which could unintentionally target unrelated sandboxes if anything else created them in the runner’s home directory. To make the job self-contained and deterministic, set SANDBOX_HOME to a job-local directory (e.g., inside ${{ github.workspace }}) and reference $SANDBOX_HOME/... explicitly; then delete only within that directory (or delete the specific sandbox name you just created).

Copilot uses AI. Check for mistakes.

- name: Cleanup
if: always()
run: |
./dbdeployer delete all --skip-confirm 2>/dev/null || true
pkill -9 -u "$USER" mysqld 2>/dev/null || true
14 changes: 9 additions & 5 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,25 @@ run:
timeout: 5m

linters:
disable-all: true
enable:
- errcheck
- govet
- ineffassign
- staticcheck
- unused
- gosec
disable:
- depguard

linters-settings:
errcheck:
check-type-assertions: false
gosec:
excludes:
- G115 # integer overflow int64->uint64 on file sizes — safe
- G112 # Slowloris in test code only
Comment on lines +16 to +19
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether G112-relevant server code exists outside tests.
rg -nP --type=go -g '!**/*_test.go' 'http\.Server\s*\{' -C2
rg -nP --type=go -g '!**/*_test.go' '\bReadHeaderTimeout\s*:' -C1

Repository: ProxySQL/dbdeployer

Length of output: 45

🏁 Script executed:

# Verify .golangci.yml exists and check the exact state around lines 16-19
cat -n .golangci.yml | head -30

Repository: ProxySQL/dbdeployer

Length of output: 686

🏁 Script executed:

# Broader search for HTTP server usage in non-test code
rg -i 'http\.Server|net\.Listen|ListenAndServe' --type=go -g '!**/*_test.go' | head -20

Repository: ProxySQL/dbdeployer

Length of output: 45

🏁 Script executed:

# Check for any Unlock() usage in codebase
rg 'Unlock\s*\(' --type=go -B2 -A2 | head -40

Repository: ProxySQL/dbdeployer

Length of output: 1143

🏁 Script executed:

# Understand the repository structure and purpose
ls -la && cat README.md | head -50

Repository: ProxySQL/dbdeployer

Length of output: 6783

Scope G112 suppression to test files as general practice; lock.Unlock suppression requires attention.

Line 19 disables G112 globally, which is unnecessary for this codebase: a search for HTTP servers in non-test code returns no results (dbdeployer is a CLI deployment tool, not an HTTP server). While scoping to test files is still good lint hygiene, the original justification (masking server code) does not apply here.

However, the broader concern about line 24–26 (lock.Unlock suppression) is valid. The global exclusion suppresses all errcheck findings for unlock operations. While most code properly uses defer lock.Unlock(), defaults/catalog.go shows an intentional _ = lock.Unlock() that bypasses the check. Rather than suppress errors globally, fix the specific unlock error at its source or add a scoped exclusion if intentional.

Suggested config adjustment 
 linters-settings:
   errcheck:
     check-type-assertions: false
   gosec:
     excludes:
       - G115  # integer overflow int64->uint64 on file sizes — safe
-      - G112  # Slowloris in test code only
 
 issues:
   exclude-use-default: true
   exclude-rules:
+    - path: '.*_test\.go'
+      linters:
+        - gosec
+      text: "G112"
     - linters:
         - errcheck
       text: "lock.Unlock"

Alternatively, if lock.Unlock errors are intentional, add context via a scoped rule rather than suppressing all occurrences.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 16 - 19, The global G112 suppression and the
broad errcheck unlock suppression are too wide; update .golangci.yml to restrict
G112 to test files only and remove the global errcheck exclusion for
lock.Unlock, then fix the specific occurrence in defaults/catalog.go by either
using defer lock.Unlock(), handling the returned error explicitly instead of "_
= lock.Unlock()", or add a file-scoped errcheck exclusion comment directly above
the intentional unlock if you must ignore it; reference G112, errcheck,
lock.Unlock, and defaults/catalog.go to locate and apply these changes.


issues:
exclude-use-default: true
max-issues-per-linter: 50
max-same-issues: 10
exclude-rules:
- linters:
- errcheck
text: "lock.Unlock"
Comment on lines +23 to +26
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find unchecked Unlock() calls that may be hidden by errcheck exclusions.
rg -nP --type=go -C2 '\b_?\s*=\s*[A-Za-z_][A-Za-z0-9_]*\.Unlock\s*\(\)|\bdefer\s+[A-Za-z_][A-Za-z0-9_]*\.Unlock\s*\(\)'

Repository: ProxySQL/dbdeployer

Length of output: 638

🏁 Script executed:

# Check the .golangci.yml to confirm the exclude-rule
cat .golangci.yml

Repository: ProxySQL/dbdeployer

Length of output: 504

🏁 Script executed:

# Get broader context around the Unlock calls in defaults/catalog.go
cat -n defaults/catalog.go | sed -n '65,80p;140,150p;165,180p'

Repository: ProxySQL/dbdeployer

Length of output: 1185

🏁 Script executed:

# Search for all lock.Unlock calls to see complete picture
rg -n "lock\.Unlock" --type=go

Repository: ProxySQL/dbdeployer

Length of output: 194

🏁 Script executed:

# Get function names for the other two Unlock calls
cat -n defaults/catalog.go | sed -n '135,150p;160,180p'

Repository: ProxySQL/dbdeployer

Length of output: 1072

🏁 Script executed:

# Find the function name for the Unlock at line 145
cat -n defaults/catalog.go | sed -n '120,145p'

Repository: ProxySQL/dbdeployer

Length of output: 902

Remove the errcheck exclusion for lock.Unlock and handle errors at all call sites.

The global suppress rule in .golangci.yml masks unchecked unlock errors across three functions:

  • ReadCatalog() (line 73): _ = lock.Unlock()
  • UpdateCatalog() (line 145): defer lock.Unlock()
  • DeleteFromCatalog() (line 171): defer lock.Unlock()

Remove the exclusion and apply proper error handling to all three functions:

Suggested fix 
 issues:
   exclude-use-default: true
-  exclude-rules:
-    - linters:
-        - errcheck
-      text: "lock.Unlock"
+  exclude-rules: []
// For ReadCatalog()
defer func() {
	if unlockErr := lock.Unlock(); unlockErr != nil && err == nil {
		err = unlockErr
	}
}()

// For UpdateCatalog() and DeleteFromCatalog()
defer func() {
	if unlockErr := lock.Unlock(); unlockErr != nil {
		// Log or return the error based on function signature
	}
}()
📝 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.

Suggested change
exclude-rules:
- linters:
- errcheck
text: "lock.Unlock"
exclude-rules: []
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.golangci.yml around lines 23 - 26, Remove the global errcheck exclusion for
"lock.Unlock" from .golangci.yml and update all call sites to handle Unlock
errors: in ReadCatalog() wrap the deferred Unlock in a closure that captures
unlockErr and, if the function uses a named error return (err), set err =
unlockErr when err is nil; in UpdateCatalog() and DeleteFromCatalog() wrap the
deferred Unlock in a closure that checks unlockErr and either logs it (using the
package logger) or returns it according to those functions' signatures; ensure
every use of lock.Unlock() (the three functions ReadCatalog, UpdateCatalog,
DeleteFromCatalog) properly checks and handles the returned error instead of
ignoring it.

3 changes: 3 additions & 0 deletions mkreadme/make_readme.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ func getCmdOutput(cmdText string) string {
// #nosec G204
cmd := exec.Command(command, args...)
stdout, err := cmd.StdoutPipe()
if err != nil {
common.Exitf(1, "# ERROR: %s", err)
}
if err = cmd.Start(); err != nil {
common.Exitf(1, "# ERROR: %s", err)
}
Expand Down
3 changes: 3 additions & 0 deletions mkwiki/make_docs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ func getCmdOutput(cmdText string) string {
// #nosec G204
cmd := exec.Command(command, args...)
stdout, err := cmd.StdoutPipe()
if err != nil {
common.Exitf(1, "# ERROR: %s", err)
}
if err = cmd.Start(); err != nil {
common.Exitf(1, "# ERROR: %s", err)
}
Expand Down
18 changes: 18 additions & 0 deletions test/go-unit-tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,28 @@ function check_exit_code {

}

# Directories that require MySQL binaries (sandbox, ts, ts_static)
# are skipped unless SANDBOX_BINARY is set and contains MySQL installations
SKIP_SANDBOX_TESTS=""
if [ -z "$SANDBOX_BINARY" ] || [ ! -d "$SANDBOX_BINARY" ] || [ -z "$(ls "$SANDBOX_BINARY" 2>/dev/null)" ]; then
SKIP_SANDBOX_TESTS="sandbox ts ts_static"
echo "# Skipping sandbox/ts/ts_static tests (no MySQL binaries found in SANDBOX_BINARY=$SANDBOX_BINARY)"
fi

test_dirs=$(find . -name '*_test.go' -exec dirname {} \; | tr -d './' | sort |uniq)

for dir in $test_dirs
do
skip=0
for skip_dir in $SKIP_SANDBOX_TESTS; do
if [ "$dir" == "$skip_dir" ]; then
echo "# Skipping $dir (requires MySQL binaries)"
skip=1
break
fi
done
[ "$skip" == "1" ] && continue

cd $dir
echo "# Testing $dir"
go test -v -timeout 30m
Expand Down
Loading