Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

disruptionBudget maxUnavailable value is ignored if 0 #58

Closed
topfunky opened this issue Nov 9, 2018 · 2 comments
Closed

disruptionBudget maxUnavailable value is ignored if 0 #58

topfunky opened this issue Nov 9, 2018 · 2 comments

Comments

@topfunky
Copy link

topfunky commented Nov 9, 2018

What I did:

I set my pod disruption budget's maxUnavailable to 0 in my Helm values override file. I then installed the helm chart to a minikube cluster.

What I saw:

When running helm install, I saw an error that told me the maxUnavailable had been set to -1, not the 0 that I had used.

$ helm install -f helm-consul-values.yaml ./consul-helm

Error: release pouring-monkey failed: PodDisruptionBudget.policy "pouring-monkey-consul
-server" is invalid: spec.maxUnavailable: Invalid value: -1: must be greater than or eq
ual to 0

What I expected:

I expected that setting maxUnavailable to 0 would be interpreted by the chart as a valid value.

Other context:

If I set the value to 1, everything works as expected. This suggests that a piece of logic is interpreting 0 as false and fails to use the value. Values 1 and greater are interpreted correctly.

My Helm values file has (this is the config that causes the above error):

server:
  replicas: 1
  bootstrapExpect: 1
  disruptionBudget:
    enabled: true
    maxUnavailable: 0

The issue may be related to code in _helpers.tpl which checks if .Values.server.disruptionBudget.maxUnavailable but should look for if .Values.server.disruptionBudget.enabled.

@adilyse
Copy link
Contributor

adilyse commented Nov 10, 2018

I've addressed the invalid -1 value in this PR. There is unfortunately still a larger issue of not being able to explicitly set this value to 0 that is a limitation of Helm templating (see PR for details).

adilyse pushed a commit that referenced this issue Nov 12, 2018
Provide a valid maxUnavailable value when using a single replica
@adilyse
Copy link
Contributor

adilyse commented Nov 12, 2018

I've addressed this with the following changes:

  • I've changed the calculation of the default maxUnavailableto provide a valid value when replicas=1. This should fix the issue in the vast majoring of cases.

  • I've added documentation about how someone can directly set this value to 0, since it's more involved than it should be.

Hopefully that helps!

@adilyse adilyse closed this as completed Nov 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants