Skip to content

[Core] Make model-agent reconciliation race-safe#680

Open
fredericsun wants to merge 1 commit into
mainfrom
fix-model-agent-race-condition
Open

[Core] Make model-agent reconciliation race-safe#680
fredericsun wants to merge 1 commit into
mainfrom
fix-model-agent-race-condition

Conversation

@fredericsun

Copy link
Copy Markdown
Collaborator

What this PR does

This PR makes model-agent ConfigMap, in-memory cache, and periodic self-healing reconciliation safe under concurrent model updates and deletion.

Key changes:

  • Apply status, metadata, progress, and delete operations to the latest ConfigMap using per-model-key mutations.
  • Retry ConfigMap Conflict and AlreadyExists errors.
  • Preserve unrelated model entries and existing status, metadata, and progress fields during partial updates.
  • Track the Kubernetes resource UID associated with each cached model.
  • Fence a terminally deleted CR generation so delayed status, metadata, progress, or self-healing operations cannot restore it.
  • Evict deleted models from the in-memory cache so periodic reconciliation cannot restore their ConfigMap entries.
  • Revalidate copied reconciliation snapshots before restoring them, including after ConfigMap update conflicts.
  • Distinguish terminal CR deletion from node-local cleanup:
    • Terminal deletion fences the deleted UID.
    • Node-local cleanup only evicts the cache entry, allowing the same CR UID to become eligible for the node again.
  • Avoid installing a broad UID fence when an unexpected empty UID is received.
  • Preserve full ConfigMap recreation when the entire ConfigMap is missing.

Why we need it

Each model-agent stores multiple model entries in one node-level ConfigMap. Status, metadata, progress, deletion, and periodic self-healing can update that ConfigMap concurrently.

Previously, these operations could race because they were based on independently fetched ConfigMap snapshots.

For example:

  1. A delete operation and a status update read the same ConfigMap version.
  2. The delete operation removes model A and writes successfully.
  3. The status operation retries using data derived from its older snapshot.
  4. That older snapshot still contains model A, so the deleted entry can be restored while updating another model.

A similar race could occur when periodic self-healing copied a cache entry before deletion, then attempted to restore it after deletion had removed the entry.

A ConfigMap update conflict during deletion could also leave the entry present because the delete path did not retry against the latest ConfigMap.

These races can leave stale node state behind, prevent model deletion from converging, and block controllers waiting for every node entry to be removed.

The new implementation retries mutations against the latest ConfigMap, limits each operation to its intended model key, and uses the model UID plus live-cache ownership to reject stale work.

It also preserves an important node-local workflow. If a CR temporarily stops matching a node and later becomes eligible again, the same CR generation must be allowed to write its state back. Therefore, only CRs with a deletion timestamp receive a persistent UID fence.

No download scheduling or Gopher queue behavior is changed by this PR.

How to test

Unit tests

Run:

go test ./pkg/modelagent

The model-agent tests cover:

  • ConfigMap conflict retry during deletion.
  • Concurrent per-key mutations without restoring another deleted model.
  • Delayed status, metadata, and progress updates from a deleted UID.
  • Allowing a new UID with the same model ID to write normally.
  • Node-local cleanup followed by the same UID becoming eligible again.
  • Blocking a stale periodic-reconciliation snapshot after cleanup.
  • Empty-UID cleanup without installing a broad fence.
  • Preserving existing ConfigMap fields during partial updates.
  • Full ConfigMap recreation when the ConfigMap is missing.

Live model-agent validation

Validated in a live cluster with the PR model-agent image:

  1. Deleted a model while Gopher was actively downloading it.

    • The download was cancelled.
    • A delayed failure/status callback did not restore the ConfigMap entry.
    • The entry remained absent for more than six minutes, including a periodic reconciliation cycle.
  2. Changed a model's node selection from node A to node B and then back to node A.

    • The old node removed its local state.
    • The same CR UID successfully wrote state again when it became eligible for node A.
    • No permanent fence was installed by node-local cleanup.
  3. Reconciled two models concurrently in the same node ConfigMap.

    • Both entries reached Ready with metadata.
    • An unrelated existing entry remained unchanged.
    • Concurrent deletion removed both test entries.

E2E regression validation

Created and deleted a imported model in a live cluster

Verified that:

  • CP created the imported model successfully.
  • OME created and processed the corresponding BaseModel.
  • Five model-agent nodes recorded the model entry.
  • CP transitioned the imported model to DELETED.
  • The OME BaseModel disappeared.
  • All related node ConfigMap entries disappeared.
  • The BaseModel and ConfigMap entries remained absent for more than seven minutes, covering the five-minute periodic reconciliation interval.
  • No stale restore or related model-agent error was observed.

Checklist

  • Tests added/updated (if applicable)
  • Docs updated (not applicable; no user-facing API or configuration changes)
  • make test passes locally

@github-actions github-actions Bot added model-agent Model agent changes tests Test changes labels Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model-agent Model agent changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants