`image load`: overwrite same image unless `--overwrite=false` #11366

spowelljr · 2021-05-10T22:34:08Z

Before:
If you had an image already loaded into minikube, and then created a new image with the same name:tag as one already loaded into minikube and ran minikube image load it would skip loading the image since it detected a matching name:tag, even though the image itself is different.

After:
minikube image load will load the image, even if an image with the existing name:tag is already loaded into minikube.

minikube-bot · 2021-05-10T22:41:04Z

Can one of the admins verify this patch?

medyagh

lets add an option for user to still skip overwrite if they want to (not the default behaviour)

it could be

--over-write=true

with a default value of false

cmd/minikube/cmd/image.go

spowelljr · 2021-05-11T22:38:14Z

/ok-to-test

medyagh · 2021-05-12T01:00:41Z

/retest-this-please

k8s-ci-robot · 2021-05-12T04:14:41Z

@spowelljr: PR needs rebase.

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.

cmd/minikube/cmd/image.go

pkg/minikube/machine/cache_images.go

pkg/minikube/image/cache.go

…it non-optional

medyagh · 2021-05-12T22:18:34Z

pkg/minikube/machine/cache_images.go

@@ -269,8 +269,15 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, src st
 		return errors.Wrap(err, "runtime")
 	}

-	if err := r.RemoveImage(imgName); err != nil {
-		klog.Warningf("remove image: %v", err)
+	exists, err := imageExists(r, imgName)


I worry this add performance downside to it, how about instead of Image List, we do something cheaper ? try to remove it, and if the error message says it doesnt exist then only klog.Info if error message is not that klog.Warnning

or alternatively we could do

klog.Warniningf("failed to remove image (which might be okay if it doesn't exists")

minikube-pr-bot · 2021-05-12T22:23:53Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 48.9s    | 51.5s               |
| enable ingress | 42.6s    | 37.9s               |
+----------------+----------+---------------------+

Times for minikube (PR 11366) start: 48.4s 55.8s 52.3s 50.9s 49.9s
Times for minikube start: 47.2s 51.7s 47.5s 51.3s 46.6s

Times for minikube ingress: 35.8s 34.8s 61.8s 44.2s 36.3s
Times for minikube (PR 11366) ingress: 43.3s 35.3s 37.7s 34.8s 38.2s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 21.6s    | 22.0s               |
| enable ingress | 39.7s    | 32.7s               |
+----------------+----------+---------------------+

Times for minikube start: 21.4s 22.5s 21.0s 21.4s 21.6s
Times for minikube (PR 11366) start: 21.7s 22.3s 22.5s 22.0s 21.9s

Times for minikube ingress: 33.0s 70.5s 30.0s 28.5s 36.5s
Times for minikube (PR 11366) ingress: 36.5s 28.6s 29.5s 26.6s 42.5s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 44.7s    | 43.8s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 42.8s 47.3s 43.1s 47.1s 43.3s
Times for minikube (PR 11366) start: 42.8s 43.3s 43.3s 42.9s 46.8s

minikube-pr-bot · 2021-05-12T23:15:14Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 47.4s    | 46.5s               |
| enable ingress | 35.1s    | 36.8s               |
+----------------+----------+---------------------+

Times for minikube (PR 11366) start: 46.2s 46.2s 47.6s 46.0s 46.5s
Times for minikube start: 46.6s 46.4s 49.5s 46.0s 48.6s

Times for minikube ingress: 36.3s 34.3s 34.7s 35.9s 34.2s
Times for minikube (PR 11366) ingress: 34.8s 37.3s 34.3s 34.8s 42.8s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 21.4s    | 21.6s               |
| enable ingress | 34.5s    | 34.9s               |
+----------------+----------+---------------------+

Times for minikube start: 21.6s 21.3s 20.8s 21.0s 22.4s
Times for minikube (PR 11366) start: 21.6s 21.2s 21.6s 22.2s 21.7s

Times for minikube (PR 11366) ingress: 37.0s 37.0s 34.5s 33.5s 32.5s
Times for minikube ingress: 33.0s 34.5s 36.5s 34.5s 34.0s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 45.8s    | 45.5s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 43.0s 43.7s 47.7s 46.8s 47.7s
Times for minikube (PR 11366) start: 43.8s 47.2s 44.0s 49.3s 43.1s

minikube-pr-bot · 2021-05-13T22:19:34Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 49.3s    | 46.7s               |
| enable ingress | 39.9s    | 39.6s               |
+----------------+----------+---------------------+

Times for minikube start: 47.0s 48.7s 49.2s 55.5s 46.3s
Times for minikube (PR 11366) start: 47.2s 46.5s 47.4s 46.2s 46.2s

Times for minikube ingress: 43.7s 42.2s 35.3s 34.8s 43.7s
Times for minikube (PR 11366) ingress: 42.7s 33.7s 42.8s 43.2s 35.8s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 27.2s    | 22.4s               |
| enable ingress | 33.2s    | 36.0s               |
+----------------+----------+---------------------+

Times for minikube (PR 11366) ingress: 37.0s 35.1s 38.5s 35.0s 34.5s
Times for minikube ingress: 35.0s 34.0s 32.0s 32.5s 32.5s

Times for minikube start: 47.4s 22.0s 22.2s 21.9s 22.5s
Times for minikube (PR 11366) start: 23.5s 22.5s 22.0s 21.5s 22.4s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 45.0s    | 42.6s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 44.2s 47.1s 43.5s 43.5s 47.0s
Times for minikube (PR 11366) start: 31.0s 47.9s 47.2s 43.5s 43.4s

minikube-pr-bot · 2021-05-13T22:45:02Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 49.1s    | 49.2s               |
| enable ingress | 37.5s    | 37.7s               |
+----------------+----------+---------------------+

Times for minikube start: 47.7s 46.3s 50.6s 48.5s 52.2s
Times for minikube (PR 11366) start: 50.0s 51.1s 46.9s 49.7s 48.3s

Times for minikube ingress: 42.2s 35.2s 38.2s 36.3s 35.7s
Times for minikube (PR 11366) ingress: 41.8s 42.2s 35.3s 35.3s 34.2s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 22.3s    | 25.5s               |
| enable ingress | 34.6s    | 34.3s               |
+----------------+----------+---------------------+

Times for minikube start: 23.7s 21.7s 21.9s 21.5s 22.6s
Times for minikube (PR 11366) start: 41.7s 21.8s 21.8s 21.5s 20.7s

Times for minikube ingress: 33.5s 38.0s 35.0s 35.0s 31.5s
Times for minikube (PR 11366) ingress: 35.0s 34.0s 35.0s 35.0s 32.5s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 42.1s    | 38.6s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 31.0s 43.8s 46.9s 43.1s 45.9s
Times for minikube (PR 11366) start: 30.6s 43.1s 43.4s 28.8s 47.2s

k8s-ci-robot · 2021-05-14T00:26:59Z

@spowelljr: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-minikube-build	`e1fabfd`	link	`/test pull-minikube-build`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-ci-robot · 2021-05-14T01:51:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: medyagh, spowelljr

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~~ [medyagh]

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

minikube-pr-bot · 2021-05-14T01:52:46Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 54.2s    | 56.2s               |
| enable ingress | 39.9s    | 35.4s               |
+----------------+----------+---------------------+

Times for minikube start: 56.0s 52.9s 53.8s 55.1s 53.3s
Times for minikube (PR 11366) start: 55.3s 56.8s 57.8s 55.6s 55.5s

Times for minikube ingress: 46.4s 37.5s 43.9s 37.4s 34.3s
Times for minikube (PR 11366) ingress: 37.0s 34.4s 34.8s 35.8s 34.9s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 22.9s    | 23.5s               |
| enable ingress | 29.1s    | 36.1s               |
+----------------+----------+---------------------+

Times for minikube (PR 11366) start: 23.0s 23.3s 24.0s 23.7s 23.4s
Times for minikube start: 22.1s 23.5s 23.2s 22.7s 23.0s

Times for minikube ingress: 27.5s 28.1s 27.0s 32.6s 30.1s
Times for minikube (PR 11366) ingress: 29.0s 45.1s 48.1s 30.0s 28.5s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11366) |
+----------------+----------+---------------------+
| minikube start | 47.5s    | 47.5s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 47.3s 49.5s 48.2s 43.7s 48.8s
Times for minikube (PR 11366) start: 49.4s 49.4s 43.4s 47.4s 47.8s

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 10, 2021

k8s-ci-robot requested review from blueelvis and prasadkatti May 10, 2021 22:34

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 10, 2021

spowelljr force-pushed the fixImageLoad branch from e26a100 to c99d7ed Compare May 10, 2021 22:35

medyagh requested changes May 11, 2021

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 11, 2021

spowelljr force-pushed the fixImageLoad branch from 38363a3 to c461264 Compare May 11, 2021 19:00

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 11, 2021

medyagh requested changes May 11, 2021

View reviewed changes

cmd/minikube/cmd/image.go Outdated Show resolved Hide resolved

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 11, 2021

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2021

medyagh requested changes May 12, 2021

View reviewed changes

cmd/minikube/cmd/image.go Outdated Show resolved Hide resolved

medyagh requested changes May 12, 2021

View reviewed changes

pkg/minikube/machine/cache_images.go Outdated Show resolved Hide resolved

pkg/minikube/image/cache.go Outdated Show resolved Hide resolved

spowelljr added 7 commits May 12, 2021 14:24

image load loads image even if name:tag already exists

32a91d6

added over-write flag for image load

83366b7

ran make generate-docs again

8e696da

replace over-write with overwrite

826f185

updated flag description and renamed force arg to overwrite and made …

e59ced9

…it non-optional

use overwrite flag for local images

9d3ee7e

check if image exists before trying to remove it

16dbac1

spowelljr force-pushed the fixImageLoad branch from 29a224f to 16dbac1 Compare May 12, 2021 21:26

ran make generate-docs

bcc6b46

k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 12, 2021

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 12, 2021

spowelljr removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2021

medyagh requested changes May 12, 2021

View reviewed changes

spowelljr force-pushed the fixImageLoad branch from 36cb8fa to bcc6b46 Compare May 13, 2021 20:30

medyagh changed the title ~~Image load loads image even if name:tag already loaded~~ 'image load': overwrite same image unless --overwrite=false May 13, 2021

medyagh changed the title ~~'image load': overwrite same image unless --overwrite=false~~ image load: overwrite same image unless --overwrite=false May 13, 2021

only throw error if not a "no such image" error returned from remove

e1fabfd

medyagh approved these changes May 14, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 14, 2021

medyagh merged commit 33e2650 into kubernetes:master May 14, 2021

spowelljr mentioned this pull request Jun 7, 2021

Minikube image load does not seem to load the latest image version #11597

Closed

spowelljr deleted the fixImageLoad branch June 28, 2021 17:43

ptnapoleon mentioned this pull request Sep 21, 2021

Specify overwrite=true for minikube image load DataDog/chaos-controller#413

Merged

11 tasks

afbjorklund mentioned this pull request Mar 14, 2023

"minikube image load" silently fails when repo/image:tag combination is already in use #16032

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`image load`: overwrite same image unless `--overwrite=false` #11366

`image load`: overwrite same image unless `--overwrite=false` #11366

spowelljr commented May 10, 2021 •

edited

Loading

minikube-bot commented May 10, 2021

medyagh left a comment

spowelljr commented May 11, 2021

medyagh commented May 12, 2021

k8s-ci-robot commented May 12, 2021

medyagh May 12, 2021

minikube-pr-bot commented May 12, 2021

minikube-pr-bot commented May 12, 2021

minikube-pr-bot commented May 13, 2021

minikube-pr-bot commented May 13, 2021

k8s-ci-robot commented May 14, 2021

k8s-ci-robot commented May 14, 2021

minikube-pr-bot commented May 14, 2021

image load: overwrite same image unless --overwrite=false #11366

image load: overwrite same image unless --overwrite=false #11366

Conversation

spowelljr commented May 10, 2021 • edited Loading

minikube-bot commented May 10, 2021

medyagh left a comment

Choose a reason for hiding this comment

spowelljr commented May 11, 2021

medyagh commented May 12, 2021

k8s-ci-robot commented May 12, 2021

medyagh May 12, 2021

Choose a reason for hiding this comment

minikube-pr-bot commented May 12, 2021

minikube-pr-bot commented May 12, 2021

minikube-pr-bot commented May 13, 2021

minikube-pr-bot commented May 13, 2021

k8s-ci-robot commented May 14, 2021

k8s-ci-robot commented May 14, 2021

minikube-pr-bot commented May 14, 2021

`image load`: overwrite same image unless `--overwrite=false` #11366

`image load`: overwrite same image unless `--overwrite=false` #11366

spowelljr commented May 10, 2021 •

edited

Loading