Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache reload does not appear to be working properly #10060

Open
0x-shanks opened this issue Dec 29, 2020 · 26 comments
Open

cache reload does not appear to be working properly #10060

0x-shanks opened this issue Dec 29, 2020 · 26 comments
Labels
cmd/cache Issues with the "cache" command kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.

Comments

@0x-shanks
Copy link

Steps to reproduce the issue:

  1. Pull the alpine:3.11 and alpine:3.12 image and tag alpine:3.11 as test

スクリーンショット 2020-12-30 0 23 01

  1. Add a cache with the following command
    minikube cache add alpine:test

  2. Enter the minikube container and check if the image exists

スクリーンショット 2020-12-30 0 25 05

  1. Tag alpine:3.12 as test

スクリーンショット 2020-12-30 0 26 37

  1. Reload the cache
    minikube cache reload alpine:test

  2. I check the image again, but it hasn't changed

スクリーンショット 2020-12-30 0 27 30

Optional: Full output of minikube logs command:

user@user ~ % minikube cache reload alpine:test -v=6 --alsologtostderr
I1230 00:30:52.617760   11506 out.go:221] Setting OutFile to fd 1 ...
I1230 00:30:52.618211   11506 out.go:273] isatty.IsTerminal(1) = true
I1230 00:30:52.618219   11506 out.go:234] Setting ErrFile to fd 2...
I1230 00:30:52.618223   11506 out.go:273] isatty.IsTerminal(2) = true
I1230 00:30:52.618361   11506 root.go:280] Updating PATH: /Users/user/.minikube/bin
I1230 00:30:52.618678   11506 cache.go:92] acquiring lock: {Name:mk21f3e783644f4fb459dda56815a449b17b30ed Clock:{} Delay:500ms Timeout:10m0s Cancel:<nil>}
I1230 00:30:52.618797   11506 cache.go:100] /Users/user/.minikube/cache/images/alpine_test exists
I1230 00:30:52.618810   11506 cache.go:81] cache image "alpine:test" -> "/Users/user/.minikube/cache/images/alpine_test" took 142.87µs
I1230 00:30:52.618817   11506 cache.go:66] save to tar file alpine:test -> /Users/user/.minikube/cache/images/alpine_test succeeded
I1230 00:30:52.618824   11506 cache.go:73] Successfully saved all images to host disk.
I1230 00:30:52.619292   11506 cli_runner.go:111] Run: docker ps -a --filter label=name.minikube.sigs.k8s.io --format {{.Names}}
I1230 00:30:52.768326   11506 cli_runner.go:111] Run: docker container inspect minikube --format={{.State.Status}}
I1230 00:30:52.903556   11506 ssh_runner.go:149] Run: systemctl --version
I1230 00:30:52.903697   11506 cli_runner.go:111] Run: docker container inspect -f "'{{(index (index .NetworkSettings.Ports "22/tcp") 0).HostPort}}'" minikube
I1230 00:30:53.036130   11506 sshutil.go:48] new ssh client: &{IP:127.0.0.1 Port:55007 SSHKeyPath:/Users/user/.minikube/machines/minikube/id_rsa Username:docker}
I1230 00:30:53.148720   11506 ssh_runner.go:149] Run: docker images --format {{.Repository}}:{{.Tag}}
I1230 00:30:53.207718   11506 docker.go:382] Got preloaded images: -- stdout --
alpine:test
@afbjorklund
Copy link
Collaborator

afbjorklund commented Dec 29, 2020

It only looks at the repo:tag, so it will only see that "alpine:test" is already there.

docker images --format {{.Repository}}:{{.Tag}}

There is no matching "AlwaysPullImages" option (yet?), it assumes unique tags

There is some old broken code to check id/digest

@afbjorklund afbjorklund added cmd/cache Issues with the "cache" command priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. kind/feature Categorizes issue or PR as related to a new feature. labels Dec 29, 2020
@0x-shanks
Copy link
Author

I see, so even if the Image ID changes, if the repo:tag is the same, reloading won't help, and if you want to use a different image, you have to add a unique tag?

So when is reload used?

@afbjorklund
Copy link
Collaborator

afbjorklund commented Dec 30, 2020

There might be a bug (change of behaviour) in the "cache" command, after adding the preload...

It now checks the image list (repo:tag), even before comparing the Id and/or Digest whatsoever.

Images are preloaded, skipping loading

It would also add any missing images, or any that was added after the cluster was initially created

@afbjorklund
Copy link
Collaborator

Problem here is that an image on disk doesn't have any Id or Digest, that is added by the daemon/registry.

So you need to either load it locally (eew) or talk to the registry (eeew), whichever is worst (we do both of them).

There is also some compatibility issues with crane and the library behind it: google/go-containerregistry#890

At the moment, we can't even agree on the name of the container image. Much less on the actual contents.

  • alpine
  • alpine:latest
  • docker.io/alpine
  • docker.io/library/alpine:latest

Short-term fix will probably be to re-visit the preload code and maybe add an --always flag to minikube cache.

Assuming that the daemon/registry code for digest even works, I have some doubts about that:

@0x-shanks
Copy link
Author

@afbjorklund
Thank you for your kindness.
I see that you have to add Id or Digest to the image list, not just repo:tags.

Short-term fix will probably be to re-visit the preload code and maybe add an --always flag to minikube cache.

As you said, it's short term, but I'm going to try to implement it this way, but I've never contributed to minikube, so I don't know the difficulty level.
Would it be better to wait quietly for the Id or Digest to be added?

@afbjorklund
Copy link
Collaborator

Sad thing is that neither Id nor Digest will work with all container runtimes and also offline (without a registry)...

I added a new feature to clean up the patched library, we can use this particular issue to go past the name only ?

@seperman
Copy link

Hello @afbjorklund
I'm in the same boat. Do you recommend any workarounds? I see the cached file in ~/.minikube/cache/images/localhost_5000
localhost_5000 is my local registry. When I run minikube cache delete localhost:5000/repo:tag it does remove that file. But if I do minikube cache add localhost:5000/repo:tag, it will still add the previous image even though the image is updated in the local registry. So it seems like it is caching the old image somewhere else too.

@afbjorklund
Copy link
Collaborator

If the image already exists in the docker daemon, it will not pull anything from the registry.

  1. https://pkg.go.dev/github.com/google/go-containerregistry/pkg/v1/daemon#Image

  2. https://pkg.go.dev/github.com/google/go-containerregistry/pkg/v1/remote#Image

It actually always checks with dockerd, even if there no such thing running on the host.

# daemon.Image
retrieving image: busybox:latest
daemon lookup for busybox:latest: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
# remote.Image
cache image "busybox:latest" -> "/home/anders/.minikube/cache/images/busybox" took 2.80001642s
save to tar file busybox -> /home/anders/.minikube/cache/images/busybox succeeded

@afbjorklund
Copy link
Collaborator

afbjorklund commented Feb 11, 2021

So the current workaround would be docker pull localhost:5000/repo:tag

That will update the image in the daemon with the new one from the registry.

@seperman
Copy link

seperman commented Mar 8, 2021

@afbjorklund Thanks for the response. Btw I see a new warning message on Minikube v1.18.1

"minikube cache" will be deprecated in upcoming versions, please switch to "minikube image load"

But the docs don't mention minikube image load: https://minikube.sigs.k8s.io/docs/handbook/pushing/

@afbjorklund
Copy link
Collaborator

afbjorklund commented Mar 24, 2021

@seperman : I'm actually not sure why @priyawadhwa wrote that "cache" would be deprecated e8945f2

They don't do exactly the same thing, even if they do share some code. The "cache" command has a memory

However, the "image load" and "image build" commands now have slightly better documentation (placeholders)

@mprimeaux
Copy link

@priyawadhwa Would you please explain why you committed the warning that 'cache add' would be deprecated as per the above?

Your help is appreciated.

@seperman
Copy link

Also if you could briefly explain the difference of minikube image load vs. minikube cache add, that would be great. It looks like the cache command, caches the image in ~/.minikube in addition to having it in the minikube's docker daemon. While the minikube image load only adds the image to the minikube's docker daemon. I'm curious what is a use case where you want to use minikube cache vs. minikube image load. Thanks.

@afbjorklund
Copy link
Collaborator

@mprimeaux @seperman

I think Priya has handed over the feature to other members of the team, so I'm not sure if she's able to answer (but please do if you can). Both commands are using the ~/.minikube cache folder for caching images that they pull from the daemon or from remote.

But they come from different backgrounds:

minikube cache ties into the possibility of minikube to add more images to be installed by default in the cluster, such as the kubernetes dashboard. It maintains a list in the configuration, that is pre-populated when starting a new cluster.

Available Commands:
add Add an image to local cache.
delete Delete an image from the local cache.
list List all available images from the local cache.
reload reload cached images.

minikube image is more of a replacement for the docker-env command, so that one can load and build images without needing a docker client - and also works with different container runtimes, such as containerd (with the help of buildkitd)

Available Commands:
build Build a container image in minikube
load Load a image into minikube
rm Remove one or more images
ls List images in minikube

So the main difference between "cache add" and "image load", is if it is added to the minikube config as a memory or not.

$ minikube cache list
alpine:latest
busybox:latest
$ more /.minikube/config/config.json 
{
    "cache": {
        "alpine:latest": null,
        "busybox:latest": null
    }
}

@afbjorklund
Copy link
Collaborator

The main problem in this issue, is that the current code only checks the name and the tag to see if it "already exists".

There is no equivalent to the "pull policy" of Kubernetes, where you can choose to replace the already existing images:

imagePullPolicystring | Image pull policy.
One of Always, Never, IfNotPresent. Defaults to Always if :latest tag is specified, or IfNotPresent otherwise.
Cannot be updated. More info: https://kubernetes.io/docs/concepts/containers/images#updating-images

And the current handling of the short names and the id/digest is also rather confusing and could be improved upon.

@mprimeaux
Copy link

@afbjorklund Thanks much for this detail; very helpful. The 'memory' attributes are the primary attraction of cache add as this allows for full offline usage even after minikube delete is executed.

I agree that evolving minikube image load with an imagePullPolicy equivalent represents a material improvement. In addition to offline support, saving the images represents a huge 'boost' in velocity for software engineers in their 'day to day' work. As an example, we delete our minikube clusters many times per day during and thus are still using cache add.

@afbjorklund
Copy link
Collaborator

We are currently discussing how to be best handle the Id and Digest discrepancies, between different methods and runtimes.
Something like "replace always" (or never) is simple enough, but eventually we want to "compare" two different images.

This is the needsTransfer function, that tries to determine if the local image is different from the one present in the cluster...
Some of the current methods will be quite "expensive" for tarball and for remote, when it comes to how long they take to run.

@afbjorklund
Copy link
Collaborator

For minikube image load, using the image cache just happens to be the current implementation...

Theoretically it could stream the images to the cluster right from the local daemon or the remote registry.

@seperman
Copy link

seperman commented Apr 5, 2021

Thanks for the clarification.

@dbingham
Copy link

dbingham commented Apr 23, 2021

@afbjorklund Above you said:

So the current workaround would be docker pull localhost:5000/repo:tag

That will update the image in the daemon with the new one from the registry.

Can you clarify what localhost is and which daemon & registry you are talking about here? So if I want to pull an updated version of a particular repo:tag from my host's daemon/registry into minikube, I'd minikube ssh and then do docker pull localhost:5000/repo:tag? If that's the case, I'm assuming the minikube VM has port 5000 tunneled to the host's registry somehow? Or am I misunderstanding?

I ask because I'm not getting any type of successful result from inside a minikube ssh session or from a host shell after doing eval $(minikube docker-env). In both cases, I get a connection refused on port 5000.

@dbingham
Copy link

Rereading, maybe I've misunderstood the context above. I'm trying to update the image in minikube but it isn't updating as detailed above. In my case, I've got an external registry. Normally for a remote k8s install I'd build the image and push it to the external registry, then deploy on k8s (which has access to talk to the registry). But to test locally on minikube, I don't want to push the untested, potentially incomplete image. I want to push it into minikube for testing first. So,

$ docker build -t some.other.registry/image:tag .
$ minikube cache add some.other.registry/image:tag
$ # OR minikube image load some.other.registry/image:tag

And minikube still has the prior version. I deleted the image from minikube (verified gone with cache and image commands), deleted it from my host, rebuilt it, pushed again (cache add or image load)... the old image version comes back.

The only way I was able to get it to work was to do an eval $(minikube docker-env) before the build, but that doesn't work for some of our images which need to pull their base image from the upstream registry over VPN (minikube's dockerd doesn't route its traffic through the host's VPN). So, it isn't clear to me how I can rebuild and retest these images.

@calohmn
Copy link

calohmn commented May 22, 2021

And minikube still has the prior version. I deleted the image from minikube (verified gone with cache and image commands), deleted it from my host, rebuilt it, pushed again (cache add or image load)... the old image version comes back.

@dbingham I guess it's coming from the ~/.minikube/cache/images/ directory then. I had the same issue getting a SNAPSHOT image, built in the host docker daemon, to be updated in minikube (doing minikube image rm and minikube docker env docker rmi before). I had to manually delete the corresponding image file in the ~/.minikube/cache/images/ directory before doing minikube image load.

I found that minikube cache add adds the image file to that directory, and minikube cache delete removes it from there.
minikube image load also adds the image file there, but minikube image rm doesn't remove it from there. (I'm using minikube v1.19.0.)

@afbjorklund Isn't it a bug that images are not removed from ~/.minikube/cache/images/ when using minikube image rm?

@afbjorklund
Copy link
Collaborator

afbjorklund commented May 23, 2021

I found that minikube cache add adds the image file to that directory, and minikube cache delete removes it from there.
minikube image load also adds the image file there, but minikube image rm doesn't remove it from there. (I'm using minikube v1.19.0.)
@afbjorklund Isn't it a bug that images are not removed from ~/.minikube/cache/images/ when using minikube image rm?

Probably ill-defined, at least. Actually I don't know why the image commands are using the cache directory at all.

But now that they do, it should probably have matching semantics like you say - remove it from cache, if it adds it.

@afbjorklund
Copy link
Collaborator

@dbingham

Can you clarify what localhost is and which daemon & registry you are talking about here? So if I want to pull an updated version of a particular repo:tag from my host's daemon/registry into minikube

This was for the cluster internal registry, as from the add-on. I don't think we support a host registry yet, but it would be a nice feature to have. It would be a better option, than connecting to an external host daemon or podman. Sorry for the late reply.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 21, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 20, 2021
@spowelljr spowelljr added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/cache Issues with the "cache" command kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done.
Projects
None yet
Development

No branches or pull requests

9 participants