Cleanup Dockerfile and convert non-distributable handling to ONBUILD #179
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmlb
requested review from
joelrebel,
splaspood,
turegano-equinix and
DoctorVin
as code owners
joelrebel
reviewed
Sep 3, 2024
joelrebel
reviewed
Sep 3, 2024
Add files that do not get used by Dockerfile so that changes to these files do not bust layer cache when building examples and the later stages.
Nothing too crazy here: * Better named intermediate stages * Better indentation for readability * No need to add extra args to microdnf (verified same number of packages installed with/without args)
* No need for both ARG and ENV as ARG already exports to the environment * Make RUN step easier to read by skipping in bash script instead
Change from minio to s5cmd because s5cmd is plain faster, has batch support for easier use and plays better with AWS env vars that everyone else has standardized on. Clean up the rest of the script by doing a better job explicitly checking the environment variables early and with better error messages. Also take advantage of `set -e` to avoid `&&` and indentation.
This way downstream consumers do not need to fork or otherwise build the ironlib image to add the non-distributable binaries. Take vogelkop as an example, OSS Dockerfile can depend on this image just fine. Downstream users can then do something like: ```Dockerfile FROM $vogelkopimage AS vogelkop FROM $ironlibimage COPY --from=vogelkop /usr/bin/vogelkop /usr/bin ``` And then provide the values for ONBUILD on the docker command line to fetch the non-distributable files. No need to fork ironlib nor vogelkop's Dockerfiles!
mmlb
force-pushed
the
rework-docker-builds
branch
from
754a225 to
f909b99
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do
Bunch of minor (imo) clean ups to the Dockerfile and scripts/install-non-distributable.sh to start.
Then converts the whole fetching of non-distributable files from at ironlib image build time to
ONBUILDand thus when consumers build their own images based off of ironlib's image.This looks similar to how we used to do things before except there's better handling for skipping all of it by keeping what we do today of requiring a build-arg to opt in to proprietary deps. The old ONBBUILD setup used to check for presence of the fetcher script to decide to run the fetch. That was annoying because consumers would need a whole Dockerfile/repo setup just to have the script.
How can this change be tested by a PR reviewer?
Build this docker image with/without the args to opt-in to proprietary binaries, there should be no difference between the 2 images. Lets assume you tag it
my-local-ironlib.Now do the same with/without build using the following Dockerfile:
Running
docker build --build-arg INSTALL_NON_DISTRIBUTABLE=truewill fail and complain about missing AWS creds. Build again with the build-arg and it will complete. Also note that since noARGis present in this example Dockerfile the secrets do not leak into the final image (verification left up to reader).