feat(packages): make multi-arch image tag collection atomic in build script

[wuhuizuo]

What

Refactor collect_and_push_multi_arch_images in packages/scripts/build-package-images.sh.tmpl:

• Pre-flight check: only proceed when all images' other-arch images exist (crane digest); if any is missing, skip the whole multi-arch tagging without touching anything (previously each image was checked individually, leaving earlier images already mutated when a later one was missing).
• Atomic apply: multi-arch tags are added for all images together; the result file is updated only after every push succeeds.
• On mid-way failure: no tag rollback — already-created tags are kept, only the result file is discarded (rm -f) so it never records a partially-applied state.

Verification

• shellcheck clean on regenerated scripts (amd64/arm64 profiles).
• Functional tests with a stubbed crane: skip case, mid-failure case, success case.

[ti-chi-bot]

I have already done a preliminary review for you, and I hope to help you do a better job.

Summary
This PR refactors the collect_and_push_multi_arch_images function in the build script template to make multi-arch image tagging atomic. It introduces a pre-flight check to verify all related images exist before tagging, applies all multi-arch tags in a single batch, and only updates the result file upon complete success. On failure, it cleans up the state file but does not rollback tags already pushed. The approach improves consistency and avoids partial updates. The code is well-structured and the logic clear, with appropriate use of shell features like trap.

---

Critical Issues

• Trap usage and error handling (packages/scripts/build-package-images.sh.tmpl, lines ~314-328)
  The rollback function is triggered on any EXIT which includes normal successful exit as well. Since the trap is only removed at the end, this will delete the result file even on success because the trap triggers on function exit.
  Suggested fix: Set the trap immediately before the risky operations, and remove it right after all succeed. Alternatively, use an ERR trap or a flag to detect failure before cleanup. For example:

  rollback() {
    rm -f "$result_file"
  }
  trap rollback EXIT
  
  # risky commands here
  
  trap - EXIT   # disable trap on success

  (Make sure the trap is disabled before normal return so the file is not removed on success.)

• No rollback of tags on mid-way failure
  While the description states no rollback of tags is done on partial failure, this could lead to inconsistent registry state. Consider adding a documented warning or ideally implement rollback logic if feasible.

---

Code Improvements

• Pre-flight missing flag logic (packages/scripts/build-package-images.sh.tmpl, lines ~306-315)
  The missing flag is set as a string "false" / "true". It is more idiomatic and less error-prone to use missing=0 / missing=1 or a shell boolean approach:

  missing=0
  if ! crane digest ...; then
    missing=1
  fi
  if [ "$missing" -eq 1 ]; then
    ...
  fi

• Repeated jq filtering
  The jq filter map(select(.type == "image" and .if != false)) is repeated multiple times. Consider defining it once at the start of the function into a variable to avoid redundancy and improve readability.

• Error handling for crane commands
  The crane index append and crane tag commands assume success. Consider explicitly checking their exit status and aborting/tagging failure accordingly to avoid partial tagging without cleanup.

• Use of nested loops and conditions
  The nested loops with range and if statements are somewhat complex. Consider simplifying or adding comments explaining the logic for clarity, especially around the handling of $base_tags and $tag.

---

Best Practices

• Documentation/comments

  • Add comments describing the purpose of the rollback function and the trap behavior to clarify intent.
  • Explain why rollback of tags is not performed on failure and the implications.

• Testing coverage

  • The description mentions functional tests with stubbed crane for various cases, which is good. Ensure these tests cover edge cases such as partial failures in crane tag commands and confirm the trap cleanup behaves correctly on success and failure.

• Style and naming

  • Variable missing is a bit generic; consider renaming to missing_other_arch or similar for clarity.
  • Consistent quoting for variables (some places use unquoted variables in commands - ensure all variables, especially those with file paths or tags, are quoted to avoid word splitting).

---

Summary of actionable items:

• Fix the trap rollback EXIT to avoid deleting the result file on success.
• Add error checking after crane commands and abort on failure.
• Refactor missing flag to use numeric or boolean style.
• Reduce jq filter duplication by assigning filtered artifacts to a variable.
• Add clarifying comments on rollback strategy and trap usage.
• Confirm quoting of all variables in shell commands.

[wuhuizuo]

/approve

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [wuhuizuo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment