Skip to content

Harden Streamlined Build workflow against intermittent download and nbd-lock failures - #10

Open
mobileskyfi with Copilot wants to merge 2 commits into
mainfrom
copilot/investigate-build-failures
Open

mobileskyfi with Copilot wants to merge 2 commits into
mainfrom
copilot/investigate-build-failures

Conversation

Copilot AI commented Sep 16, 2026

Copy link
Copy Markdown

Recent "Streamlined Build" CI runs failed intermittently. Investigation of the run logs turned up two distinct causes.

Root causes

  • Download step: curl exits 56, 92, and 28 (recv error, HTTP/2 stream error, timeout) hitting download.mikrotik.com. A prior fix bumped timeouts, which only covers exit 28 — curl's default --retry doesn't retry the other error classes, so those runs failed outright.
  • "Convert images" step: qemu-img: Could not open 'chr-efi.qcow2': Failed to get shared "write" lock. Race condition — qemu-nbd -d returns before the kernel finishes releasing the file lock on the qcow2 image, so the immediately-following qemu-img convert sometimes loses the race.

Changes (.github/workflows/auto.yaml)

  • Download step: add --retry-all-errors (with increased retry count/delay) so curl retries on any failure, not just its default transient-error subset.
  • Disconnect ndb step: after qemu-nbd -d, poll /sys/class/block/<dev>/pid until each device is fully torn down before moving on to image conversion, removing the lock race.
- name: Download
  run: |
    curl -skL --retry 5 --retry-all-errors --retry-delay 5 --connect-timeout 10 --max-time 300 -o chr.img.zip ...

- name: Disconnect ndb
  run: |
    sudo -E qemu-nbd -d /dev/nbd0
    sudo -E qemu-nbd -d /dev/nbd1
    for dev in nbd0 nbd1; do
      for i in $(seq 1 10); do
        [ -f "/sys/class/block/$dev/pid" ] || break
        sleep 1
      done
    done

Co-authored-by: mobileskyfi <45924209+mobileskyfi@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 40a98887-6066-4e6b-aca7-8f98f7a9637c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mobileskyfi
mobileskyfi marked this pull request as ready for review September 16, 2026 20:56
Copilot AI lite review requested due to automatic review settings September 16, 2026 20:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

NBD teardown timeout can still allow conversion to proceed while the lock remains.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Harden the Streamlined Build workflow against transient downloads and NBD teardown races.

Changes:

  • Expands curl retry handling.
  • Polls NBD device teardown before image conversion.
File summaries
File Description
.github/workflows/auto.yaml Updates download retries and NBD cleanup handling.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment on lines +168 to +171
for i in $(seq 1 10); do
[ -f "/sys/class/block/$dev/pid" ] || break
sleep 1
done

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — fixed. The loop now explicitly checks whether the sysfs pid file still exists after the 10s wait and fails the step with a clear error instead of silently falling through to the conversion step, so we no longer risk hitting the same lock race with a confusing downstream error.

Co-authored-by: mobileskyfi <45924209+mobileskyfi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants