From 1deeec27e55c60bde7aae98723bb0951a1ab9be7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jukka=20Palom=C3=A4ki?= <jukka@jpalomaki.fi>
Date: Sun, 5 Sep 2021 23:26:04 +0300
Subject: [PATCH] Make stop action safer (#6)

This might help in situations where the start job is cancelled, making the stop job more likely to successfully terminate the instance
---
 README.md        |  2 +-
 start/action.yml | 10 +++++-----
 stop/action.yml  | 26 +++++++++++++++++---------
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/README.md b/README.md
index 552a17b..9f3bc95 100644
--- a/README.md
+++ b/README.md
@@ -56,8 +56,8 @@ jobs:
       - run: echo '${{ matrix.message }}'
 
   stop-runner:
+    if: always()
     needs: [start-runner, complex-build]
-    if: always() && needs.start-runner.result == 'success'
     runs-on: ubuntu-20.04
     steps:
       - name: Stop runner
diff --git a/start/action.yml b/start/action.yml
index 1a46d59..e5dffdc 100644
--- a/start/action.yml
+++ b/start/action.yml
@@ -42,7 +42,7 @@ outputs:
     description: GitHub repository runner id
     value: ${{ steps.main.outputs.runner-id }}
   instance-id:
-    description: EC2 instance id
+    description: AWS EC2 instance id
     value: ${{ steps.main.outputs.instance-id }}
 
 runs:
@@ -78,6 +78,8 @@ runs:
 
         instance_id="$(aws ec2 run-instances --launch-template "$LAUNCH_TEMPLATE" --user-data "$user_data" | jq -r .Instances[0].InstanceId)"
 
+        echo "::set-output name=instance-id::$instance_id"
+
         echo "Started EC2 instance: $instance_id"
 
         echo "Waiting for repository runner to be registered ..."
@@ -97,11 +99,9 @@ runs:
             aws ec2 terminate-instances --instance-ids "$instance_id"
             exit 1
         else
-            echo "Repository runner registered with id: $runner_id"
+            echo "::set-output name=runner-id::$runner_id"
+            echo "Repository runner started (ID: $runner_id)"
         fi
-
-        echo "::set-output name=runner-id::$runner_id"
-        echo "::set-output name=instance-id::$instance_id"
       env:
         GH_TOKEN: ${{ inputs.github-token }}
         AWS_DEFAULT_REGION: ${{ inputs.aws-region }}
diff --git a/stop/action.yml b/stop/action.yml
index e4843c8..ae627fe 100644
--- a/stop/action.yml
+++ b/stop/action.yml
@@ -46,16 +46,24 @@ runs:
     - id: main
       shell: bash
       run: |
-        echo "Deregistering repository runner: $RUNNER_ID ..."
+        # Offline runners are automatically deleted after 30 days, so we emit a warning for runner deregistration failure
+        if [ -z "$RUNNER_ID" ]; then
+            echo "WARN: Unable to deregister GitHub repository runner (runner ID not available)"
+        else
+            echo "Deregistering GitHub repository runner: $RUNNER_ID ..."
+            gh api -X DELETE -H 'Accept: application/vnd.github.v3+json' \
+              "repos/$GITHUB_REPO/actions/runners/$RUNNER_ID" || echo "WARN: Failed to deregister GitHub repository runner (GitHub API call failed)"
+        fi
         
-        gh api -X DELETE -H 'Accept: application/vnd.github.v3+json' \
-          "repos/$GITHUB_REPO/actions/runners/$RUNNER_ID" || echo "WARN: Failed to deregister runner"
-        
-        echo "Terminating EC2 instance: $INSTANCE_ID ..."
-        
-        aws ec2 terminate-instances --instance-ids "$INSTANCE_ID" > /dev/null
-        
-        echo "Repository runner stopped"
+        # Failure to terminate the EC2 instance will incur unnecessary costs, so we fail the job to ensure the user notices
+        if [ -z "$INSTANCE_ID" ]; then
+            echo "ERROR: Unable to terminate EC2 instance (instance ID not available)"
+            exit 1
+        else
+            echo "Terminating EC2 instance: $INSTANCE_ID ..."
+            aws ec2 terminate-instances --instance-ids "$INSTANCE_ID" > /dev/null
+            echo "Repository runner stopped"
+        fi
       env:
         GH_TOKEN: ${{ inputs.github-token }}
         AWS_DEFAULT_REGION: ${{ inputs.aws-region }}