Skip to content

fix(worker): let the ConfigMap owner watch enqueue elastic state records - #722

Merged
thxCode merged 1 commit into
mainfrom
fix-721-elastic-wedge
Oct 6, 2026
Merged

thxCode merged 1 commit into
mainfrom
fix-721-elastic-wedge

Conversation

@thxCode

@thxCode thxCode commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind bug
/area worker

What this PR does / why we need it:

After an operator restart, a native elastic DP resize in progress froze: membership stayed pinned and a follow-up scale-down patch produced no terminations for 30+ minutes (#721, measured in the W09 8-GPU evidence).

Root cause: the controller watches ConfigMaps through Owns(&core.ConfigMap{}), but the ownership predicate modelDeploymentOwnedResource only accepted systemmeta resource notes. Elastic operation records carry gpustack.ai/elastic-operation=true and no resource note; observation records carried only the owner reference. Neither passed the predicate, so persisted elastic state changes after a restart could not enqueue the owning ModelDeployment.

Fix: label new observation records with gpustack.ai/elastic-observation=true and accept both elastic state labels in the predicate. No API or CRD shape changes.

Regression coverage: the ownership predicate test now requires both elastic state objects to be accepted (RED on the unmodified predicate, GREEN with the fix; full pkg/worker/controllers/worker package suite passes).

Scope note: the same evidence window also logged intermittent embedded extension API handler timeouts. Those are temporally correlated but not proven to share this cause; this PR fixes the proven reconcile wake-up defect only.

Which issue(s) this PR links to:

Fixes #721

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixed a freeze of native elastic data-plane resizing after an operator restart: persisted elastic state ConfigMaps now pass the controller's ownership watch, so a resize in progress resumes after restart.

The ModelDeployment controller watches ConfigMaps through Owns(), but the
ownership predicate only accepted systemmeta resource notes. Elastic
operation records carry gpustack.ai/elastic-operation=true and observation
records carried no marker at all, so after an operator restart, persisted
elastic state changes could not enqueue the owning ModelDeployment and a
resize in progress froze (#721).

Label observation records with gpustack.ai/elastic-observation=true and
accept both elastic state labels in the predicate.

Signed-off-by: thxCode <thxcode0824@gmail.com>
@gpustack-code-review

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

Copy link
Copy Markdown

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

  • ✅ Successfully posted inline: 0 comment(s)
  • ❌ Failed to post inline: 1 comment(s)

bug · medium

📄 pkg/worker/controllers/worker/model_deployment_elastic_reconcile.go (L114-L118)

⚠️ GitHub could not post this as an inline comment: Lines 114-118 could not be resolved (outside PR diff hunks)

The label is only set on the create path. Observation ConfigMaps that already exist (e.g. persisted by a previous version before this fix) are updated via the r.Client.Update path at the bottom of this function without ever receiving the elasticObservationLabel, so modelDeploymentOwnedResource keeps filtering them out and the exact restart-freeze bug this PR fixes (#721) still reproduces for pre-existing deployments after an upgrade. Backfill the label on the update path (and include it in the change comparison so the update actually writes it), e.g. ensureLabels(cm) before the equality short-circuit. Per repo rules, a bug fix must ship a regression test in the same PR — please verify one covers the upgrade path (existing unlabeled CM gets labeled).

💡 Suggested Change

Before:

	if cm.Data["observation.json"] == string(body) {
		return nil
	}
	cm.Data = map[string]string{"observation.json": string(body)}
	return r.Client.Update(ctx, cm)

After:

	if cm.Data["observation.json"] == string(body) && cm.Labels[elasticObservationLabel] == "true" {
		return nil
	}
	if cm.Labels == nil {
		cm.Labels = map[string]string{}
	}
	cm.Labels[elasticObservationLabel] = "true" // backfill for records created before this label existed
	cm.Data = map[string]string{"observation.json": string(body)}
	return r.Client.Update(ctx, cm)

@thxCode
thxCode merged commit 36c25c1 into main Oct 6, 2026
9 checks passed
@thxCode
thxCode deleted the fix-721-elastic-wedge branch October 6, 2026 00:06
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.

bug: elastic EP control plane wedges after operator restart mid-resize

1 participant