ivampiresp/longhorn
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Modify the LEP: Use PDB to protect Longhorn components from drains

Browse Source 
longhorn-3304

Signed-off-by: Phan Le <phan.le@suse.com>
This commit is contained in:
Phan Le 2023-03-15 17:47:48 -07:00 committed by David Ko
parent f82928c33e
commit 702c2e65d3
1 changed files with 11 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
enhancements/20230307-pdb-for-longhon-csi-and-webhook.md
View File

@ -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`: <br>
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.