Bug 2018428 - PVC is deleted along with VM even with "Delete Disks" unchecked
Summary: PVC is deleted along with VM even with "Delete Disks" unchecked
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: OpenShift Container Platform
Classification: Red Hat
Component: Console Kubevirt Plugin
Version: 4.8
Hardware: Unspecified
OS: Unspecified
urgent
high
Target Milestone: ---
: 4.10.0
Assignee: Aviv Turgeman
QA Contact: Guohua Ouyang
URL:
Whiteboard:
Depends On:
Blocks: 2019736
TreeView+ depends on / blocked
 
Reported: 2021-10-29 09:54 UTC by Guohua Ouyang
Modified: 2022-03-10 16:23 UTC (History)
10 users (show)

Fixed In Version:
Doc Type: No Doc Update
Doc Text:
Clone Of:
Environment:
Last Closed: 2022-03-10 16:23:25 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
it keeps loading in the delete modal (769.12 KB, image/gif)
2021-11-03 05:37 UTC, Guohua Ouyang
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Github openshift console pull 10382 0 None Merged Bug 2018428: PVC is deleted along with VM even with "Delete Disks" unchecked 2021-11-03 08:09:09 UTC
Red Hat Product Errata RHSA-2022:0056 0 None None None 2022-03-10 16:23:40 UTC

Description Guohua Ouyang 2021-10-29 09:54:14 UTC
Description of problem:
PVC is deleted along with VM even with "Delete Disks" unchecked, this is a regression from OCP 4.7.

Version-Release number of selected component (if applicable):
OCP 4.8

How reproducible:
100%

Steps to Reproduce:
1. Create a VM with PVC created
2. Delete the VM and uncheck "Delete Disks" on the delete modal.
3.

Actual results:
PVC is deleted along with the VM

Expected results:
PVC is not deleted if the checkbox "Delete Disks" is unchecked.

Additional info:

Comment 1 Guohua Ouyang 2021-10-29 11:15:13 UTC
@Kobi, There is a customer request for it, we may need to backport the fix to 4.8.z and 4.9.z, can we do that?

Comment 2 Tal Nisan 2021-10-31 13:13:43 UTC
(In reply to Guohua Ouyang from comment #1)
> @Kobi, There is a customer request for it, we may need to backport the fix
> to 4.8.z and 4.9.z, can we do that?

Will be taken care of ASAP, thank you for opening the bug

Comment 3 Aviv Turgeman 2021-11-01 09:00:32 UTC
Hi all,

it looks like the 'DataVolume' is owned by the VM and deleted automatically when the VM is deleted,
is this the expected behavior? 
Should we remove the option for not deleting the PVCs when deleting the VM, since the backend already take care of it?

Comment 4 Fabian Deutsch 2021-11-01 09:35:07 UTC
> it looks like the 'DataVolume' is owned by the VM and deleted automatically when the VM is deleted,
> is this the expected behavior? 

If the DV is defined in the VM's DVTemplate, then yes.

BUT If an independently existing PVC (owned by ie. a user) is attached to a VM, then this (previously existing PVC) must not be removed when the VM is getting deleted (because it is not owned by the VM).

> Should we remove the option for not deleting the PVCs when deleting the VM, since the backend already take care of it?
@

Comment 5 Fabian Deutsch 2021-11-01 09:37:11 UTC
(kbd issue, reposting)

> it looks like the 'DataVolume' is owned by the VM and deleted automatically when the VM is deleted,
> is this the expected behavior? 

If the DV is defined in the VM's DVTemplate, then yes.

BUT If an independently existing PVC (owned by ie. a user) is attached to a VM, then this (previously existing PVC) must not be removed when the VM is getting deleted (because it is not owned by the VM).

> Should we remove the option for not deleting the PVCs when deleting the VM, since the backend already take care of it?

We have VM owned (DVs based on VM DVTemplates) and VM un-owned resources (i.e. PVCs).
For VM owned resources it makes sense to have a way to opt-out of the automatic deletion. But Adam needs to tell if this is posisble in a safe manner.

For VM un-owned resources this problem does not apply, because they are anyhow not deleted when the VM is getting deleted.

Comment 6 Yaacov Zamir 2021-11-01 10:25:20 UTC
Aviv hi,

So ... my suggestion for the fix is:


The delete model will have to look at all PVC and DVs

```
[x] delete independent disks (false by default)

Disks:
Owned         2 (will be deleted automatically)
Independent   1 (will be deleted / will persist) <- string depending on the check box, ** Default to false **

@fdeutsch Does that makes sense to you ?

Comment 7 Fabian Deutsch 2021-11-01 10:34:16 UTC
> Does that makes sense to you ?

The idea yes, but I would avoid to introduce the new term "independent" (I merely used to describe it in this forum).
Instead I would use the already known or more common term "owned" and "not owned".

Comment 8 Yaacov Zamir 2021-11-01 10:36:21 UTC
p.s.
I think this feature was created before we moved to use dataVolumes and is not needed anymore since deletion of owned PVC is done by the backend.

@fdeutsch maybe we don't need this functionality at all anymore because it's done by the backend ?

Comment 9 Fabian Deutsch 2021-11-01 11:03:48 UTC
We should maintain the functionality to opt-out of DV (and thus impl PVC) removal.
We must also maintain the message to state what happens to own and not-owned disks.

But I agree that we might not need the functionality to delete un-owned disks (PVCs and DVs alike)

Comment 10 Yaacov Zamir 2021-11-01 11:26:01 UTC
> We have VM owned (DVs based on VM DVTemplates) and VM un-owned resources (i.e. PVCs).
> For VM owned resources it makes sense to have a way to opt-out of the automatic deletion. But Adam needs to tell if this is posisble in a safe manner.

Adam hi,

We used to have an implicit way to delete PVCs we created manually only for the VM, but now we are using DVs that are auto-created and also auto-deleted.
Is there a safe way to not delete the DV owned by the VM when deleting a VM?

Comment 11 Yaacov Zamir 2021-11-01 11:32:37 UTC
Aviv hi,

so depending on Adma's answer we can do:

a. we can opt out of deleting owned DV:

```
[x] delete all owned disks (default to true)

owned disks     : 1 will be deleted / will persist
un-owned disks  : 2 will persist
```

b. or, if we can't opt out of deleting the owned DVs:

```
owned disks     : 1 will be deleted
un-owned disks  : 2 will persist
```

Fabian, based on my above suggestions, did I understand correctly?

Comment 13 Fabian Deutsch 2021-11-01 15:33:51 UTC
Yes.

Comment 14 Yaacov Zamir 2021-11-01 17:21:06 UTC
Aviv hi,

please fix using option b. (https://bugzilla.redhat.com/show_bug.cgi?id=2018428#c11)

```
owned disks     : 1 will be deleted
un-owned disks  : 2 will persist
```

So we will have a fix.

p.s.
When Adam answer https://bugzilla.redhat.com/show_bug.cgi?id=2018428#c10 , we will update the fix accordingly.

Comment 15 Adam Litke 2021-11-01 18:12:15 UTC
You can always delete the owner reference on the DV and it will not get deleted.

Comment 16 Adam Litke 2021-11-01 18:16:56 UTC
I think the best behavior is as follows:
- Delete disks checked (default): Owned disks will be deleted.  Provide an optional check box for each unowned disk so that it can also be deleted
- Delete disks unchecked: Do not remove any disks (remove ownerreferences on owned DVs/PVCs to ensure they are preserved)

In both cases, the user should be shown (in the modal) what will or will not be deleted.

Comment 17 Yaacov Zamir 2021-11-01 18:22:52 UTC
Adam, I think we tried to remove the owner, and it got populated back immediately (operator auto healing ?)

Aviv, can you check if something is re-owning the DVs, or it was just us not removing the owner curreclty ?

Comment 20 Adam Litke 2021-11-02 13:57:29 UTC
As a workaround the following can be done to preserve the DataVolume and PVC for a VM disk when deleting the VM:

Edit the VM: remove the entire dataVolumeTemplates section from the VM spec
Edit the DV: remove the ownerReference from the DV
Delete the VM

The DataVolume will be left intact.

Comment 22 Guohua Ouyang 2021-11-03 05:36:21 UTC
Test on the master branch:
Good:
1. if the VM owned the disk, delete with disk with "Delete Disks" checked, disk is deleted along with the VM.
2. if the VM owned the disk, delete with disk with "Delete Disks" unchecked, disk is not deleted along with the VM.
Bad:
3. if the disk is not owned by the VM(attach existing PVC), the delete modal cannot load normally.

Comment 23 Guohua Ouyang 2021-11-03 05:37:11 UTC
Created attachment 1839493 [details]
it keeps loading in the delete modal

Comment 24 Yaacov Zamir 2021-11-03 08:08:44 UTC
should be fixed by: https://github.com/openshift/console/pull/10382
moving to modified

Comment 25 Yaacov Zamir 2021-11-03 08:24:49 UTC
> Created attachment 1839493 [details]
> it keeps loading in the delete modal

Guohua, can you check for the "keeps loading the modal" on an un-fixed version, looks like an unrelated bug we need to fix

Comment 26 Aviv Turgeman 2021-11-03 08:38:08 UTC
(In reply to Guohua Ouyang from comment #22)

> Bad:
> 3. if the disk is not owned by the VM(attach existing PVC), the delete modal
> cannot load normally.

Hi Guohua,

I can see the problem you are refering to, but this should be opened in a separate BZ.
I've opened this BZ [1] to follow up on the issue you mention. I will send a fix for this today.
thank you for noticing

Comment 27 Guohua Ouyang 2021-11-03 09:01:29 UTC
(In reply to Yaacov Zamir from comment #25)
> > Created attachment 1839493 [details]
> > it keeps loading in the delete modal
> 
> Guohua, can you check for the "keeps loading the modal" on an un-fixed
> version, looks like an unrelated bug we need to fix

Yes, it exists in un-fixed version.
So move the bug to verified

Comment 28 Yaacov Zamir 2021-11-03 09:17:40 UTC
Aviv opened: 
https://bugzilla.redhat.com/show_bug.cgi?id=2019717

Comment 31 errata-xmlrpc 2022-03-10 16:23:25 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Moderate: OpenShift Container Platform 4.10.3 security update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2022:0056


Note You need to log in before you can comment on or make changes to this bug.