Integration of Data volume using CDI populators #2722

ShellyKa13 · 2023-05-21T12:31:54Z

What this PR does / why we need it:
This PR integrates the use of datavolumes with import and upload populators. In case of having CSI storage instead of using the current path of creating a PVC with import/upload annotation which gets bound before the actual population, we create a PVC that has datasourceref pointing to the appropriate populator (import,upload) and create a volumeuploadsource/volumeimportsource with the right values according to the datavolume. In that case the appropriate populator with be triggered and the population with PVC' process will initiate which will result in having the target PVC bound only after the population succeeded.
This feature is crucial for our product to work seamlessly with DR solutions.
This is a valuable feature also when using datavolumes with VMs that are WFFC which will prevent the need for the current need of "doppelganger" pod to make the PVC bound before starting the VM.

Special notes for your reviewer:
added some TODOs I wish the reviewers to address
Still missing UTs

Release note:

Integration of Data volume using CDI populators with CSI storage

alromeros

Good job! First quick review with some comments. Also wondering if we should skip fs overhead inflation in renderPvcSpecVolumeSize when using a populator.

pkg/controller/datavolume/import-controller.go

alromeros · 2023-05-22T15:47:20Z

pkg/controller/datavolume/controller-base.go

@@ -679,6 +680,11 @@ func (r *ReconcilerBase) reconcileProgressUpdate(datavolume *cdiv1.DataVolume, p
 		datavolume.Status.Progress = "N/A"
 	}

+	if progress, ok := pvc.Annotations[cc.AnnImportProgressReporting]; ok {


Do we want to continue with the regular flow (including getPodFromPvc) if the PVC is using a populator?

if the pvc is using populator it should have this annotation, I guess we can add the check for the populators for the time in between the creation of the pvc' and the update of the progress on it

alromeros · 2023-05-22T15:57:03Z

pkg/controller/datavolume/controller-base.go

@@ -701,6 +707,8 @@ func (r *ReconcilerBase) reconcileProgressUpdate(datavolume *cdiv1.DataVolume, p
 		}
 	}
 	// We are not done yet, force a re-reconcile in 2 seconds to get an update.
+	// TODO: if I understand currectly: do we really want to force reconcile even for population which
+	// doesnt report progress such as upload?


Yeah I'm also surprised we reach this point with upload DVs

alromeros · 2023-05-22T16:06:56Z

pkg/controller/datavolume/controller-base.go

+	return r.storageClassCSIDriverExists(pvc.Spec.StorageClassName)
+}
+
+func (r *ReconcilerBase) usePopulatorsForPopulation(syncState *dvSyncState) (bool, error) {


Maybe change this function name to something like shouldUseCDIPopulator, shouldUsePopulator, or checkPopulatorRequirements? I think usePopulatorsForPopulation can sound odd.

pkg/controller/datavolume/upload-controller.go

mhenriks

Good start, I think I've given you enough to think about for now

pkg/controller/populators/import-populator.go

pkg/controller/datavolume/snapshot-clone-controller.go

mhenriks · 2023-05-23T01:13:45Z

pkg/controller/datavolume/controller-base.go

+	return r.storageClassCSIDriverExists(pvc.Spec.StorageClassName)
+}
+
+func (r *ReconcilerBase) usePopulatorsForPopulation(syncState *dvSyncState) (bool, error) {


pkg/controller/datavolume/import-controller.go

mhenriks · 2023-05-23T01:36:49Z

pkg/controller/datavolume/import-controller.go

+	dv := syncState.dvMutated
+	// This cleanup should be done if dv is marked for deletion, dv is succeeded
+	// and data volume is using the import populator
+	if dv.DeletionTimestamp == nil && dv.Status.Phase != cdiv1.Succeeded && !syncState.usePopulator {


this seems complicated to me. If the CR is owned by the dv, then only explicitly delete when the target PVC is bound? Seems simpler to me

it is redundant to do the delete if we don't use populators, and phase succeded is basically the same as target PVC is bound, so you say to remove the deletionTimestamp check when I add the ownerRef, then sure I guess we can

mhenriks · 2023-05-23T01:42:04Z

pkg/controller/datavolume/controller-base.go

+	if pvc.Spec.DataSourceRef == nil {
+		return false, nil
+	}
+	return r.storageClassCSIDriverExists(pvc.Spec.StorageClassName)


rather than look up the driver again, is there an annotation on the pvc we can safely rely on?

right, I added a new annotation to the dv that will be passed to the pvc, so yes i'll check it here instead

pkg/controller/datavolume/controller-base.go

pkg/controller/datavolume/snapshot-clone-controller.go

mhenriks · 2023-05-24T00:27:40Z

pkg/controller/datavolume/util.go

+	if err != nil {
+		return false, err
+	}
+	cc.AddAnnotation(dv, cc.AnnUsePopulator, strconv.FormatBool(usePopulator))


I don't love that this function has the side effect of mutating the datavolume, perhaps it would be better to explicitly set this somehow in one place? Currently, this function is called in multiple places

I guess I can do the update itself on a line after the call to this function and only in one place in the syncDvPvcState function after updating syncState.usePopulator

mhenriks · 2023-05-24T00:37:56Z

pkg/controller/populators/import-populator.go

+	if err := r.updateImportProgress(phase, pvc, pvcPrime); err != nil {
+		//TODO: currently I don't return err in case of failing to report progress, opinion?
+		// Also this will print error every 2 secs
+		klog.Errorf("Failed to update import progress for pvc %s/%s", pvc.Namespace, pvc.Name)


why klog and not the logr.Logger is my first question?

If we requeue every 2secs I guess it isn't a big deal to not handle errors but maybe would be nice

I didn't want to return an error cause I think we shouldn't prevent the phases to be updated just because there might be an issue with the progress reporting.
regarding the klog, I'll fix

mhenriks · 2023-05-24T00:40:40Z

hack/build/run-linters.sh

@@ -4,4 +4,4 @@ GOLANGCI_VERSION="${GOLANGCI_VERSION:-v1.52.2}"

 go install "github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_VERSION}"

-golangci-lint run --timeout=8m ./...
+golangci-lint run --timeout=16m ./...


I did exactly the same thing in #2709

I know, just wanted to make the cdi-lint lane pass until your PR will get merged

I meant this in a "great minds think alike" kind of way

mhenriks · 2023-05-25T00:25:45Z

pkg/controller/datavolume/import-controller.go

+	}
+
+	importSourceName := volumeImportSourceName(dv)
+	importSource := &cdiv1.VolumeImportSource{


Totally forgot that the controller should be watching everything it creates, so should be watching VolumeImportSources owned by datavolumes.

Also, I think it would be better to see if the resource exists before creating, saves a trip to the apiserver, still have to handle already exists because cache is not perfect

mhenriks · 2023-05-31T01:14:07Z

pkg/controller/populators/import-populator.go

@@ -188,6 +187,14 @@ func (r *ImportPopulatorReconciler) updatePVCForPopulation(pvc *corev1.Persisten
 	annotations[cc.AnnSource] = cc.SourceNone
 }

+func deleteBoundConditionIfNeeded(pvc, pvcPrime *corev1.PersistentVolumeClaim) {


What is the purpose of this?

in import there is this code:
// No scratch space, or scratch space is bound, remove annotation
delete(anno, cc.AnnBoundCondition)
delete(anno, cc.AnnBoundConditionMessage)
delete(anno, cc.AnnBoundConditionReason)
I had a test that this annotations where remove from PVC' but it wasnt reflected on the target PVC which caused issues..

mhenriks · 2023-05-31T01:16:23Z

pkg/controller/populators/populator-base.go

+			!strings.Contains(k, cc.AnnPopulatorKind) &&
+			!strings.Contains(k, "upload") &&
+			!strings.Contains(k, "import") {
+			cc.AddAnnotation(pvcCopy, k, v)


Would it be best to simply have a whitelist of allowed annotations? What's your opinion?

it seems there are too much allowed annotations comparing to annotations that shouldn't be added.. and that's how I missed a bunch, I think its easier to know which we don't want to add

Too much? Could probably count all the annotations we care about on two hands? Maybe three? They're all defined and unlikely to change. Would be nice to have one place (or maybe one place for each source) that states "these are the annotations we care about for datavolumes". I also clearly have a preference for whitelists over blacklists.

mhenriks · 2023-05-31T23:10:35Z

pkg/controller/datavolume/controller-base.go

@@ -694,6 +694,11 @@ func (r *ReconcilerBase) reconcileProgressUpdate(datavolume *cdiv1.DataVolume, p
 		return nil
 	}

+	// With upload we don't have report of progress and no need to requeue
+	if datavolume.Spec.Source != nil && datavolume.Spec.Source.Upload != nil {


Should base controller really be doing upload specific stuff? Can it be moved to upload controller?

Or something added to import?

mhenriks · 2023-06-06T00:52:16Z

pkg/controller/datavolume/import-controller.go

+		dataVolume.Spec.Source.Registry != nil &&
+		dataVolume.Spec.Source.Imageio != nil &&
+		dataVolume.Spec.Source.VDDK != nil &&
+		dataVolume.Spec.Source.Blank != nil {


shouldn't all these != be ==?

totally right :) working on the UT so probably would have figured out soon lol

mhenriks · 2023-06-06T02:03:42Z

pkg/controller/populators/populator-base.go

@@ -220,6 +213,31 @@ func (r *ReconcilerBase) createPVCPrime(pvc *corev1.PersistentVolumeClaim, sourc
 	return pvcPrime, nil
 }

+type updatePVCAnnotationsFunc func(pvc, pvcPrime *corev1.PersistentVolumeClaim)
+
+var desiredAnnotations = []string{cc.AnnPodPhase, cc.AnnPodReady, cc.AnnPodRestarts, cc.AnnPreallocationRequested, cc.AnnPreallocationApplied, cc.AnnRunningCondition, cc.AnnRunningConditionMessage, cc.AnnRunningConditionReason, cc.AnnBoundCondition, cc.AnnBoundConditionMessage, cc.AnnBoundConditionReason}


If I understand this correctly, the dv will have bound condition true if pvc' is bound. That does not seem right to me

the update boundcondition on the dv doesnt update the condition to true unless the pvc is bound:
https://github.com/kubevirt/containerized-data-importer/blob/main/pkg/controller/datavolume/conditions.go#L128-L143
so seems to me its ok.

Why bother passing over the annotations then?

I remember there was some tests checking this. I can remove this update and see how it affects our tests if it bothers you.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

The populators are handling existing PVCs. The PVC already has a defined requested size, inflating the PVC' with fsoverhead will only be on the PVC' spec and will not reflect on the target PVC, this seems undesired. Instead if the populators is using by PVC that the datavolume controller created the inflation will happen there if needed. Signed-off-by: Shelly Kagan <skagan@redhat.com>

Signed-off-by: Shelly Kagan <skagan@redhat.com>

Instead if checking if the annotation on pvcPrime is not desired go over desired list and if the annotation exists add it. Signed-off-by: Shelly Kagan <skagan@redhat.com>

Signed-off-by: Shelly Kagan <skagan@redhat.com>

ShellyKa13 · 2023-06-12T15:03:26Z

/unhold

alromeros

Really good job, don't have any comments other than the typo in the test. You'll probably push again anyway but I'm giving a provisional lgtm just in case
/test pull-containerized-data-importer-e2e-hpp-latest
/lgtm

alromeros · 2023-06-12T18:13:27Z

pkg/controller/populators/import-populator_test.go

+			Expect(ok).To(BeFalse())
+		},
+			table.Entry("with pod running phase", string(corev1.PodRunning)),
+			table.Entry("with pod succeded phase", string(corev1.PodFailed)),


Typo: Failed phase

Signed-off-by: Shelly Kagan <skagan@redhat.com>

arnongilboa · 2023-06-13T12:57:28Z

/lgtm

mhenriks · 2023-06-14T00:07:29Z

/retest-required

mhenriks · 2023-06-14T00:27:28Z

/approve
🥇

kubevirt-bot · 2023-06-14T00:27:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mhenriks

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mhenriks]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* move cleanup out of dv deletion It seemed off to call cleanup in the prepare function just because we don't call cleanup unless the dv is deleting. Instead we check in the clenup function itself if it should be done: in this 2 specific cases in case of deletion and in case the dv succeeded. The cleanup will be used in future commit also for population cleanup which we also want to happen not only on deletion. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Use populator if csi storage class exists Add new datavolume phase PendingPopulation to indicate wffc when using populators, this new phase will be used in kubevirt in order to know that there is no need for dummy pod to pass wffc phase and that the population will occur once creating the vm. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Update population targetPVC with pvc prime annotations The annotations will be used to update dv that uses the populators. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Adjust UT with new behavior Signed-off-by: Shelly Kagan <skagan@redhat.com> * updates after review Signed-off-by: Shelly Kagan <skagan@redhat.com> * Fix import populator report progress The import pod should be taken from pvcprime Signed-off-by: Shelly Kagan <skagan@redhat.com> * Prevent requeue upload dv when failing to find progress report pod Signed-off-by: Shelly Kagan <skagan@redhat.com> * Remove size inflation in populators The populators are handling existing PVCs. The PVC already has a defined requested size, inflating the PVC' with fsoverhead will only be on the PVC' spec and will not reflect on the target PVC, this seems undesired. Instead if the populators is using by PVC that the datavolume controller created the inflation will happen there if needed. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Adjust functional tests to handle dvs using populators Signed-off-by: Shelly Kagan <skagan@redhat.com> * Fix clone test Signed-off-by: Shelly Kagan <skagan@redhat.com> * add shouldUpdateProgress variable to know if need to update progress Signed-off-by: Shelly Kagan <skagan@redhat.com> * Change update of annotation from denied list to allowed list Instead if checking if the annotation on pvcPrime is not desired go over desired list and if the annotation exists add it. Signed-off-by: Shelly Kagan <skagan@redhat.com> * fix removing annotations from pv when rebinding Signed-off-by: Shelly Kagan <skagan@redhat.com> * More fixes and UT Signed-off-by: Shelly Kagan <skagan@redhat.com> * a bit more updates and UTs Signed-off-by: Shelly Kagan <skagan@redhat.com> --------- Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Enable empty schedule in DataImportCron (#2711) Allow disabling DataImportCron schedule and support external trigger Signed-off-by: Ido Aharon <iaharon@redhat.com> * expand upon #2721 (#2731) Need to replace requeue bool with requeue duration Signed-off-by: Michael Henriksen <mhenriks@redhat.com> * Add clone from snapshot functionalities to clone-populator (#2724) * Add clone from snapshot functionalities to the clone populator Signed-off-by: Alvaro Romero <alromero@redhat.com> * Update clone populator unit tests to cover clone from snapshot capabilities Signed-off-by: Alvaro Romero <alromero@redhat.com> * Fix storage class assignation in temp-source claim for host-assisted clone from snapshot This commit also includes other minor and styling-related fixes Signed-off-by: Alvaro Romero <alromero@redhat.com> --------- Signed-off-by: Alvaro Romero <alromero@redhat.com> * Prepare CDI testing for the upcoming non-CSI lane (#2730) * Update functional tests to skip incompatible default storage classes Signed-off-by: Alvaro Romero <alromero@redhat.com> * Enable the use of non-csi HPP in testing lanes This commit modifies several scripts to allow the usage of classic HPP as the default SC in tests. This allows us to test our non-populator flow with a non-csi provisioner. Signed-off-by: Alvaro Romero <alromero@redhat.com> --------- Signed-off-by: Alvaro Romero <alromero@redhat.com> * Allow snapshots as format for DataImportCron created sources (#2700) * StorageProfile API for declaring format of resulting cron disk images Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Integrate recommended format in dataimportcron controller Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Take snapclass existence into consideration when populating cloneStrategy and sourceFormat Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> --------- Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Remove leader election test (#2745) Now that we are using the standard k8s leases from the controller runtime library, there is no need to test our implementation as it is no longer in use. This will save some testing time and random failures. Signed-off-by: Alexander Wels <awels@redhat.com> * Integration of Data volume using CDI populators (#2722) * move cleanup out of dv deletion It seemed off to call cleanup in the prepare function just because we don't call cleanup unless the dv is deleting. Instead we check in the clenup function itself if it should be done: in this 2 specific cases in case of deletion and in case the dv succeeded. The cleanup will be used in future commit also for population cleanup which we also want to happen not only on deletion. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Use populator if csi storage class exists Add new datavolume phase PendingPopulation to indicate wffc when using populators, this new phase will be used in kubevirt in order to know that there is no need for dummy pod to pass wffc phase and that the population will occur once creating the vm. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Update population targetPVC with pvc prime annotations The annotations will be used to update dv that uses the populators. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Adjust UT with new behavior Signed-off-by: Shelly Kagan <skagan@redhat.com> * updates after review Signed-off-by: Shelly Kagan <skagan@redhat.com> * Fix import populator report progress The import pod should be taken from pvcprime Signed-off-by: Shelly Kagan <skagan@redhat.com> * Prevent requeue upload dv when failing to find progress report pod Signed-off-by: Shelly Kagan <skagan@redhat.com> * Remove size inflation in populators The populators are handling existing PVCs. The PVC already has a defined requested size, inflating the PVC' with fsoverhead will only be on the PVC' spec and will not reflect on the target PVC, this seems undesired. Instead if the populators is using by PVC that the datavolume controller created the inflation will happen there if needed. Signed-off-by: Shelly Kagan <skagan@redhat.com> * Adjust functional tests to handle dvs using populators Signed-off-by: Shelly Kagan <skagan@redhat.com> * Fix clone test Signed-off-by: Shelly Kagan <skagan@redhat.com> * add shouldUpdateProgress variable to know if need to update progress Signed-off-by: Shelly Kagan <skagan@redhat.com> * Change update of annotation from denied list to allowed list Instead if checking if the annotation on pvcPrime is not desired go over desired list and if the annotation exists add it. Signed-off-by: Shelly Kagan <skagan@redhat.com> * fix removing annotations from pv when rebinding Signed-off-by: Shelly Kagan <skagan@redhat.com> * More fixes and UT Signed-off-by: Shelly Kagan <skagan@redhat.com> * a bit more updates and UTs Signed-off-by: Shelly Kagan <skagan@redhat.com> --------- Signed-off-by: Shelly Kagan <skagan@redhat.com> * Run bazelisk run //robots/cmd/uploader:uploader -- -workspace /home/prow/go/src/github.com/kubevirt/project-infra/../containerized-data-importer/WORKSPACE -dry-run=false (#2751) Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com> * Allow dynamic linked build for non bazel build (#2753) The current script always passes the static ldflag to the compiler which will result in a static binary. We would like to be able to build dynamic libraries instead. cdi-containerimage-server has to be static because we are copying it into the context of a container disk container which is most likely based on a scratch container and has no libraries for us to use. Signed-off-by: Alexander Wels <awels@redhat.com> * Disable DV GC by default (#2754) * Disable DV GC by default DataVolume garbage collection is a nice feature, but unfortunately it violates fundamental principle of Kubernetes. CR should not be auto-deleted when it completes its role (Job with TTLSecondsAfter- Finished is an exception), and once CR was created we can assume it is there until explicitly deleted. In addition, CR should keep idempotency, so the same CR manifest can be applied multiple times, as long as it is a valid update (e.g. DataVolume validation webhook does not allow updating the spec). When GC is enabled, some systems (e.g GitOps / ArgoCD) may require a workaround (DV annotation deleteAfterCompletion = "false") to prevent GC and function correctly. On the next kubevirt-bot Bump kubevirtci PR (with bump-cdi), it will fail on all kubevirtci lanes with tests referring DVs, as the tests IsDataVolumeGC() looks at CDIConfig Spec.DataVolumeTTLSeconds and assumes default is enabled. This should be fixed there. Signed-off-by: Arnon Gilboa <agilboa@redhat.com> * Fix test waiting for PVC deletion with UID Signed-off-by: Arnon Gilboa <agilboa@redhat.com> * Fix clone test assuming DV was GCed Signed-off-by: Arnon Gilboa <agilboa@redhat.com> * Fix DIC controller DV/PVC deletion when snapshot is ready Signed-off-by: Arnon Gilboa <agilboa@redhat.com> --------- Signed-off-by: Arnon Gilboa <agilboa@redhat.com> --------- Signed-off-by: Ido Aharon <iaharon@redhat.com> Signed-off-by: Michael Henriksen <mhenriks@redhat.com> Signed-off-by: Alvaro Romero <alromero@redhat.com> Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> Signed-off-by: Alexander Wels <awels@redhat.com> Signed-off-by: Shelly Kagan <skagan@redhat.com> Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com> Signed-off-by: Arnon Gilboa <agilboa@redhat.com> Co-authored-by: Ido Aharon <iaharon@redhat.com> Co-authored-by: Michael Henriksen <mhenriks@redhat.com> Co-authored-by: alromeros <alromero@redhat.com> Co-authored-by: akalenyu <akalenyu@redhat.com> Co-authored-by: Shelly Kagan <skagan@redhat.com> Co-authored-by: kubevirt-bot <kubevirtbot@redhat.com> Co-authored-by: Arnon Gilboa <agilboa@redhat.com>

kubevirt-bot requested review from aglitke and mhenriks May 21, 2023 12:32

kubevirt-bot added the size/L label May 21, 2023

ShellyKa13 force-pushed the data-volume-populators-integration branch from 04bba74 to 6338aff Compare May 22, 2023 16:25

alromeros reviewed May 22, 2023

View reviewed changes

mhenriks reviewed May 23, 2023

View reviewed changes

kubevirt-bot added size/XL and removed size/L labels May 23, 2023

mhenriks reviewed May 24, 2023

View reviewed changes

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2023

ShellyKa13 force-pushed the data-volume-populators-integration branch from ed545a2 to 87994ea Compare May 24, 2023 15:23

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2023

mhenriks reviewed May 25, 2023

View reviewed changes

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2023

ShellyKa13 force-pushed the data-volume-populators-integration branch from 87994ea to a98ea49 Compare May 28, 2023 16:01

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2023

ShellyKa13 force-pushed the data-volume-populators-integration branch from a98ea49 to 30ffb3b Compare May 29, 2023 14:00

kubevirt-bot added size/XXL and removed size/XL labels May 29, 2023

alromeros mentioned this pull request May 30, 2023

Add CDI non-CSI lane kubevirt/project-infra#2795

Merged

mhenriks reviewed May 31, 2023

View reviewed changes

ShellyKa13 force-pushed the data-volume-populators-integration branch from 30ffb3b to 2a16983 Compare May 31, 2023 14:30

mhenriks reviewed May 31, 2023

View reviewed changes

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2023

mhenriks reviewed Jun 6, 2023

View reviewed changes

ShellyKa13 force-pushed the data-volume-populators-integration branch from 6838e8b to 7776b9d Compare June 6, 2023 13:38

ShellyKa13 added 8 commits June 12, 2023 17:59

Prevent requeue upload dv when failing to find progress report pod

2df98bc

Signed-off-by: Shelly Kagan <skagan@redhat.com>

Adjust functional tests to handle dvs using populators

dfb3fd8

Signed-off-by: Shelly Kagan <skagan@redhat.com>

Fix clone test

a3aac62

Signed-off-by: Shelly Kagan <skagan@redhat.com>

add shouldUpdateProgress variable to know if need to update progress

1ae48ef

Signed-off-by: Shelly Kagan <skagan@redhat.com>

Change update of annotation from denied list to allowed list

6690f5e

Instead if checking if the annotation on pvcPrime is not desired go over desired list and if the annotation exists add it. Signed-off-by: Shelly Kagan <skagan@redhat.com>

fix removing annotations from pv when rebinding

5beb9b8

Signed-off-by: Shelly Kagan <skagan@redhat.com>

More fixes and UT

beeff5a

Signed-off-by: Shelly Kagan <skagan@redhat.com>

ShellyKa13 force-pushed the data-volume-populators-integration branch from ce5395d to ffb6804 Compare June 12, 2023 14:59

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 12, 2023

kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 12, 2023

ShellyKa13 force-pushed the data-volume-populators-integration branch from ffb6804 to 8a70f17 Compare June 12, 2023 15:15

alromeros reviewed Jun 12, 2023

View reviewed changes

kubevirt-bot assigned alromeros Jun 12, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 12, 2023

mhenriks mentioned this pull request Jun 13, 2023

DataVolume Controller uses VolumeCloneSource Populator #2750

Merged

a bit more updates and UTs

8e0ad95

Signed-off-by: Shelly Kagan <skagan@redhat.com>

ShellyKa13 force-pushed the data-volume-populators-integration branch from 8a70f17 to 8e0ad95 Compare June 13, 2023 11:46

kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023

kubevirt-bot assigned arnongilboa Jun 13, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 13, 2023

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 14, 2023

kubevirt-bot merged commit 5f85c42 into kubevirt:main Jun 14, 2023
16 checks passed

Integration of Data volume using CDI populators #2722

Integration of Data volume using CDI populators #2722

Conversation

ShellyKa13 commented May 21, 2023

alromeros left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhenriks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShellyKa13 May 24, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShellyKa13 Jun 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShellyKa13 commented Jun 12, 2023

alromeros left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnongilboa commented Jun 13, 2023

mhenriks commented Jun 14, 2023

mhenriks commented Jun 14, 2023

kubevirt-bot commented Jun 14, 2023

ShellyKa13 May 24, 2023 •

edited

ShellyKa13 Jun 6, 2023 •

edited