VCenter Port management refactor

Use Criteria to fetch DVPorts
Add method for retrieving DVS by UUID
Change VCenterPort constructor to consume whole device object
Change set_vlan_id signature to consume whole VCenterPort object
Change get_vlan_id signature to consume only VCenterPort object
Change restore_vlan_id signature to consume only VCenterPort object
Extract fetching port from dvs to a function
Extract creating inherited port config spec to a function
Remove unused get_ip_pool methods from VCenterAPIClient
Catch exceptions for vrouter-uuid annotation reading
Remove NoneType check from create_and_read_instance_ip

Partial-Bug: #1777404
Change-Id: I271ff574509fbbb070acb0ddaa0a35f5a76ea5ce
(cherry picked from commit 7f3b658)

cvm/clients.py

@@ -42,13 +42,16 @@ def make_filter_spec(obj, filters):
     return filter_spec
 
 
-def make_dv_port_spec(dv_port, vlan_id):
+def make_dv_port_spec(dv_port, vlan_id=None):
     dv_port_config_spec = vim.dvs.DistributedVirtualPort.ConfigSpec()
     dv_port_config_spec.key = dv_port.key
     dv_port_config_spec.operation = 'edit'
     dv_port_config_spec.configVersion = dv_port.config.configVersion
     vlan_spec = vim.dvs.VmwareDistributedVirtualSwitch.VlanIdSpec()
-    vlan_spec.vlanId = vlan_id
+    if vlan_id:
+        vlan_spec.vlanId = vlan_id
+    else:
+        vlan_spec.inherited = True
     dv_port_setting = vim.dvs.VmwareDistributedVirtualSwitch.VmwarePortConfigPolicy()
     dv_port_setting.vlan = vlan_spec
     dv_port_config_spec.setting = dv_port_setting
@@ -63,6 +66,15 @@ def make_pg_config_vlan_override(portgroup):
     return pg_config_spec
 
 
+def fetch_port_from_dvs(dvs, port_key):
+    criteria = vim.dvs.PortCriteria()
+    criteria.portKey = port_key
+    try:
+        return next(port for port in dvs.FetchDVPorts(criteria))
+    except StopIteration:
+        return None
+
+
 class VSphereAPIClient(object):
     def __init__(self):
         self._si = None
@@ -166,50 +178,32 @@ def get_dpg_by_name(self, name):
                 return dpg
         return None
 
-    def get_ip_pool_for_dpg(self, dpg):
-        return self._get_ip_pool_by_id(dpg.summary.ipPoolId)
-
-    def _get_ip_pool_by_id(self, pool_id):
-        for ip_pool in self._si.content.ipPoolManager.QueryIpPools(self._datacenter):
-            if ip_pool.id == pool_id:
-                return ip_pool
-        return None
-
-    def set_vlan_id(self, dvs_name, key, vlan_id):
-        dvs = self._get_object([vim.dvs.VmwareDistributedVirtualSwitch], dvs_name)
-        try:
-            dv_port = next(port for port in dvs.FetchDVPorts() if port.key == key)
-        except StopIteration:
+    def set_vlan_id(self, vcenter_port):
+        dvs = self._get_dvs_by_uuid(vcenter_port.dvs_uuid)
+        dv_port = fetch_port_from_dvs(dvs, vcenter_port.port_key)
+        if not dv_port:
             return
-        dv_port_spec = make_dv_port_spec(dv_port, vlan_id)
-        logger.info('Setting VLAN ID of port %s to %d', key, vlan_id)
+        dv_port_spec = make_dv_port_spec(dv_port, vcenter_port.vlan_id)
+        logger.info('Setting VLAN ID of port %s to %d', vcenter_port.port_key, vcenter_port.vlan_id)
         dvs.ReconfigureDVPort_Task(port=[dv_port_spec])
 
-    def get_vlan_id(self, vmi_model):
-        dvs = self._get_object([vim.dvs.VmwareDistributedVirtualSwitch], vmi_model.vn_model.dvs_name)
-        try:
-            # TODO: Use criteria to fetch single port
-            dv_port = next(port for port in dvs.FetchDVPorts() if port.key == vmi_model.vcenter_port.port_key)
-        except StopIteration:
-            return None
+    def get_vlan_id(self, vcenter_port):
+        dvs = self._get_dvs_by_uuid(vcenter_port.dvs_uuid)
+        dv_port = fetch_port_from_dvs(dvs, vcenter_port.port_key)
         if not dv_port.config.setting.vlan.inherited:
             return dv_port.config.setting.vlan.vlanId
         return None
 
-    def restore_vlan_id(self, dvs_name, key):
-        dvs = self._get_object([vim.dvs.VmwareDistributedVirtualSwitch], dvs_name)
-        dv_port = [port for port in dvs.FetchDVPorts() if port.key == key][0]
-        dv_port_config_spec = vim.dvs.DistributedVirtualPort.ConfigSpec()
-        dv_port_config_spec.key = dv_port.key
-        dv_port_config_spec.operation = 'edit'
-        dv_port_config_spec.configVersion = dv_port.config.configVersion
-        vlan_spec = vim.dvs.VmwareDistributedVirtualSwitch.VlanIdSpec()
-        vlan_spec.inherited = True
-        dv_port_setting = vim.dvs.VmwareDistributedVirtualSwitch.VmwarePortConfigPolicy()
-        dv_port_setting.vlan = vlan_spec
-        dv_port_config_spec.setting = dv_port_setting
+    def restore_vlan_id(self, vcenter_port):
+        dvs = self._get_dvs_by_uuid(vcenter_port.dvs_uuid)
+        dv_port = fetch_port_from_dvs(dvs, vcenter_port.port_key)
+        dv_port_config_spec = make_dv_port_spec(dv_port)
         dvs.ReconfigureDVPort_Task(port=[dv_port_config_spec])
 
+    def _get_dvs_by_uuid(self, uuid):
+        dvs_manager = self._si.content.dvSwitchManager
+        return dvs_manager.QueryDvsByUuid(uuid)
+
     @staticmethod
     def enable_vlan_override(portgroup):
         pg_config_spec = make_pg_config_vlan_override(portgroup)
@@ -366,11 +360,8 @@ def _create_ipam(self):
         return ipam
 
     def create_and_read_instance_ip(self, instance_ip):
-        if not instance_ip:
-            return None
         try:
             return self._read_instance_ip(instance_ip.uuid)
-            # TODO: Refactor this
         except NoIdError:
             self.vnc_lib.instance_ip_create(instance_ip)
             logger.debug("Created instanceIP: %s", instance_ip.name)

cvm/models.py

@@ -97,14 +97,10 @@ def rename(self, name):
 
     def update_ports(self):
         self.ports = self._read_ports()
-        # port = next(port for port in self.ports if port.mac_address == mac_address)
-        # port.portgroup_key = portgroup_key
 
     def _read_ports(self):
         try:
-            return [VCenterPort(mac_address=device.macAddress,
-                                port_key=device.backing.port.portKey,
-                                portgroup_key=device.backing.port.portgroupKey)
+            return [VCenterPort(device)
                     for device in self.vmware_vm.config.hardware.device
                     if isinstance(device.backing, vim.vm.device.VirtualEthernetCard.DistributedVirtualPortBackingInfo)]
         except AttributeError:
@@ -307,8 +303,9 @@ def free(self, vlan_id):
 
 
 class VCenterPort(object):
-    def __init__(self, mac_address, port_key, portgroup_key):
-        self.mac_address = mac_address
-        self.port_key = port_key
-        self.portgroup_key = portgroup_key
+    def __init__(self, device):
+        self.mac_address = device.macAddress
+        self.port_key = device.backing.port.portKey
+        self.portgroup_key = device.backing.port.portgroupKey
+        self.dvs_uuid = device.backing.port.switchUuid
         self.vlan_id = None

cvm/services.py

@@ -26,9 +26,14 @@ def _can_delete_from_vnc(self, vnc_obj):
             existing_obj = self._vnc_api_client.read_vm(vnc_obj.uuid)
         if vnc_obj.get_type() == 'virtual-machine-interface':
             existing_obj = self._vnc_api_client.read_vmi(vnc_obj.uuid)
-        existing_obj_vrouter_uuid = next(pair.value
-                                         for pair in existing_obj.get_annotations().key_value_pair
-                                         if pair.key == 'vrouter-uuid')
+        try:
+            existing_obj_vrouter_uuid = next(pair.value
+                                             for pair in existing_obj.get_annotations().key_value_pair
+                                             if pair.key == 'vrouter-uuid')
+        except (AttributeError, StopIteration):
+            logger.error('Cannot read vrouter-uuid annotation for %s %s.', vnc_obj.get_type(), vnc_obj.name)
+            return False
+
         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.',
@@ -160,8 +165,7 @@ def update_vmis_vn(self, vmware_vm, mac_address):
             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.vn_model.dvs_name, vmi_model.vcenter_port.port_key)
+                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
@@ -170,11 +174,10 @@ def update_vmis_vn(self, vmware_vm, mac_address):
 
     def _create_or_update(self, vmi_model):
         with self._vcenter_api_client:
-            current_vlan_id = self._vcenter_api_client.get_vlan_id(vmi_model)
+            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.vn_model.dvs_name, vmi_model.vcenter_port.port_key, vmi_model.vcenter_port.vlan_id)
+                self._vcenter_api_client.set_vlan_id(vmi_model.vcenter_port)
         self._vnc_api_client.update_or_create_vmi(vmi_model.to_vnc())
         vmi_model.construct_instance_ip()
         if vmi_model.vnc_instance_ip:
@@ -219,8 +222,7 @@ def _delete_from_vnc(self, vmi_model):
 
     def _restore_vlan_id(self, vmi_model):
         with self._vcenter_api_client:
-            self._vcenter_api_client.restore_vlan_id(
-                vmi_model.vn_model.dvs_name, vmi_model.vcenter_port.port_key)
+            self._vcenter_api_client.restore_vlan_id(vmi_model.vcenter_port)
         vmi_model.clear_vlan_id()
 
     def _delete_vrouter_port(self, vmi_model):

tests/test_clients.py

@@ -36,13 +36,14 @@ def setUp(self):
     def test_set_vlan_id(self):
         dv_port = Mock(key='8')
         dv_port.config.configVersion = '1'
-        dvs = Mock(name='DSwitch')
+        dvs = Mock()
         dvs.FetchDVPorts.return_value = [dv_port]
-        with patch('cvm.clients.VSphereAPIClient._get_object') as get_obj_mock:
-            get_obj_mock.return_value = dvs
+        vcenter_port = Mock(port_key='8', vlan_id=10)
+        with patch('cvm.clients.VCenterAPIClient._get_dvs_by_uuid') as get_dvs_mock:
+            get_dvs_mock.return_value = dvs
             with patch('cvm.clients.SmartConnectNoSSL'):
                 with self.vcenter_client:
-                    self.vcenter_client.set_vlan_id(dvs_name=dvs.name, key='8', vlan_id=10)
+                    self.vcenter_client.set_vlan_id(vcenter_port)
 
         dvs.ReconfigureDVPort_Task.assert_called_once()
         spec = dvs.ReconfigureDVPort_Task.call_args[1].get('port', [None])[0]
@@ -71,17 +72,35 @@ def test_get_vlan_id(self):
         dv_port.config.setting.vlan.inherited = False
         dvs = Mock(name='DSwitch')
         dvs.FetchDVPorts.return_value = [dv_port]
-        vmi_model = Mock()
-        vmi_model.vcenter_port.port_key = '20'
+        vcenter_port = Mock(port_key='20')
 
-        with patch('cvm.clients.VSphereAPIClient._get_object') as get_obj_mock:
-            get_obj_mock.return_value = dvs
+        with patch('cvm.clients.VCenterAPIClient._get_dvs_by_uuid') as get_dvs_mock:
+            get_dvs_mock.return_value = dvs
             with patch('cvm.clients.SmartConnectNoSSL'):
                 with self.vcenter_client:
-                    result = self.vcenter_client.get_vlan_id(vmi_model)
+                    result = self.vcenter_client.get_vlan_id(vcenter_port)
 
         self.assertEqual(10, result)
 
+    def test_restore_vlan_id(self):
+        dv_port = Mock(key='8')
+        dv_port.config.configVersion = '1'
+        dvs = Mock()
+        dvs.FetchDVPorts.return_value = [dv_port]
+        vcenter_port = Mock(port_key='8', vlan_id=10)
+        with patch('cvm.clients.VCenterAPIClient._get_dvs_by_uuid') as get_dvs_mock:
+            get_dvs_mock.return_value = dvs
+            with patch('cvm.clients.SmartConnectNoSSL'):
+                with self.vcenter_client:
+                    self.vcenter_client.restore_vlan_id(vcenter_port)
+
+        dvs.ReconfigureDVPort_Task.assert_called_once()
+        spec = dvs.ReconfigureDVPort_Task.call_args[1].get('port', [None])[0]
+        self.assertIsNotNone(spec)
+        self.assertEqual('8', spec.key)
+        self.assertEqual('1', spec.configVersion)
+        self.assertTrue(spec.setting.vlan.inherited)
+
 
 class TestFunctions(TestCase):
     def test_make_dv_port_spec(self):
@@ -91,6 +110,16 @@ def test_make_dv_port_spec(self):
         self.assertEqual('8', spec.key)
         self.assertEqual('edit', spec.operation)
         self.assertEqual(10, spec.setting.vlan.vlanId)
+        self.assertFalse(spec.setting.vlan.inherited)
+        self.assertEqual('1', spec.configVersion)
+
+    def test_make_port_spec_restore(self):
+        dv_port = Mock(key='8')
+        dv_port.config.configVersion = '1'
+        spec = make_dv_port_spec(dv_port)
+        self.assertEqual('8', spec.key)
+        self.assertEqual('edit', spec.operation)
+        self.assertTrue(spec.setting.vlan.inherited)
         self.assertEqual('1', spec.configVersion)
 
 

tests/test_events.py

@@ -257,7 +257,9 @@ def test_vm_created(vcenter_api_client, vn_model_1, vm_created_update,
     vrouter_api_client.add_port.assert_called_once_with(vmi_model)
 
     # Check if VLAN ID has been set using VLAN Override
-    vcenter_api_client.set_vlan_id.assert_called_once_with(vmi_model.vn_model.dvs_name, '10', 2)
+    vcenter_port = vcenter_api_client.set_vlan_id.call_args[0][0]
+    assert vcenter_port.port_key == '10'
+    assert vcenter_port.vlan_id == 2
 
     # Check inner VMI model state
     assert_vmi_model_state(
@@ -329,7 +331,9 @@ def test_vm_renamed(vcenter_api_client, vn_model_1, vm_created_update,
     assert vrouter_api_client.add_port.call_count == 2
 
     # Check if VLAN ID has been set using VLAN Override
-    vcenter_api_client.set_vlan_id.assert_called_once_with(vmi_model.vn_model.dvs_name, '10', 2)
+    vcenter_port = vcenter_api_client.set_vlan_id.call_args[0][0]
+    assert vcenter_port.port_key == '10'
+    assert vcenter_port.vlan_id == 2
 
     # Check inner VMI model state
     assert_vmi_model_state(
@@ -415,7 +419,9 @@ def test_vm_reconfigured(vcenter_api_client, vn_model_1, vn_model_2, vm_created_
 
     # Check if VLAN ID has been set using VLAN Override
     assert vcenter_api_client.set_vlan_id.call_count == 2
-    assert vcenter_api_client.set_vlan_id.call_args[0] == (vmi_model.vn_model.dvs_name, '11', 4)
+    vcenter_port = vcenter_api_client.set_vlan_id.call_args[0][0]
+    assert vcenter_port.port_key == '11'
+    assert vcenter_port.vlan_id == 4
 
     # Check inner VMI model state
     assert_vmi_model_state(

tests/test_models.py

@@ -162,7 +162,7 @@ def setUp(self):
         vnc_vn = Mock(uuid='d376b6b4-943d-4599-862f-d852fd6ba425')
         vnc_vn.name = 'VM Network'
         self.vn_model = VirtualNetworkModel(vmware_vn, vnc_vn, VlanIdPool(0, 100))
-        self.vcenter_port = VCenterPort('c8:5b:76:53:0f:f5', None, '123')
+        self.vcenter_port = VCenterPort(device)
 
     def test_to_vnc(self):
         vmi_model = VirtualMachineInterfaceModel(self.vm_model, self.vn_model, self.vcenter_port)
@@ -269,7 +269,9 @@ def test_vmi_vlan_id_aquisition(self):
         self.ports.append(create_port_mock(1))
         vn_model = VirtualNetworkModel(self.dpg, None, VlanIdPool(0, 100))
         vm_model = VirtualMachineModel(*create_vmware_vm_mock([self.dpg]))
-        vcenter_port = VCenterPort('', 'dvportgroup-20', '')
+        device = Mock()
+        device.backing.port.portgroupKey = 'dvportgroup-20'
+        vcenter_port = VCenterPort(device)
         vmi_model = VirtualMachineInterfaceModel(vm_model, vn_model, vcenter_port)
 
         vmi_model.acquire_vlan_id(None)