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

Always pull prefecthq docker images in the helm chart #151

Merged
merged 4 commits into from
Dec 4, 2020

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Dec 3, 2020

Summary

Changes the default setting from IfNotPresent to Always since we are
using the 'latest' tag. This matches K8s behavior if the pull policy
were null. I considered setting this to null by default and depending
on the K8s default handling depending on the tag value, but it sounds
like the 'latest' -> 'Always' default pull policy behavior is
deprecated and I don't want to rely on it.

We will likely set this back to IfNotPresent when we pin the
prefectVersion for helm chart releases. However, some people prefer
Always because then the image cannot accidentally be grabbed by a
deployment without the proper secrets (not relevant for our images
since they are public)

Important: This does introduce the issue that if a deployment is rolled,
it could pull a different version than the other deployments. This is
not ideal for some users as it introduces version compatibility issues.
I'll add a note about versioning to the README suggesting pinning
prefectVersion for production deployments.

Edit: See rendered readme section

Importance

Installations with this chart will be stuck on old versions otherwise.

Checklist

This PR:

  • adds new tests (if appropriate)
  • adds a change file in the changes/ directory (if appropriate)

Changes the default setting from IfNotPresent to Always since we are
using the 'latest' tag. This matches K8s behavior if the pull policy
were null. I considered setting this to null by default and depending
on the K8s default handling depending on the tag value, but it sounds
like the 'latest' -> 'Always' default pull policy behavior is
deprecated and I don't want to rely on it.

We will likely set this back to IfNotPresent when we pin the
prefectVersion for helm chart releases. However, some people prefer
Always because then the image cannot accidentally be grabbed by a
deployment without the proper secrets (not relevant for our images
since they are public)
@zanieb zanieb requested review from cicdw and jlowin as code owners December 3, 2020 01:41
@zanieb zanieb marked this pull request as draft December 3, 2020 01:42
@cicdw
Copy link
Member

cicdw commented Dec 3, 2020

I know this is still a WIP but your reasoning makes sense to me ✅

@codecov-io
Copy link

Codecov Report

Merging #151 (4f40c81) into master (b959c34) will decrease coverage by 0.11%.
The diff coverage is n/a.

@zanieb zanieb marked this pull request as ready for review December 3, 2020 20:23
@zanieb zanieb requested review from jcrist and TylerWanner December 3, 2020 23:05
@joshmeek joshmeek merged commit 22f8a6d into master Dec 4, 2020
@joshmeek joshmeek deleted the helm-always-pull branch December 4, 2020 13:53
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.

4 participants