diff --git a/pkg/minikube/machine/cache_image_test.go b/pkg/minikube/machine/cache_image_test.go new file mode 100644 index 000000000000..f41ea89efadd --- /dev/null +++ b/pkg/minikube/machine/cache_image_test.go @@ -0,0 +1,246 @@ +/* +Copyright 2023 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package machine + +import ( + "reflect" + "sort" + "testing" + + "k8s.io/minikube/pkg/minikube/cruntime" +) + +func TestMergeImageLists(t *testing.T) { + testCases := [][][]cruntime.ListImage{ + // test case 0: from #16556 + // e.g. on node 1 image1 have an item with k8s.gcr.io/image1:v1.0.0 tag + // and another item with registry.k8s.io/image1:v1.0.0 tag too + { + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"registry.k8s.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"k8s.gcr.io/image2:v1.0.0"}, + Size: "1", + }, + }, + }, + // test case 1: from #16557 + // e.g. image1 have k8s.gcr.io/image1:v1.0.0 tag on node 1 and registry.k8s.io/image1:v1.0.0 on node 2 + { + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + }, + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"registry.k8s.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"k8s.gcr.io/image2:v1.0.0"}, + Size: "1", + }, + }, + }, + // test case 2: from #16557 + // e.g. image1 have k8s.gcr.io/image1:v1.0.0 tag on node 1 + // and both k8s.gcr.io/image1:v1.0.0 and registry.k8s.io/image1:v1.0.0 on node 2 + { + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"registry.k8s.io/image2:v1.0.0", "k8s.gcr.io/image2:v1.0.0"}, + Size: "1", + }, + }, + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"registry.k8s.io/image1:v1.0.0", "k8s.gcr.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"k8s.gcr.io/image2:v1.0.0"}, + Size: "1", + }, + }, + }, + // test case 3: normal case + { + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + }, + { + { + ID: "image_id_3", + RepoDigests: []string{"image_digest_3"}, + RepoTags: []string{"registry.k8s.io/image3:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_4", + RepoDigests: []string{"image_digest_4"}, + RepoTags: []string{"k8s.gcr.io/image4:v1.0.0"}, + Size: "1", + }, + }, + }, + } + testResult := [][]cruntime.ListImage{ + // result for test case 0 + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0", "registry.k8s.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"k8s.gcr.io/image2:v1.0.0", "registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + }, + // result for test case 1 + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0", "registry.k8s.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"k8s.gcr.io/image2:v1.0.0", "registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + }, + // result for test case 2 + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0", "registry.k8s.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"k8s.gcr.io/image2:v1.0.0", "registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + }, + // result for test case 3 + { + { + ID: "image_id_1", + RepoDigests: []string{"image_digest_1"}, + RepoTags: []string{"k8s.gcr.io/image1:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_2", + RepoDigests: []string{"image_digest_2"}, + RepoTags: []string{"registry.k8s.io/image2:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_3", + RepoDigests: []string{"image_digest_3"}, + RepoTags: []string{"registry.k8s.io/image3:v1.0.0"}, + Size: "1", + }, + { + ID: "image_id_4", + RepoDigests: []string{"image_digest_4"}, + RepoTags: []string{"k8s.gcr.io/image4:v1.0.0"}, + Size: "1", + }, + }, + } + for index, testCase := range testCases { + actualResult := mergeImageLists(testCase) + sort.Slice(actualResult, func(i, j int) bool { + return actualResult[i].ID < actualResult[j].ID + }) + for _, img := range actualResult { + sort.Slice(img.RepoTags, func(i, j int) bool { + return img.RepoTags[i] < img.RepoTags[j] + }) + } + if ok := reflect.DeepEqual(actualResult, testResult[index]); !ok { + t.Errorf("Failed in testcase %d,\n actual %v,\n expected %v", index, actualResult, testResult[index]) + } + } +} diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 6f9f5be616ac..39464788d2fe 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -686,7 +686,7 @@ func ListImages(profile *config.Profile, format string) error { return errors.Wrapf(err, "error loading config for profile :%v", pName) } - images := map[string]cruntime.ListImage{} + imageListsFromNodes := [][]cruntime.ListImage{} for _, n := range c.Nodes { m := config.MachineName(*c, n) @@ -715,19 +715,12 @@ func ListImages(profile *config.Profile, format string) error { klog.Warningf("Failed to list images for profile %s %v", pName, err.Error()) continue } + imageListsFromNodes = append(imageListsFromNodes, list) - for _, img := range list { - if _, ok := images[img.ID]; !ok { - images[img.ID] = img - } - } } } - uniqueImages := []cruntime.ListImage{} - for _, img := range images { - uniqueImages = append(uniqueImages, img) - } + uniqueImages := mergeImageLists(imageListsFromNodes) switch format { case "table": @@ -770,6 +763,42 @@ func ListImages(profile *config.Profile, format string) error { return nil } +// mergeImageLists merges image lists from different nodes into a single list +// all the repo tags of the same image will be preserved and grouped in one image item +func mergeImageLists(lists [][]cruntime.ListImage) []cruntime.ListImage { + images := map[string]*cruntime.ListImage{} + // key of this set is img.ID+img.repoTag + // we use this set to detect whether a tag of a image is included in images map + imageTagAndIDSet := map[string]struct{}{} + for _, list := range lists { + for _, img := range list { + img := img + if _, ok := images[img.ID]; !ok { + images[img.ID] = &img + // for a new image id, insert all repotags into the set + for _, repoTag := range img.RepoTags { + imageTagAndIDSet[img.ID+repoTag] = struct{}{} + } + } else { + for _, repoTag := range img.RepoTags { + if _, ok := imageTagAndIDSet[img.ID+repoTag]; !ok { + // if there is any repo tag which is not included in the map's corresponding item + // add it to the repo tag list + imageTagAndIDSet[img.ID+repoTag] = struct{}{} + images[img.ID].RepoTags = append(images[img.ID].RepoTags, repoTag) + } + } + } + } + + } + uniqueImages := []cruntime.ListImage{} + for _, img := range images { + uniqueImages = append(uniqueImages, *img) + } + return uniqueImages +} + // parseRepoTag splits input string for two parts: image name and image tag func parseRepoTag(repoTag string) (string, string) { idx := strings.LastIndex(repoTag, ":")