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

Enhance image history #3020

Merged
merged 1 commit into from
Jun 1, 2024
Merged

Conversation

apostasie
Copy link
Contributor

This fixes some of the issues described in #3019.

Specifically:

  • implements the human flag
  • display local dates
  • implement CreatedAt
  • enhances size display
  • do not fail when format and quiet are passed together
  • truncate snapshot unless quiet or no-trunc is passed

This PR stops using containerd/pkg/progress and instead goes directly to docker/go-units for better control.

Formatting was done in the image walker, and has been moved down to the printing part instead for better separation of concerns.

What is NOT fixed from #3019:

  • any issue related to Id, or image lookup (still uses the walker for now)
  • snapshot has been left untouched

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone May 20, 2024
@apostasie apostasie force-pushed the dev-image-history branch 4 times, most recently from 41985f3 to e4a16a5 Compare May 22, 2024 00:17
@apostasie
Copy link
Contributor Author

Hey folks @yankay @AkihiroSuda

  • docs has been added for the --human flag
  • added tests for image history overall

Pending CI happiness, let me know what you think and if you want some of that stuff modified.

@apostasie
Copy link
Contributor Author

apostasie commented May 22, 2024

Ok, windows is going to give me grief again.

How do you folks test locally on windows?

Update: I do not understand how to pull a linux image on windows - for now, I have disabled the test on Windows then.

@apostasie apostasie force-pushed the dev-image-history branch 3 times, most recently from 064eec6 to 4b963c9 Compare May 22, 2024 01:41
@apostasie apostasie force-pushed the dev-image-history branch 7 times, most recently from 2f8fedc to 1d68df8 Compare May 26, 2024 23:35
@apostasie
Copy link
Contributor Author

apostasie commented May 30, 2024

@AkihiroSuda @yankay though all your comments have been addressed, this is currently blocked, as image history fails to inspect a non-native platform image (this should be a different bug, and this PR does not fix it) - thus making my newly introduced test fail.

I can weaken the test for now (eg: not checking CreatedAt) so that we can use a native image, and then when we fix this issue, we can reintroduce the more thorough test.

Thoughts?

Signed-off-by: apostasie <spam_blackhole@farcloser.world>
@apostasie
Copy link
Contributor Author

@AkihiroSuda @yankay I disabled the test on select platforms and CI is green.
PTAL at you convenience.

Thanks a lot for your help!

@yankay
Copy link
Contributor

yankay commented May 30, 2024

Thanks @apostasie

/lgtm

@apostasie
Copy link
Contributor Author

Thanks @apostasie

/lgtm

Wondering if "/lgtm" with the slash does act as intended.
Merging still show as blocked.
Could you try with just "LGTM"?

Thanks a ton @yankay !

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit a9f2f36 into containerd:main Jun 1, 2024
22 checks 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.

None yet

3 participants