We intended to do an in-place sort of the mount-point list, but
sorted() returns a new list that wasn't captured. Move to the .sort()
function.
It seems the existing unit-test missed this. Add a new test taken
from the bug which does exhibit a sorting issue. Also added a
unit-test of just the comparitor for sanity.
Closes-Bug: 1699437
Change-Id: I8101e4a1804a4af7dbda20d48bf362c3f4ad2742
Currently we pass a reference to a global "rollback" list to create()
to keep rollback functions. Other nodes don't need to know about
global rollback state, and by passing by reference we're giving them
the chance to mess it up for everyone else.
Add a "add_rollback()" function in NodeBase for create() calls to
register rollback calls within themselves. As they hit rollback
points they can add a new entry. lambda v arguments is much of a
muchness -- but this is similar to the standard atexit() call so with
go with that pattern. A new "rollback()" call is added that the
driver will invoke on each node as it works its way backwards in case
of failure.
On error, nodes will have rollback() called in reverse order (which
then calls registered rollbacks in reverse order).
A unit test is added to test rollback behaviour
Change-Id: I65214e72c7ef607dd08f750a6d32a0b10fe97ac3
Keep track of the mount-point ordering in a state variable, rather
than a global. This path is tested by existing unit tests.
Note a prior change inserted the MountNode objects directly into a
list in self.state, which makes sorting quite easy as it can just
implement __lt__. Unfortunately we still json dump the state, and
thus we can't have aribtrary objects in it (future work may be to
check keys inserted into the status object...). So we have to do a
bit of wrangling with tuple lists and comparision functions here, but
it's not too bad.
Change-Id: I0c51e0c53c4efdb7a65ab0efe09a6780cb1affa8
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
Making the global state reference a defined part of the node makes
some parts of the block device processing easier and removes the need
for other global values.
The state is passed to PluginNodeBase.__init__() and expected to be
passed into all nodes as they are created. NodeBase.__init__() is
updated with the new paramater 'state'.
The parameter is removed from the create() call as nodes can simply
reference it at any point as "self.state".
This is similar to 1cdc8b20373c5d582ea928cfd7334469ff36dbce, except it
is based on I68840594a34af28d41d9522addcfd830bd203b97 which loads the
node-list from pickled state for later cmd_* calls. Thus we only
build the state *once*, at cmd_create() time as we build the node
list.
Change-Id: I468dbf5134947629f125504513703d6f2cdace59
Currently we keep a global list of mount-points defined in the
configuration and automatically setup dependencies between mount nodes
based on their global "mount order" (i.e. higher directories mount
first).
The current method for achieving this is roughly to add the mount
points to a dictionary indexed my mount-point, then at "get_edge()"
call build the sorted list ... unless it has already been built
because this gets called for every node.
It seems much simpler to simply keep a sorted list of the
MountPointNode objects as we add them. We don't need to implement a
sorting algorithm then, we can just use sort() and implement __lt__
for the nodes.
I believe the existing mount-order unit testing is sufficient; I'm
struggling to find a valid configuration where the mount-order is
*not* correctly specified in the configuration graph.
Change-Id: Idc05cdf42d95e230b9906773aa2b4a3b0f075598
A couple of things going on, but I think it makes sense to do them
atomically.
The NodeBase.create() argument "results" is the global state
dictionary that will be saved to "state.json", and re-loaded in later
phases and passed to them as the argument "state". So for
consistency, call this argument "state" (this fits with the change out
to start building the state dictionary earlier in the
PluginBase.__init__() calls).
Since the "state" is a pretty important part of how everything works,
move it into a separate object. This is treated as essentially a
singleton. It bundles it nicely together for some added
documentation [1].
We move instantiation of this object out of the generic
BlockDevice.__init__() call and into the actual cmd_* drivers. This
is because there's two distinct instantiation operations -- creating a
new state (during cmd_create) and loading an existing state (other
cmd_*). This is also safer -- since we know the cmd_* arguments are
looking for an existing state.json, we will fail if it somehow goes
missing.
To more fully unit test this, some testing plugins and new
entry-points are added. These add known state values which we check
for. These should be a good basis for further tests.
[1] as noted, we could probably do some fun things in the future like
make this implement a dictionary and have some saftey features like
r/o keys.
Change-Id: I90eb711b3e9b1ce139eb34bdf3cde641fd06828f
As described in pep282 [1], the variable part of a log message
should be passed in via parameter. In this case the parameters
are evaluated only when they need to be.
This patch fixes (unifies) this for DIB.
A check using pylint was added that this kind of passing parameters to
the logging subsystem is enforced in future. As a blueprint a similar
(stripped-down) approach from cinder [2] was used.
[1] https://www.python.org/dev/peps/pep-0282/
[2] https://github.com/openstack/cinder/blob/master/tox.ini
Change-Id: I2d7bcc863e4e9583d82d204438b3c781ac99824e
Signed-off-by: Andreas Florath <andreas@florath.net>
This completes the transitions started in
Ic5a61365ef0132476b11bdbf1dd96885e91c3cb6
The new file plugin.py is the place to start with this change. The
abstract base classes PluginBase and NodeBase are heavily documented.
NodeBase essentially replaces Digraph.Node
The changes in level?/*.py make no functional changes, but are just
refactoring to implement the plugin and node classes consistently.
Additionally we have added asserts during parsing & generation to
ensure plugins are implemented PluginBase, and get_nodes() is always
returning NodeBase objects for the graph.
Change-Id: Ie648e9224749491260dea65d7e8b8151a6824b9c
This switches the code to use networkx for the digraph implementation.
Note that the old implementation specifically isn't removed in this
change -- for review clarity. It will be replaced by a base class
that defines things properly to the API described below.
Plugins return a node object with three functions
get_name() : return the unique name of this node
get_nodes() : return a list of nodes for insertion into the graph.
Usually this is just "self". Some special things like partitioning
add extra nodes at this point, however.
get_edges() : return a tuple of two lists; edges_from and edges_to
As you would expect the first is a list of node names that points to
us, and the second is a list of node names we point to. Usually
this is only populated as ([self.base],[]) -- i.e. our "base" node
points to us. Some plugins, such as mounting, create links both to
and from themselves, however.
Plugins have been updated, some test cases added (error cases
specifically)
Change-Id: Ic5a61365ef0132476b11bdbf1dd96885e91c3cb6
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
Moving the exception didn't cause problems in
I925ed62bdc808f0e07862f6e0905e80b50fbe942, but in later changes where
we split blockdevice.py up a bit more, we can get a bit tangled with
circular imports.
Change-Id: I8297483f64c4e1deecd5ec88ee40e9198bb83589
"log and throw" is arguably an anti-pattern; the error message either
bubles-up into the exception, or the handler figures it out. We have
an example where this logs, and then the handler in blockdevice.py
catches it and logs it again.
Less layers is better; just raise the exception, and use log.exception
to get tracebacks where handled.
Change-Id: I8efd94fbe52a3911253753f447afdb7565849185
This patch finalizes the block device refactoring. It moves the three
remaining levels (filesystem creation, mount and fstab handling) into
the new python module.
Now it is possible to use any number of disk images, any number of
partitions and used them mounted to different directories.
Notes:
* unmount_dir : modified to only unmount the subdirs mounted by
mount_proc_sys_dev(). dib-block-device unmounts
$TMP_MOUNT_PATH/mnt (see I85e01f3898d3c043071de5fad82307cb091a64a9)
Change-Id: I592c0b1329409307197460cfa8fd69798013f1f8
Signed-off-by: Andreas Florath <andreas@florath.net>
Closes-Bug: #1664924
