-
Notifications
You must be signed in to change notification settings - Fork 12
add retries for ephemeral instance api calls #61
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
ff31d3c
3ca264f
dc0474e
363f287
b7d3d58
44a4b1e
8d3353d
89ed704
8f70f75
b3ad5bc
5f7b169
3ba8a36
0ff0b96
dba16e5
9036a17
e88acfc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| #!/bin/bash | ||
|
|
||
| # retry() function: Retries a given command up to 'retries' times with a 'wait' interval. | ||
| # Usage: retry <command> | ||
| # Example: retry my_api_call_function | ||
| retry() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lukqw I think this is where your input would really be valuable. Is this retry mechanism what you would expect a client to do if they get a failure response from the ephemeral instance API? |
||
| local retries=5 | ||
| local count=0 | ||
| local wait=5 | ||
| local output | ||
| while [ $count -lt $retries ]; do | ||
| # We disable set -e for the command and capture its output. | ||
| output=$(set +e; "$@") | ||
| local exit_code=$? | ||
| if [ $exit_code -eq 0 ]; then | ||
| echo "$output" | ||
| return 0 | ||
| fi | ||
| count=$((count + 1)) | ||
| echo "Command failed with exit code $exit_code. Retrying in $wait seconds... ($count/$retries)" >&2 | ||
| sleep $wait | ||
| done | ||
| echo "Command failed after $retries retries." >&2 | ||
| echo "$output" # Also return the output of the last failed attempt for debugging | ||
| return 1 | ||
| } | ||
|
|
||
| # Helper function to check for a JSON error response from the API | ||
| # Usage: check_for_api_error "<response_body>" "<context_message>" | ||
| check_for_api_error() { | ||
| local response="$1" | ||
| local context_message="$2" | ||
| if echo "$response" | jq -e 'if type == "object" and has("error") then true else false end' > /dev/null; then | ||
| echo "API error during '$context_message': $response" >&2 | ||
| return 1 | ||
| fi | ||
| return 0 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| name: Shutdown Ephemeral Instance | ||
| description: 'Shutdowns an Ephemeral Instance (PR Preview)' | ||
|
|
||
| inputs: | ||
| localstack-api-key: | ||
| description: 'LocalStack API key used to access the platform api' | ||
| required: true | ||
| description: 'LocalStack Auth Token used to access the platform api' | ||
| required: false | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, that's a good point! That's maybe something to revisit in the future. The term is outdated (we only have auth tokens now) and it seems a bit confusing to configure... |
||
| github-token: | ||
| description: 'Github token used to create PR comments' | ||
| required: true | ||
|
|
@@ -34,16 +35,38 @@ runs: | |
| - name: Shutdown ephemeral instance | ||
| shell: bash | ||
| run: | | ||
| response=$(curl -X DELETE \ | ||
| -s -o /dev/null -w "%{http_code}" \ | ||
| -H "ls-api-key: ${LOCALSTACK_AUTH_TOKEN:-${LOCALSTACK_API_KEY:-${{ inputs.localstack-api-key }}}}" \ | ||
| -H "content-type: application/json" \ | ||
| https://api.localstack.cloud/v1/compute/instances/$previewName) | ||
| if [[ "$response" -ne 200 ]]; then | ||
| # In case the deletion fails, e.g. if the instance cannot be found, we raise a proper error on the platform | ||
| echo "Unable to delete preview environment. API response: $response" | ||
| exit 1 | ||
| fi | ||
| AUTH_HEADER="ls-api-key: ${LOCALSTACK_AUTH_TOKEN:-${LOCALSTACK_API_KEY:-${{ inputs.localstack-api-key }}}}" | ||
| CONTENT_TYPE_HEADER="content-type: application/json" | ||
| API_URL_BASE="https://api.localstack.cloud/v1/compute/instances" | ||
|
|
||
| source ${{ github.action_path }}/../retry-function.sh | ||
| shutdown_instance() { | ||
| # The API returns a 200 on successful deletion. | ||
| # We use --fail-with-body so curl fails on server errors (5xx) and triggers the retry. | ||
| local response | ||
| response=$(curl --fail-with-body -s -w "\n%{http_code}" -X DELETE \ | ||
| -H "$AUTH_HEADER" \ | ||
| -H "$CONTENT_TYPE_HEADER" \ | ||
| "$API_URL_BASE/$previewName") | ||
| local exit_code=$? | ||
| local http_code=$(echo "$response" | tail -n1) | ||
| local body=$(echo "$response" | sed '$d') | ||
|
|
||
| if [ $exit_code -ne 0 ]; then | ||
| # A 404 means it's already gone, which is a success case for shutdown. | ||
| if [ "$http_code" -ne 404 ]; then | ||
| echo "Error deleting instance, curl failed with exit code $exit_code. API response: $body" >&2 | ||
| return 1 | ||
| fi | ||
| fi | ||
| if [ "$http_code" -eq 200 ]; then | ||
| echo "Instance '$previewName' deleted successfully." | ||
| elif [ "$http_code" -eq 404 ]; then | ||
| echo "Instance '$previewName' was already deleted (not found)." | ||
| fi | ||
| } | ||
|
|
||
| retry shutdown_instance | ||
|
|
||
| - name: Update status comment | ||
| uses: actions-cool/maintain-one-comment@v3.1.1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing the
workflow_dispatchthat I added in a previous PR, as it doesn't make sense with the logic that the ephemeral instances are currently following (e.g. searching for the PR number and adding comment to the PR)