Add a new image label if it is docker schema 1 #9215

qiutongs · 2023-10-11T03:58:36Z

Resolve #9202

This PR adds a new image label if it is docker schema 1. With the new label, users can find the docker schema1 usage by running ctr images

Local Test

$ sudo /usr/local/bin/crictl pull docker.io/progrium/stress:latest
$ sudo ctr -n k8s.io images ls
REF                                                                     TYPE                                       DIGEST                                                                  SIZE     PLATFORMS   LABELS                                                                                                                                               
docker.io/progrium/stress:latest                                        application/vnd.oci.image.manifest.v1+json sha256:fe33932960bfb41cab97a147e7c38e0c787424dd374deba976dd6e9803fb3666 86.5 MiB linux/amd64 io.containerd.image/converted-docker-schema1=sha256:e34d56d60f5caae79333cee395aae93b74791d50e3841986420d23c2ee4697bf,io.cri-containerd.image=managed 
sha256:9d98b4a6b6d1fe7346174111650132fd189b9153fb48a94a7b898f55f9d15a17 application/vnd.oci.image.manifest.v1+json sha256:fe33932960bfb41cab97a147e7c38e0c787424dd374deba976dd6e9803fb3666 86.5 MiB linux/amd64 io.cri-containerd.image=managed

$ sudo bin/ctr image pull docker.io/progrium/stress:latest
$ sudo bin/ctr image ls
REF                              TYPE                                       DIGEST                                                                  SIZE     PLATFORMS   LABELS                                                                                                               
docker.io/progrium/stress:latest application/vnd.oci.image.manifest.v1+json sha256:fe33932960bfb41cab97a147e7c38e0c787424dd374deba976dd6e9803fb3666 86.5 MiB linux/amd64 io.containerd.image/converted-docker-schema1=sha256:e34d56d60f5caae79333cee395aae93b74791d50e3841986420d23c2ee4697bf

k8s-ci-robot · 2023-10-11T03:58:46Z

Hi @qiutongs. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

samuelkarp

It'd be nice to have a unit test for this, but since there are no existing tests for pull.go I don't consider that blocking.

pull.go

qiutongs · 2023-10-12T08:02:06Z

It'd be nice to have a unit test for this, but since there are no existing tests for pull.go I don't consider that blocking.

If there is a "well-known" docker schema 1 for integration test, I probably can add a new test case in https://github.com/containerd/containerd/blob/main/integration/containerd_image_test.go

henry118

LGTM

qiutongs · 2023-10-12T16:10:42Z

/retest

k8s-ci-robot · 2023-10-12T16:10:57Z

@qiutongs: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

samuelkarp · 2023-10-12T16:32:22Z

/ok-to-test

samuelkarp · 2023-10-12T18:33:42Z

@qiutongs It looks like the Kubernetes project has a schema 1 image we can use: https://explore.ggcr.dev/?image=registry.k8s.io/busybox@sha256:4bdd623e848417d96127e16037743f0cd8b528c026e9175e22a84f639eca58ff&mt=application%2Fvnd.docker.distribution.manifest.v1%2Bprettyjws&size=3205

/cc @dims

mikebrow

/LGTM

qiutongs · 2023-10-12T22:35:09Z

@qiutongs It looks like the Kubernetes project has a schema 1 image we can use: https://explore.ggcr.dev/?image=registry.k8s.io/busybox@sha256:4bdd623e848417d96127e16037743f0cd8b528c026e9175e22a84f639eca58ff&mt=application%2Fvnd.docker.distribution.manifest.v1%2Bprettyjws&size=3205

/cc @dims

Great, let me add an integration test based on it

fuweid

LGTM

qiutongs · 2023-10-13T06:12:21Z

@qiutongs It looks like the Kubernetes project has a schema 1 image we can use: https://explore.ggcr.dev/?image=registry.k8s.io/busybox@sha256:4bdd623e848417d96127e16037743f0cd8b528c026e9175e22a84f639eca58ff&mt=application%2Fvnd.docker.distribution.manifest.v1%2Bprettyjws&size=3205
/cc @dims

Great, let me add an integration test based on it

Added a new test case in containerd_image_test.go

qiutongs · 2023-10-13T07:12:36Z

/retest

samuelkarp · 2023-10-13T07:19:39Z

/retest

This only works for the prow jobs. Most of containerd's CI is via GitHub Actions. I'll trigger a retest for you.

qiutongs · 2023-10-13T18:42:33Z

/retest

This only works for the prow jobs. Most of containerd's CI is via GitHub Actions. I'll trigger a retest for you.

Thank you. And I don't think the failed windows integration test is caused by this PR.

mv: cannot remove 'C:/Windows/Temp/test-integration/containerd.log': Device or resource busy

samuelkarp · 2023-10-13T18:44:09Z

=== RUN   TestContainerdImageWithDockerSchema1
    containerd_image_test.go:244: make sure the test image doesn't exist in the cri plugin
    containerd_image_test.go:251: pull the image into containerd
time="2023-10-13T07:57:01Z" level=info msg="apply failure, attempting cleanup" error="failed to extract layer sha256:2c84284818d186d88a16ac7fa731d4b71ba69ecfe11b4ce00413366833cb2403: hcsshim::ProcessBaseLayer \\\\?\\C:\\ProgramData\\containerd\\root-test\\io.containerd.snapshotter.v1.windows\\snapshots\\114: The system cannot find the path specified.: unknown" key="extract-724054400-oMXA sha256:2c84284818d186d88a16ac7fa731d4b71ba69ecfe11b4ce00413366833cb2403"
    containerd_image_test.go:254: 
        	Error Trace:	D:/a/containerd/containerd/src/github.com/containerd/containerd/integration/containerd_image_test.go:254
        	Error:      	Received unexpected error:
        	            	failed to unpack image on snapshotter : failed to extract layer sha256:2c84284818d186d88a16ac7fa731d4b71ba69ecfe11b4ce00413366833cb2403: hcsshim::ProcessBaseLayer \\?\C:\ProgramData\containerd\root-test\io.containerd.snapshotter.v1.windows\snapshots\114: The system cannot find the path specified.: unknown
        	Test:       	TestContainerdImageWithDockerSchema1
--- FAIL: TestContainerdImageWithDockerSchema1 (1.07s)

This image is not a multi-platform image and won't work on Windows. Can you skip the test on Windows?

Signed-off-by: Qiutong Song <songqt01@gmail.com>

…h upstream containerd/main update fork-external/main branch to upstream containerd/main at commit f90f80d Related work items: containerd#8736, containerd#8861, containerd#8924, containerd#8934, containerd#9027, containerd#9076, containerd#9104, containerd#9118, containerd#9141, containerd#9155, containerd#9177, containerd#9183, containerd#9184, containerd#9186, containerd#9187, containerd#9191, containerd#9200, containerd#9205, containerd#9211, containerd#9214, containerd#9215, containerd#9221, containerd#9223, containerd#9228, containerd#9231, containerd#9234, containerd#9242, containerd#9246, containerd#9247, containerd#9251, containerd#9253, containerd#9254, containerd#9255, containerd#9268

k8s-ci-robot added the needs-ok-to-test label Oct 11, 2023

qiutongs force-pushed the docker-schema1-label branch from ae00d22 to 4d85c64 Compare October 11, 2023 03:59

qiutongs mentioned this pull request Oct 11, 2023

Expose docker schema 1 image usage by adding a new label #9202

Closed

samuelkarp added cherry-pick/1.6.x Change to be cherry picked to release/1.6 branch cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Oct 11, 2023

samuelkarp approved these changes Oct 11, 2023

View reviewed changes

lengrongfu reviewed Oct 11, 2023

View reviewed changes

pull.go Show resolved Hide resolved

qiutongs force-pushed the docker-schema1-label branch from 4d85c64 to 9ef677a Compare October 12, 2023 04:33

qiutongs requested review from henry118 and lengrongfu October 12, 2023 08:02

henry118 approved these changes Oct 12, 2023

View reviewed changes

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Oct 12, 2023

k8s-ci-robot requested a review from dims October 12, 2023 18:33

mikebrow approved these changes Oct 12, 2023

View reviewed changes

fuweid approved these changes Oct 13, 2023

View reviewed changes

fuweid requested review from dmcgowan and lengrongfu and removed request for lengrongfu October 13, 2023 02:56

qiutongs force-pushed the docker-schema1-label branch from 9ef677a to a155388 Compare October 13, 2023 06:10

qiutongs force-pushed the docker-schema1-label branch from a155388 to e839d00 Compare October 13, 2023 06:16

samuelkarp approved these changes Oct 13, 2023

View reviewed changes

Add a new image label if it is docker schema 1

7712375

Signed-off-by: Qiutong Song <songqt01@gmail.com>

qiutongs force-pushed the docker-schema1-label branch from e839d00 to 7712375 Compare October 16, 2023 04:11

samuelkarp approved these changes Oct 16, 2023

View reviewed changes

samuelkarp merged commit cc9389a into containerd:main Oct 16, 2023

qiutongs deleted the docker-schema1-label branch October 16, 2023 17:29

This was referenced Oct 17, 2023

[release/1.7] Add a new image label if it is docker schema 1 #9252

Merged

[release/1.6] Add a new image label if it is docker schema 1 #9267

Merged

samuelkarp mentioned this pull request Oct 24, 2023

Expose usage of deprecated features #9292

Closed

Add a new image label if it is docker schema 1 #9215

Add a new image label if it is docker schema 1 #9215

Uh oh!

Conversation

qiutongs commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Local Test

Uh oh!

k8s-ci-robot commented Oct 11, 2023

Uh oh!

samuelkarp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

qiutongs commented Oct 12, 2023

Uh oh!

henry118 left a comment

Choose a reason for hiding this comment

Uh oh!

qiutongs commented Oct 12, 2023

Uh oh!

k8s-ci-robot commented Oct 12, 2023

Uh oh!

samuelkarp commented Oct 12, 2023

Uh oh!

samuelkarp commented Oct 12, 2023

Uh oh!

mikebrow left a comment

Choose a reason for hiding this comment

Uh oh!

qiutongs commented Oct 12, 2023

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

qiutongs commented Oct 13, 2023

Uh oh!

qiutongs commented Oct 13, 2023

Uh oh!

samuelkarp commented Oct 13, 2023

Uh oh!

qiutongs commented Oct 13, 2023

Uh oh!

samuelkarp commented Oct 13, 2023

Uh oh!

Uh oh!

qiutongs commented Oct 11, 2023 •

edited

Loading