From 6d4eec056be83bf5afeac7f963be156d60f6f35f Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Fri, 6 Aug 2021 00:24:28 +0000 Subject: [PATCH 1/5] fix loading an image from tar failing on existing delete --- pkg/minikube/machine/cache_images.go | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 36c2fe2ad446..ad97ddcf8d93 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -269,11 +269,8 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, src st return errors.Wrap(err, "runtime") } - if err := r.RemoveImage(imgName); err != nil { - errStr := strings.ToLower(err.Error()) - if !strings.Contains(errStr, "no such image") { - return errors.Wrap(err, "removing image") - } + if err := removeExistingImage(r, src, imgName); err != nil { + return err } klog.Infof("Loading image from: %s", src) @@ -309,6 +306,26 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, src st return nil } +func removeExistingImage(r cruntime.Manager, src string, imgName string) error { + // if loading an image from tar, skip deleting as we don't have the actual image name + // ie. imgName = "C:\this_is_a_dir\image.tar.gz" + if src == imgName { + return nil + } + + err := r.RemoveImage(imgName) + if err == nil { + return nil + } + + errStr := strings.ToLower(err.Error()) + if !strings.Contains(errStr, "no such image") { + return errors.Wrap(err, "removing image") + } + + return nil +} + // pullImages pulls images to the container run time func pullImages(cruntime cruntime.Manager, images []string) error { klog.Infof("PullImages start: %s", images) From 9a955c23c4c05dc57eedbefb91289a0ff2507582 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Fri, 6 Aug 2021 15:01:38 -0700 Subject: [PATCH 2/5] added test that loads an image from a local file --- test/integration/functional_test.go | 51 +++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 0516e39712e1..6dbca5e7443c 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -152,6 +152,7 @@ func TestFunctional(t *testing.T) { {"NodeLabels", validateNodeLabels}, {"LoadImage", validateLoadImage}, {"RemoveImage", validateRemoveImage}, + {"LoadImageFromFile", validateLoadImageFromFile}, {"BuildImage", validateBuildImage}, {"ListImages", validateListImages}, {"NonActiveRuntimeDisabled", validateNotActiveRuntimeDisabled}, @@ -264,6 +265,56 @@ func validateLoadImage(ctx context.Context, t *testing.T, profile string) { } +// validateLoadImageFromFile makes sure that `minikube image load` works from a local file +func validateLoadImageFromFile(ctx context.Context, t *testing.T, profile string) { + if NoneDriver() { + t.Skip("load image not available on none driver") + } + if GithubActionRunner() && runtime.GOOS == "darwin" { + t.Skip("skipping on github actions and darwin, as this test requires a running docker daemon") + } + defer PostMortemLogs(t, profile) + // pull busybox + busyboxImage := "busybox:1.31" + rr, err := Run(t, exec.CommandContext(ctx, "docker", "pull", busyboxImage)) + if err != nil { + t.Fatalf("failed to setup test (pull image): %v\n%s", err, rr.Output()) + } + + newImage := fmt.Sprintf("docker.io/library/busybox:load-from-file-%s", profile) + rr, err = Run(t, exec.CommandContext(ctx, "docker", "tag", busyboxImage, newImage)) + if err != nil { + t.Fatalf("failed to setup test (tag image) : %v\n%s", err, rr.Output()) + } + + // save image to file + imageFile := "busybox.tar.gz" + rr, err := Run(t, exec.CommandContext(ctx, "docker", "save", newImage, "|", "gzip", ">", imageFile)) + if err != nil { + t.Fatalf("failed to save image to file: %v\n%s", err, rr.Output()) + } + defer os.Remove(imageFile) + + // try to load the new image into minikube + imagePath, err := filepath.Abs(imageFile) + if err != nil { + t.Fatalf("failed to get absolute path of file %q: %v", imageFile, err) + } + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "image", "load", imagePath)) + if err != nil { + t.Fatalf("loading image into minikube: %v\n%s", err, rr.Output()) + } + + // make sure the image was correctly loaded + rr, err = inspectImage(ctx, t, profile, newImage) + if err != nil { + t.Fatalf("listing images: %v\n%s", err, rr.Output()) + } + if !strings.Contains(rr.Output(), fmt.Sprintf("busybox:load-from-file-%s", profile)) { + t.Fatalf("expected %s to be loaded into minikube but the image is not there", newImage) + } +} + // validateRemoveImage makes sures that `minikube image rm` works as expected func validateRemoveImage(ctx context.Context, t *testing.T, profile string) { if NoneDriver() { From b859ede3c014ccf8b30e0f8f8d99807009c99dc1 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Fri, 6 Aug 2021 15:08:35 -0700 Subject: [PATCH 3/5] fix linting --- test/integration/functional_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 6dbca5e7443c..b06f79707771 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -289,7 +289,7 @@ func validateLoadImageFromFile(ctx context.Context, t *testing.T, profile string // save image to file imageFile := "busybox.tar.gz" - rr, err := Run(t, exec.CommandContext(ctx, "docker", "save", newImage, "|", "gzip", ">", imageFile)) + rr, err = Run(t, exec.CommandContext(ctx, "docker", "save", newImage, "|", "gzip", ">", imageFile)) if err != nil { t.Fatalf("failed to save image to file: %v\n%s", err, rr.Output()) } From 51117f93ba632e750dfacbd20879420b9d3909c4 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Mon, 9 Aug 2021 10:52:53 -0700 Subject: [PATCH 4/5] fixed test --- test/integration/functional_test.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index b06f79707771..91ec1e1ea187 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -281,15 +281,9 @@ func validateLoadImageFromFile(ctx context.Context, t *testing.T, profile string t.Fatalf("failed to setup test (pull image): %v\n%s", err, rr.Output()) } - newImage := fmt.Sprintf("docker.io/library/busybox:load-from-file-%s", profile) - rr, err = Run(t, exec.CommandContext(ctx, "docker", "tag", busyboxImage, newImage)) - if err != nil { - t.Fatalf("failed to setup test (tag image) : %v\n%s", err, rr.Output()) - } - // save image to file - imageFile := "busybox.tar.gz" - rr, err = Run(t, exec.CommandContext(ctx, "docker", "save", newImage, "|", "gzip", ">", imageFile)) + imageFile := "busybox.tar" + rr, err = Run(t, exec.CommandContext(ctx, "docker", "save", "-o", imageFile, busyboxImage)) if err != nil { t.Fatalf("failed to save image to file: %v\n%s", err, rr.Output()) } @@ -306,12 +300,12 @@ func validateLoadImageFromFile(ctx context.Context, t *testing.T, profile string } // make sure the image was correctly loaded - rr, err = inspectImage(ctx, t, profile, newImage) + rr, err = listImages(ctx, t, profile) if err != nil { t.Fatalf("listing images: %v\n%s", err, rr.Output()) } - if !strings.Contains(rr.Output(), fmt.Sprintf("busybox:load-from-file-%s", profile)) { - t.Fatalf("expected %s to be loaded into minikube but the image is not there", newImage) + if !strings.Contains(rr.Output(), busyboxImage) { + t.Fatalf("expected %s to be loaded into minikube but the image is not there", busyboxImage) } } From 4ffd0de205e883af868d98d36584ec3f9a8fb8a5 Mon Sep 17 00:00:00 2001 From: Steven Powell Date: Mon, 9 Aug 2021 13:41:44 -0700 Subject: [PATCH 5/5] fix test --- test/integration/functional_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index 91ec1e1ea187..10bee2cbbfd7 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -281,9 +281,16 @@ func validateLoadImageFromFile(ctx context.Context, t *testing.T, profile string t.Fatalf("failed to setup test (pull image): %v\n%s", err, rr.Output()) } + tag := fmt.Sprintf("load-from-file-%s", profile) + taggedImage := fmt.Sprintf("docker.io/library/busybox:%s", tag) + rr, err = Run(t, exec.CommandContext(ctx, "docker", "tag", busyboxImage, taggedImage)) + if err != nil { + t.Fatalf("failed to setup test (tag image) : %v\n%s", err, rr.Output()) + } + // save image to file imageFile := "busybox.tar" - rr, err = Run(t, exec.CommandContext(ctx, "docker", "save", "-o", imageFile, busyboxImage)) + rr, err = Run(t, exec.CommandContext(ctx, "docker", "save", "-o", imageFile, taggedImage)) if err != nil { t.Fatalf("failed to save image to file: %v\n%s", err, rr.Output()) } @@ -304,8 +311,8 @@ func validateLoadImageFromFile(ctx context.Context, t *testing.T, profile string if err != nil { t.Fatalf("listing images: %v\n%s", err, rr.Output()) } - if !strings.Contains(rr.Output(), busyboxImage) { - t.Fatalf("expected %s to be loaded into minikube but the image is not there", busyboxImage) + if !strings.Contains(rr.Output(), tag) { + t.Fatalf("expected %s to be loaded into minikube but the image is not there", taggedImage) } }