Upload populator #2678

ShellyKa13 · 2023-03-30T15:07:02Z

What this PR does / why we need it:
The upload populator controller can be used
standalone without the need of datavolume.
It reconciles pvc with upload dataSourceRef
and uses populators API to populated the pvc
with an upload command.
The controller creates pvc' with upload
annotation. After the upload completes it
rebinds the pv to the original target pvc and
deletes pvc prime.
Eventually we get a bound PVC which is already
populated.

you can read more in jira card: https://issues.redhat.com/browse/CNV-27346

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:
Still missing more tests and UT

Release note:

Created upload populator controller

ShellyKa13 · 2023-03-30T15:10:49Z

tests/upload_test.go

Note: I moved the existing tests under new Contexts and adjusted the before and after a bit, you should focus your review more on the Context("Upload population"

alromeros

Very good job Shelly, I'll continue with the review next week since I don't have that much time now, just adding a comment to consider.

alromeros · 2023-03-30T15:24:58Z

pkg/controller/populators/upload-populator.go

+	return uploadPopulator, nil
+}
+
+func addUploadPopulatorWatches(mgr manager.Manager, c controller.Controller, log logr.Logger) error {


I think these watches and indexer could also be included in addCommonPopulatorsWatches, maybe just adding a typeToWatch argument as we do here https://github.com/kubevirt/containerized-data-importer/blob/main/pkg/controller/datavolume/clone-controller-base.go#L697.

mhenriks

Great start!

Let's really think about what to name the populator resources

mhenriks · 2023-04-02T01:16:00Z

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go

+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+// +kubebuilder:object:root=true
+// +kubebuilder:storageversion
+type UploadSource struct {


Naming is hard, but I think we can do better here, perhaps open the discussion more broadly?

mhenriks · 2023-04-02T01:16:42Z

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go

+
+// UploadSourceSpec defines specification for UploadSource
+type UploadSourceSpec struct {
+}


ContentType? kubevirt vs archive?

Maybe add a preallocation field?

mhenriks · 2023-04-02T01:32:11Z

pkg/uploadproxy/uploadproxy.go

+		return app.urlResolver(pvcNamespace, pvcPrimeName, path), nil
+	}
+
+	return app.urlResolver(pvcNamespace, pvcName, path), nil


Maybe it would be better if the proxy knows nothing of populator implementation details, rather maybe we can put a new annotation on the target pvc that has either 1) the equivalent of whatever app.urlResolver returns here or 2) just service name for pvc prime

2 may be more secure that way traffic can't be rerouted to another namespace

as far as I see it the uploadproxy has to get the pvc prime in order to check the annotation and pod phase on the pvc. hence it has to know the pvc prime name anyways. Im not sure its so bad that it will know the format of getting pvc' name for that. you are also not revealing anything.
You are relying here on function createUploadServiceNameFromPvcName which is in the logic of the upload controller so I dont see how different it is between relying on the pvcprimename function.
If you update an annotation on the target pvc you rely on the annotation name that the populator updates so seems pretty much the same for me without the need to do an update on the target PVC and reconcile again to create the pvc'.

I am pretty strongly against the upload proxy knowing anything about populator implementation details

what about checking that the pvc has all the relevant annotations for the upload? you need the pvcprime name for that to do get the pvcprime to check those
Also just making sure, you do need to make the differentiation between regular pvc and one with populator cause in the 2 case you need to watch for this special annotation you are talking about and it can be racy, i.e creating the targetpvc and doing the upload and the upload populator adding this special annotation on the pvc

mhenriks · 2023-04-02T01:39:47Z

pkg/controller/populators/upload-populator.go

+	}
+
+	if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &corev1.PersistentVolumeClaim{}, dataSourceRefField, func(obj client.Object) []string {
+		if dsr := obj.(*corev1.PersistentVolumeClaim).Spec.DataSourceRef; dsr != nil {


You should check that DataSourceRef.Kind is what this populator cares about

mhenriks · 2023-04-02T01:44:53Z

pkg/controller/populators/upload-populator.go

+
+	dataSourceRef := pvc.Spec.DataSourceRef
+	if dataSourceRef != nil {
+		if !IsPVCDataSourceRefKind(pvc, cdiv1.UploadSourceRef) || dataSourceRef.Name == "" {


If you do the watches right, Reconcile should only be called for the right kind

I think its better to be safe and make sure again.

sure, but I think it should not be required. I think it is entirely possible to only reconcile PVCs that reference the upload populator

mhenriks · 2023-04-02T01:58:15Z

pkg/controller/populators/populator-base.go

+	// Setup watches
+	if err := c.Watch(&source.Kind{Type: &corev1.PersistentVolumeClaim{}}, &handler.EnqueueRequestForObject{}); err != nil {
+		return err
+	}


Don't want to reconcile all PVCs, only ones that that refer to whatever populator we are interested in so this watch should queue those.

ALSO

We want to reconcile targets when prc primes are updated (so we know when population complete). So PVC primes should be owned buy the target and/or have an annotation that links them (unique annotation per populator). The watch function will queue the target when pvc prime is updated

regarding the reconciling all PVCs, I saw that that's how it is done in all the other controllers, they dont check for specific annotation so I thought it should be the same. I'm also up for improving that.

what about when we watch the pod or the PV, in there you only have information for PVC' how are you queuing the target PVC in this type of watches?

Should not care about Pods, also don't think that PVs are important either because the PVC will get updated when binding happens

so you say I can remove the watch for pods and pvs then? in that case I guess we can reconcile only the target PVC

I guess @mhenriks meant using something like getDataVolumeOp, so it will reconcile only the relevant PVC according to the controller (upload/import/clone...)

Ignore it ^^^, you do it much better.

mhenriks · 2023-04-02T02:05:20Z

pkg/controller/populators/upload-populator.go

+			// Ignore PVCs that aren't for this populator to handle
+			return reconcile.Result{}, nil
+		}
+		return r.reconcilePVCPrime(pvc)


it is cleaner to just reconcile target pvc and have the watch will trigger target pvc reconcile when pvc prime changes

what about when we reconcile pvc prime because of update to the upload pod? the upload pod has no information on the target PVC, or should we also add to it the information as done for the pvc prime?

Populator should not care about the upload pod. THe upload PVC (pvc prime) has all relevant status

akalenyu · 2023-04-02T09:05:51Z

pkg/operator/controller/controller_test.go

@@ -1700,6 +1700,7 @@ func createNotReadyEventValidationMap() map[string]bool {
 	match[normalCreateSuccess+" *v1.CustomResourceDefinition datasources.cdi.kubevirt.io"] = false
 	match[normalCreateSuccess+" *v1.CustomResourceDefinition dataimportcrons.cdi.kubevirt.io"] = false
 	match[normalCreateSuccess+" *v1.CustomResourceDefinition objecttransfers.cdi.kubevirt.io"] = false
+	match[normalCreateSuccess+" *v1.CustomResourceDefinition uploadsource.cdi.kubevirt.io"] = false


uploadsources?

akalenyu

The PR is great, just some suggestions following offline discussions
Mainly the bind.immediate annotation discussion

akalenyu · 2023-04-04T09:41:17Z

pkg/controller/populators/populator-base.go

+)
+
+// ReconcilerBase members
+type ReconcilerBase struct {


Maybe PopulatorReconcilerBase? Don't we need to embed ReconcilerBase here?

did you mean embed reconcile.Reconciler? I remember I did and there was an issue with that, I cant remember what. Why do you think we need to embed? we put the UploadPopulatorReconciler that we define in the NewUploadPopulator as the reconciler for the controller and it accepts it as an object that implements the reconciler interface. So the struct implements the interface without the embedment.

and regarding the naming since its in the populator package I dont see the need to add 'populator' prefix to the name

I mean if this reconciler builds up on the base DV reconciler, it may be better to embed the fields instead of copying them

I dont think it should embed the dv basereconiler

I see, the populators sit in a different package
Regarding embedding the reconcile interface, maybe @arnongilboa can advice why we did that

I think reconcile.Reconciler iface can be removed from ReconcilerBase. afair it was originally meant to make utest easier, but it's not needed anymore. See #2698.

akalenyu · 2023-04-04T09:41:46Z

tests/framework/pvc.go

@@ -101,6 +121,25 @@ func createConsumerPod(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {
 	utils.DeletePodNoGrace(f.K8sClient, executorPod, namespace)
 }

+func createConsumerPodForPouplationPVC(targetPvc *k8sv1.PersistentVolumeClaim, f *Framework) {


typo pouplation

akalenyu · 2023-04-04T09:50:43Z

pkg/controller/populators/upload-populator.go

+	return uploadSource, nil
+}
+
+func getContentType(uploadSource *cdiv1.UploadSource) cdiv1.DataVolumeContentType {


can reuse

containerized-data-importer/pkg/controller/datavolume/controller-base.go

Lines 1045 to 1050 in da82a97

func getContentType(dv *cdiv1.DataVolume) cdiv1.DataVolumeContentType {

if dv.Spec.ContentType == cdiv1.DataVolumeArchive {

return cdiv1.DataVolumeArchive

}

return cdiv1.DataVolumeKubeVirt

}

I'll unite them and also use the code for GetContentType(pvc)

akalenyu · 2023-04-04T10:05:52Z

pkg/controller/populators/populator-base.go

+	}
+	annotations := make(map[string]string)
+	annotations[annPopulationTargetPVC] = pvc.Name
+	annotations[cc.AnnImmediateBinding] = ""


So how come this is needed? Won't PVC' bind when the original binds?

@mhenriks @ShellyKa13 Should we support this WFFC escape hatch annotations with populators,
so people can force bind as easily as they do with DVs?
If we do go this path, it may be something than can sit in the population base so import populator could take advantage of this too

regarding the first question as you mentioned in other commit, its the other way around, the whole population process will start only when there is some consumer of the PVC that will add the selectedNode annotation, then we put the selectedNode also on the PVC' and we want it to start the upload- for that we add the immediate bind annotation. Once upload completed PVC' is bound, we rebind, and then targetPVC is bound and we delete PVC'.

Regarding using immedaitebound annotation on targetPVC:
I did test it locally it seems to work, do we want it I dont see a reason why not, but maybe @mhenriks you have a reason?

If we want to support immediate bind annotation on the target pvc OR a property in the CR would be good

akalenyu · 2023-04-04T10:12:42Z

tests/framework/pvc.go

@@ -42,6 +43,18 @@ func (f *Framework) CreateBoundPVCFromDefinition(def *k8sv1.PersistentVolumeClai
 	return pvc
 }

+// CreateBoundPopulationPVCFromDefinition is a wrapper around utils.CreatePVCFromDefinition that also force binds pvc on
+// on WaitForFirstConsumer storage class by executing f.ForceBindIfWaitForFirstConsumer(pvc)
+func (f *Framework) CreateBoundPopulationPVCFromDefinition(def *k8sv1.PersistentVolumeClaim) *k8sv1.PersistentVolumeClaim {


Just making sure I understand:
With populators, Bound means the population is done, so there is no way the original target PVC
will ever be bound before somebody curld the upload endpoint and that process completed successfully?

This was not a problem with Immediate binding storage classes because the whole ForceBindIfWaitForFirstConsumer logic was skipped?

exactly, also the name of this function is probably not accurate, ill change that.
But yet with populators the PVC is bound only after the population, hence you cant wait here for the consumer pod to be running, I create here the consumer pod just to get the selectedNode annotation to be able to do the population

mhenriks · 2023-04-04T20:02:02Z

pkg/controller/populators/populator-base.go

+	annotations := make(map[string]string)
+	annotations[cc.AnnImmediateBinding] = ""
+	if waitForFirstConsumer {
+		annotations[AnnSelectedNode] = pvc.Annotations[AnnSelectedNode]


Are we sure this works correctly? Does this annotation get looked at by the scheduler when creating a pod using that pvc? I figured it was the other way around (annotation added after scheduling)

My assumption was that we would have a special annotation on PVC' that the upload controller would look at and set spec.nodeName on the upload pod

arnongilboa · 2023-04-20T11:40:32Z

pkg/controller/populators/populator-base.go

+		ResourceVersion: targetPVC.ResourceVersion,
+	}
+	r.log.V(3).Info("Rebinding PV to target PVC", "PVC", targetPVC.Name)
+	if err := r.client.Update(context.TODO(), pv); err != nil {


Why don't we delete pvcPrime before we update the PV ClaimRef to targetPVC? are we sure it's not racy and PVC controller won't override it as pvcPrime.Spec.VolumeName still points the PV?

the volume name is updated according to the PV claimRef and not the other way around. Also if you delete the pvcPrime while it has the claimRef wouldn't the dynamically provisioned PV be deleted?
And also its whats done in k8s -> https://github.com/kubernetes-csi/lib-volume-populator/blob/master/populator-machinery/controller.go#L648-L673 :)

Cool, so I learned something new. I simply played with this flow here. I guess it was different in my case as I used PersistentVolumeReclaimRetain? I saw that if I don't delete the original PVC it will take the PV again.

arnongilboa · 2023-04-20T11:59:06Z

pkg/controller/populators/populator-base.go

+		matchingFields := client.MatchingFields{dataSourceRefField: indexKeyFunc(obj.GetNamespace(), obj.GetName())}
+		if err := mgr.GetClient().List(context.TODO(), &pvcs, matchingFields); err != nil {
+			log.Error(err, "Unable to list PVCs", "matchingFields", matchingFields)
+			return reqs


be consistent with the named returns here (either with reqs or without it)

arnongilboa · 2023-04-20T12:01:38Z

pkg/controller/populators/populator-base.go

+			return reqs
+		}
+		for _, pvc := range pvcs.Items {
+			reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: pvc.Namespace, Name: pvc.Name}})


use client.ObjectKeyFromObject(pvc) instead

arnongilboa · 2023-04-20T14:23:52Z

pkg/controller/populators/upload-populator.go

+				r.recorder.Eventf(pvc, corev1.EventTypeWarning, errCreatingPVCPrime, err.Error())
+				return reconcile.Result{}, err
+			}
+		} else {


better always return reconcile.Result{}, err in the if and get rid of the else

arnongilboa · 2023-04-20T14:29:11Z

pkg/controller/populators/upload-populator.go

+	// Wait for the bind controller to rebind the PV
+	if pvcPrime != nil {
+		if corev1.ClaimLost != pvcPrime.Status.Phase {
+			return reconcile.Result{RequeueAfter: 2 * time.Second}, err


You are already returning on all err != nil flows, so I guess you can return nil here?

ShellyKa13 · 2023-04-24T14:10:22Z

/hold
will wait until the import populator PR is merged and then work on rebasing and adjusting code.

The upload populator controller can be used standalone without the need of datavolume. It reconciles pvc with upload dataSourceRef and uses populators API to populated the pvc with an upload command. The controller creates pvc' with upload annotation. After the upload completes it rebinds the pv to the original target pvc and deletes pvc prime. Eventually we get a bound PVC which is already populated. Signed-off-by: Shelly Kagan <skagan@redhat.com>

In case of pvc with datasourceref to upload population we should create the url to the upload server with the pvc' name. Signed-off-by: Shelly Kagan <skagan@redhat.com>

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

ShellyKa13 · 2023-05-01T07:19:38Z

/retest pull-containerized-data-importer-e2e-destructive

kubevirt-bot · 2023-05-01T07:19:40Z

@ShellyKa13: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test pull-containerized-data-importer-e2e-ceph
/test pull-containerized-data-importer-e2e-destructive
/test pull-containerized-data-importer-e2e-hpp-latest
/test pull-containerized-data-importer-e2e-hpp-previous
/test pull-containerized-data-importer-e2e-istio
/test pull-containerized-data-importer-e2e-nfs
/test pull-containerized-data-importer-e2e-upg
/test pull-containerized-data-importer-fossa

The following commands are available to trigger optional jobs:

/test pull-cdi-apidocs
/test pull-cdi-generate-verify
/test pull-cdi-linter
/test pull-cdi-unit-test

Use /test all to run all jobs.

In response to this:

/retest pull-containerized-data-importer-e2e-destructive

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ShellyKa13 · 2023-05-01T07:20:21Z

/test pull-containerized-data-importer-e2e-destructive

arnongilboa · 2023-05-01T07:47:28Z

pkg/controller/populators/upload-populator.go

+
+func (r *UploadPopulatorReconciler) reconcileTargetPVC(pvc, pvcPrime *corev1.PersistentVolumeClaim) (reconcile.Result, error) {
+	updated, err := r.updatePVCPrimeNameAnnotation(pvc, pvcPrime.Name)
+	if updated || err != nil {


why do we need to wait until pvc is actually updated?

Same question

until its actually updated the upload cant start anyways, no really need to continue the reconcile loop

arnongilboa · 2023-05-01T11:47:29Z

/lgtm

mhenriks · 2023-05-01T18:55:43Z

/hold
/approve

let's make sure @alromeros takes another look

kubevirt-bot · 2023-05-01T18:55:52Z

[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

alromeros

Very good job! Just have some comments, most of them nitpicks. The only missing thing that may be important is the preallocation field in the CRD spec.

Maybe it's also a good idea to set AnnContentType in the target PVC somewhere in populator-base, as discussed with @mhenriks.

alromeros · 2023-05-02T12:03:26Z

pkg/controller/populators/upload-populator.go

+const (
+	uploadPopulatorName = "upload-populator"
+
+	// UploadFailed provides a const to indicate upload has failed


Typo: different name in the comment and const.

alromeros · 2023-05-02T12:10:12Z

pkg/controller/populators/populators_suite_test.go

 	"kubevirt.io/containerized-data-importer/tests/reporters"
 )

 func TestPopulators(t *testing.T) {
 	RegisterFailHandler(Fail)
-	RunSpecsWithDefaultAndCustomReporters(t, "Populators Suite", reporters.NewReporters())
+	RunSpecsWithDefaultAndCustomReporters(t, "Populators controllers Suite", reporters.NewReporters())


Supreme nitpick and not required at all, but I'd either use "Populators Suite" (since populators could be considered controllers) or Populator controllers Suite to avoid the double plural.

alromeros · 2023-05-02T12:53:12Z

pkg/controller/populators/upload-populator.go

+
+func (r *UploadPopulatorReconciler) reconcileTargetPVC(pvc, pvcPrime *corev1.PersistentVolumeClaim) (reconcile.Result, error) {
+	updated, err := r.updatePVCPrimeNameAnnotation(pvc, pvcPrime.Name)
+	if updated || err != nil {


Same question

alromeros · 2023-05-02T12:54:20Z

pkg/controller/populators/upload-populator_test.go

+		expectEvent(r, errUploadFailed)
+	})
+
+	It("should rebind PV to target PVC", func() {


Very good tests, nice job!

alromeros · 2023-05-02T13:07:33Z

pkg/uploadproxy/uploadproxy.go

 		if err != nil {
 			if k8serrors.IsNotFound(err) {
 				return false, fmt.Errorf("rejecting Upload Request for PVC %s that doesn't exist", pvcName)
 			}

 			return false, err
 		}
+		// If using upload populator then need to check upload possibility to the PVC'
+		if populators.IsPVCDataSourceRefKind(pvc, cdiv1.VolumeUploadSourceRef) {
+			pvcPrimeName, ok := pvc.Annotations[populators.AnnPVCPrimeName]


I would try to hide most of this pvcPrime-specific code in a separate function. I think it looks better to have additional functions doing populator stuff so you can skip them if you just want to follow the regular code flow. I'm also ok with the annotation name.

alromeros · 2023-05-02T13:08:15Z

staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1/types.go

+
+// UploadSourceSpec defines specification for UploadSource
+type UploadSourceSpec struct {
+}


Maybe add a preallocation field?

Also some other small fixes Signed-off-by: Shelly Kagan <skagan@redhat.com>

ShellyKa13 · 2023-05-03T09:06:06Z

@alromeros added preallocation and small fixes in the new commit. I don't want to add the content type to the target pvc in this commit, dont think its relevant

ShellyKa13 · 2023-05-03T09:22:57Z

/retest

alromeros · 2023-05-03T14:53:27Z

Good job!
/lgtm

ShellyKa13 · 2023-05-03T15:04:17Z

/unhold

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Mar 30, 2023

ShellyKa13 changed the title ~~Upload populator~~ [WIP] Upload populator Mar 30, 2023

kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL labels Mar 30, 2023

kubevirt-bot requested review from aglitke and awels March 30, 2023 15:07

ShellyKa13 commented Mar 30, 2023

View reviewed changes

alromeros reviewed Mar 30, 2023

View reviewed changes

mhenriks reviewed Apr 2, 2023

View reviewed changes

akalenyu reviewed Apr 2, 2023

View reviewed changes

ShellyKa13 force-pushed the upload-populator branch 2 times, most recently from d09ffc2 to da82a97 Compare April 3, 2023 12:01

akalenyu reviewed Apr 4, 2023

View reviewed changes

ShellyKa13 force-pushed the upload-populator branch 2 times, most recently from 7140e87 to 5df0fd5 Compare April 4, 2023 16:40

mhenriks reviewed Apr 4, 2023

View reviewed changes

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

arnongilboa reviewed Apr 20, 2023

View reviewed changes

ShellyKa13 force-pushed the upload-populator branch from 5df0fd5 to a7e7009 Compare April 20, 2023 14:49

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

ShellyKa13 force-pushed the upload-populator branch from a7e7009 to 90e2a4f Compare April 20, 2023 15:03

ShellyKa13 changed the title ~~[WIP] Upload populator~~ Upload populator Apr 20, 2023

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 20, 2023

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2023

ShellyKa13 added 4 commits April 30, 2023 18:39

Adjust upload-proxy to handle upload population

ff9f5cb

In case of pvc with datasourceref to upload population we should create the url to the upload server with the pvc' name. Signed-off-by: Shelly Kagan <skagan@redhat.com>

Add tests for upload population

9cedbb2

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

Add unit tests for upload populator

3496cf5

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

ShellyKa13 force-pushed the upload-populator branch from 08cdb3c to 3496cf5 Compare April 30, 2023 16:31

ShellyKa13 requested review from mhenriks, akalenyu, alromeros and arnongilboa May 1, 2023 07:19

arnongilboa reviewed May 1, 2023

View reviewed changes

kubevirt-bot assigned arnongilboa May 1, 2023

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

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

alromeros reviewed May 2, 2023

View reviewed changes

Add preallocation to volumeuploadsource crd

53c17ad

Also some other small fixes Signed-off-by: Shelly Kagan <skagan@redhat.com>

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

kubevirt-bot assigned alromeros May 3, 2023

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

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

kubevirt-bot merged commit e6c835c into kubevirt:main May 4, 2023
1 check passed

	func getContentType(dv *cdiv1.DataVolume) cdiv1.DataVolumeContentType {
	if dv.Spec.ContentType == cdiv1.DataVolumeArchive {
	return cdiv1.DataVolumeArchive
	}
	return cdiv1.DataVolumeKubeVirt
	}

Upload populator #2678

Upload populator #2678

Conversation

ShellyKa13 commented Mar 30, 2023 • edited Loading

Choose a reason for hiding this comment

alromeros left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akalenyu 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

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

mhenriks Apr 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnongilboa Apr 20, 2023 • edited Loading

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 Apr 24, 2023

ShellyKa13 commented May 1, 2023

kubevirt-bot commented May 1, 2023

ShellyKa13 commented May 1, 2023

arnongilboa May 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnongilboa commented May 1, 2023

mhenriks commented May 1, 2023

kubevirt-bot commented May 1, 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

Choose a reason for hiding this comment

ShellyKa13 commented May 3, 2023

ShellyKa13 commented Mar 30, 2023 •

edited

Loading

mhenriks Apr 4, 2023 •

edited

Loading

arnongilboa Apr 20, 2023 •

edited

Loading

arnongilboa May 1, 2023 •

edited

Loading