Adding Git binary to core rp image #5568
Conversation
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
build/git.mk
Outdated
| sudo make install install-doc install-html install-info | ||
| mkdir -p $(OUT_DIR)/gitbinary | ||
| cp /tmp/gitbinary/bin/git $(OUT_DIR)/gitbinary/ | ||
| chmod +x $(OUT_DIR)/gitbinary/git |
There was a problem hiding this comment.
Git installation goes through, but looks like the binary isn't executable based on the error (install directory permissions seemed ok)
Error: Failed to download module\n\nCould not download module \"\" (main.tf.json:25) source code from\n\"git::https://github.com/Azure/terraform-azurerm-cosmosdb?ref=v1.0.0\": error\ndownloading 'https://github.com/Azure/terraform-azurerm-cosmosdb?ref=v1.0.0':\nerror running /usr/local/bin/git: .\n\n\nError: Failed to download module\n\nCould not download module \"\" (main.tf.json:25) source code from\n\"git::https://github.com/Azure/terraform-azurerm-cosmosdb?ref=v1.0.0\": error\ndownloading 'https://github.com/Azure/terraform-azurerm-cosmosdb?ref=v1.0.0':\nerror running /usr/local/bin/git: .\n\n\nError: Failed to update module manifest\n\nUnable to write the module manifest file: open\n.terraform/modules/modules.json: no such file or directory
Test Results2 549 tests ±0 2 542 ✔️ ±0 1m 57s ⏱️ -1s Results for commit 32321fb. ± Comparison against base commit 3300c29. This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
For the detailed report, please go to
|
kachawla
force-pushed
the
kachawla/git-binary
branch
from
01f594d to
4fc6031
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
kachawla
force-pushed
the
kachawla/git-binary
branch
from
4fc6031 to
07a3847
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
kachawla/git-binary
branch
from
07a3847 to
779d954
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
kachawla/git-binary
branch
from
779d954 to
cfcfb64
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
kachawla/git-binary
branch
from
cfcfb64 to
6998519
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
kachawla/git-binary
branch
from
6998519 to
322cb07
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
kachawla/git-binary
branch
from
322cb07 to
0856378
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
deploy/images/appcore-rp/Dockerfile
Outdated
| @@ -7,6 +7,7 @@ ARG TARGETARCH | |||
| WORKDIR / | |||
|
|
|||
| COPY ./linux_${TARGETARCH:-amd64}/release/appcore-rp / | |||
| COPY ./gitbinary/git /usr/local/bin/ | |||
There was a problem hiding this comment.
An option may be to include directly in Docker File:
RUN apt-get update && apt-get install -y git
not sure if it is ideal approach but copy-pasting from chatgpt:
This command will update the package index and install Git in the container. The -y flag is used to automatically answer "yes" to any prompts that may appear during the installation process. Note that you should also include the apt-get update command to ensure that the package index is up to date before installing Git.
There was a problem hiding this comment.
Tried this as the first thing, but the base image is distroless so package managers are not available, hence the need to build it from the source. Let me know if you meant something else.
There was a problem hiding this comment.
FYI we discussed offline and decided to pivot back to alpine.
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
build/git.mk
Outdated
| mkdir -p $(OUT_DIR)/gitbinary | ||
| cp /tmp/gitbinary/bin/git $(OUT_DIR)/gitbinary/ | ||
| chmod +x $(OUT_DIR)/gitbinary/git | ||
| sudo rm -rf /tmp/gitbinary |
There was a problem hiding this comment.
We can undo this now right?
There was a problem hiding this comment.
yes, removing it :) thanks for your help with the options here
deploy/images/appcore-rp/Dockerfile
Outdated
| RUN apk --no-cache add ca-certificates | ||
|
|
||
| # Install Git (required for Terraform module downloads) | ||
| RUN apk --no-cache add git |
There was a problem hiding this comment.
If possible, you really want to combine these two steps (line 7-10). It will result in fewer layers
There was a problem hiding this comment.
I'm glad you found --no-cache. That was the thing I suspected I would have to give feedback about 😆
deploy/images/appcore-rp/Dockerfile
Outdated
| @@ -1,14 +1,23 @@ | |||
| # Note: distroless already includes ca-certificates | |||
| FROM --platform=${TARGETPLATFORM:-linux/amd64} gcr.io/distroless/static:nonroot | |||
| FROM --platform=${TARGETPLATFORM:-linux/amd64} alpine:latest | |||
There was a problem hiding this comment.
Can we pin to a specific version instead of latest? Suggest 3.18 https://github.com/alpinelinux/docker-alpine/blob/571516c7ce3ecec3c70541d13529a4abb33ba362/x86_64/Dockerfile
There was a problem hiding this comment.
Good point, updated
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
kachawla
force-pushed
the
kachawla/git-binary
branch
from
0c75594 to
b297999
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
kachawla
force-pushed
the
kachawla/git-binary
branch
from
b297999 to
6e2baf3
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
kachawla
force-pushed
the
kachawla/git-binary
branch
from
6e2baf3 to
32321fb
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
Description
Terraform uses Git to download modules from TF registry, so we need Git as a part of core rp since Terraform recipe execution will happen inside core RP for now.
To achieve this, switching CoreRP to use Alpine image instead of distroless. We can revert back to distroless when we move Terraform execution to separate container.
Issue reference
Fixes: https://dev.azure.com/azure-octo/Incubations/_workitems/edit/7919
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: