fix(Docker): Pass build args to runtime images

[upbqdn]

Motivation

When we create the release binaries in our CI, we pass some params from the release workflow (https://github.com/zcashfoundation/zebra/blob/ee54a59ee20fcae73a04f26198146b23ccd0de39/.github/workflows/release-binaries.yml#L46-L51) to the build workflow (https://github.com/zcashfoundation/zebra/blob/ee54a59ee20fcae73a04f26198146b23ccd0de39/.github/workflows/build-docker-image.yml#L156-L166). The latter then passes the params to Dockerfile as build-args. Docker then uses them, and also passes them to the deps image as env vars. However, when we create the runtime image, we cherry-pick only the required binaries and files, and we put them into a fresh image that doesn't contain the env vars from the deps image. This also means that the env vars that runtime-entrypoint.sh relies on are not set.

Depends-On: #7158

Solution

Pick up ARGs passed to the Dockerfile for runtime images and pass them to the image as env vars. Note that ARGs are scoped (https://docs.docker.com/engine/reference/builder/#scope) between FROM instructions, so we need to declare them separately in the runtime image.

Review

I wasn't completely sure about which env vars to pick and also about the default values. I'd be glad if the reviewers checked that we

• have all env vars we need,
• there are no redundant ones, and
• all of them have the right default value (some are not set by default).

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

[teor2345]

This looks good to me, but I think we might also want these env vars:

• SHORT_SHA: tells us which commit Zebra was built from in the logs

zebra/zebrad/src/application.rs

Line 177 in f6afec2

const GIT_COMMIT_GCLOUD: Option<&str> = option_env!("SHORT_SHA");

I'd also like Gustavo to review this PR, because he made the last change to this config, and Dockerfiles are his thing.

[upbqdn]

I added SHORT_SHA.

[teor2345]

Run gcloud compute ssh generate-checkpoints-testnet-7175-merge-d97a99f
ERROR: (gcloud.compute.ssh) There was a problem refreshing your current auth tokens: HTTPSConnectionPool(host='sts.googleapis.com', port=443): Read timed out. (read timeout=120)

Temporary failure on the testnet checkpoints job.

[gustavovalverde]

The only environment variables needed at build time are: RUST_BACKTRACE, COLORBT_SHOW_HIDDEN, RUST_LOG, and CARGO_HOME.

The ones added here are just environment variables for runtime, not build time.

If we need environment variables available at runtime, then we should add those here:
https://github.com/ZcashFoundation/zebra/blob/main/.github/workflows/continous-delivery.yml#L266
and
https://github.com/ZcashFoundation/zebra/blob/main/.github/workflows/continous-delivery.yml#L367

Most of these environment variables are wrongly defined here as build args: https://github.com/zcashfoundation/zebra/blob/ee54a59ee20fcae73a04f26198146b23ccd0de39/.github/workflows/build-docker-image.yml#L156-L166 (the ones only used at runtime should be removed from here)

That's why this might be confusing.

[teor2345]

The only environment variables needed at build time are: RUST_BACKTRACE, COLORBT_SHOW_HIDDEN, RUST_LOG, and CARGO_HOME.

SHORT_SHA is compiled into the Zebra binaries at build time, so Zebra's logs show the commit it was built from. This environmental variable is not read at runtime (it's already in the binary as a constant).

zebra/zebrad/src/application.rs

Line 177 in f6afec2

const GIT_COMMIT_GCLOUD: Option<&str> = option_env!("SHORT_SHA");

(In Rust, env!() is a build-time macro, and env::var() is a runtime function. There's also an optional variant of the macro.)

FEATURES and TEST_FEATURES are used at build time, they control which Rust features are compiled into production and test binaries. They are also used at runtime by the entry point script to configure Zebra.

And for completeness, RUST_BACKTRACE, COLORBT_SHOW_HIDDEN, and RUST_LOG change how the Rust compiler logs at build time, and how Zebra logs at runtime.

[gustavovalverde]

My previous reply could be confusing based on how the Dockerfile has been built so far, so I'll reformulate based on @teor2345 answers.

SHORT_SHA is compiled into the Zebra binaries at build time, so Zebra's logs show the commit it was built from. This environmental variable is not read at runtime (it's already in the binary as a constant).

SHORT_SHA is required to be on the Dockerfile as and ARG and ENV as we have no other means to add an environment variable while building in our CI other than using arguments in the pipeline, like this: https://github.com/ZcashFoundation/zebra/blob/main/.github/workflows/build-docker-image.yml#L158. But not only that, we need it in the Dockerfile as theSHORT_SHA value is only available at build time, the user can't provide this value.

But it should be added here: https://github.com/ZcashFoundation/zebra/blob/main/docker/Dockerfile#L66-L95 in the deps stage so it's available for both tests and release stages (not explicitly on runtime, as this uses the release stage values).

FEATURES and TEST_FEATURES are used at build time, they control which Rust features are compiled into production and test binaries. They are also used at runtime by the entry point script to configure Zebra.

Both of these variables have default ARG for tests and production in the deps stage: https://github.com/ZcashFoundation/zebra/blob/main/docker/Dockerfile#L85-L89 which should be enough to set default values at build time.

If the user would like to build with a different value in production, they'd just need to build with docker build --build-arg FEATURES=<user_values> ..., but we also use this in the entrypoint as a variable, so it should be (in L85):

ARG FEATURES="default-release-binaries"
ENV FEATURES=${FEATURES}

And for completeness, RUST_BACKTRACE, COLORBT_SHOW_HIDDEN, and RUST_LOG change how the Rust compiler logs at build time, and how Zebra logs at runtime.

These can be kept as-is in the deps stage.

And about the other ones which were added in the PR:

• RPC_PORT : Not needed on the Dockerfile as it's not required at build time and the user can set this value using docker run -e RPC_PORT=<user_port> ...

• RUST_BACKTRACE, COLORBT_SHOW_HIDDEN, and RUST_LOG: Not needed on the Dockerfile at runtime as their defaults are already set in the deps stage, and the user can change those for the runtime using docker run -e RUST_BACKTRACE=<user_value> COLORBT_SHOW_HIDDEN=<user_value> ... or even using a whole .env file with --env-file ./.env

Summarizing:

• If an environment variable value is only available when building the image in CI, and it's dynamic (as the SHA), it must be defined as an ARG and ENVindeps`
• If an environment variable value should be available when building the image, and it's static (as the RUST_BACKTRACE), it's not required to be defined, unless you'd like to set defaults, then it must be defined as an ARG and ENV in deps
• If a variable value should be available when building the image, but not as an environment variable, and ARG should be enough (the ARG must be used in the RUN commands, and won't be available on the image)
• If a variable value is only required when running the image, this can be omitted from the Dockerfile if defined in the entrypoint.sh

[gustavovalverde]

Extra info.

This once helped me to understand this better
Summary: https://vsupalov.com/docker-arg-vs-env/
Deep-dive: https://vsupalov.com/docker-arg-env-variable-guide/

[upbqdn]

RPC_PORT : Not needed on the Dockerfile as it's not required at build time and the user can set this value using docker run -e RPC_PORT=<user_port> ...

We need a way of passing the RPC port from the release-binaries workflow to runtime-entrypoint.sh. Similarly for other env vars such as $FEATURES. I tried explaining what the issue is in the PR description. Using Docker's ARG instruction is the only solution I could think of.

[upbqdn]

From reading the discussion above, I think we only need the env vars in this PR that are required by runtime-entrypoint.sh. Those are $RPC_PORT and $FEATURES.

[gustavovalverde]

This was superseded by #7200