Merge #46

46: add confidence checks to mitigate race conditions [semver:minor] r=eddiewebb a=eddiewebb

### Checklist

<!--
	thank you for contributing to CircleCI Concurrency Control Orb!
	before submitting your request, please go through the following
	items and place an x in the [ ] if they have been completed
-->

- [x] All new jobs, commands, executors, parameters have descriptions
- [ ] Examples have been added for any significant new features
- [ ] README has been updated, if necessary

### Motivation, issues

this helps mitigate the issue in #26 by adding redundant checks before allowing job to proceed

### Description

add confidence threshold to repeat running job check.


Co-authored-by: Eddie Webb <[email protected]>

Author: bors[bot]

src/@orb.yml

@@ -3,11 +3,15 @@ description: |
   Allows jobs or entire workflows to be queued to ensure they run in serial.  
   This is ideal for deployments or other activities that must not run concurrently.
   May optionaly consider branch-level isolation if unique branches should run concurrently. 
-  This orb requires the project to have an API to query build statuses.
+  This orb requires the project to have an API key in order to query build states.
+  
+display:
+  home_url: https://eddiewebb.github.io/circleci-queue/
+  source_url: https://github.com/eddiewebb/circleci-queue
 
 examples:
   queue_workflow:
-    description: Used typically as first job and will queue until no jobs from a previous workflow are running
+    description: Used typically as first job and will queue until no previous workflows are running
     usage:
       version: 2.1
       orbs:
@@ -24,7 +28,10 @@ examples:
                   - queue/block_workflow
 
   single_concurrency_job:
-    description: Used to ensure that a single job (deploy) is not run concurrently
+    description: |
+      Used to ensure that a only single job (deploy) is not run concurrently. 
+      By default will only queue if the same job from previous worfklows is running on the same branch. 
+      This allows safe jobs like build/test to overlap, minimizing overall queue times.
     usage:
       version: 2.1
       orbs:

src/commands/until_front_of_line.yml

@@ -3,11 +3,6 @@ parameters:
     type: boolean
     default: true
     description: "Should we only consider jobs running on the same branch?"
-  consider-job:
-    type: boolean
-    # deprecated, do not use
-    description: "Deprecated. Please see block-workflow."
-    default: true
   block-workflow:
     type: boolean
     # this is false at COMMAND level as intention is to only block CURRENT job.
@@ -29,6 +24,10 @@ parameters:
     type: string
     default: "github"
     description: "Override VCS to 'bitbucket' if needed."
+  confidence:
+    type: string
+    default: "1"
+    description: "Due to scarce API, we need to requery the recent jobs list to ensure we're not just in a pending state for previous jobs.  This number indicates the threhold for API returning no previous pending jobs. Default is a single confirmation."
 steps:
   - run:
       name: Queue Until Front of Line
@@ -101,7 +100,7 @@ steps:
           load_current_workflow_values
 
           # falsey parameters are empty strings, so always compare against 'true' 
-          if [ "<<parameters.consider-job>>" != "true" ] || [ "<<parameters.block-workflow>>" = "true" ] ;then
+          if [ "<<parameters.block-workflow>>" = "true" ] ;then
             echo "Orb parameter block-workflow is true."
             echo "This job will block until no previous workflows have *any* jobs running."
             oldest_running_build_num=`jq 'sort_by(.workflows.created_at)| .[0].build_num' /tmp/augmented_jobstatus.json`
@@ -166,15 +165,23 @@ steps:
         #
         # Queue Loop
         #
+        confidence=0
         while true; do
           update_comparables
           echo "This Workflow Timestamp: $my_commit_time"
           echo "Oldest Workflow Timestamp: $oldest_commit_time"
           if [[ "$oldest_commit_time" > "$my_commit_time" ]] || [[ "$oldest_commit_time" = "$my_commit_time" ]] ; then
             # API returns Y-M-D HH:MM (with 24 hour clock) so alphabetical string compare is accurate to timestamp compare as well
-            # in event of race, everyone wins
-            echo "Front of the line, WooHoo!, Build continuing"
-            break
+            # recent-jobs API does not include pending, so it is posisble we queried in between a workfow transition, and we;re NOT really front of line.
+            if [ $confidence -lt <<parameters.confidence>> ];then
+              # To grow confidence, we check again with a delay.
+              confidence=$((confidence+1))
+              echo "API shows no previous jobs/workflows, but it is possible a previous workflow has pending jobs not yet visible in API."
+              echo "Rerunning check ${confidence}/<<parameters.confidence>>"
+            else
+              echo "Front of the line, WooHoo!, Build continuing"
+              break
+            fi
           else
             echo "This build (${CIRCLE_BUILD_NUM}) is queued, waiting for build number (${oldest_running_build_num}) to complete."
             echo "Total Queue time: ${wait_time} seconds."

src/jobs/block_workflow.yml

@@ -3,11 +3,6 @@ parameters:
     type: boolean
     default: true
     description: "Should we only consider jobs running on the same branch?"
-  consider-job:
-    type: boolean
-    # deprecated, do not use.
-    default: false
-    description: "Deprecated. Please see block-workflow."
   block-workflow:
     type: boolean
     # this is true at JOB level as intention is to block workflow
@@ -24,20 +19,24 @@ parameters:
   only-on-branch:
     type: string
     default: "*"
-    description: "Only queue on specified branch"
+    description: "Only queue on specified branch. Default is to enforce serialization on all branches."
   vcs-type:
     type: string
     default: "github"
     description: "Override VCS to 'bitbucket' if needed."
+  confidence:
+    type: string
+    default: "1"
+    description: "Due to scarce API, we need to requery the recent jobs list to ensure we're not just in a pending state for previous jobs.  This number indicates the threhold for API returning no previous pending jobs. Default is a single confirmation."
 
 docker:
-  - image: circleci/node:10 #its just really popular, and therefore fast (cached)
+  - image: cimg/base:stable
 resource_class: small
 steps:
   - until_front_of_line:
       consider-branch: <<parameters.consider-branch>>
-      consider-job: <<parameters.consider-job>>
       time: <<parameters.time>>
       dont-quit: <<parameters.dont-quit>>
       only-on-branch: <<parameters.only-on-branch>>
       vcs-type: <<parameters.vcs-type>>
+      confidence: <<parameters.confidence>>

test/bats_helper.bash

@@ -1,12 +1,19 @@
 #!/bin/bash
 
 
+function process_config_with {
+	append_project_configuration $1 > $INPUT_PROJECT_CONFIG
+ 	circleci config process $INPUT_PROJECT_CONFIG > ${PROCESSED_PROJECT_CONFIG}
+ 	yq read -j ${PROCESSED_PROJECT_CONFIG} > ${JSON_PROJECT_CONFIG}
+
+ 	#assertions use output, tests can override outptu to test additional commands beyond parsing.
+ 	output=`cat  ${PROCESSED_PROJECT_CONFIG}`
+}
+
 function append_project_configuration {
 	if [ -z "$BATS_IMPORT_DEV_ORB" ]; then
-		echo "#Using \`inline\` orb assembly, to test against published orb, set BATS_IMPORT_DEV_ORB to fully qualified path" >&3
 		assemble_inline $1
 	else
-		echo "#BATS_IMPORT_DEV_ORB env var is set, all config will be tested against imported orb $BATS_IMPORT_DEV_ORB" >&3
 		assemble_external $1
 	fi
 }
@@ -39,14 +46,6 @@ function assemble_external {
 	fi
 }
 
-function process_config_with {
-	append_project_configuration $1 > $INPUT_PROJECT_CONFIG
- 	circleci config process $INPUT_PROJECT_CONFIG > ${PROCESSED_PROJECT_CONFIG}
- 	yq read -j ${PROCESSED_PROJECT_CONFIG} > ${JSON_PROJECT_CONFIG}
-
- 	#assertions use output, tests can override outptu to test additional commands beyond parsing.
- 	output=`cat  ${PROCESSED_PROJECT_CONFIG}`
-}
 
 
 #

test/inputs/command-non-default.yml

@@ -8,5 +8,5 @@ jobs:
           # non-defaults for each paramter
           time: "1"
           consider-branch: false
-          consider-job: false
           dont-quit: true
+          block-workflow: true

test/test_expansion.bats

@@ -13,6 +13,13 @@ function setup {
 
   # the name used in example config files.
   INLINE_ORB_NAME="queue"
+
+
+  if [ -z "$BATS_IMPORT_DEV_ORB" ]; then
+    echo "#Using \`inline\` orb assembly, to test against published orb, set BATS_IMPORT_DEV_ORB to fully qualified path" >&3
+  else
+    echo "#BATS_IMPORT_DEV_ORB env var is set, all config will be tested against imported orb $BATS_IMPORT_DEV_ORB" >&3
+  fi
 }
 
 
@@ -44,6 +51,46 @@ function setup {
 }
 
 
+# See https://github.com/eddiewebb/circleci-queue/issues/26 for explanation of race condition
+@test "Race condition on previous workflow does not fool us" {
+  # given
+  process_config_with test/inputs/command-defaults.yml
+  export TESTING_MOCK_WORKFLOW_RESPONSES=test/api/workflows
+
+  # when
+  assert_jq_match '.jobs | length' 1 #only 1 job
+  assert_jq_match '.jobs["build"].steps | length' 1 #only 1 steps
+
+  jq -r '.jobs["build"].steps[0].run.command' $JSON_PROJECT_CONFIG > ${BATS_TMPDIR}/script-${BATS_TEST_NUMBER}.bash
+
+  export CIRCLECI_API_KEY="madethisup"
+  export CIRCLE_BUILD_NUM="2"
+  export CIRCLE_JOB="singlejob"
+  export CIRCLE_PROJECT_USERNAME="madethisup"
+  export CIRCLE_PROJECT_REPONAME="madethisup"
+  export CIRCLE_REPOSITORY_URL="madethisup"
+  export CIRCLE_BRANCH="madethisup"
+  export CIRCLE_PR_REPONAME=""
+
+  # set API Payload to temp location
+  export TESTING_MOCK_RESPONSE=/tmp/dynamic_response.json
+  # set initial response to mimic in-btween race condition, no running jobs
+  cp test/api/jobs/nopreviousjobs.json /tmp/dynamic_response.json
+  # in 11 seconds (> 10) switch to return the running job BACKGROUND PROCESS
+  (sleep 11 && cp test/api/jobs/onepreviousjobsamename.json /tmp/dynamic_response.json) &
+
+  run bash ${BATS_TMPDIR}/script-${BATS_TEST_NUMBER}.bash
+
+
+  assert_contains_text "Max Queue Time: 1 minutes"
+  assert_contains_text "Rerunning check 1/1" 
+  assert_contains_text "This build (${CIRCLE_BUILD_NUM}) is queued, waiting for build number (3) to complete."
+  assert_contains_text "Max wait time exceeded"
+  assert_contains_text "Cancelleing build 2"
+  [[ "$status" == "1" ]]
+}
+
+
 @test "Command: script will proceed with no previous jobs" {
   # given
   process_config_with test/inputs/command-defaults.yml
@@ -130,6 +177,7 @@ function setup {
   [[ "$status" == "1" ]]
 }
 
+
 @test "Command: script with dont-quit will not fail current job" {
   # given
   process_config_with test/inputs/command-non-default.yml
@@ -254,7 +302,7 @@ function setup {
 
 
 
-@test "Command: script will queue on different job when consider-job is false" {
+@test "Command: script will queue on different job when block-workflow is true" {
   # given
   process_config_with test/inputs/command-non-default.yml
   export TESTING_MOCK_RESPONSE=test/api/jobs/onepreviousjob-differentname.json