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

feat(hatchery/kubernetes): make default ImagePullPolicy configurable #5402

Closed
wants to merge 1 commit into from
Closed

feat(hatchery/kubernetes): make default ImagePullPolicy configurable #5402

wants to merge 1 commit into from

Conversation

phsym
Copy link
Contributor

@phsym phsym commented Aug 28, 2020

  1. Description
    This modifies hatchery:kubernetes configuration to add a defaultPullPolicy configuration field for hatchery config.
    If defined, it sets ImagePullPolicy field in worker pod specs, which by default is set to Always by Kubernetes.
    The config field is named defaultPullPolicy in case the pull policy become customizable per worker-model in a future release.
    When no value is given, it's defaulted to Always in order to fallback to the current behaviour.
  • ImagePullPolicy = Never is needed mainly for developing and testing worker images, for example in a dev environment like minikube, kind, kubernetes for windows, or more generally when uploading to a registry is undesirable.
  • ImagePullPolicy = IfNotPresent could be usefull to not block a pipeline execution if the registry is down while the image is already present on the node.
  1. About tests
    When a worker pod is started (or during registration), run a kubectl describe pod <worker pod name> -o yaml and verify the imagePullPolicy value on the pod's container spec

  2. Mentions

@ovh/cds

Signed-off-by: phsym [email protected]

@yesnault
Copy link
Member

yesnault commented Aug 31, 2020

Hi @phsym

Good catch! there is a behavior not implemented in the k8s hatchery, but which is present on the swarm and marathon hatchery.
If the worker model contains the docker tag :latest, there is a force pull asked to marathon / docker.
I'd prefer to implement the same behaviour in k8s hatchery: if the docker image end with :latest, we can set the policy to Always, otherwise we can set to IfNotPresent - without adding new configuration in Hatchery conf.

@phsym
Copy link
Contributor Author

phsym commented Aug 31, 2020

@yesnault What about this :

In addition to Never, Always, IfNotPresent which are already supported by Kubernetes, I can add AlwaysLatest which would emulate the behavior you described.

Thus, AlwaysLatest would become the default value for the parameter.

Other possible names I have in mind :

  • LatestOnly
  • Latest
  • PullLatest
  • ForceLatest

@yesnault
Copy link
Member

I prefer to not add additional configuration to the k8s hatchery and keep same behaviour between swarm / marathon and k8s hatcheries.
But, if we add defaultPullPolicy in k8s, we should add it in marathon / docker hatcheries too, with 'AlwaysLatest' to keep same behaviour as today. Is there a usecase that we really need to add it in hatchery configuration? if yes, perhaps that adding an explicitly option in worker model configuration would cover all user's needs

@yesnault
Copy link
Member

yesnault commented Sep 1, 2020

@phsym PR opened to align force pull between swarm / marathon and k8s hatcheries: #5405

Is there a use case for you that is not covered by PR 5405 ?

@phsym
Copy link
Contributor Author

phsym commented Sep 2, 2020

@yesnault I'm good with your proposal indeed. Wen can close this PR.

@phsym phsym closed this Sep 2, 2020
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