Merge pull request #53 from js-murph/main

feat: Concurrent file uploads
buildkite-plugins · Jul 3, 2023 · 8b7e6e2 · 8b7e6e2
2 parents 4ab17e0 + dbb5716
commit 8b7e6e2
Show file tree

Hide file tree

Showing 4 changed files with 146 additions and 11 deletions.
diff --git a/README.md b/README.md
@@ -90,6 +90,12 @@ Adds an annotation to the build run with a link to the uploaded report.
 
 Default value: `false`
 
+#### `upload-concurrency`(number)
+
+The number of concurrent file uploads to perform to the Buildkite analytics API.
+
+Default value: `1`
+
 ## Examples
 
 ### Upload a JUnit file

diff --git a/hooks/pre-exit b/hooks/pre-exit
@@ -6,8 +6,9 @@ FORMAT="${BUILDKITE_PLUGIN_TEST_COLLECTOR_FORMAT:-}"
 TIMEOUT="${BUILDKITE_PLUGIN_TEST_COLLECTOR_TIMEOUT:-30}"
 BASE_PATH="${BUILDKITE_PLUGIN_TEST_COLLECTOR_BASE_PATH:-.}"
 ANNOTATE="${BUILDKITE_PLUGIN_TEST_COLLECTOR_ANNOTATION_LINK:-false}"
+UPLOAD_CONCURRENCY="${BUILDKITE_PLUGIN_TEST_COLLECTOR_UPLOAD_CONCURRENCY:-1}"
 REPORT_URLS_FILE=$(mktemp)
-CURL_RESP_FILE="${CURL_RESP_FILE:-response.json}"
+CURL_RESP_FILE="${CURL_RESP_FILE:-}"
 DEBUG="false"
 
 if [[ "${BUILDKITE_PLUGIN_TEST_COLLECTOR_DEBUG:-}" =~ ^(true|on|1|always)$ ]]; then
@@ -94,6 +95,7 @@ save-report-url() {
 # Upload failures should not fail the build, and should have a sensible timeout,
 # so that Test Analytics availability doesn't affect build reliability.
 upload() {
+  local response_file="$4"
   local file="$3"
   local format="$2"
 
@@ -124,7 +126,7 @@ upload() {
   fi
 
   if [[ "$ANNOTATE" != "false" ]]; then
-    curl_args+=("-o" "${CURL_RESP_FILE}")
+    curl_args+=("-o" "${response_file}")
   fi
 
   curl_args+=("${BUILDKITE_PLUGIN_TEST_COLLECTOR_API_URL:-https://analytics-api.buildkite.com/v1/uploads}")
@@ -152,24 +154,69 @@ find_and_upload() {
   matching_files=()
   while IFS=$'' read -r matching_file ; do
     matching_files+=("$matching_file")
-  done < <("${FIND_CMD[@]}" "${BASE_PATH}" -path "${BASE_PATH}/${FILES_PATTERN}")
+  done < <("${FIND_CMD[@]}" "${BASE_PATH}" -path "${BASE_PATH}/${FILES_PATTERN}" | sort)
 
   if [[ "${#matching_files[@]}" -eq "0" ]]; then
     echo "No files found matching '${FILES_PATTERN}'"
     if [[ "${BUILDKITE_COMMAND_EXIT_STATUS:-0}" -eq "0" ]]; then
       exit "${BUILDKITE_PLUGIN_TEST_COLLECTOR_MISSING_ERROR:-1}"
     fi
   else
+    declare -a uploads_in_progress=()
+    echo "Uploading '${#matching_files[@]}' files matching '${FILES_PATTERN}'"
+
     # needs to be part of else for bash4.3 compatibility
     for file in "${matching_files[@]}"; do
-      echo "Uploading '$file'..."
-      if ! upload "$TOKEN_VALUE" "$FORMAT" "${file}"; then
-        echo "Error uploading, will continue"
-      fi
-      if [[ "$ANNOTATE" != "false" ]]; then
-        save-report-url "${CURL_RESP_FILE}"
-      fi
+      iterations_waited=0
+      while [[ "${#uploads_in_progress[@]}" -ge $UPLOAD_CONCURRENCY ]]; do
+        iterations_waited=$((iterations_waited + 1))
+        if [[ "${DEBUG}" == "true" ]]; then
+          echo "Waiting for uploads to finish..."
+        fi
+
+        sleep 1
+
+        for index in "${!uploads_in_progress[@]}"; do
+          # Note: kill -0 does not kill the pid, it provides a *nix compatible way to test the pid is responding.
+          if ! kill -0 "${uploads_in_progress[index]}" > /dev/null; then
+            unset 'uploads_in_progress[index]'
+          elif [[ "$iterations_waited" -gt $TIMEOUT ]]; then
+            echo "Upload '${uploads_in_progress[index]}' has been running for more than '${TIMEOUT}' seconds, killing it"
+            kill "${uploads_in_progress[index]}"
+            unset 'uploads_in_progress[index]'
+          fi
+        done
+      done
+
+      # Spawn a subcommand group to allow parallel uploads
+      {
+        echo "Uploading '$file'..."
+
+        if [[ -n "${CURL_RESP_FILE}" ]]; then
+          response_file="${CURL_RESP_FILE}"
+        else
+          response_file="$(mktemp -t 'response.XXXXXX')"
+        fi
+
+        if ! upload "$TOKEN_VALUE" "$FORMAT" "${file}" "${response_file}"; then
+          echo "Error uploading, will continue"
+        fi
+
+        if [[ "$ANNOTATE" != "false" ]]; then
+          save-report-url "${response_file}"
+        fi
+
+        if [[ "${DEBUG}" == "true" ]]; then
+          echo "Finished uploading '$file'"
+        fi
+      } &
+
+      # Store the PID of the upload
+      uploads_in_progress+=($!)
     done
+
+    # Wait for all uploads to finish
+    wait "${uploads_in_progress[@]}"
   fi
 }
 
@@ -194,4 +241,4 @@ else
 fi
 if [ "$ANNOTATE" != "false" ]; then
   annotation-link "${REPORT_URLS_FILE}"
-fi
+fi
diff --git a/plugin.yml b/plugin.yml
@@ -35,6 +35,8 @@ configuration:
       type: integer
     annotation-link:
       type: boolean
+    upload-concurrency:
+      type: integer
   required:
     - files
     - format

diff --git a/tests/pre-exit-success.bats b/tests/pre-exit-success.bats
@@ -57,6 +57,47 @@ COMMON_CURL_OPTIONS='--form \* --form \* --form \* --form \* --form \* --form \*
   assert_output --partial "curl success 3"
 }
 
+@test "Uploads multiple files concurrently does not break basic functionality" {
+   # would love to test functionality but can not do so due to limitations on bats-mock :(
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_FILES='**/*/junit-*.xml'
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_UPLOAD_CONCURRENCY='3'
+
+  skip "bats-mock does not currently support concurrency, so we can't test this reliably"
+
+  stub curl "-X POST --silent --show-error --max-time 30 --form format=junit ${COMMON_CURL_OPTIONS} \* -H \* : echo \"curl success \${10}\""
+
+  run "$PWD/hooks/pre-exit"
+
+  unstub curl
+
+  assert_success
+  assert_output --partial "Uploading './tests/fixtures/junit-1.xml'..."
+  assert_output --partial "Uploading './tests/fixtures/junit-2.xml'..."
+  assert_output --partial "Uploading './tests/fixtures/junit-3.xml'..."
+  assert_equal "$(echo "$output" | grep -c "curl success")" "3"
+}
+
+@test "Concurrency waits when the queue is full" {
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_FILES='**/*/junit-*.xml'
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_UPLOAD_CONCURRENCY='2'
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_DEBUG='true'
+
+  stub curl \
+    "-X POST --silent --show-error --max-time 30 --form format=junit ${COMMON_CURL_OPTIONS} --form \* \* -H \* : sleep 3; echo \"curl success \${10}\"" \
+    "-X POST --silent --show-error --max-time 30 --form format=junit ${COMMON_CURL_OPTIONS} --form \* \* -H \* : echo \"curl success \${10}\""
+
+  run "$PWD/hooks/pre-exit"
+
+  unstub curl
+
+  assert_success
+  assert_output --partial "Uploading './tests/fixtures/junit-1.xml'..."
+  assert_output --partial "Uploading './tests/fixtures/junit-2.xml'..."
+  assert_output --partial "Waiting for uploads to finish..."
+  assert_output --partial "Uploading './tests/fixtures/junit-3.xml'..."
+  assert_equal "$(echo "$output" | grep -c "curl success")" "3"
+}
+
 @test "Single file pattern through array" {
   export BUILDKITE_PLUGIN_TEST_COLLECTOR_FILES_0='**/*/junit-1.xml'
   unset BUILDKITE_PLUGIN_TEST_COLLECTOR_FILES
@@ -149,6 +190,25 @@ COMMON_CURL_OPTIONS='--form \* --form \* --form \* --form \* --form \* --form \*
   assert_output --partial "curl success"
 }
 
+@test "Concurrency gracefully handles command-group timeout" {
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_FILES='**/*/junit-*.xml'
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_UPLOAD_CONCURRENCY='2'
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_TIMEOUT='3'
+
+  stub curl "if [ \${10} != 'data=@\"./tests/fixtures/junit-3.xml\"' ]; then echo sleeping for \${10}; sleep 10 & wait \$!; else echo curl success \${10}; fi"
+
+  run "$PWD/hooks/pre-exit"
+
+  unstub curl
+
+  assert_success
+  assert_output --partial "Uploading './tests/fixtures/junit-1.xml'..."
+  assert_output --partial "Uploading './tests/fixtures/junit-2.xml'..."
+  assert_equal "$(echo "$output" | grep -c "has been running for more than")" "2"
+  assert_output --partial "Uploading './tests/fixtures/junit-3.xml'..."
+  assert_equal "$(echo "$output" | grep -c "curl success")" "1"
+}
+
 @test "Git available sends plugin version" {
   stub git "rev-parse --short HEAD : echo 'some-commit-id'"
   stub curl "-X POST --silent --show-error --max-time 30 --form format=junit ${COMMON_CURL_OPTIONS} --form \* \* -H \* : echo \"curl success with \${30}\""
@@ -234,3 +294,23 @@ COMMON_CURL_OPTIONS='--form \* --form \* --form \* --form \* --form \* --form \*
   assert_output --partial "Uploading './tests/fixtures/junit-1.xml'..."
   assert_output --partial "Error uploading, will continue"
 }
+
+@test "Concurrency gracefully handles failure" {
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_FILES='**/*/junit-*.xml'
+  export BUILDKITE_PLUGIN_TEST_COLLECTOR_UPLOAD_CONCURRENCY='2'
+
+  stub curl \
+    "-X POST --silent --show-error --max-time 30 --form format=junit ${COMMON_CURL_OPTIONS} \* -H \* : exit 10" \
+    "-X POST --silent --show-error --max-time 30 --form format=junit ${COMMON_CURL_OPTIONS} \* -H \* : echo 'curl success'"
+
+  run "$PWD/hooks/pre-exit"
+
+  unstub curl
+
+  assert_success
+  assert_output --partial "Uploading './tests/fixtures/junit-1.xml'..."
+  assert_output --partial "Uploading './tests/fixtures/junit-2.xml'..."
+  assert_equal "$(echo "$output" | grep -c "Error uploading, will continue")" "2"
+  assert_output --partial "Uploading './tests/fixtures/junit-3.xml'..."
+  assert_equal "$(echo "$output" | grep -c "curl success")" "1"
+}