CVM checks ownership of VNC entities before modification

Closes-Bug: #1779885
Change-Id: I63c6ce9593b259fbf752350f5e282ba02688b243

cvm/models.py

@@ -84,7 +84,6 @@ def __init__(self, vmware_vm, vm_properties):
         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):
         self.vmware_vm = vmware_vm  # TODO: Consider removing this
@@ -135,16 +134,15 @@ def tools_running(self):
 
     @property
     def vnc_vm(self):
-        if not self._vnc_vm:
-            self._vnc_vm = VirtualMachine(name=self.uuid,
-                                          display_name=self.name,
-                                          id_perms=ID_PERMS)
-            self._vnc_vm.set_uuid(self.uuid)
-            self._vnc_vm.annotations = self.vnc_vm.annotations or KeyValuePairs()
-            self._vnc_vm.annotations.add_key_value_pair(
-                KeyValuePair('vrouter-uuid', self.vrouter_uuid)
-            )
-        return self._vnc_vm
+        vnc_vm = VirtualMachine(name=self.uuid,
+                                display_name=self.name,
+                                id_perms=ID_PERMS)
+        vnc_vm.set_uuid(self.uuid)
+        vnc_vm.annotations = KeyValuePairs()
+        vnc_vm.annotations.add_key_value_pair(
+            KeyValuePair('vrouter-uuid', self.vrouter_uuid)
+        )
+        return vnc_vm
 
     def __repr__(self):
         return 'VirtualMachineModel(uuid=%s, name=%s, vrouter_uuid=%s, vm_properties=%s, interfaces=%s)' % \
@@ -191,7 +189,6 @@ def __init__(self, vm_model, vn_model, vcenter_port):
         self.vn_model = vn_model
         self.vcenter_port = vcenter_port
         self.ip_address = None
-        self.vnc_vmi = None
         self.vnc_instance_ip = None
         self.parent = None
         self.security_group = None
@@ -209,23 +206,23 @@ def display_name(self):
     def refresh_port_key(self):
         self.vcenter_port.port_key = find_vmi_port_key(self.vm_model.vmware_vm, self.vcenter_port.mac_address)
 
-    def to_vnc(self):
-        if not self.vnc_vmi:
-            self.vnc_vmi = VirtualMachineInterface(name=self.uuid,
-                                                   display_name=self.display_name,
-                                                   parent_obj=self.parent,
-                                                   id_perms=ID_PERMS)
-            self.vnc_vmi.set_uuid(self.uuid)
-            self.vnc_vmi.add_virtual_machine(self.vm_model.vnc_vm)
-            self.vnc_vmi.set_virtual_network(self.vn_model.vnc_vn)
-            self.vnc_vmi.set_virtual_machine_interface_mac_addresses(MacAddressesType([self.vcenter_port.mac_address]))
-            self.vnc_vmi.set_port_security_enabled(True)
-            self.vnc_vmi.set_security_group(self.security_group)
-            self.vnc_vmi.annotations = self.vnc_vmi.annotations or KeyValuePairs()
-            self.vnc_vmi.annotations.add_key_value_pair(
-                KeyValuePair('vrouter-uuid', self.vm_model.vrouter_uuid)
-            )
-        return self.vnc_vmi
+    @property
+    def vnc_vmi(self):
+        vnc_vmi = VirtualMachineInterface(name=self.uuid,
+                                          display_name=self.display_name,
+                                          parent_obj=self.parent,
+                                          id_perms=ID_PERMS)
+        vnc_vmi.set_uuid(self.uuid)
+        vnc_vmi.add_virtual_machine(self.vm_model.vnc_vm)
+        vnc_vmi.set_virtual_network(self.vn_model.vnc_vn)
+        vnc_vmi.set_virtual_machine_interface_mac_addresses(MacAddressesType([self.vcenter_port.mac_address]))
+        vnc_vmi.set_port_security_enabled(True)
+        vnc_vmi.set_security_group(self.security_group)
+        vnc_vmi.annotations = KeyValuePairs()
+        vnc_vmi.annotations.add_key_value_pair(
+            KeyValuePair('vrouter-uuid', self.vm_model.vrouter_uuid)
+        )
+        return vnc_vmi
 
     def construct_instance_ip(self):
         if not self._should_construct_instance_ip():
@@ -247,7 +244,7 @@ def construct_instance_ip(self):
         # TODO: Check if setting this to None works (remove if statement)
         instance_ip.set_uuid(instance_ip_uuid)
         instance_ip.set_virtual_network(self.vn_model.vnc_vn)
-        instance_ip.set_virtual_machine_interface(self.to_vnc())
+        instance_ip.set_virtual_machine_interface(self.vnc_vmi)
         instance_ip.annotations = instance_ip.annotations or KeyValuePairs()
         instance_ip.annotations.add_key_value_pair(
             KeyValuePair('vrouter-uuid', self.vm_model.vrouter_uuid)

cvm/services.py

@@ -25,7 +25,7 @@ def __init__(self, vnc_api_client, database, esxi_api_client=None, vcenter_api_c
         if self._esxi_api_client:
             self._vrouter_uuid = esxi_api_client.read_vrouter_uuid()
 
-    def _can_delete_from_vnc(self, vnc_obj):
+    def _can_modify_in_vnc(self, vnc_obj):
         if vnc_obj.get_type() == 'virtual-machine':
             existing_obj = self._vnc_api_client.read_vm(vnc_obj.uuid)
         if vnc_obj.get_type() == 'virtual-machine-interface':
@@ -40,7 +40,7 @@ def _can_delete_from_vnc(self, vnc_obj):
 
         if existing_obj_vrouter_uuid == self._vrouter_uuid:
             return True
-        logger.error('%s %s is managed by vRouter %s and cannot be deleted from VNC.',
+        logger.error('%s %s is managed by vRouter %s and cannot be modified in VNC.',
                      vnc_obj.get_type(), vnc_obj.name, existing_obj_vrouter_uuid)
         return False
 
@@ -91,7 +91,7 @@ def delete_unused_vms_in_vnc(self):
             vm_model = self._database.get_vm_model_by_uuid(vnc_vm.uuid)
             if vm_model:
                 continue
-            if self._can_delete_from_vnc(vnc_vm):
+            if self._can_modify_in_vnc(vnc_vm):
                 logger.info('Deleting %s from VNC', vnc_vm.name)
                 self._vnc_api_client.delete_vm(vnc_vm.uuid)
 
@@ -100,7 +100,7 @@ def remove_vm(self, name):
         logger.info('Deleting %s', vm_model)
         if not vm_model:
             return
-        if self._can_delete_from_vnc(vm_model.vnc_vm):
+        if self._can_modify_in_vnc(vm_model.vnc_vm):
             self._vnc_api_client.delete_vm(vm_model.vnc_vm.uuid)
         self._database.delete_vm_model(vm_model.uuid)
         vm_model.destroy_property_filter()
@@ -117,7 +117,8 @@ def rename_vm(self, old_name, new_name):
         logger.info('Renaming %s to %s', old_name, new_name)
         vm_model = self._database.get_vm_model_by_name(old_name)
         vm_model.rename(new_name)
-        self._vnc_api_client.update_or_create_vm(vm_model.vnc_vm)
+        if self._can_modify_in_vnc(vm_model.vnc_vm):
+            self._vnc_api_client.update_or_create_vm(vm_model.vnc_vm)
         self._database.save(vm_model)
 
     def update_vm_models_interfaces(self, vmware_vm):
@@ -217,7 +218,7 @@ def _add_vnc_info_to(self, vmi_model):
         vmi_model.security_group = self._default_security_group
 
     def _update_in_vnc(self, vmi_model):
-        self._vnc_api_client.update_or_create_vmi(vmi_model.to_vnc())
+        self._vnc_api_client.update_or_create_vmi(vmi_model.vnc_vmi)
 
     def _add_instance_ip_to(self, vmi_model):
         vmi_model.construct_instance_ip()
@@ -233,7 +234,7 @@ def _delete_unused_vmis(self):
             vmi_model = self._database.get_vmi_model_by_uuid(vnc_vmi.get_uuid())
             if vmi_model:
                 continue
-            if self._can_delete_from_vnc(vnc_vmi):
+            if self._can_modify_in_vnc(vnc_vmi):
                 logger.info('Deleting %s from VNC.', vnc_vmi.name)
                 self._vnc_api_client.delete_vmi(vnc_vmi.get_uuid())
 
@@ -257,7 +258,6 @@ def _delete(self, vmi_model):
 
     def _delete_from_vnc(self, vmi_model):
         self._vnc_api_client.delete_vmi(vmi_model.uuid)
-        vmi_model.vnc_vmi = None
 
     def _restore_vlan_id(self, vmi_model):
         with self._vcenter_api_client:
@@ -274,7 +274,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.vnc_vmi):
+            if self._can_modify_in_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)
@@ -285,7 +285,8 @@ def rename_vmis(self, new_name):
         vmi_models = self._database.get_vmi_models_by_vm_uuid(vm_model.uuid)
         for vmi_model in vmi_models:
             vmi_model.vm_model = vm_model
-            self._update_in_vnc(vmi_model)
+            if self._can_modify_in_vnc(vmi_model.vnc_vmi):
+                self._update_in_vnc(vmi_model)
             self._update_vrouter_port(vmi_model)
 
     @staticmethod

tests/test_events.py

@@ -309,6 +309,10 @@ def test_vm_renamed(vcenter_api_client, vn_model_1, vm_created_update,
         database,
         vlan_id_pool=vlan_id_pool
     )
+    vmi_service._can_modify_in_vnc = Mock()
+    vmi_service._can_modify_in_vnc.return_value = True
+    vm_service._can_modify_in_vnc = Mock()
+    vmi_service._can_modify_in_vnc.return_value = True
     vrouter_port_service = VRouterPortService(vrouter_api_client, database)
     vm_updated_handler = VmUpdatedHandler(vm_service, vn_service, vmi_service, vrouter_port_service)
     vm_renamed_handler = VmRenamedHandler(vm_service, vmi_service, vrouter_port_service)
@@ -538,3 +542,4 @@ def test_contrail_vm(vcenter_api_client, vm_created_update, esxi_api_client,
 
     # There were no calls to vrouter_api
     vrouter_api_client.add_port.assert_not_called()
+

tests/test_models.py

@@ -169,7 +169,7 @@ def test_to_vnc(self):
         vmi_model.parent = self.project
         vmi_model.security_group = self.security_group
 
-        vnc_vmi = vmi_model.to_vnc()
+        vnc_vmi = vmi_model.vnc_vmi
 
         self.assertEqual(vnc_vmi.name, vmi_model.uuid)
         self.assertEqual(vnc_vmi.parent_name, self.project.name)
@@ -180,11 +180,11 @@ def test_to_vnc(self):
         self.assertEqual(vnc_vmi.get_id_perms(), ID_PERMS)
 
     @patch('cvm.models.find_vm_mac_address')
-    @patch('cvm.models.VirtualMachineInterfaceModel.to_vnc')
+    @patch('cvm.models.VirtualMachineInterfaceModel.vnc_vmi')
     @patch('cvm.models.VirtualMachineInterfaceModel._should_construct_instance_ip')
-    def test_construct_instance_ip(self, should_construct, to_vnc_mock, _):
+    def test_construct_instance_ip(self, should_construct, vnc_vmi_mock, _):
         should_construct.return_value = True
-        to_vnc_mock.return_value.uuid = 'd376b6b4-943d-4599-862f-d852fd6ba425'
+        vnc_vmi_mock.uuid = 'd376b6b4-943d-4599-862f-d852fd6ba425'
 
         vmi_model = VirtualMachineInterfaceModel(self.vm_model, self.vn_model, self.vcenter_port)
         vmi_model.construct_instance_ip()

tests/test_services.py

@@ -45,7 +45,7 @@ def test_update_new_vm(self):
         self.assertEqual(self.vmware_vm, vm_model.vmware_vm)
         self.assertEqual({'c8:5b:76:53:0f:f5': 'dportgroup-50'},
                          {vm_model.ports[0].mac_address: vm_model.ports[0].portgroup_key})
-        self.vnc_client.update_or_create_vm.assert_called_once_with(vm_model.vnc_vm)
+        self.vnc_client.update_or_create_vm.assert_called_once()
         self.database.save.assert_called_once_with(vm_model)
 
     def test_create_property_filter(self):
@@ -67,8 +67,8 @@ def test_destroy_property_filter(self):
         vm_model = Mock()
         self.database.get_vm_model_by_name.return_value = vm_model
 
-        with patch('cvm.services.VirtualMachineService._can_delete_from_vnc') as can_delete:
-            can_delete.return_value = True
+        with patch('cvm.services.VirtualMachineService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
             self.vm_service.remove_vm('VM')
 
         vm_model.destroy_property_filter.assert_called_once()
@@ -110,8 +110,8 @@ def test_delete_unused_vms(self):
         vnc_vm.set_uuid('d376b6b4-943d-4599-862f-d852fd6ba425')
         self.vnc_client.get_all_vms.return_value = [vnc_vm]
 
-        with patch('cvm.services.VirtualMachineService._can_delete_from_vnc') as can_delete:
-            can_delete.return_value = True
+        with patch('cvm.services.VirtualMachineService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
             self.vm_service.delete_unused_vms_in_vnc()
 
         self.vnc_client.delete_vm.assert_called_once_with('d376b6b4-943d-4599-862f-d852fd6ba425')
@@ -121,8 +121,8 @@ def test_remove_vm(self):
         vm_model.vnc_vm.uuid = 'd376b6b4-943d-4599-862f-d852fd6ba425'
         self.database.get_vm_model_by_name.return_value = vm_model
 
-        with patch('cvm.services.VirtualMachineService._can_delete_from_vnc') as can_delete:
-            can_delete.return_value = True
+        with patch('cvm.services.VirtualMachineService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
             self.vm_service.remove_vm('VM')
 
         self.database.delete_vm_model.assert_called_once_with(vm_model.uuid)
@@ -155,7 +155,9 @@ def test_rename_vm(self):
         vmware_vm = Mock()
         vmware_vm.configure_mock(name='VM-renamed')
 
-        self.vm_service.rename_vm('VM', 'VM-renamed')
+        with patch('cvm.services.VirtualMachineService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
+            self.vm_service.rename_vm('VM', 'VM-renamed')
 
         vm_model.rename.assert_called_once_with('VM-renamed')
         self.vnc_client.update_or_create_vm.assert_called_once()
@@ -316,21 +318,22 @@ def test_sync_no_vmis(self):
     def test_sync_deletes_unused_vmis(self):
         self.vnc_client.get_vmis_by_project.return_value = [Mock()]
 
-        with patch('cvm.services.VirtualMachineInterfaceService._can_delete_from_vnc') as can_delete:
-            can_delete.return_value = True
+        with patch('cvm.services.VirtualMachineInterfaceService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
             self.vmi_service.sync_vmis()
 
         self.vnc_client.delete_vmi.assert_called_once()
 
     def test_remove_vmis_for_vm_model(self):
         device = Mock(macAddress='mac_addr')
         vmi_model = VirtualMachineInterfaceModel(self.vm_model, self.vn_model, VCenterPort(device))
+        self.vmi_service._add_vnc_info_to(vmi_model)
         vmi_model.vnc_instance_ip = Mock()
         self.database.save(vmi_model)
         self.database.save(self.vm_model)
 
-        with patch('cvm.services.VirtualMachineInterfaceService._can_delete_from_vnc') as can_delete:
-            can_delete.return_value = True
+        with patch('cvm.services.VirtualMachineInterfaceService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
             self.vmi_service.remove_vmis_for_vm_model(self.vm_model.name)
 
         self.assertNotIn(vmi_model, self.database.get_all_vmi_models())
@@ -356,7 +359,9 @@ def test_rename_vmis(self):
         self.vm_model.update(*create_vmware_vm_mock(name='VM-renamed'))
         self.database.save(self.vm_model)
 
-        self.vmi_service.rename_vmis('VM-renamed')
+        with patch('cvm.services.VirtualMachineInterfaceService._can_modify_in_vnc') as can_modify:
+            can_modify.return_value = True
+            self.vmi_service.rename_vmis('VM-renamed')
 
         self.assertEqual('vmi-VM Portgroup-VM-renamed', vmi_model.display_name)
         self.assertEqual(0, self.vnc_client.create_and_read_instance_ip.call_count)
@@ -499,7 +504,7 @@ def test_vnc_vm_true(self):
             [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)
+        result = self.vm_service._can_modify_in_vnc(vnc_vm)
 
         self.assertTrue(result)
 
@@ -509,7 +514,7 @@ def test_vnc_vm_false(self):
             [KeyValuePair('vrouter-uuid', 'vrouter_uuid_2')]))
         self.vnc_api_client.read_vm.return_value = vnc_vm
 
-        result = self.vm_service._can_delete_from_vnc(vnc_vm)
+        result = self.vm_service._can_modify_in_vnc(vnc_vm)
 
         self.assertFalse(result)
 
@@ -519,7 +524,7 @@ def test_vnc_vmi_true(self):
             [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)
+        result = self.vmi_service._can_modify_in_vnc(vnc_vmi)
 
         self.assertTrue(result)
 
@@ -529,15 +534,15 @@ def test_vnc_vmi_false(self):
             [KeyValuePair('vrouter-uuid', 'vrouter_uuid_2')]))
         self.vnc_api_client.read_vmi.return_value = vnc_vmi
 
-        result = self.vmi_service._can_delete_from_vnc(vnc_vmi)
+        result = self.vmi_service._can_modify_in_vnc(vnc_vmi)
 
         self.assertFalse(result)
 
     def test_no_annotations(self):
         vnc_vm = vnc_api.VirtualMachine('VM', vnc_api.Project())
         self.vnc_api_client.read_vm.return_value = vnc_vm
 
-        result = self.vm_service._can_delete_from_vnc(vnc_vm)
+        result = self.vm_service._can_modify_in_vnc(vnc_vm)
 
         self.assertFalse(result)
 
@@ -547,7 +552,7 @@ def test_no_vrouter_uuid(self):
             [KeyValuePair('key', 'value')]))
         self.vnc_api_client.read_vm.return_value = vnc_vm
 
-        result = self.vm_service._can_delete_from_vnc(vnc_vm)
+        result = self.vm_service._can_modify_in_vnc(vnc_vm)
 
         self.assertFalse(result)