fix(vdsnapshot): resolve fsfreeze race condition

[hardcoretime]

Description

Why do we need it, and what problem does it solve?

Sometimes, when the virtual disk snapshot controller creates a batch of snapshots with required consistency, a race condition in the freeze or unfreeze filesystem operation can cause an inconsistent snapshot. To prevent this, the KubeVirt virtual machine instance is now annotated with a "freeze" or "unfreeze" request, and the service checks that the type of this request matches the fsFreezeStatus. The fsFreezeStatus is considered trusted only if it matches the request type.

What is the expected result?

Snapshots with required consistency now execute without race conditions in the filesystem freeze process.

Checklist

• The code is covered by unit tests.
• e2e tests passed.
• Documentation updated according to the changes.
• Changes were tested in the Kubernetes cluster manually.

Changelog entries

section: vdsnapshot
type: fix
summary: "Snapshots with required consistency now execute without race conditions in the filesystem freeze process."

[Isteb4k]

Let’s add to the error which exact value was found in the annotation

[Isteb4k]

If the kvvmi is fetched from the cache once for all functions (in GetKubeVirtVirtualMachineInstance), then there’s no point in checking for Untrusted every time. You already checked that at the very beginning in SyncFSFreezeRequest on line 173.
At this stage of the handler’s processing, such an error should no longer occur. This means it’s an unexpected case that should be handled as an internal error (return reconcile.Result{}, err)

[Isteb4k]

Just if vdSnapshot.Status.Consistent == nil {. No need to overwrite *vdSnapshot.Status.Consistent if it has already been set.

[Isteb4k]

Let’s add a comment explaining why reconciliation is needed here and what problem it helps prevent

[Isteb4k]

The function should be removed. Instead of it, we should use unfreezeFilesystem.

[Isteb4k]

Terrible cyclomatic complexity

[Isteb4k]

This way is much simpler:

		if vdSnapshot.Status.Consistent == nil {
			if vm == nil || vm.Status.Phase == v1alpha2.MachineStopped || isFSFrozen {
				vdSnapshot.Status.Consistent = ptr.To(true)
				return reconcile.Result{RequeueAfter: 2 * time.Second}, nil
			}

			if vdSnapshot.Spec.RequiredConsistency {
				err = fmt.Errorf("virtual disk snapshot is not consistent because the virtual machine %s has not been stopped or its filesystem has not been frozen", vm.Name)
				setPhaseConditionToFailed(cb, &vdSnapshot.Status.Phase, err)
				return reconcile.Result{}, err
			}
		}

		err = h.unfreezeFilesystem(ctx, vdSnapshot.Name, vm, kvvmi)
		if err != nil {
			if k8serrors.IsConflict(err) {
				return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
			}
			return reconcile.Result{}, err
		}