mirror validation should pass with local image only; more testing for mirroring #47
Conversation
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| ${REGISTRY_BIN}& | ||
| registry_pid=$! | ||
| trap "kill -9 $registry_pid" EXIT |
There was a problem hiding this comment.
The script starts a background process for ${REGISTRY_BIN} and sets a trap to kill it on exit. However, using kill -9 is generally discouraged because it does not allow the process to clean up. A gentler signal like TERM should be used first.
Recommendation: Replace kill -9 $registry_pid with kill $registry_pid to allow the process to terminate gracefully. If the process does not terminate, then consider using kill -9 as a fallback.
mirror/validate_image.sh
Outdated
| REMOTE="{src_image}" | ||
| LOCAL="{dst_image}@{digest}" | ||
| echo "Validating images $REMOTE and $LOCAL" | ||
| {crane_tool} validate -v --fast --remote $REMOTE || {crane_tool} validate -v --fast --remote $LOCAL || exit 1 |
There was a problem hiding this comment.
The current validation logic does not provide specific error messages for each validation failure. This can make debugging difficult as it is unclear which image validation failed.
Recommended Solution:
Update the script to provide specific error messages for each validation failure. For example:
{crane_tool} validate -v --fast --remote $REMOTE || { echo "Validation failed for remote image: $REMOTE"; exit 1; }
{crane_tool} validate -v --fast --remote $LOCAL || { echo "Validation failed for local image: $LOCAL"; exit 1; }
Mirror always verify for remote image availability in a generated test to make sure we catch errors in image location or digest earlier.
However, the goal of the mirror was to increase external images availability if they suddenly disappear. The test should not fail in this case.
This PR modifies test to check local image if remote does not exist.
Also, it add more integration testing for mirroring