From 472007dd7ce5c5b861b29870a852ad7c99e1ea01 Mon Sep 17 00:00:00 2001 From: Fedor Dikarev Date: Tue, 4 Feb 2025 19:58:02 +0100 Subject: [PATCH] ci: unify Dockerfiles, set bash as SHELL for debian layers, make cpan step as separate RUN (#10645) ## Problem Ref: https://github.com/neondatabase/cloud/issues/23461 and follow-up after: https://github.com/neondatabase/neon/pull/10553 we used `echo` to set-up `.wgetrc` and `.curlrc`, and there we used `\n` to make these multiline configs with one echo command. The problem is that Debian `/bin/sh`'s built-in echo command behaves differently from the `/bin/echo` executable and from the `echo` built-in in `bash`. Namely, it does not support the`-e` option, and while it does treat `\n` as a newline, passing `-e` here will add that `-e` to the output. At the same time, when we use different base images, for example `alpine/curl`, their `/bin/sh` supports and requires `-e` for treating escape sequences like `\n`. But having different `echo` and remembering difference in their behaviour isn't best experience for the developer and makes bad experience maintaining Dockerfiles. Work-arounds: - Explicitly use `/bin/bash` (like in this PR) - Use `/bin/echo` instead of the shell's built-in echo function - Use printf "foo\n" instead of echo -e "foo\n" ## Summary of changes 1. To fix that, we process with the option setting `/bin/bash` as a SHELL for the debian-baysed layers 2. With no changes for `alpine/curl` based layers. 3. And one more change here: in `extensions` layer split to the 2 steps: installing dependencies from `CPAN` and installing `lcov` from github, so upgrading `lcov` could reuse previous layer with installed cpan modules. --- build-tools.Dockerfile | 22 +++++++++++++++++----- compute/compute-node.Dockerfile | 10 +++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/build-tools.Dockerfile b/build-tools.Dockerfile index f744b44808..3ade57b175 100644 --- a/build-tools.Dockerfile +++ b/build-tools.Dockerfile @@ -3,8 +3,13 @@ ARG DEBIAN_VERSION=bookworm FROM debian:bookworm-slim AS pgcopydb_builder ARG DEBIAN_VERSION +# Use strict mode for bash to catch errors early +SHELL ["/bin/bash", "-euo", "pipefail", "-c"] + +# By default, /bin/sh used in debian images will treat '\n' as eol, +# but as we use bash as SHELL, and built-in echo in bash requires '-e' flag for that. RUN echo 'Acquire::Retries "5";' > /etc/apt/apt.conf.d/80-retries && \ - echo -e "retry_connrefused = on\ntimeout=15\ntries=5\n" > /root/.wgetrc \ + echo -e "retry_connrefused = on\ntimeout=15\ntries=5\n" > /root/.wgetrc && \ echo -e "--retry-connrefused\n--connect-timeout 15\n--retry 5\n--max-time 300\n" > /root/.curlrc RUN if [ "${DEBIAN_VERSION}" = "bookworm" ]; then \ @@ -55,7 +60,8 @@ ARG DEBIAN_VERSION # Add nonroot user RUN useradd -ms /bin/bash nonroot -b /home -SHELL ["/bin/bash", "-c"] +# Use strict mode for bash to catch errors early +SHELL ["/bin/bash", "-euo", "pipefail", "-c"] RUN mkdir -p /pgcopydb/bin && \ mkdir -p /pgcopydb/lib && \ @@ -66,7 +72,7 @@ COPY --from=pgcopydb_builder /usr/lib/postgresql/16/bin/pgcopydb /pgcopydb/bin/p COPY --from=pgcopydb_builder /pgcopydb/lib/libpq.so.5 /pgcopydb/lib/libpq.so.5 RUN echo 'Acquire::Retries "5";' > /etc/apt/apt.conf.d/80-retries && \ - echo -e "retry_connrefused = on\ntimeout=15\ntries=5\n" > /root/.wgetrc \ + echo -e "retry_connrefused = on\ntimeout=15\ntries=5\n" > /root/.wgetrc && \ echo -e "--retry-connrefused\n--connect-timeout 15\n--retry 5\n--max-time 300\n" > /root/.curlrc # System deps @@ -190,8 +196,14 @@ RUN set -e \ # It includes several bug fixes on top on v2.0 release (https://github.com/linux-test-project/lcov/compare/v2.0...master) # And patches from us: # - Generates json file with code coverage summary (https://github.com/neondatabase/lcov/commit/426e7e7a22f669da54278e9b55e6d8caabd00af0.tar.gz) -RUN for package in Capture::Tiny DateTime Devel::Cover Digest::MD5 File::Spec JSON::XS Memory::Process Time::HiRes JSON; do yes | perl -MCPAN -e "CPAN::Shell->notest('install', '$package')"; done \ - && wget https://github.com/neondatabase/lcov/archive/426e7e7a22f669da54278e9b55e6d8caabd00af0.tar.gz -O lcov.tar.gz \ +RUN set +o pipefail && \ + for package in Capture::Tiny DateTime Devel::Cover Digest::MD5 File::Spec JSON::XS Memory::Process Time::HiRes JSON; do \ + yes | perl -MCPAN -e "CPAN::Shell->notest('install', '$package')";\ + done && \ + set -o pipefail +# Split into separate step to debug flaky failures here +RUN wget https://github.com/neondatabase/lcov/archive/426e7e7a22f669da54278e9b55e6d8caabd00af0.tar.gz -O lcov.tar.gz \ + && ls -laht lcov.tar.gz && sha256sum lcov.tar.gz \ && echo "61a22a62e20908b8b9e27d890bd0ea31f567a7b9668065589266371dcbca0992 lcov.tar.gz" | sha256sum --check \ && mkdir -p lcov && tar -xzf lcov.tar.gz -C lcov --strip-components=1 \ && cd lcov \ diff --git a/compute/compute-node.Dockerfile b/compute/compute-node.Dockerfile index ea29630001..9379856eab 100644 --- a/compute/compute-node.Dockerfile +++ b/compute/compute-node.Dockerfile @@ -96,8 +96,10 @@ ARG DEBIAN_VERSION # Use strict mode for bash to catch errors early SHELL ["/bin/bash", "-euo", "pipefail", "-c"] +# By default, /bin/sh used in debian images will treat '\n' as eol, +# but as we use bash as SHELL, and built-in echo in bash requires '-e' flag for that. RUN echo 'Acquire::Retries "5";' > /etc/apt/apt.conf.d/80-retries && \ - echo -e "retry_connrefused = on\ntimeout=15\ntries=5\n" > /root/.wgetrc \ + echo -e "retry_connrefused = on\ntimeout=15\ntries=5\n" > /root/.wgetrc && \ echo -e "--retry-connrefused\n--connect-timeout 15\n--retry 5\n--max-time 300\n" > /root/.curlrc RUN case $DEBIAN_VERSION in \ @@ -1068,6 +1070,7 @@ ENV PATH="/home/nonroot/.cargo/bin:$PATH" USER nonroot WORKDIR /home/nonroot +# See comment on the top of the file regading `echo` and `\n` RUN echo -e "--retry-connrefused\n--connect-timeout 15\n--retry 5\n--max-time 300\n" > /home/nonroot/.curlrc RUN curl -sSO https://static.rust-lang.org/rustup/dist/$(uname -m)-unknown-linux-gnu/rustup-init && \ @@ -1584,6 +1587,7 @@ FROM alpine/curl:${ALPINE_CURL_VERSION} AS exporters ARG TARGETARCH # Keep sql_exporter version same as in build-tools.Dockerfile and # test_runner/regress/test_compute_metrics.py +# See comment on the top of the file regading `echo`, `-e` and `\n` RUN echo -e "--retry-connrefused\n--connect-timeout 15\n--retry 5\n--max-time 300\n" > /root/.curlrc; \ if [ "$TARGETARCH" = "amd64" ]; then\ postgres_exporter_sha256='027e75dda7af621237ff8f5ac66b78a40b0093595f06768612b92b1374bd3105';\ @@ -1700,6 +1704,10 @@ ENV PGDATABASE=postgres ######################################################################################### FROM debian:$DEBIAN_FLAVOR ARG DEBIAN_VERSION + +# Use strict mode for bash to catch errors early +SHELL ["/bin/bash", "-euo", "pipefail", "-c"] + # Add user postgres RUN mkdir /var/db && useradd -m -d /var/db/postgres postgres && \ echo "postgres:test_console_pass" | chpasswd && \