parsing ref consistently for refCache, including adding `docker.io/library` prefix

[pacoxu]

Fixes #7698

For the refCache below, the ref needs to be consistent.

There is a test case in #7698, I can quickly reproduce it with the second method there.

Before restarting and importing image, the refCahe is using ref: import-2022-11-28@sha256:c8a01d8610c642f0e6b83af5f6185b3eed34e322354be5a515da1a21453ac966" and crictl cannot remove it correctly.

After restarting containerd, the cache became ref: docker.io/library/import-2022-11-28@sha256:c8a01d8610c642f0e6b83af5f6185b3eed34e322354be5a515da1a21453ac966" and crictl can remove it correctly.

---

I add another cleanup in services/images/local.go without docker.io/library/ prefix.

• see https://github.com/containerd/containerd/pull/7728#issuecomment-1351174499

[k8s-ci-robot]

Hi @pacoxu. 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.

[pacoxu]

/cc @pmengelbert
for the issue

/cc @mxpv @dmcgowan for you are recent contributors pkg/cri/store/image/image.go

[k8s-ci-robot]

@pacoxu: GitHub didn't allow me to request PR reviews from the following users: pmengelbert.

Note that only containerd members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @pmengelbert
for the issue

/cc @mxpv @dmcgowan for you are recent contributors pkg/cri/store/image/image.go

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.

[pacoxu]

The failure seems to be unrelated.

• https://github.com/containerd/containerd/pull/7728/checks?check_run_id=9742873492

[sozercan]

/cc @cpuguy83

[ruiwen-zhao]

Mostly for my own education, can you help me understand why restarting containerd changed the ref in the cache? Is another path of code triggered upon containerd start?

[pacoxu]

Mostly for my own education, can you help me understand why restarting containerd changed the ref in the cache? Is another path of code triggered upon containerd start?

As I described in https://github.com/containerd/containerd/pull/7728#discussion_r1039120522, when we first import it with ctr, ref cache add cache with prefix.

Or we can fix it during import only?

[ruiwen-zhao]

Mostly for my own education, can you help me understand why restarting containerd changed the ref in the cache? Is another path of code triggered upon containerd start?

As I described in #7728 (comment), when we first import it with ctr, ref cache add cache with prefix.

Or we can fix it during import only?

Sorry maybe I am misunderstanding the issue here. According to https://github.com/containerd/containerd/issues/7698 and section "II. Manually load an image with --digests=true", the issue seems to be that crictl only removes repoTags but fails to remove the actual image, which seems to be problematic because cri attempts to remove all refereneces [1]. Is my understanding correct? If so then we should try to fix cri's image removal, no?

[1] https://github.com/containerd/containerd/blob/main/pkg/cri/server/image_remove.go#L49

[pacoxu]

I did some tests with some more logs.

The first crictl rmi $imgeid shows all ref removing are triggered. But import image is not deleted indeed.

12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.735311390+08:00" level=debug msg="image remove i 0,ref docker.io/library/hello:world"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.735862343+08:00" level=debug msg="image remove Delete err <nil>"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.735974214+08:00" level=debug msg="image remove i 1,ref docker.io/library/import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.736032431+08:00" level=debug msg="image remove Delete err image \"docker.io/library/import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59\": not found"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.736090162+08:00" level=debug msg="image remove i 2,ref docker.io/library/import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.736120975+08:00" level=debug msg="image remove Delete err image \"docker.io/library/import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59\": not found"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.736145294+08:00" level=debug msg="image remove i 3,ref sha256:c55b4d09bd7bbc82fec32fe98a363c99f476cc6c5eaed89968aaadc1f84e443e"
12月 14 17:46:15 paco-centos-9 containerd[4192410]: time="2022-12-14T17:46:15.744530837+08:00" level=debug msg="image remove Delete err <nil>"

After restarting containerd, the removing is done when runnging crictl rmi $imgeid.

12月 14 17:34:03 paco-centos-9 containerd[4192410]: time="2022-12-14T17:34:03.709711741+08:00" level=debug msg="Loaded image \"import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59\""
12月 14 17:34:16 paco-centos-9 containerd[4192410]: time="2022-12-14T17:34:16.171106366+08:00" level=debug msg="image remove i 0,ref import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59"
12月 14 17:34:16 paco-centos-9 containerd[4192410]: time="2022-12-14T17:34:16.171214731+08:00" level=debug msg="delete image" name="import-2022-12-14@sha256:a0010348fb08fdacb9c3fbe798e18d20c19e98aeb3c3bf7c471279f63ab1ee59"

[pacoxu]

It seems that the local image removing is without docker.io/library/ prefix.

BTW,

• nerdctl inspect shows no docker.io/library/ prefix.
• crictl inspect shows docker.io/library/ prefix.
• ctr i ls shows no docker.io/library/ prefix.

[pacoxu]

So I add another cleanup in services/images/local.go without docker.io/library/ prefix.

[pacoxu]

TestContainerPids failed. It seems unrelated. https://github.com/containerd/containerd/pull/7728/checks?check_run_id=10658354511

[ruiwen-zhao]

Thanks Paco! LGTM except for the two minor comments.

[ruiwen-zhao]

/lgtm

[k8s-ci-robot]

PR needs rebase.

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.

[github-actions[bot]]

This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed.

[github-actions[bot]]

This PR was closed because it has been stalled for 7 days with no activity.