diff --git a/enhancements/20230307-pdb-for-longhon-csi-and-webhook.md b/enhancements/20230307-pdb-for-longhon-csi-and-webhook.md index 5bd9a96..60b69d9 100644 --- a/enhancements/20230307-pdb-for-longhon-csi-and-webhook.md +++ b/enhancements/20230307-pdb-for-longhon-csi-and-webhook.md @@ -17,19 +17,14 @@ https://github.com/longhorn/longhorn/issues/3304 ### Goals 1. Have better ways to protect Longhorn components (`csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, `longhorn-conversion-webhook`) without demanding the users to specify the draining flags to skip these pods. -1. Protecting `share-manager` in the single-node cluster. ## Proposal -Our existing solutions to protect these components are: -* For `instance-manager`: dynamically create/delete instance manager PDB -* For Daemonset pods in `longhorn-system` namespace: we advise the users to specify `--ignore-daemonsets` to ignore them in the `kubectl drain` command. This actually follows the [best practice](https://kubernetes.io/docs/tasks/administer-cluster/safely-drain-node/#:~:text=If%20there%20are%20pods%20managed%20by%20a%20DaemonSet%2C%20you%20will%20need%20to%20specify%20%2D%2Dignore%2Ddaemonsets%20with%20kubectl%20to%20successfully%20drain%20the%20node) -* For `csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, and `longhorn-conversion-webhook`: we advise the user to specify `--pod-selector` to ignore these pods -* For `share-manager`, we currently don't have any advice/protection - - -1. For `csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, and `longhorn-conversion-webhook` - +1. Our existing solutions to protect these components are: + * For `instance-manager`: dynamically create/delete instance manager PDB + * For Daemonset pods in `longhorn-system` namespace: we advise the users to specify `--ignore-daemonsets` to ignore them in the `kubectl drain` command. This actually follows the [best practice](https://kubernetes.io/docs/tasks/administer-cluster/safely-drain-node/#:~:text=If%20there%20are%20pods%20managed%20by%20a%20DaemonSet%2C%20you%20will%20need%20to%20specify%20%2D%2Dignore%2Ddaemonsets%20with%20kubectl%20to%20successfully%20drain%20the%20node) + * For `csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, and `longhorn-conversion-webhook`: we advise the user to specify `--pod-selector` to ignore these pods +1. Proposal for `csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, and `longhorn-conversion-webhook`:
The problem with the existing solution is that sometime, users could not specify `--pod-selector` for the `kubectl drain` command. For example, for the users that are using the project [System Upgrade Controller](https://github.com/rancher/system-upgrade-controller), they don't have option to specify `--pod-selector`. Also, we would like to have a more automatic way instead of relying on the user to set kubectl drain options. @@ -41,10 +36,6 @@ Our existing solutions to protect these components are: This should work for both single-node and multi-node cluster. -1. For `share-manager` - - If there is at least 1 workload pod on the same node with the share manager pod, prevent it from being evicted first via PDB. - Otherwise, we remove the PDB to allow the `share-manager` to be drained. ### User Stories @@ -53,9 +44,7 @@ Our existing solutions to protect these components are: #### Story 1 Before the enhancement, users would need to specify the drain options for drain command to exclude Longhorn pods. Sometimes, this is not possible when users use third-party solution to drain and upgrade kubernetes, such as System Upgrade Controller. -Also, before the enhancement, we don't protect the `share-manager` pod in single-node cluster. -After the enhancement, the user can doesn't need to specify the drain options for the drain command to exclude Longhorn pods, and we protect the `share-manager` pod in single-node cluster #### Story 2 ### User Experience In Detail @@ -70,26 +59,19 @@ None ### Implementation Overview -1. For `csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, and `longhorn-conversion-webhook` - Create a new controller inside Longhorn manager called `longhorn-pdb-controller`, the controller listens for the changes for `csi-attacher`, `csi-provisioner`, `longhorn-admission-webhook`, `longhorn-conversion-webhook`, and Longhorn volumes to adjust the PDB correspondingly. -1. For `share-manager` - -We will create PDB for `share-manager` in share-manager-controller when the `share-manager` is used by workloads on the same node. -Otherwise, when share-manager doesn't exist or when the `share-manager` is only used by workload on different nodes, remove the PDB. - ### Test plan -Integration test plan. - -For engine enhancement also requires engine integration test plan. +https://github.com/longhorn/longhorn/issues/3304#issuecomment-1467174481 ### Upgrade strategy -Anything that requires if the user wants to upgrade to this enhancement. +No Upgrade is needed -## Note [optional] +## Note -Additional notes. +In the original Github ticket, we mentioned that we need to add PDB to protect share manager pod from being drained before its workload pods because if share manager pod doesn't exist then its volume cannot be unmounted in the CSI flow. +However, with the fix https://github.com/longhorn/longhorn/issues/5296, we can always umounted the volume even if the share manager is not running. +Therefore, we don't need to protect share manager pod.