Merge "Use networkx for digraph"

diskimage_builder/block_device/blockdevice.py

@@ -20,6 +20,8 @@ import shutil
 import sys
 import yaml
 
+import networkx as nx
+
 from stevedore import extension
 
 from diskimage_builder.block_device.config import \
@@ -27,7 +29,6 @@ from diskimage_builder.block_device.config import \
 from diskimage_builder.block_device.exception import \
     BlockDeviceSetupException
 from diskimage_builder.block_device.utils import exec_sudo
-from diskimage_builder.graph.digraph import Digraph
 
 
 logger = logging.getLogger(__name__)
@@ -166,40 +167,86 @@ class BlockDevice(object):
             json.dump(state, fd)
 
     def create_graph(self, config, default_config):
-        logger.debug("Create graph [%s]" % config)
+        """Generate configuration digraph
+
+        Generate the configuration digraph from the config
+
+        :param config: graph configuration file
+        :param default_config: default parameters (from --params)
+        :return: tuple with the graph object, nodes in call order
+        """
         # This is the directed graph of nodes: each parse method must
         # add the appropriate nodes and edges.
-        dg = Digraph()
+        dg = nx.DiGraph()
 
         for config_entry in config:
-            if len(config_entry) != 1:
+            # this should have been checked by generate_config
-                logger.error("Invalid config entry: more than one key "
+            assert len(config_entry) == 1
-                             "on top level [%s]" % config_entry)
+
-                raise BlockDeviceSetupException(
-                    "Top level config must contain exactly one key per entry")
             logger.debug("Config entry [%s]" % config_entry)
             cfg_obj_name = list(config_entry.keys())[0]
             cfg_obj_val = config_entry[cfg_obj_name]
 
-            # As the first step the configured objects are created
+            # Instantiate a "plugin" object, passing it the
-            # (if it exists)
+            # configuration entry
             if cfg_obj_name not in self.plugin_manager:
-                logger.error("Configured top level element [%s] "
+                raise BlockDeviceSetupException(
-                             "does not exists." % cfg_obj_name)
+                    ("Config element [%s] is not implemented" % cfg_obj_name))
-                return 1
             cfg_obj = self.plugin_manager[cfg_obj_name].plugin(
                 cfg_obj_val, default_config)
 
-            # At this point it is only possible to add the nodes:
+            # Ask the plugin for the nodes it would like to insert
-            # adding the edges needs all nodes first.
+            # into the graph.  Some plugins, such as partitioning,
-            cfg_obj.insert_nodes(dg)
+            # return multiple nodes from one config entry.
+            nodes = cfg_obj.get_nodes()
+            for node in nodes:
+                # would only be missing if a plugin was way out of
+                # line and didn't put it in...
+                assert node.name
+                # ensure node names are unique.  networkx by default
+                # just appends the attribute to the node dict for
+                # existing nodes, which is not what we want.
+                if node.name in dg.node:
+                    raise BlockDeviceSetupException(
+                        "Duplicate node name: %s" % (node.name))
+                logger.debug("Adding %s : %s", node.name, node)
+                dg.add_node(node.name, obj=node)
 
-        # Now that all the nodes exists: add also the edges
+        # Now find edges
-        for node in dg.get_iter_nodes_values():
+        for name, attr in dg.nodes(data=True):
-            node.insert_edges(dg)
+            obj = attr['obj']
+            # Unfortunately, we can not determine node edges just from
+            # the configuration file.  It's not always simply the
+            # "base:" pointer.  So ask nodes for a list of nodes they
+            # want to point to.  *mostly* it's just base: ... but
+            # mounting is different.
+            #  edges_from are the nodes that point to us
+            #  edges_to are the nodes we point to
+            edges_from, edges_to = obj.get_edges()
+            logger.debug("Edges for %s: f:%s t:%s", name,
+                         edges_from, edges_to)
+            for edge_from in edges_from:
+                if edge_from not in dg.node:
+                    raise BlockDeviceSetupException(
+                        "Edge not defined: %s->%s" % (edge_from, name))
+                dg.add_edge(edge_from, name)
+            for edge_to in edges_to:
+                if edge_to not in dg.node:
+                    raise BlockDeviceSetupException(
+                        "Edge not defined: %s->%s" % (name, edge_to))
+                dg.add_edge(name, edge_to)
+
+        # this can be quite helpful debugging but needs pydotplus.
+        # run "dotty /tmp/out.dot"
+        #  XXX: maybe an env var that dumps to a tmpdir or something?
+        # nx.nx_pydot.write_dot(dg, '/tmp/graph_dump.dot')
+
+        # Topological sort (i.e. create a linear array that satisfies
+        # dependencies) and return the object list
+        call_order_nodes = nx.topological_sort(dg)
+        logger.debug("Call order: %s", list(call_order_nodes))
+        call_order = [dg.node[n]['obj'] for n in call_order_nodes]
 
-        call_order = dg.topological_sort()
-        logger.debug("Call order [%s]" % (list(call_order)))
         return dg, call_order
 
     def create(self, result, rollback):

diskimage_builder/block_device/level0/localloop.py

@@ -48,13 +48,13 @@ class LocalLoop(Digraph.Node):
         Digraph.Node.__init__(self, self.name)
         self.filename = os.path.join(self.image_dir, self.name + ".raw")
 
-    def insert_edges(self, dg):
+    def get_edges(self):
         """Because this is created without base, there are no edges."""
-        pass
+        return ([], [])
 
-    def insert_nodes(self, dg):
+    def get_nodes(self):
-        """Adds self as a node to the given digraph"""
+        """Returns nodes for adding to the graph"""
-        dg.add_node(self)
+        return [self]
 
     @staticmethod
     def image_create(filename, size):

diskimage_builder/block_device/level1/partition.py

@@ -55,11 +55,6 @@ class Partition(Digraph.Node):
 
         self.ptype = int(config['type'], 16) if 'type' in config else 0x83
 
-    def __repr__(self):
-        return "<Partition [%s] on [%s] size [%s] prev [%s]>" \
-            % (self.name, self.base, self.size,
-               self.prev_partition.name if self.prev_partition else "UNSET")
-
     def get_flags(self):
         return self.flags
 
@@ -72,13 +67,12 @@ class Partition(Digraph.Node):
     def get_name(self):
         return self.name
 
-    def insert_edges(self, dg):
+    def get_edges(self):
-        bnode = dg.find(self.base)
+        edge_from = [self.base]
-        assert bnode is not None
+        edge_to = []
-        dg.create_edge(bnode, self)
         if self.prev_partition is not None:
-            logger.debug("Insert edge [%s]" % self)
+            edge_from.append(self.prev_partition.name)
-            dg.create_edge(self.prev_partition, self)
+        return (edge_from, edge_to)
 
     def create(self, result, rollback):
         self.partitioning.create(result, rollback)

diskimage_builder/block_device/level1/partitioning.py

@@ -84,10 +84,9 @@ class Partitioning(Digraph.Node):
             fd.seek(0, 2)
             return fd.tell()
 
-    def insert_nodes(self, dg):
+    def get_nodes(self):
-        for part in self.partitions:
+        # We just add partitions
-            logger.debug("Insert node [%s]" % part)
+        return self.partitions
-            dg.add_node(part)
 
     def _all_part_devices_exist(self, expected_part_devices):
         for part_device in expected_part_devices:

diskimage_builder/block_device/level2/mkfs.py

@@ -96,15 +96,10 @@ class Filesystem(Digraph.Node):
 
         logger.debug("Filesystem created [%s]" % self)
 
-    def __repr__(self):
+    def get_edges(self):
-        return "<Filesystem base [%s] name [%s] type [%s]>" \
+        edge_from = [self.base]
-            % (self.base, self.name, self.type)
+        edge_to = []
-
+        return (edge_from, edge_to)
-    def insert_edges(self, dg):
-        logger.debug("Insert edge [%s]" % self)
-        bnode = dg.find(self.base)
-        assert bnode is not None
-        dg.create_edge(bnode, self)
 
     def create(self, result, rollback):
         logger.info("create called; result [%s]" % result)
@@ -174,7 +169,8 @@ class Mkfs(object):
         fs = Filesystem(self.config)
         self.filesystems[fs.get_name()] = fs
 
-    def insert_nodes(self, dg):
+    def get_nodes(self):
+        nodes = []
         for _, fs in self.filesystems.items():
-            logger.debug("Insert node [%s]" % fs)
+            nodes.append(fs)
-            dg.add_node(fs)
+        return nodes

diskimage_builder/block_device/level3/mount.py

@@ -45,11 +45,7 @@ class MountPoint(Digraph.Node):
         Digraph.Node.__init__(self, self.name)
         logger.debug("MountPoint created [%s]" % self)
 
-    def __repr__(self):
+    def get_node(self):
-        return "<MountPoint base [%s] name [%s] mount_point [%s]>" \
-            % (self.base, self.name, self.mount_point)
-
-    def insert_node(self, dg):
         global mount_points
         if self.mount_point in mount_points:
             raise BlockDeviceSetupException(
@@ -57,9 +53,9 @@ class MountPoint(Digraph.Node):
                 % self.mount_point)
         logger.debug("Insert node [%s]" % self)
         mount_points[self.mount_point] = self
-        dg.add_node(self)
+        return self
 
-    def insert_edges(self, dg):
+    def get_edges(self):
         """Insert all edges
 
         After inserting all the nodes, the order of the mounting and
@@ -74,7 +70,8 @@ class MountPoint(Digraph.Node):
         ensures that during mounting (and umounting) the correct
         order is used.
         """
-        logger.debug("Insert edge [%s]" % self)
+        edge_from = []
+        edge_to = []
         global mount_points
         global sorted_mount_points
         if sorted_mount_points is None:
@@ -86,11 +83,11 @@ class MountPoint(Digraph.Node):
         mpi = sorted_mount_points.index(self.mount_point)
         if mpi > 0:
             # If not the first: add also the dependency
-            dg.create_edge(mount_points[sorted_mount_points[mpi - 1]], self)
+            dep = mount_points[sorted_mount_points[mpi - 1]]
+            edge_from.append(dep.name)
 
-        bnode = dg.find(self.base)
+        edge_from.append(self.base)
-        assert bnode is not None
+        return (edge_from, edge_to)
-        dg.create_edge(bnode, self)
 
     def create(self, result, rollback):
         logger.debug("mount called [%s]" % self.mount_point)
@@ -142,12 +139,13 @@ class Mount(object):
         self.mount_base = self.params['mount-base']
 
         self.mount_points = {}
-
         mp = MountPoint(self.mount_base, self.config)
         self.mount_points[mp.get_name()] = mp
 
-    def insert_nodes(self, dg):
+    def get_nodes(self):
         global sorted_mount_points
         assert sorted_mount_points is None
+        nodes = []
         for _, mp in self.mount_points.items():
-            mp.insert_node(dg)
+            nodes.append(mp.get_node())
+        return nodes

diskimage_builder/block_device/level4/fstab.py

@@ -36,15 +36,13 @@ class Fstab(Digraph.Node):
         self.dump_freq = self.config.get('dump-freq', 0)
         self.fsck_passno = self.config.get('fsck-passno', 2)
 
-    def insert_nodes(self, dg):
+    def get_nodes(self):
-        logger.debug("Insert node")
+        return [self]
-        dg.add_node(self)
 
-    def insert_edges(self, dg):
+    def get_edges(self):
-        logger.debug("Insert edge [%s]" % self)
+        edge_from = [self.base]
-        bnode = dg.find(self.base)
+        edge_to = []
-        assert bnode is not None
+        return (edge_from, edge_to)
-        dg.create_edge(bnode, self)
 
     def create(self, result, rollback):
         logger.debug("fstab create called [%s]" % self.name)

diskimage_builder/block_device/tests/config/bad_edge_graph.yaml

@@ -0,0 +1,28 @@
+- local_loop:
+    name: image0
+
+- partitioning:
+    base: image0
+    name: mbr
+    label: mbr
+    partitions:
+      - flags: [boot, primary]
+        name: root
+        base: image0
+        size: 100%
+
+- mount:
+    base: mkfs_root
+    name: mount_mkfs_root
+    mount_point: /
+
+- fstab:
+    base: mount_mkfs_root
+    name: fstab_mount_mkfs_root
+    fsck-passno: 1
+    options: defaults
+
+- mkfs:
+    base: this_is_not_a_node
+    name: mkfs_root
+    type: ext4

diskimage_builder/block_device/tests/config/duplicate_name.yaml

@@ -0,0 +1,28 @@
+- local_loop:
+    name: this_is_a_duplicate
+
+- partitioning:
+    base: this_is_a_duplicate
+    name: root
+    label: mbr
+    partitions:
+      - flags: [boot, primary]
+        name: root
+        base: image0
+        size: 100%
+
+- mount:
+    base: mkfs_root
+    name: this_is_a_duplicate
+    mount_point: /
+
+- fstab:
+    base: mount_mkfs_root
+    name: fstab_mount_mkfs_root
+    fsck-passno: 1
+    options: defaults
+
+- mkfs:
+    base: root
+    name: mkfs_root
+    type: ext4

diskimage_builder/block_device/tests/config/multi_key_node.yaml

@@ -0,0 +1,8 @@
+- mkfs:
+    name: root_fs
+    base: root_part
+    type: xfs
+  mount:
+    name: mount_root_fs
+    base: root_fs
+    mount_point: /

diskimage_builder/block_device/tests/config/simple_graph.yaml

@@ -1,6 +1,7 @@
 - mkfs:
     name: root_fs
     base: root_part
+    type: xfs
 
 - mount:
     name: mount_root_fs

diskimage_builder/block_device/tests/config/simple_tree.yaml

@@ -1,5 +1,6 @@
 - mkfs:
     name: root_fs
     base: root_part
+    type: xfs
     mount:
       mount_point: /

diskimage_builder/block_device/tests/test_config.py

@@ -69,12 +69,22 @@ class TestGraphGeneration(TestConfig):
 class TestConfigParsing(TestConfig):
     """Test parsing config file into a graph"""
 
+    # test an entry in the config not being a valid plugin
     def test_config_bad_plugin(self):
         config = self.load_config_file('bad_plugin.yaml')
         self.assertRaises(BlockDeviceSetupException,
                           config_tree_to_graph,
                           config)
 
+    # test a config that has multiple keys for a top-level entry
+    def test_config_multikey_node(self):
+        config = self.load_config_file('multi_key_node.yaml')
+        self.assertRaisesRegexp(BlockDeviceSetupException,
+                                "Config entry top-level should be a single "
+                                "dict:",
+                                config_tree_to_graph,
+                                config)
+
     # a graph should remain the same
     def test_graph(self):
         graph = self.load_config_file('simple_graph.yaml')
@@ -106,6 +116,23 @@ class TestConfigParsing(TestConfig):
 
 class TestCreateGraph(TestGraphGeneration):
 
+    # Test a graph with bad edge pointing to an invalid node
+    def test_invalid_missing(self):
+        config = self.load_config_file('bad_edge_graph.yaml')
+        self.assertRaisesRegexp(BlockDeviceSetupException,
+                                "Edge not defined: this_is_not_a_node",
+                                self.bd.create_graph,
+                                config, self.fake_default_config)
+
+    # Test a graph with bad edge pointing to an invalid node
+    def test_duplicate_name(self):
+        config = self.load_config_file('duplicate_name.yaml')
+        self.assertRaisesRegexp(BlockDeviceSetupException,
+                                "Duplicate node name: "
+                                "this_is_a_duplicate",
+                                self.bd.create_graph,
+                                config, self.fake_default_config)
+
     # Test digraph generation from deep_graph config file
     def test_deep_graph_generator(self):
         config = self.load_config_file('deep_graph.yaml')

requirements.txt

@@ -2,6 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 Babel!=2.4.0,>=2.3.4 # BSD
+networkx>=1.10 # BSD
 pbr!=2.1.0,>=2.0.0 # Apache-2.0
 PyYAML>=3.10.0 # MIT
 flake8<2.6.0,>=2.5.4 # MIT