This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch backport-ci-sync-v3-4-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 78fd3734b2592dce83af10995ba632815b5b02fe Author: Jarek Potiuk <[email protected]> AuthorDate: Sun Oct 25 19:41:19 2026 +0100 Let the fork CI monitor gate pull requests and stop repeating Slack alerts (#75611) GitHub authorises convertPullRequestToDraft and markPullRequestReadyForReview against the contents permission, and the monitor's token only had read access. Every attempt to draft a gated pull request failed with "Resource not accessible by integration", and because drafting was the first mutation, the state comment, the Fork CI and CI passed statuses were never applied either: the gate did nothing for the only gated pull request so far. contents: write also allows pushing to the repository, so it must not reach the job that installs and runs Breeze and its dependencies. The draft state is advisory, the required statuses gate merging, so it can be changed by a separate job that runs nothing but gh and jq. The same failure was posted to Slack every 15 minutes. A persisting problem should be posted once, and again only if it goes away and comes back. Generated-by: Claude Opus 5 (cherry picked from commit 06771a97969594dc6833797cf5d787da690fe565) --- .github/workflows/fork-ci-monitor.yml | 170 +++++++++++++++++++-- .../doc/ci/08_fork_ci_for_external_contributors.md | 45 ++++-- .../doc/images/output_ci_fork-ci-reconcile.svg | 46 ++++-- .../doc/images/output_ci_fork-ci-reconcile.txt | 2 +- .../src/airflow_breeze/commands/ci_commands.py | 25 ++- .../airflow_breeze/commands/ci_commands_config.py | 2 + .../src/airflow_breeze/utils/fork_ci_github.py | 22 --- .../src/airflow_breeze/utils/fork_ci_reconcile.py | 90 ++++++++++- dev/breeze/tests/test_fork_ci_reconcile.py | 126 ++++++++++++++- 9 files changed, 451 insertions(+), 77 deletions(-) diff --git a/.github/workflows/fork-ci-monitor.yml b/.github/workflows/fork-ci-monitor.yml index e94f8fd0457..ec34b9615b0 100644 --- a/.github/workflows/fork-ci-monitor.yml +++ b/.github/workflows/fork-ci-monitor.yml @@ -31,15 +31,7 @@ on: # yamllint disable-line rule:truthy description: "Compute the changes without applying them to pull requests" type: boolean default: false -permissions: - # Listing the runs and jobs of the gate tampering check, and deleting the runs that only gated a - # pull request to fork CI; unlisted scopes are none. - actions: write - contents: read - # Labels, draft state, comments and closing of gated pull requests. - pull-requests: write - # The `Fork CI / Tests (AMD)` commit status on pull request head commits. - statuses: write +permissions: {} concurrency: group: fork-ci-monitor cancel-in-progress: false @@ -48,10 +40,32 @@ jobs: reconcile: name: "Reconcile pull requests gated to fork CI" if: github.repository == 'apache/airflow' + permissions: + # Listing the runs and jobs of the gate tampering check, and deleting the runs that only + # gated a pull request to fork CI. + actions: write + contents: read + # Labels, comments and closing of gated pull requests. + pull-requests: write + # The `Fork CI / Tests (AMD)` commit status on pull request head commits. + statuses: write + outputs: + draft-changes: ${{ steps.reconcile.outputs.draft-changes }} # The breeze action frees disk space with sudo and docker, which ubuntu-slim does not provide. runs-on: ubuntu-latest timeout-minutes: 20 steps: + # What earlier runs reported, so that a problem that persists is posted to Slack once + # rather than every 15 minutes. Losing it, for example to cache eviction, re-posts once. + # Restored before anything that can fail, so a failing setup is reported once too, and + # outside the checkout, where nothing but the steps below reads it. + - name: "Restore what earlier runs reported" + id: restore-reported + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ${{ runner.temp }}/fork-ci-monitor + key: fork-ci-monitor-reported-${{ github.run_id }}-${{ github.run_attempt }} + restore-keys: fork-ci-monitor-reported- - name: "Checkout main" uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -59,31 +73,163 @@ jobs: persist-credentials: false - name: "Install Breeze" uses: ./.github/actions/breeze + # The reconciler updates a working copy; it replaces the recorded one only once Slack has + # the message, so a failed post is retried by the next run instead of being suppressed. - name: "Reconcile" id: reconcile env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} DRY_RUN: ${{ inputs.dry-run && 'true' || 'false' }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - run: breeze ci fork-ci-reconcile --output-file slack-message.json + run: | + cp "${RUNNER_TEMP}/fork-ci-monitor/reported.json" reported.json 2>/dev/null || true + breeze ci fork-ci-reconcile --output-file slack-message.json --reported-file reported.json \ + --draft-changes-file draft-changes.json + echo "draft-changes=$(jq -c . draft-changes.json)" >> "${GITHUB_OUTPUT}" # A dry run reports what it would have done, which must not read as done on Slack. - name: "Post to Slack" + id: post if: steps.reconcile.outputs.has-alerts == 'true' && !inputs.dry-run uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 with: method: chat.postMessage token: ${{ secrets.SLACK_BOT_TOKEN }} + # Fail the step on a rejected post, so it is not recorded as reported. + errors: true payload-file-path: "slack-message.json" + - name: "Record what this run reported" + if: steps.reconcile.outcome == 'success' && steps.post.outcome != 'failure' && !inputs.dry-run + run: | + mkdir -p "${RUNNER_TEMP}/fork-ci-monitor" + cp reported.json "${RUNNER_TEMP}/fork-ci-monitor/reported.json" + rm -f "${RUNNER_TEMP}/fork-ci-monitor/failed" # A run that fails before writing its message, for example on a discovery query or a rate - # limit it cannot wait out, would otherwise stay silent. - - name: "Report a failed run to Slack" + # limit it cannot wait out, would otherwise stay silent. Only the first of consecutive failed + # runs is reported. + - name: "Check whether the previous run's failure was reported" + id: previous-failure if: failure() && !inputs.dry-run + run: | + if [[ -f "${RUNNER_TEMP}/fork-ci-monitor/failed" ]]; then + echo "reported=true" >> "${GITHUB_OUTPUT}" + else + echo "reported=false" >> "${GITHUB_OUTPUT}" + fi + - name: "Report a failed run to Slack" + id: report-failure + if: failure() && !inputs.dry-run && steps.previous-failure.outputs.reported == 'false' uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 with: method: chat.postMessage token: ${{ secrets.SLACK_BOT_TOKEN }} + errors: true payload: | channel: "internal-airflow-ci-cd" text: >- Fork CI monitor run failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + - name: "Record the reported failure" + if: failure() && steps.report-failure.outcome == 'success' + run: mkdir -p "${RUNNER_TEMP}/fork-ci-monitor" && touch "${RUNNER_TEMP}/fork-ci-monitor/failed" + # A dry run posts nothing, so what it saw must not suppress the next run's message. + - name: "Save what was reported" + if: always() && !inputs.dry-run && steps.restore-reported.outcome == 'success' + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ${{ runner.temp }}/fork-ci-monitor + key: fork-ci-monitor-reported-${{ github.run_id }}-${{ github.run_attempt }} + + # GitHub authorises the convertPullRequestToDraft and markPullRequestReadyForReview mutations + # against `contents`, not `pull-requests`: with `contents: read` they fail with "Resource not + # accessible by integration". `contents: write` would also let the token push to the + # repository, so it is kept out of the reconcile job, which installs and runs Breeze and its + # dependencies. This job runs no code but `gh` and `jq` on the reconciler's list. + draft-state: + name: "Change the draft state of pull requests gated to fork CI" + needs: reconcile + # Runs whenever the reconciler wrote its list, even when only its Slack post failed, and with + # an empty list too, so that a failure set that cleared is recorded as cleared. + if: >- + !cancelled() && !inputs.dry-run && github.repository == 'apache/airflow' + && needs.reconcile.outputs.draft-changes != '' + permissions: + contents: write + # The draft state, and removing `fork ci required` from a released pull request once it + # is undrafted. + pull-requests: write + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + # The pull requests whose change failed last time, so a persisting failure is posted to + # Slack once, as in the reconcile job. + - name: "Restore the previously failed changes" + id: restore-failed + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ${{ runner.temp }}/fork-ci-draft-state + key: fork-ci-monitor-draft-state-${{ github.run_id }}-${{ github.run_attempt }} + restore-keys: fork-ci-monitor-draft-state- + - name: "Change the draft state" + id: change + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + DRAFT_CHANGES: ${{ needs.reconcile.outputs.draft-changes }} + run: | + failed=() + while IFS=$'\t' read -r number node_id draft release; do + # This token can push to the repository; act only on well-formed entries. + if [[ ! "${number}" =~ ^[0-9]+$ || ! "${node_id}" =~ ^PR_[A-Za-z0-9_-]+$ \ + || ! "${draft}" =~ ^(true|false)$ || ! "${release}" =~ ^(true|false)$ ]]; then + echo "::error::Skipped a malformed draft change" + failed+=("malformed") + continue + fi + if [[ "${draft}" == "true" ]]; then + mutation="convertPullRequestToDraft" + else + mutation="markPullRequestReadyForReview" + fi + echo "#${number}: ${mutation}" + if ! gh api graphql -f id="${node_id}" -f query="mutation(\$id: ID!) { + ${mutation}(input: {pullRequestId: \$id}) { clientMutationId } }" > /dev/null; then + failed+=("${number}") + continue + fi + # The label brings the pull request back to the reconciler, which retries the + # undraft while it is there. + label="repos/${REPO}/issues/${number}/labels/fork%20ci%20required" + if [[ "${release}" == "true" ]] && ! gh api -X DELETE "${label}" > /dev/null; then + failed+=("${number}") + fi + done < <(jq -r '.[] | [.number, .node_id, .draft, .release] | @tsv' <<< "${DRAFT_CHANGES}") + mkdir -p "${RUNNER_TEMP}/fork-ci-draft-state" + printf '%s\n' "${failed[@]}" | sort -n | sed '/^$/d' > failed.txt + previous="${RUNNER_TEMP}/fork-ci-draft-state/failed.txt" + if [[ -s failed.txt ]] && ! cmp -s failed.txt "${previous}"; then + echo "new-failures=true" >> "${GITHUB_OUTPUT}" + fi + echo "failed=$(paste -sd, failed.txt)" >> "${GITHUB_OUTPUT}" + - name: "Post failed changes to Slack" + id: post + if: steps.change.outputs.new-failures == 'true' + uses: slackapi/slack-github-action@dcb1066f776dd043e64d0e8ba94ca15cc7e1875d # v4.0.0 + with: + method: chat.postMessage + token: ${{ secrets.SLACK_BOT_TOKEN }} + errors: true + payload: | + channel: "internal-airflow-ci-cd" + text: >- + Fork CI monitor could not change the draft state of pull requests + ${{ steps.change.outputs.failed }}: + ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + - name: "Record the failed changes" + if: steps.change.outcome == 'success' && steps.post.outcome != 'failure' + run: cp failed.txt "${RUNNER_TEMP}/fork-ci-draft-state/failed.txt" + - name: "Save the failed changes" + if: always() && steps.restore-failed.outcome == 'success' + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ${{ runner.temp }}/fork-ci-draft-state + key: fork-ci-monitor-draft-state-${{ github.run_id }}-${{ github.run_attempt }} diff --git a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md index effe96065e7..1b2bd742f90 100644 --- a/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md +++ b/dev/breeze/doc/ci/08_fork_ci_for_external_contributors.md @@ -727,6 +727,14 @@ over 3000 characters or more than 50 blocks, so items are split across sections, shortened to 500 characters, and sections past the 50th are replaced by a pointer to the run log, which always lists every item in full. +Each item is posted once while it persists. The run records its items in a file +(`--reported-file`) that the workflow carries to the next run through the Actions cache, and the +message holds only the items the previous run did not report. The workflow keeps the new record +only once Slack accepted the message, so a failed post is retried by the next run. An item that +goes away and comes back is posted again; a lost cache entry re-posts everything once. The +message for a run that failed before writing its message is posted for the first of consecutive +failed runs only. A dry run saves nothing, so it never suppresses the next message. + Attention items are expected outcomes that a maintainer should still see: a pull request closed for gate tampering, and a pull request carrying both override labels. They are listed in their own section of the message so they are not mistaken for reconciler failures. @@ -737,7 +745,7 @@ Unknown means any of: - A check suite for `Tests (AMD)`, in the fork or in `apache/airflow`, with a status or conclusion outside the modelled set. - A workflow state outside `active` / `disabled_fork` / `disabled_manually` / not found. -- A mutation that returned an error (label, draft, undraft, comment, close, status). +- A mutation that returned an error (label, comment, close, status). - A head repository that no longer exists. - A state comment whose marker cannot be parsed. @@ -761,18 +769,35 @@ budget stays observable. `.github/workflows/fork-ci-monitor.yml`: - `on: schedule: cron '*/15 * * * *'` and `workflow_dispatch` with a `dry-run` input. -- `permissions: actions: write, contents: read, pull-requests: write, statuses: write` with a - justification comment. Comments and labels on pull requests are covered by - `pull-requests: write`; the commit status needs `statuses: write`; the gate tampering check - lists runs and their jobs, and the reconciler deletes gated runs, which needs `actions: write` - because every scope a `permissions` block leaves out is `none`. +- Two jobs, each with its own `permissions` block and a justification comment; the workflow + grants nothing by default. +- The `reconcile` job: `actions: write, contents: read, pull-requests: write, statuses: write`. + Comments and labels on pull requests are covered by `pull-requests: write`; the commit status + needs `statuses: write`; the gate tampering check lists runs and their jobs, and the + reconciler deletes gated runs, which needs `actions: write` because every scope a + `permissions` block leaves out is `none`. +- The `draft-state` job: `contents: write, pull-requests: write`. GitHub authorises the draft + and undraft mutations (`convertPullRequestToDraft`, `markPullRequestReadyForReview`) against + `contents: write` and refuses them with `contents: read` ("Resource not accessible by + integration"). That scope also allows pushing to the repository, so the reconciler, which + installs and runs Breeze and its dependencies, does not get it: it writes the draft changes + as a job output (`--draft-changes-file`), and this job applies them with `gh` and `jq` only, + without a checkout. On a release that undrafts, it also removes `fork ci required` after + the undraft, so a failed undraft keeps the label and the next run retries it. Pull requests + whose change failed are posted to Slack once while the same set keeps failing. It validates + every entry before acting on it, runs even when only the reconciler's Slack post failed, and + runs with an empty list too, so a failure set that cleared is recorded as cleared. It is + skipped on a dry run. The draft state is advisory: the commit statuses + keep a pull request from merging, so a failed draft change does not weaken the gate. - `runs-on: ubuntu-latest` (the breeze setup action frees disk space with `sudo` and `docker`, which `ubuntu-slim` does not provide), `timeout-minutes: 20`, `concurrency: fork-ci-monitor` with `cancel-in-progress: false` so runs never overlap. -- Steps: checkout `main` with `persist-credentials: false`, `./.github/actions/breeze`, - `breeze ci fork-ci-reconcile` with `GH_TOKEN`, then the Slack post step guarded by - `has-alerts` and skipped on a dry run, whose alerts describe changes it did not make, and a - second Slack step that reports a run failing before it could write its message. +- Steps: restoring the reported items from the Actions cache into `$RUNNER_TEMP`, checkout + `main` with `persist-credentials: false`, `./.github/actions/breeze`, `breeze ci fork-ci-reconcile` with `GH_TOKEN`, + then the Slack post step guarded by `has-alerts` and skipped on a dry run, whose alerts + describe changes it did not make, a second Slack step that reports the first of consecutive + runs failing before they could write their message, and saving what Slack accepted to the + cache. - Every `gh` call times out after 120 seconds, and a rate limit is retried with backoff up to four attempts, without waiting after the last one. diff --git a/dev/breeze/doc/images/output_ci_fork-ci-reconcile.svg b/dev/breeze/doc/images/output_ci_fork-ci-reconcile.svg index 1d374d7a871..d57c68e0978 100644 --- a/dev/breeze/doc/images/output_ci_fork-ci-reconcile.svg +++ b/dev/breeze/doc/images/output_ci_fork-ci-reconcile.svg @@ -1,4 +1,4 @@ -<svg class="rich-terminal" viewBox="0 0 1482 489.2" xmlns="http://www.w3.org/2000/svg";> +<svg class="rich-terminal" viewBox="0 0 1482 586.8" xmlns="http://www.w3.org/2000/svg";> <!-- Generated with Rich https://www.textualize.io --> <style> @@ -43,7 +43,7 @@ <defs> <clipPath id="breeze-ci-fork-ci-reconcile-clip-terminal"> - <rect x="0" y="0" width="1463.0" height="438.2" /> + <rect x="0" y="0" width="1463.0" height="535.8" /> </clipPath> <clipPath id="breeze-ci-fork-ci-reconcile-line-0"> <rect x="0" y="1.5" width="1464" height="24.65"/> @@ -96,9 +96,21 @@ <clipPath id="breeze-ci-fork-ci-reconcile-line-16"> <rect x="0" y="391.9" width="1464" height="24.65"/> </clipPath> +<clipPath id="breeze-ci-fork-ci-reconcile-line-17"> + <rect x="0" y="416.3" width="1464" height="24.65"/> + </clipPath> +<clipPath id="breeze-ci-fork-ci-reconcile-line-18"> + <rect x="0" y="440.7" width="1464" height="24.65"/> + </clipPath> +<clipPath id="breeze-ci-fork-ci-reconcile-line-19"> + <rect x="0" y="465.1" width="1464" height="24.65"/> + </clipPath> +<clipPath id="breeze-ci-fork-ci-reconcile-line-20"> + <rect x="0" y="489.5" width="1464" height="24.65"/> + </clipPath> </defs> - <rect fill="#292929" stroke="rgba(255,255,255,0.35)" stroke-width="1" x="1" y="1" width="1480" height="487.2" rx="8"/><text class="breeze-ci-fork-ci-reconcile-title" fill="#c5c8c6" text-anchor="middle" x="740" y="27">Command: ci fork-ci-reconcile</text> + <rect fill="#292929" stroke="rgba(255,255,255,0.35)" stroke-width="1" x="1" y="1" width="1480" height="584.8" rx="8"/><text class="breeze-ci-fork-ci-reconcile-title" fill="#c5c8c6" text-anchor="middle" x="740" y="27">Command: ci fork-ci-reconcile</text> <g transform="translate(26,22)"> <circle cx="0" cy="0" r="7" fill="#ff5f57"/> <circle cx="22" cy="0" r="7" fill="#febc2e"/> @@ -114,18 +126,22 @@ </text><text class="breeze-ci-fork-ci-reconcile-r1" x="12.2" y="93.2" textLength="1073.6" clip-path="url(#breeze-ci-fork-ci-reconcile-line-3)">Reconcile pull requests whose CI runs in contributor forks (AIP-120). Run on a schedule.</text><text class="breeze-ci-fork-ci-reconcile-r1" x="1464" y="93.2" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-3)"> </text><text class="breeze-ci-fork-ci-reconcile-r1" x="1464" y="117.6" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-4)"> </text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="142" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-5)">╭─</text><text class="breeze-ci-fork-ci-reconcile-r5" x="24.4" y="142" textLength="305" clip-path="url(#breeze-ci-fork-ci-reconcile-line-5)"> Fork CI reconcile flags </text><text class="breeze-ci-fork-ci-reconcile-r5" x="329.4" y="142" textLength="1110.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-5)">───────────────────── [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="166.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-6)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="166.4" textLength="231.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-6)">--config           </text><text class="breeze-ci-fork-ci-reconcile-r1" x="329.4" y="166.4" textLength="1110.2" clip-path="url(#breeze-ci-fork-ci-reconcile [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="190.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-7)">│</text><text class="breeze-ci-fork-ci-reconcile-r6" x="329.4" y="190.8" textLength="73.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-7)">(FILE)</text><text class="breeze-ci-fork-ci-reconcile-r5" x="1451.8" y="190.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-7)">│</text><text class="breeze-ci-fork-ci-reconcile-r1" x="146 [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="215.2" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-8)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="215.2" textLength="231.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-8)">--output-file      </text><text class="breeze-ci-fork-ci-reconcile-r1" x="329.4" y="215.2" textLength="890.6" clip-path="url(#breeze-ci-fork-ci-reconcile-line-8)">Where the&# [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="239.6" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-9)">│</text><text class="breeze-ci-fork-ci-reconcile-r5" x="329.4" y="239.6" textLength="231.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-9)">slack-message.json]</text><text class="breeze-ci-fork-ci-reconcile-r6" x="573.4" y="239.6" textLength="73.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-9)">(FILE)</text><text class="breeze-ci-fork-ci-re [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="264" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-10)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="264" textLength="231.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-10)">--run-url          </text><text class="breeze-ci-fork-ci-reconcile-r1" x="329.4" y="264" textLength="683.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-10)" [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="288.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-11)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="288.4" textLength="231.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-11)">--github-repository</text><text class="breeze-ci-fork-ci-reconcile-r7" x="280.6" y="288.4" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-11)">-g</text><text class="breeze-ci-fork-ci-reco [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="312.8" textLength="1464" clip-path="url(#breeze-ci-fork-ci-reconcile-line-12)">╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯</text><text class="breeze-ci-fork-ci-reconcile-r1" x="1464" y="312.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-12)"> -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="337.2" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-13)">╭─</text><text class="breeze-ci-fork-ci-reconcile-r5" x="24.4" y="337.2" textLength="195.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-13)"> Common options </text><text class="breeze-ci-fork-ci-reconcile-r5" x="219.6" y="337.2" textLength="1220" clip-path="url(#breeze-ci-fork-ci-reconcile-line-13)">─────────────────────────────── [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="361.6" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-14)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="361.6" textLength="109.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-14)">--verbose</text><text class="breeze-ci-fork-ci-reconcile-r7" x="158.6" y="361.6" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-14)">-v</text><text class="breeze-ci-fork-ci-reconcile-r1" [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="386" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-15)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="386" textLength="109.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-15)">--dry-run</text><text class="breeze-ci-fork-ci-reconcile-r7" x="158.6" y="386" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-15)">-D</text><text class="breeze-ci-fork-ci-reconcile-r1" x="207 [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="410.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-16)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="410.4" textLength="109.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-16)">--help   </text><text class="breeze-ci-fork-ci-reconcile-r7" x="158.6" y="410.4" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-16)">-h</text><text class="breeze-ci-fork-ci [...] -</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="434.8" textLength="1464" clip-path="url(#breeze-ci-fork-ci-reconcile-line-17)">╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯</text><text class="breeze-ci-fork-ci-reconcile-r1" x="1464" y="434.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-17)"> +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="166.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-6)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="166.4" textLength="244" clip-path="url(#breeze-ci-fork-ci-reconcile-line-6)">--config            </text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="166.4" textLength="1098" clip-path="url(#breeze-ci-fork-ci-reconci [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="190.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-7)">│</text><text class="breeze-ci-fork-ci-reconcile-r6" x="341.6" y="190.8" textLength="73.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-7)">(FILE)</text><text class="breeze-ci-fork-ci-reconcile-r5" x="1451.8" y="190.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-7)">│</text><text class="breeze-ci-fork-ci-reconcile-r1" x="146 [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="215.2" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-8)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="215.2" textLength="244" clip-path="url(#breeze-ci-fork-ci-reconcile-line-8)">--output-file       </text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="215.2" textLength="890.6" clip-path="url(#breeze-ci-fork-ci-reconcile-line-8)">Where t [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="239.6" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-9)">│</text><text class="breeze-ci-fork-ci-reconcile-r5" x="341.6" y="239.6" textLength="231.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-9)">slack-message.json]</text><text class="breeze-ci-fork-ci-reconcile-r6" x="585.6" y="239.6" textLength="73.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-9)">(FILE)</text><text class="breeze-ci-fork-ci-re [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="264" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-10)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="264" textLength="244" clip-path="url(#breeze-ci-fork-ci-reconcile-line-10)">--reported-file     </text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="264" textLength="1098" clip-path="url(#breeze-ci-fork-ci-reconcile-line-10)">The items the [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="288.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-11)">│</text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="288.4" textLength="744.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-11)">run's items. Without it every item is reported on every run. </text><text class="breeze-ci-fork-ci-reconcile-r6" x="1085.8" y="288.4" textLength=" [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="312.8" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-12)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="312.8" textLength="244" clip-path="url(#breeze-ci-fork-ci-reconcile-line-12)">--draft-changes-file</text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="312.8" textLength="1098" clip-path="url(#breeze-ci-fork-ci-reconcile-line-12)">Where the draft state cha [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="337.2" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-13)">│</text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="337.2" textLength="890.6" clip-path="url(#breeze-ci-fork-ci-reconcile-line-13)">permission the reconciler does not get. Without it they are only logged. </text><text class="breeze-ci-fork-ci-reconcile-r6" x="1232.2" y="337.2" [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="361.6" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-14)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="361.6" textLength="244" clip-path="url(#breeze-ci-fork-ci-reconcile-line-14)">--run-url           </text><text class="breeze-ci-fork-ci-reconcile-r1" x="341.6" y="361.6" textLength="683.2" clip-path="url(#breeze-ci-fork-ci-reconcile [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="386" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-15)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="386" textLength="244" clip-path="url(#breeze-ci-fork-ci-reconcile-line-15)">--github-repository </text><text class="breeze-ci-fork-ci-reconcile-r7" x="292.8" y="386" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-15)">-g</text><text class="breeze-ci-fork-ci-reconc [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="410.4" textLength="1464" clip-path="url(#breeze-ci-fork-ci-reconcile-line-16)">╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯</text><text class="breeze-ci-fork-ci-reconcile-r1" x="1464" y="410.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-16)"> +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="434.8" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-17)">╭─</text><text class="breeze-ci-fork-ci-reconcile-r5" x="24.4" y="434.8" textLength="195.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-17)"> Common options </text><text class="breeze-ci-fork-ci-reconcile-r5" x="219.6" y="434.8" textLength="1220" clip-path="url(#breeze-ci-fork-ci-reconcile-line-17)">─────────────────────────────── [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="459.2" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-18)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="459.2" textLength="109.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-18)">--verbose</text><text class="breeze-ci-fork-ci-reconcile-r7" x="158.6" y="459.2" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-18)">-v</text><text class="breeze-ci-fork-ci-reconcile-r1" [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="483.6" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-19)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="483.6" textLength="109.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-19)">--dry-run</text><text class="breeze-ci-fork-ci-reconcile-r7" x="158.6" y="483.6" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-19)">-D</text><text class="breeze-ci-fork-ci-reconcile-r1" [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="508" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-20)">│</text><text class="breeze-ci-fork-ci-reconcile-r4" x="24.4" y="508" textLength="109.8" clip-path="url(#breeze-ci-fork-ci-reconcile-line-20)">--help   </text><text class="breeze-ci-fork-ci-reconcile-r7" x="158.6" y="508" textLength="24.4" clip-path="url(#breeze-ci-fork-ci-reconcile-line-20)">-h</text><text class="breeze-ci-fork-ci-recon [...] +</text><text class="breeze-ci-fork-ci-reconcile-r5" x="0" y="532.4" textLength="1464" clip-path="url(#breeze-ci-fork-ci-reconcile-line-21)">╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯</text><text class="breeze-ci-fork-ci-reconcile-r1" x="1464" y="532.4" textLength="12.2" clip-path="url(#breeze-ci-fork-ci-reconcile-line-21)"> </text> </g> </g> diff --git a/dev/breeze/doc/images/output_ci_fork-ci-reconcile.txt b/dev/breeze/doc/images/output_ci_fork-ci-reconcile.txt index d57a8fbe16b..a439cd53386 100644 --- a/dev/breeze/doc/images/output_ci_fork-ci-reconcile.txt +++ b/dev/breeze/doc/images/output_ci_fork-ci-reconcile.txt @@ -1 +1 @@ -5ce2992bb7c33fd4d23d854f2bfa1cef +78f39cbb63bd960f9da120ee39eeffd9 diff --git a/dev/breeze/src/airflow_breeze/commands/ci_commands.py b/dev/breeze/src/airflow_breeze/commands/ci_commands.py index 0dbc5027942..1f173e5b588 100644 --- a/dev/breeze/src/airflow_breeze/commands/ci_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/ci_commands.py @@ -1694,6 +1694,20 @@ def set_milestone( envvar="OUTPUT_FILE", help="Where the Slack message is written when there are items for maintainers.", ) [email protected]( + "--reported-file", + type=click.Path(dir_okay=False, path_type=Path), + envvar="REPORTED_FILE", + help="The items the previous run reported, left out of the Slack message, and replaced by " + "this run's items. Without it every item is reported on every run.", +) [email protected]( + "--draft-changes-file", + type=click.Path(dir_okay=False, path_type=Path), + envvar="DRAFT_CHANGES_FILE", + help="Where the draft state changes are written, as JSON, for the job that applies them with " + "the permission the reconciler does not get. Without it they are only logged.", +) @click.option( "--run-url", default="", @@ -1703,7 +1717,14 @@ def set_milestone( @option_github_repository @option_verbose @option_dry_run -def fork_ci_reconcile(config_path: Path | None, output_file: Path, run_url: str, github_repository: str): +def fork_ci_reconcile( + config_path: Path | None, + output_file: Path, + reported_file: Path | None, + draft_changes_file: Path | None, + run_url: str, + github_repository: str, +): from datetime import UTC, datetime from airflow_breeze.utils.fork_ci_gate import load_config @@ -1719,6 +1740,8 @@ def fork_ci_reconcile(config_path: Path | None, output_file: Path, run_url: str, run_url=run_url, output_file=output_file, workflows_dir=AIRFLOW_ROOT_PATH / ".github" / "workflows", + reported_file=reported_file, + draft_changes_file=draft_changes_file, ) github_output = os.environ.get("GITHUB_OUTPUT") if github_output: diff --git a/dev/breeze/src/airflow_breeze/commands/ci_commands_config.py b/dev/breeze/src/airflow_breeze/commands/ci_commands_config.py index cc99bcf1c14..99042fc78a3 100644 --- a/dev/breeze/src/airflow_breeze/commands/ci_commands_config.py +++ b/dev/breeze/src/airflow_breeze/commands/ci_commands_config.py @@ -125,6 +125,8 @@ CI_PARAMETERS: dict[str, list[dict[str, str | list[str]]]] = { "options": [ "--config", "--output-file", + "--reported-file", + "--draft-changes-file", "--run-url", "--github-repository", ], diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py b/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py index 73501627e77..73a44503acb 100644 --- a/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py +++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_github.py @@ -144,28 +144,6 @@ class GitHubClient: None, ) - def convert_to_draft(self, pr: int, node_id: str) -> None: - self._mutate( - f"#{pr}: convert to draft", - ["api", "graphql"], - { - "query": "mutation($id: ID!) { convertPullRequestToDraft(input: {pullRequestId: $id}) " - "{ clientMutationId } }", - "variables": {"id": node_id}, - }, - ) - - def mark_ready(self, pr: int, node_id: str) -> None: - self._mutate( - f"#{pr}: mark ready for review", - ["api", "graphql"], - { - "query": "mutation($id: ID!) { markPullRequestReadyForReview(input: {pullRequestId: $id}) " - "{ clientMutationId } }", - "variables": {"id": node_id}, - }, - ) - def create_comment(self, pr: int, body: str) -> None: self._mutate( f"#{pr}: create comment", diff --git a/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py b/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py index 61b44ff7899..41ffaab6f26 100644 --- a/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py +++ b/dev/breeze/src/airflow_breeze/utils/fork_ci_reconcile.py @@ -28,7 +28,7 @@ import base64 import functools import json from collections.abc import Callable, Iterable, Sequence -from dataclasses import dataclass, field, replace +from dataclasses import asdict, dataclass, field, replace from datetime import datetime, timedelta from enum import Enum from pathlib import Path @@ -1179,9 +1179,29 @@ def count_commits_behind(client: GitHubClient, repo: str, pr: PullRequestSnapsho return compare.get("behind_by") -def apply_plan(client: GitHubClient, pr: PullRequestSnapshot, plan: Plan) -> None: - if plan.draft is True: - client.convert_to_draft(pr.number, pr.node_id) +@dataclass(frozen=True) +class DraftChange: + """A draft state change, applied by the monitor's draft-state job rather than the reconciler.""" + + number: int + node_id: str + draft: bool + # Remove `fork ci required` once the pull request is undrafted. + release: bool = False + + +def apply_plan(client: GitHubClient, pr: PullRequestSnapshot, plan: Plan) -> DraftChange | None: + """ + Apply the plan, except for the draft state change, which is returned. + + GitHub authorises draft changes against `contents: write`, which the reconciler, running + Breeze and its dependencies, does not get; a separate job with no other code applies them. + """ + draft_change = None + if plan.draft is not None: + verb = "convert to draft" if plan.draft else "mark ready for review" + console_print(f"[info]#{pr.number}: {verb} (left to the draft-state job)[/]") + draft_change = DraftChange(pr.number, pr.node_id, plan.draft) if plan.add_labels: client.add_labels(pr.number, plan.add_labels) for label in plan.remove_labels: @@ -1198,14 +1218,17 @@ def apply_plan(client: GitHubClient, pr: PullRequestSnapshot, plan: Plan) -> Non client.set_status(pr.number, pr.head_sha, *plan.ci_passed, context=CI_PASSED_CONTEXT) for comment in plan.new_comments: client.create_comment(pr.number, comment) - if plan.draft is False: - client.mark_ready(pr.number, pr.node_id) if plan.close: client.close(pr.number) # Last, because the label is what brings the pull request back to the next run: when an # earlier mutation fails, the next run retries the release instead of losing track of it. + # A release that undrafts leaves it to the draft-state job, for the same reason. if LABEL_FORK_CI_REQUIRED in plan.remove_labels: - client.remove_label(pr.number, LABEL_FORK_CI_REQUIRED) + if draft_change is not None and not draft_change.draft: + draft_change = replace(draft_change, release=True) + else: + client.remove_label(pr.number, LABEL_FORK_CI_REQUIRED) + return draft_change def _escape_slack(text: str) -> str: @@ -1255,6 +1278,41 @@ def build_slack_payload( } +def drop_reported( + reported_file: Path, + *, + unknown: list[tuple[int, str]], + attention: list[tuple[int, str]], + notes: list[str], + record: bool = True, +) -> tuple[list[tuple[int, str]], list[tuple[int, str]], list[str]]: + """ + Leave out the items the previous run already reported, and record this run's items. + + Only the previous run's items are kept, so an item that went away and comes back is posted + again. A missing or unreadable file reports everything. + """ + current: dict[str, list[Any]] = { + "unknown": [list(item) for item in unknown], + "attention": [list(item) for item in attention], + "notes": notes, + } + try: + previous = json.loads(reported_file.read_text()) + except (OSError, ValueError): + previous = {} + if not isinstance(previous, dict): + previous = {} + if record: + reported_file.write_text(json.dumps(current, indent=2)) + + def drop(key: str, items: list[Any]) -> list[Any]: + seen = previous.get(key) or [] + return [item for item, stored in zip(items, current[key]) if stored not in seen] + + return drop("unknown", unknown), drop("attention", attention), drop("notes", notes) + + def run_reconcile( client: GitHubClient, config: GateConfig, @@ -1263,6 +1321,8 @@ def run_reconcile( run_url: str, output_file: Path, workflows_dir: Path, + reported_file: Path | None = None, + draft_changes_file: Path | None = None, channel: str = "internal-airflow-ci-cd", default_branch: str = "main", ) -> bool: @@ -1272,10 +1332,16 @@ def run_reconcile( :param config: the gate configuration of the default branch's checkout. :param workflows_dir: ``.github/workflows`` of a checkout of the default branch, whose triggers decide which recorded runs were legitimate. + :param reported_file: the items of the previous run, replaced by this run's. Items in it are + left out of the Slack message, so an item that persists is posted once. A dry run reads + it but leaves it unchanged, because it posts nothing. + :param draft_changes_file: where the draft state changes for the draft-state job are written, + as a JSON list; always written, empty when there are none. """ repo = client.repo unknown: list[tuple[int, str]] = [] attention: list[tuple[int, str]] = [] + draft_changes: list[DraftChange] = [] configs = BaseGateConfigs(client, repo=repo, default_branch=default_branch, default_config=config) discover(client, configs, repo=repo, now=now, unknown=unknown) @@ -1320,7 +1386,9 @@ def run_reconcile( ), count_behind=functools.partial(count_commits_behind, client, repo, pr), ) - apply_plan(client, pr, plan) + draft_change = apply_plan(client, pr, plan) + if draft_change is not None: + draft_changes.append(draft_change) # The fork's result is in the `Fork CI / Tests (AMD)` status; the gated run's skipped # checks only clutter the pull request. Runs with any project CI work stay for the # tampering check. @@ -1338,6 +1406,12 @@ def run_reconcile( for number, reason in [*unknown, *attention]: console_print(f"[warning]#{number}: {reason}[/]") console_print(f"[info]Reconciled {len(prs)} pull requests; GitHub calls: {dict(client.calls)}[/]") + if draft_changes_file is not None: + draft_changes_file.write_text(json.dumps([asdict(change) for change in draft_changes])) + if reported_file is not None: + unknown, attention, notes = drop_reported( + reported_file, unknown=unknown, attention=attention, notes=notes, record=not client.dry_run + ) has_alerts = bool(unknown or attention or notes) if has_alerts: payload = build_slack_payload( diff --git a/dev/breeze/tests/test_fork_ci_reconcile.py b/dev/breeze/tests/test_fork_ci_reconcile.py index aaa5e40bbc6..ec8828ac9b7 100644 --- a/dev/breeze/tests/test_fork_ci_reconcile.py +++ b/dev/breeze/tests/test_fork_ci_reconcile.py @@ -38,6 +38,7 @@ from airflow_breeze.utils.fork_ci_messages import ( from airflow_breeze.utils.fork_ci_reconcile import ( BaseGateConfigs, BypassFinder, + DraftChange, ForkObservation, ForkState, Plan, @@ -48,6 +49,7 @@ from airflow_breeze.utils.fork_ci_reconcile import ( classify, count_commits_behind, discover, + drop_reported, fetch_fork_observations, fetch_workflow_state, find_state_comment, @@ -1440,7 +1442,7 @@ def test_run_reconcile_isolates_failures_and_reports_them(client, tmp_path): client.graphql.side_effect = [ _search_result(), _search_result(), - _search_result(_pr_node(1), _pr_node(2)), + _search_result(_pr_node(1), _pr_node(2, isDraft=False)), {"pr0": {"object": None}, "pr1": {"object": None}}, ] workflow_state_calls = [] @@ -1456,11 +1458,22 @@ def test_run_reconcile_isolates_failures_and_reports_them(client, tmp_path): client.rest_get.side_effect = rest_get + draft_changes_file = tmp_path / "draft-changes.json" + has_alerts = run_reconcile( - client, CONFIG, now=NOW, run_url="https://run";, output_file=output, workflows_dir=tmp_path + client, + CONFIG, + now=NOW, + run_url="https://run";, + output_file=output, + workflows_dir=tmp_path, + draft_changes_file=draft_changes_file, ) assert has_alerts is True + assert json.loads(draft_changes_file.read_text()) == [ + {"number": 2, "node_id": "PR_2", "draft": True, "release": False} + ] payload = json.loads(output.read_text()) assert payload["channel"] == "internal-airflow-ci-cd" text = "\n".join(block["text"]["text"] for block in payload["blocks"]) @@ -1521,13 +1534,26 @@ def test_command_writes_has_alerts_output(mock_run, tmp_path, monkeypatch): github_output = tmp_path / "github_output" monkeypatch.setenv("GITHUB_OUTPUT", str(github_output)) - result = CliRunner().invoke(fork_ci_reconcile, ["--dry-run", "--output-file", str(tmp_path / "s.json")]) + result = CliRunner().invoke( + fork_ci_reconcile, + [ + "--dry-run", + "--output-file", + str(tmp_path / "s.json"), + "--reported-file", + str(tmp_path / "reported.json"), + "--draft-changes-file", + str(tmp_path / "draft-changes.json"), + ], + ) assert result.exit_code == 0, result.output assert github_output.read_text() == "has-alerts=true\n" github_client = mock_run.call_args.args[0] assert github_client.dry_run is True assert github_client.repo == "apache/airflow" + assert mock_run.call_args.kwargs["reported_file"] == tmp_path / "reported.json" + assert mock_run.call_args.kwargs["draft_changes_file"] == tmp_path / "draft-changes.json" @pytest.mark.parametrize( @@ -1557,21 +1583,105 @@ def test_run_reconcile_without_gated_pull_requests(client, tmp_path, total_count "fails", [pytest.param(False, id="all-succeed"), pytest.param(True, id="status-fails")] ) def test_fork_ci_required_is_removed_after_everything_else(client, fails): - plan, _, _ = _classify(_pr(labels=("fork ci required", "use project ci")), GREEN) + pr = _pr(is_draft=False, labels=("fork ci required", "use project ci")) + plan, _, _ = _classify(pr, GREEN) calls = mock.Mock() - client.mark_ready.side_effect = lambda *args: calls("mark_ready") client.remove_label.side_effect = lambda pr, label: calls("remove_label", label) - client.set_status.side_effect = GitHubError("gh api failed: HTTP 500") if fails else None + client.set_status.side_effect = ( + GitHubError("gh api failed: HTTP 500") if fails else lambda *args, **kwargs: calls("set_status") + ) if fails: with pytest.raises(GitHubError): - apply_plan(client, _pr(), plan) + apply_plan(client, pr, plan) client.remove_label.assert_not_called() else: - apply_plan(client, _pr(), plan) + apply_plan(client, pr, plan) assert calls.call_args_list[-1] == mock.call("remove_label", "fork ci required") [email protected]( + ("plan", "expected", "removes_fork_ci_required"), + [ + pytest.param( + Plan(draft=True, remove_labels=["fork ci required"]), + DraftChange(42, "PR_node", draft=True), + True, + id="draft", + ), + pytest.param(Plan(draft=False), DraftChange(42, "PR_node", draft=False), False, id="undraft"), + pytest.param( + Plan(draft=False, remove_labels=["fork ci required"]), + DraftChange(42, "PR_node", draft=False, release=True), + False, + id="undraft-on-release", + ), + pytest.param(Plan(), None, False, id="unchanged"), + ], +) +def test_apply_plan_leaves_draft_changes_to_the_draft_state_job( + client, plan, expected, removes_fork_ci_required +): + assert apply_plan(client, _pr(), plan) == expected + assert (mock.call(42, "fork ci required") in client.remove_label.call_args_list) is ( + removes_fork_ci_required + ) + + +def test_drop_reported_posts_each_item_once_while_it_persists(tmp_path): + reported_file = tmp_path / "reported.json" + + def report(unknown=(), attention=(), notes=()): + return drop_reported( + reported_file, unknown=list(unknown), attention=list(attention), notes=list(notes) + ) + + assert report(unknown=[(1, "a")], notes=["n"]) == ([(1, "a")], [], ["n"]) + assert report(unknown=[(1, "a")], attention=[(2, "b")], notes=["n"]) == ([], [(2, "b")], []) + assert report(attention=[(2, "b")]) == ([], [], []) + assert report(unknown=[(1, "a")], attention=[(2, "b")]) == ([(1, "a")], [], []) + + [email protected]("content", [None, "not json", "[]"]) +def test_drop_reported_reports_everything_without_a_readable_previous_run(tmp_path, content): + reported_file = tmp_path / "reported.json" + if content is not None: + reported_file.write_text(content) + + assert drop_reported(reported_file, unknown=[(1, "a")], attention=[], notes=[]) == ([(1, "a")], [], []) + assert json.loads(reported_file.read_text()) == {"unknown": [[1, "a"]], "attention": [], "notes": []} + + [email protected]( + ("first_dry_run", "second_has_alerts"), + [ + pytest.param(False, False, id="posted"), + pytest.param(True, True, id="dry-run-posts-nothing"), + ], +) +def test_run_reconcile_posts_a_persisting_item_once(client, tmp_path, first_dry_run, second_has_alerts): + output = tmp_path / "slack-message.json" + client.rest_get_pages.return_value = [{"total_count": 1500, "workflow_runs": []}] + + def reconcile(dry_run=False): + client.dry_run = dry_run + client.graphql.side_effect = [_search_result(), _search_result(), _search_result()] + return run_reconcile( + client, + CONFIG, + now=NOW, + run_url="https://run";, + output_file=output, + workflows_dir=tmp_path, + reported_file=tmp_path / "reported.json", + ) + + assert reconcile(dry_run=first_dry_run) is True + output.unlink() + assert reconcile() is second_has_alerts + assert output.exists() is second_has_alerts + + def test_slack_payload_stays_within_slack_limits(): attention = [(n, "x" * 2000) for n in range(500)]
