Skip to content

Run dependency explanations separately from main AMD canaries - #74311

Open
zozo123 wants to merge 9 commits into
apache:mainfrom
zozo123:ci-upgrade-diagnostics
Open

zozo123 wants to merge 9 commits into
apache:mainfrom
zozo123:ci-upgrade-diagnostics

Conversation

@zozo123

@zozo123 zozo123 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Scheduled and manually dispatched AMD canaries on main currently wait for detailed dependency upgrade explanations, which can require a full resolution for each outdated package. Keep the quick freshness summary in the canary and run the detailed explanations in a separate Dependency upgrade report workflow after the canary completes.

Canary (finalize-tests.yml)

  • On main schedule and dispatch canaries, the Deps step runs constraints-version-check --no-explain-why --save-constraints-to files/dependency-report. Breeze resolves the constraints branch to a commit, downloads the constraints at that commit, and saves both. A failure to save only warns.
  • The inputs are uploaded per canary attempt with continue-on-error, so the report path cannot turn the canary red.
  • Other events and release branches keep --explain-why in the canary.

Report (dependency-upgrade-report.yml, workflow_run)

  • It checks that the triggering attempt is a completed main schedule or dispatch AMD canary of this repository.
  • It checks out the canary's source revision and downloads that run's saved constraints and CI image by artifact ID; it never substitutes newer data. Entries whose inputs or image are missing or expired are skipped with a warning and listed in the summary, and the others are still explained.
  • Inputs that were already explained for an earlier canary attempt are not explained again, so re-running a flaky canary job does not queue a second report.
  • On RunsOn, the canary's image lives in RunsOn's S3 cache with only a digest artifact. The report then runs on a RunsOn runner and fetches exactly that tarball by its digest, verified as stash-restore does. On public runners it downloads the image artifact as before.
  • It has read-only permissions, runs at most three jobs at a time with a 150-minute limit each, cleans up disk like the image build jobs, and saves partial output on failure.
  • It runs in the default branch's context. No step saves a cache, and the docs say not to add one.

Breeze

  • constraints-version-check gets a new --save-constraints-to option. The report analyzes the saved file with --constraints-file.
  • download_constraints_file and the new get_branch_commit honour CONSTRAINTS_GITHUB_REPOSITORY, defaulting to apache/airflow.
  • The resolver skips a redundant pinned resolution when the shared baseline already selects the latest version.

Measurements from the four latest scheduled main AMD canaries (38993077423, 39038765293, 39088484466, 39133923075):

  • The canary takes 172–215 min today and would take about 79–114 min without the explanations, 92–108 min shorter.
  • 438–515 runner-minutes of explanations per canary move to the report. The report adds about 50–70 runner-minutes of setup on top.
  • The slowest explanations (constraints-source-providers) take 105–120 min per job, hence the 150-minute limit.

Validation

  • Breeze unit suite: 1,384 passed. 7 tests were skipped because the SVN/Java prerequisites are not installed locally. The report-planning tests are skipped when node is not installed.
  • prek checks on the changed files passed, including workflow lint and the regenerated command documentation.
  • The workflow_run handoff and the --save-constraints-to path only run on main canaries, so the first real report will come from the first canary after merge.

Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-6) and Claude Code (Opus 5.5)

Generated-by: Codex (GPT-6) and Claude Code (Opus 5.5), following the guidelines

@zozo123 zozo123 changed the title Keep dependency upgrade explanations off the canary completion path Run dependency explanations separately from main AMD canaries Oct 6, 2026
@zozo123
zozo123 force-pushed the ci-upgrade-diagnostics branch from 6491f0b to 9db131f Compare October 6, 2026 06:01
@potiuk potiuk removed the backport-to-v3-4-test Backport to v3-4-test label Oct 6, 2026
zozo123 and others added 3 commits October 14, 2026 11:50
Detailed upgrade investigations grow with the number of outdated packages and delay canary completion even when test preparation gets faster. Maintainers need prompt test results while retaining reproducible upgrade diagnostics.
Diagnostic reruns must retain their original inputs without gaining cache write access on the default branch. The workflow-run event already provides that boundary and the standard rerun action preserves the source canary.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, moving the explanations out of the canary itself makes sense. Details are inline; the first one (a single missing artifact stops every report) looks likely to make the report fail on most runs.


Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting

Comment thread .github/workflows/finalize-tests.yml Resolved
Comment thread dev/breeze/doc/ci/05_workflows.md Resolved
@potiuk

potiuk commented Oct 18, 2026

Copy link
Copy Markdown
Member

Could you also share some measurements of the actual savings: how much shorter the main AMD canary gets with the explanations moved out, against the runner time the separate report uses (up to 15 jobs of up to 90 minutes, three at a time)?


Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting

zozo123 and others added 2 commits October 18, 2026 20:11
Picks up the httpx2 Dag processor client fix (apache#74731) that the core test jobs and the
airflow-core mypy check failed on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Skip entries whose saved constraints or CI image are missing or expired,
  with a warning and a summary line, instead of failing every report.
- Name report inputs by canary attempt and explain each input once, so a
  re-run canary no longer queues a second report from the same inputs.
- Post the same notice on every attempt when the canary saved no inputs.
- Free up disk space and move Docker to /mnt before loading the image.
- Save the checked constraints through breeze's --save-constraints-to,
  pinned to the resolved constraints commit; failing to save only warns,
  so the canary no longer depends on raw `gh api` calls.
- Say that no cache step, not workflow_run, keeps the cache untouched, and
  skip the plan tests when node is unavailable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zozo123

zozo123 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Here is where this stands.

Review comments (all fixed in 5edef09 unless noted)

  • One missing artifact stopped every report: unavailable entries are now skipped with a warning and listed under "Skipped" in the summary. The other entries are still planned.
  • The stash artifact name relied on the action's internals: a test now builds the name from the stash key in ci-image-build.yml and the action's naming rule at the pinned SHA. A rename on either side fails the test.
  • A re-run with no saved inputs failed hard: every attempt now posts the same notice.
  • A re-run canary queued a second report: inputs now carry the canary attempt, and inputs already explained for an earlier attempt are not explained again. Concurrency is per run id and attempt.
  • Disk space: the explain job runs free_up_disk_space.sh, make_mnt_writeable.sh and move_docker_to_mnt.sh like the image build job, and a test keeps the two in sync.
  • Raw gh api calls in the canary: replaced by constraints-version-check --save-constraints-to. The constraints are downloaded at the resolved commit and that commit is recorded. A failure to save only warns, so it cannot turn the canary red.
  • CONSTRAINTS_GITHUB_REPOSITORY was ignored (dc37b56): download_constraints_file and get_branch_commit now read it and fall back to apache/airflow when it is unset or empty. CI here behaves as before.
  • workflow_run cache wording: the docs now say the workflow runs in the default branch's context and that no step may save a cache.
  • Tests that need node are skipped when it is not on PATH.

RunsOn canaries (35938b5): since #74889, scheduled canaries keep the CI image in RunsOn's S3 cache, and only its digest is a GitHub artifact. The plan accepts that digest, and the explain job runs on a RunsOn runner and fetches exactly that tarball by its digest, verified as stash-restore does. Without this, every entry would have been skipped.

Timeout (a505227): the explain job limit goes from 90 to 150 min. The constraints-source-providers explanations take 105–120 min in the canary, so a 90 min limit would cut them off.

Measurements from the four latest scheduled main AMD canaries (38993077423, 39038765293, 39088484466, 39133923075), taken from job timestamps and the per-row timestamps in the Deps step logs:

per canary
canary wall clock today 172–215 min
without the explanations (estimate) 79–114 min, i.e. 92–108 min shorter
explanation work that moves to the report 438–515 runner-min across the 12 Deps jobs
report overhead on top of that about 12 × 4–6 min = 50–70 runner-min (checkout, disk cleanup, image download and load)

How I estimated it:

  • The check computes each outdated package's explanation inline, before it prints that package's row.
  • Rows for up-to-date packages take 0.03–0.11 s each, so the plain summary for about 800 packages takes 0.4–1.5 min. The rest of the step is explanation.
  • Today the canary ends with the slowest Deps …:constraints-source-providers job. The last job that isn't a Deps job finishes 93–109 min earlier.

Earlier failures: the 14 failing checks before main was merged in (6648f4f) were not caused by this PR. mypy-airflow-core (Static checks) and dag_processing/test_api_client.py (core test jobs) failed on the Oct 14 base and are fixed on main by #74731. One K8s job hit a stash-restore API flake.

CI: 35938b5, which merges in main and adds the RunsOn change, is green: 137 checks passed, none failed.

Still open

  • The report workflow and the --save-constraints-to path run only for main schedule/dispatch canaries. PR CI covers them through the breeze tests only, so the first real report will come from the first canary after merge.
  • Follow-ups I left out of this PR:
    • Saving the CI image under a dedicated artifact name. It would add a multi-GB upload to every canary.
    • Breeze image builds still pass apache/airflow as the CONSTRAINTS_GITHUB_REPOSITORY build arg. That was already the case before this PR.

Explaining constraints-source-providers took 105-120 minutes per job in
the last four main canaries, past the report job's 90-minute timeout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zozo123

zozo123 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor Author

The measurements are now part of the updated summary: #74311 (comment)

CONSTRAINTS_GITHUB_REPOSITORY names the repository holding the
constraints-* branches, apache/airflow by default, and the Dockerfiles
read it. Breeze's download_constraints_file and get_branch_commit always
used apache/airflow instead, so setting it changed nothing for
constraints-version-check, the SBOM and provider dependency downloads,
or the constraints commit saved for the dependency report.

Both functions now read CONSTRAINTS_GITHUB_REPOSITORY and fall back to
apache/airflow when it is unset or empty, so existing callers and CI on
apache/airflow behave as before, and a saved commit always comes from
the repository its constraints were downloaded from. Other files fetched
through download_file_from_github still come from apache/airflow.
@zozo123
zozo123 requested a review from potiuk October 19, 2026 05:43

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @potiuk - WDYT?

Since apache#74889 scheduled canaries run on RunsOn, where the CI image stash
goes to RunsOn's S3 cache and only its digest is a GitHub artifact, so
every report entry was skipped as "CI image missing". The plan now also
accepts that digest artifact; the explain job then runs on a RunsOn
runner, downloads exactly that tarball by its digest and checks it like
stash-restore does. It prepares the disk with the workflow version's
runner scripts, as the image build job now does.
@zozo123

zozo123 commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

I merged main and adapted the report to #74889.

Why: scheduled canaries now run on RunsOn. There the CI image stash goes to RunsOn's S3 cache, and only its digest is a GitHub artifact: the latest canaries, e.g. 40200075185, only have ci-image-save-v3-linux_amd64-<py>-s3-main. So every report entry would have been skipped as "CI image missing".

What changed in 35938b5:

  • The plan also accepts that digest artifact.
  • The explain job then runs on a RunsOn runner, with the same label template and the 8 CPU / 32 GB size as the Deps jobs.
  • It downloads exactly that tarball by its digest, verifies its sha256, and extracts it into /mnt, as stash-restore does. On public runners nothing changes.
  • Tests tie the digest suffix to stash-save, the S3 layout to its sha256/<digest>.tar objects, and the runner label to breeze's RUNS_ON_AMD_RUNNER_TEMPLATE.

@shahar1, this touches the RunsOn path; could you take a look? The CI run for this commit is also waiting for a maintainer to approve it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants