Move LPAR scrub tasks from build_map to Create task

In response to https://review.openstack.org/#/c/324730/3 we need to
be able to create the build_map sooner in spawn flow, before the
LPAR is created. To do this we need to move the LPAR scrub out of
the build_map initialization and into the Create task right after
the LPAR is created.

Partial-Bug: #1587942
Change-Id: Ie3879836c593703b215c54c67abeb358ea0bda82

nova_powervm/tests/virt/powervm/tasks/test_vm.py

@@ -22,6 +22,7 @@ from nova import test
 
 from nova_powervm.virt.powervm.tasks import vm as tf_vm
 
+from pypowervm import const as pvmc
 from taskflow import engines as tf_eng
 from taskflow.patterns import linear_flow as tf_lf
 
@@ -32,25 +33,41 @@ class TestVMTasks(test.TestCase):
         self.apt = mock.Mock()
         self.instance = mock.Mock()
 
+    @mock.patch('pypowervm.utils.transaction.FeedTask')
+    @mock.patch('pypowervm.tasks.partition.build_active_vio_feed_task')
     @mock.patch('pypowervm.tasks.storage.add_lpar_storage_scrub_tasks')
     @mock.patch('nova_powervm.virt.powervm.vm.crt_lpar')
-    def test_create(self, mock_vm_crt, mock_stg):
+    def test_create(self, mock_vm_crt, mock_stg, mock_bld, mock_ftsk):
         nvram_mgr = mock.Mock()
         nvram_mgr.fetch.return_value = 'data'
         lpar_entry = mock.Mock()
 
+        # Test create with normal (non-recreate) ftsk
         crt = tf_vm.Create(self.apt, 'host_wrapper', self.instance,
-                           'flavor', 'stg_ftsk', nvram_mgr=nvram_mgr,
+                           'flavor', mock_ftsk, nvram_mgr=nvram_mgr,
                            slot_mgr='slot_mgr')
         mock_vm_crt.return_value = lpar_entry
         crt_entry = crt.execute()
 
+        mock_ftsk.execute.assert_not_called()
         mock_vm_crt.assert_called_once_with(self.apt, 'host_wrapper',
                                             self.instance, 'flavor',
                                             nvram='data', slot_mgr='slot_mgr')
         self.assertEqual(lpar_entry, crt_entry)
         nvram_mgr.fetch.assert_called_once_with(self.instance)
 
+        mock_ftsk.name = 'create_scrubber'
+        mock_bld.return_value = mock_ftsk
+        # Test create with recreate ftsk
+        rcrt = tf_vm.Create(self.apt, 'host_wrapper', self.instance,
+                            'flavor', None, nvram_mgr=nvram_mgr,
+                            slot_mgr='slot_mgr')
+        mock_bld.assert_called_once_with(
+            self.apt, name='create_scrubber',
+            xag={pvmc.XAG.VIO_SMAP, pvmc.XAG.VIO_FMAP})
+        rcrt.execute()
+        mock_ftsk.execute.assert_called_once_with()
+
     @mock.patch('nova_powervm.virt.powervm.vm.get_pvm_uuid')
     @mock.patch('nova_powervm.virt.powervm.tasks.vm.Create.execute')
     @mock.patch('nova_powervm.virt.powervm.vm.dlt_lpar')

nova_powervm/tests/virt/powervm/test_driver.py

@@ -560,12 +560,14 @@ class TestPowerVMDriver(test.TestCase):
         # Invoke the method.
         self.drv.spawn('context', self.inst, powervm.EMPTY_IMAGE,
                        'injected_files', 'admin_password')
-
-        # Recreate uses all XAGs.
-        self.build_tx_feed.assert_called_once_with(
-            self.drv.adapter, xag={pvm_const.XAG.VIO_FMAP,
-                                   pvm_const.XAG.VIO_STOR,
-                                   pvm_const.XAG.VIO_SMAP})
+        xags = {pvm_const.XAG.VIO_FMAP, pvm_const.XAG.VIO_SMAP,
+                pvm_const.XAG.VIO_STOR}
+        calls = [mock.call(self.drv.adapter, xag=xags),
+                 mock.call(self.drv.adapter, name='create_scrubber',
+                           xag=xags - {pvm_const.XAG.VIO_STOR})]
+        # Recreate uses all XAGs, builds special FeedTask for immediate
+        # scrubbing.
+        self.build_tx_feed.assert_has_calls(calls)
         # _vol_drv_iter gets called once in spawn itself, and once under
         # _add_volume_connection_tasks.
         # TODO(IBM): Find a way to make the call just once.  Unless it's cheap.

nova_powervm/tests/virt/powervm/test_slot.py

@@ -101,28 +101,16 @@ class TestSwiftSlotManager(test.TestCase):
             self.inst.uuid + '_slot_map')
 
     @mock.patch('pypowervm.tasks.slot_map.RebuildSlotMap')
-    @mock.patch('nova_powervm.virt.powervm.vm.get_vm_id')
-    @mock.patch('pypowervm.tasks.storage.add_lpar_storage_scrub_tasks')
     @mock.patch('pypowervm.tasks.storage.ComprehensiveScrub')
-    def test_init_recreate_map(self, mock_ftsk, mock_alsst, mock_vm_id,
-                               mock_rebuild_slot):
+    def test_init_recreate_map(self, mock_ftsk, mock_rebuild_slot):
         vios1, vios2 = mock.Mock(uuid='uuid1'), mock.Mock(uuid='uuid2')
         mock_ftsk.return_value.feed = [vios1, vios2]
         self.slot_mgr.init_recreate_map(mock.Mock(), self._vol_drv_iter())
-        # get_vm_id called with converted UUID
-        mock_vm_id.assert_called_once_with(
-            mock.ANY, '22E71B38-160F-4650-BBDC-2A10CD507E2B')
         self.assertEqual(1, mock_ftsk.call_count)
-        mock_alsst.assert_called_once_with(
-            [mock_vm_id.return_value], mock_ftsk.return_value,
-            lpars_exist=True)
         mock_rebuild_slot.assert_called_once_with(
             self.slot_mgr, mock.ANY, {'udid': ['uuid2']}, ['a', 'b'])
 
     @mock.patch('pypowervm.tasks.slot_map.RebuildSlotMap')
-    @mock.patch('nova_powervm.virt.powervm.vm.get_vm_id', new=mock.Mock())
-    @mock.patch('pypowervm.tasks.storage.add_lpar_storage_scrub_tasks',
-                new=mock.Mock())
     @mock.patch('pypowervm.tasks.storage.ComprehensiveScrub')
     def test_init_recreate_map_fails(self, mock_ftsk, mock_rebuild_slot):
         vios1, vios2 = mock.Mock(uuid='uuid1'), mock.Mock(uuid='uuid2')

nova_powervm/virt/powervm/driver.py

@@ -404,9 +404,14 @@ class PowerVMDriver(driver.ComputeDriver):
         nvram_mgr = (self.nvram_mgr if self.nvram_mgr and
                      (recreate or instance.vm_state in FETCH_NVRAM_STATES)
                      else None)
-        flow_spawn.add(tf_vm.Create(self.adapter, self.host_wrapper, instance,
-                                    flavor, stg_ftsk, nvram_mgr=nvram_mgr,
-                                    slot_mgr=slot_mgr))
+
+        # If we're recreating pass None in for the FeedTask. This will make the
+        # Create task produce a FeedTask that will be used to scrub stale
+        # adapters immediately after the LPAR is created.
+        flow_spawn.add(tf_vm.Create(
+            self.adapter, self.host_wrapper, instance, flavor,
+            stg_ftsk=(None if recreate else stg_ftsk), nvram_mgr=nvram_mgr,
+            slot_mgr=slot_mgr))
 
         # Create a flow for the IO
         flow_spawn.add(tf_net.PlugVifs(

nova_powervm/virt/powervm/slot.py

@@ -24,7 +24,6 @@ from pypowervm.tasks import storage as pvm_tstor
 
 from nova_powervm.virt.powervm import exception as p_exc
 from nova_powervm.virt.powervm.i18n import _LW
-from nova_powervm.virt.powervm import vm
 
 LOG = logging.getLogger(__name__)
 
@@ -119,9 +118,6 @@ class NovaSlotManager(slot_map.SlotMapStore):
         # we're creating.  It is critical that this happen *before* we create
         # any of the mappings we actually want this VM to have.
         scrub_ftsk = pvm_tstor.ComprehensiveScrub(adapter)
-        lpar_id = vm.get_vm_id(adapter, vm.get_pvm_uuid(self.instance))
-        pvm_tstor.add_lpar_storage_scrub_tasks([lpar_id], scrub_ftsk,
-                                               lpars_exist=True)
         scrub_ftsk.execute()
         self._vios_wraps = scrub_ftsk.feed
 

nova_powervm/virt/powervm/tasks/vm.py

@@ -15,6 +15,8 @@
 #    under the License.
 
 from oslo_log import log as logging
+from pypowervm import const as pvm_const
+from pypowervm.tasks import partition as pvm_tpar
 from pypowervm.tasks import power
 from pypowervm.tasks import storage as pvm_stg
 import six
@@ -56,7 +58,7 @@ class Create(pvm_task.PowerVMTask):
 
     """The task for creating a VM."""
 
-    def __init__(self, adapter, host_wrapper, instance, flavor, stg_ftsk,
+    def __init__(self, adapter, host_wrapper, instance, flavor, stg_ftsk=None,
                  nvram_mgr=None, slot_mgr=None):
         """Creates the Task for creating a VM.
 
@@ -79,7 +81,10 @@ class Create(pvm_task.PowerVMTask):
         :param host_wrapper: The managed system wrapper
         :param instance: The nova instance.
         :param flavor: The nova flavor.
-        :param stg_ftsk: A FeedTask managing storage I/O operations.
+        :param stg_ftsk: (Optional, Default: None) A FeedTask managing storage
+                         I/O operations.  If None, one will be built locally
+                         and executed immediately. Otherwise it is the caller's
+                         responsibility to execute the FeedTask.
         :param nvram_mgr: The NVRAM manager to fetch the NVRAM from. If None,
                           the NVRAM will not be fetched.
         :param slot_mgr: A NovaSlotManager.  Used to store/retrieve the
@@ -90,7 +95,9 @@ class Create(pvm_task.PowerVMTask):
         self.adapter = adapter
         self.host_wrapper = host_wrapper
         self.flavor = flavor
-        self.stg_ftsk = stg_ftsk
+        self.stg_ftsk = stg_ftsk or pvm_tpar.build_active_vio_feed_task(
+            adapter, name='create_scrubber',
+            xag={pvm_const.XAG.VIO_SMAP, pvm_const.XAG.VIO_FMAP})
         self.nvram_mgr = nvram_mgr
         self.slot_mgr = slot_mgr
 
@@ -106,6 +113,18 @@ class Create(pvm_task.PowerVMTask):
                            self.flavor, nvram=data, slot_mgr=self.slot_mgr)
         pvm_stg.add_lpar_storage_scrub_tasks([wrap.id], self.stg_ftsk,
                                              lpars_exist=True)
+        # If the stg_ftsk passed in was None and we initialized a
+        # 'create_scrubber' stg_ftsk then run it immediately. We do
+        # this because we moved the LPAR storage scrub tasks out of the
+        # build_map initialization. This was so that we could construct the
+        # build map earlier in the spawn, just before the LPAR is created.
+        # Only rebuilds should be passing in None for stg_ftsk.
+        if self.stg_ftsk.name == 'create_scrubber':
+            LOG.info(_LI('Scrubbing storage for instance %s as part of '
+                         'rebuild.'), self.instance.name,
+                     instance=self.instance)
+            self.stg_ftsk.execute()
+
         return wrap
 
     def revert_impl(self, result, flow_failures, **kwargs):