Add support to choose docker build target architecture #1350
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
andresgomezfrr
requested review from
wild-endeavor,
kumare3,
eapolinario and
pingsutw
as code owners
|
Thank you for opening this pull request! 🙌 These tips will help get your PR across the finish line:
|
andresgomezfrr
force-pushed
the
add-platform-flytekit-script
branch
from
c8272aa
to
d9ae67d
Compare
honnix
reviewed
Nov 28, 2022
Codecov Report
@@ Coverage Diff @@
## master #1350 +/- ##
==========================================
+ Coverage 68.94% 69.04% +0.10%
==========================================
Files 291 291
Lines 26656 26699 +43
Branches 2508 2514 +6
==========================================
+ Hits 18377 18435 +58
+ Misses 7789 7772 -17
- Partials 490 492 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
honnix
previously approved these changes
Nov 29, 2022
|
Please fix DCO by following https://github.com/flyteorg/flytekit/pull/1350/checks?check_run_id=9764766999 |
andresgomezfrr
force-pushed
the
add-platform-flytekit-script
branch
from
6b078a4
to
4019a7a
Compare
honnix
previously approved these changes
Nov 29, 2022
pingsutw
reviewed
Nov 29, 2022
honnix
reviewed
Nov 30, 2022
|
|
||
| # This build command is the raison d'etre of this script, it ensures that the version is injected into the image itself | ||
| docker build . --build-arg tag="$FLYTE_INTERNAL_IMAGE" -t "$FLYTE_INTERNAL_IMAGE" -f "${DOCKERFILE_PATH}" | ||
| eval "$(docker build ${DOCKER_PLATFORM_OPT} . --build-arg tag=${FLYTE_INTERNAL_IMAGE} -t ${FLYTE_INTERNAL_IMAGE} -f ${DOCKERFILE_PATH})" |
There was a problem hiding this comment.
I don't understand this change. Would the following not work?
docker build ${DOCKER_PLATFORM_OPT} . --build-arg tag=${FLYTE_INTERNAL_IMAGE} -t ${FLYTE_INTERNAL_IMAGE} -f ${DOCKERFILE_PATH}
There was a problem hiding this comment.
Let's check if the linter passes with that. The previous change was based on the linter's suggestion.
honnix
reviewed
Nov 30, 2022
Signed-off-by: Andres Gomez Ferrer <andresg@spotify.com>
Signed-off-by: Andres Gomez Ferrer <andresg@spotify.com>
Signed-off-by: Andres Gomez Ferrer <andresg@spotify.com>
Co-authored-by: Honnix <honnix@users.noreply.github.com> Signed-off-by: Andres Gomez Ferrer <andresg@spotify.com>
andresgomezfrr
force-pushed
the
add-platform-flytekit-script
branch
from
d79f5d2
to
f442671
Compare
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
honnix
force-pushed
the
add-platform-flytekit-script
branch
from
86232c7
to
bd96c45
Compare
|
Hopefully bd96c45 could fix the shellcheck issue. |
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
honnix
approved these changes
Dec 1, 2022
wild-endeavor
approved these changes
Dec 1, 2022
|
Congrats on merging your first pull request! 🎉 |
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.
TL;DR
Add new
TARGET_PLATFORM_BUILDENV var to set the target platform architecture to allow to build docker image on a machine with a different architecture than the docker image will be executed.Type
Are all requirements met?
Complete description
NA
Tracking Issue
flyteorg/flyte#3097
Follow-up issue
NA