diff --git a/diskimage_builder/block_device/blockdevice.py b/diskimage_builder/block_device/blockdevice.py index 38da314c..8d2cd514 100644 --- a/diskimage_builder/block_device/blockdevice.py +++ b/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: - logger.error("Invalid config entry: more than one key " - "on top level [%s]" % config_entry) - raise BlockDeviceSetupException( - "Top level config must contain exactly one key per entry") + # this should have been checked by generate_config + assert len(config_entry) == 1 + 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 - # (if it exists) + # Instantiate a "plugin" object, passing it the + # configuration entry if cfg_obj_name not in self.plugin_manager: - logger.error("Configured top level element [%s] " - "does not exists." % cfg_obj_name) - return 1 + raise BlockDeviceSetupException( + ("Config element [%s] is not implemented" % cfg_obj_name)) 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: - # adding the edges needs all nodes first. - cfg_obj.insert_nodes(dg) + # Ask the plugin for the nodes it would like to insert + # into the graph. Some plugins, such as partitioning, + # 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 - for node in dg.get_iter_nodes_values(): - node.insert_edges(dg) + # Now find edges + for name, attr in dg.nodes(data=True): + 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): diff --git a/diskimage_builder/block_device/level0/localloop.py b/diskimage_builder/block_device/level0/localloop.py index aa33531d..2fe6c12c 100644 --- a/diskimage_builder/block_device/level0/localloop.py +++ b/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): - """Adds self as a node to the given digraph""" - dg.add_node(self) + def get_nodes(self): + """Returns nodes for adding to the graph""" + return [self] @staticmethod def image_create(filename, size): diff --git a/diskimage_builder/block_device/level1/partition.py b/diskimage_builder/block_device/level1/partition.py index 7a61176c..50c96047 100644 --- a/diskimage_builder/block_device/level1/partition.py +++ b/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 "" \ - % (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): - bnode = dg.find(self.base) - assert bnode is not None - dg.create_edge(bnode, self) + def get_edges(self): + edge_from = [self.base] + edge_to = [] if self.prev_partition is not None: - logger.debug("Insert edge [%s]" % self) - dg.create_edge(self.prev_partition, self) + edge_from.append(self.prev_partition.name) + return (edge_from, edge_to) def create(self, result, rollback): self.partitioning.create(result, rollback) diff --git a/diskimage_builder/block_device/level1/partitioning.py b/diskimage_builder/block_device/level1/partitioning.py index e11ae962..7b04344b 100644 --- a/diskimage_builder/block_device/level1/partitioning.py +++ b/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): - for part in self.partitions: - logger.debug("Insert node [%s]" % part) - dg.add_node(part) + def get_nodes(self): + # We just add partitions + return self.partitions def _all_part_devices_exist(self, expected_part_devices): for part_device in expected_part_devices: diff --git a/diskimage_builder/block_device/level2/mkfs.py b/diskimage_builder/block_device/level2/mkfs.py index 9d891597..71aef546 100644 --- a/diskimage_builder/block_device/level2/mkfs.py +++ b/diskimage_builder/block_device/level2/mkfs.py @@ -96,15 +96,10 @@ class Filesystem(Digraph.Node): logger.debug("Filesystem created [%s]" % self) - def __repr__(self): - return "" \ - % (self.base, self.name, self.type) - - 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 get_edges(self): + edge_from = [self.base] + edge_to = [] + return (edge_from, edge_to) 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) - dg.add_node(fs) + nodes.append(fs) + return nodes diff --git a/diskimage_builder/block_device/level3/mount.py b/diskimage_builder/block_device/level3/mount.py index b709f451..f8ef8314 100644 --- a/diskimage_builder/block_device/level3/mount.py +++ b/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): - return "" \ - % (self.base, self.name, self.mount_point) - - def insert_node(self, dg): + def get_node(self): 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) - assert bnode is not None - dg.create_edge(bnode, self) + edge_from.append(self.base) + return (edge_from, edge_to) 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 diff --git a/diskimage_builder/block_device/level4/fstab.py b/diskimage_builder/block_device/level4/fstab.py index cf450884..b0883984 100644 --- a/diskimage_builder/block_device/level4/fstab.py +++ b/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): - logger.debug("Insert node") - dg.add_node(self) + def get_nodes(self): + return [self] - 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 get_edges(self): + edge_from = [self.base] + edge_to = [] + return (edge_from, edge_to) def create(self, result, rollback): logger.debug("fstab create called [%s]" % self.name) diff --git a/diskimage_builder/block_device/tests/config/bad_edge_graph.yaml b/diskimage_builder/block_device/tests/config/bad_edge_graph.yaml new file mode 100644 index 00000000..2ffef014 --- /dev/null +++ b/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 diff --git a/diskimage_builder/block_device/tests/config/duplicate_name.yaml b/diskimage_builder/block_device/tests/config/duplicate_name.yaml new file mode 100644 index 00000000..d6ce411a --- /dev/null +++ b/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 diff --git a/diskimage_builder/block_device/tests/config/multi_key_node.yaml b/diskimage_builder/block_device/tests/config/multi_key_node.yaml new file mode 100644 index 00000000..ef460837 --- /dev/null +++ b/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: / \ No newline at end of file diff --git a/diskimage_builder/block_device/tests/config/simple_graph.yaml b/diskimage_builder/block_device/tests/config/simple_graph.yaml index e1eda146..59c0aeeb 100644 --- a/diskimage_builder/block_device/tests/config/simple_graph.yaml +++ b/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 diff --git a/diskimage_builder/block_device/tests/config/simple_tree.yaml b/diskimage_builder/block_device/tests/config/simple_tree.yaml index d44f319b..67add797 100644 --- a/diskimage_builder/block_device/tests/config/simple_tree.yaml +++ b/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: / \ No newline at end of file diff --git a/diskimage_builder/block_device/tests/test_config.py b/diskimage_builder/block_device/tests/test_config.py index af2ee9ad..911deb70 100644 --- a/diskimage_builder/block_device/tests/test_config.py +++ b/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') diff --git a/requirements.txt b/requirements.txt index 4419fef0..15651bb3 100644 --- a/requirements.txt +++ b/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