-
Notifications
You must be signed in to change notification settings - Fork 117
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
Reduce terraform deployer build verbosity #665
Reduce terraform deployer build verbosity #665
Conversation
RUN apk add --no-cache git openssh curl | ||
|
||
# Announcements for new releases: https://groups.google.com/g/google-cloud-sdk-announce |
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.
👍
ENV GCLOUD_SDK_VERSION 369.0.0 | ||
ENV PATH $PATH:/usr/local/gcloud/google-cloud-sdk/bin | ||
RUN curl "https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud-sdk-$GCLOUD_SDK_VERSION-linux-x86_64.tar.gz" > /tmp/google-cloud-sdk.tar.gz \ | ||
RUN curl --silent "https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud-sdk-$GCLOUD_SDK_VERSION-linux-x86_64.tar.gz" > /tmp/google-cloud-sdk.tar.gz \ |
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.
curl -s <url> -o /tmp/google-cloud-sdk.tar.gz
&& tar -C /usr/local/gcloud -xvf /tmp/google-cloud-sdk.tar.gz \ | ||
&& /usr/local/gcloud/google-cloud-sdk/install.sh -q --override-components="bq" \ | ||
&& tar -C /usr/local/gcloud -xf /tmp/google-cloud-sdk.tar.gz \ | ||
&& /usr/local/gcloud/google-cloud-sdk/install.sh -q --override-components="bq" &>/dev/null \ |
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.
According to the public docs, there is --disable-prompts
flag. We can give it a try and let the installer decide what's rubbish and what isn't :)
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.
updated. Where can I look at the logs? I've not been able to find the pipeline where this log is displayed :(
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.
Do you mean this?
[2022-01-26T14:00:31.542Z] ---> Running in f3dbb078bd82
[2022-01-26T14:00:41.632Z] Welcome to the Google Cloud SDK!
[2022-01-26T14:00:41.632Z] WARNING: You appear to be running this script as root. This may cause
[2022-01-26T14:00:41.632Z] the installation to be inaccessible to users other than the root user.
[2022-01-26T14:00:43.561Z] usage: install.py [-h] [--usage-reporting USAGE_REPORTING]
[2022-01-26T14:00:43.561Z] [--screen-reader SCREEN_READER] [--rc-path RC_PATH]
[2022-01-26T14:00:43.561Z] [--command-completion COMMAND_COMPLETION]
[2022-01-26T14:00:43.561Z] [--path-update PATH_UPDATE] [--disable-installation-options]
[2022-01-26T14:00:43.561Z] [--override-components [OVERRIDE_COMPONENTS [OVERRIDE_COMPONENTS ...]]]
[2022-01-26T14:00:43.561Z] [--additional-components ADDITIONAL_COMPONENTS [ADDITIONAL_COMPONENTS ...]]
[2022-01-26T14:00:43.561Z] [--quiet] [--install-python INSTALL_PYTHON]
[2022-01-26T14:00:43.561Z] install.py: error: unrecognized arguments: --disable-prompts
[2022-01-26T14:00:47.792Z] Service 'terraform' failed to build: The command '/bin/sh -c curl --silent "https://dl.google.com/dl/cloudsdk/channels/rapid/downloads/google-cloud-sdk-$GCLOUD_SDK_VERSION-linux-x86_64.tar.gz" > /tmp/google-cloud-sdk.tar.gz && mkdir -p /usr/local/gcloud && tar -C /usr/local/gcloud -xf /tmp/google-cloud-sdk.tar.gz && /usr/local/gcloud/google-cloud-sdk/install.sh -q --override-components="bq" --disable-prompts && rm /tmp/google-cloud-sdk.tar.gz' returned a non-zero code: 2
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.
It looks there are different options available, sorry for linking wrong docs. Could you please adjust the command?
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.
--disable-prompts
is not available with the installation method in use, the tests for 8e505f2 broke with:
[2022-01-26T14:00:41.632Z] Welcome to the Google Cloud SDK!
[2022-01-26T14:00:41.632Z] WARNING: You appear to be running this script as root. This may cause
[2022-01-26T14:00:41.632Z] the installation to be inaccessible to users other than the root user.
[2022-01-26T14:00:43.561Z] usage: install.py [-h] [--usage-reporting USAGE_REPORTING]
[2022-01-26T14:00:43.561Z] [--screen-reader SCREEN_READER] [--rc-path RC_PATH]
[2022-01-26T14:00:43.561Z] [--command-completion COMMAND_COMPLETION]
[2022-01-26T14:00:43.561Z] [--path-update PATH_UPDATE] [--disable-installation-options]
[2022-01-26T14:00:43.561Z] [--override-components [OVERRIDE_COMPONENTS [OVERRIDE_COMPONENTS ...]]]
[2022-01-26T14:00:43.561Z] [--additional-components ADDITIONAL_COMPONENTS [ADDITIONAL_COMPONENTS ...]]
[2022-01-26T14:00:43.561Z] [--quiet] [--install-python INSTALL_PYTHON]
[2022-01-26T14:00:43.561Z] install.py: error: unrecognized arguments: --disable-prompts
Reverting the commit
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.
Looks more readable, and seems to take less space (tough maybe more components need to be installed with the optional packages).
FROM ubuntu:20.04
RUN apt-get -q update && apt-get install -yq curl apt-transport-https ca-certificates gnupg && apt-get clean
ADD google-cloud-sdk.list /etc/apt/sources.list.d/google-cloud-sdk.list
RUN curl https://packages.cloud.google.com/apt/doc/apt-key.gpg | apt-key add -
ENV GCLOUD_SDK_VERSION 369.0.0-0
RUN apt-get -q update && \
apt-get -yq install google-cloud-sdk=${GCLOUD_SDK_VERSION} && \
apt-get clean
da5b98a5dec3 About a minute ago /bin/sh -c apt-get -q update && apt-get … 620MB
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.
eb91706519b5 5 minutes ago /bin/sh -c curl "https://dl.google.com/dl/cl… 1.05GB
Yeah, that's something I wanted originally to omit, but it looks like it may not be so easy unless we modify the service deployer image to pull it in runtime? That's probably a topic for a different discussion.
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.
@mtojek I'm already using the versioned archive installation. install.sh
triggers a python script that takes care of installing other components so I don't think can be safely skipped.
Could you please try again without calling install.sh at all? I suppose you need to unpack the archive in the correct place instead of /tmp.
I tried it but the result is not good. This is what happens when running gcloud
in the docker container without running install:
/workspace # gcloud
ERROR: (gcloud) Command name argument expected.
ERROR: gcloud failed to load (gcloud.interactive): Problem loading gcloud.interactive: cannot import name 'Mapping' from 'collections' (/usr/local/lib/python3.10/collections/__init__.py).
This usually indicates corruption in your gcloud installation or problems with your Python interpreter.
[...]
PS We will definitely need a package system test to verify if it works as expected.
This is in the works!
I'm moving forward to using Ubuntu as base image.
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 updated the image to use Ubuntu.
These are the dimensions for the two images:
ubuntu 978MB
alpine 594MB
@jsoriano the gcloud SDK when installed from a versioned archive is a lot less that 1.05GB, as is possible to specify which components to install thus reducing the overall size.
I'm happy with both, let me know what do you think of the current change :)
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.
Much better now and managed in a normal way :)
I will drop a few nit-picks.
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 think we have an agreement. It's more like a final polishing of the PR.
&& tar -C /usr/local/gcloud -xvf /tmp/google-cloud-sdk.tar.gz \ | ||
&& /usr/local/gcloud/google-cloud-sdk/install.sh -q --override-components="bq" \ | ||
&& rm /tmp/google-cloud-sdk.tar.gz | ||
ENV GCLOUD_SDK_VERSION 370.0.0-0 |
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.
Please move this to the top of the file (after FROM)
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.
done
&& /usr/local/gcloud/google-cloud-sdk/install.sh -q --override-components="bq" \ | ||
&& rm /tmp/google-cloud-sdk.tar.gz | ||
ENV GCLOUD_SDK_VERSION 370.0.0-0 | ||
RUN echo "deb [signed-by=/usr/share/keyrings/cloud.google.gpg] http://packages.cloud.google.com/apt cloud-sdk main" | tee -a /etc/apt/sources.list.d/google-cloud-sdk.list \ |
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 think you mixed all commands a bit.
- Install GPG key and cloud-sdk repo
- apt-get update -yq && apt-get install -yq curl apt-transport-https.... - google-cloud-sdk=${GCLOUD_SDK_VERSION} && apt-get clean
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.
curl apt-transport-https gnupg
are required to add the deb repo and install the repo key, so two steps are required unfortunately.
Would you prefer to split GPG key and repo addition from the actual package installation?
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.
You can try something like this:
apt-get update -yq && \
apt-get install apt-transport-https gnupg && \
deb [signed-by=/usr/share/keyrings/cloud.google.gpg] http://packages.cloud.google.com/apt cloud-sdk main" | tee -a /etc/apt/sources.list.d/google-cloud-sdk.list && \
apt-get update -yq && \
apt-get install -yq google-cloud-sdk=${GCLOUD_SDK_VERSION} && \
apt-get clean
The aim is to consider this as an atomic piece to prevent any APT repo update from breaking the container build.
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 updated it with the suggestion. Just consider that if in the future another repo will be required I think would be best to extract the dependency installation so there is no implicit dependency.
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.
@mtojek I extracted it again, as Hashicorp has it's own repo that has the same requirements. It happened sooner than expected :D
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.
Something is fishy with the terraform as it doesn't start. Could you please investigate it?
@@ -1,17 +1,14 @@ | |||
FROM hashicorp/terraform:light as terraform |
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.
Now that we have apt maybe we can also install terraform using it :) https://www.terraform.io/downloads
And we could also pin the version.
This is unrelated to this PR in any case.
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.
Actually it may help if this is a PATH issue or library issue :)
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.
Added installation through deb thanks for the suggestion!
@endorama Please pull the latest main branch. |
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 fetched this code locally, run it, and checked logs for the terraform service deployer:
$ docker logs 49fee7edacf7
/run.sh: 3: set: Illegal option -o pipefail
You have to update the terraform_deployer_run.sh
to /bin/bash
. sh
most likely points to a different shell.
internal/compose/compose.go
Outdated
// Service is up and running and it's healthy | ||
if containerDescription.State.Status == "running" && containerDescription.State.Health.Status == "healthy" { | ||
continue | ||
for _, containerDescription := range descriptions { |
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.
Let's extract this change around ticker to a separate PR and focus here on improving the terraform Docker image.
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.
Done #687
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.
So, could you please remove it from this PR?
internal/compose/compose.go
Outdated
// Service is up and running and it's healthy | ||
if containerDescription.State.Status == "running" && containerDescription.State.Health.Status == "healthy" { | ||
continue | ||
for _, containerDescription := range descriptions { |
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.
So, could you please remove it from this PR?
Reduce output from gcloud SDK installation process, by silencing the various steps. This is needed to reduce image build log verbosity; gcloud SDK installation output is also difficult to read on terminal that do not support colours.
/test |
@@ -1,7 +1,9 @@ | |||
#!sh | |||
#!/usr/bin/env bash | |||
|
|||
set -euxo pipefail |
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.
Let's drop -x
to have a clean nice output in container logs. This script doesn't contain tangled logic which may cause issues.
@@ -1,7 +1,9 @@ | |||
#!sh | |||
#!/usr/bin/env bash |
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.
The reason why the CI is failing is because of the Dockerfile ENTRYPOINT set to sh
: here.
Let's quickly refactor it to /bin/bash
instead and verify.
set -euxo pipefail | ||
set -euo pipefail | ||
|
||
echo "BASH_VERSION: $BASH_VERSION" |
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.
Could you please remove this line?
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.
LGTM
🎉🎉 thank you! |
In #665 we removed `-x` to debug an issue with script shebang, but the intention wasn't to remove it entirely from the script. This commit sets `-x` for the whole the script.
Reduce output from gcloud SDK installation process, by silencing the various steps.
This is needed to reduce image build log verbosity; gcloud SDK installation output is also difficult to read on terminal that do not support colours.