Always delete pvc when tenant is deleted #268

kerneltime · 2020-08-27T00:19:37Z

If we want to make this optional, we can add a new field in the CRD.

> kubectl get pvc                                   
NAME                   STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
data0-minio-zone-0-0   Bound    pvc-cf812963-bed1-4487-a260-4cb8bec6921a   1Ti        RWO            standard       3m19s
data0-minio-zone-0-1   Bound    pvc-a2b768ef-af27-452a-b7b7-f91e1311ac1a   1Ti        RWO            standard       3m19s
data0-minio-zone-0-2   Bound    pvc-b5a6652e-99db-4e46-96c5-52e906ecb5f2   1Ti        RWO            standard       3m19s
data0-minio-zone-0-3   Bound    pvc-e7fdefae-4143-4cd3-b7f3-084efaf64edd   1Ti        RWO            standard       3m19s
data1-minio-zone-0-0   Bound    pvc-94c90bbc-861d-4e99-9f40-e1a408e3ff5d   1Ti        RWO            standard       3m19s
data1-minio-zone-0-1   Bound    pvc-09569871-2613-451e-aa01-9fa5299e13bf   1Ti        RWO            standard       3m19s
data1-minio-zone-0-2   Bound    pvc-ce6a1890-8c4a-4dcf-a39d-1eb81b67509b   1Ti        RWO            standard       3m19s
data1-minio-zone-0-3   Bound    pvc-78982b3e-4c19-451e-950c-6569d9a96e52   1Ti        RWO            standard       3m19s
data2-minio-zone-0-0   Bound    pvc-0697ab06-de65-4beb-886f-cbaac11bcab4   1Ti        RWO            standard       3m19s
data2-minio-zone-0-1   Bound    pvc-2db7d109-b678-4ba3-a093-92615e87381f   1Ti        RWO            standard       3m19s
data2-minio-zone-0-2   Bound    pvc-4ee2510a-0591-497a-86ae-4da6d42191f7   1Ti        RWO            standard       3m19s
data2-minio-zone-0-3   Bound    pvc-bb946db4-0bf9-4306-9ba5-3cae4508f93d   1Ti        RWO            standard       3m19s
data3-minio-zone-0-0   Bound    pvc-d3c13b68-258d-4061-b6cd-7a9a0f2ef167   1Ti        RWO            standard       3m19s
data3-minio-zone-0-1   Bound    pvc-c242cfdf-f16d-4173-8b5e-0658067433d8   1Ti        RWO            standard       3m19s
data3-minio-zone-0-2   Bound    pvc-c52551ed-2469-454a-abd3-df1766b16f1f   1Ti        RWO            standard       3m19s
data3-minio-zone-0-3   Bound    pvc-a88b0298-40dd-41db-97f6-63a17921de57   1Ti        RWO            standard       3m19s
 ~/go/src/github.com/minio/minio-operator  master >1 *8 !2 ?5 ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 05:20:39 PM 
> kubectl delete -f examples/tenant-console.yaml          
secret "minio-creds-secret" deleted
secret "console-secret" deleted
tenant.minio.min.io "minio" deleted
 ~/go/src/github.com/minio/minio-operator  master >1 *8 !2 ?5 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- kind-kind kube  05:20:48 PM 
> kubectl get pvc                               
NAME                   STATUS        VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
data0-minio-zone-0-0   Terminating   pvc-cf812963-bed1-4487-a260-4cb8bec6921a   1Ti        RWO            standard       3m31s
data0-minio-zone-0-1   Terminating   pvc-a2b768ef-af27-452a-b7b7-f91e1311ac1a   1Ti        RWO            standard       3m31s
data0-minio-zone-0-2   Terminating   pvc-b5a6652e-99db-4e46-96c5-52e906ecb5f2   1Ti        RWO            standard       3m31s
data0-minio-zone-0-3   Terminating   pvc-e7fdefae-4143-4cd3-b7f3-084efaf64edd   1Ti        RWO            standard       3m31s
data1-minio-zone-0-0   Terminating   pvc-94c90bbc-861d-4e99-9f40-e1a408e3ff5d   1Ti        RWO            standard       3m31s
data1-minio-zone-0-1   Terminating   pvc-09569871-2613-451e-aa01-9fa5299e13bf   1Ti        RWO            standard       3m31s
data1-minio-zone-0-2   Terminating   pvc-ce6a1890-8c4a-4dcf-a39d-1eb81b67509b   1Ti        RWO            standard       3m31s
data1-minio-zone-0-3   Terminating   pvc-78982b3e-4c19-451e-950c-6569d9a96e52   1Ti        RWO            standard       3m31s
data2-minio-zone-0-0   Terminating   pvc-0697ab06-de65-4beb-886f-cbaac11bcab4   1Ti        RWO            standard       3m31s
data2-minio-zone-0-1   Terminating   pvc-2db7d109-b678-4ba3-a093-92615e87381f   1Ti        RWO            standard       3m31s
data2-minio-zone-0-2   Terminating   pvc-4ee2510a-0591-497a-86ae-4da6d42191f7   1Ti        RWO            standard       3m31s
data2-minio-zone-0-3   Terminating   pvc-bb946db4-0bf9-4306-9ba5-3cae4508f93d   1Ti        RWO            standard       3m31s
data3-minio-zone-0-0   Terminating   pvc-d3c13b68-258d-4061-b6cd-7a9a0f2ef167   1Ti        RWO            standard       3m31s
data3-minio-zone-0-1   Terminating   pvc-c242cfdf-f16d-4173-8b5e-0658067433d8   1Ti        RWO            standard       3m31s
data3-minio-zone-0-2   Terminating   pvc-c52551ed-2469-454a-abd3-df1766b16f1f   1Ti        RWO            standard       3m31s
data3-minio-zone-0-3   Terminating   pvc-a88b0298-40dd-41db-97f6-63a17921de57   1Ti        RWO            standard       3m31s

dvaldivia

Why not introduce the option right away and make it off by default, this behavior is dangerous, specially if undocumented, the default behavior in kubernetes for statefulset is not to delete the PVCs.

If we enable this behavior this way we also cause that we cannot upgrade the tenant resource in the future (migrate v1 to v2 for example) since that would trigger deletion of all the PVCs. We need a more comprehensive solution than this.

kerneltime · 2020-08-27T00:36:44Z

Why not introduce the option right away and make it off by default, this behavior is dangerous, specially if undocumented, the default behavior in kubernetes for statefulset is not to delete the PVCs.

If we enable this behavior this way we also cause that we cannot upgrade the tenant resource in the future (migrate v1 to v2 for example) since that would trigger deletion of all the PVCs. We need a more comprehensive solution than this.

Yes, I plan to add that and that is why the question. This is the recommended scheme for handling this requirement. There are other complications for linking a PVC to a stateful set as the stateful set pod is created after the PVC.
This should not block upgrades from tenant CRD version from v1 to v2 but would need additional handling if the upgrade mechanism is to delete the stateful and upgrade it.. it is possible to remove the owner reference.

kerneltime · 2020-08-27T01:37:32Z

Example of removing owner reference:

> kubectl get pvc                               
NAME                   STATUS        VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
data0-minio-zone-0-0   Bound         pvc-49593fbb-047a-4670-a9c1-8e96a50927d6   1Ti        RWO            standard       93s
data0-minio-zone-0-1   Terminating   pvc-7fcdac7e-bfba-44f9-9b96-da2be7dda8e6   1Ti        RWO            standard       93s
data0-minio-zone-0-2   Terminating   pvc-6eb8f49b-8253-4a6d-8097-237646294e96   1Ti        RWO            standard       92s
data0-minio-zone-0-3   Terminating   pvc-935288ea-ab4e-41ce-a08d-c256f05876bb   1Ti        RWO            standard       92s
data1-minio-zone-0-0   Terminating   pvc-bd79a8dc-1a64-44b4-b979-968d6e0b88c0   1Ti        RWO            standard       93s
data1-minio-zone-0-1   Terminating   pvc-e95db9b6-bb7a-4b97-b5ef-8a355cad6b27   1Ti        RWO            standard       93s
data1-minio-zone-0-2   Terminating   pvc-be5a8fe7-f849-4fbb-91c2-03eb36cd4b68   1Ti        RWO            standard       92s
data1-minio-zone-0-3   Terminating   pvc-d8c98416-f098-4a85-a7fb-dc1693bfa381   1Ti        RWO            standard       92s
data2-minio-zone-0-0   Terminating   pvc-bf07691e-5efc-4d67-b674-770520639126   1Ti        RWO            standard       93s
data2-minio-zone-0-1   Terminating   pvc-7122e408-ad28-40d8-ace4-f7e35c7a70d9   1Ti        RWO            standard       92s
data2-minio-zone-0-2   Terminating   pvc-4a142148-d1a3-4bcd-8975-f0a40ecc40ef   1Ti        RWO            standard       92s
data2-minio-zone-0-3   Terminating   pvc-97918bca-7391-4226-a90a-11ef241e8184   1Ti        RWO            standard       92s
data3-minio-zone-0-0   Terminating   pvc-fa17ab35-ede0-4e3c-8ddd-0060ec37364b   1Ti        RWO            standard       93s
data3-minio-zone-0-1   Terminating   pvc-a9327eb2-1ee3-487d-8f85-3973da78eef9   1Ti        RWO            standard       92s
data3-minio-zone-0-2   Terminating   pvc-67a10c38-e945-48af-a5f9-b970f3930a32   1Ti        RWO            standard       92s
data3-minio-zone-0-3   Terminating   pvc-85567add-7803-45b7-a509-1271360f3ada   1Ti        RWO            standard       92s

wlan0 · 2020-08-30T01:19:36Z

LGTM, minor questions

pkg/apis/minio.min.io/v1/types.go

pkg/controller/cluster/main-controller.go

pkg/resources/statefulsets/minio-statefulset.go

pkg/controller/cluster/main-controller.go

nitisht

Few comments / questions

pkg/controller/cluster/main-controller.go

Add finalizer for tenant. Signed-off-by: Ritesh H Shukla <ritesh@minio.io>

nitisht

LGTM 👍

wlan0 · 2020-09-01T16:41:58Z

LGTM

kerneltime requested review from harshavardhana, dvaldivia and nitisht August 27, 2020 00:19

dvaldivia requested changes Aug 27, 2020

View reviewed changes

kerneltime force-pushed the cleanup branch 3 times, most recently from 881ed42 to c59fbd7 Compare August 27, 2020 01:33

kerneltime force-pushed the cleanup branch 6 times, most recently from 4c8aeae to 8c5c473 Compare August 28, 2020 18:05

harshavardhana previously approved these changes Aug 28, 2020

View reviewed changes

harshavardhana requested a review from dvaldivia August 28, 2020 19:11

kerneltime dismissed harshavardhana’s stale review via 1c7fa2f August 29, 2020 20:57

kerneltime force-pushed the cleanup branch from 8c5c473 to 1c7fa2f Compare August 29, 2020 20:57

kerneltime requested a review from wlan0 August 29, 2020 20:58

kerneltime force-pushed the cleanup branch from 1c7fa2f to 219169b Compare August 29, 2020 21:12

wlan0 approved these changes Aug 30, 2020

View reviewed changes

wlan0 suggested changes Aug 30, 2020

View reviewed changes

pkg/apis/minio.min.io/v1/types.go Outdated Show resolved Hide resolved

pkg/controller/cluster/main-controller.go Show resolved Hide resolved

dvaldivia requested changes Aug 30, 2020

View reviewed changes

kerneltime force-pushed the cleanup branch from 219169b to 916970a Compare August 30, 2020 04:05

harshavardhana requested review from wlan0, dvaldivia and harshavardhana August 31, 2020 04:20

harshavardhana previously approved these changes Aug 31, 2020

View reviewed changes

nitisht suggested changes Sep 1, 2020

View reviewed changes

pkg/controller/cluster/main-controller.go Show resolved Hide resolved

pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved

pkg/controller/cluster/main-controller.go Outdated Show resolved Hide resolved

pkg/controller/cluster/main-controller.go Show resolved Hide resolved

Allow deletion pvc when tenant is deleted

4de74bc

Add finalizer for tenant. Signed-off-by: Ritesh H Shukla <ritesh@minio.io>

kerneltime dismissed harshavardhana’s stale review via 4de74bc September 1, 2020 15:12

kerneltime force-pushed the cleanup branch from 916970a to 4de74bc Compare September 1, 2020 15:12

nitisht approved these changes Sep 1, 2020

View reviewed changes

harshavardhana approved these changes Sep 1, 2020

View reviewed changes

wlan0 approved these changes Sep 1, 2020

View reviewed changes

harshavardhana merged commit 23be5c3 into minio:master Sep 1, 2020

kerneltime deleted the cleanup branch September 1, 2020 17:29

dvaldivia mentioned this pull request Sep 25, 2020

Revert Tenant Finalizers #312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always delete pvc when tenant is deleted #268

Always delete pvc when tenant is deleted #268

kerneltime commented Aug 27, 2020 •

edited

dvaldivia left a comment

kerneltime commented Aug 27, 2020

kerneltime commented Aug 27, 2020

wlan0 commented Aug 30, 2020 •

edited

nitisht left a comment

nitisht left a comment

wlan0 commented Sep 1, 2020

Always delete pvc when tenant is deleted #268

Always delete pvc when tenant is deleted #268

Conversation

kerneltime commented Aug 27, 2020 • edited

dvaldivia left a comment

Choose a reason for hiding this comment

kerneltime commented Aug 27, 2020

kerneltime commented Aug 27, 2020

wlan0 commented Aug 30, 2020 • edited

nitisht left a comment

Choose a reason for hiding this comment

nitisht left a comment

Choose a reason for hiding this comment

wlan0 commented Sep 1, 2020

kerneltime commented Aug 27, 2020 •

edited

wlan0 commented Aug 30, 2020 •

edited