Remove 'state' argument from later cmd_* calls

With I468dbf5134947629f125504513703d6f2cdace59 each node has a
reference to the global state object.  This means it gets pickled into
the node-list, which is loaded for later calls.  There is no need to
reload the state.json it and pass it for later cmd_* calls, as the
nodes can see it via the unpickled self.state

Change-Id: I9e2f8910f17599d92ee33e7df8e36d8ed4d44575

diskimage_builder/block_device/blockdevice.py

@@ -396,36 +396,34 @@ class BlockDevice(object):
     def cmd_umount(self):
         """Unmounts the blockdevice and cleanup resources"""
 
-        # State should have been created by prior calls; we only need
+        # If the state is not here, cmd_cleanup removed it?  Nothing
-        # the dict.  If it is not here, it has been cleaned up already
+        # more to do?
-        # (? more details?)
+        #  XXX: better understand this...
-        try:
+        if not os.path.exists(self.node_pickle_file_name):
-            state = BlockDeviceState(self.state_json_file_name)
-        except BlockDeviceSetupException:
             logger.info("State already cleaned - no way to do anything here")
             return 0
 
-        # Deleting must be done in reverse order
         call_order = pickle.load(open(self.node_pickle_file_name, 'rb'))
         reverse_order = reversed(call_order)
 
         for node in reverse_order:
-            node.umount(state)
+            node.umount()
 
         return 0
 
     def cmd_cleanup(self):
         """Cleanup all remaining relicts - in good case"""
-        # State should have been created by prior calls; we only need
-        # the dict
-        state = BlockDeviceState(self.state_json_file_name)
 
-        # Deleting must be done in reverse order
+        # Cleanup must be done in reverse order
-        call_order = pickle.load(open(self.node_pickle_file_name, 'rb'))
+        try:
+            call_order = pickle.load(open(self.node_pickle_file_name, 'rb'))
+        except IOError:
+            raise BlockDeviceSetupException("Pickle file not found")
+
         reverse_order = reversed(call_order)
 
         for node in reverse_order:
-            node.cleanup(state)
+            node.cleanup()
 
         logger.info("Removing temporary state dir [%s]", self.state_dir)
         shutil.rmtree(self.state_dir)
@@ -434,16 +432,16 @@ class BlockDevice(object):
 
     def cmd_delete(self):
         """Cleanup all remaining relicts - in case of an error"""
-        # State should have been created by prior calls; we only need
-        # the dict
-        state = BlockDeviceState(self.state_json_file_name)
 
         # Deleting must be done in reverse order
-        call_order = pickle.load(open(self.node_pickle_file_name, 'rb'))
+        try:
+            call_order = pickle.load(open(self.node_pickle_file_name, 'rb'))
+        except IOError:
+            raise BlockDeviceSetupException("Pickle file not found")
         reverse_order = reversed(call_order)
 
         for node in reverse_order:
-            node.delete(state)
+            node.delete()
 
         logger.info("Removing temporary state dir [%s]", self.state_dir)
         shutil.rmtree(self.state_dir)

diskimage_builder/block_device/level0/localloop.py

@@ -119,14 +119,11 @@ class LocalLoopNode(NodeBase):
                      self.name, block_device, self.filename)
         return
 
-    def umount(self, state):
+    def umount(self):
-        loopdev_detach(state['blockdev'][self.name]['device'])
+        loopdev_detach(self.state['blockdev'][self.name]['device'])
 
-    def cleanup(self, state):
+    def delete(self):
-        pass
+        image_delete(self.state['blockdev'][self.name]['image'])
-
-    def delete(self, state):
-        image_delete(state['blockdev'][self.name]['image'])
 
 
 class LocalLoop(PluginBase):

diskimage_builder/block_device/level3/mount.py

@@ -94,12 +94,12 @@ class MountPointNode(NodeBase):
             self.state['mount_order'] = []
         self.state['mount_order'].append(self.mount_point)
 
-    def umount(self, state):
+    def umount(self):
         logger.info("Called for [%s]", self.name)
-        exec_sudo(["umount", state['mount'][self.mount_point]['path']])
+        exec_sudo(["umount", self.state['mount'][self.mount_point]['path']])
 
-    def delete(self, state):
+    def delete(self):
-        self.umount(state)
+        self.umount()
 
 
 class Mount(PluginBase):

diskimage_builder/block_device/plugin.py

@@ -95,33 +95,29 @@ class NodeBase(object):
         """
         return
 
-    def umount(self, state):
+    def umount(self):
         """Umount actions
 
-        Actions to taken when ``dib-block-device umount`` is called
+        Actions to taken when ``dib-block-device umount`` is called.
+        The nodes are called in the reverse order to :func:`create`
 
-        :param state: the current state dictionary.  This is the
-          `state` dictionary from :func:`create` before this call is
-          made.
         :return: None
+
         """
         return
 
-    def cleanup(self, state):
+    def cleanup(self):
         """Cleanup actions
 
         Actions to taken when ``dib-block-device cleanup`` is called.
         This is the cleanup path in the *success* case.  The nodes are
         called in the reverse order to :func:`create`
 
-        :param state: the current state dictionary.  This is the
-          `state` dictionary from :func:`create` before this call is
-          made.
         :return: None
         """
         return
 
-    def delete(self, state):
+    def delete(self):
         """Cleanup actions
 
         Actions to taken when ``dib-block-device delete`` is called.
@@ -129,9 +125,6 @@ class NodeBase(object):
         *failure*.  The nodes are called in the reverse order to
         :func:`create`
 
-        :param state: the current state dictionary.  This is the
-          `state` dictionary from :func:`create` before this call is
-          made.
         :return: None
         """
         return

diskimage_builder/block_device/tests/plugin/test_a.py

@@ -40,9 +40,9 @@ class TestANode(NodeBase):
         self.state['test_a']['value2'] = 'bar'
         return
 
-    def umount(self, state):
+    def umount(self):
         # Umount is run in reverse.  This key should exist from test_b
-        state['umount'].append('test_a')
+        self.state['umount'].append('test_a')
 
 
 class TestA(PluginBase):

diskimage_builder/block_device/tests/plugin/test_b.py

@@ -38,11 +38,14 @@ class TestBNode(NodeBase):
         self.state['test_b']['value'] = 'baz'
         return
 
-    def umount(self, state):
+    def umount(self):
+        # these values should have persisteted from create()
+        assert self.state['test_b']['value'] == 'baz'
+
         # umount run in reverse.  this should run before test_a
-        assert 'umount' not in state
+        assert 'umount' not in self.state
-        state['umount'] = []
+        self.state['umount'] = []
-        state['umount'].append('test_b')
+        self.state['umount'].append('test_b')
 
 
 class TestB(PluginBase):

diskimage_builder/block_device/tests/test_state.py

@@ -80,7 +80,9 @@ class TestState(TestStateBase):
 
         # run umount, which should load the picked nodes and run in
         # reverse.  This will create some state in "test_b" that it
-        # added to by "test_a" ... ensuring it was run backwards.
+        # added to by "test_a" ... ensuring it was run backwards.  It
+        # also checks the state was persisted through the pickling
+        # process.
         bd_obj.cmd_umount()
 
     # Test state going missing between phases
@@ -95,20 +97,29 @@ class TestState(TestStateBase):
         bd_obj.cmd_create()
 
         # cmd_create should have persisted this to disk
-        state_file = os.path.join(self.build_dir.path,
+        state_file = bd_obj.state_json_file_name
-                                  'states', 'block-device',
-                                  'state.json')
         self.assertThat(state_file, FileExists())
+        pickle_file = bd_obj.node_pickle_file_name
+        self.assertThat(pickle_file, FileExists())
 
         # simulate the state somehow going missing, and ensure that
         # later calls notice
         os.unlink(state_file)
+        os.unlink(pickle_file)
+        # This reads from the state dump json file
         self.assertRaisesRegex(BlockDeviceSetupException,
                                "State dump not found",
-                               bd_obj.cmd_cleanup)
+                               bd_obj.cmd_getval, 'image-path')
         self.assertRaisesRegex(BlockDeviceSetupException,
                                "State dump not found",
                                bd_obj.cmd_writefstab)
+
+        # this uses the pickled nodes
         self.assertRaisesRegex(BlockDeviceSetupException,
-                               "State dump not found",
+                               "Pickle file not found",
                                bd_obj.cmd_delete)
+        self.assertRaisesRegex(BlockDeviceSetupException,
+                               "Pickle file not found",
+                               bd_obj.cmd_cleanup)
+
+        # XXX: figure out unit test for umount