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

@@ -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)
 

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)

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)