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

[helm loki/promtail] make UpdateStrategy configurable #1898

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

stefanandres
Copy link

@stefanandres stefanandres commented Apr 6, 2020

This commit makes UpdateStrategy of the promtail daemonset configurable.
On large installations, you may want to increase the value of maxUnavailable.

This fixes #1878

What this PR does / why we need it:

See #1897

Which issue(s) this PR fixes:
Fixes #1897

Special notes for your reviewer:

I've reused the existing variable. This may break when you have this configured to the non-default value. Please advice if I shall add another variable or something else.

The codeis inspired by https://github.com/helm/charts/blob/master/stable/nginx-ingress/values.yaml#L139 and https://github.com/helm/charts/blob/master/stable/nginx-ingress/templates/controller-daemonset.yaml#L25

Checklist

  • Documentation added
  • Tests updated

This commit makes UpdateStrategy of the promtail daemonset configurable.
On large installations, you may want to increase the value of maxUnavailable.

This fixes grafana#1878
@stefanandres
Copy link
Author

The new default will change:

    updateStrategy:
-     rollingUpdate:
-       maxUnavailable: 1
-     type: RollingUpdate
+     {}

This will also now default to the k8s default, which is RollingUpdate with maxUnavailable: 1 (like before)

$ kubectl get daemonset -o yaml|grep -i -A3 updatestrategy:
    updateStrategy:
      rollingUpdate:
        maxUnavailable: 1
      type: RollingUpdate
--
    updateStrategy:
      rollingUpdate:
        maxUnavailable: 1
      type: RollingUpdate

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit 94cd4f4 into grafana:master Apr 6, 2020
@stefanandres stefanandres deleted the promtail-1878 branch April 6, 2020 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

entry out of order' for stream [helm loki/promtail] update of DaemonSet takes a long with a lot of nodes
2 participants