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.

Sign up for GitHub

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

Jump to bottom

feat(helm): Avoid unnecessary pod restart on each helm chart version #12174

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(helm): Avoid unnecessary pod restart on each helm chart version #12174

wants to merge 4 commits into from

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented Mar 11, 2024 •
edited
Loading

What this PR does / why we need it:

Currenty, loki restarts on each new helm chart version. The reason here is that the whole Kubernetes secrets is used to generated the config checksum. The Kubernetes secrets contains labels, which contains the helm chart version. A new version would change the checksum, too.

From my point of view, a restart is only necessary, if PodSpec or config has been changed.

This isn't necessary.

Ref:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@jkroepke jkroepke requested a review from a team as a code owner March 11, 2024 22:37
@jkroepke jkroepke changed the title [helm] Avoid unnecessary pod restart on each helm chart version fix: [helm] Avoid unnecessary pod restart on each helm chart version Mar 11, 2024
@jkroepke jkroepke changed the title fix: [helm] Avoid unnecessary pod restart on each helm chart version refactor: [helm] Avoid unnecessary pod restart on each helm chart version Mar 11, 2024
@jkroepke jkroepke force-pushed the helm/checksum branch 2 times, most recently from 4b308e9 to ce0a5e5 Compare March 11, 2024 22:44
@JStickler
Copy link
Contributor

@jkroepke Could you please complete these items on the PR checklist? For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md.

@jkroepke
Copy link
Contributor Author

Done. Sorry, I miss files to commit on my machine

@jkroepke
Copy link
Contributor Author

Conflicts resolved.

@jkroepke
Copy link
Contributor Author

I did a merge from main here. I would appreciate a review soon.

@jkroepke jkroepke marked this pull request as draft July 6, 2024 12:55
@jkroepke jkroepke force-pushed the helm/checksum branch 3 times, most recently from 49744ee to 45bd0fe Compare July 6, 2024 21:30
@jkroepke
helm: Avoid unnecessary pod restart on each helm chart version
8b2f9a1 
Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
@jkroepke jkroepke marked this pull request as ready for review July 6, 2024 21:34
@jkroepke jkroepke requested a review from MichelHollands July 6, 2024 22:09
@jkroepke jkroepke changed the title refactor: [helm] Avoid unnecessary pod restart on each helm chart version [helm] Avoid unnecessary pod restart on each helm chart version Jul 6, 2024
@jkroepke
Copy link
Contributor Author

merged from main

@jkroepke jkroepke changed the title [helm] Avoid unnecessary pod restart on each helm chart version feat(helm): Avoid unnecessary pod restart on each helm chart version Aug 23, 2024
@jkroepke
Copy link
Contributor Author

@JStickler @MichelHollands I would appreciate an review here.

@pull-request-size pull-request-size bot added size/S and removed size/M labels Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichelHollands MichelHollands Awaiting requested review from MichelHollands

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area/helm size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jkroepke @JStickler @MichelHollands