-
Notifications
You must be signed in to change notification settings - Fork 88
feat(docker): Add mirror configuration options for manylinux build containers #1849
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
- Add ALMALINUX_MIRROR build arg for package mirrors - Add QUAY_MIRROR build arg for base image registry - Add USE_NETWORK_HOST option for --network=host mode
WalkthroughConfiguration changes add build-time arguments for customizable Docker image sources across manylinux architectures. QUAY_MIRROR specifies the base image registry, ALMALINUX_MIRROR optionally overrides AlmaLinux repository mirrors, and USE_NETWORK_HOST enables network isolation during container builds. Changes applied consistently to both aarch64 and x86_64 variants. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution Docstrings generation - FAILED No docstrings were generated. |
junhaoliao
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.
- shall we update the musllinux_1_2-x86_64 and musllinux_1_2-aarch64 Dockerfiles to use QUAY_MIRROR as well?
- for consistency, shall we update other build.sh to accept USE_NETWORK_HOST as well?
- Let's update
docs/src/dev-docs/tooling-containers.md. consider a table that describes the variable name, default and description for each. we should be clear that both ALMALINUX_MIRROR and QUAY_MIRROR should be a hostname + path, not full URLs like https://xxx
| ARG ALMALINUX_MIRROR | ||
| RUN if [ -n "${ALMALINUX_MIRROR}" ]; then \ | ||
| sed -i -e 's|^mirrorlist=|#mirrorlist=|g' \ | ||
| -e "s|^# baseurl=https://repo.almalinux.org|baseurl=https://${ALMALINUX_MIRROR}|g" \ |
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.
just in case the maintainers decide to use different amount of spaces? it might never happen though
| -e "s|^# baseurl=https://repo.almalinux.org|baseurl=https://${ALMALINUX_MIRROR}|g" \ | |
| -e "s|^#[[:space:]]*baseurl=https://repo.almalinux.org|baseurl=https://${ALMALINUX_MIRROR}|g" \ |
| ARG ALMALINUX_MIRROR | ||
| RUN if [ -n "${ALMALINUX_MIRROR}" ]; then \ | ||
| sed -i -e 's|^mirrorlist=|#mirrorlist=|g' \ | ||
| -e "s|^# baseurl=https://repo.almalinux.org|baseurl=https://${ALMALINUX_MIRROR}|g" \ |
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.
| -e "s|^# baseurl=https://repo.almalinux.org|baseurl=https://${ALMALINUX_MIRROR}|g" \ | |
| -e "s|^#[[:space:]]*baseurl=https://repo.almalinux.org|baseurl=https://${ALMALINUX_MIRROR}|g" \ |
| docker buildx build | ||
| --platform linux/arm64 | ||
| --build-arg "ALMALINUX_MIRROR=${ALMALINUX_MIRROR:-}" | ||
| --build-arg "QUAY_MIRROR=${QUAY_MIRROR:-quay.io}" |
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.
we may not need this -quay.io fallback given we have specified so in the Dockerfile?
if the needs are clearly scoped to manylinux only, i'm fine deferring this to another PR. (let's ask the rabbit to create a follow up issue |
discussed offline - muslinux changes will be made in another PR |
Description
Add optional mirror configuration for building manylinux Docker containers in environments with restricted network access (e.g., behind firewalls).
New environment variables for
build.sh:QUAY_MIRROR- Alternative mirror for quay.ioALMALINUX_MIRROR- Alternative mirror for AlmaLinux packagesUSE_NETWORK_HOST- Set totrueto enable--network=hostfor buildsExample usage:
Checklist
Validation performed
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.