Support DV garbage collection #8134

arnongilboa · 2022-07-20T13:13:36Z

Signed-off-by: Arnon Gilboa agilboa@redhat.com

What this PR does / why we need it:
CDI added support for garbage collection of completed DVs.
Here we add the KubeVirt support, and adapt tests accordingly, so in case of garbage collected DV it won't be referred.
See design here.

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:

Release note:

Support DataVolume garbage collection

mhenriks

Good start, couple questions

mhenriks · 2022-07-21T01:47:13Z

hack/cluster-deploy.sh

@@ -21,6 +21,9 @@ set -ex pipefail

 DOCKER_TAG=${DOCKER_TAG:-devel}
 KUBEVIRT_DEPLOY_CDI=${KUBEVIRT_DEPLOY_CDI:-true}
+##### DO NOT APPROVE #####
+# FIXME: default should be false, as we want to cover the existing no-GC flows
+CDI_DV_GC=${CDI_DV_GC:-0}


You planning a GC specific lane?

Already added an optional lane pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

mhenriks · 2022-07-21T01:54:44Z

pkg/controller/virtinformers.go

@@ -409,6 +409,9 @@ func GetVMIInformerIndexers() cache.Indexers {
 				if vol.PersistentVolumeClaim != nil {
 					pvcs = append(pvcs, fmt.Sprintf("%s/%s", vmi.Namespace, vol.PersistentVolumeClaim.ClaimName))
 				}
+				if vol.DataVolume != nil {
+					pvcs = append(pvcs, fmt.Sprintf("%s/%s", vmi.Namespace, vol.DataVolume.Name))
+				}


Can you explain the rationale for this? To me, it seems to muddy things a bit. Why not lookup in both indexes? We do that in a couple places, like here: https://github.com/kubevirt/kubevirt/blob/main/pkg/virt-controller/watch/vm.go#L1720-L1733

Seems like a good idea. I'll try and update.

mhenriks · 2022-07-21T01:55:59Z

pkg/virt-controller/watch/export/export.go

@@ -693,7 +693,8 @@ func (ctrl *VMExportController) isKubevirtContentType(pvc *corev1.PersistentVolu
 	if pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == corev1.PersistentVolumeBlock {
 		return true
 	}
-	isKubevirt := pvc.Annotations[annContentType] == string(cdiv1.DataVolumeKubeVirt)
+	contentType, ok := pvc.Annotations[annContentType]
+	isKubevirt := ok && (contentType == string(cdiv1.DataVolumeKubeVirt) || contentType == "")


In what cases was this change necessary?

KubeVirt was mistakenly (inconsistently with CDI) regarding a PVC with default (empty string) content type as non-KubeVirt (which means Archive, I guess).

isnt it also kubevirt if !ok? if we dont have the annotation at all?

@ShellyKa13 if the PVC doesn't have the ContentType annotation at all (unlike empty which defaults to KubeVirt), we cannot say it's KubeVirt content type as CDI did not set the annotation.

mhenriks · 2022-07-21T01:59:33Z

tests/datavolume_gc.go

+	. "kubevirt.io/kubevirt/tests/framework/matcher"
+)
+
+func DeleteDataVolume(dv *cdiv1.DataVolume) {


Pretty sure this is not the right place for these helper functions. Perhaps in libstorage?

Sure, I can move it to libstorage. @ShellyKa13 ack?

yeah Im not even sure we need a new file maybe all this functions can get into tests/libstorage/datavolume.go..
If you think it should be in a different file then tests/libstorage/datavolume_gc.go

mhenriks · 2022-07-21T02:02:11Z

pkg/virt-controller/watch/vm.go

+			// Don't create DV if PVC already exists
+			pvc, err := c.getPersistentVolumeClaimFromCache(vm.Namespace, template.Name)
+			if err == nil && pvc != nil {
+				populatedFor := pvc.Annotations["cdi.kubevirt.io/storage.populatedFor"]


Is this annotation added when DV is GC'd? Otherwise, it shouldn't always be there

No, it's already used in the restore for creating a DV when its populated PVC already exists.
@ShellyKa13 ?

yes in the restore in case of datavolumetemplate we update the populatedFor to be the name of the new dvrestore name

Not sure what is going on here is correct. I don't think "populatedFor" should have any effect here. It is used for more things than vm restore, btw.

As-is, it appears to me that any "populatedFor" DataVolumes will never be garbage collected. Or more precisely, they will always get recreated after they are garbage collected.

I think VM controller should simply only create the DV if the PVC does not exist.

That means that the restore controller has to create the DVs during restore, though.

As-is, it appears to me that any "populatedFor" DataVolumes will never be garbage collected. Or more precisely, they will always get recreated after they are garbage collected.

not sure I understand why they will always be recreated, actually this code exactly should make sure that they will not be recreated (if annotation exists continue).
But either way I do understand what you are saying regarding the datavolume should be created in the restore and I agree, I did wonder why we create the pvc but not the datavolume in the restore process.
The question is if we decided that the default behavior is as if gc is on then on the case of restore there is no need to create the data volumes at all since the pvcs are already created from the volumesnapshots.

I think the "right" thing to do is to have restore create the DV even if it is garbage collected right away. BUT this may be a tricky thing to do if GC is enabled. How do we track that a DataVolume was created successfully if it may be garbage collected right away? May have to persist in restore status. Basically we have to make sure that the restore code does not consistently keep creating DVs.

@mhenriks things get a bit complex with your "right" direction. I don't see why not keep restore consistent with the new behavior, so if PVC exists DV is not created. Afaik no KubeVirt code currently requires/refers the DV in the restore flow, and all CI lanes look green with this behavior for both GC enabled/disabled.

I really don't like the idea of mixing this compact PR with improvement of the restore DV creation. We sure won't duplicate the VM controller logic, so a quite heavy refactoring will be needed there.
However, I understand that for some reason (preparation for DV source = volume snapshot?) you don't want to get rid of the restore DV creation when PVC exist.
Let's consider this simple solution: when restore creates the PVC it is annotated with dvRequired, so when VM controller encounters PVC with that annotation, it creates the DV and removes the PVC annotation.
wdyt?

I just thought of an easy way to track whether restore controller created the DataVolume. Initially, the restore PVC has no owner. When DV is created, it will take ownership of the PVC. When the DV is garbage collected, the VM will own the PVC. So, restore process will only create the DV when it does not exist and the PVC has no owner. What do you think?

As you can probably guess, my preference is to not change existing behavior unless there is a very good reason.

Looks like a clean logic, so why not keep DV creation in the VM controller with this minor condition? of course, you can later move DV creation to restore, but not in this PR.

ShellyKa13 · 2022-07-21T07:03:53Z

pkg/virt-controller/watch/snapshot/snapshot.go

+		if err == nil {
+			return storageClassName, nil
+		}
+		pvcKey := cacheKeyFunc(namespace, volume.VolumeSource.DataVolume.Name)


this is already done at the end of getStorageClassNameForDV: https://github.com/arnongilboa/kubevirt/blob/9acc0d9206a3850a1daa86e4de41964032def920/pkg/virt-controller/watch/snapshot/snapshot.go#L852-L858

getStorageClassNameForDV returns error if DV is not in cache (e.g. GC), but I agree handling it there is better.

ShellyKa13 · 2022-07-21T10:17:22Z

pkg/virt-controller/watch/vm.go

+			// Don't create DV if PVC already exists
+			pvc, err := c.getPersistentVolumeClaimFromCache(vm.Namespace, template.Name)
+			if err == nil && pvc != nil {
+				populatedFor := pvc.Annotations["cdi.kubevirt.io/storage.populatedFor"]


yes in the restore in case of datavolumetemplate we update the populatedFor to be the name of the new dvrestore name

ShellyKa13 · 2022-07-21T10:22:21Z

tests/datavolume_gc.go

+	. "kubevirt.io/kubevirt/tests/framework/matcher"
+)
+
+func DeleteDataVolume(dv *cdiv1.DataVolume) {


yeah Im not even sure we need a new file maybe all this functions can get into tests/libstorage/datavolume.go..
If you think it should be in a different file then tests/libstorage/datavolume_gc.go

ShellyKa13 · 2022-07-21T10:38:27Z

tests/datavolume_gc.go

+
+	err = virtCli.CdiClient().CdiV1beta1().DataVolumes(dv.Namespace).Delete(context.Background(), dv.Name, metav1.DeleteOptions{})
+	if !IsDataVolumeGC(virtCli) {
+		Expect(err).To(BeNil())


some places added dv = nil, not sure if should be added here too

Adding that. Let's see what @mhenriks thinks about DeleteDataVolume(dv **v1beta1.DataVolume).

no dont change it to **, just do dv=nil were it was deleted just after calling this function

why not nil them all in the func? I don't see any risk

ShellyKa13 · 2022-07-21T10:42:59Z

tests/utils.go

@@ -106,6 +106,8 @@ const (
 	StartingVMInstance           = "Starting a VirtualMachineInstance"
 	WaitingVMInstanceStart       = "Waiting until the VirtualMachineInstance will start"
 	CouldNotFindComputeContainer = "could not find compute container for pod"
+	DeletingDataVolume           = "Deleting the DataVolume"
+	VerifyingGC                  = "Verifying DataVolume garbage collection"


this 2 doesnt seem to belong here, should be in the same file as the helper functions they are used in

akalenyu · 2022-07-21T14:16:42Z

pkg/virt-controller/watch/vm.go

@@ -479,6 +479,15 @@ func (c *VMController) handleDataVolumes(vm *virtv1.VirtualMachine, dataVolumes
 			}
 		}
 		if !exists {
+			// Don't create DV if PVC already exists
+			pvc, err := c.getPersistentVolumeClaimFromCache(vm.Namespace, template.Name)
+			if err == nil && pvc != nil {


requeue if err != nil?

Sure. Nice catch!

arnongilboa · 2022-07-24T20:32:46Z

/retest-required

arnongilboa · 2022-07-25T07:28:17Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

arnongilboa · 2022-07-25T10:01:37Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

akalenyu · 2022-07-25T10:23:01Z

/test ?

kubevirt-bot · 2022-07-25T10:23:04Z

@akalenyu: The following commands are available to trigger required jobs:

/test pull-kubevirt-apidocs
/test pull-kubevirt-build
/test pull-kubevirt-build-arm64
/test pull-kubevirt-check-unassigned-tests
/test pull-kubevirt-client-python
/test pull-kubevirt-e2e-k8s-1.22-operator
/test pull-kubevirt-e2e-k8s-1.22-sig-compute
/test pull-kubevirt-e2e-k8s-1.22-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.22-sig-monitoring
/test pull-kubevirt-e2e-k8s-1.22-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-storage
/test pull-kubevirt-e2e-k8s-1.23-operator
/test pull-kubevirt-e2e-k8s-1.23-operator-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-compute
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-cgroupsv2
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-network
/test pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-storage
/test pull-kubevirt-e2e-k8s-1.23-sig-storage-cgroupsv2
/test pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot
/test pull-kubevirt-e2e-k8s-1.24-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.24-operator
/test pull-kubevirt-e2e-k8s-1.24-sig-compute
/test pull-kubevirt-e2e-k8s-1.24-sig-network
/test pull-kubevirt-e2e-k8s-1.24-sig-storage
/test pull-kubevirt-e2e-kind-1.22-sriov
/test pull-kubevirt-e2e-kind-1.23-vgpu
/test pull-kubevirt-e2e-kind-1.23-vgpu-nonroot
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-generate
/test pull-kubevirt-manifests
/test pull-kubevirt-prom-rules-verify
/test pull-kubevirt-unit-test
/test pull-kubevirt-verify-go-mod
/test pull-kubevirtci-bump-kubevirt

The following commands are available to trigger optional jobs:

/test build-kubevirt-builder
/test pull-kubevirt-check-tests-for-flakes
/test pull-kubevirt-code-lint
/test pull-kubevirt-e2e-arm64
/test pull-kubevirt-e2e-k8s-1.22-sig-compute-realtime
/test pull-kubevirt-e2e-k8s-1.22-sig-performance
/test pull-kubevirt-e2e-k8s-1.23-single-node
/test pull-kubevirt-e2e-k8s-1.23-swap-enabled
/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc
/test pull-kubevirt-e2e-kind-1.22-sriov-nonroot
/test pull-kubevirt-fossa
/test pull-kubevirt-gosec
/test pull-kubevirt-goveralls
/test pull-kubevirt-verify-rpms

Use /test all to run the following jobs that were automatically triggered:

pull-kubevirt-apidocs
pull-kubevirt-build
pull-kubevirt-build-arm64
pull-kubevirt-check-tests-for-flakes
pull-kubevirt-check-unassigned-tests
pull-kubevirt-client-python
pull-kubevirt-code-lint
pull-kubevirt-e2e-k8s-1.22-operator
pull-kubevirt-e2e-k8s-1.22-sig-compute
pull-kubevirt-e2e-k8s-1.22-sig-compute-migrations
pull-kubevirt-e2e-k8s-1.22-sig-network
pull-kubevirt-e2e-k8s-1.22-sig-performance
pull-kubevirt-e2e-k8s-1.22-sig-storage
pull-kubevirt-e2e-k8s-1.23-operator
pull-kubevirt-e2e-k8s-1.23-operator-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-compute
pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations
pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-network
pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-storage
pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot
pull-kubevirt-e2e-k8s-1.24-ipv6-sig-network
pull-kubevirt-e2e-k8s-1.24-operator
pull-kubevirt-e2e-k8s-1.24-sig-compute
pull-kubevirt-e2e-k8s-1.24-sig-network
pull-kubevirt-e2e-k8s-1.24-sig-storage
pull-kubevirt-e2e-kind-1.22-sriov
pull-kubevirt-e2e-kind-1.23-vgpu
pull-kubevirt-e2e-kind-1.23-vgpu-nonroot
pull-kubevirt-e2e-windows2016
pull-kubevirt-fossa
pull-kubevirt-generate
pull-kubevirt-goveralls
pull-kubevirt-manifests
pull-kubevirt-prom-rules-verify
pull-kubevirt-unit-test
pull-kubevirt-verify-go-mod

In response to this:

/test ?

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.

akalenyu · 2022-07-25T10:23:49Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

akalenyu · 2022-07-25T10:26:11Z

/test pull-kubevirt-fossa

arnongilboa · 2022-07-25T13:09:59Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

arnongilboa · 2022-07-25T15:34:59Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

arnongilboa · 2022-07-25T15:35:27Z

/test pull-kubevirt-e2e-kind-1.23-vgpu-nonroot

mhenriks · 2022-08-10T21:19:25Z

/lgtm

mhenriks · 2022-08-10T21:19:42Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

kubevirt-commenter-bot · 2022-08-11T01:54:24Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

arnongilboa · 2022-08-11T03:16:42Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

arnongilboa · 2022-08-11T04:59:28Z

/retest

kubevirt-bot · 2022-08-11T06:23:02Z

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

/test pull-kubevirt-apidocs
/test pull-kubevirt-build
/test pull-kubevirt-build-arm64
/test pull-kubevirt-check-unassigned-tests
/test pull-kubevirt-client-python
/test pull-kubevirt-e2e-k8s-1.22-operator
/test pull-kubevirt-e2e-k8s-1.22-sig-compute
/test pull-kubevirt-e2e-k8s-1.22-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.22-sig-monitoring
/test pull-kubevirt-e2e-k8s-1.22-sig-network
/test pull-kubevirt-e2e-k8s-1.22-sig-storage
/test pull-kubevirt-e2e-k8s-1.23-operator
/test pull-kubevirt-e2e-k8s-1.23-operator-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-compute
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-cgroupsv2
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-network
/test pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot
/test pull-kubevirt-e2e-k8s-1.23-sig-storage
/test pull-kubevirt-e2e-k8s-1.23-sig-storage-cgroupsv2
/test pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot
/test pull-kubevirt-e2e-k8s-1.24-ipv6-sig-network
/test pull-kubevirt-e2e-k8s-1.24-operator
/test pull-kubevirt-e2e-k8s-1.24-sig-compute
/test pull-kubevirt-e2e-k8s-1.24-sig-network
/test pull-kubevirt-e2e-k8s-1.24-sig-storage
/test pull-kubevirt-e2e-kind-1.22-sriov
/test pull-kubevirt-e2e-kind-1.23-vgpu
/test pull-kubevirt-e2e-kind-1.23-vgpu-nonroot
/test pull-kubevirt-e2e-windows2016
/test pull-kubevirt-generate
/test pull-kubevirt-manifests
/test pull-kubevirt-prom-rules-verify
/test pull-kubevirt-unit-test
/test pull-kubevirt-verify-go-mod
/test pull-kubevirtci-bump-kubevirt

The following commands are available to trigger optional jobs:

/test build-kubevirt-builder
/test pull-kubevirt-check-tests-for-flakes
/test pull-kubevirt-code-lint
/test pull-kubevirt-e2e-arm64
/test pull-kubevirt-e2e-k8s-1.22-sig-compute-realtime
/test pull-kubevirt-e2e-k8s-1.22-sig-performance
/test pull-kubevirt-e2e-k8s-1.23-single-node
/test pull-kubevirt-e2e-k8s-1.23-swap-enabled
/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc
/test pull-kubevirt-e2e-kind-1.22-sriov-nonroot
/test pull-kubevirt-fossa
/test pull-kubevirt-gosec
/test pull-kubevirt-goveralls
/test pull-kubevirt-verify-rpms

Use /test all to run the following jobs that were automatically triggered:

pull-kubevirt-apidocs
pull-kubevirt-build
pull-kubevirt-build-arm64
pull-kubevirt-check-tests-for-flakes
pull-kubevirt-check-unassigned-tests
pull-kubevirt-client-python
pull-kubevirt-code-lint
pull-kubevirt-e2e-k8s-1.22-operator
pull-kubevirt-e2e-k8s-1.22-sig-compute
pull-kubevirt-e2e-k8s-1.22-sig-compute-migrations
pull-kubevirt-e2e-k8s-1.22-sig-network
pull-kubevirt-e2e-k8s-1.22-sig-performance
pull-kubevirt-e2e-k8s-1.22-sig-storage
pull-kubevirt-e2e-k8s-1.23-operator
pull-kubevirt-e2e-k8s-1.23-operator-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-compute
pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations
pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-compute-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-network
pull-kubevirt-e2e-k8s-1.23-sig-network-nonroot
pull-kubevirt-e2e-k8s-1.23-sig-storage
pull-kubevirt-e2e-k8s-1.23-sig-storage-nonroot
pull-kubevirt-e2e-k8s-1.24-ipv6-sig-network
pull-kubevirt-e2e-k8s-1.24-operator
pull-kubevirt-e2e-k8s-1.24-sig-compute
pull-kubevirt-e2e-k8s-1.24-sig-network
pull-kubevirt-e2e-k8s-1.24-sig-storage
pull-kubevirt-e2e-kind-1.22-sriov
pull-kubevirt-e2e-kind-1.23-vgpu
pull-kubevirt-e2e-kind-1.23-vgpu-nonroot
pull-kubevirt-e2e-windows2016
pull-kubevirt-fossa
pull-kubevirt-generate
pull-kubevirt-goveralls
pull-kubevirt-manifests
pull-kubevirt-prom-rules-verify
pull-kubevirt-unit-test
pull-kubevirt-verify-go-mod

In response to this:

/retest pull-kubevirt-e2e-k8s-1.22-sig-storage
/retest pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations

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.

arnongilboa · 2022-08-11T06:23:44Z

/test pull-kubevirt-e2e-k8s-1.22-sig-storage
/test pull-kubevirt-e2e-k8s-1.23-sig-compute-migrations

kubevirt-commenter-bot · 2022-08-11T10:58:05Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

arnongilboa · 2022-08-11T12:18:03Z

/test pull-kubevirt-e2e-kind-1.22-sriov

kubevirt-commenter-bot · 2022-08-11T16:58:48Z

/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

mhenriks · 2022-08-11T17:32:13Z

/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc

mhenriks · 2022-08-11T17:32:34Z

/hold

arnongilboa · 2022-08-11T21:40:36Z

@mhenriks @ShellyKa13 @awels @akalenyu
The failure in the dv-gc lane is due to a bug in CDI 1.50.0, which was fixed in 1.51.0.
So this PR is blocked by kubevirt/kubevirtci#827 - please get it in asap.

CDI added support for [garbage collection of completed DVs] (kubevirt/containerized-data-importer#2233). Here we add the KubeVirt support, and adapt tests accordingly, so in case of garbage collected DV it won't be referred. See design [here](https://github.com/kubevirt/community/blob/main/ design-proposals/garbage-collect-completed-dvs.md). Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

kubevirt-bot · 2022-08-13T20:06:09Z

@arnongilboa: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc	`6131e49`	link	false	`/test pull-kubevirt-e2e-k8s-1.24-sig-storage-dv-gc`
pull-kubevirt-fossa	`ef62cbf`	link	false	`/test pull-kubevirt-fossa`

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. I understand the commands that are listed here.

ShellyKa13 · 2022-08-15T06:43:36Z

/lgtm

ShellyKa13 · 2022-08-15T06:44:28Z

/unhold

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/L labels Jul 20, 2022

kubevirt-bot requested review from alicefr and kbidarkar July 20, 2022 13:14

arnongilboa force-pushed the dv_gc branch from fd84f58 to 2c0ecbb Compare July 20, 2022 13:14

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 20, 2022

mhenriks reviewed Jul 21, 2022

View reviewed changes

arnongilboa force-pushed the dv_gc branch 2 times, most recently from 9acc0d9 to e4fe7bf Compare July 21, 2022 10:33

ShellyKa13 reviewed Jul 21, 2022

View reviewed changes

arnongilboa force-pushed the dv_gc branch from 6371be9 to 9f8cba8 Compare July 21, 2022 13:49

akalenyu reviewed Jul 21, 2022

View reviewed changes

arnongilboa force-pushed the dv_gc branch 2 times, most recently from 285eb36 to 27797be Compare July 24, 2022 15:48

kubevirt-bot added size/XL and removed size/L labels Jul 24, 2022

arnongilboa force-pushed the dv_gc branch from 27797be to 5e2d824 Compare July 25, 2022 06:53

arnongilboa force-pushed the dv_gc branch from 5e2d824 to e8dd8ec Compare July 25, 2022 20:52

kubevirt-bot assigned mhenriks Aug 10, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2022

ShellyKa13 mentioned this pull request Aug 11, 2022

First phase of separating storage code to storage pkg #8040

Merged

kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 11, 2022

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2022

arnongilboa force-pushed the dv_gc branch from 6131e49 to ef62cbf Compare August 13, 2022 20:00

kubevirt-bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 13, 2022

kubevirt-bot assigned ShellyKa13 Aug 15, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 15, 2022

kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 15, 2022

kubevirt-bot merged commit 595051e into kubevirt:main Aug 15, 2022

kubevirt-bot mentioned this pull request Sep 14, 2022

Macvtap binding mode for pod network #7648

Open

Support DV garbage collection #8134

Support DV garbage collection #8134

Conversation

arnongilboa commented Jul 20, 2022 • edited

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

ShellyKa13 Jul 26, 2022 • 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

arnongilboa Jul 27, 2022 • 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

arnongilboa Jul 21, 2022 • 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

arnongilboa commented Jul 24, 2022

arnongilboa commented Jul 25, 2022

arnongilboa commented Jul 25, 2022

akalenyu commented Jul 25, 2022

kubevirt-bot commented Jul 25, 2022

akalenyu commented Jul 25, 2022

akalenyu commented Jul 25, 2022

arnongilboa commented Jul 25, 2022

arnongilboa commented Jul 25, 2022

arnongilboa commented Jul 25, 2022

mhenriks commented Aug 10, 2022

mhenriks commented Aug 10, 2022

kubevirt-commenter-bot commented Aug 11, 2022

arnongilboa commented Aug 11, 2022

arnongilboa commented Aug 11, 2022

kubevirt-bot commented Aug 11, 2022

arnongilboa commented Aug 11, 2022

kubevirt-commenter-bot commented Aug 11, 2022

arnongilboa commented Aug 11, 2022

kubevirt-commenter-bot commented Aug 11, 2022

mhenriks commented Aug 11, 2022

mhenriks commented Aug 11, 2022

arnongilboa commented Aug 11, 2022

kubevirt-bot commented Aug 13, 2022

ShellyKa13 commented Aug 15, 2022

ShellyKa13 commented Aug 15, 2022

arnongilboa commented Jul 20, 2022 •

edited

ShellyKa13 Jul 26, 2022 •

edited

arnongilboa Jul 27, 2022 •

edited

arnongilboa Jul 21, 2022 •

edited