Bug 2051693 - DataSource (which has a golden image and was opted-in/out using cdi label) will be reconciled and will not actually be opted out
Summary: DataSource (which has a golden image and was opted-in/out using cdi label) wi...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Container Native Virtualization (CNV)
Classification: Red Hat
Component: SSP
Version: 4.10.0
Hardware: Unspecified
OS: Unspecified
urgent
urgent
Target Milestone: ---
: 4.10.0
Assignee: Dominik Holler
QA Contact: Ruth Netser
URL:
Whiteboard:
Depends On:
Blocks: 2051909
TreeView+ depends on / blocked
 
Reported: 2022-02-07 19:21 UTC by Ruth Netser
Modified: 2022-03-16 16:07 UTC (History)
5 users (show)

Fixed In Version: kubevirt-ssp-operator-container-v4.10.0-48 in v4.10.0-677
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 2051909 (view as bug list)
Environment:
Last Closed: 2022-03-16 16:06:49 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github kubevirt containerized-data-importer pull 2142 0 None Merged Reconcile DataSource PVC on update if managed by DataImportCron 2022-02-09 17:06:05 UTC
Github kubevirt containerized-data-importer pull 2147 0 None Merged [release-v1.43] Reconcile DataSource PVC on update if managed by DataImportCron 2022-02-10 08:50:47 UTC
Github kubevirt ssp-operator pull 307 0 None open Fix disabling auto-update for a single DataSource 2022-02-14 11:49:39 UTC
Github kubevirt ssp-operator pull 308 0 None open [release-v0.13] Fix disabling auto-update for a single DataSource 2022-02-14 13:02:22 UTC
Red Hat Issue Tracker CNV-16252 0 None None None 2022-02-17 13:01:32 UTC
Red Hat Product Errata RHSA-2022:0947 0 None None None 2022-03-16 16:07:04 UTC

Description Ruth Netser 2022-02-07 19:21:27 UTC
Description of problem:
Deleted DataSource (which has a golden image and was opted-in/out with cdi label) is not reconciled
If a DataSource is opted in using the label, if the label is removed (to opt out), DIC will re-add it, which means that effectively the DataSource will continue to be managed by DIC.

Version-Release number of selected component (if applicable):
CNV 4.10.0, SSP v4.10.0-47

How reproducible:
100%

Steps to Reproduce:
1. opt out from enableCommonBootImageImport
2. import fedora golden image (named "fedora" under "openshift-virtualization-os-images" namespace)
3. opt in to enableCommonBootImageImport
4. Add "cdi.kubevirt.io/dataImportCron: 'true'" label to fedora DataSource 
(make sure the DataSource is updated with auto-updated fedora DV)
5. Delete the label
6. Delete fedora DataSource

Actual results:
The DataSource is not recreated

Expected results:
DataSource should be reconciled.
workaround - opt out -> opt in 
The workaround is not a good solution as it would affect all DataSources with existing golden images PVCs

Additional info:

Comment 1 Andrej Krejcir 2022-02-08 09:37:07 UTC
Currently, when the label is added to DataSource, SSP removes annotations from the DataSource that mark it as owned by SSP. As a result SSP operator will not watch and react to changes on this DS. 
CDI watches the DS and recreates it, if it is deleted. But when the label is removed, then CDI does not recreate it anymore.

I will try to solve this.

Comment 2 Andrej Krejcir 2022-02-08 09:55:23 UTC
I think we cannot solve this without adding a new annotation to DataSource, because without it, there would be race between SSP and CDI:

1. User adds label cdi.kubevirt.io/dataImportCron: 'true' to DataSource
2. SSP creates DataImportCron
3. CDI updated the DataSource
4. User removes the label from DS
5. Now there is a race:
  - If DIC is modified in any way, it triggers CDI reconciliation and the label is added back to DS
  - SSP wants to remove the DIC, because DS does not have the label.


I will add a new annotation to DataSource, that will specify if auto-update should be enabled for it.

Comment 3 Arnon Gilboa 2022-02-08 13:21:13 UTC
akrejcir

What about this solution?

If DataSource has NO dataImportCron label (or user removed it), CDI will:
(1) Not add a label (DIC controller)
(2) Not update the PVC (DIC controller)
(3) Update status conditions as usual (DataSource controller)

If DataSource has dataImportCron label (or user added it), CDI will:
(1) Update the label to the managing DataImportCron (DIC controller)
(2) Update the PVC on new imports (DIC controller)
(3) Update status conditions as usual (DataSource controller)

Comment 4 Andrej Krejcir 2022-02-08 13:32:59 UTC
That would help for this bug.

Then change in SSP will be simple.

Comment 5 Ruth Netser 2022-02-08 13:37:49 UTC
(In reply to Arnon Gilboa from comment #3)
> akrejcir
> 
> What about this solution?
> 
> If DataSource has NO dataImportCron label (or user removed it), CDI will:
> (1) Not add a label (DIC controller)
> (2) Not update the PVC (DIC controller)
> (3) Update status conditions as usual (DataSource controller)
In this case SSP will need to delete the dataImportCron
> 
> If DataSource has dataImportCron label (or user added it), CDI will:
> (1) Update the label to the managing DataImportCron (DIC controller)
> (2) Update the PVC on new imports (DIC controller)
> (3) Update status conditions as usual (DataSource controller)

Comment 6 Arnon Gilboa 2022-02-08 16:37:55 UTC
@akrejcir

If SSP want the DataSource to be immediately reconciled and updated to point the last PVC imported by the DIC, you should set `cdi.kubevirt.io/dataImportCron` to the name of the responsible DIC. Otherwise its PVC will be updated only in the next import.

Comment 7 Arnon Gilboa 2022-02-10 09:31:53 UTC
@akrejcir

changing status back to ASSIGNED, as both SSP and CDI fixes are needed.

Comment 8 Ruth Netser 2022-02-15 14:09:39 UTC
Verified with SSP operator v4.10.0-48

Tested:
- Feature disabled - DAS is reconciled when deleted/edited
- Feature enabled (auto-update DV) - DAS and DIC are reconciled when deleted/edited
- Feature enabled (dangling DAS) - DAS is reconciled when deleted/edited
- Feature enabled, existing PVC (without dic label) - DAS is reconciled when deleted/edited, DIC is not created
- Feature enabled, existing PVC (without dic label) - delete existing PVC - DIC is created, auto-update DV is imported
- Feature enabled, existing PVC (add dic label) - DIC is created, DAS and DIC are reconciled when deleted/edited
- Feature enabled, existing PVC (add -> delete dic label) - DAS is reconciled when deleted/edited; DIC is deleted

Comment 12 errata-xmlrpc 2022-03-16 16:06:49 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 Virtualization 4.10.0 Images security and bug fix 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:0947


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