feat(orion): catalog VM images in RustFS/mono and pick by image_id - #2185
Conversation
Upload built qcow2 blobs to object storage, register metadata for list/delete, resolve image_id to a signed URL on start-runner, and stop needrestart from SIGTERM-killing overnight VMs.
Split catalog UI under Orion Client, add presigned PUT + register flow, and set RustFS CORS for local moon origins.
Allow build-custom-image to upload/register across envs, avoid Start Runner hangs behind the update lock, and sign Orion image URLs with an optional public RustFS host so out-of-cluster schedulers can download.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cd7182183
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| PUBLISH_SOURCE="$PUBLISHED_IMAGE" | ||
| if [ ! -f "$PUBLISH_SOURCE" ]; then | ||
| PUBLISH_SOURCE="$CUSTOM_IMAGE" |
There was a problem hiding this comment.
When an existing flat image is locked by a running VM, the publish block deliberately skips overwriting PUBLISHED_IMAGE, and NEW_DIGEST is subsequently computed from the newly built CUSTOM_IMAGE; this selection still chooses the existing old PUBLISHED_IMAGE merely because it exists. Stage 8 therefore uploads old bytes under the new image's digest and registers that mismatched digest, causing qlean's integrity verification to reject every runner start using the new catalog entry. Use CUSTOM_IMAGE after a skipped publish, or compute the digest from the selected upload source.
Useful? React with 👍 / 👎.
| if let Some(client) = state.orion_scheduler_client() | ||
| && let Ok(list) = client.list_vms().await |
There was a problem hiding this comment.
When the configured scheduler's /status request times out, returns an error, or produces invalid JSON, let Ok(list) silently skips the in-use check and proceeds to delete both storage objects and the catalog row. Thus a transient scheduler outage turns an operation documented to return 409 for referenced images into a destructive success; propagate the lookup failure instead of treating it as an empty VM list.
Useful? React with 👍 / 👎.
| let image_name = req | ||
| .image_name | ||
| .as_deref() | ||
| .map(str::trim) | ||
| .filter(|s| !s.is_empty()) | ||
| .unwrap_or("debian-13-buck2"); | ||
| let (object_key, info_object_key) = OrionVmImageService::object_keys(&digest, image_name); |
There was a problem hiding this comment.
When an admin selects a qcow2 whose derived image name contains # or ?, the unvalidated name is interpolated directly into this URL. XMLHttpRequest treats those characters as a fragment or query delimiter, so mono stores the upload under a truncated key while registration retains the full returned object_key; subsequent signed GETs target an object that does not exist. Percent-encode path segments or restrict image names to a safe character set before constructing both keys and URLs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae8f131a1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| for f in "$SCRIPT_DIR/bin/orion-scheduler" \ | ||
| "$SCRIPT_DIR/etc/target_config.json.template" \ | ||
| "$SCRIPT_DIR/etc/needrestart-orion-scheduler.conf" \ |
There was a problem hiding this comment.
Every bundle produced by the Stage bundle section of .github/workflows/orion-scheduler-release.yml copies the binary, target config, systemd unit, and installer, but not etc/needrestart-orion-scheduler.conf. This newly required preflight entry therefore makes install.sh exit with “missing bundle file” for every published release; stage the new config in the bundle or make this optional.
Useful? React with 👍 / 👎.
| let deleted = svc | ||
| .delete(&id) | ||
| .await |
There was a problem hiding this comment.
When an admin starts a runner and deletes its selected image concurrently, startup can resolve the catalog row and generate its signed URL before the scheduler has recorded the VM; deletion then observes no matching VM and removes the object here. The accepted runner subsequently fails when qlean opens the now-missing URL, despite the endpoint's advertised in-use protection. Mark or lock the catalog row before signing so deletion and startup cannot pass each other.
Useful? React with 👍 / 👎.
| let mut am = existing.into_active_model(); | ||
| am.object_key = Set(input.object_key); | ||
| am.info_object_key = Set(input.info_object_key); |
There was a problem hiding this comment.
If an admin uploads the same image bytes again under a different editable image name, the digest lookup finds the existing row and replaces object_key, but nothing deletes the prior multi-gigabyte qcow2. Later deletion only removes the new key, leaving the old object permanently orphaned in storage; preserve a canonical key or explicitly clean up the displaced key when it changes.
Useful? React with 👍 / 👎.
| NEW_DIGEST=$(shasum -a 256 "$PUBLISHED_IMAGE" 2>/dev/null | awk '{print $1}') | ||
| if [ -z "$NEW_DIGEST" ]; then | ||
| NEW_DIGEST=$(sha256sum "$PUBLISHED_IMAGE" | awk '{print $1}') |
There was a problem hiding this comment.
On Linux hosts that provide sha256sum but not the optional shasum utility, this assignment fails and set -e -o pipefail terminates the script immediately, so the fallback on the next line is never reached. This breaks the advertised dependency-light MOCK_UPLOAD=1 path; test command availability first or place the shasum attempt in a conditional that is exempt from errexit.
Useful? React with 👍 / 👎.
2 participants
No description provided.