kubernetes-sigs / aws-ebs-csi-driver

CSI driver for Amazon EBS https://aws.amazon.com/ebs/
Apache License 2.0
998 stars 800 forks source link

Add toggle for PodDisruptionBudget in chart #2109

Closed AndrewSirenko closed 3 months ago

AndrewSirenko commented 3 months ago

Is this a bug fix or adding new feature?

/feature

What is this PR about? / Why do we need it?

Fixes #1934

The EBS CSI Controller Pod's PodDisruptionBudget ensures that 1 controller Pod is always running, even when many nodes are being drained at once. This ensures that PersistentVolumes are always able to be Attached/Detached Created/Deleted, and that stateful pods are not delayed by missing PVs.

Some customers have requested ability to disable this PodDisruptionBudget in order to prioritize node draining / Karpenter consolidation over the availability of the EBS CSI Driver. While we don't recommend this, this PR exposes the parameter for those who use EBS only with non-critical applications.

**What testing is

See the following rendered helm templates with and without --set controller.podDisruptionBudget=false, along with the diff

change.txt default.txt diff.txt done?**

Diff:

2,22d1
< # Source: aws-ebs-csi-driver/templates/poddisruptionbudget-controller.yaml
< apiVersion: policy/v1
< kind: PodDisruptionBudget
< metadata:
<   name: ebs-csi-controller
<   namespace: default
<   labels:
<     app.kubernetes.io/name: aws-ebs-csi-driver
<     app.kubernetes.io/instance: release-name
<     helm.sh/chart: aws-ebs-csi-driver-2.33.0
<     app.kubernetes.io/version: "1.33.0"
<     app.kubernetes.io/component: csi-driver
<     app.kubernetes.io/managed-by: Helm
< spec:
<   selector:
<     matchLabels:
<       app: ebs-csi-controller
<       app.kubernetes.io/name: aws-ebs-csi-driver
<       app.kubernetes.io/instance: release-name
<   maxUnavailable: 1
< ---

Proof in deployed cluster:

❯ helm upgrade \
  --install aws-ebs-csi-driver \
  ... 
  --set controller.podDisruptionBudget=false

❯ kubectl get pdb -A
NAMESPACE     NAME                           MIN AVAILABLE   MAX UNAVAILABLE   ALLOWED DISRUPTIONS   AGE
kube-system   aws-node-termination-handler   N/A             1                 1                     105m
kube-system   cilium-operator                N/A             1                 1                     105m
kube-system   kube-dns                       N/A             50%               1                     105m
github-actions[bot] commented 3 months ago

Code Coverage Diff

This PR does not change the code coverage

ElijahQuinones commented 3 months ago

/retest

torredil commented 3 months ago

/lgtm

AndrewSirenko commented 3 months ago

/hold

Wait for #2106 to merge first

torredil commented 3 months ago

/approve

k8s-ci-robot commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: torredil

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/OWNERS)~~ [torredil] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
AndrewSirenko commented 3 months ago

/unhold