Skip to content

test: Add minio for MTC tile storage#8790

Open
beautifulentropy wants to merge 2 commits into
mainfrom
add-s3-storage
Open

test: Add minio for MTC tile storage#8790
beautifulentropy wants to merge 2 commits into
mainfrom
add-s3-storage

Conversation

@beautifulentropy

Copy link
Copy Markdown
Member

Fixes #8789

@beautifulentropy beautifulentropy marked this pull request as ready for review June 8, 2026 21:04
@beautifulentropy beautifulentropy requested a review from a team as a code owner June 8, 2026 21:04
@beautifulentropy beautifulentropy requested a review from ezekiel June 8, 2026 21:04
ezekiel
ezekiel previously approved these changes Jun 9, 2026
@ezekiel ezekiel requested review from a team, aarongable and jsha and removed request for a team and aarongable June 9, 2026 15:19
@beautifulentropy beautifulentropy requested a review from ezekiel June 9, 2026 22:28

@ezekiel ezekiel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested deleting some errant ( and also totally harmless ) empty line remnants.

Comment thread test.sh
print_heading "Running Unit Tests"
flush_redis


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

Comment thread test.sh
print_heading "Running Integration Tests"
flush_redis


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

@jsha jsha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good with a quibble.

Comment thread docker-compose.yml
/bin/sh -c "
until mc alias set local http://boulder-minio:9000 minioadmin minioadmin >/dev/null 2>&1; do sleep 1; done;
mc mb --ignore-existing local/boulder-mtc-tiles;
exit 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the purpose of exit 0 to ignore errors from the mb subcommand? I would think --ignore-existing means mb will exit 0 even if the bucket exists. I think we can drop the explicit exit 0

@aarongable aarongable left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with two questions that might be helped by additional documentation.

Comment thread docker-compose.yml

bminio:
image: minio/minio:RELEASE.2025-09-07T16-13-09Z
command: server /data --console-address ":9001"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on this documentation, I don't think specifying --console-address ":9001" is necessary. If you point your browser at the API port (:9000), it auto-redirects to the randomly-selected console port, so there's no need to configure it unless you need a static port for allowlisting. But we're not allowlisting this port anywhere (docker isn't exposing it to the host, so we can't point our browsers at it anyway) so there's no point in making it static.

Comment thread docker-compose.yml
- bouldernet
entrypoint: >
/bin/sh -c "
until mc alias set local http://boulder-minio:9000 minioadmin minioadmin >/dev/null 2>&1; do sleep 1; done;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Based on these docs, I think this is using alias set as a stand-in for ping: just doing the simplest possible operation to confirm that minio is actually up. Is that understanding correct, and if yes, can it be documented here for future readers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add tile storage to integration test environment

4 participants