Skip to content

[Core] Reuse OCI model artifacts by Hugging Face origin#664

Open
op109lvb wants to merge 2 commits into
mainfrom
oci-reuse-latest
Open

[Core] Reuse OCI model artifacts by Hugging Face origin#664
op109lvb wants to merge 2 commits into
mainfrom
oci-reuse-latest

Conversation

@op109lvb

@op109lvb op109lvb commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR adds shared node-local artifact reuse for HF-origin OCI Object Storage models by using HF model ID + commit SHA as the artifact identity. It downloads once to a canonical parent, symlinks model-local paths to that parent, tracks parent/child relationships, and adds restart/retry handling so shared parents are recovered or validated safely after model-agent restarts.

More details

Adds node-local reuse for HF-origin OCI Object Storage artifacts when a model CR provides HF model ID, HF commit SHA, ReuseIfExists, and a SHA-scoped OCI Object Storage URI.

Model-agent now treats <hf-model-id>/<sha> as the shared artifact identity:

  • Downloads the artifact once into _artifacts/<hf-model-id>/<sha>.
  • Creates each model-local path as a symlink to that shared parent.
  • Records parent/child relationships in the node ConfigMap.
  • Avoids reusing legacy/self-parent entries that do not have shared parent metadata.

It also adds lifecycle handling for the shared parent:

  • Deleting one child only removes that child symlink.
  • Deleting the last child removes the shared parent when no references remain.
  • DownloadOverride repairs/redownloads the shared parent, since all children point to the same files.

Restart and concurrency safety is included:

  • Uses the synthetic HF parent ConfigMap Updating state to ensure only one worker validates or rebuilds a shared parent at a time.
  • On model-agent startup, recovers synthetic HF parent entries left in Updating: if the ready marker exists, marks the parent Ready; if missing, marks it Failed so later child tasks can rebuild.
  • Validates each Ready HF parent once after model-agent restart.
  • Retries recovery/validation on transient ConfigMap read/update failures.

Legacy behavior is preserved for model CRs that do not provide the HF reuse metadata. Backfill is out of scope.

Why we need it

Multiple model CRs can point to the same HF model revision after the artifact has been copied into OCI Object Storage. Reusing the node-local artifact by HF model ID + SHA avoids duplicate downloads and saves node disk space, while still preserving repair behavior for corrupted files.

Fixes #

How to test

go test ./pkg/modelagent ./pkg/modelparser

Run model-agent unit tests covering HF parent path selection, symlink reuse, child tracking, delete cleanup, startup recovery, startup validation, and DownloadOverride repair behavior.

E2E: create two model CRs with the same HF model ID + SHA, ReuseIfExists, and the same SHA-scoped OCI Object Storage URI. Verify the first model downloads to _artifacts/<hf-model-id>/<sha> and the second model-local path is a symlink to the same parent.

E2E coverage:

  • First CR with HF model ID + SHA downloads OCI Object Storage artifact to _artifacts/<hf-model-id>/<sha>.
  • Second CR with same HF model ID + SHA creates a symlink to the same shared parent.
  • Delete one child CR: shared parent remains.
  • Delete last child CR: shared parent is cleaned up.
  • Existing legacy CRs without shared HF metadata keep old behavior.
  • Restart model-agent: stale Updating parent entries recover or fail safely.
  • Restart model-agent: Ready HF parent is validated once before reuse.
  • Corrupted shared parent: DownloadOverride redownloads the parent artifact.
  • ClusterBaseModel and BaseModel both preserve expected DownloadOverride behavior.

Checklist

  • Tests added/updated (if applicable)
  • Docs updated (if applicable)
  • make test passes locally

@github-actions github-actions Bot added model-agent Model agent changes tests Test changes labels Jul 13, 2026
}

func canonicalHuggingFaceArtifactPath(destPath string, identity ArtifactIdentity) string {
return filepath.Join(filepath.Dir(destPath), constants.ModelArtifactsDirectory, filepath.FromSlash(strings.Trim(strings.TrimSpace(identity.HFModelID), "/")), strings.ToLower(identity.HFCommitSHA))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we add sanitization against ../ sequences

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HF model IDs should not contain those sequences per Hugging Face repo ID rules, and upstream should already validate that. I still addressed this by adding model-agent-side validation as defense-in-depth before using the annotation to build local artifact paths.

if task != nil && task.BaseModel != nil {
return canonicalHuggingFaceArtifactPath(destPath, identity)
}
return filepath.Join(modelRootDir, filepath.FromSlash(strings.Trim(strings.TrimSpace(identity.HFModelID), "/")), strings.ToLower(identity.HFCommitSHA))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

@op109lvb
op109lvb requested a review from EdHasNoLife July 15, 2026 00:53
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