Improve performance when loading docker images already present in the cluster

[gyorokpeter]

What would you like to be added: When doing kind docker load image, it should check if the image is available and it should only update the tag if so.

Why is this needed: Major time sink when repeatedly switching back and forth between two images (e.g. debug and release version).

Current behavior:

$ kind load docker-image IMAGE:debug --name xxx
Image: "IMAGE:debug" with ID "sha256:aaaa" not yet present on node "xxx-control-plane", loading... 
$ kind load docker-image IMAGE:release --name xxx
Image: "IMAGE:release" with ID "sha256:bbbb" not yet present on node "xxxücontrol-plane", loading... 
$ kind load docker-image IMAGE:debug --name xxx
Image: "IMAGE:debug" with ID "sha256:aaaa" not yet present on node "xxx-control-plane", loading... 

Kind version 0.15.0

Expected behavior: the third command should be a quick operation, only resetting the tag "IMAGE:debug" instead of uploading the image again.

[BenTheElder]

This is a bug, or else the image changed, the code is already written to optimize the case that the image is already present including if the tag just needs to be applied to an existing image.

can you share a more specific step by step reproducer including cluster creation, docker version, etc?

also, I noticed there's a stray character in your output for some reason 🤔

[gyorokpeter]

1. Create cluster

$ docker --version 
Docker version 20.10.17, build 100c701 

$ kind create cluster --name demo 
Creating cluster "demo" ... 
✓ Ensuring node image (kindest/node:v1.25.0) 🖼 
✓ Preparing nodes 📦 
✓ Writing configuration 📜 
✓ Starting control-plane 🕹️ 
✓ Installing CNI 🔌 
✓ Installing StorageClass 💾 
Set kubectl context to "kind-demo" 
You can now use your cluster with: 
kubectl cluster-info --context kind-demo 
Thanks for using kind! 😊 

2. Create example images

$ cat hello.sh 
echo hello world 
$ cat Dockerfile 
FROM alpine 
COPY hello.sh /bin 
ENTRYPOINT ["/bin/hello.sh"] 

$ docker build . 
[+] Building 5.0s (7/7) FINISHED 
=> [internal] load build definition from Dockerfile                                                               0.0s 
=> => transferring dockerfile: 103B                                                                               0.0s 
=> [internal] load .dockerignore                                                                                  0.0s 
=> => transferring context: 2B                                                                                    0.0s 
=> [internal] load metadata for docker.io/library/alpine:latest                                                   3.0s 
=> [internal] load build context                                                                                  0.1s 
=> => transferring context: 52B                                                                                   0.0s 
=> [1/2] FROM docker.io/library/alpine@sha256:bc41182d7ef5ffc53a40b044e725193bc10142a1243f395ee852a8d9730fc2ad    1.6s 
=> => resolve docker.io/library/alpine@sha256:bc41182d7ef5ffc53a40b044e725193bc10142a1243f395ee852a8d9730fc2ad    0.0s 
=> => sha256:bc41182d7ef5ffc53a40b044e725193bc10142a1243f395ee852a8d9730fc2ad 1.64kB / 1.64kB                     0.0s 
=> => sha256:1304f174557314a7ed9eddb4eab12fed12cb0cd9809e4c28f29af86979a3c870 528B / 528B                         0.0s 
=> => sha256:9c6f0724472873bb50a2ae67a9e7adcb57673a183cea8b06eb778dca859181b5 1.47kB / 1.47kB                     0.0s 
=> => sha256:213ec9aee27d8be045c6a92b7eac22c9a64b44558193775a1a7f626352392b49 2.81MB / 2.81MB                     1.3s 
=> => extracting sha256:213ec9aee27d8be045c6a92b7eac22c9a64b44558193775a1a7f626352392b49                          0.1s 
=> [2/2] COPY hello.sh /bin                                                                                       0.1s 
=> exporting to image                                                                                             0.1s 
=> => exporting layers                                                                                            0.1s 
=> => writing image sha256:7a3584fd34888420bfc59b582870fddd22161a49252909a4a04825c2147f7755                       0.0s 
Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them 

Add a little change to get a new image

$ cat hello.sh 
echo hello world! 
$ docker build . 
[+] Building 2.0s (7/7) FINISHED 
=> [internal] load build definition from Dockerfile                                                               0.0s 
=> => transferring dockerfile: 37B                                                                                0.0s 
=> [internal] load .dockerignore                                                                                  0.0s 
=> => transferring context: 2B                                                                                    0.0s 
=> [internal] load metadata for docker.io/library/alpine:latest                                                   1.6s 
=> [internal] load build context                                                                                  0.0s 
=> => transferring context: 53B                                                                                   0.0s 
=> CACHED [1/2] FROM docker.io/library/alpine@sha256:bc41182d7ef5ffc53a40b044e725193bc10142a1243f395ee852a8d9730  0.0s 
=> [2/2] COPY hello.sh /bin                                                                                       0.1s 
=> exporting to image                                                                                             0.1s 
=> => exporting layers                                                                                            0.1s 
=> => writing image sha256:9d561b3b6fe09fbe1ef4b0586d808b2bc6c7c43e3086411ef07abe5af63149a0                       0.0s 

3. Tag the image and upload to kind using tag

$ docker tag sha256:7a3584fd34888420bfc59b582870fddd22161a49252909a4a04825c2147f7755 myimage:debug 
$ kind load docker-image --name demo myimage:debug 
Image: "myimage:debug" with ID "sha256:7a3584fd34888420bfc59b582870fddd22161a49252909a4a04825c2147f7755" not yet present on node "demo-control-plane", loading... 
$ docker tag sha256:9d561b3b6fe09fbe1ef4b0586d808b2bc6c7c43e3086411ef07abe5af63149a0 myimage:debug 
$ kind load docker-image --name demo myimage:debug 
Image: "myimage:debug" with ID "sha256:9d561b3b6fe09fbe1ef4b0586d808b2bc6c7c43e3086411ef07abe5af63149a0" not yet present on node "demo-control-plane", loading...

So far so good, but if we try to roll back the tag...

$ docker tag sha256:7a3584fd34888420bfc59b582870fddd22161a49252909a4a04825c2147f7755 myimage:debug 
$ kind load docker-image --name demo myimage:debug 
Image: "myimage:debug" with ID "sha256:7a3584fd34888420bfc59b582870fddd22161a49252909a4a04825c2147f7755" not yet present on node "demo-control-plane", loading... 

So it is uploading the image again even though it's already there. They can be seen in:

$ docker exec -it demo-control-plane bash -c "crictl images -no-trunc" 
...
docker.io/library/myimage                  debug                sha256:7a3584fd34888420bfc59b582870fddd22161a49252909a4a04825c2147f7755   5.83MB 
docker.io/library/import-2022-09-12        <none>               sha256:9d561b3b6fe09fbe1ef4b0586d808b2bc6c7c43e3086411ef07abe5af63149a0   5.83MB
...

[harshanarayana]

@BenTheElder Not sure if this is a bug but can definitely be optimized.

https://github.com/kubernetes-sigs/kind/blob/e3a4cd8884aa6ffde81e39888db9fec7aa1c24e9/pkg/cmd/kind/load/docker-image/docker-image.go#L156-L160

Uses the image name to extract the ID and does a match of current ID against the found ID.

Since the image was re-tagged, the returned ID containing the SHA doesn't match the one found from the image detected via the crictl inspecti <image> and this will try to reload it. I think we can optimize this a bit for sure to check both tag and SHA separately and make a decision based on the two to see if the image needs to be reloaded or just tagged.

[harshanarayana]

Otherwise, We could also extend the check https://github.com/kubernetes-sigs/kind/blob/e3a4cd8884aa6ffde81e39888db9fec7aa1c24e9/pkg/cmd/kind/load/docker-image/docker-image.go#L244-L258 to do this optimisation.

[BenTheElder]

Thanks for the detailed reproducer! /triage accepted

@harshanarayana yes I would call this a missing edge case of the re-tag logic

[harshanarayana]

/assign

@BenTheElder Let me open a PR to make the necessary changes

[harshanarayana]

I did a quick test for this and optimizing this can have an unwanted side effect I think

root@demo-control-plane:/# crictl images --digests --no-trunc | grep debug
docker.io/library/myimage                       debug                                                              sha256:50b471c68797a477c88b63d9c4a3c36e82c2335efdbd49590ef7ad49d4843e37   sha256:0d233417b3c4b8c50158223434bf0cdebfeb593e7e77ff9cc17009ddd6cc143e   5.83MB
docker.io/library/myimage                       debug                                                              sha256:6fd35340bdcc6affc36acdede3fe25e14260c47786ff40052d0823a03e6cf7f8   sha256:3639e066b378c1e720cef8225adc0d3b4dac2cb7b3a8bc484ecbc6c9f9d388ad   5.83MB
root@demo-control-plane:/#

If I just re-tag the old image based on the hash match to the new one instead of loading it, seems like there will now be two images with the same name and tag. Unless one is using the sha value for referencing the image, won't this be confusing ? @BenTheElder @gyorokpeter

[gyorokpeter]

Shouldn't the sha256 uniquely identify the image? In that case it should never happen that there are two images with the same hash. However to me this is implementation detail, as a user I only care about the fact that the image should be present in the cluster and resetting the tag to an already existing image should be quick as it should be a similar operation to a file move.

[harshanarayana]

@gyorokpeter

kubectl run nginx --image myimage:debug I ran this command and the following happens. This can be super confusing if one forgets the re-tagging bit.

root@demo-control-plane:/# crictl images --digests --no-trunc | grep debug
docker.io/library/myimage                       debug                                                              sha256:50b471c68797a477c88b63d9c4a3c36e82c2335efdbd49590ef7ad49d4843e37   sha256:0d233417b3c4b8c50158223434bf0cdebfeb593e7e77ff9cc17009ddd6cc143e   5.83MB
docker.io/library/myimage                       debug                                                              sha256:6fd35340bdcc6affc36acdede3fe25e14260c47786ff40052d0823a03e6cf7f8   sha256:3639e066b378c1e720cef8225adc0d3b4dac2cb7b3a8bc484ecbc6c9f9d388ad   5.83MB
root@demo-control-plane:/# kubectl describe pods nginx | grep Image
    Image:          myimage:debug
    Image ID:       docker.io/library/import-2022-09-13@sha256:50b471c68797a477c88b63d9c4a3c36e82c2335efdbd49590ef7ad49d4843e37
root@demo-control-plane:/#
root@demo-control-plane:/# crictl images --digests --no-trunc | grep import-2022-09-13
root@demo-control-plane:/#

this is the confusion I am talking about. If you notice, the image used for the Pod, docker.io/library/import-2022-09-13@sha256:50b471c68797a477c88b63d9c4a3c36e82c2335efdbd49590ef7ad49d4843e37 this points to something called docker.io/library/import-2022-09-13 but with hash. But that hash maps to myimage:debug

I think this is more of a behavior inconsistency in how containerd manages the images .

I will open a PR that optimizes the image load but I think the behavior one will see inside the pod can be a bit misleading.

[BenTheElder]

That doesn't seem right, I haven't had a chance to dig back into this but maybe we need to untag the old one when adding the tag? Possibly a missing feature of ctr, or a flag we're missing. We should match the behavior when a new image digest is pulled for a tag, this is meant to be a pull alternative.

[harshanarayana]

Agreed @BenTheElder I am trying to find what needs to be done to get the tag behavior sorted. Will update here what I can find later in the day.

[Aryan-Deshpande]

hey @harshanarayana can I help on this issue as well, would like to learn and help!

[harshanarayana]

@Aryan-Deshpande

diff --git a/pkg/cmd/kind/load/docker-image/docker-image.go b/pkg/cmd/kind/load/docker-image/docker-image.go
index 162b0601..396e7ef7 100644
--- a/pkg/cmd/kind/load/docker-image/docker-image.go
+++ b/pkg/cmd/kind/load/docker-image/docker-image.go
@@ -136,7 +136,7 @@ func runE(logger log.Logger, flags *flagpole, args []string) error {
                imageID := imageIDs[i]
                processed := false
                for _, node := range candidateNodes {
-                       exists, reTagRequired := checkIfImageReTagRequired(node, imageID, imageName, nodeutils.ImageTags)
+                       exists, reTagRequired := checkIfImageReTagRequired(logger, node, imageID, imageName, nodeutils.ImageTags)
                        if exists && !reTagRequired {
                                continue
                        }
@@ -241,9 +241,16 @@ func removeDuplicates(slice []string) []string {
 }

 // checkIfImageExists makes sure we only perform the reverse lookup of the ImageID to tag map
-func checkIfImageReTagRequired(node nodes.Node, imageID, imageName string, tagFetcher imageTagFetcher) (exists, reTagRequired bool) {
+func checkIfImageReTagRequired(logger log.Logger, node nodes.Node, imageID, imageName string, tagFetcher imageTagFetcher) (exists, reTagRequired bool) {
        tags, err := tagFetcher(node, imageID)
        if len(tags) == 0 || err != nil {
+               id, _ := nodeutils.ImageID(node, imageID)
+               if id != "" {
+                       reTagRequired = true
+                       exists = true
+                       err = nil
+                       return
+               }
                exists = false
                return
        }

This is the patchset I am working with. I have not had any luck identifying the weird behavior I am noticing.

root@kind-control-plane:/# crictl images | grep -e myimage -e import
docker.io/library/import-2022-10-06        <none>               0d233417b3c4b       5.83MB
docker.io/library/myimage                  debug                1b00977d1ccce       151MB
docker.io/library/myimage                  debug                2dc39ba059dcd       80.4MB
root@kind-control-plane:/# crictl inspecti 0d233417b3c4b  | jq .status.id
"sha256:0d233417b3c4b8c50158223434bf0cdebfeb593e7e77ff9cc17009ddd6cc143e"
root@kind-control-plane:/# crictl inspecti 0d233417b3c4b  | jq -r .status.id
sha256:0d233417b3c4b8c50158223434bf0cdebfeb593e7e77ff9cc17009ddd6cc143e
root@kind-control-plane:/# ctr -n k8s.io i tag sha256:0d233417b3c4b8c50158223434bf0cdebfeb593e7e77ff9cc17009ddd6cc143e docker.io/library/myimage:debug --force
docker.io/library/myimage:debug
root@kind-control-plane:/# crictl images | grep -e myimage -e import
docker.io/library/myimage                  debug                0d233417b3c4b       5.83MB
docker.io/library/myimage                  debug                1b00977d1ccce       151MB
docker.io/library/import-2022-10-06        <none>               2dc39ba059dcd       80.4MB
root@kind-control-plane:/#

@BenTheElder Here is some weird nature assocaited with the ctr i tag command. Which has no argument other than --force

import-2022-10-06 part is something that gets put in place by containerd when I retag a different ID with the same tag that is already present. The date bit changes accordingly.