-
Notifications
You must be signed in to change notification settings - Fork 10
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
#221 🚀 improve build performance; #230 🔧 use ghcr.io as snapshot repo #229
Conversation
aebf26b
to
fc9f1c1
Compare
<images> | ||
<image> | ||
<build> | ||
<buildx> |
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 (Informational): configure the docker-maven-plugin
to use the buildx docker-container
driver at deploy time and to/from caching to improve build speed.
<DOCKER_BASELINE_REPO_ID>${docker.baseline.repo.id}/</DOCKER_BASELINE_REPO_ID> | ||
<VERSION_AISSEMBLE>${version.aissemble}</VERSION_AISSEMBLE> | ||
<SPARK_VERSION>${version.spark}</SPARK_VERSION> | ||
</args> | ||
<buildx> |
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: For normal builds, use the default (local) buildx driver, which results in faster execution than the docker-container
driver due to less I/O.
<goals> | ||
<goal>build</goal> | ||
</goals> | ||
<configuration> |
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: When the ci
profile is activated, do NOT build. We can improve performance by running both the arm64
and amd64
platform builds simultaneously.
@@ -39,10 +39,13 @@ | |||
</execution> | |||
</executions> | |||
</plugin> |
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: we were configuring the fabric8 build incorrectly within the extensions-docker
module (including an active run there). These changes correct this issue so that we can use the updated configuration above.
@@ -15,7 +15,7 @@ | |||
<packaging>docker-build</packaging> | |||
|
|||
<properties> |
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: this was ultimately resolving as target/dockerbuild/jars//*
- so I cleaned it up
LABEL org.opencontainers.image.source="https://github.com/boozallen/aissemble" | ||
|
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: moved the python configuration to the agent container, where it belongs. This also improved the speed in which it was installing overall a bit.
@@ -5,11 +5,11 @@ LABEL org.opencontainers.image.source="https://github.com/boozallen/aissemble" | |||
USER root | |||
USER jenkins | |||
|
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: The changes in versions represent IA issues flagged during the docker build of this container.
@@ -88,6 +88,21 @@ | |||
</execution> | |||
</executions> | |||
</plugin> |
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: This block was oddly in the parent pom but was only ever used here. This change aligns the declaration and use more effectively.
@@ -56,11 +58,9 @@ | |||
|
|||
<modules> |
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: bye bye bye
fc9f1c1
to
903d121
Compare
<buildx> | ||
<!-- use default cache, passed to docker via config flag.--> | ||
<builderName>default</builderName> | ||
<dockerStateDir>~/.docker</dockerStateDir> | ||
<platforms> | ||
<platform>${docker.platforms}</platform> |
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.
Q/A: Can we exclude this for this configuration? Seems like it wouldn't be used, since you can only build the same platform as the host machine.
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.
Sadly, if this is not present, the build will trigger as a non-buildx build. To avoid suggesting a more invasive change to fabric8 (and keep it simple), this is the trigger for buildx with the default buildx driver.
Docker does provide lovely, informative error messages if you try to use it out of place.
foundation/foundation-mda/src/main/resources/templates/general-docker/airflow.docker.file.vm
Outdated
Show resolved
Hide resolved
foundation/foundation-mda/src/main/resources/templates/deployment/kafka/kafka.values.yaml.vm
Show resolved
Hide resolved
f1f4ebc
to
f4e83c5
Compare
Co-authored-by: Emily Wilkins <[email protected]>
48860da
to
83f1571
Compare
This also cover #230