Skip to content

Proxy-friendly version of install-docker-repository #654

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Proxy-friendly version of install-docker-repository #654

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1ab018
Proxy-friendly version of install-docker-repository
arnaudrichard Jul 15, 2025
c3a1b1b
typo in output message
arnaudrichard Jul 16, 2025
acc725c
typo in output message
arnaudrichard Jul 16, 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
17 changes: 8 additions & 9 deletions scripts/ubuntu/1.1-install-docker-repository.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,22 @@ source "$SCRIPT_DIR/utils.sh"
print_start_of_script

set +e
print_script_step "Verify docker.download.com is reachable"
# Verify docker.download.com is reachable before attempting to install the
print_script_step "Verify download.docker.com is reachable"
# Verify download.docker.com is reachable before attempting to install the
# Docker Package Repo (network randomly fails after service restarts).
for i in {1..5}
do
timeout 2 bash -c "(echo >/dev/tcp/docker.download.com/80) &>/dev/null"
retVal=$?
if [ $retVal -eq 0 ]; then
echo "The docker.download.com is reacheable"
status_code=$(sudo curl -sS -o /dev/null -w "%{http_code}" --connect-timeout 1 "https://download.docker.com" ${https_proxy+--proxy $https_proxy} )
if [ $? -eq 0 ] && [ "$status_code" -lt 400 ]; then
echo "The download.docker.com is reachable"
break
else
echo "The docker.download.com is unreachable for try $i"
echo "The download.docker.com is unreachable for try $i"
sleep $(expr $i \* 2)
Copy link

Choose a reason for hiding this comment

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

medium

For better performance and adherence to modern shell scripting practices, it's recommended to use arithmetic expansion $((...)) instead of the external expr command1. This avoids forking a new process for a simple calculation.

Style Guide References

Suggested change
sleep $(expr $i \* 2)
sleep $((i * 2))

Footnotes

  1. The Google Shell Style Guide recommends using $(()) for arithmetic operations over the expr command for efficiency and simplicity. ↩

fi

if [ "$i" -eq '5' ]; then
echo "Failed to stablish connection with the docker.download.com service."
echo "Failed to establish connection with the download.docker.com service."
echo "Please verify your connection or try again later."
exit 1
fi
Expand All @@ -52,7 +51,7 @@ print_script_step "Add Docker's official GPG key"
sudo apt-get update -y
sudo apt-get install ca-certificates curl -y
sudo install -m 0755 -d /etc/apt/keyrings
sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc
sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc ${https_proxy+--proxy $https_proxy}
Copy link

Choose a reason for hiding this comment

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

medium

The proxy logic ${https_proxy+--proxy $https_proxy} is now used in two places (here and in the reachability check loop). To improve maintainability and avoid repetition, consider defining a variable for the proxy options at the beginning of the script and reusing it.

For example:

# At the top of the script
PROXY_CURL_OPTS=""
if [ -n "$https_proxy" ]; then
  PROXY_CURL_OPTS="--proxy $https_proxy"
fi

# ... then use it in curl commands
sudo curl ... $PROXY_CURL_OPTS

This makes the code cleaner and easier to update if more curl commands are added in the future.

sudo chmod a+r /etc/apt/keyrings/docker.asc

print_script_step "Add the repository to Apt sources"
Expand Down