-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[fix](ci) deduplicate comment-triggered TeamCity builds #66194
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: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -29,8 +29,54 @@ permissions: | |
| jobs: | ||
| check-comment-if-need-to-trigger-teamcity: | ||
|
|
||
| # This job only runs for pull request comments, and comment body contains 'run' | ||
| if: ${{ github.event.issue.pull_request && (contains(github.event.comment.body, 'run') || contains(github.event.comment.body, 'skip buildall') || contains(github.event.comment.body, 'skip check_coverage')) }} | ||
| # Keep this server-side filter aligned with the supported commands parsed below. | ||
| # A generic "run" match also catches performance reports such as "Total hot run time". | ||
| if: >- | ||
| ${{ | ||
| github.event.issue.pull_request && | ||
| ( | ||
| contains(github.event.comment.body, 'run buildall') || | ||
| contains(github.event.comment.body, 'run compile') || | ||
| contains(github.event.comment.body, 'run beut') || | ||
| contains(github.event.comment.body, 'run feut') || | ||
| contains(github.event.comment.body, 'run cloudut') || | ||
| contains(github.event.comment.body, 'run p0') || | ||
| contains(github.event.comment.body, 'run p1') || | ||
| contains(github.event.comment.body, 'run external') || | ||
| contains(github.event.comment.body, 'run cloud_p0') || | ||
| contains(github.event.comment.body, 'run cloud_p1') || | ||
| contains(github.event.comment.body, 'run vault_p0') || | ||
| contains(github.event.comment.body, 'run nonConcurrent') || | ||
| contains(github.event.comment.body, 'run check_coverage') || | ||
| contains(github.event.comment.body, 'run performance') || | ||
| contains(github.event.comment.body, 'skip buildall') || | ||
| contains(github.event.comment.body, 'skip check_coverage') | ||
| ) | ||
| }} | ||
|
|
||
| # Serialize duplicate commands for one PR. The next run then sees the build | ||
| # queued by the previous run and can safely skip an identical PR revision. | ||
|
Contributor
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. [P1] Coordinate The helper and GitHub lock cover only Compile even though this workflow skips the downstream steps because Compile triggers them. This fails in both directions: for PR 66196, Compile 1008441 finished while same-revision P0 1008463, External 1008464, Cloud P0 1008465, and NonConcurrent 1008467 were still running, so another |
||
| concurrency: | ||
| group: >- | ||
| comment-to-trigger-teamcity-${{ github.event.issue.number }}-${{ | ||
| contains(github.event.comment.body, 'run buildall') && 'buildall' || | ||
|
Contributor
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. [P1] Serialize mutations of the same concrete pipeline This key does not cover every run that executes the same check-then-trigger operation. |
||
| contains(github.event.comment.body, 'run compile') && 'compile' || | ||
| contains(github.event.comment.body, 'run beut') && 'beut' || | ||
| contains(github.event.comment.body, 'run feut') && 'feut' || | ||
| contains(github.event.comment.body, 'run cloudut') && 'cloudut' || | ||
| contains(github.event.comment.body, 'run p0') && 'p0' || | ||
| contains(github.event.comment.body, 'run p1') && 'p1' || | ||
| contains(github.event.comment.body, 'run external') && 'external' || | ||
| contains(github.event.comment.body, 'run cloud_p0') && 'cloud-p0' || | ||
| contains(github.event.comment.body, 'run cloud_p1') && 'cloud-p1' || | ||
| contains(github.event.comment.body, 'run vault_p0') && 'vault-p0' || | ||
| contains(github.event.comment.body, 'run nonConcurrent') && 'non-concurrent' || | ||
| contains(github.event.comment.body, 'run check_coverage') && 'check-coverage' || | ||
| contains(github.event.comment.body, 'run performance') && 'performance' || | ||
| contains(github.event.comment.body, 'skip buildall') && 'skip-buildall' || | ||
| 'skip-check-coverage' | ||
| }} | ||
| cancel-in-progress: false | ||
|
|
||
| runs-on: ubuntu-latest | ||
| env: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,6 +159,64 @@ get_queue_build_of_pr() { | |
| } | ||
| # get_queue_build_of_pr "$1" "$2" | ||
|
|
||
| get_active_builds_of_revision() { | ||
| # Return active build IDs for the same PR, pipeline, and revision. | ||
| # Return 0 when a duplicate exists, 1 when none exists, and 2 when lookup fails. | ||
| local PULL_REQUEST_NUM="${PULL_REQUEST_NUM:-$1}" | ||
| local COMMENT_TRIGGER_TYPE="${COMMENT_TRIGGER_TYPE:-$2}" | ||
| local COMMIT_ID_FROM_TRIGGER="${COMMIT_ID_FROM_TRIGGER:-$3}" | ||
| if [[ -z "${PULL_REQUEST_NUM}" || | ||
| -z "${COMMENT_TRIGGER_TYPE}" || | ||
| -z "${COMMIT_ID_FROM_TRIGGER}" ]]; then | ||
| echo "Usage: get_active_builds_of_revision PULL_REQUEST_NUM COMMENT_TRIGGER_TYPE COMMIT_ID_FROM_TRIGGER" >&2 | ||
| return 2 | ||
| fi | ||
|
|
||
| local PIPELINE="${comment_to_pipeline[${COMMENT_TRIGGER_TYPE}]}" | ||
| local teamcity_rest_url="http://43.132.222.7:8111/app/rest" | ||
| local queue_response | ||
| local running_response | ||
| if ! queue_response=$( | ||
| curl -sSf -X GET \ | ||
| -u OneMoreChance:OneMoreChance \ | ||
| -H "Accept: application/json" \ | ||
| "${teamcity_rest_url}/buildQueue?locator=buildType:(id:${PIPELINE})&fields=build(id,branchName,revisions(revision(version)))" | ||
|
Contributor
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. [P2] Follow TeamCity pagination when looking for duplicates TeamCity collection responses may be partial and advertise the next page through |
||
| ); then | ||
| echo "WARNING: failed to get queued builds for duplicate check" >&2 | ||
| return 2 | ||
| fi | ||
| if ! running_response=$( | ||
| curl -sSf -X GET \ | ||
| -u OneMoreChance:OneMoreChance \ | ||
| -H "Accept: application/json" \ | ||
| "${teamcity_rest_url}/builds?locator=buildType:(id:${PIPELINE}),branch:(name:pull/${PULL_REQUEST_NUM}),running:true&fields=build(id,branchName,revisions(revision(version)))" | ||
| ); then | ||
| echo "WARNING: failed to get running builds for duplicate check" >&2 | ||
| return 2 | ||
| fi | ||
|
|
||
| local build_ids | ||
| if ! build_ids=$( | ||
| printf '%s\n%s\n' "${queue_response}" "${running_response}" | | ||
| jq -s -r \ | ||
| --arg branch "pull/${PULL_REQUEST_NUM}" \ | ||
| --arg revision "${COMMIT_ID_FROM_TRIGGER}" \ | ||
| '.[] | .build[]? | | ||
|
Contributor
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. [P1] Include repeat count in the duplicate identity Repeat count changes the request: the parser preserves it and |
||
| select(.branchName == $branch and .revisions.revision[0].version == $revision) | | ||
| .id' | ||
| ); then | ||
| echo "WARNING: failed to parse active builds for duplicate check" >&2 | ||
| return 2 | ||
| fi | ||
|
|
||
| if [[ -n "${build_ids}" ]]; then | ||
| echo "${build_ids}" | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } | ||
| # get_active_builds_of_revision "$1" "$2" "$3" | ||
|
|
||
| cancel_running_build() { | ||
| local PULL_REQUEST_NUM="${PULL_REQUEST_NUM:-$1}" | ||
| local COMMENT_TRIGGER_TYPE="${COMMENT_TRIGGER_TYPE:-$2}" | ||
|
|
@@ -300,6 +358,21 @@ trigger_or_skip_build() { | |
| fi | ||
|
|
||
| if [[ "${FILE_CHANGED:-"true"}" == "true" ]]; then | ||
| local duplicate_build_ids | ||
| local duplicate_lookup_status=0 | ||
| duplicate_build_ids=$( | ||
| get_active_builds_of_revision \ | ||
| "${PULL_REQUEST_NUM}" \ | ||
| "${COMMENT_TRIGGER_TYPE}" \ | ||
| "${COMMIT_ID_FROM_TRIGGER}" | ||
| ) || duplicate_lookup_status=$? | ||
| if [[ ${duplicate_lookup_status} -eq 0 ]]; then | ||
| echo "INFO: active build(s) ${duplicate_build_ids//$'\n'/,} already exist for PR ${PULL_REQUEST_NUM}, pipeline ${COMMENT_TRIGGER_TYPE}, revision ${COMMIT_ID_FROM_TRIGGER}; skip duplicate trigger" | ||
| return 0 | ||
|
Contributor
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. [P1] Still clean up stale revisions before returning
|
||
| elif [[ ${duplicate_lookup_status} -ne 1 ]]; then | ||
| echo "WARNING: duplicate lookup failed for PR ${PULL_REQUEST_NUM}, pipeline ${COMMENT_TRIGGER_TYPE}; continue with the existing trigger flow" | ||
| fi | ||
|
|
||
| cancel_running_build "${PULL_REQUEST_NUM}" "${COMMENT_TRIGGER_TYPE}" | ||
| cancel_queue_build "${PULL_REQUEST_NUM}" "${COMMENT_TRIGGER_TYPE}" | ||
| trigger_build "${PULL_REQUEST_NUM}" "${COMMIT_ID_FROM_TRIGGER}" "${COMMENT_TRIGGER_TYPE}" "${COMMENT_REPEAT_TIMES}" | ||
|
|
||
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.
[P1] Use one canonical command grammar before entering concurrency
This admission test is not equivalent to the parser below: it examines the raw body with case-insensitive
contains, while lines 91/179 normalize whitespace and then shell-match case-sensitively. Thusrun p0(or a line break between the words) previously normalizes to validrun p0but is now skipped; converselyRUN P0is admitted and assigned thep0group, then the parser no-ops. Because the default concurrency queue keeps only one pending job, that no-op can replace a legitimate pendingp0request. Please make admission, grouping, and dispatch use the same canonical command and preserve distinct pending requests.