Skip to content

Commit 80aa21f

Browse files
authored
Merge pull request kubernetes#3552 from deepakkinni/honor_pv_v3
KEP-2644: [HonorPVReclaimPolicy] Support Statically provisioned volumes
2 parents fac005a + d55c5cf commit 80aa21f

File tree

2 files changed

+25
-19
lines changed

2 files changed

+25
-19
lines changed

keps/sig-storage/2644-honor-pv-reclaim-policy/README.md

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func (ctrl *ProvisionController) shouldDelete(ctx context.Context, volume *v1.Pe
151151
// Please refer sig-storage-lib-external-provisioner/controller/controller.go for the full code.
152152
```
153153
154-
The fix applies to both csi driver volumes and in-tree plugin volumes.
154+
The fix applies to both csi driver volumes and in-tree plugin volumes each either statically or dynamically provisioned.
155155
156156
The main approach in fixing the issue for csi driver volumes involves using an existing finalizer already implemented in sig-storage-lib-external-provisioner. It is `external-provisioner.volume.kubernetes.io/finalizer` which is added on the Persistent Volume. Currently, it is applied only during provisioning if the feature is enabled. The proposal is to not only add this finalizer to newly provisioned PVs, but also to extend the library to add the finalizer to existing PVs. Adding the finalizer prevents the PV from being removed from the API server. The finalizer will be removed only after the physical volume on the storage system is deleted.
157157
@@ -160,13 +160,18 @@ The main approach in fixing the issue for in-tree volumes involves introducing a
160160
When the PVC is deleted, the PV is moved into `Released` state and following checks are made:
161161
162162
1. In-Tree Volumes:
163-
`DeletionTimestamp` checks can be ignored, instead volume being in a `Released` state is sufficient criteria. The existing `DeletionTimestamp` check incorrectly assumes that the PV cannot be deleted prior to deleting the PVC. On deleting a PV, a `DeletionTimestamp` is set on the PV, when the PVC is deleted, an explicit sync on the PV is triggered, the existing `DeletionTimestamp` check assumes that the volume is already under deletion and skips calling the plugin to delete the volume from underlying storage.
163+
`DeletionTimestamp` checks can be ignored, instead volume being in a `Released` state is sufficient criteria. The existing `DeletionTimestamp` check incorrectly assumes that the PV cannot be deleted prior to deleting the PVC. On deleting a PV, a `DeletionTimestamp` is set on the PV, when the PVC is deleted, an explicit sync on the PV is triggered, the existing `DeletionTimestamp` check assumes that the volume is already under deletion and skips calling the plugin to delete the volume from underlying storage.
164164
165165
2. CSI driver
166166
167167
If when processing the PV update it is observed that it has `external-provisioner.volume.kubernetes.io/finalizer` finalizer and
168168
`DeletionTimestamp` set, then the volume deletion request is forwarded to the driver, provided other pre-defined conditions are met.
169169
170+
When a PV with `Delete` reclaim policy is not associated with a PVC:
171+
172+
Under the event that the PV is not associated with a PVC, either finalizers `kubernetes.io/pv-controller` or `external-provisioner.volume.kubernetes.io/finalizer` are not added to the PV.
173+
If such a PV is deleted the reclaim workflow is not executed, this is the current behavior and will be retained.
174+
170175
### Risks and Mitigations
171176
172177
The primary risk is volume deletion to a user that expect the current behavior, i.e, they do not expect the volume to be
@@ -197,6 +202,10 @@ Once the volume is deleted from the backend, the finalizer can be removed. This
197202
198203
Note: This feature should work with CSI Migration disabled or enabled.
199204
205+
Statically provisioned volumes would behave the same as dynamically provisioned volumes except in cases where the PV is not associated with a PVC, in such cases finalizer `external-provisioner.volume.kubernetes.io/finalizer` is not added.
206+
207+
If at any point a statically provisioned PV is `Bound` to a PVC, then the finalizer `external-provisioner.volume.kubernetes.io/finalizer` gets added by the external-provisioner.
208+
200209
### In-Tree Plugin volumes
201210
202211
A new finalizer `kubernetes.io/pv-controller` will be introduced. The finalizer will be added to newly created in-tree volumes as well as existing in-tree volumes. The finalizer will only be removed after the plugin successfully deletes the in-tree volume.
@@ -248,6 +257,11 @@ The pv-controller is also expected to add the finalizer to all existing in-tree
248257
249258
The pv-controller would also be responsible to add or remove the finalizer based on CSI Migration being disabled or enabled respectively.
250259
260+
261+
Statically provisioned volumes would behave the same as dynamically provisioned volumes except in cases where the PV is not associated with a PVC, in such cases finalizer `kubernetes.io/pv-controller` is not added.
262+
263+
If at any point a statically provisioned PV is `Bound` to a PVC, then the finalizer `kubernetes.io/pv-controller` gets added by the pv-controller.
264+
251265
### Test Plan
252266
253267
#### E2E tests
@@ -268,23 +282,16 @@ An e2e test to exercise deletion of PV prior to deletion of PVC for a `Bound` PV
268282
* Deployed in production and have gone through at least one K8s upgrade.
269283
270284
### Upgrade / Downgrade Strategy
271-
* Upgrade from old Kubernetes(1.22) to new Kubernetes(1.23) with `HonorPVReclaimPolicy` flag enabled
285+
286+
* Upgrade from old Kubernetes(1.25) to new Kubernetes(1.26) with `HonorPVReclaimPolicy` flag enabled
272287
273288
In the above scenario, the upgrade will cause change in default behavior as described in the current KEP. Additionally,
274289
if there are PVs that have a valid associated PVC and deletion timestamp set, then a finalizer is added to the PV.
275290
276-
* Downgrade from new Kubernetes(1.23) to old Kubernetes(1.22).
291+
* Downgrade from new Kubernetes(1.26) to old Kubernetes(1.25).
277292
278293
In this case, there may be PVs with the deletion finalizer that the older Kubernetes does not remove. Such PVs will be in the API server forever unless if the user manually removes them.
279294
280-
* Upgrade from old Kubernetes(1.23) to new Kubernetes(1.24) with `HonorPVReclaimPolicy` flag enabled
281-
282-
On Kubernetes(1.23) with `HonorPVReclaimPolicy` flag enabled, in-tree plugin volumes still exhibited the issue described in the kep. On upgrading to Kubernetes(1.24) with `HonorPVReclaimPolicy` flag enabled, the in-tree plugin volumes will exhibit the new behavior described in this kep.
283-
284-
* Downgrade from new Kubernetes(1.24) to old Kubernetes(1.23).
285-
286-
In this case, the in-tree plugin volume PVs will have the newly introduced finalizer, such PVs cannot be removed from the API server unless the user manually removes the finalizer.
287-
288295
### Version Skew Strategy
289296
The fix is part of `kube-controller-manager` and `external-provisioner`.
290297
@@ -382,8 +389,7 @@ No.
382389
383390
## Implementation History
384391
385-
1.23: Alpha
386-
1.24: Beta
392+
1.26: Alpha
387393
388394
## Drawbacks
389395
None. The current behavior could be considered as a drawback, the KEP presents the fix to the drawback. The current

keps/sig-storage/2644-honor-pv-reclaim-policy/kep.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ owning-sig: sig-storage
66
participating-sigs:
77
status: implementable
88
creation-date: 2021-05-05
9-
last-updated: 2022-01-24
9+
last-updated: 2022-10-05
1010
reviewers:
1111
- "@xing-yang"
1212
- "@jsafrane"
@@ -16,14 +16,14 @@ approvers:
1616
see-also:
1717
replaces:
1818

19-
stage: beta
19+
stage: alpha
2020

21-
latest-milestone: "v1.24"
21+
latest-milestone: "v1.26"
2222

2323
milestone:
2424
alpha: "v1.23"
25-
beta: "v1.24"
26-
stable: "v1.25"
25+
beta: "v1.27"
26+
stable: "v1.28"
2727

2828
feature-gates:
2929
- name: HonorPVReclaimPolicy

0 commit comments

Comments
 (0)