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

This should also be backported to the release-1.7 branch.

/cc @bart0sh

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

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

@klihub this differs a bit from cri-o/cri-o#7971. Do you have thoughts on changes here?

@klihub this differs a bit from cri-o/cri-o#7971. Do you have thoughts on changes here?

I just took a quick look at this, but I think the biggest difference is that in 7971 I opted for using the package level functions whenever possible (Configure(), Refresh() and Inject*()), while this PR looks closer to a literal replacement of GetRegistry() with GetDefaultCache(). I don't have a strong opinion on this, but the general idea was that we try to single out enough of the functionality to package level functions (which implicitly operate on the default cache), to cover the most common use cases (so that you wouldn't need to call GetDefaultCache() in those cases). But in the end it ends up running the same code anyway...

LGTM for the CRI related code changes.

In fedora39 vagrant integration one test case, TestUpgrade, fails in the test-cri-integration. I don't see how these changes could directly cause that, but combing through the logs more carefully would be a good idea.

I don't know if this is the reason for TestUpgrade failing and if it is then just a transient: 2024-04-10T13:44:52.3584657Z default: E0410 13:44:52.351485 48352 remote_runtime.go:134] RunPodSandbox from runtime service failed: rpc error: code = Unknown desc = failed to get sandbox image "registry.k8s.io/pause:3.8": failed to pull image "registry.k8s.io/pause:3.8": failed to pull and unpack image "registry.k8s.io/pause:3.8": failed to copy: httpReadSeeker: failed open: failed to do request: Get "https://prod-registry-k8s-io-us-west-1.s3.dualstack.us-west-1.amazonaws.com/containers/images/sha256:4873874c08efc72e9729683a83ffbb7502ee729e9a5ac097723806ea7fa13517": dial tcp: lookup prod-registry-k8s-io-us-west-1.s3.dualstack.us-west-1.amazonaws.com: no such host

LGTM for the CRI related code changes.
In fedora39 vagrant integration one test case, TestUpgrade, fails in the test-cri-integration. I don't see how these changes could directly cause that, but combing through the logs more carefully would be a good idea.

I don't know if this is the reason for TestUpgrade failing and if it is then just a transient...

Looks like it was a transient.