Skip to content

hypervisor: recover a VMIRS stranded at Spec.Replicas == 0 - #6269

Open
andrewd-zededa wants to merge 1 commit into
lf-edge:masterfrom
andrewd-zededa:eve-k-vmirs-replicas-reconcile
Open

hypervisor: recover a VMIRS stranded at Spec.Replicas == 0#6269
andrewd-zededa wants to merge 1 commit into
lf-edge:masterfrom
andrewd-zededa:eve-k-vmirs-replicas-reconcile

Conversation

@andrewd-zededa

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

Copy link
Copy Markdown
Contributor

Description

DetachUtilVmirsReplicaReset scales a VMIRS 0 then 1 under one bounded context; if the deadline expires, the API server flaps, or the process dies between the two writes, Replicas stays 0 forever. Nothing else in pillar ever reads or repairs Spec.Replicas, and StopReplicaVMI deletes the VMIRS rather than scaling it, so zero replicas is never a legitimate steady state.

Two compounding gaps let this go unnoticed and unrepaired: Start() treats IsAlreadyExists on Create() as success without checking the existing object's replica count, and Info() reports a zero-replica VMIRS (no VMI, no virt-launcher pod) as UNKNOWN with a nil error, before scheduledOnMe's !onMe short-circuit is ever reached. That left domainmgr reporting the app RUNNING to the controller indefinitely while the cluster ran it with zero replicas.

  • Info() now checks desired replicas for IsMetaReplicaVMI domains before the scheduledOnMe short-circuit, and reports HALTED with an error on a stranded VMIRS. This is deliberately HALTED, not BROKEN, so verifyStatus's recovery branch sets BootFailed and (for kube) publishes BOOTING while skipping Delete/Cleanup, letting maybeRetryBoot drive recovery through the existing retry loop.
  • Start()'s IsAlreadyExists branch now calls ensureVmirsReplicas, which raises Spec.Replicas to at least 1 with a bounded Get/mutate/Update retry on conflicting writers. The repair only ever writes upward on observing zero, so concurrent callers converge and it can never produce the stranded state itself.
  • Both new k8s API calls are bounded by kubeapi.KubeAPITimeout() (exported getter over kubeapi's existing private constant, to avoid a third redeclaration of the same budget) rather than context.Background(), since Info()/Start() run on domainmgr's watchdog-timed verifyStatus tick and an unbounded call against a degraded apiserver would otherwise risk a watchdog reboot.
  • DetachOldWorkload no longer discards DetachUtilVmirsReplicaReset's return value, so a stranded VMIRS is attributable in logs.
  • StopReplicaVMI no longer logs "Stop VMI Replicaset error " on every successful stop, which polluted the logs used to verify this fix.

Adds an evetest chaos test (evetest/tests/cluster) that deploys an app on a single-node EVE-K cluster, scales its VMIRS to 0 replicas directly over SSH to reproduce the stranded state without needing a real failover, and asserts via the EVE API that the app surfaces a retryable warning and then self-heals back to RUNNING.

PR dependencies

None

How to test and validate this PR

Unit tests (pkg/pillar/hypervisor/kubevirt_test.go): covers
vmirsStranded, vmirsDesiredReplicas, ensureVmirsReplicas (including
conflict-retry). make -C pkg/pillar test passes, no regressions.

evetest (TestVMIRSStrandedReplicasRecovery, tests/cluster): deploys
an app on eve-k, scales its VMIRS to 0 via SSH, asserts self-recovery via
the EVE API. Build/vet-clean; not yet run against real hardware — run with
make evetest NAME=TestVMIRSStrandedReplicasRecovery.

Manual repro:

kubectl -n eve-kube-app patch vmirs <name> --type=merge -p '{"spec":{"replicas":0}}'

Before: app stays falsely RUNNING with 0 replicas forever. After: within
one timer.boot.retry interval, DomainStatus shows a retryable warning,
then Replicas returns to 1 and the app recovers to RUNNING.

Also check no regression on the normal paths (start/purge/stop/failover).

Changelog notes

Enhanced error recovery on failover of app instances during periods of kube-apiserver instability.

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.

Comment thread pkg/pillar/hypervisor/kubevirt.go Outdated
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 39.43662% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 23.33%. Comparing base (5d69266) to head (601a7fa).
⚠️ Report is 31 commits behind head on master.

Files with missing lines Patch % Lines
pkg/pillar/hypervisor/kubevirt.go 41.79% 37 Missing and 2 partials ⚠️
pkg/pillar/kubeapi/kubeapi.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6269      +/-   ##
==========================================
+ Coverage   22.93%   23.33%   +0.39%     
==========================================
  Files         510      520      +10     
  Lines       93473    95248    +1775     
==========================================
+ Hits        21440    22223     +783     
- Misses      70292    71100     +808     
- Partials     1741     1925     +184     

☔ 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.

@andrewd-zededa
andrewd-zededa force-pushed the eve-k-vmirs-replicas-reconcile branch from 80ed190 to e73b0f9 Compare August 4, 2026 00:45
@github-actions
github-actions Bot requested a review from naiming-zededa August 4, 2026 00:45
@andrewd-zededa

Copy link
Copy Markdown
Contributor Author

evetest run of latest:

HARNESS  time="2026-08-04T01:13:42Z" level=info msg="BuildImage "lfedge/eve:0.0.0-pr-6269-e73b0f9c-k-amd64" succeeded (docker image was already present)."
...
HARNESS  time="2026-08-04T01:25:04Z" level=info msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" is RUNNING"
TEST     time="2026-08-04T01:25:04Z" level=info msg="Injecting fault: eve exec kube kubectl -n eve-kube-app patch vmirs vmirs-chaos-app-9e01e-0 --type=merge -p '{"spec":{"replicas":0}}'"
TEST     time="2026-08-04T01:25:05Z" level=info msg="Waiting for: app reports a retryable warning for the stranded VMIRS..."
HARNESS  time="2026-08-04T01:25:23Z" level=info msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" state changed: RUNNING -> BOOTING"
HARNESS  time="2026-08-04T01:25:23Z" level=warning msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" error: domain 9e01e8f6-d181-4529-ba24-fb191c3f285d.1.1 vmirs vmirs-chaos-app-9e01e-0 scaled to 0 replicas"
TEST     time="2026-08-04T01:25:23Z" level=info msg="Waiting for: app recovered to RUNNING with no error..."
HARNESS  time="2026-08-04T01:25:30Z" level=warning msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" error: domain 9e01e8f6-d181-4529-ba24-fb191c3f285d.1.1 vmirs vmirs-chaos-app-9e01e-0 scaled to 0 replicas"
HARNESS  time="2026-08-04T01:25:30Z" level=warning msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" error: domain 9e01e8f6-d181-4529-ba24-fb191c3f285d.1.1 vmirs vmirs-chaos-app-9e01e-0 scaled to 0 replicas"
HARNESS  time="2026-08-04T01:25:30Z" level=warning msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" error: domain 9e01e8f6-d181-4529-ba24-fb191c3f285d.1.1 vmirs vmirs-chaos-app-9e01e-0 scaled to 0 replicas"
HARNESS  time="2026-08-04T01:26:04Z" level=info msg="App "9e01e8f6-d181-4529-ba24-fb191c3f285d" (vmirs-chaos-app) on device "edge-dev" state changed: BOOTING -> RUNNING"
...
--- PASS: TestVMIRSStrandedReplicasRecovery (832.18s)

@andrewd-zededa andrewd-zededa added stable Should be backported to stable release(s) next-17.0.x-rc PR must be present in the next 17.0.x-lts release labels Aug 4, 2026
@zedi-pramodh

Copy link
Copy Markdown

I am already outdated :) AFAIR, we never want to bring the replica set to 0, we only delete the VMIRS. The idea is not to loose the vmirs and when all nodes are rebooted at same time, atleast one of the nodes will start it. What changed now ?

DetachUtilVmirsReplicaReset scales a VMIRS 0 then 1 under one bounded
context; if the deadline expires, the API server flaps, or the process
dies between the two writes, Replicas stays 0 forever. Nothing else in
pillar ever reads or repairs Spec.Replicas, and StopReplicaVMI deletes
the VMIRS rather than scaling it, so zero replicas is never a
legitimate steady state.

Two compounding gaps let this go unnoticed and unrepaired: Start()
treats IsAlreadyExists on Create() as success without checking the
existing object's replica count, and Info() reports a zero-replica
VMIRS (no VMI, no virt-launcher pod) as UNKNOWN with a nil error,
before scheduledOnMe's !onMe short-circuit is ever reached. That left
domainmgr reporting the app RUNNING to the controller indefinitely
while the cluster ran it with zero replicas.

- Info() now checks desired replicas for IsMetaReplicaVMI domains
  before the scheduledOnMe short-circuit, and reports HALTED with an
  error on a stranded VMIRS. This is deliberately HALTED, not BROKEN,
  so verifyStatus's recovery branch sets BootFailed and (for kube)
  publishes BOOTING while skipping Delete/Cleanup, letting
  maybeRetryBoot drive recovery through the existing retry loop.
- Start()'s IsAlreadyExists branch now calls ensureVmirsReplicas,
  which raises Spec.Replicas to at least 1 with a bounded
  Get/mutate/Update retry on conflicting writers. The repair only ever
  writes upward on observing zero, so concurrent callers converge and
  it can never produce the stranded state itself.
- Both new k8s API calls are bounded by kubeapi.KubeAPITimeout()
  (exported getter over kubeapi's existing private constant, to avoid
  a third redeclaration of the same budget) rather than
  context.Background(), since Info()/Start() run on domainmgr's
  watchdog-timed verifyStatus tick and an unbounded call against a
  degraded apiserver would otherwise risk a watchdog reboot.
- DetachOldWorkload no longer discards DetachUtilVmirsReplicaReset's
  return value, so a stranded VMIRS is attributable in logs.
- StopReplicaVMI no longer logs "Stop VMI Replicaset error <nil>" on
  every successful stop, which polluted the logs used to verify this
  fix.

Adds an evetest chaos test (evetest/tests/cluster) that deploys an app
on a single-node EVE-K cluster, scales its VMIRS to 0 replicas directly
over SSH to reproduce the stranded state without needing a real
failover, and asserts via the EVE API that the app surfaces a retryable
warning and then self-heals back to RUNNING.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Durbin <andrewd@zededa.com>
@andrewd-zededa
andrewd-zededa force-pushed the eve-k-vmirs-replicas-reconcile branch from e73b0f9 to 601a7fa Compare August 4, 2026 01:57
@andrewd-zededa

Copy link
Copy Markdown
Contributor Author

I am already outdated :) AFAIR, we never want to bring the replica set to 0, we only delete the VMIRS. The idea is not to loose the vmirs and when all nodes are rebooted at same time, atleast one of the nodes will start it. What changed now ?

This is just error recovery if the scale up here was unable to occur:

func DetachUtilVmirsReplicaReset(log *base.LogObject, vmiRsName string) (err error) {

@zedi-pramodh

Copy link
Copy Markdown

DetachUtilVmirsReplicaReset

I see, so did we add this to quickly terminate the stuck VMI and restart it again ? Was this code part of 17.0.0-lts ?

@andrewd-zededa

Copy link
Copy Markdown
Contributor Author

DetachUtilVmirsReplicaReset

I see, so did we add this to quickly terminate the stuck VMI and restart it again ? Was this code part of 17.0.0-lts ?

This is to handle an issue where the new vmi is not getting scheduled, and yes part of the lts.

@andrewd-zededa

Copy link
Copy Markdown
Contributor Author

Latest evetest run:

HARNESS  time="2026-08-04T02:11:18Z" level=info msg="BuildImage "lfedge/eve:0.0.0-pr-6269-601a7faf-k-amd64" succeeded (docker image was already present)."
...
HARNESS  time="2026-08-04T02:22:04Z" level=info msg="App "58eadf56-b98b-48fd-9121-a26d76f1a3b3" (vmirs-chaos-app) on device "edge-dev" state changed to RUNNING"
HARNESS  time="2026-08-04T02:22:04Z" level=info msg="App "58eadf56-b98b-48fd-9121-a26d76f1a3b3" (vmirs-chaos-app) on device "edge-dev" is RUNNING"
TEST     time="2026-08-04T02:22:04Z" level=info msg="Injecting fault: eve exec kube kubectl -n eve-kube-app patch vmirs vmirs-chaos-app-58ead-0 --type=merge -p '{"spec":{"replicas":0}}'"
TEST     time="2026-08-04T02:22:06Z" level=info msg="Waiting for: app reports a retryable warning for the stranded VMIRS..."
HARNESS  time="2026-08-04T02:22:32Z" level=info msg="App "58eadf56-b98b-48fd-9121-a26d76f1a3b3" (vmirs-chaos-app) on device "edge-dev" state changed: RUNNING -> BOOTING"
HARNESS  time="2026-08-04T02:22:32Z" level=warning msg="App "58eadf56-b98b-48fd-9121-a26d76f1a3b3" (vmirs-chaos-app) on device "edge-dev" error: domain 58eadf56-b98b-48fd-9121-a26d76f1a3b3.1.1 vmirs vmirs-chaos-app-58ead-0 scaled to 0 replicas"
TEST     time="2026-08-04T02:22:32Z" level=info msg="Waiting for: app recovered to RUNNING with no error..."
HARNESS  time="2026-08-04T02:23:16Z" level=info msg="App "58eadf56-b98b-48fd-9121-a26d76f1a3b3" (vmirs-chaos-app) on device "edge-dev" state changed: BOOTING -> RUNNING"
...
--- PASS: TestVMIRSStrandedReplicasRecovery (829.92s)
PASS
ok  	github.com/lf-edge/eve/evetest/tests/cluster	829.971s

@andrewd-zededa
andrewd-zededa marked this pull request as ready for review August 4, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

next-17.0.x-rc PR must be present in the next 17.0.x-lts release stable Should be backported to stable release(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants