8000
Skip to content

[ci] Stop jobs hanging on apt by restoring the cached apt action and bounding raw apt calls - #18518

Merged
jesserockz merged 7 commits into
devfrom
ci-apt-hang
Aug 19, 2026
Merged

[ci] Stop jobs hanging on apt by restoring the cached apt action and bounding raw apt calls#18518
jesserockz merged 7 commits into
devfrom
ci-apt-hang

Conversation

@bdraco
@bdraco bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member

What does this implement/fix?

Jobs that run plain apt-get update on the runner are hanging on a slow apt mirror; on #18513 the component batch job is stuck in its apt step and the CodSpeed job is stuck inside the CodSpeed action, which installs libc6-dbg with its own unbounded apt call. Steps that go through awalsh128/cache-apt-pkgs-action are fine, a cache hit never touches apt.

This brings the cached apt action back for integration-tests and test-build-components-split (removed in #17463 because pull request runs each wrote their own cache copy) and adds a seed-apt-cache job that runs only on pushes with the same package list and version, so the single shared entry is written on dev and pull request jobs always hit it and never save. The benchmarks job gets a 30 minute timeout and pre-installs libc6-dbg with a bounded retry loop so the CodSpeed runner skips apt; ci-api-proto.yml is pull request only so nothing could seed it, it keeps plain apt wrapped in the same bounded retry loop. The cached apt action has no --no-install-recommends, so the component batch job goes back to the package set it had before #17463 with the recommended packages of libsdl2-dev included; seed-apt-cache is listed in ci-status so a broken seed is visible on dev instead of quietly bringing per pull request cache copies back.

Same fixes as esphome/esphome-device-builder#2586 and esphome/esphome-device-builder#2603.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New developer-facing feature (adds functionality for component developers; no end-user configuration 8000 change)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — policy
  • Developer breaking change (an API change that could break external components) — policy
  • Undocumented C++ API change (removal or change of undocumented public methods that lambda users may depend on) — policy
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

  • fixes

Pull request in esphome.io with documentation (if applicable):

  • esphome/esphome.io#<esphome.io PR number goes here>

Pull request in developers.esphome.io with developer documentation (if applicable):

  • esphome/developers.esphome.io#<developers.esphome.io PR number goes here>

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040/RP2350
  • BK72xx
  • RTL87xx
  • LN882x
  • nRF52840

Example entry for config.yaml:

# Example config.yaml

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

@esphbot review

@esphome esphome Bot added code-quality github-actions Pull requests that update GitHub Actions code medium-pr PR < 100 lines labels Aug 19, 2026
@codecov
codecov Bot commented Aug 19, 2026
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.75%. Comparing base (470226c) to head (e5e4270).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##              dev   #18518   +/-   ##
=======================================
  Coverage   88.75%   88.75%           
=======================================
  Files          68       68           
  Lines       15398    15398           
  Branches     2332     2332           
=======================================
  Hits        13667    13667           
  Misses       1421     1421           
  Partials      310      310           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@esphbot
esphbot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot esphbot 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.

Tip

No blocking issues found — ready to merge.

…g pre-install best effort, and gate ci-status on the apt cache seed
@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

35e4e0f

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

@esphbot review

@esphome esphome Bot removed the medium-pr PR < 100 lines label Aug 19, 2026
@esphbot
esphbot commented Aug 19, 2026
8000 Copy link
Copy Markdown
Contributor

@bdraco: 35e4e0f

Re-reviewed at that commit. All four prior findings resolved.

Retry budget now 480 s under a 600 s cap. timeout moved inside sudo, -k 15 added. libc6-dbg step gained timeout-minutes plus continue-on-error. seed-apt-cache added to ci-status needs — verified ci.yml:1460 accepts skipped, so pull requests stay green. Recommends drop documented in step comment.

Two nits left, both non-blocking: flat 60 s bound could hard-fail a congested mirror, and integration-tests step name still says ccache while pulling libsdl2-dev for key parity.

@esphbot
esphbot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot esphbot 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.

Tip

No blocking issues found — ready to merge.

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

f036958

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

@esphbot review

@esphbot
esphbot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

Previous review — superseded by a 8000 newer review below.

@esphbot esphbot 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.

Tip

No blocking issues found — ready to merge.

…ep its retry ladder inside a third of the job budget
@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

f0e0244

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

@esphbot review

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author
  1. Bounded-apt retry loop is duplicated verbatim across two workflows
    .github/workflows/ci-api-proto.yml:56-68

overkill for this case. could consider if it gets copied more

github-actions[bot]
github-actions Bot previously requested changes Aug 19, 2026
@github-actions github-actions Bot 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.

You have altered the generated proto files but they do not match what is expected.
Please run "script/api_protobuf/api_protobuf.py" and commit the changes.

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

31f5b22

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

@esphbot review

@github-actions
github-actions Bot dismissed their stale review August 19, 2026 19:49

Files now match the expected proto files.

@esphbot
esphbot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

@bdraco: > 2. Bounded-apt retry loop is duplicated verbatim across two workflows

Agreed. Two copies, both correct today. Extracting a composite action for one shared ladder buys little. Dropping this finding.

@esphbot
esphbot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot esphbot 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.

Tip

No blocking issues found — ready to merge.

…call cannot stall, and skip the index refresh when the image's lists suffice
@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

acaa9dd

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

@esphbot review

@bdraco
bdraco marked this pull request as ready for review August 19, 2026 20:26
@bdraco
bdraco requested a review from a team as a code owner August 19, 2026 20:26
Copilot AI lite review requested due to automatic review settings August 19, 2026 20:26
@esphome
esphome Bot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

👋 Hi there! This PR modifies 2 file(s) with codeowners.

@esphome/core - As codeowner(s) of the affected files, your review would be appreciated! 🙏

Note: Automatic review request may have failed, but you're still welcome to review.

Copilot AI 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.

Pull request overview

This PR updates ESPHome’s GitHub Actions workflows to prevent CI jobs from hanging on slow/unresponsive apt mirrors by reintroducing cached apt installs where they can be shared, and by adding explicit time bounds around remaining raw apt usage.

Changes:

  • Add a push-only seed-apt-cache job to populate a shared apt cache entry for libsdl2-dev and ccache, and restore that cache in pull-request-only jobs to avoid unbounded apt work.
  • Switch integration-tests and test-build-components-split to use the cached apt action, and pin integration-tests to ubuntu-24.04 to match the seeded environment.
  • Add time bounding/backstops for the CodSpeed benchmarks apt behavior (job timeout + bounded pre-install of libc6-dbg) and wrap PR-only ci-api-proto.yml apt usage with bounded retries/timeouts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/workflows/ci.yml Adds apt cache seeding/restoring for shared PR jobs; pins runner image where needed; bounds benchmark job apt behavior.
.github/workflows/ci-api-proto.yml Wraps PR-only apt installs with bounded timeouts/retry logic to avoid hangs.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@esphbot
esphbot commented Aug 19, 2026
Copy link
Copy Markdown
Contributor

PR Review — [ci] Stop jobs hanging on apt by restoring the cached apt action and bounding raw apt calls

Merge-ready. The rewrite since the last review (acaa9dd, e5e4270) is a genuine improvement over the retry ladder it replaces, and I verified the new design against the actual CI logs rather than by reading it.

What this revision gets right:

  • The "install first, update only on failure" flip is empirically correct, not just plausible. In the API Proto CI run for e5e4270 the bare apt-get install -y protobuf-compiler succeeded in ~5 s against the image's shipped lists — the rescue path never ran. That is the whole justification for the reordering, and it holds: the index refresh really is the part a congested mirror makes slow.
  • Persisting the timeouts to /etc/apt/apt.conf.d/99ci-acquire-timeouts is the right fix for the CodSpeed problem. -o flags could never reach the runner's own apt-get; a config file can. The filename is valid for apt.conf.d (no extension, safe characters) and sorts after the image's own drop-ins, so it wins. The tee heredoc survives YAML block-scalar dedent correctly — verified in the rendered step log.
  • The dpkg -s guard is placed after the tee, not before it. That ordering matters: the apt config still lands on the common path where libc6-dbg is already present. Easy detail to get backwards.
  • The pre-install achieves what it claims. In the ci-apt-hang-test run, the step installed libc6-dbg, and the CodSpeed action then logged Packages restored from cache: valgrind, libc6-dbg — it skipped its own unbounded apt entirely.
  • Timing budgets fit. Worst case per bounded step is 90+30+300+300 plus kill grace ≈ 775 s, inside the 15-minute step cap; the benchmarks job ran ~5 min end to end, so even the degenerate apt path leaves ample room in the 30-minute job budget.
  • Cache-key discipline verified end to end. The key is cache-apt-pkgs_<md5> with no OS component, exactly as the comment at ci.yml:342 claims — so pinning seed-apt-cache, integration-tests and test-build-components-split all to ubuntu-24.04 is the correct fix, and the action SHA 553a35b really is v1.6.3.
  • ci-status gating is sound. seed-apt-cache is in needs and the check accepts skipped, so PRs pay nothing for the skipped seed while a broken seed still reddens dev.
  • Coverage is complete. Every apt call site under .github/ at head is either behind the cache action or inside a bounded timeout -k invocation.

Two non-blocking notes:

  • version: 1.1 is an unquoted YAML float — harmless today, but a future 1.10 bump silently collapses to 1.1 and reuses the stale entry the bump was meant to invalidate.
  • The cache-key inputs are hand-synced across three jobs with only a comment enforcing it; drift fails quietly (per-PR cache copies come back) rather than loudly. A .github/actions/ composite would match the existing pattern here.

🟢 Suggestions

1. Quote the cache `version` so a future `1.10` bump does not collide with `1.1`
.github/workflows/ci.yml:85

version: 1.1 is an unquoted YAML float. It renders as the string 1.1 today (confirmed in the run log for ci-apt-hang-test: VERSION: 1.1), so nothing is broken now.

The trap is the next bump. YAML parses 1.10 as the float 1.10, which serialises back to 1.1 — identical to the current value. Since the action folds version into the cache key (cache-apt-pkgs_350f0c22dff655794a1e5137ede2c133), a 1.9 → 1.10 bump intended to force a fresh entry would silently reuse the stale one, which is exactly the failure this knob exists to prevent.

Quoting it costs nothing and matches how the neighbouring pin is written in ci-api-proto.yml (version: "0.11.15" for setup-buf). Applies to all three call sites: ci.yml:85, ci.yml:364, ci.yml:954.

          version: 1.1
2. Three hand-synced copies of the cache-key inputs; a composite action would make the invariant structural
.github/workflows/ci.yml:951-954

packages: libsdl2-dev ccache / version: 1.1 / the action SHA now appear identically in three jobs (ci.yml:82-85, ci.yml:361-364, ci.yml:951-954), and the only thing holding them in sync is a comment repeated in each place.

The drift consequence is quiet rather than loud: if one copy diverges, that job simply misses the shared entry and starts saving its own PR-scoped copy — the exact behaviour #17463 removed the action for. Nothing turns red; the job just gets slower and burns cache quota, so it would likely go unnoticed for a long time.

The repo already has the right shape for this — .github/actions/ holds restore-python, cache-esp-idf, cache-sdk-nrf. A .github/actions/apt-packages composite that owns the package list, the version and the SHA would turn "must match exactly" from a comment into something that cannot drift.

Non-blocking, and adjacent to the shell retry-loop duplication @bdraco already dismissed ("overkill for this case. could consider if it gets copied more") — noting it because this one has three copies and a silent failure mode rather than two copies and a loud one. Same call is reasonable here.

        uses: awalsh128/cache-apt-pkgs-action@553a35bb8ebd9fcabcb1c9451aa4c98e1b4ca8a9 # v1.6.3
        with:
          packages: libsdl2-dev ccache
          version: 1.1

Checklist

  • Diff delivers what the PR description promises
  • All raw apt call sites in .github/ are bounded
  • apt.conf.d drop-in is validly named and takes effect
  • Heredoc survives YAML block-scalar dedent
  • Retry budgets fit inside step timeouts, and steps inside job budgets
  • Install-before-update ordering verified against real CI logs
  • CodSpeed runner actually skips its own apt after the pre-install
  • Cache key scoped so restores are safe across runner images
  • Pinned action SHA matches its claimed tag
  • ci-status gate tolerates the skipped seed job on pull requests
  • Error paths fail closed rather than hanging
  • Cache-key inputs cannot silently drift between jobs — suggestion #1, suggestion #2
  • Comments stay accurate after the change
  • No hardcoded secrets or credentials
  • No scope creep beyond the two workflow files

Automated review by Kōan (Claude) HEAD=e5e4270 6 min 57s

@esphbot esphbot 6D38 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.

Tip

No blocking issues found — ready to merge.

@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

decline remaining bot suggestions YAGNI and also already addressed above

@bdraco bdraco added this to the 2026.8.0 milestone Aug 19, 2026
@bdraco
bdraco commented Aug 19, 2026
Copy link
Copy Markdown
Member Author

tagged for backport since somewhere between 20-30% of jobs currently get stuck and this will likely affect the release

https://github.com/esphome/esphome/actions?query=is%3Ain_progress

@jesserockz
jesserockz merged commit 0b1065f into dev Aug 19, 2026
38 checks passed
@jesserockz
jesserockz deleted the ci-apt-hang branch August 19, 2026 21:30
@jesserockz jesserockz mentioned this pull request Aug 19, 2026
bdraco added a commit to guillempages/esphome that referenced this pull request Aug 19, 2026
@github-actions github-ac 436E tions Bot locked and limited conversation to collaborators Aug 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cherry-picked code-quality github-actions Pull requests that update GitHub Actions code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0