Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add platform flag to store save #329

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

amartin120
Copy link
Contributor

Please check below, if the PR fulfills these requirements:

  • Commit(s) and code follow the repositories guidelines.
  • Test(s) have been added or updated to support these change(s).
  • Doc(s) have been added or updated to support these change(s).

Associated Links:

Types of Changes:

  • bugfix

Proposed Changes:

  • An image index a.k.a. multi-arch image isn't really docker compatible so this PR adjusts hauler store save to have a --platform flag to give the user a chance to only export the platform of their choice. Any platform skipped as a result of this flag will be logged as a warning.

  • If the --platform flag isn't provided, a warning gets logged during save if your store contains an image index... just as a heads up.

  • Docker also isn't compatible with the unknown/unknown platform that some image index might contain. we're going to skip those or otherwise docker load will fail to import.

Verification/Testing of Changes:

  • example of omitting the platform flag and hauler just throwing a warning when it processes an image index.
> hauler store save                                                                                                                                                                                                                                                           
2024-09-23 15:18:20 WRN index [index.docker.io/bitnami/cert-manager:1.15.3]: contains multiple platforms and could cause issues when importing into docker/containerd
  • example of using the --platform flag to skip anything other than what's specified.
> hauler store save --platform linux/arm64                                                                                                                                                                                                                                     
2024-09-23 15:19:05 WRN index [index.docker.io/bitnami/cert-manager:1.15.3]: digest=sha256:5c827d71196f865e168e3fed85ed1bd54863fc8f01760a833253474aed4a1441, platform=linux/amd64: does not match the supplied platform, skipping
2024-09-23 15:19:05 INF saved store [store] -> [/Users/amartin/projects/hauler/haul.tar.zst]
  • example of an image index containing unknown/unknown
2024-09-23 15:18:20 WRN index [index.docker.io/library/busybox:latest]: digest=sha256:c230832bd3b0be59a6c47ed64294f9ce71e91b327957920b6929a0caa8353140, platform=unknown/unknown: skipping 'unknown/unknown' platform

Additional Context:

@amartin120 amartin120 self-assigned this Sep 23, 2024
@zackbradys zackbradys added enhancement New feature or request size/M Denotes an issue/PR requiring a relatively moderate amount of work labels Sep 23, 2024
@zackbradys zackbradys added this to the Hauler v1.1.0 milestone Sep 23, 2024
@amartin120
Copy link
Contributor Author

amartin120 commented Sep 23, 2024

@dweomer I'm second guessing the flag being named platform. It might be mistaken for the entire save operation when ultimately it's just the 'manifest.json'.

How about --manifest-platform or --docker-platform? Thoughts?

Copy link
Contributor

@dweomer dweomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think --platform is right and consistent with other subcommand usages.

@amartin120 amartin120 merged commit caaed30 into hauler-dev:main Sep 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/M Denotes an issue/PR requiring a relatively moderate amount of work
Projects
Status: Resolved
Development

Successfully merging this pull request may close these issues.

3 participants