[zoroyihan7]

Why

A local model today depends on S3: without it the model fails within seconds ("S3 storage is not enabled"), and deleting a model never removes its files. The cleanup job runs in the control-plane namespace without the workspace volume mounted, and three code paths release the finalizer even when cleanup fails. So the Model resource cannot be the single owner of model weights.

This PR makes it one: the Model downloads, records and removes its weights, with or without S3.

What changes

1. No S3 needed for local models. With S3 disabled, handlePending goes straight to Downloading. The per-workspace download job fetches from HuggingFace directly into the workspace storage path (model.downloader_image, huggingface-cli download, with a fallback to hf download). The S3-staged flow is unchanged when S3 is enabled.
2. SECRET is optional in the download OpsJob webhook and in the download controller, so public models need no token. When the model has a token, the token Secret is shared with the target workspace (workspace.ids annotation) so the job can mount it.
3. The download only fetches what is needed to load the model: *.safetensors, *.json, tokenizer*, *.model, *.tiktoken, *.txt, *.py, *.jinja, excluding original/*. The HuggingFace cache sits under the model directory, so a large model does not fill the container's ephemeral storage and a retry resumes. Nothing is deleted on failure; deletion is the model's job. The script prints MODEL_SIZE_BYTES=<n>, which ends up in status.localPaths[].sizeBytes.
4. Deleting a model removes its files, and never releases early:
   • Downloads are stopped first: the S3 upload Job, the download OpsJobs and their workloads must be gone before cleanup starts.
   • Each status.localPaths entry is removed by a new model-cleanup OpsJob. It runs in the entry's workspace, so the volume is mounted, and it runs as the model owner. It is handled by the existing download controller and is not exposed through the OpsJob API. The entry leaves status.localPaths only after its directory is gone.
   • The three "cleanup failed, release anyway" paths now keep the finalizer and retry (failed job deleted, status.message says why, retry after 30s). The same applies to the S3 cleanup Job, which now fails when aws s3 rm fails.
   • Guards:
     • Only an absolute, clean path with a segment below a models directory, inside a workspace volume and not the volume root, is ever removed. This is checked in the controller, in the cleanup script, and in the OpsJob webhook.
     • A directory that another live model still points at is kept.
     • The webhook refuses a cleanup while a download or cleanup on the same path is still running.
5. No new model on a path that is still being cleaned up:
   • The API rejects such a create with 409 / ResourceProcessing.
   • The controller re-checks before starting: a pending model waits while another model's deletion holds its directory, and fails if a live model already owns the directory.
6. Jobs run as the model owner (user id and name labels) instead of the system user. Storage that maps users to their own uid then writes and removes files with the owner's identity.
7. Failover stays on the same cluster. It only moves to a workspace on the same cluster that mounts the same volume root as the path. An equal mount path on another cluster is a different filesystem.
8. The list API can show models still being deleted: GET /models?includeDeleting=true adds models whose CR still exists with a deletion timestamp (isDeleted=true, deletionTime, remaining localPaths).
9. Global download concurrency limit: model.max_concurrent_downloads (default 4, negative = unlimited) counts unfinished model download jobs across all workspaces, read from the API server (not the cache). Paths over the limit stay Pending with "Waiting for a download slot (n/m in use)". The download timeout is now model.download_timeout_second (default 10800).

Also:

• HuggingFace models are stored as <owner>--<name>. HuggingFace forbids -- inside a segment, so two repositories can no longer map to the same directory (a-b/c and a/b-c both used to become a-b-c).
• A target.subpath that already contains a models segment is used as the parent directory as is: <volume>/<subpath>/<owner>--<name>. Otherwise the layout is unchanged: <volume>/[subpath/]models/<dir>.
• The download always targets the absolute path recorded in status, so download and cleanup cannot disagree about the directory.
• cleanup_image in the resource-manager config template was rendered under cd: and never read. It is now under model:.

Review follow-up

A review round found three defects, fixed in the third commit:

• A private workspace without a volume got a download target on the container's own filesystem. It now gets none, and the model fails.
• A public model deduplicated workspaces by path alone across clusters. It is now downloaded once per cluster and path.
• A recorded path that failed the cleanup guard released the model. It now keeps the finalizer.

Known limitation, not addressed: the "another model points at this path" checks compare path strings only. On two clusters with the same mount path they are therefore conservative: files are kept or a create waits, but nothing is deleted wrongly.

Status for consumers

status.phase (Ready / Failed), status.localPaths[].{workspace,path,status,sizeBytes}, and includeDeleting on the list API are enough for an external system to tell "available", "failed", and "still occupying storage".

Deployment notes

• The download and cleanup jobs are kind: Job workloads in the target workspace. Clusters with admission that only trusts approved workload creators must allow the Job controller to create their pods.
• The job identity is the model owner. On storage that maps users to their own uid, the owner needs write access to the parent of the model directory. Choose target.subpath so that the parent is writable by that user. The directory itself is created by the job.
• model.cleanup_image (default alpine:3.18) must be pullable in the workspace cluster.

Testing

• Unit tests (fake client): no-S3 download end to end, from create through Ready (path and size) to deletion and release. Also: token mounting, cleanup failure / create failure / S3 cleanup failure keep the finalizer, downloads stopped before cleanup, shared path kept, unsafe paths refused, pending model waits for a deletion on its path, concurrency limit, same-cluster failover, SECRET optional (controller + webhook), entry point only for Model-owned jobs, cleanup webhook validation, API create rejection and includeDeleting, directory naming and layout.
• TestModelLifecycleEnvtest (-tags integration, needs KUBEBUILDER_ASSETS) runs the controller against a real API server with the chart CRDs. It covers the same lifecycle, that sizeBytes survives the CRD schema, and that a failed cleanup keeps the model until a later cleanup succeeds.
• Mutation check: for each key rule (empty storage root, per-cluster dedup, refused path, failure keeps finalizer, shared path, stop before cleanup, concurrency cap, same-cluster failover, optional secret in controller and webhook, wait for deletion in controller and API, no-S3 path, path guard in controller and webhook, CRD field), the rule was broken by hand. The corresponding test went red each time, and the change was reverted.
• The download and cleanup scripts were run as written, with huggingface_hub==0.24.0 (the version in the model-downloader image), against a small public repository:
  • only the listed file types were fetched (no .bin / .h5);
  • the size line was printed;
  • a re-run was a no-op;
  • an unknown repository exits non-zero;
  • cleanup removes the directory and refuses a non-model path.

Not verified on a live cluster: the new controller code has not been deployed. In particular, the following are untested: the launcher entry point in the download image, size reporting via pod logs, and secret mirroring into the workspace.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 79.76767% with 209 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../resource-manager/pkg/resource/model_controller.go	79.14%	102 Missing and 69 partials ⚠️
...FE/apiserver/pkg/handlers/model-handlers/models.go	82.70%	12 Missing and 11 partials ⚠️
SaFE/webhooks/pkg/ops_job_webhook.go	76.00%	3 Missing and 3 partials ⚠️
...rce-manager/pkg/ops_job/download_job_controller.go	89.47%	2 Missing and 2 partials ⚠️
SaFE/common/pkg/config/config.go	50.00%	2 Missing ⚠️
SaFE/common/pkg/ops_job/log.go	83.33%	1 Missing and 1 partial ⚠️
SaFE/job-manager/pkg/syncer/pod_handler.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!