Introduce a new annotation to identify failed over VMs

We need a new annotation for failed over VMs so we can skip some
webhook checks.  This is different from imported, or registered VMs
since we will need to relax additional webhook checks such as allowing
for network manipulation.

Remove the test failover annotation since that is no longer needed.

api/v1alpha3/virtualmachine_types.go

@@ -198,17 +198,25 @@ const (
 	// VMware vCenter (VC) that is managing this virtual machine.
 	ManagerID = GroupName + "/manager-id"
 
-	// RegisteredVMAnnotation on a VirtualMachine represents that a virtual machine has
-	// been registered using the RegisterVM API after a restore, or a fail-over operation by
-	// a vendor. The presence of this annotation is used to bypass some validation checks
-	// that are otherwise applicable to all VirtualMachine create/update requests.
-	RegisteredVMAnnotation = GroupName + "/registered-vm"
+	// RestoredVMAnnotation on a VirtualMachine represents that a virtual
+	// machine has been restored using the RegisterVM API, typically by a
+	// VADP based data protection vendor. The presence of this annotation is
+	// used to bypass some validation checks that are otherwise
+	// applicable to all VirtualMachine create/update requests.
+	RestoredVMAnnotation = GroupName + "/restored-vm"
 
 	// ImportedVMAnnotation on a VirtualMachine represents that a traditional virtual
 	// machine has been imported into Supervisor using the ImportVM API. The presence of this
 	// annotation is used to bypass some validation checks that are otherwise applicable
 	// to all VirtualMachine create/update requests.
 	ImportedVMAnnotation = GroupName + "/imported-vm"
+
+	// FailedOverVMAnnotation on a VirtualMachine resource represents that a virtual
+	// machine has been failed over from one site to the other, typically as part of a
+	// disaster recovery workflow.  The presence of this annotation is used to bypass
+	// some validation checks that are otherwise applicable to all VirtualMachine
+	// create/update requests.
+	FailedOverVMAnnotation = GroupName + "/failed-over-vm"
 )
 
 const (

pkg/backup/api/backup_types.go

@@ -108,20 +108,4 @@ const (
 	// specifying this key, backup/restore and/or disaster recovery solutions are
 	// responsible to register the VM with Supervisor.
 	DisableAutoRegistrationExtraConfigKey = "vmservice.virtualmachine.disableAutomaticRegistration"
-
-	// TestFailoverLabelKey label signifies that a VirtualMachine is going through
-	// a test recovery plan scenario. Typically, this involves failing over a VM
-	// to a dedicated temporary network so as to not exhaust, and cause IP address
-	// collisions with the production network. This label on a VirtualMachine
-	// custom resource allows vendors to change the VM's network interface to
-	// connect to the test network after the VM is registered post a failover.
-	//
-	// VM operator does not support mutable networks, so this label allows only
-	// select vendors (e.g., SRM) to support recovery plan test workflows while we
-	// build support for true network mutability.
-	//
-	// The value of this label does not matter. Its presence is considered a
-	// sufficient evidence to indicate that the virtual machine is going through
-	// test fail-over.
-	TestFailoverLabelKey = "virtualmachine." + GroupName + "/test-failover"
 )

pkg/util/annotations/helpers.go

@@ -9,8 +9,12 @@ import (
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
 )
 
-func HasRegisterVM(o metav1.Object) bool {
-	return hasAnnotation(o, vmopv1.RegisteredVMAnnotation)
+func HasRestoredVM(o metav1.Object) bool {
+	return hasAnnotation(o, vmopv1.RestoredVMAnnotation)
+}
+
+func HasFailOverVM(o metav1.Object) bool {
+	return hasAnnotation(o, vmopv1.FailedOverVMAnnotation)
 }
 
 func HasImportVM(o metav1.Object) bool {

pkg/util/annotations/helpers_test.go

@@ -48,20 +48,20 @@ var _ = DescribeTable(
 )
 
 var _ = DescribeTable(
-	"HasRegisterVM",
+	"HasRestoredVM",
 	func(in map[string]string, out bool) {
 		vm := &vmopv1.VirtualMachine{
 			ObjectMeta: metav1.ObjectMeta{
 				Annotations: in,
 			},
 		}
-		actual := annotations.HasRegisterVM(vm)
+		actual := annotations.HasRestoredVM(vm)
 		Expect(actual).To(Equal(out))
 	},
 	Entry("nil", nil, false),
 	Entry("not present", map[string]string{"foo": "bar"}, false),
-	Entry("present but empty ", map[string]string{vmopv1.RegisteredVMAnnotation: ""}, true),
-	Entry("present and not empty ", map[string]string{vmopv1.RegisteredVMAnnotation: "true"}, true),
+	Entry("present but empty ", map[string]string{vmopv1.RestoredVMAnnotation: ""}, true),
+	Entry("present and not empty ", map[string]string{vmopv1.RestoredVMAnnotation: "true"}, true),
 )
 
 var _ = DescribeTable(
@@ -80,3 +80,20 @@ var _ = DescribeTable(
 	Entry("present but empty ", map[string]string{vmopv1.ImportedVMAnnotation: ""}, true),
 	Entry("present and not empty ", map[string]string{vmopv1.ImportedVMAnnotation: "true"}, true),
 )
+
+var _ = DescribeTable(
+	"HasFailOverVM",
+	func(in map[string]string, out bool) {
+		vm := &vmopv1.VirtualMachine{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: in,
+			},
+		}
+		actual := annotations.HasFailOverVM(vm)
+		Expect(actual).To(Equal(out))
+	},
+	Entry("nil", nil, false),
+	Entry("not present", map[string]string{"foo": "bar"}, false),
+	Entry("present but empty ", map[string]string{vmopv1.FailedOverVMAnnotation: ""}, true),
+	Entry("present and not empty ", map[string]string{vmopv1.FailedOverVMAnnotation: "true"}, true),
+)

webhooks/virtualmachine/validation/virtualmachine_validator.go

@@ -33,7 +33,6 @@ import (
 
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha3"
 	"github.com/vmware-tanzu/vm-operator/api/v1alpha3/sysprep"
-	backupapi "github.com/vmware-tanzu/vm-operator/pkg/backup/api"
 	"github.com/vmware-tanzu/vm-operator/pkg/builder"
 	pkgcfg "github.com/vmware-tanzu/vm-operator/pkg/config"
 	"github.com/vmware-tanzu/vm-operator/pkg/constants"
@@ -154,13 +153,13 @@ func (v validator) ValidateDelete(*pkgctx.WebhookRequestContext) admission.Respo
 	return admission.Allowed("")
 }
 
-// Updates to VM's image are only allowed if it is a Registered VM (used for failover
-// in disaster recovery).
+// Updates to VM's image are only allowed if it is a failed over VM.
 func (v validator) validateImageOnUpdate(ctx *pkgctx.WebhookRequestContext, vm, oldVM *vmopv1.VirtualMachine) field.ErrorList {
 	var allErrs field.ErrorList
 
+	// Allow resetting of image if this is a failover operation.
 	if vmopv1util.IsImagelessVM(*vm) && pkgcfg.FromContext(ctx).Features.VMIncrementalRestore {
-		if annotations.HasRegisterVM(vm) {
+		if annotations.HasFailOverVM(vm) {
 			if !vmopv1util.ImageRefsEqual(vm.Spec.Image, oldVM.Spec.Image) {
 				if !ctx.IsPrivilegedAccount {
 					allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "image"), restrictedToPrivUsers))
@@ -402,8 +401,8 @@ func (v validator) validateImageOnCreate(ctx *pkgctx.WebhookRequestContext, vm *
 		(pkgcfg.FromContext(ctx).Features.VMImportNewNet ||
 			pkgcfg.FromContext(ctx).Features.VMIncrementalRestore):
 		// TODO: Simplify this once mobility operator starts creating VMs with correct annotation.
-		// Skip validations on images if it is a VM created using ImportVM, or RegisterVM.
-		if annotations.HasImportVM(vm) || annotations.HasRegisterVM(vm) {
+		// Skip validations on images if it is a VM that is imported, registered, or failed over.
+		if annotations.HasImportVM(vm) || annotations.HasRestoredVM(vm) || annotations.HasFailOverVM(vm) {
 			// Restrict creating imageless VM resources to privileged users.
 			if !ctx.IsPrivilegedAccount {
 				allErrs = append(allErrs, field.Forbidden(f, restrictedToPrivUsers))
@@ -1237,9 +1236,9 @@ func (v validator) validateImmutableNetwork(ctx *pkgctx.WebhookRequestContext, v
 		return append(allErrs, field.Forbidden(p.Child("interfaces"), "network interfaces cannot be added or removed"))
 	}
 
-	// Skip comparing interfaces if this is a test fail-over to allow vendors to
-	// connect network each interface to test network.
-	if _, ok := vm.Labels[backupapi.TestFailoverLabelKey]; ok {
+	// Skip comparing interfaces if this is a fail-over to allow vendors to
+	// connect network each interface to the test, or the production network.
+	if _, ok := vm.Annotations[vmopv1.FailedOverVMAnnotation]; ok {
 		return allErrs
 	}
 
@@ -1337,8 +1336,12 @@ func (v validator) validateAnnotation(ctx *pkgctx.WebhookRequestContext, vm, old
 		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.FirstBootDoneAnnotation), modifyAnnotationNotAllowedForNonAdmin))
 	}
 
-	if vm.Annotations[vmopv1.RegisteredVMAnnotation] != oldVM.Annotations[vmopv1.RegisteredVMAnnotation] {
-		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.RegisteredVMAnnotation), modifyAnnotationNotAllowedForNonAdmin))
+	if vm.Annotations[vmopv1.RestoredVMAnnotation] != oldVM.Annotations[vmopv1.RestoredVMAnnotation] {
+		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.RestoredVMAnnotation), modifyAnnotationNotAllowedForNonAdmin))
+	}
+
+	if vm.Annotations[vmopv1.FailedOverVMAnnotation] != oldVM.Annotations[vmopv1.FailedOverVMAnnotation] {
+		allErrs = append(allErrs, field.Forbidden(annotationPath.Key(vmopv1.FailedOverVMAnnotation), modifyAnnotationNotAllowedForNonAdmin))
 	}
 
 	if vm.Annotations[vmopv1.ImportedVMAnnotation] != oldVM.Annotations[vmopv1.ImportedVMAnnotation] {