Move create_graph into config.py

This was suggested in a review comment in
I8a5d62a076a5a50597f2f1df3a8615afba6dadb2.  It works out quite nicely
because the BlockDevice() driver now doesn't need to know anything
about stevedore or plugins, and just works on the node list.  It also
simplifies the unit testing by not having to call create_graph through
a BlockDevice object.

Change-Id: I98512f6cf42e256d2ea8225a0b496d303bf357b8
This commit is contained in:
Ian Wienand 2017-05-26 10:32:59 +10:00
parent deb832d685
commit 3fdd9df983
4 changed files with 106 additions and 118 deletions

View File

@ -20,16 +20,8 @@ import shutil
import sys import sys
import yaml import yaml
import networkx as nx from diskimage_builder.block_device.config import config_tree_to_graph
from diskimage_builder.block_device.config import create_graph
from stevedore import extension
from diskimage_builder.block_device.config import \
config_tree_to_graph
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
from diskimage_builder.block_device.plugin import NodeBase
from diskimage_builder.block_device.plugin import PluginBase
from diskimage_builder.block_device.utils import exec_sudo from diskimage_builder.block_device.utils import exec_sudo
@ -147,9 +139,6 @@ class BlockDevice(object):
self.params['build-dir'], "states/block-device") self.params['build-dir'], "states/block-device")
self.state_json_file_name \ self.state_json_file_name \
= os.path.join(self.state_dir, "state.json") = os.path.join(self.state_dir, "state.json")
self.plugin_manager = extension.ExtensionManager(
namespace='diskimage_builder.block_device.plugin',
invoke_on_load=False)
self.config_json_file_name \ self.config_json_file_name \
= os.path.join(self.state_dir, "config.json") = os.path.join(self.state_dir, "config.json")
@ -168,95 +157,8 @@ class BlockDevice(object):
with open(self.state_json_file_name, "w") as fd: with open(self.state_json_file_name, "w") as fd:
json.dump(state, fd) json.dump(state, fd)
def create_graph(self, config, default_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 = nx.DiGraph()
for config_entry in config:
# 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]
# Instantiate a "plugin" object, passing it the
# configuration entry
# XXX would a "factory" pattern for plugins, where we make
# a method call on an object stevedore has instantiated be
# better here?
if cfg_obj_name not in self.plugin_manager:
raise BlockDeviceSetupException(
("Config element [%s] is not implemented" % cfg_obj_name))
plugin = self.plugin_manager[cfg_obj_name].plugin
assert issubclass(plugin, PluginBase)
cfg_obj = plugin(cfg_obj_val, default_config)
# 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()
assert isinstance(nodes, list)
for node in nodes:
# plugins should return nodes...
assert isinstance(node, NodeBase)
# 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 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]
return dg, call_order
def create(self, result, rollback): def create(self, result, rollback):
dg, call_order = self.create_graph(self.config, self.params) dg, call_order = create_graph(self.config, self.params)
for node in call_order: for node in call_order:
node.create(result, rollback) node.create(result, rollback)
@ -413,7 +315,7 @@ class BlockDevice(object):
return 0 return 0
# Deleting must be done in reverse order # Deleting must be done in reverse order
dg, call_order = self.create_graph(self.config, self.params) dg, call_order = create_graph(self.config, self.params)
reverse_order = reversed(call_order) reverse_order = reversed(call_order)
if dg is None: if dg is None:
@ -427,7 +329,7 @@ class BlockDevice(object):
"""Cleanup all remaining relicts - in good case""" """Cleanup all remaining relicts - in good case"""
# Deleting must be done in reverse order # Deleting must be done in reverse order
dg, call_order = self.create_graph(self.config, self.params) dg, call_order = create_graph(self.config, self.params)
reverse_order = reversed(call_order) reverse_order = reversed(call_order)
for node in reverse_order: for node in reverse_order:
@ -442,7 +344,7 @@ class BlockDevice(object):
"""Cleanup all remaining relicts - in case of an error""" """Cleanup all remaining relicts - in case of an error"""
# Deleting must be done in reverse order # Deleting must be done in reverse order
dg, call_order = self.create_graph(self.config, self.params) dg, call_order = create_graph(self.config, self.params)
reverse_order = reversed(call_order) reverse_order = reversed(call_order)
for node in reverse_order: for node in reverse_order:

View File

@ -11,11 +11,14 @@
# under the License. # under the License.
import logging import logging
import networkx as nx
from stevedore import extension from stevedore import extension
from diskimage_builder.block_device.exception import \ from diskimage_builder.block_device.exception import \
BlockDeviceSetupException BlockDeviceSetupException
from diskimage_builder.block_device.plugin import NodeBase
from diskimage_builder.block_device.plugin import PluginBase
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -138,6 +141,95 @@ def config_tree_to_graph(config):
return output return output
def create_graph(config, default_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 (a :class:`nx.Digraph`),
ordered list of :class:`NodeBase` objects
"""
# This is the directed graph of nodes: each parse method must
# add the appropriate nodes and edges.
dg = nx.DiGraph()
for config_entry in config:
# 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]
# Instantiate a "plugin" object, passing it the
# configuration entry
# XXX : would a "factory" pattern for plugins, where we
# make a method call on an object stevedore has instantiated
# be better here?
if not is_a_plugin(cfg_obj_name):
raise BlockDeviceSetupException(
("Config element [%s] is not implemented" % cfg_obj_name))
plugin = _extensions[cfg_obj_name].plugin
assert issubclass(plugin, PluginBase)
cfg_obj = plugin(cfg_obj_val, default_config)
# 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()
assert isinstance(nodes, list)
for node in nodes:
# plugins should return nodes...
assert isinstance(node, NodeBase)
# 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 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]
return dg, call_order
# #
# On partitioning: objects # On partitioning: objects
# #

View File

@ -16,10 +16,8 @@ import os
import testtools import testtools
import yaml import yaml
from diskimage_builder.block_device.blockdevice \ from diskimage_builder.block_device.config import config_tree_to_graph
import BlockDevice from diskimage_builder.block_device.config import create_graph
from diskimage_builder.block_device.config \
import config_tree_to_graph
from diskimage_builder.block_device.exception import \ from diskimage_builder.block_device.exception import \
BlockDeviceSetupException BlockDeviceSetupException
@ -63,8 +61,6 @@ class TestGraphGeneration(TestConfig):
'mount-base': '/fake', 'mount-base': '/fake',
} }
self.bd = BlockDevice(self.fake_default_config)
class TestConfigParsing(TestConfig): class TestConfigParsing(TestConfig):
"""Test parsing config file into a graph""" """Test parsing config file into a graph"""
@ -121,7 +117,7 @@ class TestCreateGraph(TestGraphGeneration):
config = self.load_config_file('bad_edge_graph.yaml') config = self.load_config_file('bad_edge_graph.yaml')
self.assertRaisesRegexp(BlockDeviceSetupException, self.assertRaisesRegexp(BlockDeviceSetupException,
"Edge not defined: this_is_not_a_node", "Edge not defined: this_is_not_a_node",
self.bd.create_graph, create_graph,
config, self.fake_default_config) config, self.fake_default_config)
# Test a graph with bad edge pointing to an invalid node # Test a graph with bad edge pointing to an invalid node
@ -130,15 +126,14 @@ class TestCreateGraph(TestGraphGeneration):
self.assertRaisesRegexp(BlockDeviceSetupException, self.assertRaisesRegexp(BlockDeviceSetupException,
"Duplicate node name: " "Duplicate node name: "
"this_is_a_duplicate", "this_is_a_duplicate",
self.bd.create_graph, create_graph,
config, self.fake_default_config) config, self.fake_default_config)
# Test digraph generation from deep_graph config file # Test digraph generation from deep_graph config file
def test_deep_graph_generator(self): def test_deep_graph_generator(self):
config = self.load_config_file('deep_graph.yaml') config = self.load_config_file('deep_graph.yaml')
graph, call_order = self.bd.create_graph(config, graph, call_order = create_graph(config, self.fake_default_config)
self.fake_default_config)
call_order_list = [n.name for n in call_order] call_order_list = [n.name for n in call_order]
@ -155,8 +150,7 @@ class TestCreateGraph(TestGraphGeneration):
def test_multiple_partitions_graph_generator(self): def test_multiple_partitions_graph_generator(self):
config = self.load_config_file('multiple_partitions_graph.yaml') config = self.load_config_file('multiple_partitions_graph.yaml')
graph, call_order = self.bd.create_graph(config, graph, call_order = create_graph(config, self.fake_default_config)
self.fake_default_config)
call_order_list = [n.name for n in call_order] call_order_list = [n.name for n in call_order]
# The sort creating call_order_list is unstable. # The sort creating call_order_list is unstable.

View File

@ -15,6 +15,7 @@ import mock
import diskimage_builder.block_device.tests.test_config as tc import diskimage_builder.block_device.tests.test_config as tc
from diskimage_builder.block_device.config import create_graph
from diskimage_builder.block_device.level3.mount import MountPointNode from diskimage_builder.block_device.level3.mount import MountPointNode
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -27,8 +28,7 @@ class TestMountOrder(tc.TestGraphGeneration):
config = self.load_config_file('multiple_partitions_graph.yaml') config = self.load_config_file('multiple_partitions_graph.yaml')
graph, call_order = self.bd.create_graph(config, graph, call_order = create_graph(config, self.fake_default_config)
self.fake_default_config)
result = {} result = {}
result['filesys'] = {} result['filesys'] = {}