MTV-1656 | Disable snapshot consolidation

Issue: When deleting the snapshot the users with slow storage could
fail to get data corruption due to the consolidation process. The
consolidation writes the snapshot overlay to the disk and if the user has
slow storage it can take a long time. However, the Forklift Controller does not
check if the consolidation is finished and creates a new snapshot. This new
snapshot could be created in the middle of the writing process and it would
not see the changes in the underlying image from the consolidation.

Wrong order
1. Delete snapshot
2. Create snapshot
3. Consolidate

Correct order
1. Delete snapshot
2. Consolidate
3. Create snapshot

Fix: Disable consolidation in the middle of migration and consolidate in
RemoveFinalSnapshot phase. This will make the warm migration faster as
we don't need to wait for the consolidation but at the same time we will
consolidate at the end of migration so we won't cause any issues on the
users env.

Ref: https://issues.redhat.com/browse/MTV-1656

Signed-off-by: Martin Necas <[email protected]>

pkg/controller/plan/adapter/base/doc.go

@@ -106,7 +106,7 @@ type Client interface {
 	// Create a snapshot of the source VM.
 	CreateSnapshot(vmRef ref.Ref, hostsFunc util.HostsFunc) (string, error)
 	// Remove a snapshot.
-	RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) error
+	RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) error
 	// Check if a snapshot is ready to transfer.
 	CheckSnapshotReady(vmRef ref.Ref, snapshot string) (ready bool, err error)
 	// Set DataVolume checkpoints.

pkg/controller/plan/adapter/ocp/client.go

@@ -40,7 +40,7 @@ func (r *Client) CreateSnapshot(vmRef ref.Ref, hostsFunc util.HostsFunc) (string
 }
 
 // Remove a VM snapshot. No-op for this provider.
-func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
+func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
 	return
 }
 

pkg/controller/plan/adapter/openstack/client.go

@@ -125,7 +125,7 @@ func (r *Client) SetCheckpoints(vmRef ref.Ref, precopies []planapi.Precopy, data
 }
 
 // Remove a VM snapshot. No-op for this provider.
-func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
+func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
 	return
 }
 

pkg/controller/plan/adapter/ova/client.go

@@ -51,7 +51,7 @@ func (r *Client) CreateSnapshot(vmRef ref.Ref, hostsFunc util.HostsFunc) (snapsh
 }
 
 // Remove a VM snapshot. No-op for this provider.
-func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
+func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
 	return
 }
 

pkg/controller/plan/adapter/ovirt/client.go

@@ -105,7 +105,7 @@ func (r *Client) CheckSnapshotReady(vmRef ref.Ref, snapshot string) (ready bool,
 }
 
 // Remove a VM snapshot. No-op for this provider.
-func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc) (err error) {
+func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hostsFunc util.HostsFunc, consolidate bool) (err error) {
 	return
 }
 

pkg/controller/plan/adapter/vsphere/client.go

@@ -67,11 +67,11 @@ func (r *Client) CheckSnapshotReady(vmRef ref.Ref, snapshot string) (ready bool,
 }
 
 // Remove a VM snapshot.
-func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hosts util.HostsFunc) (err error) {
+func (r *Client) RemoveSnapshot(vmRef ref.Ref, snapshot string, hosts util.HostsFunc, consolidate bool) (err error) {
 	r.Log.V(1).Info("RemoveSnapshot",
 		"vmRef", vmRef,
 		"snapshot", snapshot)
-	err = r.removeSnapshot(vmRef, snapshot, false, hosts)
+	err = r.removeSnapshot(vmRef, snapshot, false, hosts, consolidate)
 	return
 }
 
@@ -371,7 +371,7 @@ func nullableHosts() (hosts map[string]*v1beta1.Host, err error) {
 }
 
 // Remove a VM snapshot and optionally its children.
-func (r *Client) removeSnapshot(vmRef ref.Ref, snapshot string, children bool, hosts util.HostsFunc) (err error) {
+func (r *Client) removeSnapshot(vmRef ref.Ref, snapshot string, children bool, hosts util.HostsFunc, consolidate bool) (err error) {
 	r.Log.Info("Removing snapshot",
 		"vmRef", vmRef,
 		"snapshot", snapshot,
@@ -381,7 +381,7 @@ func (r *Client) removeSnapshot(vmRef ref.Ref, snapshot string, children bool, h
 	if err != nil {
 		return
 	}
-	_, err = vm.RemoveSnapshot(context.TODO(), snapshot, children, nil)
+	_, err = vm.RemoveSnapshot(context.TODO(), snapshot, children, &consolidate)
 	if err != nil {
 		err = liberr.Wrap(err)
 		return

pkg/controller/plan/migration.go

@@ -517,7 +517,7 @@ func (r *Migration) removeLastWarmSnapshot(vm *plan.VMStatus) {
 		return
 	}
 	snapshot := vm.Warm.Precopies[n-1].Snapshot
-	if err := r.provider.RemoveSnapshot(vm.Ref, snapshot, r.kubevirt.loadHosts); err != nil {
+	if err := r.provider.RemoveSnapshot(vm.Ref, snapshot, r.kubevirt.loadHosts, true); err != nil {
 		r.Log.Error(
 			err,
 			"Failed to clean up warm migration snapshots.",
@@ -995,7 +995,11 @@ func (r *Migration) execute(vm *plan.VMStatus) (err error) {
 			break
 		}
 		n := len(vm.Warm.Precopies)
-		err = r.provider.RemoveSnapshot(vm.Ref, vm.Warm.Precopies[n-1].Snapshot, r.kubevirt.loadHosts)
+		consolidate := false
+		if vm.Phase == RemoveFinalSnapshot {
+			consolidate = true
+		}
+		err = r.provider.RemoveSnapshot(vm.Ref, vm.Warm.Precopies[n-1].Snapshot, r.kubevirt.loadHosts, consolidate)
 		if err != nil {
 			step.AddError(err.Error())
 			err = nil