Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

Several charts default values prevent an operator from draining a worker #7127

Closed
kiall opened this issue Aug 10, 2018 · 4 comments
Closed
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@kiall
Copy link
Collaborator

kiall commented Aug 10, 2018

Version of Helm and Kubernetes:

# kubectl version
Client Version: version.Info{Major:"1", Minor:"9", GitVersion:"v1.9.8", GitCommit:"c138b85178156011dc934c2c9f4837476876fb07", GitTreeState:"clean", BuildDate:"2018-06-18T14:12:08Z", GoVersion:"go1.9.4", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"9", GitVersion:"v1.9.8", GitCommit:"c138b85178156011dc934c2c9f4837476876fb07", GitTreeState:"clean", BuildDate:"2018-06-18T14:12:08Z", GoVersion:"go1.9.4", Compiler:"gc", Platform:"linux/amd64"}

caasp-admin:~ # helm version
Client: &version.Version{SemVer:"v2.8.2", GitCommit:"2.8.2", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.9.1", GitCommit:"20adb27c7c5868466912eebdf6664e7390ebe710", GitTreeState:"clean"}

Which chart:

stable/nginx-ingress and stable/memcached (these are the ones I noticed!)

What happened:

The default configuration is to create a single replica, in addition to a PodDisruptionBudget with a minAvailable of 1. This results in Kubernetes being unable to drain the node, as doing so would violate the PodDisruptionBudget.

What you expected to happen:

The helm charts should not install a PodDisruptionBudget when the number of replicas for the pod is 1.

How to reproduce it (as minimally and precisely as possible):

  1. helm install stable/memcached
  2. kubectl drain {Node that the memcached pod landed on}

Anything else we need to know:

There is a kubernetes issue that is semi-related - essentially saying "works as intended" - kubernetes/kubernetes#48307 - I think I agree with the outcome, the helm charts have defined an "unreasonable" availability requirement, and kubernetes shouldn't ignore the availability requirements it given - even if unreasonable.

@kiall
Copy link
Collaborator Author

kiall commented Aug 10, 2018

For memcached, I created this PR yesterday: #7098

@mattfarina
Copy link
Contributor

This is a good catch. It would be good for the automation to pick this up. That's all packaged up over at https://github.com/helm/chart-testing

Thanks for the PR

@stale
Copy link

stale bot commented Sep 9, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 9, 2018
@stale
Copy link

stale bot commented Sep 23, 2018

This issue is being automatically closed due to inactivity.

@stale stale bot closed this as completed Sep 23, 2018
vdboor referenced this issue May 1, 2019
* add support for pod disruption budget

Signed-off-by: Jay Howard <jay.howard@rmn.com>

* support pod disruption budget parameters explicitly; bump minor version

Signed-off-by: Jay Howard <jay.howard@rmn.com>

* remove default values for minAvailable and maxUnavailable since they're mutually exclusive and neither is required

Signed-off-by: Jay Howard <jay.howard@rmn.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

2 participants