Unify CreateVersionMetalFromUrl into CreateVersionMetal#5655
Open
pykello wants to merge 2 commits into
Open
Conversation
Fold the URL-source archive prog into `CreateVersionMetal` as a second class entry point. The new prog exposes two `.assemble_from_*` methods, two archive labels (`archive_from_vm` and `archive_from_url`, byte-for-byte the current bodies), and a shared `#finish` that derives both `archive_size_mib` (from `physical_size_bytes`) and `actual_size_mib` (from `logical_size_bytes`) out of the stats file the host backend already writes for both sources. - `.assemble_from_vm` rolls up the source-VM preflight (arch / metal / single volume / stopped / `track_written` / `key_encryption_key_1` / size cap) and writes a strand at label `archive_from_vm`. Behavior is unchanged from the old `CreateVersionMetal.assemble` except that `actual_size_mib` is no longer pre-computed from `source_vm.storage_size_gib`; `#finish` reads `logical_size_bytes` from the stats file instead, so both source paths derive size the same way. - `.assemble_from_url` picks a `VhostBlockBackend` host with archive support and writes a strand at label `archive_from_url`. Behavior is unchanged from the old `CreateVersionMetalFromUrl.assemble`. The destroy prog (`DestroyVersionMetal`) is untouched; this commit only collapses the two create entry points. The single caller of `CreateVersionMetalFromUrl` in tree (`helpers/machine_image.rb`'s `.assemble` call moves to `.assemble_from_vm`) and the `spec/thawed_mock` `allow_mocking` entry are updated; `prog/machine_image/create_version_metal_from_url.rb` and its spec are deleted. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The `archive_from_vm` / `archive_from_url` labels currently loop forever when the host-side daemon reports `Failed` or an unexpected status — the row stays at `status='creating'`, the strand keeps waking the scheduler, the R2 prefix keeps any partially-uploaded objects, and there is no signal a user or operator can react to. After 5 consecutive non-progressing daemon states (`Failed` plus the "Unexpected daemonizer2 status" else branch), the prog now hops to a new `cleanup` label, which buds `DestroyVersionMetal` at its `destroy_objects` entry point and waits for it to pop before the create prog itself pops with `"Metal machine image version archive failed"`. Why bud rather than call `DestroyVersionMetal.assemble`: the parent strand is at `metal.id`, so a sibling assemble would collide on the primary key. Bud creates a child strand with a fresh id whose frame carries `subject_id = metal.id`, and `DestroyVersionMetal`'s `subject_is :machine_image_version_metal` resolves through that subject_id transparently. We enter at `destroy_objects` rather than the assemble preflight because the row we're abandoning is still at `status='creating'`, has no billing record (created in `#finish`), and was never wired into `latest_version_id` — so the assemble preflight is a no-op for this row anyway. `ARCHIVE_MAX_RETRIES = 5` is a constant for now; it can be lifted to `Config` later if we discover real-world archive paths that legitimately need more headroom. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fold the URL-source archive prog into
CreateVersionMetalas a second class entry point. The new prog exposes two.assemble_from_*methods, two archive labels (archive_from_vmandarchive_from_url, byte-for-byte the current bodies), and a shared#finishthat derives botharchive_size_mib(fromphysical_size_bytes) andactual_size_mib(fromlogical_size_bytes) out of the stats file the host backend already writes for both sources..assemble_from_vmrolls up the source-VM preflight (arch / metal / single volume / stopped /track_written/key_encryption_key_1/ size cap) and writes a strand at labelarchive_from_vm. Behavior is unchanged from the oldCreateVersionMetal.assembleexcept thatactual_size_mibis no longer pre-computed fromsource_vm.storage_size_gib;#finishreadslogical_size_bytesfrom the stats file instead, so both source paths derive size the same way..assemble_from_urlpicks aVhostBlockBackendhost with archive support and writes a strand at labelarchive_from_url. Behavior is unchanged from the oldCreateVersionMetalFromUrl.assemble.The destroy prog (
DestroyVersionMetal) is untouched; this commit only collapses the two create entry points.The single caller of
CreateVersionMetalFromUrlin tree (helpers/machine_image.rb's.assemblecall moves to.assemble_from_vm) and thespec/thawed_mockallow_mockingentry are updated;prog/machine_image/create_version_metal_from_url.rband its spec are deleted.