Merge "VMI sync refactor" into R5.0

cvm/controllers.py

@@ -69,8 +69,8 @@ def _handle_event(self, event):
     def _handle_vm_updated_event(self, event):
         vmware_vm = event.vm.vm
         try:
-            vm_model = self._vm_service.update(vmware_vm)
-            self._vmi_service.update_vmis_for_vm_model(vm_model)
+            self._vm_service.update(vmware_vm)
+            self._vmi_service.update_vmis()
             self._vrouter_port_service.sync_ports()
         except vmodl.fault.ManagedObjectNotFound:
             logger.info('Skipping event for a non-existent VM.')
@@ -141,9 +141,8 @@ def _handle_event(self, event):
             device = device_spec.device
             if isinstance(device, vim.vm.device.VirtualVmxnet3):
                 logger.info('Detected VmReconfiguredEvent with %s device', type(device))
-                mac_address = device.macAddress
-                self._vm_service.update_vm_models_interface(vmware_vm)
-                self._vmi_service.update_vmis_vn(vmware_vm, mac_address)
+                self._vm_service.update_vm_models_interfaces(vmware_vm)
+                self._vmi_service.update_vmis()
                 self._vrouter_port_service.sync_ports()
             else:
                 logger.info('Detected VmReconfiguredEvent with unsupported %s device', type(device))

cvm/database.py

@@ -13,6 +13,8 @@ def __init__(self):
         self.vm_models = {}
         self.vn_models = {}
         self.vmi_models = {}
+        self.vmis_to_update = []
+        self.vmis_to_delete = []
         self.ports_to_update = []
         self.ports_to_delete = []
 

cvm/models.py

@@ -84,6 +84,7 @@ def __init__(self, vmware_vm, vm_properties):
         self.vrouter_uuid = find_vrouter_uuid(vmware_vm.summary.runtime.host)
         self.property_filter = None
         self.ports = self._read_ports()
+        self.vmi_models = self._construct_interfaces()
         self._vnc_vm = None
 
     def update(self, vmware_vm, vm_properties):
@@ -98,6 +99,9 @@ def rename(self, name):
     def update_ports(self):
         self.ports = self._read_ports()
 
+    def update_vmis(self):
+        self.vmi_models = self._construct_interfaces()
+
     def _read_ports(self):
         try:
             return [VCenterPort(device)
@@ -107,6 +111,10 @@ def _read_ports(self):
             logger.error('Could not read ports for %s.', self.name)
         return None
 
+    def _construct_interfaces(self):
+        return [VirtualMachineInterfaceModel(self, None, port)
+                for port in self.ports]
+
     def destroy_property_filter(self):
         self.property_filter.DestroyPropertyFilter()
 

cvm/services.py

@@ -49,21 +49,24 @@ def update(self, vmware_vm):
         vm_properties = self._esxi_api_client.read_vm_properties(vmware_vm)
         vm_model = self._database.get_vm_model_by_uuid(vmware_vm.config.instanceUuid)
         if vm_model:
-            return self._update(vm_model, vmware_vm, vm_properties)
-        return self._create(vmware_vm, vm_properties)
+            self._update(vm_model, vmware_vm, vm_properties)
+            return
+        self._create(vmware_vm, vm_properties)
 
     def _update(self, vm_model, vmware_vm, vm_properties):
         vm_model.update(vmware_vm, vm_properties)
+        for vmi_model in vm_model.vmi_models:
+            self._database.vmis_to_update.append(vmi_model)
         self._database.save(vm_model)
-        return vm_model
 
     def _create(self, vmware_vm, vm_properties):
         vm_model = VirtualMachineModel(vmware_vm, vm_properties)
+        for vmi_model in vm_model.vmi_models:
+            self._database.vmis_to_update.append(vmi_model)
         self._add_property_filter_for_vm(vm_model, ['guest.toolsRunningStatus', 'guest.net'])
         if not vm_model.is_contrail_vm:
             self._vnc_api_client.update_or_create_vm(vm_model.vnc_vm)
         self._database.save(vm_model)
-        return vm_model
 
     def _add_property_filter_for_vm(self, vm_model, filters):
         property_filter = self._esxi_api_client.add_filter(vm_model.vmware_vm, filters)
@@ -108,9 +111,18 @@ def rename_vm(self, old_name, new_name):
         self._vnc_api_client.update_or_create_vm(vm_model.vnc_vm)
         self._database.save(vm_model)
 
-    def update_vm_models_interface(self, vmware_vm):
+    def update_vm_models_interfaces(self, vmware_vm):
         vm_model = self._database.get_vm_model_by_uuid(vmware_vm.config.instanceUuid)
+        old_vmi_models = {vmi_model.uuid: vmi_model for vmi_model in vm_model.vmi_models}
         vm_model.update_ports()
+        vm_model.update_vmis()
+        new_vmi_models = {vmi_model.uuid: vmi_model for vmi_model in vm_model.vmi_models}
+
+        for uuid, new_vmi_model in new_vmi_models.items():
+            old_vmi_models.pop(uuid, None)
+            self._database.vmis_to_update.append(new_vmi_model)
+
+        self._database.vmis_to_delete += old_vmi_models.values()
 
 
 class VirtualNetworkService(Service):
@@ -135,50 +147,35 @@ def __init__(self, vcenter_api_client, vnc_api_client, database, esxi_api_client
                                                              vcenter_api_client=vcenter_api_client)
 
     def sync_vmis(self):
-        self._create_new_vmis()
+        self.update_vmis()
         self._delete_unused_vmis()
 
-    def _create_new_vmis(self):
-        for vm_model in self._database.get_all_vm_models():
-            self.update_vmis_for_vm_model(vm_model)
-
-    def update_vmis_for_vm_model(self, vm_model):
-        if vm_model.is_contrail_vm:
-            return
-
-        existing_vmi_models = {vmi_model.vcenter_port.mac_address: vmi_model
-                               for vmi_model in self._database.get_vmi_models_by_vm_uuid(vm_model.uuid)}
-        for vcenter_port in vm_model.ports:
-            existing_vmi_models.pop(vcenter_port.mac_address, None)
-            self.update_vmis_vn(vm_model.vmware_vm, vcenter_port.mac_address)
-
-        for unused_vmi_model in existing_vmi_models.values():
-            self._delete(unused_vmi_model)
+    def update_vmis(self):
+        vmis_to_update = [vmi_model for vmi_model in self._database.vmis_to_update]
+        for vmi_model in vmis_to_update:
+            if vmi_model.vm_model.is_contrail_vm:
+                continue
+            self.update_vmis_vn(vmi_model)
+            self._database.vmis_to_update.remove(vmi_model)
 
-    def update_vmis_vn(self, vmware_vm, mac_address):
-        vm_model = self._database.get_vm_model_by_uuid(vmware_vm.config.instanceUuid)
-        vcenter_port = next(port for port in vm_model.ports if port.mac_address == mac_address)
-        vmi_model = self._database.get_vmi_model_by_uuid(VirtualMachineInterfaceModel.get_uuid(mac_address))
-        vn_model = self._database.get_vn_model_by_key(vcenter_port.portgroup_key)
-        if not vmi_model:
-            vmi_model = VirtualMachineInterfaceModel(vm_model, vn_model, vcenter_port)
-            vmi_model.parent = self._project
-            vmi_model.security_group = self._default_security_group
-        if vmi_model.vn_model != vn_model:
-            with self._vcenter_api_client:
-                self._vcenter_api_client.restore_vlan_id(vmi_model.vcenter_port)
-            vmi_model.clear_vlan_id()
-            vmi_model.vn_model = vn_model
-            vmi_model.vcenter_port = vcenter_port
+        for vmi_model in self._database.vmis_to_delete:
             self._delete(vmi_model)
-        self._create_or_update(vmi_model)
+
+    def update_vmis_vn(self, new_vmi_model):
+        old_vmi_model = self._database.get_vmi_model_by_uuid(new_vmi_model.uuid)
+        new_vmi_model.vn_model = self._database.get_vn_model_by_key(new_vmi_model.vcenter_port.portgroup_key)
+        if old_vmi_model and old_vmi_model.vn_model != new_vmi_model.vn_model:
+            self._delete(old_vmi_model)
+        self._create_or_update(new_vmi_model)
 
     def _create_or_update(self, vmi_model):
         with self._vcenter_api_client:
             current_vlan_id = self._vcenter_api_client.get_vlan_id(vmi_model.vcenter_port)
             vmi_model.acquire_vlan_id(current_vlan_id)
             if not current_vlan_id:
                 self._vcenter_api_client.set_vlan_id(vmi_model.vcenter_port)
+        vmi_model.parent = self._project
+        vmi_model.security_group = self._default_security_group
         self._vnc_api_client.update_or_create_vmi(vmi_model.to_vnc())
         vmi_model.construct_instance_ip()
         if vmi_model.vnc_instance_ip:
@@ -212,6 +209,9 @@ def update_nic(self, nic_info):
             pass
 
     def _delete(self, vmi_model):
+        with self._vcenter_api_client:
+            self._vcenter_api_client.restore_vlan_id(vmi_model.vcenter_port)
+            vmi_model.clear_vlan_id()
         self._delete_from_vnc(vmi_model)
         self._restore_vlan_id(vmi_model)
         self._database.delete_vmi_model(vmi_model.uuid)
@@ -276,7 +276,8 @@ def _delete_port(self, uuid):
         self._vrouter_api_client.delete_port(uuid)
 
     def _update_ports(self):
-        for vmi_model in self._database.ports_to_update:
+        ports = [vmi_model for vmi_model in self._database.ports_to_update]
+        for vmi_model in ports:
             if self._port_needs_an_update(vmi_model):
                 self._update_port(vmi_model)
             self._set_port_state(vmi_model)

tests/test_services.py

@@ -36,8 +36,9 @@ def setUp(self):
         self.vm_service = VirtualMachineService(self.esxi_api_client, self.vnc_client, self.database)
 
     def test_update_new_vm(self):
-        vm_model = self.vm_service.update(self.vmware_vm)
+        self.vm_service.update(self.vmware_vm)
 
+        vm_model = self.database.save.call_args[0][0]
         self.assertIsNotNone(vm_model)
         self.assertEqual(self.vm_properties, vm_model.vm_properties)
         self.assertEqual(self.vmware_vm, vm_model.vmware_vm)
@@ -53,11 +54,12 @@ def test_create_property_filter(self):
         )
         self.esxi_api_client.add_filter.return_value = property_filter
 
-        vm_model = self.vm_service.update(self.vmware_vm)
+        self.vm_service.update(self.vmware_vm)
 
         self.esxi_api_client.add_filter.assert_called_once_with(
             self.vmware_vm, ['guest.toolsRunningStatus', 'guest.net']
         )
+        vm_model = self.database.save.call_args[0][0]
         self.assertEqual(property_filter, vm_model.property_filter)
 
     def test_destroy_property_filter(self):
@@ -71,11 +73,12 @@ def test_destroy_property_filter(self):
         vm_model.destroy_property_filter.assert_called_once()
 
     def test_update_existing_vm(self):
-        old_vm_model = Mock()
+        old_vm_model = Mock(vmi_models=[])
         self.database.get_vm_model_by_uuid.return_value = old_vm_model
 
-        new_vm_model = self.vm_service.update(self.vmware_vm)
+        self.vm_service.update(self.vmware_vm)
 
+        new_vm_model = self.database.save.call_args[0][0]
         self.assertEqual(old_vm_model, new_vm_model)
         old_vm_model.update.assert_called_once_with(self.vmware_vm, self.vm_properties)
         self.vnc_client.update_or_create_vm.assert_not_called()
@@ -179,10 +182,11 @@ def setUp(self):
     def test_create_vmis_proper_vm_dpg(self):
         """ A new VMI is being created with proper VM/DPG pair. """
         self.database.save(self.vm_model)
+        self.database.vmis_to_update.append(self.vm_model.vmi_models[0])
         other_vn_model = create_vn_model(name='DPortGroup', key='dportgroup-51', uuid='uuid_2')
         self.database.save(other_vn_model)
 
-        self.vmi_service.update_vmis_for_vm_model(self.vm_model)
+        self.vmi_service.update_vmis()
 
         self.assertEqual(1, len(self.database.get_all_vmi_models()))
         saved_vmi = self.database.get_all_vmi_models()[0]
@@ -195,8 +199,10 @@ def test_create_vmis_proper_vm_dpg(self):
     def test_no_update_for_no_dpgs(self):
         """ No new VMIs are created for VM not connected to any DPG. """
         self.vm_model.ports = {}
+        self.vm_model.vmi_models = []
+        self.database.save(self.vm_model)
 
-        self.vmi_service.update_vmis_for_vm_model(self.vm_model)
+        self.vmi_service.update_vmis()
 
         self.assertEqual(0, len(self.database.get_all_vmi_models()))
         self.vnc_client.update_or_create_vmi.assert_not_called()
@@ -219,8 +225,12 @@ def test_update_existing_vmi(self):
         self.database.save(vmi_model)
         device.backing.port.portgroupKey = 'dportgroup-51'
         self.vm_model.ports[0] = VCenterPort(device)
+        self.vm_model.vmi_models[0] = VirtualMachineInterfaceModel(
+            self.vm_model, None, self.vm_model.ports[0]
+        )
+        self.database.vmis_to_update.append(self.vm_model.vmi_models[0])
 
-        self.vmi_service.update_vmis_for_vm_model(self.vm_model)
+        self.vmi_service.update_vmis()
 
         self.assertEqual(1, len(self.database.get_all_vmi_models()))
         saved_vmi = self.database.get_all_vmi_models()[0]
@@ -230,24 +240,10 @@ def test_update_existing_vmi(self):
         self.assertIn(second_vn_model.uuid, [ref['uuid'] for ref in vnc_vmi.get_virtual_network_refs()])
         self.assertIn(saved_vmi, self.database.ports_to_update)
 
-    def test_removes_unused_vmis(self):
-        """ VMIs are deleted when the VM is no longer connected to corresponding DPG. """
-        device = Mock(macAddress='c8:5b:76:53:0f:f5')
-        vmi_model = VirtualMachineInterfaceModel(self.vm_model, self.vn_model,
-                                                 VCenterPort(device))
-        vmi_model.vnc_instance_ip = Mock()
-        self.database.save(vmi_model)
-
-        self.vm_model.ports = {}
-        self.vmi_service.update_vmis_for_vm_model(self.vm_model)
-
-        self.assertFalse(self.database.get_all_vmi_models())
-        self.vnc_client.delete_vmi.assert_called_once_with(
-            VirtualMachineInterfaceModel.get_uuid('c8:5b:76:53:0f:f5'))
-        self.assertIn(vmi_model.uuid, self.database.ports_to_delete)
-
     def test_sync_vmis(self):
         self.database.save(self.vm_model)
+        self.database.save(Mock(vn_model=self.vn_model))
+        self.database.vmis_to_update.append(self.vm_model.vmi_models[0])
         self.vnc_client.get_vmis_by_project.return_value = []
 
         self.vmi_service.sync_vmis()
@@ -256,6 +252,7 @@ def test_sync_vmis(self):
 
     def test_syncs_one_vmi_once(self):
         self.database.save(self.vm_model)
+        self.database.vmis_to_update.append(self.vm_model.vmi_models[0])
         self.vnc_client.get_vmis_by_project.return_value = []
 
         with patch.object(self.database, 'save') as database_save_mock: