From 22abcaf9ca2d58271659c56432e27247525ef94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Szczepa=C5=84ski?= Date: Tue, 12 Jun 2018 15:52:32 +0200 Subject: [PATCH] Add tests for _can_delete_from_vnc Remove some code duplication for checking if VM can be deleted from VNC Change-Id: I8a86a3d19269bb5d3962811f6e8282902b6aed8a Partial-Bug: #1776434 --- cvm/clients.py | 14 ++------- cvm/services.py | 36 ++++++++++++----------- tests/test_services.py | 66 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 82 insertions(+), 34 deletions(-) diff --git a/cvm/clients.py b/cvm/clients.py index 5aaad2d..990708b 100644 --- a/cvm/clients.py +++ b/cvm/clients.py @@ -214,19 +214,9 @@ def __init__(self, vnc_cfg): self.id_perms.set_enable(True) def delete_vm(self, vnc_vm): - # TODO: Remove code duplication -- see VirtualMachineService._can_delete_from_vnc try: - existing_vm = self.read_vm(vnc_vm.uuid) - vrouter_uuid = next(pair.value - for pair in vnc_vm.get_annotations().key_value_pair - if pair.key == 'vrouter-uuid') - if any([pair.key == 'vrouter-uuid' and pair.value == vrouter_uuid - for pair in existing_vm.get_annotations().key_value_pair]): - self.vnc_lib.virtual_machine_delete(id=vnc_vm.uuid) - logger.info('Virtual Machine removed: %s', vnc_vm.name) - else: - logger.error('Virtual Machine %s is managed by vRouter %s and cannot be deleted from VNC.', - vnc_vm.name, vrouter_uuid) + self.vnc_lib.virtual_machine_delete(id=vnc_vm.uuid) + logger.info('Virtual Machine removed: %s', vnc_vm.name) except NoIdError: logger.error('Virtual Machine not found: %s', vnc_vm.name) diff --git a/cvm/services.py b/cvm/services.py index 3aae00a..b6ae98a 100644 --- a/cvm/services.py +++ b/cvm/services.py @@ -48,8 +48,7 @@ def _add_property_filter_for_vm(self, vm_model, filters): def sync_vms(self): self._get_vms_from_vmware() - # TODO: Find a way to determine which VMs are safe to delete - # self._delete_unused_vms_in_vnc() + self._delete_unused_vms_in_vnc() def _get_vms_from_vmware(self): vmware_vms = self._esxi_api_client.get_all_vms() @@ -60,30 +59,32 @@ def _delete_unused_vms_in_vnc(self): vnc_vms = self._vnc_api_client.get_all_vms() for vnc_vm in vnc_vms: vm_model = self._database.get_vm_model_by_uuid(vnc_vm.uuid) - if not vm_model: + if vm_model: + continue + if self._can_delete_from_vnc(vnc_vm): logger.info('Deleting %s from VNC', vnc_vm.name) - self._vnc_api_client.delete_vm(vnc_vm) + self._vnc_api_client.delete_vm(vnc_vm.uuid) def remove_vm(self, name): vm_model = self._database.get_vm_model_by_name(name) if not vm_model: return None - if self._can_delete_from_vnc(vm_model): + if self._can_delete_from_vnc(vm_model.vnc_vm): self._vnc_api_client.delete_vm(vm_model.vnc_vm) self._database.delete_vm_model(vm_model.uuid) vm_model.destroy_property_filter() return vm_model - def _can_delete_from_vnc(self, vm_model): - existing_vm = self._vnc_api_client.read_vm(vm_model.uuid) + def _can_delete_from_vnc(self, vnc_vm): + existing_vm = self._vnc_api_client.read_vm(vnc_vm.uuid) vrouter_uuid = next(pair.value - for pair in vm_model.vnc_vm.get_annotations().key_value_pair + for pair in vnc_vm.get_annotations().key_value_pair if pair.key == 'vrouter-uuid') if any([pair.key == 'vrouter-uuid' and pair.value == vrouter_uuid for pair in existing_vm.get_annotations().key_value_pair]): return True logger.error('Virtual Machine %s is managed by vRouter %s and cannot be deleted from VNC.', - vm_model.name, vrouter_uuid) + vnc_vm.name, vrouter_uuid) return False def set_tools_running_status(self, vmware_vm, value): @@ -128,8 +129,7 @@ def __init__(self, vcenter_api_client, vnc_api_client, vrouter_api_client, datab def sync_vmis(self): self._create_new_vmis() - # TODO: Find a way to determine which VMIs are safe to delete. - # self._delete_unused_vmis() + self._delete_unused_vmis() def _create_new_vmis(self): for vm_model in self._database.get_all_vm_models(): @@ -183,7 +183,9 @@ def _create_or_update(self, vmi_model): def _delete_unused_vmis(self): for vnc_vmi in self._vnc_api_client.get_vmis_by_project(self._project): vmi_model = self._database.get_vmi_model_by_uuid(vnc_vmi.get_uuid()) - if not vmi_model: + if vmi_model: + continue + if self._can_delete_from_vnc(vnc_vmi): logger.info('Deleting %s from VNC.', vnc_vmi.name) self._vnc_api_client.delete_vmi(vnc_vmi.get_uuid()) @@ -226,16 +228,16 @@ def _restore_vlan_id(self, vmi_model): self._vcenter_api_client.restore_vlan_id(vmi_model.vn_model.dvs_name, vmi_model.port_key) vmi_model.clear_vlan_id() - def _can_delete_from_vnc(self, vmi_model): - existing_vmi = self._vnc_api_client.read_vmi(vmi_model.uuid) + def _can_delete_from_vnc(self, vnc_vmi): + existing_vmi = self._vnc_api_client.read_vmi(vnc_vmi.uuid) vrouter_uuid = next(pair.value - for pair in vmi_model.vnc_vmi.get_annotations().key_value_pair + for pair in vnc_vmi.get_annotations().key_value_pair if pair.key == 'vrouter-uuid') if any([pair.key == 'vrouter-uuid' and pair.value == vrouter_uuid for pair in existing_vmi.get_annotations().key_value_pair]): return True logger.error('Virtual Machine Interface %s is managed by vRouter %s and cannot be deleted from VNC.', - vmi_model.vnc_vmi.display_name, vrouter_uuid) + vnc_vmi.display_name, vrouter_uuid) return False def _delete_vrouter_port(self, vmi_model): @@ -250,7 +252,7 @@ def remove_vmis_for_vm_model(self, vm_name): return vmi_models = self._database.get_vmi_models_by_vm_uuid(vm_model.uuid) for vmi_model in vmi_models: - if self._can_delete_from_vnc(vmi_model): + if self._can_delete_from_vnc(vmi_model.vnc_vmi): self._delete_from_vnc(vmi_model) self._restore_vlan_id(vmi_model) self._database.delete_vmi_model(vmi_model.uuid) diff --git a/tests/test_services.py b/tests/test_services.py index ba35e64..c761d9f 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -1,8 +1,9 @@ -from unittest import TestCase, skip +from unittest import TestCase from mock import Mock, patch from pyVmomi import vim # pylint: disable=no-name-in-module from vnc_api import vnc_api +from vnc_api.gen.resource_xsd import KeyValuePair, KeyValuePairs from cvm.clients import VNCAPIClient from cvm.constants import (VNC_ROOT_DOMAIN, VNC_VCENTER_DEFAULT_SG, @@ -97,14 +98,15 @@ def test_sync_no_vms(self): self.database.save.assert_not_called() self.vnc_client.update_vm.assert_not_called() - @skip('We need to figure out a way to determine which VMs are to be deleted') def test_delete_unused_vms(self): self.esxi_api_client.get_all_vms.return_value = [] vnc_vm = vnc_api.VirtualMachine('d376b6b4-943d-4599-862f-d852fd6ba425') vnc_vm.set_uuid('d376b6b4-943d-4599-862f-d852fd6ba425') self.vnc_client.get_all_vms.return_value = [vnc_vm] - self.vm_service.sync_vms() + with patch('cvm.services.VirtualMachineService._can_delete_from_vnc') as can_delete: + can_delete.return_value = True + self.vm_service.sync_vms() self.database.save.assert_not_called() self.vnc_client.delete_vm.assert_called_once_with('d376b6b4-943d-4599-862f-d852fd6ba425') @@ -269,11 +271,12 @@ def test_sync_no_vmis(self): self.assertEqual(0, len(self.database.get_all_vmi_models())) - @skip('We need to figure out a way to determine which VMIs are to be deleted') def test_sync_deletes_unused_vmis(self): self.vnc_client.get_vmis_by_project.return_value = [Mock()] - self.vmi_service.sync_vmis() + with patch('cvm.services.VirtualMachineInterfaceService._can_delete_from_vnc') as can_delete: + can_delete.return_value = True + self.vmi_service.sync_vmis() self.vnc_client.delete_vmi.assert_called_once() @@ -478,3 +481,56 @@ def test_read_no_ipam(self): [VNC_ROOT_DOMAIN, VNC_VCENTER_PROJECT, VNC_VCENTER_IPAM], ipam.fq_name ) + + +class TestCanDeleteFromVnc(TestCase): + def setUp(self): + self.vnc_api_client = Mock() + self.vm_service = VirtualMachineService(None, self.vnc_api_client, None) + self.vmi_service = VirtualMachineInterfaceService(None, self.vnc_api_client, None, None) + + def test_vnc_vm_true(self): + vnc_vm = Mock() + vnc_vm.get_annotations.return_value = KeyValuePairs( + [KeyValuePair('vrouter-uuid', 'vrouter_uuid_1')]) + self.vnc_api_client.read_vm.return_value = vnc_vm + + result = self.vm_service._can_delete_from_vnc(vnc_vm) + + self.assertTrue(result) + + def test_vnc_vm_false(self): + vnc_vm_1 = Mock() + vnc_vm_1.get_annotations.return_value = KeyValuePairs( + [KeyValuePair('vrouter-uuid', 'vrouter_uuid_1')]) + vnc_vm_2 = Mock() + vnc_vm_2.get_annotations.return_value = KeyValuePairs( + [KeyValuePair('vrouter-uuid', 'vrouter_uuid_2')]) + self.vnc_api_client.read_vm.return_value = vnc_vm_1 + + result = self.vm_service._can_delete_from_vnc(vnc_vm_2) + + self.assertFalse(result) + + def test_vnc_vmi_true(self): + vnc_vmi = Mock() + vnc_vmi.get_annotations.return_value = KeyValuePairs( + [KeyValuePair('vrouter-uuid', 'vrouter_uuid_1')]) + self.vnc_api_client.read_vmi.return_value = vnc_vmi + + result = self.vmi_service._can_delete_from_vnc(vnc_vmi) + + self.assertTrue(result) + + def test_vnc_vmi_false(self): + vnc_vmi_1 = Mock() + vnc_vmi_1.get_annotations.return_value = KeyValuePairs( + [KeyValuePair('vrouter-uuid', 'vrouter_uuid_1')]) + vnc_vmi_2 = Mock() + vnc_vmi_2.get_annotations.return_value = KeyValuePairs( + [KeyValuePair('vrouter-uuid', 'vrouter_uuid_2')]) + self.vnc_api_client.read_vmi.return_value = vnc_vmi_1 + + result = self.vmi_service._can_delete_from_vnc(vnc_vmi_2) + + self.assertFalse(result)