From 288ae447d3cb7a948be043003a799947d9a80968 Mon Sep 17 00:00:00 2001 From: Eduartdo Esteban Date: Mon, 25 Mar 2024 13:36:45 -0700 Subject: [PATCH] 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 Signed-off-by: Alvaro Romero --- pkg/plugin/vm_backup_item_action.go | 10 ++---- pkg/plugin/vm_restore_item_action.go | 6 ++++ pkg/plugin/vm_restore_item_action_test.go | 11 ++++++ pkg/plugin/vmi_backup_item_action.go | 6 ++-- pkg/plugin/vmi_restore_item_action.go | 6 ++++ pkg/plugin/vmi_restore_item_action_test.go | 20 +++++++++++ pkg/util/util.go | 30 ++++++++++++++-- pkg/util/util_test.go | 40 ++++++++++++++++++++++ 8 files changed, 115 insertions(+), 14 deletions(-) diff --git a/pkg/plugin/vm_backup_item_action.go b/pkg/plugin/vm_backup_item_action.go index 94642b23..7a7b8948 100644 --- 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 index 90bede72..24d601b6 100644 --- 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 index 87e2e12f..ba239d00 100644 --- 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 index f4fdc154..2a9deeb0 100644 --- 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 index 6eabe526..92cac216 100644 --- 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 index c8d8bdb3..89fc9e95 100644 --- 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 index 36152b12..079a200d 100644 --- 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 index 6b669b7b..9e7008d5 100644 --- 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) + }) + } +}