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

Only pull images if they are not available locally #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zonque
Copy link

@zonque zonque commented Mar 29, 2024

Avoid excessive downloads and only pull images if needed. This makes the rollout faster and also helps to reduce traffic when cronjobs are used.

Avoid excessive downloads and only pull images if needed. This makes the
rollout faster and also helps to reduce traffic when cronjobs are used.
@zonque zonque changed the title Only pull images unless they are available locally Only pull images if they are not available locally Mar 29, 2024
@collimarco
Copy link
Contributor

Thanks for your suggestion.

Maybe you have misunderstood the meaning of "Always". This is the definition:

every time the kubelet launches a container, the kubelet queries the container image registry to resolve the name to an image digest. If the kubelet has a container image with that exact digest cached locally, the kubelet uses its cached image; otherwise, the kubelet pulls the image with the resolved digest, and uses that image to launch the container.
https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy

Based on that, I don't think that you are going to save bandwidth if you use "IfNotPresent" instead of "Always".

It's just a quick digest check with the registry, not a download of the image.

@zonque
Copy link
Author

zonque commented Apr 7, 2024

I have seen the image being pulled from DockerHub each time a cronjob is performed 🤔
With my patch applied, this no longer happens.

FWIW, this is a deployment based on k3s.

@collimarco
Copy link
Contributor

@zonque Have you tried to open an issue in the k3s repository to report that strange behavior (incompatible with k8s documentation)?

@zonque
Copy link
Author

zonque commented Apr 7, 2024

Hmm. The reason is probably that k3s does not come with a full registry by default. But tbh, I find the k8s documentation a bit ambiguous, and would rather argue that there is no need to pull an image if it does exist locally, and hence 'IfNotPresent' appears the right choice 🤷

Could this setting be made configurable by the user of Cuber maybe?

@collimarco
Copy link
Contributor

I find the k8s documentation a bit ambiguous, and would rather argue that there is no need to pull an image if it does exist locally

K8s documentation is pretty clear: only the image hash is checked, there isn't any download if the image is already present locally. You can also find a confirmation of that by searching online.

hence 'IfNotPresent' appears the right choice

It depends. There are pros and cons.

@zonque
Copy link
Author

zonque commented Apr 28, 2024

It depends. There are pros and cons.

I'm curious what the cons would be. IfNotPresent works very well for my setups so far 🙂

@collimarco
Copy link
Contributor

@zonque You can find many discussions by searching "imagepullpolicy ifnotpresent vs always".

For example:

  • if you set Cuber release you can set an image not generated by Cuber (and in that case the same tag, like "latest", may be reused for different images / builds, for example in a test or staging environment)
  • even in production having Always makes sure that the image in the repository is always the same image that is running in Kubernetes (you don't have the risk of using an outdated cache or inconsistencies for unexpected reasons). That can also help for compliance.

@zonque
Copy link
Author

zonque commented Apr 28, 2024

Understood. But can we make it configurable maybe so more use cases are covered?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants