Skip to content

Make install_packages resilient to 500 Server Error #3286

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 1, 2025
Merged

Make install_packages resilient to 500 Server Error #3286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6363e96
Make `install_packages` resilient to 500 Server Error
gsoldevila Nov 19, 2025
9062abd
Merge branch 'master' into make-install-packages-more-resilient
bmorelli25 Nov 21, 2025
f5f84a9
Move script to .docker
gsoldevila Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .docker/install_packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/sh

MAX_ATTEMPTS=5
WAIT_TIME=10
RETRY_COUNT=0

# Check if any package names were provided
if [ $# -eq 0 ]; then
echo "Error: install_packages requires at least one package name." >&2
exit 1
fi

# Loop for retry attempts
while [ $RETRY_COUNT -lt $MAX_ATTEMPTS ]; do
echo "Attempt $((RETRY_COUNT + 1)) of $MAX_ATTEMPTS: Running install_packages $@"
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable $@ should be quoted as "$@" to properly handle arguments containing spaces or special characters. Without quotes, arguments with spaces will be split into multiple arguments.

Suggested change
echo "Attempt $((RETRY_COUNT + 1)) of $MAX_ATTEMPTS: Running install_packages $@"
echo "Attempt $((RETRY_COUNT + 1)) of $MAX_ATTEMPTS: Running install_packages \"$@\""
echo "Arguments: $@"

Copilot uses AI. Check for mistakes.

# Execute the actual install command
install_packages "$@"

# Check the exit status of the previous command
if [ $? -eq 0 ]; then
echo "Packages installed successfully."
exit 0 # Success, exit the function
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "exit the function" but this is a script, not a function. The comment should say "exit the script" for clarity.

Suggested change
exit 0 # Success, exit the function
exit 0 # Success, exit the script

Copilot uses AI. Check for mistakes.
else
RETRY_COUNT=$((RETRY_COUNT + 1))
if [ $RETRY_COUNT -lt $MAX_ATTEMPTS ]; then
echo "Installation failed. Retrying in $WAIT_TIME seconds..."
sleep $WAIT_TIME
fi
fi
Comment on lines +18 to +30
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using $? in a condition is fragile. The exit status should be captured immediately after the command. Consider rewriting as:

if install_packages "$@"; then
    echo "Packages installed successfully."
    exit 0
else
    RETRY_COUNT=$((RETRY_COUNT + 1))
    # ... rest of error handling
fi

This ensures the exit status is evaluated correctly without risk of being overwritten by intervening commands.

Copilot uses AI. Check for mistakes.
done

echo "ERROR: Package installation failed after $MAX_ATTEMPTS attempts." >&2
exit 1
15 changes: 8 additions & 7 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ RUN echo "deb http://archive.debian.org/debian/ buster main" > /etc/apt/sources.

# TODO install_packages calls apt-get update and then nukes the list files after. We should avoid multiple calls to apt-get update.....
Copy link
Contributor Author

@gsoldevila gsoldevila Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nik9000 I don't fully understand your comment above, but I wonder if the wrapper script that I'm introducing can help in any way.

# We could probably fix this by running the update and installs ourself with `RUN --mount type=cache` but that is "experimental"
COPY --chmod=755 .docker/install_packages.sh /usr/local/bin/

# Fix for Debian Buster EOL - point to archive repositories
RUN sed -i 's/deb.debian.org/archive.debian.org/g' /etc/apt/sources.list && \
sed -i 's/security.debian.org/archive.debian.org/g' /etc/apt/sources.list && \
sed -i '/buster-updates/d' /etc/apt/sources.list

RUN install_packages apt-transport-https gnupg2 ca-certificates
RUN install_packages.sh apt-transport-https gnupg2 ca-certificates
COPY .docker/apt/keys/nodesource.gpg /
RUN apt-key add /nodesource.gpg
COPY .docker/apt/sources.list.d/nodesource.list /etc/apt/sources.list.d/
RUN install_packages \
RUN install_packages.sh \
build-essential python2 \
# needed for compiling native modules on ARM
nodejs ruby \
Expand All @@ -43,7 +44,7 @@ ENV LC_ALL en_US.UTF-8


FROM base AS ruby_deps
RUN install_packages \
RUN install_packages.sh \
bundler \
# Fetches ruby dependencies
ruby-dev make cmake gcc libc-dev patch
Expand All @@ -61,7 +62,7 @@ FROM base AS node_deps
COPY .docker/apt/keys/yarn.gpg /
RUN apt-key add /yarn.gpg
COPY .docker/apt/sources.list.d/yarn.list /etc/apt/sources.list.d/
RUN install_packages yarn=1.22.19-1
RUN install_packages.sh yarn=1.22.19-1
COPY package.json /
COPY yarn.lock /
ENV YARN_CACHE_FOLDER=/tmp/.yarn-cache
Expand All @@ -74,7 +75,7 @@ RUN yarn install --frozen-lockfile --production
# Dockerfiles to make the images to serve previews and air gapped docs.
FROM base AS build
LABEL MAINTAINERS="Nik Everett <nik@elastic.co>"
RUN install_packages \
RUN install_packages.sh \
git \
# Clone source repositories and commit to destination repositories
libnss-wrapper \
Expand Down Expand Up @@ -110,7 +111,7 @@ RUN rm -rf /var/log/nginx && rm -rf /run/nginx
##### Everything below this run tests
FROM base AS py_test
# There's not a published wheel for yamale, so we need setuptools and wheel
RUN install_packages python3 python3-pip python3-setuptools python3-wheel python3-dev libxml2-dev libxslt-dev zlib1g-dev
RUN install_packages.sh python3 python3-pip python3-setuptools python3-wheel python3-dev libxml2-dev libxslt-dev zlib1g-dev
RUN pip3 install \
beautifulsoup4==4.8.1 \
lxml==4.4.2 \
Expand All @@ -131,4 +132,4 @@ COPY --from=ruby_test /usr/local/bin/rspec /usr/local/bin/rspec
COPY --from=ruby_test /usr/local/bin/rubocop /usr/local/bin/rubocop

FROM py_test AS diff_tool
RUN install_packages git
RUN install_packages.sh git