Clear MAC address by label

This commit allows to optionally clear the MAC address in VM and VMIs during restore. Currently, the way to go for modular backup/restore behavior in velero plugin is by using labels instead of annotations. Labels are easier to add to the restore/backup objects during creation as the velero client has a flag to include them. This commit also changes some const names to use PascalCase to match exported const naming standard. Signed-off-by: Eduardo Esteban <[email protected]> Signed-off-by: Alvaro Romero <[email protected]>
kubevirt · Dec 16, 2024 · 288ae44 · 288ae44
1 parent 8d5aefb
commit 288ae44
Show file tree

Hide file tree

Showing 8 changed files with 115 additions and 14 deletions.
diff --git a/pkg/plugin/vm_backup_item_action.go b/pkg/plugin/vm_backup_item_action.go
@@ -41,12 +41,6 @@ type VMBackupItemAction struct {
 	log logrus.FieldLogger
 }
 
-const (
-	// MetadataBackupLabel indicates that the object will be backed up for metadata purposes.
-	// This allows skipping restore and consistency-specific checks while ensuring the object is backed up.
-	MetadataBackupLabel = "velero.kubevirt.io/metadataBackup"
-)
-
 // NewVMBackupItemAction instantiates a VMBackupItemAction.
 func NewVMBackupItemAction(log logrus.FieldLogger) *VMBackupItemAction {
 	return &VMBackupItemAction{log: log}
@@ -89,7 +83,7 @@ func (p *VMBackupItemAction) Execute(item runtime.Unstructured, backup *v1.Backu
 
 	// we can skip all checks that ensure consistency
 	// if we just want to backup for metadata purposes
-	if !metav1.HasLabel(backup.ObjectMeta, MetadataBackupLabel) {
+	if !util.IsMetadataBackup(backup) {
 		skipVolume := func(volume kvcore.Volume) bool {
 			return volumeInDVTemplates(volume, vm)
 		}
@@ -157,7 +151,7 @@ var isVMIExcludedByLabel = func(vm *kvcore.VirtualMachine) (bool, error) {
 		return false, nil
 	}
 
-	label, ok := labels[util.VELERO_EXCLUDE_LABEL]
+	label, ok := labels[util.VeleroExcludeLabel]
 	return ok && label == "true", nil
 }
 

diff --git a/pkg/plugin/vm_restore_item_action.go b/pkg/plugin/vm_restore_item_action.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	kvcore "kubevirt.io/api/core/v1"
+	"kubevirt.io/kubevirt-velero-plugin/pkg/util"
 	"kubevirt.io/kubevirt-velero-plugin/pkg/util/kvgraph"
 )
 
@@ -63,6 +64,11 @@ func (p *VMRestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (
 		return nil, errors.WithStack(err)
 	}
 
+	if util.ShouldClearMacAddress(input.Restore) {
+		p.log.Info("Clear virtual machine MAC addresses")
+		util.ClearMacAddress(&vm.Spec.Template.Spec)
+	}
+
 	item, err := runtime.DefaultUnstructuredConverter.ToUnstructured(vm)
 	if err != nil {
 		return nil, errors.WithStack(err)

diff --git a/pkg/plugin/vm_restore_item_action_test.go b/pkg/plugin/vm_restore_item_action_test.go
@@ -5,7 +5,9 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -49,6 +51,15 @@ func TestVmRestoreExecute(t *testing.T) {
 				},
 			},
 		},
+		Restore: &velerov1.Restore{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-restore",
+				Namespace: "default",
+			},
+			Spec: velerov1.RestoreSpec{
+				IncludedNamespaces: []string{"default"},
+			},
+		},
 	}
 
 	logrus.SetLevel(logrus.InfoLevel)

diff --git a/pkg/plugin/vmi_backup_item_action.go b/pkg/plugin/vmi_backup_item_action.go
@@ -105,7 +105,7 @@ func (p *VMIBackupItemAction) Execute(item runtime.Unstructured, backup *v1.Back
 
 	if isVMIOwned(vmi) {
 		util.AddAnnotation(item, AnnIsOwned, "true")
-	} else if !metav1.HasLabel(backup.ObjectMeta, MetadataBackupLabel) {
+	} else if !util.IsMetadataBackup(backup) {
 		restore, err := util.RestorePossible(vmi.Spec.Volumes, backup, vmi.Namespace, func(volume kvcore.Volume) bool { return false }, p.log)
 		if err != nil {
 			return nil, nil, errors.WithStack(err)
@@ -156,7 +156,7 @@ var isVMExcludedByLabel = func(vmi *kvcore.VirtualMachineInstance) (bool, error)
 		return false, err
 	}
 
-	label, ok := vm.GetLabels()[util.VELERO_EXCLUDE_LABEL]
+	label, ok := vm.GetLabels()[util.VeleroExcludeLabel]
 	return ok && label == "true", nil
 }
 
@@ -174,6 +174,6 @@ func (p *VMIBackupItemAction) isPodExcludedByLabel(vmi *kvcore.VirtualMachineIns
 		return false, nil
 	}
 
-	label, ok := labels[util.VELERO_EXCLUDE_LABEL]
+	label, ok := labels[util.VeleroExcludeLabel]
 	return ok && label == "true", nil
 }
diff --git a/pkg/plugin/vmi_restore_item_action.go b/pkg/plugin/vmi_restore_item_action.go
@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	kvcore "kubevirt.io/api/core/v1"
 
+	"kubevirt.io/kubevirt-velero-plugin/pkg/util"
 	"kubevirt.io/kubevirt-velero-plugin/pkg/util/kvgraph"
 )
 
@@ -85,6 +86,11 @@ func (p *VMIRestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput)
 		return nil, err
 	}
 
+	if util.ShouldClearMacAddress(input.Restore) {
+		p.log.Info("Clear virtual instance machine MAC addresses")
+		util.ClearMacAddress(&vmi.Spec)
+	}
+
 	// Restricted labels must be cleared otherwise the VMI will be rejected.
 	// The restricted labels contain runtime information about the underlying KVM object.
 	labels := removeRestrictedLabels(vmi.GetLabels())

diff --git a/pkg/plugin/vmi_restore_item_action_test.go b/pkg/plugin/vmi_restore_item_action_test.go
@@ -5,8 +5,10 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -50,6 +52,15 @@ func TestVmiRestoreExecute(t *testing.T) {
 						},
 					},
 				},
+				Restore: &velerov1.Restore{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-restore",
+						Namespace: "default",
+					},
+					Spec: velerov1.RestoreSpec{
+						IncludedNamespaces: []string{"default"},
+					},
+				},
 			},
 			false,
 			map[string]string{},
@@ -75,6 +86,15 @@ func TestVmiRestoreExecute(t *testing.T) {
 						},
 					},
 				},
+				Restore: &velerov1.Restore{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-restore",
+						Namespace: "default",
+					},
+					Spec: velerov1.RestoreSpec{
+						IncludedNamespaces: []string{"default"},
+					},
+				},
 			},
 			false,
 			map[string]string{

diff --git a/pkg/util/util.go b/pkg/util/util.go
@@ -24,7 +24,17 @@ import (
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
 )
 
-const VELERO_EXCLUDE_LABEL = "velero.io/exclude-from-backup"
+const (
+	// MetadataBackupLabel indicates that the object will be backed up for metadata purposes.
+	// This allows skipping restore and consistency-specific checks while ensuring the object is backed up.
+	MetadataBackupLabel = "velero.kubevirt.io/metadataBackup"
+
+	// ClearMacAddressLabel indicates that the MAC address should be cleared as part of the restore workflow.
+	ClearMacAddressLabel = "velero.kubevirt.io/clear-mac-address"
+
+	// VeleroExcludeLabel is used to exclude an object from Velero backups.
+	VeleroExcludeLabel = "velero.io/exclude-from-backup"
+)
 
 func GetK8sClient() (*kubernetes.Clientset, error) {
 	loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
@@ -209,7 +219,7 @@ var IsDVExcludedByLabel = func(namespace, dvName string) (bool, error) {
 		return false, nil
 	}
 
-	label, ok := labels[VELERO_EXCLUDE_LABEL]
+	label, ok := labels[VeleroExcludeLabel]
 	return ok && label == "true", nil
 }
 
@@ -230,7 +240,7 @@ var IsPVCExcludedByLabel = func(namespace, pvcName string) (bool, error) {
 		return false, nil
 	}
 
-	label, ok := labels[VELERO_EXCLUDE_LABEL]
+	label, ok := labels[VeleroExcludeLabel]
 	return ok && label == "true", nil
 }
 
@@ -307,3 +317,17 @@ func getNamespaceAndNetworkName(vmiNamespace, fullNetworkName string) (string, s
 	}
 	return vmiNamespace, fullNetworkName
 }
+
+func ShouldClearMacAddress(restore *velerov1.Restore) bool {
+	return metav1.HasLabel(restore.ObjectMeta, ClearMacAddressLabel)
+}
+
+func IsMetadataBackup(backup *velerov1.Backup) bool {
+	return metav1.HasLabel(backup.ObjectMeta, MetadataBackupLabel)
+}
+
+func ClearMacAddress(vmiSpec *kvv1.VirtualMachineInstanceSpec) {
+	for i := 0; i < len(vmiSpec.Domain.Devices.Interfaces); i++ {
+		vmiSpec.Domain.Devices.Interfaces[i].MacAddress = ""
+	}
+}
diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go
@@ -8,6 +8,7 @@ import (
 	velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	v1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	kvcore "kubevirt.io/api/core/v1"
 )
@@ -416,3 +417,42 @@ func TestRestorePossible(t *testing.T) {
 		})
 	}
 }
+
+func TestIsMacAddressCleared(t *testing.T) {
+	testCases := []struct {
+		name     string
+		resource string
+		restore  velerov1.Restore
+		expected bool
+	}{
+		{"Clear MAC address should return false with no label",
+			"Restore",
+			velerov1.Restore{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{},
+				},
+			},
+			false,
+		},
+		{"Clear MAC address should return true with ClearMacAddressLabel label",
+			"Restore",
+			velerov1.Restore{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						ClearMacAddressLabel: "",
+					},
+				},
+			},
+			true,
+		},
+	}
+
+	logrus.SetLevel(logrus.ErrorLevel)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := ShouldClearMacAddress(&tc.restore)
+
+			assert.Equal(t, tc.expected, result)
+		})
+	}
+}