Skip to content

fix(aws-healthomics): reconcile WDL migration Phase 1 with the ECR pull-through cache SOP - #178

Merged
nadetastic merged 1 commit into
kirodotdev:mainfrom
hmkim:fix/container-strategy-contradiction
Aug 6, 2026
Merged

fix(aws-healthomics): reconcile WDL migration Phase 1 with the ECR pull-through cache SOP#178
nadetastic merged 1 commit into
kirodotdev:mainfrom
hmkim:fix/container-strategy-contradiction

Conversation

@hmkim

@hmkim hmkim commented Aug 4, 2026

Copy link
Copy Markdown

What

Reconciles the WDL migration guide's Phase 1 with the ECR Pull Through Cache SOP in the same Power. Today the two files prescribe opposite outcomes, and Phase 1 does not reference the other SOP or any of the eight container MCP tools it documents.

Also strengthens the container verification criterion from accessible to contains the tools the task invokes, because accessibility is not the property that makes a task run.

Only migration-guide-for-wdl.md is touched. The ECR SOP is unchanged — it is the file I believe is correct.

1. The two files require opposite end states

ecr-pull-through-cache.md:15:

Migrating existing workflows: Use container registry maps to avoid changing container URIs in workflow definitions.

ecr-pull-through-cache.md:135:

IF migrating existing workflows that reference public container URIs you MUST create a container registry map (preferred) ALTERNATIVELY you MAY replace the existing container URIs with the new ECR Private URIs

But migration-guide-for-wdl.md:50 makes this a completion criterion for migration:

  • Zero references to external registries remain.

Using a registry map means quay.io/... stays in the WDL — that is the entire point of the map. The migration SOP's Done WHEN therefore defines the preferred approach's end state as failure. An agent that satisfies Phase 1 as written cannot have used a registry map, and an agent that follows the ECR SOP cannot satisfy Phase 1.

Phase 1 also instructs the agent to author scripts/migrate_containers_to_ecr.sh doing manual docker pull / tag / docker push (:35-39), while the same Power documents CreatePullThroughCacheForHealthOmics, CloneContainerToECR, CreateContainerRegistryMap, CheckContainerAvailability, and four more (ecr-pull-through-cache.md:24-33). Phase 1 mentions none of them and does not link to that SOP.

Change — Phase 1 step 3 makes the approach an explicit per-container choice, step 4 routes staging through the MCP tools and links the ECR SOP, and the Done WHEN bullets are split so each approach has its own criterion. Zero references to external registries remain is preserved, scoped to the URI-replacement case.

The Container Runtime (Before/After) pattern showed only the URI-replacement outcome, which would have contradicted the new preference, so it now shows both — the registry-map "After" leaves docker: unchanged and adds only cpu/memory.

2. Which approach applies when

Rather than declaring one approach correct, I added a decision rule to ## Technical Patterns — transport vs. identity. It is what makes both files right about different cases:

  • Same repository and tag, fetched from closer by → map, WDL untouched. The image is byte-identical, so the redirect hides nothing.
  • A different repository (so a different tool set or different versions) → edit the WDL. The command block's correctness now depends on which image is pulled. A task whose runtime says bwa while a map silently redirects it elsewhere runs something other than what the definition says, and that is invisible in the workflow definition.

This distinction is also measurable, and one run shows both halves at once. A workflow with mixed references was registered without --container-registry-map:

workflow 2238961   ACTIVE       registration accepted it
run      9421504   FAILED

  bwa_mem_align       COMPLETED   (carried a rewritten ECR URI — needed no map)
  sort_and_index      never ran   (carried quay.io/... — needed the map)

statusMessage:
  ECR image URI: quay.io/biocontainers/samtools:1.21--h50ea8bc_0 has an
  invalid structure. Provide a valid ECR image URI and retry.

Three things worth noting about that shape, all now stated in step 4:

  • CreateWorkflow returned ACTIVE. The registry map is a workflow attribute supplied at creation, and registration did not validate the definition against it. A workflow that names public registries registers cleanly and is unrunnable. (I've worded this as "does not appear to validate" in the steering text, since this is one observation rather than documented behavior.)
  • The failure is not fail-fast. Storage was provisioned, an image pulled, bwa ran to completion and wrote its BAM — then the run died on the next task's URI. Everything before it was billed.
  • The message names samtools, and nothing is wrong with the samtools reference. It is a valid public quay.io URI. What is missing is a workflow-level attribute unrelated to samtools.

3. Accessible is not the same as contains

workflow-development.md:60 states the actual requirement:

Containers MUST contain all software used in the script/command.

No procedure in the Power checks it. migration-guide-for-wdl.md:51 offers instead:

  • At least 5 key containers verified accessible from ECR.

Accessibility is a different property, and "at least 5" is an arbitrary sample of an inventory the same phase just enumerated in full.

Measured — quay.io/biocontainers/bwa:0.7.18--he4a0461_0:

/usr/local/bin/bwa
samtools: NOT PRESENT

The task using it pipes bwa mem | samtools view -bS, so it dies at the pipe with samtools: command not found. Everything upstream passed honestly: aws ecr describe-images OK, miniwdl check OK, CreateWorkflow ACTIVE. The image genuinely exists — that is why nothing objects. CheckContainerAvailability cannot see this either; it reports availability, as named.

The only check that answers the question is running the image:

docker run --rm <image> sh -lc 'command -v bwa samtools bcftools'

Swapping to a multi-tool mulled-v2-* image fixes it, and that is a different repository — so by the rule in §2 it belongs in the WDL, not in a map. (In our case that also shifted samtools 1.21 → 1.20, which is the kind of thing worth recording in the inventory.)

Change — the Done WHEN bullet becomes per-task command coverage rather than a count of accessible images. Step 5 gives the docker run check, a heuristic for deriving the binary list from the command block (so the criterion is executable rather than aspirational), what to do when a binary is absent, and why no static gate substitutes for it. Step 6 covers the multi-tool image case.

Scope

migration-guide-for-wdl.md only. Not changed: ecr-pull-through-cache.md, the Nextflow migration guide (the same Phase 1 pattern likely applies there, but I have not measured it), and resource limit values.

No file overlap with #175. That PR covers a different axis — registries that pull-through cache does not support, staged via Registry Staging Mapping — whereas this one is about choosing between a registry map and URI replacement for PTC-supported registries. If #175 merges first I'm happy to add a pointer from Phase 1 step 4 to image-staging.md for the non-PTC case, or to rebase in whichever order suits you.

Measurement conditions

2026-07, ap-northeast-2, two accounts (one with no pre-existing HealthOmics resources), miniwdl 1.15.0 / Python 3.12.13. AWS account IDs are masked as 123456789012 where they appeared in output; run and workflow IDs are left as-is since they are meaningless without the account.

…he ECR SOP

migration-guide-for-wdl.md Phase 1 required "Zero references to external
registries remain" as a completion criterion, but ecr-pull-through-cache.md
states that migrating workflows SHOULD use a container registry map
(preferred) precisely so container URIs do NOT change. A map leaves the
public URIs in the WDL, so the migration SOP defined the preferred
approach's end state as failure. Phase 1 also prescribed hand-written
docker pull/push scripts without referencing the eight container MCP tools
documented in the same Power.

Make the approach an explicit per-container choice, route staging through
the MCP tools, link the ECR SOP, and split the Done WHEN criteria so each
approach has its own. Add a transport-vs-identity decision rule to
Technical Patterns: a map is right where the same repository and tag is
served from private ECR, and the WDL must be edited where the repository
changes, since the command block's correctness then depends on which image
is pulled. Show both outcomes in the Container Runtime pattern, which
previously showed only the URI-replacement result.

Also replace "At least 5 key containers verified accessible from ECR" with
per-task command coverage. Accessibility is not containment: an image can
exist, pass describe-images, miniwdl check and CreateWorkflow, and still
lack a tool the command block pipes to, which surfaces only as
"command not found" once the task runs.
@nadetastic
nadetastic merged commit e7095f5 into kirodotdev:main Aug 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants