openstack
/
diskimage-builder
Code Issues Proposed changes

Add a more generic tree->graph parser 

This moves to a more generic config parser that doesn't have plugins
parsing part of the tree.

I understand why it ended up that way; we have "partitions" key which
has special semantics compared to others keys and there was a desire
to keep it isolated from core tree->graph code.  But this isn't really
isolated; you have to reverse-engineer several module-crossing
boundaries, extras classes and repetitive recursive functions.

Ultimately, plugins should have access to the node graph, but not
participate in configuration parsing.  This way we ensure that plugins
can't invent new methods of configuration parsing.

Note: unit tests produce the same tree -> graph conversion as the old
method.  i.e. this is not intended to have a functional change.

Change-Id: I8a5d62a076a5a50597f2f1df3a8615afba6dadb2
This commit is contained in:
Ian Wienand 2017-05-17 15:54:02 +10:00
parent 7341542f2c
commit 00da1982ce
14 changed files with 193 additions and 180 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
diskimage_builder/block_device/blockdevice.py
View File

@ -22,6 +22,8 @@ import yaml
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.utils import exec_sudo
@ -119,30 +121,6 @@ class BlockDevice(object):
else:
v['label'] = "cloudimg-rootfs"
@staticmethod
def _config_tree_to_digraph(tconfig, plugin_manager):
"""Converts a possible tree-like config into a complete digraph"""
dconfig = []
for config_entry in tconfig:
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")
logger.debug("Config entry [%s]" % config_entry)
config_key = list(config_entry)[0]
config_value = config_entry[config_key]
name = config_value['name'] \
if 'name' in config_value else None
if config_key not in plugin_manager:
dconfig.append(config_entry)
else:
plugin_manager[config_key].plugin \
.tree_config.config_tree_to_digraph(
config_key, config_value, dconfig, name,
plugin_manager)
return dconfig
@staticmethod
def _load_json(file_name):
if os.path.exists(file_name):
@ -211,6 +189,7 @@ class BlockDevice(object):
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:
# adding the edges needs all nodes first.
cfg_obj.insert_nodes(dg)
@ -238,8 +217,7 @@ class BlockDevice(object):
with open(self.params['config'], "rt") as config_fd:
self.config = yaml.safe_load(config_fd)
logger.debug("Config before merge [%s]" % self.config)
self.config = self._config_tree_to_digraph(self.config,
self.plugin_manager)
self.config = config_tree_to_graph(self.config)
logger.debug("Config before merge [%s]" % self.config)
self._merge_into_config()
logger.debug("Final config [%s]" % self.config)

173
diskimage_builder/block_device/config.py Normal file
View File

@ -0,0 +1,173 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import logging
from stevedore import extension
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
logger = logging.getLogger(__name__)
_extensions = extension.ExtensionManager(
namespace='diskimage_builder.block_device.plugin',
invoke_on_load=False)
# check if a given name is registered as a plugin
def is_a_plugin(name):
return any(
_extensions.map(lambda x: x.name == name))
def recurse_config(config, parent_base=None):
"""Convert a config "tree" to it's canonical name/base graph version
This is a recursive function to convert a YAML layout "tree"
config into a "flat" graph-based config.
Arguments:
:param config: the incoming config dictionary
:param parent_base: the name of the parent node, if any
:return: a list of expanded, graph-based config items
"""
output = []
this = {}
# We should only have one key, with multiple values, being the
# config entries. e.g. (this was checked by config_tree_to_graph)
# mkfs:
# type: ext4
# label: 1234
assert len(config.items()) == 1
for k, v in config.items():
key = k
values = v
# If we don't have a base, we take the parent base; first element
# can have no base, however.
if 'base' not in values:
if parent_base is not None:
this['base'] = parent_base
else:
this['base'] = values['base']
# If we don't have a name, it is made up as "key_base"
if 'name' not in values:
this['name'] = "%s_%s" % (key, this['base'])
else:
this['name'] = values['name']
# Go through the the values dictionary. Either this is a "plugin"
# key that needs to be recursed, or it is a value that is part of
# this config entry.
for nk, nv in values.items():
if nk == "partitions":
# "partitions" is a special key of the "partitioning"
# object. It is a list. Each list-entry gets treated
# as a top-level entry, so we need to recurse it's
# keys. But instead of becoming its own entry in the
# graph, it gets attached to the .partitions attribute
# of the parent. (see end for example)
this['partitions'] = []
for partition in nv:
new_part = {}
for pk, pv in partition.items():
if is_a_plugin(pk):
output.extend(
recurse_config({pk: pv}, partition['name']))
else:
new_part[pk] = pv
new_part['base'] = this['base']
this['partitions'].append(new_part)
elif is_a_plugin(nk):
# is this key a plugin directive? If so, we recurse
# into it.
output.extend(recurse_config({nk: nv}, this['name']))
else:
# A value entry; just save as part of this entry
this[nk] = nv
output.append({k: this})
return output
def config_tree_to_graph(config):
"""Turn a YAML config into a graph config
Our YAML config is a list of entries. Each
Arguments:
:parm config: YAML config; either graph or tree
:return: graph-based result
"""
output = []
for entry in config:
# Top-level entries should be a dictionary and have a plugin
# registered for it
if not isinstance(entry, dict):
raise BlockDeviceSetupException(
"Config entry not a dict: %s" % entry)
keys = list(entry.keys())
if len(keys) != 1:
raise BlockDeviceSetupException(
"Config entry top-level should be a single dict: %s" % entry)
if not is_a_plugin(keys[0]):
raise BlockDeviceSetupException(
"Config entry is not a plugin value: %s" % entry)
output.extend(recurse_config(entry))
return output
#
# On partitioning: objects
#
# To be concrete --
#
# partitioning:
# base: loop0
# name: mbr
# partitions:
# - name: partition1
# foo: bar
# mkfs:
# type: xfs
# mount:
# mount_point: /
#
# gets turned into the following graph:
#
# partitioning:
# partitions:
# - name: partition1
# base: image0
# foo: bar
#
# mkfs:
# base: partition1
# name: mkfs_partition1
# type: xfs
#
# mount:
# base: mkfs_partition1
# name: mount_mkfs_partition1
# mount_point: /

3
diskimage_builder/block_device/level0/localloop.py
View File

@ -18,7 +18,6 @@ import subprocess
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
from diskimage_builder.block_device.tree_config import TreeConfig
from diskimage_builder.block_device.utils import parse_abs_size_spec
from diskimage_builder.graph.digraph import Digraph
@ -32,8 +31,6 @@ class LocalLoop(Digraph.Node):
This class handles local loop devices that can be used
for VM image installation.
"""
tree_config = TreeConfig("local_loop")
def __init__(self, config, default_config):
logger.debug("Creating LocalLoop object; config [%s] "
"default_config [%s]" % (config, default_config))

4
diskimage_builder/block_device/level1/__init__.py
View File

@ -14,7 +14,5 @@
# under the License.
from diskimage_builder.block_device.level1.partitioning import Partitioning
from diskimage_builder.block_device.level1.partitioning \
import PartitioningTreeConfig
__all__ = [Partitioning, PartitioningTreeConfig]
__all__ = [Partitioning]

29
diskimage_builder/block_device/level1/partition.py
View File

@ -14,44 +14,15 @@ import logging
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
from diskimage_builder.block_device.tree_config import TreeConfig
from diskimage_builder.graph.digraph import Digraph
logger = logging.getLogger(__name__)
class PartitionTreeConfig(object):
@staticmethod
def config_tree_to_digraph(config_key, config_value, pconfig, dconfig,
base_name, plugin_manager):
logger.debug("called [%s] [%s] [%s]"
% (config_key, config_value, base_name))
assert config_key == Partition.type_string
for partition in config_value:
name = partition['name']
nconfig = {
'name': name,
'base': base_name
}
for k, v in partition.items():
if k not in plugin_manager:
nconfig[k] = v
else:
plugin_manager[k].plugin \
.tree_config.config_tree_to_digraph(
k, v, dconfig, name, plugin_manager)
pconfig.append(nconfig)
logger.debug("finished [%s] [%s]" % (nconfig, dconfig))
class Partition(Digraph.Node):
type_string = "partitions"
tree_config = TreeConfig("partitions")
flag_boot = 1
flag_primary = 2

28
diskimage_builder/block_device/level1/partitioning.py
View File

@ -22,8 +22,6 @@ from diskimage_builder.block_device.exception import \
from diskimage_builder.block_device.level1.mbr import MBR
from diskimage_builder.block_device.level1.partition import \
Partition
from diskimage_builder.block_device.level1.partition import \
PartitionTreeConfig
from diskimage_builder.block_device.utils import exec_sudo
from diskimage_builder.block_device.utils import parse_abs_size_spec
from diskimage_builder.block_device.utils import parse_rel_size_spec
@ -33,34 +31,8 @@ from diskimage_builder.graph.digraph import Digraph
logger = logging.getLogger(__name__)
class PartitioningTreeConfig(object):
@staticmethod
def config_tree_to_digraph(config_key, config_value, dconfig,
default_base_name, plugin_manager):
logger.debug("called [%s] [%s] [%s]"
% (config_key, config_value, default_base_name))
assert config_key == "partitioning"
base_name = config_value['base'] if 'base' in config_value \
else default_base_name
nconfig = {'base': base_name}
for k, v in config_value.items():
if k != 'partitions':
nconfig[k] = v
else:
pconfig = []
PartitionTreeConfig.config_tree_to_digraph(
k, v, pconfig, dconfig, base_name, plugin_manager)
nconfig['partitions'] = pconfig
dconfig.append({config_key: nconfig})
logger.debug("finished new [%s] complete [%s]" % (nconfig, dconfig))
class Partitioning(Digraph.Node):
tree_config = PartitioningTreeConfig()
def __init__(self, config, default_config):
logger.debug("Creating Partitioning object; config [%s]" % config)
# Because using multiple partitions of one base is done

2
diskimage_builder/block_device/level2/mkfs.py
View File

@ -17,7 +17,6 @@ import uuid
from diskimage_builder.block_device.exception \
import BlockDeviceSetupException
from diskimage_builder.block_device.tree_config import TreeConfig
from diskimage_builder.block_device.utils import exec_sudo
from diskimage_builder.graph.digraph import Digraph
@ -164,7 +163,6 @@ class Mkfs(object):
"""
type_string = "mkfs"
tree_config = TreeConfig("mkfs")
def __init__(self, config, default_config):
logger.debug("Create Mkfs object; config [%s]" % config)

2
diskimage_builder/block_device/level3/mount.py
View File

@ -17,7 +17,6 @@ import os
from diskimage_builder.block_device.exception \
import BlockDeviceSetupException
from diskimage_builder.block_device.tree_config import TreeConfig
from diskimage_builder.block_device.utils import exec_sudo
from diskimage_builder.block_device.utils import sort_mount_points
from diskimage_builder.graph.digraph import Digraph
@ -131,7 +130,6 @@ class MountPoint(Digraph.Node):
class Mount(object):
type_string = "mount"
tree_config = TreeConfig("mount")
def __init__(self, config, params):
logger.debug("Mounting object; config [%s]" % config)

2
diskimage_builder/block_device/level4/fstab.py
View File

@ -14,7 +14,6 @@
import logging
from diskimage_builder.block_device.tree_config import TreeConfig
from diskimage_builder.graph.digraph import Digraph
@ -24,7 +23,6 @@ logger = logging.getLogger(__name__)
class Fstab(Digraph.Node):
type_string = "fstab"
tree_config = TreeConfig("fstab")
def __init__(self, config, params):
logger.debug("Fstab object; config [%s]" % config)

1
diskimage_builder/block_device/tests/config/deep_graph.yaml
View File

@ -1,5 +1,4 @@
- local_loop:
base: image0
name: image0
- partitioning:

1
diskimage_builder/block_device/tests/config/multiple_partitions_graph.yaml
View File

@ -1,5 +1,4 @@
- local_loop:
base: image0
name: image0
- partitioning:

36
diskimage_builder/block_device/tests/test_config.py
View File

@ -18,6 +18,10 @@ import yaml
from diskimage_builder.block_device.blockdevice \
import BlockDevice
from diskimage_builder.block_device.config \
import config_tree_to_graph
from diskimage_builder.block_device.exception import \
BlockDeviceSetupException
logger = logging.getLogger(__name__)
@ -62,52 +66,40 @@ class TestGraphGeneration(TestConfig):
self.bd = BlockDevice(self.fake_default_config)
# NOTE: inherits from TestGraphGeneration for simplicity to get
# BlockDevice.plugin_manager object for _config_tree_to_diagraph.
# Config parsing can be moved separately (and not require a
# BlockDevice object) in a later change.
class TestConfigParsing(TestGraphGeneration):
class TestConfigParsing(TestConfig):
"""Test parsing config file into a graph"""
def test_config_bad_plugin(self):
# Currently, configuration parsing does not notice a missing
# plugin. This is left as a stub
return
# config = self.load_config_file('bad_plugin.yaml')
# self.assertRaises(BlockDeviceSetupException,
# self.bd._config_tree_to_digraph,
# config, self.bd.plugin_manager)
config = self.load_config_file('bad_plugin.yaml')
self.assertRaises(BlockDeviceSetupException,
config_tree_to_graph,
config)
# a graph should remain the same
def test_graph(self):
graph = self.load_config_file('simple_graph.yaml')
parsed_graph = self.bd._config_tree_to_digraph(graph,
self.bd.plugin_manager)
self.assertEqual(parsed_graph, graph)
parsed_graph = config_tree_to_graph(graph)
self.assertItemsEqual(parsed_graph, graph)
# equivalence of simple tree to graph
def test_simple_tree(self):
tree = self.load_config_file('simple_tree.yaml')
graph = self.load_config_file('simple_graph.yaml')
parsed_graph = self.bd.\
_config_tree_to_digraph(tree,
self.bd.plugin_manager)
parsed_graph = config_tree_to_graph(tree)
self.assertItemsEqual(parsed_graph, graph)
# equivalence of a deeper tree to graph
def test_deep_tree(self):
tree = self.load_config_file('deep_tree.yaml')
graph = self.load_config_file('deep_graph.yaml')
parsed_graph = self.bd.\
_config_tree_to_digraph(tree, self.bd.plugin_manager)
parsed_graph = config_tree_to_graph(tree)
self.assertItemsEqual(parsed_graph, graph)
# equivalence of a complicated multi-partition tree to graph
def test_multipart_tree(self):
tree = self.load_config_file('multiple_partitions_tree.yaml')
graph = self.load_config_file('multiple_partitions_graph.yaml')
parsed_graph = self.bd._config_tree_to_digraph(tree,
self.bd.plugin_manager)
parsed_graph = config_tree_to_graph(tree)
logger.debug(parsed_graph)
self.assertItemsEqual(parsed_graph, graph)

60
diskimage_builder/block_device/tree_config.py
View File

@ -1,60 +0,0 @@
# Copyright 2016-2017 Andreas Florath (andreas@florath.net)
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import logging
logger = logging.getLogger(__name__)
class TreeConfig(object):
"""Supports simple tree-like configuration
When using the new (complete) configuration there is a need to
specify the complete digraph of block level configurations. This
provides great flexibility for the cost of complex configurations:
each and every single element must be completely specified. In
many simple use cases the configuration flexibility is not
needed.
With the help of this object the simple to use and short tree-like
configuration is converted automatically into the complete digraph
configuration which can be used to create the block device
elements.
"""
def __init__(self, type_string):
self.type_string = type_string
def config_tree_to_digraph(self, config_key, config_value, dconfig,
default_base_name, plugin_manager):
logger.debug("called [%s] [%s] [%s]"
% (config_key, config_value, default_base_name))
base_name = config_value['base'] if 'base' in config_value \
else default_base_name
name = config_value['name'] \
if 'name' in config_value \
else "%s_%s" % (config_key, base_name)
assert config_key == self.type_string
nconfig = {'base': base_name, 'name': name}
for k, v in config_value.items():
if k not in plugin_manager:
nconfig[k] = v
else:
plugin_manager[k].plugin \
.tree_config.config_tree_to_digraph(
k, v, dconfig, name, plugin_manager)
dconfig.append({self.type_string: nconfig})
logger.debug("finished new [%s] complete [%s]" % (nconfig, dconfig))

2
doc/source/user_guide/building_an_image.rst
View File

@ -157,7 +157,7 @@ is exactly the same as writing
mount:
name: mount_root_fs
base: root_fs
mount_point: /
mount_point: /
Non existing `name` and `base` entries in the tree notation are
automatically generated: the `name` is the name of the base module