Sanitize PowerVM partition name

The restrictions on PowerVM partition names are more stringent than
those on nova instance names.  To avoid annoying API errors, if a name
is accepted by nova, we scrub it (using a provided pypowervm interface)
such that it becomes acceptable for PowerVM.

Related-Bug: #1535338

Change-Id: Ic494f4ec05d9f6b1b2c93843ad404c6bee0bfba8

nova_powervm/tests/virt/powervm/test_driver.py

@@ -15,6 +15,9 @@
 #    under the License.
 #
 
+from __future__ import absolute_import
+
+import fixtures
 import logging
 import mock
 from oslo_serialization import jsonutils
@@ -84,33 +87,31 @@ class TestPowerVMDriver(test.TestCase):
         self.vol_fix = self.useFixture(fx.VolumeAdapter())
         self.vol_drv = self.vol_fix.drv
 
-        self.crt_lpar_p = mock.patch('nova_powervm.virt.powervm.vm.crt_lpar')
+        self.crt_lpar = self.useFixture(fixtures.MockPatch(
-        self.crt_lpar = self.crt_lpar_p.start()
+            'nova_powervm.virt.powervm.vm.crt_lpar')).mock
-        self.addCleanup(self.crt_lpar_p.stop)
 
-        self.get_inst_wrap_p = mock.patch('nova_powervm.virt.powervm.vm.'
+        self.get_inst_wrap = self.useFixture(fixtures.MockPatch(
-                                          'get_instance_wrapper')
+            'nova_powervm.virt.powervm.vm.get_instance_wrapper')).mock
-        self.get_inst_wrap = self.get_inst_wrap_p.start()
-        self.addCleanup(self.get_inst_wrap_p.stop)
 
         wrap = pvm_lpar.LPAR.wrap(pvmhttp.load_pvm_resp(
             LPAR_HTTPRESP_FILE).response)[0]
         self.crt_lpar.return_value = wrap
         self.get_inst_wrap.return_value = wrap
 
-        self.build_tx_feed_p = mock.patch('nova_powervm.virt.powervm.vios.'
+        self.build_tx_feed = self.useFixture(fixtures.MockPatch(
-                                          'build_tx_feed_task')
+            'nova_powervm.virt.powervm.vios.build_tx_feed_task')).mock
-        self.build_tx_feed = self.build_tx_feed_p.start()
+
-        self.addCleanup(self.build_tx_feed_p.stop)
         self.useFixture(pvm_fx.FeedTaskFx([pvm_vios.VIOS.wrap(
             pvmhttp.load_pvm_resp(VIOS_HTTPRESP_FILE).response)]))
         self.stg_ftsk = pvm_tx.FeedTask('fake', pvm_vios.VIOS.getter(self.apt))
         self.build_tx_feed.return_value = self.stg_ftsk
 
-        scrub_stg_p = mock.patch('pypowervm.tasks.storage.'
+        self.scrub_stg = self.useFixture(fixtures.MockPatch(
-                                 'add_lpar_storage_scrub_tasks')
+            'pypowervm.tasks.storage.add_lpar_storage_scrub_tasks')).mock
-        self.scrub_stg = scrub_stg_p.start()
+
-        self.addCleanup(scrub_stg_p.stop)
+        self.san_lpar_name = self.useFixture(fixtures.MockPatch(
+            'pypowervm.util.sanitize_partition_name_for_api')).mock
+        self.san_lpar_name.side_effect = lambda name: name
 
         # Create an instance to test with
         self.inst = objects.Instance(**powervm.TEST_INST_SPAWNING)
@@ -912,19 +913,22 @@ class TestPowerVMDriver(test.TestCase):
         mock_dst_int.assert_called_with(
             'context', self.inst, block_device_info=mock_bdms,
             destroy_disks=True, shutdown=True)
+        self.san_lpar_name.assert_not_called()
 
         # Test delete during migrate / resize
         self.inst.task_state = task_states.RESIZE_REVERTING
-        mock_getqp.return_value = ('resize_' + self.inst.name)[:31]
+        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(),
                              block_device_info=mock_bdms)
         # We shouldn't delete our resize_ instances
         mock_dst_int.assert_not_called()
+        self.san_lpar_name.assert_called_with('resize_' + self.inst.name)
+        self.san_lpar_name.reset_mock()
 
         # Now test migrating...
-        mock_getqp.return_value = ('migrate_' + self.inst.name)[:31]
+        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(),
@@ -1044,23 +1048,25 @@ class TestPowerVMDriver(test.TestCase):
 
         # Boot disk resize
         boot_flav = objects.Flavor(vcpus=1, memory_mb=2048, root_gb=12)
-        # Tasks expected to be added for resize to the same host
+        # Tasks expected to be added for migrate
         expected = [
             'pwr_off_lpar',
             'extend_disk_boot',
             'disconnect_vol_*',
             'disconnect_vol_*',
             'fake',
-            'rename_lpar_resize_instance-00000001',
+            'rename_lpar_migrate_instance-00000001',
         ]
+        dest_host = host + '1'
         with fx.DriverTaskFlow() as taskflow_fix:
             self.drv.migrate_disk_and_power_off(
-                'context', self.inst, host, boot_flav, 'network_info',
+                'context', self.inst, dest_host, boot_flav, 'network_info',
                 mock_bdms)
             taskflow_fix.assert_tasks_added(self, expected)
             # Check the size set in the resize task
             extend_task = taskflow_fix.tasks_added[1]
             self.assertEqual(extend_task.size, 12)
+        self.san_lpar_name.assert_called_with('migrate_' + self.inst.name)
 
     @mock.patch('nova.objects.flavor.Flavor.get_by_id')
     def test_finish_migration(self, mock_get_flv):
@@ -1098,6 +1104,7 @@ class TestPowerVMDriver(test.TestCase):
                 'context', mig, self.inst, disk_info, 'network_info',
                 powervm.IMAGE1, 'resize_instance', block_device_info=mock_bdms)
             taskflow_fix.assert_tasks_added(self, expected)
+        self.san_lpar_name.assert_not_called()
 
         # Tasks expected to be added for resize to the same host
         expected = [
@@ -1115,6 +1122,8 @@ class TestPowerVMDriver(test.TestCase):
                 'context', mig_same_host, self.inst, disk_info, 'network_info',
                 powervm.IMAGE1, 'resize_instance', block_device_info=mock_bdms)
             taskflow_fix.assert_tasks_added(self, expected)
+        self.san_lpar_name.assert_called_with('resize_' + self.inst.name)
+        self.san_lpar_name.reset_mock()
 
         # Tasks expected to be added for resize to the same host, no BDMS,
         # and no power_on
@@ -1126,6 +1135,7 @@ class TestPowerVMDriver(test.TestCase):
                 'context', mig_same_host, self.inst, disk_info, 'network_info',
                 powervm.IMAGE1, 'resize_instance', power_on=False)
             taskflow_fix.assert_tasks_added(self, expected)
+        self.san_lpar_name.assert_called_with('resize_' + self.inst.name)
 
     @mock.patch('nova_powervm.virt.powervm.driver.vm')
     @mock.patch('nova_powervm.virt.powervm.tasks.vm.vm')

nova_powervm/tests/virt/powervm/test_vm.py

@@ -15,6 +15,9 @@
 #    under the License.
 #
 
+from __future__ import absolute_import
+
+import fixtures
 import logging
 import mock
 
@@ -59,6 +62,10 @@ class TestVMBuilder(test.TestCase):
         self.host_w = mock.MagicMock()
         self.lpar_b = vm.VMBuilder(self.host_w, self.adpt)
 
+        self.san_lpar_name = self.useFixture(fixtures.MockPatch(
+            'pypowervm.util.sanitize_partition_name_for_api')).mock
+        self.san_lpar_name.side_effect = lambda name: name
+
     def test_conf_values(self):
         # Test driver CONF values are passed to the standardizer
         self.flags(uncapped_proc_weight=75, proc_units_factor=.25,
@@ -82,6 +89,8 @@ class TestVMBuilder(test.TestCase):
 
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         # Test dedicated procs, min/max vcpu and sharing mode
         flavor.extra_specs = {'powervm:dedicated_proc': 'true',
@@ -95,24 +104,32 @@ class TestVMBuilder(test.TestCase):
                              'min_vcpu': '1', 'max_vcpu': '3'})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         # Test shared proc sharing mode
         flavor.extra_specs = {'powervm:uncapped': 'true'}
         test_attrs = dict(lpar_attrs, **{'sharing_mode': 'uncapped'})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         # Test availability priority
         flavor.extra_specs = {'powervm:availability_priority': '150'}
         test_attrs = dict(lpar_attrs, **{'avail_priority': '150'})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         # Test processor compatibility
         flavor.extra_specs = {'powervm:processor_compatibility': 'POWER8'}
         test_attrs = dict(lpar_attrs, **{'processor_compatibility': 'POWER8'})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         flavor.extra_specs = {'powervm:processor_compatibility': 'POWER6+'}
         test_attrs = dict(
@@ -120,6 +137,8 @@ class TestVMBuilder(test.TestCase):
             **{'processor_compatibility': pvm_bp.LPARCompat.POWER6_PLUS})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         flavor.extra_specs = {'powervm:processor_compatibility':
                               'POWER6+_Enhanced'}
@@ -128,6 +147,8 @@ class TestVMBuilder(test.TestCase):
                            pvm_bp.LPARCompat.POWER6_PLUS_ENHANCED})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         # Test min, max proc units
         flavor.extra_specs = {'powervm:min_proc_units': '0.5',
@@ -136,6 +157,8 @@ class TestVMBuilder(test.TestCase):
                                          'max_proc_units': '2.0'})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
+        self.san_lpar_name.reset_mock()
 
         # Test min, max mem
         flavor.extra_specs = {'powervm:min_mem': '1024',
@@ -143,6 +166,7 @@ class TestVMBuilder(test.TestCase):
         test_attrs = dict(lpar_attrs, **{'min_mem': '1024', 'max_mem': '4096'})
         self.assertEqual(self.lpar_b._format_flavor(instance, flavor),
                          test_attrs)
+        self.san_lpar_name.assert_called_with(instance.name)
 
     @mock.patch('pypowervm.wrappers.shared_proc_pool.SharedProcPool.search')
     def test_spp_pool_id(self, mock_search):
@@ -188,6 +212,10 @@ class TestVM(test.TestCase):
             traits=pvm_fx.LocalPVMTraits)).adpt
         self.apt.helpers = [pvm_log.log_helper]
 
+        self.san_lpar_name = self.useFixture(fixtures.MockPatch(
+            'pypowervm.util.sanitize_partition_name_for_api')).mock
+        self.san_lpar_name.side_effect = lambda name: name
+
         lpar_http = pvmhttp.load_pvm_resp(LPAR_HTTPRESP_FILE, adapter=self.apt)
         self.assertNotEqual(lpar_http, None,
                             "Could not load %s " %
@@ -341,6 +369,7 @@ class TestVM(test.TestCase):
         entry.update.assert_called_once_with()
         self.assertEqual(name, entry.name)
         self.assertEqual('NewEntry', new_entry)
+        self.san_lpar_name.assert_called_with(name)
 
     @mock.patch('nova_powervm.virt.powervm.vm.get_instance_wrapper')
     def test_rename(self, mock_get_inst):
@@ -353,6 +382,8 @@ class TestVM(test.TestCase):
         self.assertEqual('new_name', entry.name)
         entry.update.assert_called_once_with()
         self.assertEqual('NewEntry', new_entry)
+        self.san_lpar_name.assert_called_with('new_name')
+        self.san_lpar_name.reset_mock()
 
         # Test optional entry parameter
         entry.reset_mock()
@@ -363,6 +394,7 @@ class TestVM(test.TestCase):
         self.assertEqual('new_name', entry.name)
         entry.update.assert_called_once_with()
         self.assertEqual('NewEntry', new_entry)
+        self.san_lpar_name.assert_called_with('new_name')
 
     def test_add_IBMi_attrs(self):
         inst = mock.Mock()

nova_powervm/virt/powervm/driver.py

@@ -578,7 +578,7 @@ class PowerVMDriver(driver.ComputeDriver):
         :param migrate_data: implementation specific params
         """
         if instance.task_state == task_states.RESIZE_REVERTING:
-            LOG.info(_LI('Destroy called for migrated instance.'),
+            LOG.info(_LI('Destroy called for migrated/resized instance.'),
                      instance=instance)
             # This destroy is part of resize or migrate.  It's called to
             # revert the resize/migration on the destination host.
@@ -1074,9 +1074,18 @@ class PowerVMDriver(driver.ComputeDriver):
 
         return disk_info
 
-    def _gen_resize_name(self, instance, same_host=False):
+    @staticmethod
+    def _gen_resize_name(instance, same_host=False):
+        """Generate a temporary name for the source VM being resized/migrated.
+
+        :param instance: nova.objects.instance.Instance being migrated/resized.
+        :param same_host: Boolean indicating whether this resize is being
+                          performed for the sake of a resize (True) or a
+                          migration (False).
+        :return: A new name which can be assigned to the source VM.
+        """
         prefix = 'resize_' if same_host else 'migrate_'
-        return (prefix + instance.name)[:31]
+        return pvm_util.sanitize_partition_name_for_api(prefix + instance.name)
 
     def finish_migration(self, context, migration, instance, disk_info,
                          network_info, image_meta, resize_instance,

nova_powervm/virt/powervm/vm.py

@@ -30,6 +30,7 @@ from pypowervm.tasks import cna
 from pypowervm.tasks import ibmi
 from pypowervm.tasks import power
 from pypowervm.tasks import vterm
+from pypowervm import util as pvm_util
 from pypowervm.utils import lpar_builder as lpar_bldr
 from pypowervm.utils import uuid as pvm_uuid
 from pypowervm.utils import validation as vldn
@@ -294,7 +295,8 @@ class VMBuilder(object):
         # The attrs are what is sent to pypowervm to convert the lpar.
         attrs = {}
 
-        attrs[lpar_bldr.NAME] = instance.name
+        attrs[lpar_bldr.NAME] = pvm_util.sanitize_partition_name_for_api(
+            instance.name)
         # The uuid is only actually set on a create of an LPAR
         attrs[lpar_bldr.UUID] = pvm_uuid.convert_uuid_to_pvm(instance.uuid)
         attrs[lpar_bldr.MEM] = flavor.memory_mb
@@ -560,7 +562,7 @@ def update(adapter, host_wrapper, instance, flavor, entry=None, name=None):
 
     # Set the new name if the instance name is not desired.
     if name:
-        entry.name = name
+        entry.name = pvm_util.sanitize_partition_name_for_api(name)
     # Write out the new specs, return the updated version
     return entry.update()
 
@@ -580,7 +582,7 @@ def rename(adapter, host_uuid, instance, name, entry=None):
     if not entry:
         entry = get_instance_wrapper(adapter, instance, host_uuid)
 
-    entry.name = name
+    entry.name = pvm_util.sanitize_partition_name_for_api(name)
     return entry.update()