Skip to content

fix(worker): surface the blocked elastic drive when the world is unprovable - #739

Merged
thxCode merged 1 commit into
mainfrom
fix-738-unprovable-park
Oct 6, 2026
Merged

thxCode merged 1 commit into
mainfrom
fix-738-unprovable-park

Conversation

@thxCode

@thxCode thxCode commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind bug

/area worker

What this PR does / why we need it:

With no durable operation record, an elastic pass that cannot prove the live world finished silently: the observation kept the proof failure and the ElasticResize condition leveled to True/NoRefusal, so the deployment read healthy while the engine and the pod set held a width the spec no longer asks for. No condition changed, no record was created, and no later pass had any more drive than the one that parked, so the wedge held indefinitely — observed live with desired=2 against a live width of 4 while the effective layer reported "native rank identities are unknown".

The unprovable park is now loud when it matters. With no record and a live width that disagrees with the spec, or cannot be named at all, the pass writes the ElasticResize condition Unknown with a new WorldUnprovable reason whose message carries the desired width, the observed width and the proof that failed. Unknown because nothing was refused, and a distinct reason because True/NoRefusal is the healthy state that kept the park invisible. A live width that agrees with the spec owes no corrective scale and parks as quietly as before, and the observation reason keeps the proof's own failure either way.

The blocked state acts on nothing — no record, no engine command, no member touched — because acting on unproven ranks is the safety design. Recovery is ordinary: a pass that proves the world reaches the corrective block, recreates the resize, and levels the condition back through the record-driven writer. No timers and no requeue changes; the ordinary observation cadence re-drives the pass. Both directions are pinned by tests: the blocked surfacing for an unknown live width and for a wider live width that fails its forward proof, and the level-back once the world proves again.

Which issue(s) this PR links to:

Fixes #738

Special notes for your reviewer:

  • The condition vocabulary gains a third state (Unknown/WorldUnprovable) beside False/ScaleRefused and True/NoRefusal; it is declared and documented beside the convergence that decides it, and nothing outside the elastic reconcile reads the condition, so no consumer changes.
  • The two early returns this touches are the ones behind the re-probe (#734); the re-probe itself is untouched, and a narrower or unknown live world keeps its hold — it now just says so on the condition instead of reading healthy.

Does this PR introduce a user-facing change?

Fixed a silent park where a lost elastic resize record left the engine and the pod set wider than the spec forever: the ModelDeployment ElasticResize condition now turns Unknown with the WorldUnprovable reason, carrying the desired width, the observed width and the failed proof, until a pass proves the live world again and the corrective resize is recreated.

…ovable

With no operation record, an elastic pass that cannot prove the live world
finished silently: the observation kept the proof failure and the ElasticResize
condition leveled to True/NoRefusal, so the deployment read healthy while the
engine and the pod set held a width the spec no longer asks for. No condition
changed, no record was created, and no later pass had any more drive than the
one that parked, so the wedge held indefinitely.

The unprovable park is now loud when it matters. With no record and a live
width that disagrees with the spec, or cannot be named at all, the pass writes
the condition Unknown with a new WorldUnprovable reason whose message carries
the desired width, the observed width and the proof that failed. Unknown
because nothing was refused, and a distinct reason because True/NoRefusal is
the healthy state that kept the park invisible. A live width that agrees with
the spec owes no corrective scale and parks as quietly as before, and the
observation reason keeps the proof's own failure either way. The blocked state
acts on nothing - no record, no engine command, no member touched - because
acting on unproven ranks is the safety design. Recovery is ordinary: a pass
that proves the world reaches the corrective block, recreates the resize, and
levels the condition back through the record-driven writer. No timers and no
requeue changes; the ordinary observation cadence re-drives the pass.

Fixes #738

Signed-off-by: thxCode <thxcode0824@gmail.com>
@thxCode thxCode added kind/bug Something isn't working area/worker The worker control plane and its controllers labels Oct 6, 2026
@gpustack-code-review

gpustack-code-review Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 0 comment(s)
  • 📋 Routed to summary by policy: 1 comment(s)

test · low

📄 pkg/worker/controllers/worker/model_deployment_elastic_reconcile.go (L468-L471)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

Reminder (per project rules): this is a user-visible behavior fix in reconcile logic (silent wedge → Unknown/WorldUnprovable condition), so it must ship a regression test in the same PR. Note *_test.go is excluded from this review, so this is a reminder only — no claim is made about whether such a test is present.

@thxCode

thxCode commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Review round summary (all three feedback buckets read to exhaustion: 0 top-level reviews, 0 inline threads, 1 issue comment — the OpenCodeReview summary).

Triaged items: 1.

  1. [test · low] model_deployment_elastic_reconcile.go L468-L471 — reminder that a user-visible reconcile change must ship a regression test in the same PR. Already satisfied, no change needed: the PR ships TestAnUnprovableWorldSurfacesTheBlockedDrive in model_deployment_elastic_reconcile_test.go, covering both blocked shapes (live width disagrees with the spec, and live width unreadable) plus the quiet park when the live width matches. The finding itself states no claim about test presence since *_test.go is outside its review scope. Verified: go test ./pkg/worker/controllers/worker/ -run TestAnUnprovableWorldSurfacesTheBlockedDrive -count=1 green on this head, and CI unit (amd64 + arm64) pass.

Fixed: 0. Not fixed (already satisfied): 1. Deferred: 0.

@thxCode
thxCode merged commit f8416e8 into main Oct 6, 2026
9 checks passed
@thxCode
thxCode deleted the fix-738-unprovable-park branch October 7, 2026 00:22
thxCode added a commit that referenced this pull request Oct 7, 2026
…apes (#740)

The ElasticResize condition and the behaviors that merged in #727, #732,
#737 and #739 had no documentation. Add the condition to the status
vocabulary and describe the refusal-retry bound, the two self-healing
stale shapes, and the diagnosis entry in Elastic EP.

Signed-off-by: thxCode <thxcode0824@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/worker The worker control plane and its controllers kind/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: elastic width parks away from spec forever when the op record is lost mid-transition

1 participant