Skip to content

fix: add unpack logic after container commit to images.#3268

Merged
AkihiroSuda merged 1 commit intocontainerd:mainfrom
lingdie:main
Aug 8, 2024
Merged

fix: add unpack logic after container commit to images.#3268
AkihiroSuda merged 1 commit intocontainerd:mainfrom
lingdie:main

Conversation

@lingdie
Copy link
Copy Markdown
Contributor

@lingdie lingdie commented Aug 1, 2024

May fix this: #827

@lingdie
Copy link
Copy Markdown
Contributor Author

lingdie commented Aug 1, 2024

test commands and results:

nerdctl -n k8s.io commit 555dd4b423742 lingdie/commit:dev
WARN[0000] Image lacks label "nerdctl/platform", assuming the platform to be "linux/amd64"
sha256:55920a9d373f7778901f57501cf15cb5233f69ceadc38ceae827bc7404455643

nerdctl -n k8s.io inspect lingdie/commit:dev
[
    {
        "Id": "sha256:55920a9d373f7778901f57501cf15cb5233f69ceadc38ceae827bc7404455643",
        "RepoTags": [
            "lingdie/commit:dev"
        ],
        "RepoDigests": [
            "lingdie/commit@sha256:3684882f267f612a499de1786a627e216b8db0510fede8d817429606e64f546d"
        ],
        "Parent": "",
        "Comment": "",
        "Created": "2024-08-01T08:07:58.833742114Z",
        "DockerVersion": "",
        "Author": "",
        "Config": {
            "AttachStdin": false,
            "Env": [
                "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
            ],
            "Cmd": [
                "/bin/bash"
            ],
            "WorkingDir": "/root",
            "Labels": {
                "org.opencontainers.image.ref.name": "ubuntu",
                "org.opencontainers.image.version": "24.04"
            }
        },
        "Architecture": "amd64",
        "Os": "linux",
        "Size": 87519232,
        "VirtualSize": 87519232,
        "RootFS": {
            "Type": "layers",
            "Layers": [
                "sha256:a30a5965a4f7d9d5ff76a46eb8939f58e95be844de1ac4a4b452d5d31158fdea",
                "sha256:5f70bf18a086007016e948b04aed3b82103a36bea41755b6cddfaf10ace3c6ef",
                "sha256:97b66fcb143832ac7d953ff66d09c77c2ba31290d3aa422b2bc27762f55e0a73"
            ]
        },
        "Metadata": {
            "LastTagTime": "0001-01-01T00:00:00Z"
        }
    }
]

nerdctl -n k8s.io push lingdie/commit:dev
INFO[0000] pushing as a reduced-platform image (application/vnd.docker.distribution.manifest.v2+json, sha256:3684882f267f612a499de1786a627e216b8db0510fede8d817429606e64f546d)
manifest-sha256:3684882f267f612a499de1786a627e216b8db0510fede8d817429606e64f546d: done           |++++++++++++++++++++++++++++++++++++++|
config-sha256:55920a9d373f7778901f57501cf15cb5233f69ceadc38ceae827bc7404455643:   done           |++++++++++++++++++++++++++++++++++++++|
elapsed: 4.4 s                                                                    total:  2.5 Ki (579.0 B/s)

@lingdie lingdie changed the title add unpack logic after container commit to images. fix: add unpack logic after container commit to images. Aug 1, 2024
@apostasie
Copy link
Copy Markdown
Contributor

@lingdie you should DCO and sign your commits (git commit -s -S) to make project checks happy, and squash your commits.
If you could add one, a test for this would be great (that would demonstrate the fix is addressing the issue).

Now, this overall is reminiscent of #2327 .
It feels like there is something deeper that is wrong in our push logic that should take care of this for a variety of cases (eg: image must be expanded).

@fahedouch + @AkihiroSuda do you have a good intuition on this overall?

Signed-off-by: yy <lingdie.yy@outlook.com>
@lingdie
Copy link
Copy Markdown
Contributor Author

lingdie commented Aug 2, 2024

It's not easy to add tests. The environment corresponding to the issue is to commit the pod container in k8s. I cannot create a test environment and test cases from the existing tests.

It would be great if you could provide me with more information.

And please rerun ci, some ci tests failed because of docker.io 502 error 😢...

@apostasie
Copy link
Copy Markdown
Contributor

It's not easy to add tests. The environment corresponding to the issue is to commit the pod container in k8s. I cannot create a test environment and test cases from the existing tests.

It would be great if you could provide me with more information.

Ah!
Yep, we should probably have a kube testing environment.
Do you have an opinion on what we should go with to help with that? kind?

I recently cleaned-up that part in nydus, so, maybe we could reuse some of that stuff to spin-up our env:
https://github.com/containerd/nydus-snapshotter/tree/main/tests/helpers

And please rerun ci, some ci tests failed because of docker.io 502 error 😢...

I cannot do that, but @AkihiroSuda can :-).

@AkihiroSuda AkihiroSuda requested a review from ktock August 5, 2024 01:18
@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 5, 2024
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 merged commit fff7495 into containerd:main Aug 8, 2024
@apostasie apostasie mentioned this pull request Aug 12, 2024
@apostasie
Copy link
Copy Markdown
Contributor

@ouyangningdong
Copy link
Copy Markdown

sudo /usr/local/bin/nerdctl -n k8s.io push kubesharkwork
FATA[0000] failed to create a tmp single-platform image "docker.io/library/kubesharkwork:latest-tmp-reduced-platform": content digest sha256:a58ecd4f0c864650a4286c3c2d49c7219a3f2fc8d7a0bf478aa9834acfe14ae7: not found

@apostasie
Copy link
Copy Markdown
Contributor

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.

4 participants