diff --git a/nova_powervm/conf/powervm.py b/nova_powervm/conf/powervm.py index 9a4abaa9..7246963f 100644 --- a/nova_powervm/conf/powervm.py +++ b/nova_powervm/conf/powervm.py @@ -53,6 +53,11 @@ powervm_opts = [ default='localdisk', help='The disk driver to use for PowerVM disks. ' 'Valid options are: localdisk, ssp'), + cfg.StrOpt('pvm_vswitch_for_ovs', + default='OpenStackOVS', + help="Name of the PowerVM virtual switch to be used when " + "mapping Open vSwitch ports to PowerVM virtual Ethernet " + "devices") ] localdisk_opts = [ diff --git a/nova_powervm/tests/virt/powervm/test_driver.py b/nova_powervm/tests/virt/powervm/test_driver.py index cf6ecf7b..0c80352b 100644 --- a/nova_powervm/tests/virt/powervm/test_driver.py +++ b/nova_powervm/tests/virt/powervm/test_driver.py @@ -743,6 +743,7 @@ class TestPowerVMDriver(test.TestCase): 'context', 'instance', 'bdms', flow, 'stg_ftsk') self.assertEqual(2, flow.add.call_count) + @mock.patch('nova_powervm.virt.powervm.tasks.network.UnplugVifs.execute') @mock.patch('nova_powervm.virt.powervm.driver.PowerVMDriver.' '_is_booted_from_volume') @mock.patch('nova_powervm.virt.powervm.vm.dlt_lpar') @@ -755,13 +756,13 @@ class TestPowerVMDriver(test.TestCase): @mock.patch('nova.objects.flavor.Flavor.get_by_id') def test_destroy_internal( self, mock_get_flv, mock_pvmuuid, mock_val_vopt, mock_dlt_vopt, - mock_pwroff, mock_dlt, mock_boot_from_vol): + mock_pwroff, mock_dlt, mock_boot_from_vol, mock_unplug_vifs): """Validates the basic PowerVM destroy.""" # BDMs mock_bdms = self._fake_bdms() mock_boot_from_vol.return_value = False # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, [mock.Mock], block_device_info=mock_bdms) # Power off was called @@ -769,6 +770,9 @@ class TestPowerVMDriver(test.TestCase): self.drv.host_uuid, force_immediate=True) + # Unplug should have been called + self.assertTrue(mock_unplug_vifs.called) + # Validate that the vopt delete was called self.assertTrue(mock_dlt_vopt.called) @@ -811,7 +815,7 @@ class TestPowerVMDriver(test.TestCase): self.drv.disk_dvr.disconnect_image_disk.reset_mock() # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, None, block_device_info=mock_bdms) # Validate root device in bdm was checked. @@ -828,7 +832,7 @@ class TestPowerVMDriver(test.TestCase): self.drv.disk_dvr.disconnect_image_disk.reset_mock() # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, None, block_device_info=mock_bdms, destroy_disks=False) mock_pwroff.assert_called_with(self.drv.adapter, self.inst, @@ -842,7 +846,7 @@ class TestPowerVMDriver(test.TestCase): instance_id=self.inst.name) # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, None, block_device_info=mock_bdms) assert_not_called() @@ -853,8 +857,9 @@ class TestPowerVMDriver(test.TestCase): 'b16fb039d63b/LogicalPartition/1B5FB633-16D1-4E10-A14' '5-E6FB905161A3?group=None') mock_pvmuuid.side_effect = pvm_exc.HttpError(mock_resp) + # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, [], block_device_info=mock_bdms) assert_not_called() @@ -867,7 +872,7 @@ class TestPowerVMDriver(test.TestCase): # Invoke the method. self.assertRaises(exc.InstanceTerminationFailure, self.drv.destroy, 'context', self.inst, - mock.Mock(), block_device_info=mock_bdms) + [], block_device_info=mock_bdms) assert_not_called() # Test generic exception @@ -875,7 +880,7 @@ class TestPowerVMDriver(test.TestCase): # Invoke the method. self.assertRaises(exc.InstanceTerminationFailure, self.drv.destroy, 'context', self.inst, - mock.Mock(), block_device_info=mock_bdms) + [], block_device_info=mock_bdms) assert_not_called() @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid') @@ -887,11 +892,11 @@ class TestPowerVMDriver(test.TestCase): with mock.patch.object(self.drv, '_destroy') as mock_dst_int: # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, [], block_device_info=mock_bdms) mock_dst_int.assert_called_with( 'context', self.inst, block_device_info=mock_bdms, - destroy_disks=True, shutdown=True) + destroy_disks=True, shutdown=True, network_info=[]) self.san_lpar_name.assert_not_called() # Test delete during migrate / resize @@ -899,7 +904,7 @@ class TestPowerVMDriver(test.TestCase): mock_getqp.return_value = 'resize_' + self.inst.name with mock.patch.object(self.drv, '_destroy') as mock_dst_int: # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, [], block_device_info=mock_bdms) # We shouldn't delete our resize_ instances mock_dst_int.assert_not_called() @@ -910,12 +915,12 @@ class TestPowerVMDriver(test.TestCase): mock_getqp.return_value = 'migrate_' + self.inst.name with mock.patch.object(self.drv, '_destroy') as mock_dst_int: # Invoke the method. - self.drv.destroy('context', self.inst, mock.Mock(), + self.drv.destroy('context', self.inst, [], block_device_info=mock_bdms) # If it is a migrated instance, it should be deleted. mock_dst_int.assert_called_with( 'context', self.inst, block_device_info=mock_bdms, - destroy_disks=True, shutdown=True) + destroy_disks=True, shutdown=True, network_info=[]) def test_attach_volume(self): """Validates the basic PowerVM attach volume.""" @@ -965,6 +970,7 @@ class TestPowerVMDriver(test.TestCase): # Verify the disconnect volume was not invoked self.assertEqual(0, self.vol_drv.disconnect_volume.call_count) + @mock.patch('nova_powervm.virt.powervm.tasks.network.UnplugVifs.execute') @mock.patch('nova_powervm.virt.powervm.vm.dlt_lpar') @mock.patch('nova_powervm.virt.powervm.vm.power_off') @mock.patch('nova_powervm.virt.powervm.media.ConfigDrivePowerVM.' @@ -974,7 +980,7 @@ class TestPowerVMDriver(test.TestCase): @mock.patch('nova.objects.flavor.Flavor.get_by_id') def test_destroy_rollback( self, mock_get_flv, mock_val_vopt, mock_dlt_vopt, mock_pwroff, - mock_dlt): + mock_dlt, mock_unplug_vifs): """Validates the basic PowerVM destroy rollback mechanism works.""" # Set up the mocks to the tasks. mock_get_flv.return_value = self.inst.get_flavor() @@ -992,11 +998,12 @@ class TestPowerVMDriver(test.TestCase): # Invoke the method. self.assertRaises(exc.InstanceTerminationFailure, self.drv.destroy, - 'context', self.inst, mock.Mock(), + 'context', self.inst, [], block_device_info=mock_bdms) # Validate that the vopt delete was called self.assertTrue(mock_dlt_vopt.called) + self.assertTrue(mock_unplug_vifs.called) # Validate that the volume detach was called self.assertEqual(2, self.vol_drv.disconnect_volume.call_count) diff --git a/nova_powervm/tests/virt/powervm/test_vif.py b/nova_powervm/tests/virt/powervm/test_vif.py index 71ca4302..4d6916e8 100644 --- a/nova_powervm/tests/virt/powervm/test_vif.py +++ b/nova_powervm/tests/virt/powervm/test_vif.py @@ -15,6 +15,7 @@ # under the License. import mock +import netifaces from nova import exception from nova import test @@ -85,7 +86,7 @@ class TestVifFunctions(test.TestCase): # Test an invalid vif type self.assertRaises(exception.VirtualInterfacePlugException, vif._build_vif_driver, self.adpt, 'host_uuid', - mock_inst, {'type': 'ovs'}) + mock_inst, {'type': 'bad'}) class TestVifSeaDriver(test.TestCase): @@ -142,3 +143,100 @@ class TestVifSeaDriver(test.TestCase): self.assertEqual(1, cnas[0].delete.call_count) self.assertEqual(0, cnas[1].delete.call_count) self.assertEqual(1, cnas[2].delete.call_count) + + +class TestVifOvsDriver(test.TestCase): + + def setUp(self): + super(TestVifOvsDriver, self).setUp() + + self.adpt = self.useFixture(pvm_fx.AdapterFx( + traits=pvm_fx.LocalPVMTraits)).adpt + self.inst = mock.MagicMock(uuid='inst_uuid') + self.drv = vif.PvmOvsVifDriver(self.adpt, 'host_uuid', self.inst) + + @mock.patch('nova.utils.execute') + @mock.patch('nova.network.linux_net.create_ovs_vif_port') + @mock.patch('nova_powervm.virt.powervm.vif.PvmOvsVifDriver.' + 'get_trunk_dev_name') + @mock.patch('pypowervm.tasks.cna.crt_p2p_cna') + @mock.patch('nova_powervm.virt.powervm.mgmt.get_mgmt_partition') + @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid') + def test_plug(self, mock_pvm_uuid, mock_mgmt_lpar, mock_p2p_cna, + mock_trunk_dev_name, mock_crt_ovs_vif_port, mock_exec): + # Mock the data + mock_pvm_uuid.return_value = 'lpar_uuid' + mock_mgmt_lpar.return_value = mock.Mock(uuid='mgmt_uuid') + mock_trunk_dev_name.return_value = 'device' + + cna_w, trunk_wraps = mock.MagicMock(), [mock.MagicMock()] + mock_p2p_cna.return_value = cna_w, trunk_wraps + + # Run the plug + vif = {'network': {'bridge': 'br0'}, 'address': 'aa:bb:cc:dd:ee:ff', + 'id': 'vif_id'} + self.drv.plug(vif) + + # Validate the calls + mock_crt_ovs_vif_port.assert_called_once_with( + 'br0', 'device', 'vif_id', 'aa:bb:cc:dd:ee:ff', 'inst_uuid') + mock_p2p_cna.assert_called_once_with( + self.adpt, 'host_uuid', 'lpar_uuid', ['mgmt_uuid'], 'OpenStackOVS', + crt_vswitch=True, mac_addr='aa:bb:cc:dd:ee:ff') + mock_exec.assert_called_once_with('ip', 'link', 'set', 'device', 'up', + run_as_root=True) + + @mock.patch('netifaces.ifaddresses') + @mock.patch('netifaces.interfaces') + def test_get_trunk_dev_name(self, mock_interfaces, mock_ifaddresses): + trunk_w = mock.Mock(mac='01234567890A') + + mock_link_addrs1 = { + netifaces.AF_LINK: [{'addr': '00:11:22:33:44:55'}, + {'addr': '00:11:22:33:44:66'}]} + mock_link_addrs2 = { + netifaces.AF_LINK: [{'addr': '00:11:22:33:44:77'}, + {'addr': '01:23:45:67:89:0a'}]} + + mock_interfaces.return_value = ['a', 'b'] + mock_ifaddresses.side_effect = [mock_link_addrs1, mock_link_addrs2] + + # The mock_link_addrs2 (or interface b) should be the match + self.assertEqual('b', self.drv.get_trunk_dev_name(trunk_w)) + + # If you take out the correct adapter, make sure it fails. + mock_interfaces.return_value = ['a'] + mock_ifaddresses.side_effect = [mock_link_addrs1] + self.assertRaises(exception.VirtualInterfacePlugException, + self.drv.get_trunk_dev_name, trunk_w) + + @mock.patch('pypowervm.tasks.cna.find_trunks') + @mock.patch('nova.network.linux_net.delete_ovs_vif_port') + @mock.patch('nova_powervm.virt.powervm.vif.PvmOvsVifDriver.' + 'get_trunk_dev_name') + @mock.patch('nova_powervm.virt.powervm.vif.PvmOvsVifDriver.' + '_find_cna_for_vif') + @mock.patch('nova_powervm.virt.powervm.vm.get_cnas') + def test_unplug(self, mock_get_cnas, mock_find_cna, mock_trunk_dev_name, + mock_del_ovs_port, mock_find_trunks): + # Set up the mocks + mock_cna = mock.Mock() + mock_get_cnas.return_value = [mock_cna, mock.Mock()] + mock_find_cna.return_value = mock_cna + + t1, t2 = mock.MagicMock(), mock.MagicMock() + mock_find_trunks.return_value = [t1, t2] + + mock_trunk_dev_name.return_value = 'fake_dev' + + # Call the unplug + vif = {'address': 'aa:bb:cc:dd:ee:ff', 'network': {'bridge': 'br-int'}} + self.drv.unplug(vif) + + # The trunks and the cna should have been deleted + self.assertTrue(t1.delete.called) + self.assertTrue(t2.delete.called) + self.assertTrue(mock_cna.delete.called) + + # Validate the OVS port delete call was made + mock_del_ovs_port.assert_called_with('br-int', 'fake_dev') diff --git a/nova_powervm/virt/powervm/driver.py b/nova_powervm/virt/powervm/driver.py index 1eaaeadf..5dac2bfc 100644 --- a/nova_powervm/virt/powervm/driver.py +++ b/nova_powervm/virt/powervm/driver.py @@ -459,7 +459,7 @@ class PowerVMDriver(driver.ComputeDriver): return False def _destroy(self, context, instance, block_device_info=None, - destroy_disks=True, shutdown=True): + network_info=None, destroy_disks=True, shutdown=True): """Internal destroy method used by multiple operations. @@ -472,6 +472,8 @@ class PowerVMDriver(driver.ComputeDriver): case, the storage mappings have already been removed from the original VM, so no work to do. + :param network_info: The network information associated with the + instance :param destroy_disks: Indicates if disks should be destroyed :param shutdown: Indicate whether to shutdown the VM first """ @@ -495,6 +497,12 @@ class PowerVMDriver(driver.ComputeDriver): stg_ftsk = vios.build_tx_feed_task(self.adapter, self.host_uuid, xag=xag) + # Call the unplug VIFs task. While CNAs get removed from the LPAR + # directly on the destroy, this clears up the I/O Host side. + flow.add(tf_vm.Get(self.adapter, self.host_uuid, instance)) + flow.add(tf_net.UnplugVifs(self.adapter, instance, network_info, + self.host_uuid)) + # Add the disconnect/deletion of the vOpt to the transaction # manager. flow.add(tf_stg.DeleteVOpt(self.adapter, self.host_uuid, instance, @@ -599,8 +607,10 @@ class PowerVMDriver(driver.ComputeDriver): # Run the destroy self._log_operation('destroy', instance) - self._destroy(context, instance, block_device_info=block_device_info, - destroy_disks=destroy_disks, shutdown=True) + self._destroy( + context, instance, block_device_info=block_device_info, + network_info=network_info, destroy_disks=destroy_disks, + shutdown=True) def attach_volume(self, context, connection_info, instance, mountpoint, disk_bus=None, device_type=None, encryption=None): diff --git a/nova_powervm/virt/powervm/tasks/network.py b/nova_powervm/virt/powervm/tasks/network.py index 2d0e50e9..6b50aca3 100644 --- a/nova_powervm/virt/powervm/tasks/network.py +++ b/nova_powervm/virt/powervm/tasks/network.py @@ -57,7 +57,7 @@ class UnplugVifs(task.Task): """ self.adapter = adapter self.instance = instance - self.network_infos = network_infos + self.network_infos = network_infos or [] self.host_uuid = host_uuid super(UnplugVifs, self).__init__(name='unplug_vifs', diff --git a/nova_powervm/virt/powervm/vif.py b/nova_powervm/virt/powervm/vif.py index d9482984..315afb18 100644 --- a/nova_powervm/virt/powervm/vif.py +++ b/nova_powervm/virt/powervm/vif.py @@ -16,9 +16,13 @@ import abc import logging +import netifaces import six from nova import exception +from nova.network import linux_net +from nova import utils +from oslo_config import cfg from oslo_utils import importutils from pypowervm.tasks import cna as pvm_cna from pypowervm.wrappers import managed_system as pvm_ms @@ -28,6 +32,7 @@ from nova_powervm.virt.powervm.i18n import _ from nova_powervm.virt.powervm.i18n import _LE from nova_powervm.virt.powervm.i18n import _LI from nova_powervm.virt.powervm.i18n import _LW +from nova_powervm.virt.powervm import mgmt from nova_powervm.virt.powervm import vm LOG = logging.getLogger(__name__) @@ -35,7 +40,10 @@ LOG = logging.getLogger(__name__) SECURE_RMC_VSWITCH = 'MGMTSWITCH' SECURE_RMC_VLAN = 4094 -VIF_MAPPING = {'pvm_sea': 'nova_powervm.virt.powervm.vif.PvmSeaVifDriver'} +VIF_MAPPING = {'pvm_sea': 'nova_powervm.virt.powervm.vif.PvmSeaVifDriver', + 'ovs': 'nova_powervm.virt.powervm.vif.PvmOvsVifDriver'} + +CONF = cfg.CONF class VirtualInterfaceUnplugException(exception.NovaException): @@ -56,8 +64,8 @@ def _build_vif_driver(adapter, host_uuid, instance, vif): """ if vif.get('type') is None: raise exception.VirtualInterfacePlugException( - _("vif_type parameter must be present " - "for this vif_driver implementation")) + _("vif_type parameter must be present for this vif_driver " + "implementation")) # Check the type to the implementations if VIF_MAPPING.get(vif['type']): @@ -171,30 +179,31 @@ class PvmVifDriver(object): cna_w_list = vm.get_cnas(self.adapter, self.instance, self.host_uuid) - for cna_w in cna_w_list: - # If the MAC address matched, attempt the delete. - if vm.norm_mac(cna_w.mac) == vif['address']: - LOG.info(_LI('Deleting VIF with mac %(mac)s for instance ' - '%(inst)s.'), - {'mac': vif['address'], 'inst': self.instance.name}) - try: - cna_w.delete() - except Exception as e: - LOG.error(_LE('Unable to unplug VIF with mac %(mac)s ' - 'for instance %(inst)s.'), - {'mac': vif['address'], - 'inst': self.instance.name}) - LOG.exception(e) - raise VirtualInterfaceUnplugException() - - # Break from the loop as we had a successful unplug. - # This prevents from going to 'else' loop. - break - else: + cna_w = self._find_cna_for_vif(cna_w_list, vif) + if not cna_w: LOG.warning(_LW('Unable to unplug VIF with mac %(mac)s for ' 'instance %(inst)s. The VIF was not found on ' 'the instance.'), {'mac': vif['address'], 'inst': self.instance.name}) + return + + LOG.info(_LI('Deleting VIF with mac %(mac)s for instance %(inst)s.'), + {'mac': vif['address'], 'inst': self.instance.name}) + try: + cna_w.delete() + except Exception as e: + LOG.error(_LE('Unable to unplug VIF with mac %(mac)s for instance ' + '%(inst)s.'), {'mac': vif['address'], + 'inst': self.instance.name}) + LOG.exception(e) + raise VirtualInterfaceUnplugException() + + def _find_cna_for_vif(self, cna_w_list, vif): + for cna_w in cna_w_list: + # If the MAC address matched, attempt the delete. + if vm.norm_mac(cna_w.mac) == vif['address']: + return cna_w + return None class PvmSeaVifDriver(PvmVifDriver): @@ -206,3 +215,72 @@ class PvmSeaVifDriver(PvmVifDriver): vlan = vif['network']['meta'].get('vlan', 1) return pvm_cna.crt_cna(self.adapter, self.host_uuid, lpar_uuid, vlan, mac_addr=vif['address']) + + +class PvmOvsVifDriver(PvmVifDriver): + """The Open vSwitch VIF driver for PowerVM.""" + + def plug(self, vif): + # Create the trunk and client adapter. + lpar_uuid = vm.get_pvm_uuid(self.instance) + mgmt_uuid = mgmt.get_mgmt_partition(self.adapter).uuid + cna_w, trunk_wraps = pvm_cna.crt_p2p_cna( + self.adapter, self.host_uuid, lpar_uuid, [mgmt_uuid], + CONF.powervm.pvm_vswitch_for_ovs, crt_vswitch=True, + mac_addr=vif['address']) + + # There will only be one trunk wrap, as we have created with just the + # mgmt lpar. Next step is to set the device up and connect to the OVS + dev = self.get_trunk_dev_name(trunk_wraps[0]) + utils.execute('ip', 'link', 'set', dev, 'up', run_as_root=True) + linux_net.create_ovs_vif_port(vif['network']['bridge'], dev, + self.get_ovs_interfaceid(vif), + vif['address'], self.instance.uuid) + + def get_ovs_interfaceid(self, vif): + return vif.get('ovs_interfaceid') or vif['id'] + + def get_trunk_dev_name(self, trunk_w): + # The mac address from the API is of format: 01234567890A + # We need it in format: 01:23:45:67:89:0a + # That means we need to add colons and lower case it + mac_addr = ":".join(trunk_w.mac[i:i + 2] + for i in range(0, len(trunk_w.mac), 2)).lower() + + # Use netifaces to find the appropriate matching interface name + # TODO(thorst) I don't like this logic. Seems gross. + ifaces = netifaces.interfaces() + for iface in ifaces: + link_addrs = netifaces.ifaddresses(iface)[netifaces.AF_LINK] + for link_addr in link_addrs: + if link_addr.get('addr') == mac_addr: + return iface + + raise exception.VirtualInterfacePlugException( + _("Unable to find appropriate Trunk Device for mac " + "%(mac_addr)s.") % {'mac_addr': mac_addr}) + + def unplug(self, vif, cna_w_list=None): + # Need to find the adapters if they were not provided + if not cna_w_list: + cna_w_list = vm.get_cnas(self.adapter, self.instance, + self.host_uuid) + + # Find the CNA for this vif. + cna_w = self._find_cna_for_vif(cna_w_list, vif) + if not cna_w: + LOG.warning(_LW('Unable to unplug VIF with mac %(mac)s for ' + 'instance %(inst)s. The VIF was not found on ' + 'the instance.'), + {'mac': vif['address'], 'inst': self.instance.name}) + return + + # Find and delete the trunk adapters + trunks = pvm_cna.find_trunks(self.adapter, cna_w) + for trunk in trunks: + dev = self.get_trunk_dev_name(trunk) + linux_net.delete_ovs_vif_port(vif['network']['bridge'], dev) + trunk.delete() + + # Now delete the client CNA + super(PvmOvsVifDriver, self).unplug(vif, cna_w_list=cna_w_list) diff --git a/requirements.txt b/requirements.txt index 0b8e761d..2f109c88 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,6 @@ pbr>=1.6 Babel>=1.3 +netifaces>=0.10.4 # MIT six>=1.9.0 oslo.config>=3.4.0 # Apache-2.0 oslo.log>=1.14.0 # Apache-2.0