Skip to content

Fix broken TestImageInspectDifferentValidReferencesForTheSameImage#3483

Merged
AkihiroSuda merged 1 commit intocontainerd:mainfrom
apostasie:urgent-fix-CI-bustage
Oct 2, 2024
Merged

Fix broken TestImageInspectDifferentValidReferencesForTheSameImage#3483
AkihiroSuda merged 1 commit intocontainerd:mainfrom
apostasie:urgent-fix-CI-bustage

Conversation

@apostasie
Copy link
Copy Markdown
Contributor

@apostasie apostasie commented Oct 2, 2024

Fix #3479

Reviewers: since the official busybox is a moving target, and has just shifted under our feet, I replaced it with a copy under apostasie/busybox.

This is a short term fix so that the CI does not stay bust - but I do not like the idea of having images in my personal account.

(Part of) the current intent of this test was apparently to test variants of image names only available on the hub.

To get rid of my images here, here are our options:

@apostasie apostasie mentioned this pull request Oct 2, 2024
@apostasie
Copy link
Copy Markdown
Contributor Author

cc @AkihiroSuda

Tagging @manugupt1

Review at your convenience.
CI failures are a Github Actions fluke, and the usual IPFSCompose.

@apostasie apostasie marked this pull request as ready for review October 2, 2024 05:24
Comment thread cmd/nerdctl/image/image_inspect_test.go
@apostasie apostasie force-pushed the urgent-fix-CI-bustage branch from 061a0d9 to f40cc41 Compare October 2, 2024 05:41
@apostasie apostasie requested a review from AkihiroSuda October 2, 2024 05:42
func TestImageInspectDifferentValidReferencesForTheSameImage(t *testing.T) {
nerdtest.Setup()

platform := runtime.GOOS + "/" + runtime.GOARCH
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Earlier version of this test was using linux/amd64, because it was hardcoding the sha.
When I rewrote the test a few weeks ago, I removed the static sha and replaced by a lookup, allowing to use whatever platform - though I did not remove the platform mechanism itself for some reason.
It is unnecesary.

@apostasie apostasie force-pushed the urgent-fix-CI-bustage branch from f40cc41 to 839f957 Compare October 2, 2024 05:48
@AkihiroSuda
Copy link
Copy Markdown
Member

Could you update the commit message to explain why :stable is no longer testable?

@apostasie
Copy link
Copy Markdown
Contributor Author

apostasie commented Oct 2, 2024

CI is green-ish (Docker failure is unrelated and will get fixed in #3464).

As of 10/01/2024, busybox latest tag (1.37.0) is no longer the same thing as stable (1.36.1).
It is unclear if that was made on purpose, or a problem with Docker publication process.

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

Could you update the commit message to explain why :stable is no longer testable?

Doing now.

@apostasie apostasie force-pushed the urgent-fix-CI-bustage branch from 839f957 to c661d16 Compare October 2, 2024 06:26
Copy link
Copy Markdown
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 added this to the v2.0.0 milestone Oct 2, 2024
@AkihiroSuda AkihiroSuda merged commit c156ee6 into containerd:main Oct 2, 2024
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.

[URGENT] TestImageInspectDifferentValidReferencesForTheSameImage is broken on main

2 participants