From 2e8b2267b35133fe3320422fb1ad64bd88545090 Mon Sep 17 00:00:00 2001 From: Heather Booker Date: Thu, 13 Jun 2019 14:56:41 -0400 Subject: [PATCH 1/3] Add error handling and configurable retries to curl calls in curlgh --- README.md | 3 ++- bin/common.sh | 48 +++++++++++++++++++++++++++++++++++++++++++- bin/out | 18 ----------------- test/in/error-503.sh | 46 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 20 deletions(-) create mode 100644 test/in/error-503.sh diff --git a/README.md b/README.md index dd8d2d1..0848c34 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,8 @@ Parameters: * `description` - a short description of the status * `description_path` - path to an input file whose data is the value of `description` * `target_url` - the target URL to associate with the status (default: concourse build link) - + * `sleep_duration` - the amount of time in seconds to wait while retrying for the status (default: 3) + * `max_attempts` - the number of times to retry for the status to be ready (default: 5) ## Example diff --git a/bin/common.sh b/bin/common.sh index 45b30b0..890060a 100644 --- a/bin/common.sh +++ b/bin/common.sh @@ -50,5 +50,51 @@ curlgh () { else skip_verify_arg="" fi - curl $skip_verify_arg -s -H "Authorization: token $source_access_token" $@ + + attempts=0 + maxAttempts="${max_attempts:-5}" + sleepDuration="${sleep_duration:-3}" + while [[ $attempts -lt $maxAttempts ]]; do + attempts=$((attempts+=1)) + curl $skip_verify_arg -s -D/tmp/responseheaders -H "Authorization: token $source_access_token" $@ > /tmp/rawresponse + + httpStatus=$(head -n1 /tmp/responseheaders | sed 's|HTTP.* \([0-9]*\) .*|\1|') + if [[ "$httpStatus" -eq "200" ]]; then # If HTTP status is OK, skip to extracting the statuses + break; + fi + + # Various error handling (authn, authz, rate-limiting, transient API errors) + if [[ "$httpStatus" -ge 400 ]]; then + if [[ "$httpStatus" -lt 500 ]]; then # 4XX range + if [[ $(grep -i 'rate-limit' /tmp/rawresponse || echo '0') -ge 1 ]]; then + now=$(date "+%s") + ratelimitReset=$(cat /tmp/responseheaders | sed -n 's|X-RateLimit-Reset: \([0-9]*\)|\1|p') + + sleepDuration=$((ratelimitReset-now)) + if [[ "$sleepDuration" -lt 1 ]]; then # Protects against timing issue + sleepDuration=1 + fi + echo "Limited by the API rate limit. Script will retry at $(date -d@$((now+sleepDuration)))" >&2 + else + fatal "Authentication error against the GitHub API" + fi + else # 5XX range + echo "Unexpected HTTP $(echo $httpStatus) when querying the GitHub API" >&2 + sleepDuration=5 + fi + else # Other status code that's not 200 OK, nor in the 400+ range + fatal "Unexpected HTTP status code when querying the GitHub API: $(echo $httpStatus)" + fi + + # Exit if we have reach the maximum number of attemps, or sleep and retry otherwise + if [[ $attempts -eq $maxAttempts ]]; then + fatal "Maximum number of attempts reached while trying to query the GitHub API" + else + echo "Will retry in $sleepDuration seconds" >&2 + sleep $sleepDuration + fi + + done + + cat /tmp/rawresponse } diff --git a/bin/out b/bin/out index a3107bf..06677f3 100755 --- a/bin/out +++ b/bin/out @@ -87,14 +87,6 @@ jq -c -n \ | curlgh -d@- "$source_endpoint/repos/$source_repository/statuses/$commit" \ > /tmp/gh-result -# -# check retry counter -# - -REMAINING_TRIES=5 - -while [[ $REMAINING_TRIES -gt 0 ]]; do - # # lookup # @@ -116,16 +108,6 @@ curlgh "$source_endpoint/repos/$source_repository/commits/$source_branch/status" && jq -e '.status' < /tmp/status > /dev/null \ && break -# -# decrease retry counter and loop -# - -REMAINING_TRIES=$(($REMAINING_TRIES - 1)) - -sleep 1 - -done - # # concourse # diff --git a/test/in/error-503.sh b/test/in/error-503.sh new file mode 100644 index 0000000..01e4e6f --- /dev/null +++ b/test/in/error-503.sh @@ -0,0 +1,46 @@ +#!/bin/sh + +set -eu + +DIR=$( dirname "$0" )/../.. + +cat < $TMPDIR/http.req-$$ & +HTTP/1.0 503 Service Unavailable + +EOF + +in_dir=$TMPDIR/status-$$ + +mkdir $in_dir + +set +e + +$DIR/bin/in "$in_dir" > $TMPDIR/resource-$$ 2>&1 < Date: Thu, 13 Jun 2019 14:56:41 -0400 Subject: [PATCH 2/3] Add configurable retry settings on both in and out --- README.md | 2 ++ bin/in | 26 ++++++++++++++++++++++++-- bin/out | 29 ++++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0848c34..efe7deb 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,8 @@ Parameters: * `target_url` - the target URL to associate with the status (default: concourse build link) * `sleep_duration` - the amount of time in seconds to wait while retrying for the status (default: 3) * `max_attempts` - the number of times to retry for the status to be ready (default: 5) + * `retry_timeout` - the amount of time in seconds to wait while retrying for the status (Default: 3) + * `retry_count` - the number of times to retry for the status to be ready (Default: 5) ## Example diff --git a/bin/in b/bin/in index 9b6710b..1b9a477 100755 --- a/bin/in +++ b/bin/in @@ -11,6 +11,13 @@ eval $( jq -r '{ "version_status": .version.status } | to_entries[] | .key + "=" + @sh "\(.value)"' < /tmp/stdin ) +# +# check retry counter +# + +REMAINING_TRIES="${retry_count:-5}" + +while true; do # # lookup @@ -30,9 +37,24 @@ curlgh "$source_endpoint/repos/$source_repository/commits/$version_commit/status # validate # -jq -e '.status' < /tmp/status > /dev/null \ - || fatal "Status not found on $( jq -r '.sha' < /tmp/status )" +jq -e '.status' < /tmp/status > /dev/null +check_status="$?" + +if [ "$check_status" -eq 0 ]; then + break +elif [ "$check_status" -gt 0 ] && [ "$REMAINING_TRIES" -le 1 ]; then + fatal "Status not found on $( jq -r '.sha' < /tmp/status )" +fi + +# +# decrease retry counter and loop +# + +REMAINING_TRIES=$(($REMAINING_TRIES - 1)) + +sleep "${retry_timeout:-3}" +done # # concourse diff --git a/bin/out b/bin/out index 06677f3..e4b2fd4 100755 --- a/bin/out +++ b/bin/out @@ -87,6 +87,14 @@ jq -c -n \ | curlgh -d@- "$source_endpoint/repos/$source_repository/statuses/$commit" \ > /tmp/gh-result +# +# check retry counter +# + +REMAINING_TRIES="${retry_count:-5}" + +while true; do + # # lookup # @@ -104,9 +112,24 @@ curlgh "$source_endpoint/repos/$source_repository/commits/$source_branch/status" # validate # -[[ -s /tmp/status ]] \ - && jq -e '.status' < /tmp/status > /dev/null \ - && break +jq -e '.status' < /tmp/status > /dev/null +check_status="$?" + +if [ "$check_status" -eq 0 ]; then + break +elif [ "$check_status" -gt 0 ] && [ "$REMAINING_TRIES" -le 1 ]; then + fatal "Status not found on $( jq -r '.sha' < /tmp/status )" +fi + +# +# decrease retry counter and loop +# + +REMAINING_TRIES=$(($REMAINING_TRIES - 1)) + +sleep "${retry_timeout:-3}" + +done # # concourse From fb9b1580b3db82eea00db3785897e3aace2f6137 Mon Sep 17 00:00:00 2001 From: Alexandre Leveille Date: Thu, 20 Jun 2019 15:02:41 -0400 Subject: [PATCH 3/3] Make the variables case and names consistent for the latest changes --- README.md | 6 ++--- bin/common.sh | 52 +++++++++++++++++++++------------------ bin/in | 8 +++--- bin/out | 6 +++-- test/in/missing-status.sh | 2 +- 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index efe7deb..e690bf0 100644 --- a/README.md +++ b/README.md @@ -43,10 +43,8 @@ Parameters: * `description` - a short description of the status * `description_path` - path to an input file whose data is the value of `description` * `target_url` - the target URL to associate with the status (default: concourse build link) - * `sleep_duration` - the amount of time in seconds to wait while retrying for the status (default: 3) - * `max_attempts` - the number of times to retry for the status to be ready (default: 5) - * `retry_timeout` - the amount of time in seconds to wait while retrying for the status (Default: 3) - * `retry_count` - the number of times to retry for the status to be ready (Default: 5) + * `retry_count` - the number of times to retry for the status to be ready (default: 5) + * `retry_delay` - the amount of time in seconds to wait while retrying for the status (default: 3) ## Example diff --git a/bin/common.sh b/bin/common.sh index 890060a..a92e171 100644 --- a/bin/common.sh +++ b/bin/common.sh @@ -51,49 +51,53 @@ curlgh () { skip_verify_arg="" fi - attempts=0 - maxAttempts="${max_attempts:-5}" - sleepDuration="${sleep_duration:-3}" - while [[ $attempts -lt $maxAttempts ]]; do - attempts=$((attempts+=1)) + REMAINING_TRIES="${retry_count:-5}" + + while true; do + # Output the response headers and body to two separate files so that we can easily work on them both curl $skip_verify_arg -s -D/tmp/responseheaders -H "Authorization: token $source_access_token" $@ > /tmp/rawresponse - httpStatus=$(head -n1 /tmp/responseheaders | sed 's|HTTP.* \([0-9]*\) .*|\1|') - if [[ "$httpStatus" -eq "200" ]]; then # If HTTP status is OK, skip to extracting the statuses + http_status=$(head -n1 /tmp/responseheaders | sed 's|HTTP.* \([0-9]*\) .*|\1|') + # If HTTP status is OK, break the retry loop now to carry on (skip all error handling & retries) + if [ "$http_status" -eq "200" ]; then break; fi - # Various error handling (authn, authz, rate-limiting, transient API errors) - if [[ "$httpStatus" -ge 400 ]]; then - if [[ "$httpStatus" -lt 500 ]]; then # 4XX range - if [[ $(grep -i 'rate-limit' /tmp/rawresponse || echo '0') -ge 1 ]]; then + if [ "$http_status" -ge 400 ]; then + if [ "$http_status" -le 499 ]; then # 400-499 range + if [ $(grep -i 'rate-limit' /tmp/rawresponse || echo '0') -ge 1 ]; then now=$(date "+%s") - ratelimitReset=$(cat /tmp/responseheaders | sed -n 's|X-RateLimit-Reset: \([0-9]*\)|\1|p') + ratelimit_reset=$(cat /tmp/responseheaders | sed -n 's|X-RateLimit-Reset: \([0-9]*\)|\1|p') - sleepDuration=$((ratelimitReset-now)) - if [[ "$sleepDuration" -lt 1 ]]; then # Protects against timing issue - sleepDuration=1 + sleep_duration="$((ratelimit_reset - now))" + # If our system clock is in advance to GitHub's the result of sleep_duration might be a negative number + if [[ "$sleep_duration" -lt 1 ]]; then + sleep_duration="1" fi - echo "Limited by the API rate limit. Script will retry at $(date -d@$((now+sleepDuration)))" >&2 + echo "Limited by the API rate limit. Script will retry at $( date -d@$((now + sleep_duration)) )" >&2 else fatal "Authentication error against the GitHub API" fi - else # 5XX range - echo "Unexpected HTTP $(echo $httpStatus) when querying the GitHub API" >&2 - sleepDuration=5 + else # 500+ range + echo "Unexpected HTTP $(echo $http_status) when querying the GitHub API" >&2 + sleep_duration="${retry_delay:-3}" fi else # Other status code that's not 200 OK, nor in the 400+ range - fatal "Unexpected HTTP status code when querying the GitHub API: $(echo $httpStatus)" + fatal "Unexpected HTTP status code when querying the GitHub API: $(echo $http_status)" fi + # Exit if we have reach the maximum number of attemps, or sleep and retry otherwise - if [[ $attempts -eq $maxAttempts ]]; then + if [ "$REMAINING_TRIES" -le 0 ]; then fatal "Maximum number of attempts reached while trying to query the GitHub API" - else - echo "Will retry in $sleepDuration seconds" >&2 - sleep $sleepDuration fi + echo "Will retry in $sleep_duration seconds" >&2 + + REMAINING_TRIES=$(($REMAINING_TRIES - 1)) + + sleep $sleep_duration + done cat /tmp/rawresponse diff --git a/bin/in b/bin/in index 1b9a477..31df29d 100755 --- a/bin/in +++ b/bin/in @@ -32,17 +32,17 @@ curlgh "$source_endpoint/repos/$source_repository/commits/$version_commit/status }' \ > /tmp/status - # # validate # - +set +e jq -e '.status' < /tmp/status > /dev/null check_status="$?" +set -e if [ "$check_status" -eq 0 ]; then break -elif [ "$check_status" -gt 0 ] && [ "$REMAINING_TRIES" -le 1 ]; then +elif [ "$check_status" -gt 0 ] && [ "$REMAINING_TRIES" -le 0 ]; then fatal "Status not found on $( jq -r '.sha' < /tmp/status )" fi @@ -52,7 +52,7 @@ fi REMAINING_TRIES=$(($REMAINING_TRIES - 1)) -sleep "${retry_timeout:-3}" +sleep "${retry_delay:-3}" done diff --git a/bin/out b/bin/out index e4b2fd4..f854bdc 100755 --- a/bin/out +++ b/bin/out @@ -112,12 +112,14 @@ curlgh "$source_endpoint/repos/$source_repository/commits/$source_branch/status" # validate # +set +e jq -e '.status' < /tmp/status > /dev/null check_status="$?" +set -e if [ "$check_status" -eq 0 ]; then break -elif [ "$check_status" -gt 0 ] && [ "$REMAINING_TRIES" -le 1 ]; then +elif [ "$check_status" -gt 0 ] && [ "$REMAINING_TRIES" -le 0 ]; then fatal "Status not found on $( jq -r '.sha' < /tmp/status )" fi @@ -127,7 +129,7 @@ fi REMAINING_TRIES=$(($REMAINING_TRIES - 1)) -sleep "${retry_timeout:-3}" +sleep "${retry_delay:-3}" done diff --git a/test/in/missing-status.sh b/test/in/missing-status.sh index 9c528b0..7dc4bef 100755 --- a/test/in/missing-status.sh +++ b/test/in/missing-status.sh @@ -19,7 +19,7 @@ mkdir $in_dir set +e -$DIR/bin/in "$in_dir" > $TMPDIR/resource-$$ 2>&1 < $TMPDIR/resource-$$ 2>&1 <