-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix(docker): download Terraform and conftest versions maching image architecture #2101
Merged
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
Terraform versions earlier than 0.11.15 does not have binaries available for both amd64, arm64 and armv7, so are being dropped as we can't install the older versions in all the architectures the Docker image is built for.
… is for This avoids us having arm64 binaries for the ARM Docker images, which won't work.
This doesn't fix the armv7 Docker image because conftest doesn't have a binary available for that, so it for now still downloads the x86_64 binary, which is likely to not work - but it's the same as it did before.
chenrui333
approved these changes
Mar 2, 2022
chenrui333
added a commit
that referenced
this pull request
Mar 2, 2022
Signed-off-by: Rui Chen <[email protected]>
Included this change into the 0.18.4 release. (cut the release too early) |
chenrui333
added a commit
that referenced
this pull request
Mar 3, 2022
* debug setup * Revert "fix(docker): download Terraform and conftest versions maching image architecture (#2101)" This reverts commit 579e583. * Revert "fix(docker): fix installation of git-lfs in armv7 image (#2100)" This reverts commit 8af7883. * Revert "fix(docker): fix base image for multi-platform build (#2099)" This reverts commit 571543f. * Revert "debug setup" This reverts commit 274501a.
Sorry about that! I haven't had a chance to look into it today, but I hope to find some time tomorrow so we can get it sorted out. |
chenrui333
pushed a commit
that referenced
this pull request
Mar 4, 2022
* fix(docker): fix base image for multi-platform build (#2099) * Correct indentation of run commands * Split installation of packages into the ones needed at run time and build time This allows us to now repeat the packages which need to be uninstalled again by making use of a virtual package, which - when removed - removes the packages installed as a dependency of it. * Remove unnecessary `rm -rf /var/cache/apk/*` command It's no needed when `apt add` is run with the `--no-cache` option. * Add vertical spacing so it's clearer what is happening when * Test the downloaded binaries to make sure they work on the platform This can help find issues where binaries are downloaded for the wrong platform compared to the architecture the Docker image is built for. * Install dumb-init via apk It's available as a package for Alpine Linux in version 1.2.5 as well, which makes it easier to handle for the different architectures. * Get git-lfs binaries in the right architecture for the Docker image This makes use of the `TARGETPLATFORM` argument which automatically is populated by Docker BuildKit with a string such as "linux/amd64" when the image is being build for an x86_64 architecture. * Install gosu for the right architecture The `case` statement was taken from https://github.com/BretFisher/multi-platform-docker-build as a way of translating the platform name into what we needed for downloading gosu. * fix(docker): fix installation of git-lfs in armv7 image (#2100) This uses a similar pattern than what is used for `GOSU_ARCH` to map the `TARGETPLATFORM` argument into the name of the architecture git-lfs use for their release binaries, as "linux/arm/v7" otherwise would be mapped into "v7" which is wrong. * fix(docker): download Terraform and conftest versions maching image architecture (#2101) * Remove Terraform versions from Docker image which don't have all archs Terraform versions earlier than 0.11.15 does not have binaries available for both amd64, arm64 and armv7, so are being dropped as we can't install the older versions in all the architectures the Docker image is built for. * Download Terraform version depending on the architecture Docker image is for This avoids us having arm64 binaries for the ARM Docker images, which won't work. * Download arm64 conftest binaries for arm64 Docker image This doesn't fix the armv7 Docker image because conftest doesn't have a binary available for that, so it for now still downloads the x86_64 binary, which is likely to not work - but it's the same as it did before. * Correct path to dumb-init in docker-entrypoint.sh The path changed after dumb-init was switched to be installed via `apk` rather than downloaded directly as a binary.
krrrr38
pushed a commit
to krrrr38/atlantis
that referenced
this pull request
Dec 16, 2022
…rchitecture (runatlantis#2101) * Remove Terraform versions from Docker image which don't have all archs Terraform versions earlier than 0.11.15 does not have binaries available for both amd64, arm64 and armv7, so are being dropped as we can't install the older versions in all the architectures the Docker image is built for. * Download Terraform version depending on the architecture Docker image is for This avoids us having arm64 binaries for the ARM Docker images, which won't work. * Download arm64 conftest binaries for arm64 Docker image This doesn't fix the armv7 Docker image because conftest doesn't have a binary available for that, so it for now still downloads the x86_64 binary, which is likely to not work - but it's the same as it did before.
krrrr38
pushed a commit
to krrrr38/atlantis
that referenced
this pull request
Dec 16, 2022
Signed-off-by: Rui Chen <[email protected]>
krrrr38
pushed a commit
to krrrr38/atlantis
that referenced
this pull request
Dec 16, 2022
* debug setup * Revert "fix(docker): download Terraform and conftest versions maching image architecture (runatlantis#2101)" This reverts commit 579e583. * Revert "fix(docker): fix installation of git-lfs in armv7 image (runatlantis#2100)" This reverts commit 8af7883. * Revert "fix(docker): fix base image for multi-platform build (runatlantis#2099)" This reverts commit 571543f. * Revert "debug setup" This reverts commit 274501a.
krrrr38
pushed a commit
to krrrr38/atlantis
that referenced
this pull request
Dec 16, 2022
* fix(docker): fix base image for multi-platform build (runatlantis#2099) * Correct indentation of run commands * Split installation of packages into the ones needed at run time and build time This allows us to now repeat the packages which need to be uninstalled again by making use of a virtual package, which - when removed - removes the packages installed as a dependency of it. * Remove unnecessary `rm -rf /var/cache/apk/*` command It's no needed when `apt add` is run with the `--no-cache` option. * Add vertical spacing so it's clearer what is happening when * Test the downloaded binaries to make sure they work on the platform This can help find issues where binaries are downloaded for the wrong platform compared to the architecture the Docker image is built for. * Install dumb-init via apk It's available as a package for Alpine Linux in version 1.2.5 as well, which makes it easier to handle for the different architectures. * Get git-lfs binaries in the right architecture for the Docker image This makes use of the `TARGETPLATFORM` argument which automatically is populated by Docker BuildKit with a string such as "linux/amd64" when the image is being build for an x86_64 architecture. * Install gosu for the right architecture The `case` statement was taken from https://github.com/BretFisher/multi-platform-docker-build as a way of translating the platform name into what we needed for downloading gosu. * fix(docker): fix installation of git-lfs in armv7 image (runatlantis#2100) This uses a similar pattern than what is used for `GOSU_ARCH` to map the `TARGETPLATFORM` argument into the name of the architecture git-lfs use for their release binaries, as "linux/arm/v7" otherwise would be mapped into "v7" which is wrong. * fix(docker): download Terraform and conftest versions maching image architecture (runatlantis#2101) * Remove Terraform versions from Docker image which don't have all archs Terraform versions earlier than 0.11.15 does not have binaries available for both amd64, arm64 and armv7, so are being dropped as we can't install the older versions in all the architectures the Docker image is built for. * Download Terraform version depending on the architecture Docker image is for This avoids us having arm64 binaries for the ARM Docker images, which won't work. * Download arm64 conftest binaries for arm64 Docker image This doesn't fix the armv7 Docker image because conftest doesn't have a binary available for that, so it for now still downloads the x86_64 binary, which is likely to not work - but it's the same as it did before. * Correct path to dumb-init in docker-entrypoint.sh The path changed after dumb-init was switched to be installed via `apk` rather than downloaded directly as a binary.
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.
This firstly drops Terraform versions prior to 0.11.15 because they don't have all the architectures available that Docker images are built for. I don't assume many people use those versions anymore. Should this be communicated in some special way as it technically is a breaking change?
Terraform is now downloaded for the architecture the Docker image is being built for and the same goes for conftest, although this makes it apparent there's no binary available for conftest for armv7.
I have kept it downloading the x86_64 binary for now in order to keep this change simpler, but I guess the alternatives are: