From 200af10f6f6097842d2e8ebf72265280a90c2137 Mon Sep 17 00:00:00 2001 From: Jakob Karge Date: Thu, 5 Sep 2024 15:55:24 +0200 Subject: [PATCH] vmware: Encrypt vmotion by default & Let flavor set it Vmotions should be encrypted, but not for really large RAM sizes because encryption impacts vmotion performance too much. Let the flavor define one of "required", "disabled" or "opportunistic". The default in our VMware configuration is "opportunistic", which means "use encryption for the vmotion if both source and target support it", which should be all of our landscape. We default to "required" explicitly now in Nova for every new & resized general- purpose-flavored VM. If the flavor sets an invalid value, the default will be used and a warning logged. Change-Id: Iba0fd2f2edc8fae210f6c4bab0218703e87ecfbc --- nova/tests/unit/virt/vmwareapi/test_vm_util.py | 12 ++++++++++++ nova/tests/unit/virt/vmwareapi/test_vmops.py | 16 ++++++++++++++++ nova/virt/vmwareapi/vm_util.py | 8 +++++++- nova/virt/vmwareapi/vmops.py | 9 +++++++++ 4 files changed, 44 insertions(+), 1 deletion(-) diff --git a/nova/tests/unit/virt/vmwareapi/test_vm_util.py b/nova/tests/unit/virt/vmwareapi/test_vm_util.py index aff040c533f..0748eba9c71 100644 --- a/nova/tests/unit/virt/vmwareapi/test_vm_util.py +++ b/nova/tests/unit/virt/vmwareapi/test_vm_util.py @@ -426,6 +426,7 @@ def test_get_resize_spec(self): expected.memoryAllocation = memoryAllocation expected.extraConfig = [] expected.memoryReservationLockedToMax = False + expected.migrateEncryption = None self.assertEqual(expected, result) @@ -459,6 +460,7 @@ def test_get_resize_spec_with_limits(self): expected.memoryAllocation = memoryAllocation expected.extraConfig = [] expected.memoryReservationLockedToMax = False + expected.migrateEncryption = None self.assertEqual(expected, result) @@ -883,6 +885,7 @@ def _create_vm_config_spec(self): spec.name = self._instance.uuid spec.instanceUuid = self._instance.uuid spec.deviceChange = [] + spec.migrateEncryption = None spec.numCPUs = 2 spec.version = None @@ -963,6 +966,7 @@ def test_get_vm_create_spec_with_allocations(self): extra_specs) expected = fake_factory.create('ns0:VirtualMachineConfigSpec') expected.deviceChange = [] + expected.migrateEncryption = None expected.guestId = constants.DEFAULT_OS_TYPE expected.instanceUuid = self._instance.uuid expected.memoryMB = self._instance.memory_mb @@ -1018,6 +1022,7 @@ def test_get_vm_create_spec_with_limit(self): expected.instanceUuid = self._instance.uuid expected.name = self._instance.uuid expected.deviceChange = [] + expected.migrateEncryption = None expected.extraConfig = [] extra_config = fake_factory.create("ns0:OptionValue") @@ -1072,6 +1077,7 @@ def test_get_vm_create_spec_with_share(self): expected.instanceUuid = self._instance.uuid expected.name = self._instance.uuid expected.deviceChange = [] + expected.migrateEncryption = None expected.extraConfig = [] extra_config = fake_factory.create('ns0:OptionValue') @@ -1125,6 +1131,7 @@ def test_get_vm_create_spec_with_share_custom(self): expected.instanceUuid = self._instance.uuid expected.name = self._instance.uuid expected.deviceChange = [] + expected.migrateEncryption = None expected.extraConfig = [] extra_config = fake_factory.create('ns0:OptionValue') @@ -1173,6 +1180,7 @@ def test_get_vm_create_spec_with_metadata(self): expected.name = self._instance.uuid expected.instanceUuid = self._instance.uuid expected.deviceChange = [] + expected.migrateEncryption = None expected.numCPUs = 2 expected.version = None @@ -1216,6 +1224,7 @@ def test_get_vm_create_spec_with_firmware(self): expected.name = self._instance.uuid expected.instanceUuid = self._instance.uuid expected.deviceChange = [] + expected.migrateEncryption = None expected.numCPUs = 2 expected.version = None @@ -2058,6 +2067,7 @@ def test_get_vm_create_spec_with_console_delay(self): expected.name = self._instance.uuid expected.instanceUuid = self._instance.uuid expected.deviceChange = [] + expected.migrateEncryption = None expected.numCPUs = 2 expected.version = None @@ -2102,6 +2112,7 @@ def test_get_vm_create_spec_with_cores_per_socket(self): extra_specs) expected = fake_factory.create('ns0:VirtualMachineConfigSpec') expected.deviceChange = [] + expected.migrateEncryption = None expected.guestId = 'otherGuest' expected.instanceUuid = self._instance.uuid expected.memoryMB = self._instance.memory_mb @@ -2147,6 +2158,7 @@ def test_get_vm_create_spec_with_memory_allocations(self): extra_specs) expected = fake_factory.create('ns0:VirtualMachineConfigSpec') expected.deviceChange = [] + expected.migrateEncryption = None expected.guestId = 'otherGuest' expected.instanceUuid = self._instance.uuid expected.memoryMB = self._instance.memory_mb diff --git a/nova/tests/unit/virt/vmwareapi/test_vmops.py b/nova/tests/unit/virt/vmwareapi/test_vmops.py index 393c95f9834..a0a587e1ed5 100644 --- a/nova/tests/unit/virt/vmwareapi/test_vmops.py +++ b/nova/tests/unit/virt/vmwareapi/test_vmops.py @@ -1420,6 +1420,22 @@ def test_reserve_max_flavor_memory_for_memory_reserved_flavor(self): self.assertEqual(specs.memory_limits.reservation, self._instance.flavor.memory_mb) + def _test_vmotion_encryption(self, encryption, expected=None): + if encryption is not None: + self._instance.flavor.extra_specs.update({ + "vmware:vmotion_encryption": encryption}) + specs = self._vmops._get_extra_specs(self._instance.flavor, + self._image_meta) + self.assertEqual(specs.vmotion_encryption, expected) + + def test_vmotion_encryption_values(self): + self._test_vmotion_encryption("required", "required") + self._test_vmotion_encryption("opportunistic", "opportunistic") + self._test_vmotion_encryption("disabled", "disabled") + self._test_vmotion_encryption("INVALID", "required") + # no extra_specs entry at all: + self._test_vmotion_encryption(None, "required") + @mock.patch.object(vmops.VMwareVMOps, '_extend_virtual_disk') @mock.patch.object(vmops.VMwareVMOps, '_get_extra_specs') @mock.patch.object(ds_util, 'disk_move') diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py index d2000a252ea..08d4ee6219a 100644 --- a/nova/virt/vmwareapi/vm_util.py +++ b/nova/virt/vmwareapi/vm_util.py @@ -114,7 +114,8 @@ def __init__(self, cpu_limits=None, hw_version=None, vif_limits=None, hv_enabled=None, firmware=None, hw_video_ram=None, numa_prefer_ht=None, numa_vcpu_max_per_virtual_node=None, - migration_data_timeout=None, evc_mode_key=None): + migration_data_timeout=None, evc_mode_key=None, + vmotion_encryption=None): """ExtraSpecs object holds extra_specs for the instance.""" self.cpu_limits = cpu_limits or Limits() self.memory_limits = memory_limits or Limits() @@ -130,6 +131,7 @@ def __init__(self, cpu_limits=None, hw_version=None, self.numa_vcpu_max_per_virtual_node = numa_vcpu_max_per_virtual_node self.migration_data_timeout = migration_data_timeout self.evc_mode_key = evc_mode_key + self.vmotion_encryption = vmotion_encryption def get_capped_evc_mode(self, evc_modes, evc_mode_sort_keys, max_evc_mode_key): @@ -439,6 +441,8 @@ def get_vm_create_spec(client_factory, instance, data_store_name, config_spec.deviceChange = devices + config_spec.migrateEncryption = extra_specs.vmotion_encryption + # add vm-uuid and iface-id.x values for Neutron extra_config = [] opt = client_factory.create('ns0:OptionValue') @@ -632,6 +636,8 @@ def get_vm_resize_spec(client_factory, vcpus, memory_mb, extra_specs, resize_spec.extraConfig = extra_config + resize_spec.migrateEncryption = extra_specs.vmotion_encryption + if metadata: resize_spec.annotation = metadata return resize_spec diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py index 6978fa3244a..a1f9be4ac0a 100644 --- a/nova/virt/vmwareapi/vmops.py +++ b/nova/virt/vmwareapi/vmops.py @@ -530,6 +530,15 @@ def _get_extra_specs(self, flavor, image_meta=None): raise error_util.EvcModeDoesNotExist(evc_mode=evc_mode_key) extra_specs.evc_mode_key = evc_mode_key + vmotion_encryption = flavor.extra_specs.get( + "vmware:vmotion_encryption", "required") + if vmotion_encryption in ["disabled", "opportunistic", "required"]: + extra_specs.vmotion_encryption = vmotion_encryption + else: + LOG.warning("Invalid vmware:vmotion_encryption value: %s. " + "Using default value: required", vmotion_encryption) + extra_specs.vmotion_encryption = "required" + return extra_specs def _get_storage_policy(self, flavor):