-
Notifications
You must be signed in to change notification settings - Fork 98
chore: Removing default imagePullPolicy to honor Kubernetes defaults #287
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
Conversation
Signed-off-by: Leanne Ahern <[email protected]>
Signed-off-by: Leanne Ahern <[email protected]>
Signed-off-by: Leanne Ahern <[email protected]>
Signed-off-by: Leanne Ahern <[email protected]>
Signed-off-by: Leanne Ahern <[email protected]>
rm3l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I think the failing tests should pass once #284 is merged.
|
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
|
not stale |
rm3l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OpinionatedHeron Now that #284 has been merged, could you please update your PR branch, so we can verify if the CI passes? Thanks.
rm3l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OpinionatedHeron A chart version bump is required: https://github.com/backstage/charts/actions/runs/18904993004/job/54100204359?pr=287#step:7:40
|
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
|
not stale |
Signed-off-by: Leanne Ahern <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OpinionatedHeron I think a chart version bump is still required.
|
Sorry, it is updated on my local files - I didn't realize it was not added to the commit. |
|
Should be bumped with a patch now. Sorry about that - don't know why it went bit haywire. Thanks for the catch. |
rm3l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@OpinionatedHeron Thanks for the contribution, apologies for the rigmorole that can happen with the chart bumping and pre-commit stuff 😢 but we got there!! 🚀 Thanks @rm3l too for helping out! 👍 |
Description of the change
Making image pullPolicy optional, so that default K8s behavior is honored. This change leaves the pullPolicy unset by default and allows Kubernetes to apply its tag-based default behavior ('Always' for latest, 'IfNotPresent' for other tags).
This change should improve startup times as it will not query the container image registry every time, if not necessary. Leaving the pullPolicy unset gives users the expected, standard behavior.
Additional Information
Based on documentation provided - https://kubernetes.io/docs/concepts/containers/images/#image-pull-policy
Checklist
Chart.yamlaccording to semver.values.yamland added to the README.md. The helm-docs utility can be used to generate the necessary content. Usehelm-docs --dry-runto preview the content.ct lintcommand.