kube: wait for longhorn instance-manager before declaring storage ready

[eriknordmark]

Description

Fixes #6258.

An EVE-k node reports cluster storage as ready while it still cannot attach any volume. The first app deployed after a conversion or a fresh install then sits in volume creation for minutes, while its CDI upload pod repeats:

AttachVolume.Attach failed for volume "pvc-<uuid>" :
  rpc error: code = Aborted desc = volume pvc-<uuid> is not ready for workloads

Readiness gated only on the Longhorn DaemonSets. A volume's engine and replica processes run inside the per-node instance-manager pod, which is owned by an InstanceManager CR rather than a DaemonSet, so the existing sweep could not observe it. That pod is normally still pulling longhornio/longhorn-instance-manager — 441,724,959 bytes, the largest image in the Longhorn set — at the moment the DaemonSets report ready. Measured at 9m51s on one device, and over twenty minutes on slower storage.

Both readiness paths now require a running InstanceManager whose spec.nodeID is this node: checkLonghornReady() in pillar, and Longhorn_is_ready() in pkg/kube/longhorn-utils.sh.

Longhorn creates the CR during node setup rather than on first volume request, so waiting on it cannot deadlock against a volume whose own creation is itself gated on storage readiness.

Everything needed was already vendored — no dependency change.

PR dependencies

None. Note this overlaps in area with #6240, which also touches checkLonghornReady() but inside the DaemonSet loop, whereas this adds a check after it. Whichever lands second needs only a trivial rebase.

How to test and validate this PR

Automated: make -C pkg/pillar test covers it. TestInstanceManagerRunningOnNode has 8 subtests — running / starting / error / stopped states, this-node vs another-node, several InstanceManagers with one running, none present — plus TestInstanceManagerRunningOnNodeListError for the API-error path.

On a device, the behavior to confirm is that storage is no longer declared ready early:

1. Install or convert to EVE-k on a node with no pre-pulled Longhorn images, so the instance-manager image has to be downloaded.
2. While the node is coming up, watch kubectl -n longhorn-system get pod -l longhorn.io/component=instance-manager — it will sit in ContainerCreating for several minutes pulling ~440 MB.
3. During that window, confirm the node does not report cluster storage ready: kubectl -n longhorn-system get instancemanagers.longhorn.io -o json | jq '.items[] | {node: .spec.nodeID, state: .status.currentState}' shows a non-running state, and volumes stay unready rather than being offered and failing to attach.
4. Deploy an app with a volume. Before this change the volume attach fails repeatedly with not ready for workloads while the pull is in flight; after it, the deployment simply waits until storage is genuinely ready and then attaches on the first try.

The premature-ready window was characterized across a 7-leg kvm→EVE-k conversion matrix on amd64 (all 7 legs passed — it cost wall-clock time, not correctness), which is where the 9m51s and 20-minute figures come from.

Changelog notes

Fixed an issue where a Kubernetes-enabled (EVE-k) node could report cluster storage as ready before it was able to attach volumes, causing the first application deployed on a newly installed or converted node to fail its volume attach repeatedly for several minutes before recovering on its own.

PR Backports

• 17.0-stable: To be backported — checkLonghornReady is present there.
• 16.0-stable: No, the affected code is not present on that branch.
• 14.5-stable: No, the affected code is not present on that branch.
• 13.4-stable: No, the affected code is not present on that branch.

(Verified by checking for func checkLonghornReady in pkg/pillar/kubeapi/kubeapi.go on each branch.)

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.

Reasons for the unchecked boxes: no documentation change — this corrects an existing readiness check rather than adding a knob or a user-visible feature. The pillar half of this patch has now run on an amd64 device (QEMU under Eden) across the full seven-leg kvm→EVE-k conversion matrix, 7/7; arm64 is still untested, and the pkg/kube/longhorn-utils.sh half has not run anywhere, because the branch used for that matrix carries #5971 and #5971 deletes that file. Labels: stable should be added for the 17.0-stable backport.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.52%. Comparing base (cc3374a) to head (041e860).
⚠️ Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/pillar/kubeapi/longhorninstancemanager.go	50.00%	14 Missing ⚠️
pkg/pillar/kubeapi/kubeapi.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6259      +/-   ##
==========================================
+ Coverage   24.13%   24.52%   +0.38%     
==========================================
  Files         512      523      +11     
  Lines       93537    95281    +1744     
==========================================
+ Hits        22575    23363     +788     
- Misses      69177    69942     +765     
- Partials     1785     1976     +191     

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

Marking this draft: as written it regresses the twodisk-zfs kvm→EVE-k conversion leg. Do not merge yet.

Reproduction

Same host, same eden escripts, same topology; the only difference is this change.

image	twodisk-zfs
baseline, without this change	PASS (78m19s)
with this change	FAIL
with this change (independent repeat)	FAIL

No host suspend during either failing run, so the wall-clock step budgets were real. The other six legs (ext4-shrink, ext4-grow, twodisk-ext4, zfs-grow, ext4-toofull, zfs-notail) pass with the change in place.

The leg fails in the escript's "wait for volumemgr ready (incl kubeapi.WaitForKubernetes)" step. VolumeMgrStatus never reaches Initialized:true.

Mechanism

WaitForKubernetes bounds node + kubevirt + Longhorn readiness with a single fixed deadline:

nodeReadyErr = wait.PollImmediate(time.Second, time.Minute*20, func() (bool, error) {
    ...
    if opts.WaitForLonghorn {
        if err := checkLonghornReady(client, nodeName); err != nil {

This change adds "a running InstanceManager on this node" to checkLonghornReady. That condition is correct — it is the whole point, see #6258 — but it is also the slowest thing in the predicate: the instance-manager pod pulls longhornio/longhorn-instance-manager, 441,724,959 bytes, measured at 8m20s / 8m41s on the faster topologies and 20+ minutes on two-disk ZFS. On that topology it alone exhausts the 20-minute budget, so the poll expires and volumemgr never initializes.

Measured on the failing leg: longhorn-manager pull 13m37s, longhorn-ui 12m11s, instance-manager still ContainerCreating at 17m. All Longhorn components did eventually reach Running — the node was healthy, just slower than the ceiling allows.

A diagnostic trap worth flagging: on expiry the reported condition is whichever sub-check happened to be failing at that instant. The failing run recorded daemonset:longhorn-manager not running on node while that pod was in fact Running with both containers ready on the correct node. Do not read that string as the cause.

Intended fix

Raise the ceiling, not the predicate. Reverting the instance-manager condition would restore the premature-ready bug of #6258, where storage reports ready while the pod that serves volumes is still downloading and every volume attach fails with not ready for workloads.

I will push the timeout change and re-test twodisk-zfs, which is the only leg that discriminates.

[eriknordmark]

Regression resolved — undrafting.

What was wrong

The first commit added "a running InstanceManager on this node" to checkLonghornReady. That predicate is correct (#6258), but WaitForKubernetes bounded node + kubevirt + Longhorn readiness with a single fixed 20-minute deadline. On two-disk ZFS the instance-manager's ~440 MB image alone takes over twenty minutes to pull, so the poll expired, volumemgr never initialised, and cluster storage never came up at all.

Measurements

Same host, same eden escripts, same twodisk-zfs topology. Zero host suspends during any of these runs — verified from PM: suspend entry counts around each iteration, because a suspend freezes the VM while the test's wall-clock budgets keep running and would invalidate a verdict.

build	Longhorn ready budget	twodisk-zfs
baseline, no instance-manager gate	20 min	PASS (premature-ready bug present)
gate only	20 min	FAIL 4 / 4
gate + widened budget	45 min	PASS 4 / 4

Per-iteration detail on the fixed build: the instance-manager reached 1/1 Running after roughly 29–32 minutes each time, and step (a) then completed in 40–50 minutes. Zero FailedAttachVolume events across all iterations — the premature-ready failure mode of #6258 did not occur, which is the point of the first commit.

For scale, the host used for these runs was slower than the one that produced the passing baseline (longhorn-ui pulled in 12m8s here versus ~3m35s there), so the fix is holding under worse conditions than the original green run.

Why widen the budget rather than drop the predicate

Reverting the instance-manager condition would restore the bug it exists to fix: storage reports ready while the pod that actually serves volumes is still downloading, so the first app's volume attach fails repeatedly with not ready for workloads before recovering on its own. Two topologies measured that effect directly — the wait relocates from app-start into the readiness step and the retry storm disappears.

The 20-minute ceiling was already tight before this change; adding the slowest component to the predicate is what pushed it over. The widened budget applies only when the caller waits on Longhorn, so non-Longhorn callers keep the original bound.

Diagnostic note for anyone reading a failure of this shape

On expiry the recorded condition is whichever sub-check happened to be failing at that instant, which is misleading. One failing run reported daemonset:longhorn-manager not running on node while that pod was in fact Running with both containers ready on the correct node — the string was written at timeout and never refreshed. Check the live pod state before believing it.

Testing

• go test -tags k ./kubeapi/ — instance-manager state matrix (running / starting / error / stopped, this-node vs other-node, multiple CRs, none present, list error), the gate's effect on checkLonghornReady, and a guard that the Longhorn budget stays larger than the base one.
• go vet -tags k, gofmt -l, shellcheck -x all clean.
• Device: twodisk-zfs 4/4 as above. The other six kvm→EVE-k conversion legs (ext4-shrink, ext4-grow, twodisk-ext4, zfs-grow, ext4-toofull, zfs-notail) passed on the gate-only build and are unaffected by a timeout widening.

[eriknordmark]

Heads-up on an interaction with 6240 that neither PR sees on its own.

#6240 adds pkg/pillar/kubeapi/longhornready_test.go, whose
TestCheckLonghornReadyHealthy and TestCheckLonghornReadyIgnoresStrayDaemonset
call checkLonghornReady with a fake.NewSimpleClientset. #6259 makes
checkLonghornReady end with instanceManagerReady(ctx, nodeName), and the real
gate builds its Longhorn client from the on-device kubeconfig. Under a fake
clientset that file does not exist, so both tests fail with:

longhorn instance-manager: kubeconfig: stat /run/.kube/k3s/k3s.yaml: no such file or directory

No merge order is required — both are based on master and touch disjoint
lines, so they merge cleanly in either order. But whichever lands second must
carry the test fix, or master goes red. instanceManagerReady is a package-level
var, so stubbing it is enough:

// stubInstanceManagerGate satisfies the instance-manager check for tests that
// exercise the daemonset logic: the real gate builds a Longhorn client from the
// on-device kubeconfig, which a fake clientset cannot supply.
func stubInstanceManagerGate(t *testing.T) {
	saved := instanceManagerReady
	t.Cleanup(func() { instanceManagerReady = saved })
	instanceManagerReady = func(context.Context, string) error { return nil }
}

called at the top of both tests (plus a "context" import).

Found on an integration branch carrying both PRs; with the stub applied,
go test -tags k ./kubeapi/... passes.

[rene]

@claude

[claude]

Claude finished @rene's task in 1m 52s —— View job

---

Review

Read the two readiness paths (Go checkLonghornReady / WaitForKubernetes, and the Longhorn_is_ready shell gate), the new instance-manager helper, and the tests. The approach is sound — gating readiness on a running InstanceManager for this node closes the real premature-ready window in #6258, and widening the budget only for the Longhorn caller is the right way to absorb the ~440 MB pull without touching non-Longhorn paths. The watchdog stays kicked via stillRunningWatch during the longer wait, so the 45m ceiling is safe.

Two points posted inline:

• pkg/kube/longhorn-utils.sh (~L255) — the shell gate fails open on a transient kubectl error: it only rejects the literal null, so an empty result from a failed query is treated as ready. The DaemonSet check just above fails closed; the IM check should too.
• pkg/pillar/kubeapi/longhorninstancemanager.go (L89) — the gate re-reads kubeconfig and rebuilds the Longhorn clientset on every 1s poll iteration (up to ~2700× over the 45m deadline) instead of reusing the config/client the caller already holds. Efficiency, not correctness.

Neither blocks merge. The Go state matrix is well covered; the instanceManagerReady var seam is a reasonable test hook (and note the cross-PR test-stub coordination with #6240 flagged in the thread).
· lh-instance-manager-ready

[claude]

This gate fails open on a transient error. The check only rejects the literal string null; if the kubectl get instancemanagers call fails (API blip, apiserver restart), jq receives empty input and imState becomes the empty string "", which is != "null", so execution falls through and the node is declared ready.

Contrast with the DaemonSet check above ("$lhStatus" != "truetruetrue"), which fails closed — any non-exact value keeps waiting. For a readiness gate the safe default is to keep waiting unless a running instance-manager is positively observed. Consider requiring a numeric index instead:

if ! printf '%s' "$imState" | grep -qE '^[0-9]+$'; then

confirmed: true

[claude]

checkLonghornReady already receives a built kubernetes.Interface client (and WaitForKubernetes already holds a *rest.Config), yet this gate re-reads the kubeconfig from disk and rebuilds a versioned clientset on every poll iteration. WaitForKubernetes polls at a 1s interval up to the now-45-minute deadline, so on a slow instance-manager pull this does the GetKubeConfig() disk stat + NewForConfig() construction thousands of times before the CR reports running.

Building the Longhorn client once (outside the poll loop, or memoized) and passing the lister in would avoid the repeated work and let the gate be tested through the real checkLonghornReady path rather than requiring the instanceManagerReady var stub. Not a correctness bug, but worth tightening on a device-management path.

[eriknordmark]

Full-matrix result on the shipped form of this change, superseding the "the other six legs passed on the gate-only build" note above: all seven kvm→EVE-k conversion legs pass with the instance-manager gate and the widened budget in place.

Image 0.0.0-newgo-allprs2-dc05be7e, host eden, amd64, bringup 12.1.0, 4h57m38s wall:

leg	result	wall
ext4-shrink	PASS	53m35s
ext4-grow	PASS	47m52s
twodisk-ext4	PASS	46m05s
twodisk-zfs	PASS	79m44s
zfs-grow	PASS	49m55s
ext4-toofull	PASS	13m09s
zfs-notail	PASS	7m18s

PASS=7 FAIL=0 SKIP=0.

kubeapi/longhorninstancemanager.go and its test are byte-identical to this PR's head in that build, so the gate as written here is what ran. Its kubeapi.go additionally carries #6240's stray-daemonset guard, so this measures the two PRs' combination rather than this one alone.

Two limits worth stating plainly:

• The escript's step-(a) budget also moved from 40m to 85m over the same interval, and the run logs no step-(a) elapsed time. So this result cannot separate "the gate now converges" from "the budget now outlasts it" — it shows the combination is green, not which change made it green.
• pkg/kube/longhorn-utils.sh is not exercised by any pkg/kube: port cluster-init.sh to Go daemon #5971-carrying image, since that PR deletes the file. The shell half of this change still has no device coverage.