GIT-2663: optimized image load with re-tag support

[harshanarayana]

Currently, the way the kind load docker-image works is by detecting if there is an image with a matching image name and tab already exists in the cluster. However it is possible that the same image might be already present in the cluster with a different tag.

With ctr 1.3.0+ supporting ctr image tag feature, we can avoid loading the images by just re-tagging the image hash with the new name.

Current changes fallback to using the default mode it currently has on following cases to retain compatibility.

1. If the ctr in the kind image is not supporting ctr images tag command
2. If the command to retrieve the RepoTags from Image ID runs into an error
3. If the Image ID itself is missing on the node
4. In case if the re-tag fails, it will fall back to loading the image all over again like the current workflow

Closes #2663

Missing Image

❯ bin/kind load docker-image --name helm-test docker.io/library/ubuntu:18.04
Image: "ubuntu:18.04" with ID "sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93" not yet present on node "helm-test-control-plane", loading...

Image Exists with Different Tag and Name

❯ bin/kind load docker-image --name helm-test docker.io/library/different-repo-ubuntu:18.04
Image with ID: sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93 already present on the node helm-test-control-plane but is missing the tag different-repo-ubuntu:18.04. re-tagging...

No image re-tag or load required

❯ bin/kind load docker-image --name helm-test docker.io/library/different-repo-ubuntu:18.04                                                                                                                                                                        
Image: "docker.io/library/different-repo-ubuntu:18.04" with ID "sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93" found to be already present on all nodes.

Sanitizing Registry Images

❯ kind load docker-image --name upstream-k8s kind/test-ubuntu:1.2.3
Image with ID: sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93 already present on the node upstream-k8s-control-plane but is missing the tag kind/test-ubuntu:1.2.3. re-tagging...

root@upstream-k8s-control-plane:/# crictl images | grep ubun | grep kind
docker.io/kind/test-ubuntu                            1.2.3                                      c6ad7e71ba7d4       65.5MB

[k8s-ci-robot]

Welcome @harshanarayana!

It looks like this is your first PR to kubernetes-sigs/kind 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kind has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @harshanarayana. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[BenTheElder]

Thanks for the PR 😄

With ctr 1.3.0+ supporting ctr image tag feature, we can avoid loading the images by just re-tagging the image hash with the new name.

1.3.0 is older than I thought, we've had 1.3.0+ for a very long time, let's drop the check.

In case if the re-tag fails, it will fall back to loading the image all over again like the current workflow

Trying to imagine a scenario where this makes sense 🤔
What were you thinking would happen in this case?

Containerd internally tracks the docker images with docker.io/library which can be missing on the local image cache. In such cases, the detection of duplicate image just re-tags the image again. This is only when the dockerhub images are loaded without docker.io/library prefix.

This is going to be common, we should handle this translation.
Kubernetes also does this internally, it is carried over from docker.

We need to take care to remember that while containerd does this, in podman on the host side the default registry is actually configurable, so we should only perform this normalization when interacting with containerd.

[harshanarayana]

@BenTheElder Thanks for the quick review and feedbacks.

let's drop the check.

All righty. Let me take care of that.

What were you thinking would happen in this case?

Thinking about worst or the worst cases such as containerd getting restarted causing re-tag to fail or some other docker issue causing the command invocation on the container to fail. Just a precaution. Instead of returning an error, I am doing the next best thing of reloading again. Or do you think it is enough to return an error? Since we use the --force argument with ctr if the tag already exists, it will handle it and not return an error. So other than the worst case I mentioned above, I can't think of any other reason why ctr images tag would fail.

This is going to be common, we should handle this translation.

Sure. Let me take care of it. So, all we need to do is when the ID to repoTag match is being done, in the image if the registry part is missing, we default to docker.io/library and process it. Right ? Or do you see any other special handling to be done ?

[harshanarayana]

This is going to be common, we should handle this translation.

Refactored a bit of code that podman provider had into a common utils and using that in both places now. Relocated the tests as well to address the changes accordingly.

[harshanarayana]

@BenTheElder PTAL when you can. Changes have been addressed

[harshanarayana]

/cc @BenTheElder

[BenTheElder]

Sorry, I've been out and mostly unavailable.

[BenTheElder]

one major nit, thanks for working on this
cc @aojea to help keep after reviews

[harshanarayana]

@BenTheElder @aojea PTAL at this PR when time permits. I've updated the changes as suggested by @BenTheElder

[harshanarayana]

/test pull-kind-build

[BenTheElder]

instead of a global, we can pass the function, or split the function under test into two and pass the results of calling this.

[harshanarayana]

Let me refactor is to pass the function across and use that instead. Would make it cleaner.

[harshanarayana]

❯ ./kind load docker-image --name test-cluster ubuntu:18.04
Image: "ubuntu:18.04" with ID "sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93" not yet present on node "test-cluster-control-plane", loading...

❯ ./kind load docker-image --name test-cluster kind/test-ubuntu:1.2.3
Image with ID: sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93 already present on the node test-cluster-control-plane but is missing the tag kind/test-ubuntu:1.2.3. re-tagging...

❯ ./kind load docker-image --name test-cluster different-repo-ubuntu
ERROR: image: "different-repo-ubuntu" not present locally

❯ ./kind load docker-image --name test-cluster different-repo-ubuntu:18.04
Image with ID: sha256:c6ad7e71ba7d4969784c76f57c4cc9083aa96bb969d802f2ea38f4aaed90ff93 already present on the node test-cluster-control-plane but is missing the tag different-repo-ubuntu:18.04. re-tagging...

root@test-cluster-control-plane:/# crictl images | grep ubun
docker.io/library/ubuntu                   18.04                c6ad7e71ba7d4       65.5MB

root@test-cluster-control-plane:/# crictl images | grep ubun
docker.io/kind/test-ubuntu                 1.2.3                c6ad7e71ba7d4       65.5MB
docker.io/library/ubuntu                   18.04                c6ad7e71ba7d4       65.5MB

root@test-cluster-control-plane:/# crictl images | grep ubun
docker.io/kind/test-ubuntu                 1.2.3                c6ad7e71ba7d4       65.5MB
docker.io/library/different-repo-ubuntu    18.04                c6ad7e71ba7d4       65.5MB
docker.io/library/ubuntu                   18.04                c6ad7e71ba7d4       65.5MB
root@test-cluster-control-plane:/#

[BenTheElder]

Thank you for working on this, very sorry I've been so unavailable at the moment and not getting back to this reasonably.

I will be on vacation for a few weeks and then my work situation will change a bit. antonio can hopefully pick up remaining review / bikeshed :-)

[harshanarayana]

/test pull-kind-e2e-kubernetes-1-20

[harshanarayana]

/test pull-kind-e2e-kubernetes

[harshanarayana]

@aojea @BenTheElder PTAL when time permits. The suggested changes have been addressed.

[harshanarayana]

/test pull-kind-e2e-kubernetes

[BenTheElder]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, harshanarayana

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [BenTheElder]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[harshanarayana]

/test pull-kind-e2e-kubernetes