From b9b657ca859b460c57c6e46d87f1b114fa29c5fd Mon Sep 17 00:00:00 2001 From: Tharun Date: Thu, 25 Mar 2021 16:53:03 +0530 Subject: [PATCH 1/6] add minikube rm image command Signed-off-by: Tharun --- cmd/minikube/cmd/image.go | 20 ++++++ pkg/minikube/cruntime/containerd.go | 5 ++ pkg/minikube/cruntime/cri.go | 13 ++++ pkg/minikube/cruntime/crio.go | 5 ++ pkg/minikube/cruntime/cruntime.go | 3 + pkg/minikube/cruntime/cruntime_test.go | 38 ++++++++++- pkg/minikube/cruntime/docker.go | 10 +++ pkg/minikube/machine/cache_images.go | 87 ++++++++++++++++++++++++++ pkg/minikube/reason/reason.go | 1 + site/content/en/docs/commands/image.md | 40 ++++++++++++ 10 files changed, 221 insertions(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/image.go b/cmd/minikube/cmd/image.go index de3e80e389fa..1640d937c153 100644 --- a/cmd/minikube/cmd/image.go +++ b/cmd/minikube/cmd/image.go @@ -125,8 +125,28 @@ var loadImageCmd = &cobra.Command{ }, } +var removeImageCmd = &cobra.Command{ + Use: "rm IMAGE [IMAGE...]", + Short: "Remove one or more images", + Long: "Load a image into minikube", + Example: "minikube image rm image busybox", + Run: func(cmd *cobra.Command, args []string) { + if len(args) == 0 { + exit.Message(reason.Usage, "Please provide an image to remove via ") + } + profile, err := config.LoadProfile(viper.GetString(config.ProfileName)) + if err != nil { + exit.Error(reason.Usage, "loading profile", err) + } + if err := machine.RemoveImages(args, []*config.Profile{profile}); err != nil { + exit.Error(reason.GuestImageRemove, "Failed to remove image", err) + } + }, +} + func init() { imageCmd.AddCommand(loadImageCmd) + imageCmd.AddCommand(removeImageCmd) loadImageCmd.Flags().BoolVar(&imgDaemon, "daemon", false, "Cache image from docker daemon") loadImageCmd.Flags().BoolVar(&imgRemote, "remote", false, "Cache image from remote registry") } diff --git a/pkg/minikube/cruntime/containerd.go b/pkg/minikube/cruntime/containerd.go index dee2a1dfd950..ca937cca9162 100644 --- a/pkg/minikube/cruntime/containerd.go +++ b/pkg/minikube/cruntime/containerd.go @@ -249,6 +249,11 @@ func (r *Containerd) LoadImage(path string) error { return nil } +// RemoveImage removes a image +func (r *Containerd) RemoveImage(name string) error { + return removeCRIImage(r.Runner, name) +} + // CGroupDriver returns cgroup driver ("cgroupfs" or "systemd") func (r *Containerd) CGroupDriver() (string, error) { info, err := getCRIInfo(r.Runner) diff --git a/pkg/minikube/cruntime/cri.go b/pkg/minikube/cruntime/cri.go index 1a9b46a0c7df..5ad274928707 100644 --- a/pkg/minikube/cruntime/cri.go +++ b/pkg/minikube/cruntime/cri.go @@ -187,6 +187,19 @@ func killCRIContainers(cr CommandRunner, ids []string) error { return nil } +// removeCRIImage remove image using crictl +func removeCRIImage(cr CommandRunner, name string) error { + klog.Infof("Removing image: %s", name) + + crictl := getCrictlPath(cr) + args := append([]string{crictl, "rmi"}, name) + c := exec.Command("sudo", args...) + if _, err := cr.RunCmd(c); err != nil { + return errors.Wrap(err, "crictl") + } + return nil +} + // stopCRIContainers stops containers using crictl func stopCRIContainers(cr CommandRunner, ids []string) error { if len(ids) == 0 { diff --git a/pkg/minikube/cruntime/crio.go b/pkg/minikube/cruntime/crio.go index 8483752d8682..63d961aed3a9 100644 --- a/pkg/minikube/cruntime/crio.go +++ b/pkg/minikube/cruntime/crio.go @@ -177,6 +177,11 @@ func (r *CRIO) LoadImage(path string) error { return nil } +// RemoveImage removes a image +func (r *CRIO) RemoveImage(name string) error { + return removeCRIImage(r.Runner, name) +} + // CGroupDriver returns cgroup driver ("cgroupfs" or "systemd") func (r *CRIO) CGroupDriver() (string, error) { c := exec.Command("crio", "config") diff --git a/pkg/minikube/cruntime/cruntime.go b/pkg/minikube/cruntime/cruntime.go index f7111e91aff7..dd60a432a9e9 100644 --- a/pkg/minikube/cruntime/cruntime.go +++ b/pkg/minikube/cruntime/cruntime.go @@ -99,6 +99,9 @@ type Manager interface { // ImageExists takes image name and image sha checks if an it exists ImageExists(string, string) bool + // RemoveImage remove image based on name + RemoveImage(string) error + // ListContainers returns a list of managed by this container runtime ListContainers(ListOptions) ([]string, error) // KillContainers removes containers based on ID diff --git a/pkg/minikube/cruntime/cruntime_test.go b/pkg/minikube/cruntime/cruntime_test.go index 461809e56894..fc783c2aebcb 100644 --- a/pkg/minikube/cruntime/cruntime_test.go +++ b/pkg/minikube/cruntime/cruntime_test.go @@ -157,6 +157,7 @@ type FakeRunner struct { cmds []string services map[string]serviceState containers map[string]string + images map[string]string t *testing.T } @@ -167,6 +168,7 @@ func NewFakeRunner(t *testing.T) *FakeRunner { cmds: []string{}, t: t, containers: map[string]string{}, + images: map[string]string{}, } } @@ -285,6 +287,18 @@ func (f *FakeRunner) dockerInspect(args []string) (string, error) { return "", nil } +func (f *FakeRunner) dockerRmi(args []string) (string, error) { + // Skip "-f" argument + for _, id := range args[1:] { + f.t.Logf("fake docker: Removing id %q", id) + if f.images[id] == "" { + return "", fmt.Errorf("no such image") + } + delete(f.images, id) + } + return "", nil +} + // docker is a fake implementation of docker func (f *FakeRunner) docker(args []string, _ bool) (string, error) { switch cmd := args[0]; cmd { @@ -308,6 +322,9 @@ func (f *FakeRunner) docker(args []string, _ bool) (string, error) { return f.dockerInspect(args[1:]) } + case "rmi": + return f.dockerRmi(args) + case "inspect": return f.dockerInspect(args) @@ -417,7 +434,14 @@ func (f *FakeRunner) crictl(args []string, _ bool) (string, error) { delete(f.containers, id) } - + case "rmi": + for _, id := range args[1:] { + f.t.Logf("fake crictl: Removing id %q", id) + if f.images[id] == "" { + return "", fmt.Errorf("no such image") + } + delete(f.images, id) + } } return "", nil } @@ -660,6 +684,9 @@ func TestContainerFunctions(t *testing.T) { "fgh1": prefix + "coredns", "xyz2": prefix + "storage", } + runner.images = map[string]string{ + "image1": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + } cr, err := New(Config{Type: tc.runtime, Runner: runner}) if err != nil { t.Fatalf("New(%s): %v", tc.runtime, err) @@ -709,6 +736,15 @@ func TestContainerFunctions(t *testing.T) { if len(got) > 0 { t.Errorf("ListContainers(apiserver) = %v, want 0 items", got) } + + // Remove a image + if err := cr.RemoveImage("image1"); err != nil { + t.Fatalf("RemoveImage: %v", err) + } + // imageExists := cr.ImageExists("image1", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") + if len(runner.images) > 0 { + t.Errorf("RemoveImage = %v, want 0 items", len(runner.images)) + } }) } } diff --git a/pkg/minikube/cruntime/docker.go b/pkg/minikube/cruntime/docker.go index 6ce546e9fc6f..40efb9c8f4e7 100644 --- a/pkg/minikube/cruntime/docker.go +++ b/pkg/minikube/cruntime/docker.go @@ -172,6 +172,16 @@ func (r *Docker) LoadImage(path string) error { return nil } +// RemoveImage removes a image +func (r *Docker) RemoveImage(name string) error { + klog.Infof("Removing image: %s", name) + c := exec.Command("docker", "rmi", name) + if _, err := r.Runner.RunCmd(c); err != nil { + return errors.Wrap(err, "remove image docker.") + } + return nil +} + // CGroupDriver returns cgroup driver ("cgroupfs" or "systemd") func (r *Docker) CGroupDriver() (string, error) { // Note: the server daemon has to be running, for this call to return successfully diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 304d9a6b7085..255c655e4ee3 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -292,3 +292,90 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, src st klog.Infof("Transferred and loaded %s from cache", src) return nil } + +// removeImages removes images from the container run time +func removeImages(cc *config.ClusterConfig, runner command.Runner, images []string) error { + cr, err := cruntime.New(cruntime.Config{Type: cc.KubernetesConfig.ContainerRuntime, Runner: runner}) + if err != nil { + return errors.Wrap(err, "runtime") + } + + klog.Infof("RemovingImages start: %s", images) + start := time.Now() + + defer func() { + klog.Infof("RemovingImages completed in %s", time.Since(start)) + }() + + var g errgroup.Group + + for _, image := range images { + image := image + g.Go(func() error { + return cr.RemoveImage(image) + }) + } + if err := g.Wait(); err != nil { + return errors.Wrap(err, "removing images") + } + klog.Infoln("Successfully removed images") + return nil +} + +func RemoveImages(images []string, profiles []*config.Profile) error { + api, err := NewAPIClient() + if err != nil { + return errors.Wrap(err, "api") + } + defer api.Close() + + succeeded := []string{} + failed := []string{} + + for _, p := range profiles { // loading images to all running profiles + pName := p.Name // capture the loop variable + + c, err := config.Load(pName) + if err != nil { + // Non-fatal because it may race with profile deletion + klog.Errorf("Failed to load profile %q: %v", pName, err) + failed = append(failed, pName) + continue + } + + for _, n := range c.Nodes { + m := config.MachineName(*c, n) + + status, err := Status(api, m) + if err != nil { + klog.Warningf("error getting status for %s: %v", m, err) + failed = append(failed, m) + continue + } + + if status == state.Running.String() { // the not running hosts will load on next start + h, err := api.Load(m) + if err != nil { + klog.Warningf("Failed to load machine %q: %v", m, err) + failed = append(failed, m) + continue + } + cr, err := CommandRunner(h) + if err != nil { + return err + } + err = removeImages(c, cr, images) + if err != nil { + failed = append(failed, m) + klog.Warningf("Failed to load cached images for profile %s. make sure the profile is running. %v", pName, err) + continue + } + succeeded = append(succeeded, m) + } + } + } + + klog.Infof("succeeded removing to: %s", strings.Join(succeeded, " ")) + klog.Infof("failed removing to: %s", strings.Join(failed, " ")) + return nil +} diff --git a/pkg/minikube/reason/reason.go b/pkg/minikube/reason/reason.go index 854547b14f05..1bd8ebb20610 100644 --- a/pkg/minikube/reason/reason.go +++ b/pkg/minikube/reason/reason.go @@ -246,6 +246,7 @@ var ( GuestCpConfig = Kind{ID: "GUEST_CP_CONFIG", ExitCode: ExGuestConfig} GuestDeletion = Kind{ID: "GUEST_DELETION", ExitCode: ExGuestError} GuestImageLoad = Kind{ID: "GUEST_IMAGE_LOAD", ExitCode: ExGuestError} + GuestImageRemove = Kind{ID: "GUEST_IMAGE_REMOVE", ExitCode: ExGuestError} GuestLoadHost = Kind{ID: "GUEST_LOAD_HOST", ExitCode: ExGuestError} GuestMount = Kind{ID: "GUEST_MOUNT", ExitCode: ExGuestError} GuestMountConflict = Kind{ID: "GUEST_MOUNT_CONFLICT", ExitCode: ExGuestConflict} diff --git a/site/content/en/docs/commands/image.md b/site/content/en/docs/commands/image.md index af13106b244d..86c1c51ae405 100644 --- a/site/content/en/docs/commands/image.md +++ b/site/content/en/docs/commands/image.md @@ -118,3 +118,43 @@ minikube image load image.tar --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging ``` +## minikube image rm + +Remove one or more images + +### Synopsis + +Load a image into minikube + +```shell +minikube image rm IMAGE [IMAGE...] [flags] +``` + +### Examples + +``` +minikube image rm image busybox +``` + +### Options inherited from parent commands + +``` + --add_dir_header If true, adds the file directory to the header of the log messages + --alsologtostderr log to standard error as well as files + -b, --bootstrapper string The name of the cluster bootstrapper that will set up the Kubernetes cluster. (default "kubeadm") + -h, --help + --log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0) + --log_dir string If non-empty, write log files in this directory + --log_file string If non-empty, use this log file + --log_file_max_size uint Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800) + --logtostderr log to standard error instead of files + --one_output If true, only write logs to their native severity level (vs also writing to each lower severity level) + -p, --profile string The name of the minikube VM being used. This can be set to allow having multiple instances of minikube independently. (default "minikube") + --skip_headers If true, avoid header prefixes in the log messages + --skip_log_headers If true, avoid headers when opening log files + --stderrthreshold severity logs at or above this threshold go to stderr (default 2) + --user string Specifies the user executing the operation. Useful for auditing operations executed by 3rd party tools. Defaults to the operating system username. + -v, --v Level number for the log level verbosity + --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging +``` + From 82d7fcc65f99a8c0013748646085579f45900273 Mon Sep 17 00:00:00 2001 From: Tharun Date: Thu, 25 Mar 2021 19:32:13 +0530 Subject: [PATCH 2/6] address log and docs feedback Signed-off-by: Tharun --- cmd/minikube/cmd/image.go | 2 +- pkg/minikube/cruntime/docker.go | 3 +++ pkg/minikube/machine/cache_images.go | 6 +++--- site/content/en/docs/commands/image.md | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/minikube/cmd/image.go b/cmd/minikube/cmd/image.go index 1640d937c153..c0d80d654583 100644 --- a/cmd/minikube/cmd/image.go +++ b/cmd/minikube/cmd/image.go @@ -128,7 +128,7 @@ var loadImageCmd = &cobra.Command{ var removeImageCmd = &cobra.Command{ Use: "rm IMAGE [IMAGE...]", Short: "Remove one or more images", - Long: "Load a image into minikube", + Long: "Remove a image from minikube", Example: "minikube image rm image busybox", Run: func(cmd *cobra.Command, args []string) { if len(args) == 0 { diff --git a/pkg/minikube/cruntime/docker.go b/pkg/minikube/cruntime/docker.go index 40efb9c8f4e7..84a6a59b7565 100644 --- a/pkg/minikube/cruntime/docker.go +++ b/pkg/minikube/cruntime/docker.go @@ -175,6 +175,9 @@ func (r *Docker) LoadImage(path string) error { // RemoveImage removes a image func (r *Docker) RemoveImage(name string) error { klog.Infof("Removing image: %s", name) + if r.UseCRI { + return removeCRIImage(r.Runner, name) + } c := exec.Command("docker", "rmi", name) if _, err := r.Runner.RunCmd(c); err != nil { return errors.Wrap(err, "remove image docker.") diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 255c655e4ee3..03236fb27ddf 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -367,7 +367,7 @@ func RemoveImages(images []string, profiles []*config.Profile) error { err = removeImages(c, cr, images) if err != nil { failed = append(failed, m) - klog.Warningf("Failed to load cached images for profile %s. make sure the profile is running. %v", pName, err) + klog.Warningf("Failed to remove images for profile %s. make sure the profile is running. %v", pName, err) continue } succeeded = append(succeeded, m) @@ -375,7 +375,7 @@ func RemoveImages(images []string, profiles []*config.Profile) error { } } - klog.Infof("succeeded removing to: %s", strings.Join(succeeded, " ")) - klog.Infof("failed removing to: %s", strings.Join(failed, " ")) + klog.Infof("succeeded removing from: %s", strings.Join(succeeded, " ")) + klog.Infof("failed removing from: %s", strings.Join(failed, " ")) return nil } diff --git a/site/content/en/docs/commands/image.md b/site/content/en/docs/commands/image.md index 86c1c51ae405..1ca651be8e64 100644 --- a/site/content/en/docs/commands/image.md +++ b/site/content/en/docs/commands/image.md @@ -124,7 +124,7 @@ Remove one or more images ### Synopsis -Load a image into minikube +Remove a image from minikube ```shell minikube image rm IMAGE [IMAGE...] [flags] From 77ba3e93cbf90c238f5164989c37e92d70fdcf2b Mon Sep 17 00:00:00 2001 From: Tharun Date: Fri, 26 Mar 2021 01:48:35 +0530 Subject: [PATCH 3/6] refractor TestImageExists method Signed-off-by: Tharun --- pkg/minikube/cruntime/cruntime_test.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/pkg/minikube/cruntime/cruntime_test.go b/pkg/minikube/cruntime/cruntime_test.go index fc783c2aebcb..64cdf0af4169 100644 --- a/pkg/minikube/cruntime/cruntime_test.go +++ b/pkg/minikube/cruntime/cruntime_test.go @@ -63,14 +63,19 @@ func TestImageExists(t *testing.T) { sha string want bool }{ - {"docker", "missing", "0000000000000000000000000000000000000000000000000000000000000000", false}, - {"docker", "image", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", true}, - {"crio", "missing", "0000000000000000000000000000000000000000000000000000000000000000", false}, - {"crio", "image", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", true}, + {"docker", "missing-image", "0000000000000000000000000000000000000000000000000000000000000000", false}, + {"docker", "available-image", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", true}, + {"crio", "missing-image", "0000000000000000000000000000000000000000000000000000000000000000", false}, + {"crio", "available-image", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", true}, } for _, tc := range tests { + runner := NewFakeRunner(t) + runner.images = map[string]string{ + "available-image": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + } t.Run(tc.runtime, func(t *testing.T) { - r, err := New(Config{Type: tc.runtime, Runner: NewFakeRunner(t)}) + + r, err := New(Config{Type: tc.runtime, Runner: runner}) if err != nil { t.Fatalf("New(%s): %v", tc.runtime, err) } @@ -279,10 +284,11 @@ func (f *FakeRunner) dockerRm(args []string) (string, error) { func (f *FakeRunner) dockerInspect(args []string) (string, error) { if args[1] == "--format" && args[2] == "{{.Id}}" { - if args[3] == "missing" { + image, ok := f.images[args[3]] + if !ok { return "", &exec.ExitError{Stderr: []byte("Error: No such object: missing")} } - return "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", nil + return "sha256:" + image, nil } return "", nil } @@ -685,7 +691,7 @@ func TestContainerFunctions(t *testing.T) { "xyz2": prefix + "storage", } runner.images = map[string]string{ - "image1": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "image1": "latest", } cr, err := New(Config{Type: tc.runtime, Runner: runner}) if err != nil { @@ -741,7 +747,6 @@ func TestContainerFunctions(t *testing.T) { if err := cr.RemoveImage("image1"); err != nil { t.Fatalf("RemoveImage: %v", err) } - // imageExists := cr.ImageExists("image1", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") if len(runner.images) > 0 { t.Errorf("RemoveImage = %v, want 0 items", len(runner.images)) } From 3503c3ea7f02de5bbdc835279b37f70e3ce8d915 Mon Sep 17 00:00:00 2001 From: Tharun Date: Fri, 26 Mar 2021 01:50:12 +0530 Subject: [PATCH 4/6] add integration test for minikube rm image command Signed-off-by: Tharun --- test/integration/functional_test.go | 72 +++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/test/integration/functional_test.go b/test/integration/functional_test.go index aa7432e96231..4e921a73f9a1 100644 --- a/test/integration/functional_test.go +++ b/test/integration/functional_test.go @@ -133,6 +133,7 @@ func TestFunctional(t *testing.T) { {"DockerEnv", validateDockerEnv}, {"NodeLabels", validateNodeLabels}, {"LoadImage", validateLoadImage}, + {"RemoveImage", validateRemoveImage}, } for _, tc := range tests { tc := tc @@ -219,27 +220,80 @@ func validateLoadImage(ctx context.Context, t *testing.T, profile string) { } // 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(), newImage) { + t.Fatalf("expected %s to be loaded into minikube but the image is not there", newImage) + } + +} + +// validateRemoveImage makes sures that `minikube rm image` works as expected +func validateRemoveImage(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:latest" + 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()) + } + + // try to load the image into minikube + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "image", "load", busyboxImage)) + if err != nil { + t.Fatalf("loading image into minikube: %v\n%s", err, rr.Output()) + } + + // try to remove the image from minikube + rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "image", "rm", busyboxImage)) + if err != nil { + t.Fatalf("removing image from minikube: %v\n%s", err, rr.Output()) + } + // make sure the image was removed var cmd *exec.Cmd if ContainerRuntime() == "docker" { - cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "docker", "image", "inspect", newImage) - } else if ContainerRuntime() == "containerd" { - // crictl inspecti busybox:test-example - cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "sudo", "crictl", "inspecti", newImage) + cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "docker", "images") } else { - // crio adds localhost prefix - // crictl inspecti localhost/busybox:test-example - cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "sudo", "crictl", "inspecti", "localhost/"+newImage) + cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "sudo", "crictl", "images") } rr, err = Run(t, cmd) if err != nil { t.Fatalf("listing images: %v\n%s", err, rr.Output()) } - if !strings.Contains(rr.Output(), newImage) { - 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 removed from minikube but the image is there", busyboxImage) } } +func inspectImage(ctx context.Context, t *testing.T, profile string, image string) (*RunResult, error) { + var cmd *exec.Cmd + if ContainerRuntime() == "docker" { + cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "docker", "image", "inspect", image) + } else if ContainerRuntime() == "containerd" { + // crictl inspecti busybox:test-example + cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "sudo", "crictl", "inspecti", image) + } else { + // crio adds localhost prefix + // crictl inspecti localhost/busybox:test-example + cmd = exec.CommandContext(ctx, Target(), "ssh", "-p", profile, "--", "sudo", "crictl", "inspecti", "localhost/"+image) + } + rr, err := Run(t, cmd) + if err != nil { + return rr, err + } + return rr, nil +} + // check functionality of minikube after evaling docker-env // TODO: Add validatePodmanEnv for crio runtime: #10231 func validateDockerEnv(ctx context.Context, t *testing.T, profile string) { From e37f8bc1a25ce9da6b6adc3a33b5abb53bd8ab5a Mon Sep 17 00:00:00 2001 From: Tharun Date: Fri, 26 Mar 2021 23:18:35 +0530 Subject: [PATCH 5/6] add unload as an alias and fixed parent command usage Signed-off-by: Tharun --- cmd/minikube/cmd/image.go | 17 ++++++++++------- pkg/generate/docs.go | 5 +++++ site/content/en/docs/commands/image.md | 18 +++++++++++++----- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/cmd/minikube/cmd/image.go b/cmd/minikube/cmd/image.go index c0d80d654583..1f69909565e1 100644 --- a/cmd/minikube/cmd/image.go +++ b/cmd/minikube/cmd/image.go @@ -33,9 +33,8 @@ import ( // imageCmd represents the image command var imageCmd = &cobra.Command{ - Use: "image", - Short: "Load a local image into minikube", - Long: "Load a local image into minikube", + Use: "image COMMAND", + Short: "Manage images", } var ( @@ -126,10 +125,14 @@ var loadImageCmd = &cobra.Command{ } var removeImageCmd = &cobra.Command{ - Use: "rm IMAGE [IMAGE...]", - Short: "Remove one or more images", - Long: "Remove a image from minikube", - Example: "minikube image rm image busybox", + Use: "rm IMAGE [IMAGE...]", + Short: "Remove one or more images", + Example: ` +$ minikube image rm image busybox + +$ minikube image unload image busybox +`, + Aliases: []string{"unload"}, Run: func(cmd *cobra.Command, args []string) { if len(args) == 0 { exit.Message(reason.Usage, "Please provide an image to remove via ") diff --git a/pkg/generate/docs.go b/pkg/generate/docs.go index 17a2c2cad1b4..b8cbcded797e 100644 --- a/pkg/generate/docs.go +++ b/pkg/generate/docs.go @@ -96,6 +96,11 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) buf.WriteString(fmt.Sprintf("```shell\n%s\n```\n\n", cmd.UseLine())) } + if len(cmd.Aliases) > 0 { + buf.WriteString("### Aliases\n\n") + buf.WriteString(fmt.Sprintf("%s\n\n", cmd.Aliases)) + } + if len(cmd.Example) > 0 { buf.WriteString("### Examples\n\n") buf.WriteString(fmt.Sprintf("```\n%s\n```\n\n", cmd.Example)) diff --git a/site/content/en/docs/commands/image.md b/site/content/en/docs/commands/image.md index 1ca651be8e64..d4a019adfefa 100644 --- a/site/content/en/docs/commands/image.md +++ b/site/content/en/docs/commands/image.md @@ -1,17 +1,17 @@ --- title: "image" description: > - Load a local image into minikube + Manage images --- ## minikube image -Load a local image into minikube +Manage images ### Synopsis -Load a local image into minikube +Manage images ### Options inherited from parent commands @@ -124,16 +124,24 @@ Remove one or more images ### Synopsis -Remove a image from minikube +Remove one or more images ```shell minikube image rm IMAGE [IMAGE...] [flags] ``` +### Aliases + +[unload] + ### Examples ``` -minikube image rm image busybox + +$ minikube image rm image busybox + +$ minikube image unload image busybox + ``` ### Options inherited from parent commands From f022babe1e68f8194f80c9291389012eb2e64f99 Mon Sep 17 00:00:00 2001 From: Tharun Date: Fri, 26 Mar 2021 23:18:35 +0530 Subject: [PATCH 6/6] refractored image command removeImages function Signed-off-by: Tharun --- cmd/minikube/cmd/image.go | 6 +-- pkg/minikube/machine/cache_images.go | 75 +++++++++++++--------------- 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/cmd/minikube/cmd/image.go b/cmd/minikube/cmd/image.go index 1f69909565e1..a379d6151109 100644 --- a/cmd/minikube/cmd/image.go +++ b/cmd/minikube/cmd/image.go @@ -132,16 +132,14 @@ $ minikube image rm image busybox $ minikube image unload image busybox `, + Args: cobra.MinimumNArgs(1), Aliases: []string{"unload"}, Run: func(cmd *cobra.Command, args []string) { - if len(args) == 0 { - exit.Message(reason.Usage, "Please provide an image to remove via ") - } profile, err := config.LoadProfile(viper.GetString(config.ProfileName)) if err != nil { exit.Error(reason.Usage, "loading profile", err) } - if err := machine.RemoveImages(args, []*config.Profile{profile}); err != nil { + if err := machine.RemoveImages(args, profile); err != nil { exit.Error(reason.GuestImageRemove, "Failed to remove image", err) } }, diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 03236fb27ddf..bfdb975cf2dc 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -294,12 +294,7 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, src st } // removeImages removes images from the container run time -func removeImages(cc *config.ClusterConfig, runner command.Runner, images []string) error { - cr, err := cruntime.New(cruntime.Config{Type: cc.KubernetesConfig.ContainerRuntime, Runner: runner}) - if err != nil { - return errors.Wrap(err, "runtime") - } - +func removeImages(cruntime cruntime.Manager, images []string) error { klog.Infof("RemovingImages start: %s", images) start := time.Now() @@ -312,66 +307,64 @@ func removeImages(cc *config.ClusterConfig, runner command.Runner, images []stri for _, image := range images { image := image g.Go(func() error { - return cr.RemoveImage(image) + return cruntime.RemoveImage(image) }) } if err := g.Wait(); err != nil { - return errors.Wrap(err, "removing images") + return errors.Wrap(err, "error removing images") } klog.Infoln("Successfully removed images") return nil } -func RemoveImages(images []string, profiles []*config.Profile) error { +func RemoveImages(images []string, profile *config.Profile) error { api, err := NewAPIClient() if err != nil { - return errors.Wrap(err, "api") + return errors.Wrap(err, "error creating api client") } defer api.Close() succeeded := []string{} failed := []string{} - for _, p := range profiles { // loading images to all running profiles - pName := p.Name // capture the loop variable + pName := profile.Name - c, err := config.Load(pName) + c, err := config.Load(pName) + if err != nil { + klog.Errorf("Failed to load profile %q: %v", pName, err) + return errors.Wrapf(err, "error loading config for profile :%v", pName) + } + + for _, n := range c.Nodes { + m := config.MachineName(*c, n) + + status, err := Status(api, m) if err != nil { - // Non-fatal because it may race with profile deletion - klog.Errorf("Failed to load profile %q: %v", pName, err) - failed = append(failed, pName) + klog.Warningf("error getting status for %s: %v", m, err) continue } - for _, n := range c.Nodes { - m := config.MachineName(*c, n) - - status, err := Status(api, m) + if status == state.Running.String() { + h, err := api.Load(m) if err != nil { - klog.Warningf("error getting status for %s: %v", m, err) - failed = append(failed, m) + klog.Warningf("Failed to load machine %q: %v", m, err) continue } - - if status == state.Running.String() { // the not running hosts will load on next start - h, err := api.Load(m) - if err != nil { - klog.Warningf("Failed to load machine %q: %v", m, err) - failed = append(failed, m) - continue - } - cr, err := CommandRunner(h) - if err != nil { - return err - } - err = removeImages(c, cr, images) - if err != nil { - failed = append(failed, m) - klog.Warningf("Failed to remove images for profile %s. make sure the profile is running. %v", pName, err) - continue - } - succeeded = append(succeeded, m) + runner, err := CommandRunner(h) + if err != nil { + return err + } + cruntime, err := cruntime.New(cruntime.Config{Type: c.KubernetesConfig.ContainerRuntime, Runner: runner}) + if err != nil { + return errors.Wrap(err, "error creating container runtime") + } + err = removeImages(cruntime, images) + if err != nil { + failed = append(failed, m) + klog.Warningf("Failed to remove images for profile %s %v", pName, err.Error()) + continue } + succeeded = append(succeeded, m) } }