Skip to content

zedmanager: do not let a domain-held stale volume ref block a purge - #6442

Draft
andrewd-zededa wants to merge 2 commits into
lf-edge:masterfrom
andrewd-zededa:eve-k-purge-pvc-partial-annotation
Draft

andrewd-zededa wants to merge 2 commits into
lf-edge:masterfrom
andrewd-zededa:eve-k-purge-pvc-partial-annotation

Conversation

@andrewd-zededa

@andrewd-zededa andrewd-zededa commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Description

doInstall keeps a VolumeRefStatus that the current AppInstanceConfig
no longer references around, unremoved, for as long as the running domain
still uses it — so its release can be ordered after the domain is torn
down rather than yanking storage out from under a live app. That part is
correct.

But doInstall's aggregate error computation still folded that stale
entry's error into the app's own blocking error, so doInstall never
returned done=true while the stale volume was in an error state. That
deadlocks the purge: doUpdate bails out on !done before ever reaching
the code that tears down the domain — which is the only thing that lets
the stale volume actually go away. A volume that failed once (independent
of any purge) and then had a purge move the app to a new generation would
wedge that purge forever, with no automatic path out.

Fix: skip a stale, domain-held VolumeRefStatus's error (and state) when
computing doInstall's aggregate result during the DownloadAndVerify
phase — only errors on volume refs still present in the current config
should be able to block progress.

Found while debugging two apps on an eve-k dev cluster stuck indefinitely
in DownloadAndVerify after an unrelated volume-creation failure (a
separate CDI-upload-completeness bug) left one volume permanently errored
right as a purge tried to move the app to a new generation.

PR dependencies

None

How to test and validate this PR

Covered by an automated regression test:

make TESTPKGS=./pkg/pillar/cmd/zedmanager/... TESTRUN=TestDoInstall test

Covered by an automated regression test:

make TESTPKGS=./pkg/pillar/cmd/zedmanager/... TESTRUN=TestDoInstall test

TestDoInstallErroredStaleVolumeRefHeldByDomainDoesNotBlock reproduces the
exact shape: a domain still attached to an errored old-generation volume,
plus a new-generation volume already LOADED and waiting on VerifyOnly
to clear. Before the fix, doInstall returns done=false forever; after
it, done=true and the stale volume's error is not folded into the app's.

Changelog notes

Fixed a bug where an application purge on eve-k could hang indefinitely
if the app's currently-active volume was in a permanent error state when
the purge was issued — the purge would never tear down the old app
generation or start the new one. No other user-facing changes.

PR Backports

  • 17.0-stable: To be backported.
  • 16.0-stable: No, as the feature is not available there.
  • 14.5-stable: No, as the feature is not available there.
  • 13.4-stable: No, as the feature is not available there.

Checklist

  • I've provided a proper description
  • I've added the proper documentation
  • I've tested my PR on amd64 device
  • I've tested my PR on arm64 device
  • I've written the test verification instructions
  • I've set the proper labels to this PR

And the last but not least:

  • I've checked the boxes above, or I've provided a good reason why I didn't
    check them.

Please, check the boxes above after submitting the PR in interactive mode.

andrewd-zededa and others added 2 commits August 31, 2026 15:53
doInstall keeps a VolumeRefStatus that the current AppInstanceConfig no
longer references around, unremoved, as long as the running domain still
uses it - so its release can be ordered after the domain is torn down.
But its aggregate error computation still folded that stale entry's
error into the app's own blocking error, so doInstall never returned
done=true while the stale volume was in an error state.

That deadlocks the purge: doUpdate bails out on !done before ever
reaching the code that tears down the domain, which is the only thing
that lets the stale volume actually go away. A volume that failed once
(e.g. via the CDI-upload-completeness bug) before a purge moved the app
to a new generation would then wedge that purge forever, with no
automatic path out.

Skip a stale, domain-held VolumeRefStatus's error (and state) when
computing doInstall's aggregate result during the DownloadAndVerify
phase - only errors on volume refs still present in the current config
should be able to block progress.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Durbin <andrewd@zededa.com>
Covers the fix in the previous commit: doInstall must not let an error
on a VolumeRefStatus dropped from the current config, but still held
alive because the running domain uses it, block doInstall from ever
returning done. The test constructs that exact shape - a domain still
attached to an errored old-generation volume, a new-generation volume
already LOADED and waiting - and asserts doInstall reports done and
does not fold the stale volume's error into the app's own error.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Durbin <andrewd@zededa.com>
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 26.04%. Comparing base (7f50511) to head (7ef13c3).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6442      +/-   ##
==========================================
+ Coverage   25.67%   26.04%   +0.37%     
==========================================
  Files         509      519      +10     
  Lines       93304    95023    +1719     
==========================================
+ Hits        23955    24748     +793     
- Misses      67501    68248     +747     
- Partials     1848     2027     +179     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

eriknordmark added a commit to eriknordmark/eve that referenced this pull request Sep 17, 2026
Integration branch on master with the conversion chain (lf-edge#6530, lf-edge#6063, fork#6)
and the purge/PVC robustness set (lf-edge#6442, lf-edge#6406, lf-edge#6478). Records the tips each
piece was replayed from, the branch-local adapters that no PR carries, and the
storage-resizer content pin.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Sep 18, 2026
Integration branch on master with the conversion chain (lf-edge#6530, lf-edge#6063, fork#6)
and the robustness set (lf-edge#6442, lf-edge#6406, lf-edge#6478, lf-edge#6505, lf-edge#6451, lf-edge#6590, lf-edge#6599).
Records the tips each piece was replayed from, where two PRs collide and which
side won, the branch-local adapters that no PR carries, and the
storage-resizer content pin.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
eriknordmark added a commit to eriknordmark/eve that referenced this pull request Sep 18, 2026
Integration branch on master with the conversion chain plus the fault-injection
stress harness (lf-edge#6530, lf-edge#6063, fork#6, fork#7) and the robustness set (lf-edge#6442,
lf-edge#6406, lf-edge#6478, lf-edge#6505, lf-edge#6451, lf-edge#6590, lf-edge#6599). Records the tips each piece was
replayed from, where two PRs collide and which side won, the branch-local
adapters that no PR carries, and the chaos storage-resizer content pin.

Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.

1 participant