-
Notifications
You must be signed in to change notification settings - Fork 1.1k
chore: Add MinIO support for S3 snapshot tests #6501
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: main
Are you sure you want to change the base?
Conversation
🤖 Augment PR SummarySummary: Adds MinIO-backed infrastructure so S3 snapshot tests can run without hitting real AWS S3. Changes:
Notes: Keeps test files unchanged; the switch is driven by environment/config (Fixes #6412). 🤖 Was this summary useful? React with 👍 or 👎 |
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.
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
Adds infrastructure support to run S3 snapshot integration tests against a local MinIO server (instead of AWS S3) via a single S3_ENDPOINT environment variable, including CI wiring.
Changes:
- Pass a custom S3 endpoint/HTTP setting into Dragonfly instances when
S3_ENDPOINTis set. - Add pytest startup/shutdown hooks to download, start, and configure a MinIO server for S3 tests.
- Add a dedicated CI step to run S3 snapshot tests against MinIO.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/dragonfly/instance.py | Reads S3_ENDPOINT and translates it into Dragonfly --s3_endpoint / --s3_use_https args. |
| tests/dragonfly/conftest.py | Adds MinIO binary download + MinIO server lifecycle management and env setup for S3 tests. |
| .github/actions/regression-tests/action.yml | Runs S3 snapshot tests in CI with S3_ENDPOINT pointing to localhost MinIO and ensures MinIO binary is present. |
718e591 to
506aba5
Compare
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
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
romange
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.
I think there is a misunderstanding here. We still need to support aws s3 and it has slight differences with minio. This is why I wanted to have tests for both s3 and minio. We had a bug where s3 worked but minio failed imho
506aba5 to
90bbb0e
Compare
There is no misunderstanding. I added minio as an option, we have both ways to test the same tests with AWS and minio. |
| df -h | ||
|
|
||
| - name: Run S3 snapshot tests with MinIO | ||
| if: inputs.with-s3 == '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.
I still do not understand.with-s3 means - test with minio? should we call it with-minio then?
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.
I need to understand somehow that the current build has the S3 feature built in to execute MinIO tests.
I see that this solution is not good. I will try to get rid of it.
90bbb0e to
4398937
Compare
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| log_file = open(minio_log, "w") | ||
| proc = subprocess.Popen( | ||
| [str(minio_bin), "server", str(data_dir), "--address", address], | ||
| env={**os.environ, "MINIO_ROOT_USER": "minioadmin", "MINIO_ROOT_PASSWORD": "minioadmin"}, | ||
| stdout=log_file, | ||
| stderr=subprocess.STDOUT, | ||
| ) | ||
|
|
||
| bucket = "dragonfly-test" | ||
| s3 = boto3.client( | ||
| "s3", | ||
| endpoint_url=endpoint, | ||
| aws_access_key_id="minioadmin", | ||
| aws_secret_access_key="minioadmin", | ||
| region_name="us-east-1", | ||
| ) | ||
|
|
Copilot
AI
Jan 30, 2026
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.
If boto3.client(...) (or other code between starting the process and the retry loop) raises (e.g. due to an invalid endpoint), the MinIO subprocess and log file handle will be leaked. Wrap the MinIO start + boto3 setup in a try/except/finally that terminates the process, closes the log file, and deletes the temp dir on failure.
| - name: Run S3 snapshot tests with MinIO | ||
| if: inputs.s3-bucket != '' | ||
| shell: bash | ||
| run: | | ||
| cd ${GITHUB_WORKSPACE}/tests | ||
| pip3 install -r dragonfly/requirements.txt | ||
|
|
||
| export DRAGONFLY_PATH="${GITHUB_WORKSPACE}/${{inputs.build-folder-name}}/${{inputs.dfly-executable}}" | ||
|
|
||
| # Download MinIO binary (atomic: download to .tmp, then rename) | ||
| ARCH=$(uname -m) | ||
| case "$ARCH" in | ||
| x86_64) ARCH="amd64" ;; | ||
| aarch64) ARCH="arm64" ;; | ||
| *) echo "Unsupported MinIO architecture: $ARCH"; exit 1 ;; | ||
| esac | ||
| MINIO_DIR="$HOME/.cache/dragonfly-tests" | ||
| mkdir -p "$MINIO_DIR" | ||
| if [ ! -f "$MINIO_DIR/minio" ]; then | ||
| curl -fsSL "https://dl.min.io/server/minio/release/linux-${ARCH}/minio" -o "$MINIO_DIR/minio.tmp" | ||
| chmod +x "$MINIO_DIR/minio.tmp" | ||
| mv "$MINIO_DIR/minio.tmp" "$MINIO_DIR/minio" | ||
| fi | ||
|
|
||
| S3_ENDPOINT=http://localhost:9000 timeout 10m pytest -k "s3" --timeout=300 --color=yes dragonfly/snapshot_test.py --log-cli-level=INFO -v |
Copilot
AI
Jan 30, 2026
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 adds a MinIO-only S3 test run, but the main "Run PyTests" step still exports DRAGONFLY_S3_BUCKET (and AWS creds) and will run the same S3 snapshot tests against real AWS as part of the full suite. That contradicts the PR description’s "instead of real AWS S3" claim and also duplicates coverage/time. Consider making MinIO the sole S3 backend when enabled (e.g., set S3_ENDPOINT for the main pytest run and avoid setting DRAGONFLY_S3_BUCKET to the AWS bucket in that case, or introduce a dedicated input flag).
| - name: Run S3 snapshot tests with MinIO | ||
| if: inputs.s3-bucket != '' | ||
| shell: bash | ||
| run: | | ||
| cd ${GITHUB_WORKSPACE}/tests | ||
| pip3 install -r dragonfly/requirements.txt |
Copilot
AI
Jan 30, 2026
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 new MinIO step installs test requirements via pip, and the subsequent "Run PyTests" step installs the same requirements again. Since composite-action steps share the same environment, consider installing requirements once (e.g., in a single earlier step) to avoid redundant work and reduce CI runtime.
| - name: Run S3 snapshot tests with MinIO | |
| if: inputs.s3-bucket != '' | |
| shell: bash | |
| run: | | |
| cd ${GITHUB_WORKSPACE}/tests | |
| pip3 install -r dragonfly/requirements.txt | |
| - name: Install Python test requirements | |
| shell: bash | |
| run: | | |
| cd ${GITHUB_WORKSPACE}/tests | |
| pip3 install -r dragonfly/requirements.txt | |
| - name: Run S3 snapshot tests with MinIO | |
| if: inputs.s3-bucket != '' | |
| shell: bash | |
| run: | | |
| cd ${GITHUB_WORKSPACE}/tests |
4398937 to
d6f1c87
Compare
Add MinIO support for S3 snapshot tests.
Enable S3 snapshot tests to run against MinIO instead of real AWS S3, controlled by a single S3_ENDPOINT env var. MinIO binary is auto-downloaded and cached in ~/.cache/dragonfly-tests/. No changes to test files — only infrastructure (conftest.py, instance.py, CI action).
Fixes #6412