Merge pull request #138 from buildkite-plugins/Ola-Retry

SUP-5615: Add retry logic for binary download

Author: petetomasik

README.md

@@ -305,6 +305,10 @@ steps:
                 trigger: "deploy-foo-service"
 ```
 
+### Download Retry Behavior
+
+The plugin automatically retries binary downloads up to 3 times with a 5-second delay between attempts. This handles transient network issues when downloading from GitHub.
+
 ### `hooks` (optional)
 
 Currently supports a list of `commands` you wish to execute after the `watched` pipelines have been triggered

hooks/command

@@ -1,6 +1,11 @@
 #!/usr/bin/env bash
 set -euo pipefail
 
+# Retry configuration for downloads
+# Hardcoded values - meant for transient network issues, not hard outages
+max_retries=3
+retry_delay=5
+
 executable="monorepo-diff-buildkite-plugin"
 executable_version_file="${executable}.version"
 
@@ -66,6 +71,29 @@ downloader() {
   fi
 }
 
+# Retry wrapper for downloads (SUP-5615)
+download_with_retry() {
+  local _url="$1"
+  local _dest="$2"
+  local _attempt=1
+
+  while [ $_attempt -le "$max_retries" ]; do
+    say "Downloading binary (attempt ${_attempt}/${max_retries})..."
+    if downloader "$_url" "$_dest"; then
+      say "Download successful"
+      return 0
+    fi
+
+    if [ $_attempt -lt "$max_retries" ]; then
+      say "Download failed, retrying in ${retry_delay} seconds..."
+      sleep "$retry_delay"
+    fi
+    _attempt=$((_attempt + 1))
+  done
+
+  return 1
+}
+
 get_latest_version() {
   local _repo="https://api.github.com/repos/buildkite-plugins/monorepo-diff-buildkite-plugin"
   local _version=""
@@ -143,14 +171,14 @@ download_binary_and_run() {
   fi
 
   if [ -n "${_url}" ]; then
-    if ! downloader "$_url" "${executable}"; then
-      say "failed to download $_url"
+    # Use retry wrapper for resilient downloads (SUP-5615)
+    if ! download_with_retry "$_url" "${executable}"; then
+      say "Failed to download $_url after ${max_retries} attempts"
       exit 1
     fi
     echo "${_binary_version}" > "${executable_version_file}"
   fi
 
-  # todo: move it to a more secure place
   chmod +x "${executable}"
 
   ./"${executable}"

tests/command.bats

@@ -94,3 +94,58 @@ teardown() {
   # Clean up stub (it should not have been called)
   unstub curl || true
 }
+
+# Tests for download_with_retry() function (SUP-5615)
+
+@test "download retries on transient failure and succeeds" {
+  export BUILDKITE_PLUGIN_MONOREPO_DIFF_DOWNLOAD=true
+  export BUILDKITE_PLUGIN_MONOREPO_DIFF_BUILDKITE_PLUGIN_TEST_MODE=false
+  # Use pinned version to skip get_latest_version API call
+  export BUILDKITE_PLUGINS='[{"github.com/buildkite-plugins/monorepo-diff-buildkite-plugin#v1.0.0": {}}]'
+  
+  # Remove mock binary so it actually needs to download
+  rm -f "$PWD/monorepo-diff-buildkite-plugin"
+  rm -f "$PWD/monorepo-diff-buildkite-plugin.version"
+  
+  # Stub curl: fail twice on download, then succeed and create executable
+  stub curl \
+    "-sSfL * -o * : exit 1" \
+    "-sSfL * -o * : exit 1" \
+    "-sSfL * -o * : echo '#!/bin/bash' > \"\${4}\"; echo 'echo test' >> \"\${4}\"; exit 0"
+
+  run "$PWD/hooks/command"
+
+  assert_output --partial "Downloading binary (attempt 1/3)"
+  assert_output --partial "Download failed, retrying"
+  assert_output --partial "Downloading binary (attempt 2/3)"
+  assert_output --partial "Downloading binary (attempt 3/3)"
+  assert_output --partial "Download successful"
+
+  unstub curl
+}
+
+@test "download fails after max retries exhausted" {
+  export BUILDKITE_PLUGIN_MONOREPO_DIFF_DOWNLOAD=true
+  export BUILDKITE_PLUGIN_MONOREPO_DIFF_BUILDKITE_PLUGIN_TEST_MODE=false
+  # Use pinned version to skip get_latest_version API call
+  export BUILDKITE_PLUGINS='[{"github.com/buildkite-plugins/monorepo-diff-buildkite-plugin#v1.0.0": {}}]'
+  
+  # Remove mock binary so it actually needs to download
+  rm -f "$PWD/monorepo-diff-buildkite-plugin"
+  
+  # Stub curl to always fail on download
+  stub curl \
+    "-sSfL * -o * : exit 1" \
+    "-sSfL * -o * : exit 1" \
+    "-sSfL * -o * : exit 1"
+
+  run "$PWD/hooks/command"
+
+  assert_failure
+  assert_output --partial "Downloading binary (attempt 1/3)"
+  assert_output --partial "Downloading binary (attempt 2/3)"
+  assert_output --partial "Downloading binary (attempt 3/3)"
+  assert_output --partial "Failed to download"
+
+  unstub curl
+}