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.
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
manifest, push: implement
--add-compression
to push with compressed variants. #19468manifest, push: implement
--add-compression
to push with compressed variants. #19468Changes from all commits
49b8b97
346f9cb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have multi-arch image that could be used for this test without requiring a build, by any chance?
(I have no idea … and if this works, maybe we can spend CPU cycles instead of our attention.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quay.io/libpod/20221018
? Or even:00000004
if you want a tiny image.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure I'll consider it if I am going to make any further change and repush this. ( I hope this not a hard blocker since OTOH building image makes tests independent of any external image ( pulling ) which has been encouraged in past if i can recall )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC the Podman e2e tests already have some infrastructure to pre-pull / cache images, so that not every pull needs to reach out to the internet. But I don’t know the details by heart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely non-blocking: This could plausibly call c/image instead of using a subprocess.
OTOH this is easier to follow for people unfamiliar with that codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is similar to systems test for easier review and familiarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make more sense to directly execute skopeo on the host instead of pulling the image as pull are flaky and slow. We already depend on it in system test so I see no problem using it in e2e as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to my knowledge skopeo is a hard requirement so this skip makes no sense
cc @edsantiago
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests which are using
skopeo
have this check directly or indirectly so I have added it here as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? Not having remote tests is a big issue. We need tests to cover remote client and API as well.
If we cannot set this up in system tests we should use e2e tests or the bindings and API tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a
e2e
test.Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.