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

[bitnami/kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version #21489

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented Dec 10, 2023

Description of the change

This PR changes the the input of the config checksum which triggers the reload.

On main branch, the whole configmap/secret checksum is included as checksum input. The configmap contains labels which includes the helm chart version. Each new helm chart version forces a pod restart which is not always necessary.

Benefits

Avoid unnecessary pod restart on each new helm chart version

Possible drawbacks

N/A

Applicable issues

Additional information

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

…m chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
@github-actions github-actions bot requested a review from javsalgar December 10, 2023 10:39
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Dec 10, 2023
@github-actions github-actions bot removed the triage Triage is needed label Dec 10, 2023
@github-actions github-actions bot removed the request for review from javsalgar December 10, 2023 19:16
@github-actions github-actions bot requested a review from FraPazGal December 10, 2023 19:16
@FraPazGal
Copy link
Contributor

Hi @jkroepke, the changes look fine to me but it seems our CI is having some issues. I'll prefer to merge this with the CI green so let us move this to on-hold while we work on #21431 and come back to you once this bug is solved. Sorry for the inconvenience!

@FraPazGal FraPazGal added on-hold Issues or Pull Requests with this label will never be considered stale and removed in-progress labels Dec 13, 2023
@FraPazGal
Copy link
Contributor

The issue has now been solved and the PR #21431 merged, could you please update this PR with the changes in main? The CI should run without issues after that.

@FraPazGal FraPazGal added in-progress and removed on-hold Issues or Pull Requests with this label will never be considered stale labels Dec 19, 2023
@github-actions github-actions bot removed the request for review from FraPazGal December 19, 2023 18:35
@github-actions github-actions bot requested a review from CeliaGMqrz December 19, 2023 18:35
@FraPazGal FraPazGal requested review from FraPazGal and removed request for CeliaGMqrz December 19, 2023 18:35
@FraPazGal FraPazGal assigned FraPazGal and unassigned CeliaGMqrz Dec 19, 2023
@jkroepke
Copy link
Contributor Author

@FraPazGal done!

Copy link

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the stale 15 days without activity label Jan 10, 2024
@jkroepke
Copy link
Contributor Author

jkroepke commented Jan 10, 2024

@FraPazGal please re-check!

Copy link
Contributor

@FraPazGal FraPazGal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @jkroepke!

@FraPazGal FraPazGal merged commit 707ed49 into bitnami:main Jan 10, 2024
6 checks passed
jani888 pushed a commit to jani888/charts that referenced this pull request Jan 19, 2024
…each helm chart version (bitnami#21489)

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

---------

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: János Hidvégi <jani.hidvegi@gmial.com>
anthosz pushed a commit to anthosz/charts that referenced this pull request Jan 23, 2024
…each helm chart version (bitnami#21489)

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

---------

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
grouvie pushed a commit to grouvie/charts that referenced this pull request Jan 25, 2024
…each helm chart version (bitnami#21489)

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

---------

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
joancafom pushed a commit to dalbani/charts that referenced this pull request Feb 22, 2024
…each helm chart version (bitnami#21489)

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

* [kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

---------

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>
Signed-off-by: Jose Antonio Carmona <jcarmona@vmware.com>
amorey pushed a commit to kubetail-org/helm-charts that referenced this pull request Mar 1, 2024
* Avoid unnecessary restarts at new helm version: The ConfigMap where the config is stored, contains labels. The labels contains the version of the helm chart. If there is a new helm chart version, the pod gets restarted, too. It doesnt matter, if its necessary or not. To solve this, I moved the config to a named templated which can be sourced from the config map and the hash annotation. I contribute this pattern to many helm charts: ([promtail] Avoid unnecessary pod restart on each helm chart version grafana/helm-charts#2833, [bitnami/kubernetes-event-exporter] Avoid unnecessary pod restart on each helm chart version bitnami/charts#21489, [prometheus-blackbox-exporter] Avoid unnecessary pod restart on each helm chart version prometheus-community/helm-charts#4077, helm: Avoid unnecessary pod restart on each helm chart version kubernetes-sigs/external-dns#4103)
* Added securityContexts to container and pod. They contains the current best-practice. The settings are required to run kubelint together with PSA restricted.
* Added automountServiceAccountToken to the Deployment and ServiceAccount. A lot of security tools hightlight automountServiceAccountToken=true on a ServiceAccount as insecure. The best-practice is to set automountServiceAccountToken=false on a ServiceAccount and do automountServiceAccountToken=true on the Deployment. Ref: https://securecloud.blog/2021/08/17/azure-aks-reviewing-recommendations-from-security-center-disabling-automounting-api-credentials
* Added nodeSelector, affinity, tolerations, priorityClassName
Pass .Values.config, .Values.ingress.hosts.hosts through helm tpl function: If kubelint is part of a bigger umbrella helm chart, values from .Values.global can be re-used. It's also useful for the namespace option
* Adds configurable readinessProbe to deployment
* Bumps chart version number
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kubernetes-event-exporter solved stale 15 days without activity verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants