There was a race in diskimage-builder where the mkfs call after a
kpartx -avs for the loop device would fail because the device was
not yet ready. This adds a udevadm settle call after the kpartx
to make sure the udev event queue has cleared.
Change-Id: I90103b59357edebbac7a641e8980cb282d37561b
Closes-Bug: #1698337
Using the newly exposed variables from the prior change, install the
ppc bootloader to the boot partition, not the underlying loopback
device.
Change-Id: I0918e8df8797d6dbabf7af618989ab7f79ee9580
Currently we only export "image-block-device" which is the loopback
device (/dev/loopX) for the underlying image. This is the device we
install grub to (from inside the chroot ...)
This is ok for x86, but is insufficient for some platforms like PPC
which have a separate boot partition. They do not want to install to
the loop device, but do things like dd special ELF files into special
boot partitions.
The first problem seems to be that in level1/partitioning.py we have a
whole bunch of different paths that either call partprobe on the loop
device, or kpartx. We have _all_part_devices_exist() that gates the
kpartx for unknown reasons. We have detach_loopback() that does not
seem to remove losetup created devices. I don't think this does
cleanup if it uses kpartx correctly. It is extremley unclear what's
going to be mapped where.
This moves to us *only* using kpartx to map the partitions of the loop
device. We will *not* call partprobe and create the /dev/loopXpN
devices and will only have the devicemapper nodes kpartx creates.
This seems to be best. Cleanup happens inside partitioning.py.
practice. Deeper thinking about this, and more cleanup of the
variables will be welcome.
This adds "image-block-devices" (note the extra "s") which exports all
the block devices with name and path. This is in a string format that
can be eval'd to an array (you can't export arrays).
This is then used in a follow-on
(I0918e8df8797d6dbabf7af618989ab7f79ee9580) to pick the right
partition on PPC.
Change-Id: If8e33106b4104da2d56d7941ce96ffcb014907bc
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
As we add file-systems, add them to global state and check the labels
are uniqiue. Add a unit test and remove the old global value.
Bonus fixup to the length check, and a test for that too.
Change-Id: I0f5a96f687c92e000afc9c98a26c49c4b1d3f28d
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 the later cmd_* calls -- umount, cleanup, delete -- all
recreate the node graph by parsing the config file using
create_graph()
There is some need, however, to have a sense of global state when
building the node list. The problem is, this is a one time operation
-- we do not want to rebuild that state for these later calls (see the
"loaded" checks in proposed
Ic3b805f9258128d5233b21ff25579c03487c7fcc).
An insight here seems to be that these cmd_* calls do not actually
want to re-parse the configuration file and rebuild the node list;
they just want to walk the node list in reverse with the state as
provided after cmd_create().
So, rather than re-creating the node list, we might as well just
pickle it, save it to disk along side the state dictionary dump and
reload it for cmd_*.
After this, I think we can safely have PluginBase.__init__() be passed
the state. We will now know that this will only be called once,
during initial creation.
Change-Id: I68840594a34af28d41d9522addcfd830bd203b97
You can't pickle a static method reference which complicates being
able to save the node graph when the "rollback" call-back wants to
hold references to these functions. The outer module (localoop.py) is
small anyway, so from an organisation point of view the difference is
minimal. Since these are really only called with parameters from the
containing class, they could be class methods with no parameters, at
the small expense of having to fiddle the mbr test-case a bit.
Change-Id: I6f9592a4295abe1b41294b79828bc2f3c2da01c6
The supported ppc ${ARCH} is "ppc64el" (at least in the gate testing
...) so move the file to that, so gets picked up by
block_device_create_config_file
Change-Id: I9273f35cdbfb0a62404461cbc1df9b2a92155fb0
Something seems to be going on with the ppc matching in the gate test.
Small updates to see what's going on...
Change-Id: Ie48cd4ce1f983a58932a577a43746240f6866936
Because we append the function/line info after debug lines in the gate
logs, the pretty-print ends up not looking all that pretty. Pad it.
Change-Id: Ice013428342614300cd51e8b7be56e79b75b31fc
This is code motion with some small changes to make follow-on's
easier.
test_blockdevice_mbr.py is moved alongside the other tests. It is
modified slightly to use the standard base class and remove a lot of
repeated test setup; a fixture is used for the tempdir (so it doesn't
have to be torn-down, and is removed properly on error) and the partx
args are moved into the setUp() so each test doesn't have to create
it. No functional change. renamed test_mbr.py for shortness.
test_blockdevice_utils.py is merged with existing test_utils.py. No
change to the tests.
test_blockdevice.py is removed. It isn't doing anything currently; to
work it will need to take an approach based more on mocking of calls
that require elevated permissions. It's in history if we need it.
Change-Id: I87b1ea94afaaa0b44e6a57b9d073f95a63a04cf0
assertRaisesRegexp was renamed to assertRaisesRegex in Py3.2
For more details, please check:
https://docs.python.org/3/library/
unittest.html#unittest.TestCase.assertRaisesRegex
Change-Id: I705c958c0dbf1daa409ed29ccbc038426298c306
Closes-Bug: #1436957
package-installs.yaml is installing python-dev, not python2-dev,
so we need to adjust the mapping accordingly.
In addition, zypper-minimal used an dpkg specific package name,
while there is a SUSE equivalent (and zypper-minimal is anyway
SUSE family specific)
Change-Id: Ia9dd061fa46a514781808d62e5e93b03f75c6745
Ubuntu 12.04 LTS reached its regular End of Life on April 28, 2017.
Depends-On: I5e145095a10db112bb27516bfe652d2cdc052a61
Change-Id: I64af4c5183d77a75dcd062895d19b0a1330c8da8
While plugins treat the state as just a dictionary, it's nice for the
driver functions to keep state related functions encapsulated in the
state object singleton. Wrap the internal state dictionary so we can
pass the BlockDeviceState directly without dereferencing.
Change-Id: Ic0193c64d645ed1312f898cbfef87841f460799c
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
This element has not been functioning correctly for some time due to
an incorrect path to select-boot-kernel-initrd (should be /usr/local/bin).
The dracut-regenerate element can be used to regenerate dracut ramdisks
and is more flexible than this element.
Change-Id: I33d555ffd4a92b2948b2ea4a66b151f0422ccb8c
Closes-Bug: #1688546
This patch removes the ccache handling from the base element. For
mostly all systems this was never used at all.
This is working towards the removal of the base element from DIB
Change-Id: Ieb16ef612ebd98470993dcd6f55b3a22d37084ba
Signed-off-by: Andreas Florath <andreas@florath.net>
In python3, the standard out data returned by
subprocess.Popen.communicate() will in most cases be bytes rather than a
string and must therefore be decoded.
Without this fix we hit the following error:
TypeError: a bytes-like object is required, not 'str'
Change-Id: I6d75f867ebfdb925970c3397175214b9050d7632
Closes-Bug: #1694463
Currently openSUSE 42.3 has entered feature freeze mode
so it is a good point in time to verify that 42.3 builds
are working successfully. Also test opensuse-minimal for
platforms that support it (need working zypper package)
Change-Id: I4c613e1e68cb7375c29d544bbf70b5da9bf21414
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 is an initial creation of pylint with a basic indent checker.
Small issues corrected. Job added to gate with
Ib554a284e92583cc1d6a5c2219b3922852ca4c73
Change-Id: I7e24d8348db3aef79e1395d12692199a1f80161a
Co-Authored-By: Andreas Florath <andreas@florath.net>
This is consistent with how dpkg based images are configured
and minimizes the nodepool images drastically (avoid installing
texlive for example)
Change-Id: I98fb31bc0e06869e9770fae3dbd62f0d86acb879
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 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
Add a range of unit-testing for configuration parsing, graph
generation and mount-point generation. Unfortunately there's some
global variable hacks, and some stubs, but it's a start.
Change-Id: I9e4f950c2c2ea656fc0c1a14594059fb4c62fa35
There's an increasing amount of pydoc based documentation. Output the
module reference and link it into the developers main page.
One fixup to the rst needed
Change-Id: I1d43a1fe1c735eb4559e3d2b40834d1c8115c586
This is a follow-on to 57ef187632.
There's two things going on here; DIB_MANIFEST_IMAGE_DIR is *outside*
the chroot on the build host. We copy the files here for posterity, I
guess. MANIFEST_IMAGE_PATH is *inside* the chroot and are the files
we want to ensure are locked to root.
The prior change modified the permissions on DIB_MANIFEST_IMAGE_DIR.
So the first time you build, it works -- then the second time,
assuming you're using the same output filename, it hits the root-owned
manifest directories and causes a build failure.
I have built with this and checked that the manifest files in the
image are locked to root:
$ virt-ls -a ./test.qcow2 -l /etc/dib-manifests
total 32
drwxr-xr-x 2 0 0 4096 May 24 03:39 .
drwxr-xr-x 53 0 0 4096 May 24 03:39 ..
-rw------- 1 0 0 15236 May 24 03:39 dib-manifest-dpkg-test
-rw------- 1 0 0 35 May 24 03:39 dib_arguments
-rw------- 1 0 0 137 May 24 03:39 dib_environment
Related-Bug: #1671842
Change-Id: I08319d0b5fcc461d40fe0be8427dcf0e37ad21e6
Move Partition() object creation into the actual Partition object,
rather than having the logic within the Partitioning() object
Change-Id: I833ed419a0fca38181a9e2db28e5af87500d8ba4
Split Partition() into it's own file for clarity. This will be
followed-on by less dependence between Partitions and Partition
Change-Id: I860f6a1787c0e4fe99f93919ac37cf7d80bfaae9
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
Because in some cases (e.g. partitioning) the order is needed,
add weights to the digraph to get an (somewhat) stable
topological sort.
Change-Id: I5ef1acc6338ac93c593faa0eafe26cbed42ed887
Signed-off-by: Andreas Florath <andreas@florath.net>
Per [1] this is the "official" CDN mirror, which I think is the most
appropriate for the default. I think this addresses the concerns
httpredir service, which I don't think ever quite got out of beta.
[1] https://wiki.debian.org/DebianGeoMirror
Change-Id: I55f2a00b8bbb0f0a20d3be229e4c2c32a7b69057
Instead, either use the bash built-in of type to ensure it exists. Since
which is an external dep, things can fail oddly in a constrained
environment.
Also add a dib-lint test for this.
Change-Id: I645029f5b5bfe1198c89ce10fd3246be8636e8af
Signed-off-by: Jesse Keating <omgjlk@us.ibm.com>
This new element will allow to regenerate dracut
on the produced images, to enable different modules. It
relies on a yaml blob to specify modules and packages
needed. It defaults to installing lvm and crypt.
Change-Id: I292fb70cde41ee6053b7b81a67931bcdaaa6d664
"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
Manifests files can release sensitive information and therefore should
have restrictive permissions.
Change-Id: I64d6c830217a7d8b0172df2dc774079dcd1e2a68
Related-Bug: #1671842
A majority of the "plugins" aren't implementing the plugin class.
Clearly we need some refactoring of the ideas here. Remove for
simplicity.
Change-Id: If399a371b171f4fd17cfa5856fe55daca4c86e60
To avoid failures with double unmount, skip unmounting
the mountpoints that are managed by block device.
Change-Id: I228779eb9bf544a27a53e5017c87573023fd375a
With new block device definition, where content of the image
can be mounted on different partitions, is not enough with
executing setfiles on root directory. Instead of that, expose
all the mountpoints on the image, and apply setfiles on them.
Change-Id: I153f979722eaec49eab93d7cd398c5589b9bfc44
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
The args agument was only used to find the symbol for the getval
command. Have the command pass the symbol to find in directly. We
can therefore remove the args paramater to the BlockDevice() creation.
Change-Id: I8e357131b70a00e4a2c4792c009f6058d1d5ae9e
Move argument parsing to subparsers, rather than positional arguments.
This better reflects the tool's role as a driver and allows
sub-commands to deal with arguments in a natural way.
Change-Id: Iae8c368e0f3fe47abfddb9e0a1558bd5b3423aee
I accidentally dropped the clearing of this file when it moved to
cmd.py during rebase of I1919f6e865acae14ee95cd025c9c7b75ca266a9c
Change-Id: Ibe9fcde594770cb51c732cc253987308dc038083
DIB_BLOCK_DEVICE_PARAMS_YAML should be exported, and the
dib-block-device will take this as the value of --params. Remove this
to simplify the command-line
Change-Id: I6764ed223ecd36f9d24e19f164b6a927380b410f
This creates a BlockDeviceCmd object to hold the main() function.
This doesn't really do anything different right now, but sets a base
for using argparse subparsers to handle the command-line
Change-Id: I4acf95ff4d554a3b4e7e2244ab1706631b98458f
This moves the YAML parameter parsing into the command-line driver.
It makes the argument optional so it can be taken from the environment
variable directly. The parsed YAML is passed to the BlockDevice
object.
Change-Id: I6fa5e5b7d1fccfc7cf47d6e4a1fa6e560734680d
To avoid any confusion, commands passed to exec_sudo() should be a
list of "str"s. Log a message if we see unicode issues.
This also adds a debug trace of all output. stderr is captured.
This is modified to raise CalledProcessError on failure, like
check_call(). Calls that are ok to fail will need to explicitly catch
and ignore this.
The two calls that we expect to fail are wrapped
We wish to try rolling back if one of these command raises an
exception. Modify the create handler to initiate rollback on all
exceptions.
Change-Id: Iee4fa41ffaf243e4728bf3a5eeec5c8fa8d2dadc
The await function is essentially a non-standard check_output call.
Let's use standard calls to increase maintainability.
Change-Id: I2c25e1cd7122791fcaa86b46bd801e661471bc9e
As this method can be introduced without any dependency,
provide it on an independent change to simplify reviews.
This is a partial refactor based on
I592c0b1329409307197460cfa8fd69798013f1f8
Change-Id: Idaf3d2b3b3e23d0b9d6bc071d67b961a829ae422
Co-Authored-By: Andreas Florath <andreas@florath.net>
This is a partial refactor from change
I592c0b1329409307197460cfa8fd69798013f1f8
Change-Id: I8822e68e41c4ebd47eea9ffed4557efc130a7bf7
Co-Authored-By: Andreas Florath <andreas@florath.net>
Add a new method in the block device library called
exec_sudo, so it can be reused.
This is a partial refactor of change
I592c0b1329409307197460cfa8fd69798013f1f8
Change-Id: Id621f6d029e1275a35c4fd3f19b57c8518076134
Co-Authored-By: Andreas Florath <andreas@florath.net>
