Failure recovery for OpenStack deployment#2400
Failure recovery for OpenStack deployment#2400cb-github-robot merged 1 commit intocloud-barista:mainfrom
Conversation
Signed-off-by: Seokho Son <shsongist@gmail.com>
|
/approve |
There was a problem hiding this comment.
Pull request overview
This PR improves resilience of the OpenStack DevStack deployment script by adding retry behavior for transient network/package failures and by tuning git settings to reduce clone/pull failures on large repos.
Changes:
- Added a generic
retry()helper and wrapped apt update/upgrade/install steps with retries. - Added stack-user git configuration (HTTP/1.1, buffers, low-speed thresholds) to reduce GnuTLS/transfer failures.
- Reworked the DevStack clone/update block to run under
retry.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sudo apt-get update -qq | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get upgrade -y -qq \ | ||
| retry sudo apt-get update -qq | ||
| retry sudo DEBIAN_FRONTEND=noninteractive apt-get upgrade -y -qq \ |
There was a problem hiding this comment.
retry sudo DEBIAN_FRONTEND=noninteractive apt-get upgrade ... relies on sudo VAR=value ... environment injection, which is often blocked by sudoers unless SETENV is enabled. Since this script already aims for unattended installs, make the environment passing explicit (e.g., run sudo env DEBIAN_FRONTEND=noninteractive ... apt-get upgrade ... or sudo -E with appropriate env_keep) so the command behaves consistently across hosts.
| retry sudo DEBIAN_FRONTEND=noninteractive apt-get upgrade -y -qq \ | |
| retry sudo env DEBIAN_FRONTEND=noninteractive NEEDRESTART_MODE=a NEEDRESTART_SUSPEND=1 apt-get upgrade -y -qq \ |
| retry sudo -u stack bash -c "OPENSTACK_BRANCH='$OPENSTACK_BRANCH' | ||
| cd /opt/stack | ||
| if [ -d devstack ]; then | ||
| echo 'DevStack directory exists, pulling latest...' | ||
| cd devstack && git checkout "$OPENSTACK_BRANCH" && git pull | ||
| cd devstack && git checkout \"\$OPENSTACK_BRANCH\" && git pull | ||
| else | ||
| git clone https://opendev.org/openstack/devstack -b "$OPENSTACK_BRANCH" | ||
| git clone https://opendev.org/openstack/devstack -b \"\$OPENSTACK_BRANCH\" | ||
| fi | ||
| DEVSTACK_CLONE | ||
| " |
There was a problem hiding this comment.
The new sudo -u stack bash -c "OPENSTACK_BRANCH='$OPENSTACK_BRANCH' ..." embeds the user-provided --branch value into a shell string. If the branch contains a single-quote or newline, it can break quoting and inject extra shell commands. Prefer passing the branch via sudo -u stack env OPENSTACK_BRANCH="$OPENSTACK_BRANCH" bash -c '...' (or revert to the quoted heredoc pattern) to avoid injection and reduce brittle escaping (e.g., \"\$OPENSTACK_BRANCH\").
| while [ $attempt -le $max_attempts ]; do | ||
| if "$@"; then | ||
| return 0 | ||
| fi | ||
| echo "Attempt $attempt/$max_attempts failed. Retrying in ${delay}s..." | ||
| sleep $delay |
There was a problem hiding this comment.
In retry(), the test and sleep arguments are unquoted ([ $attempt -le $max_attempts ], sleep $delay). While these locals are currently always set, quoting them (and using [[ ... ]]) avoids word-splitting/globbing pitfalls and makes the helper safer to reuse if values ever become empty or non-numeric.
| while [ $attempt -le $max_attempts ]; do | |
| if "$@"; then | |
| return 0 | |
| fi | |
| echo "Attempt $attempt/$max_attempts failed. Retrying in ${delay}s..." | |
| sleep $delay | |
| while [[ "$attempt" -le "$max_attempts" ]]; do | |
| if "$@"; then | |
| return 0 | |
| fi | |
| echo "Attempt $attempt/$max_attempts failed. Retrying in ${delay}s..." | |
| sleep "$delay" |
No description provided.