With `--keep-unit` we now run with the privileges and resources of the
caller. We no longer require external services to extend our privileges.
This also means we no longer have to configure our unit sandbox
manually, but simply rely on kernel sandboxing to do the right thing.
Create a preliminary NEWS entry for the upcoming v13 release. Lots of
fixes all over the place, but most of them not worth mentioning for the
release. However, we do have some feature improvements as well.
Although we are doing a good job of finding osbuild breakage when we
test osbuild-composer, we aren't finding osbuild-composer breakage
when we test osbuild.
Run the image tests from osbuild-composer whenever we do CI for osbuild.
Fixes#362.
Signed-off-by: Major Hayden <major@redhat.com>
Split off the argument parser as well as the manifest parser into
helper functions. Drop the pylint hints from the main function now that
it is considerably smaller.
This extracts the CLI entrypoint into `main_cli.py` and prepares the
codebase for the introduction of additional entrypoints. This should
not contain any functional changes.
The idea behind this is to add `main_api.py` (and maybe more in the
future), which will be similar to `main_cli.py` but contain the
`osbuild-api` entrypoint. This will make all entrypoints nicely symetric
and the only difference will be `setup.py` selecting the right
entrypoint for each executable, as well as `__main__.py` selecting the
entrypoint for the module itself (which we will keep to the CLI for
compatibility).
Make sure access to the shared ostree metadata is locked properly. This
is the default since 2018.5, but lets be explicit here. This also makes
sure that the option exists and the local version supports locked and
protected access.
It is unclear whether the `ostree init` honors that as well. It really
should, and if it doesn't we can always report it upstream.
Move the 'test_util_selinux.py' test into the module-unittest
subdirectory.
Drop the '__main__' hookup while at it. `python -m unittest --help`
explains how you can run individual tests.
Add convenience targets to `Makefile` which can run common sets of
tests. For now, add a target for pylint, module-unittests,
pipeline-runtime-tests, as well as all tests.
Currently, it is quite cumbersome to run a reasonable test-setup
locally. Pylint invokation is rather complex, the unittests and runtime
tests in ./test are mixed, and not all tests in ./test can necessarily
be run from a development system.
This commit prepares for a simpler setup:
* Add `make test-pylint` to run pylint as it is run by CI.
* Add `make test-module` to run all module-unittests. This is meant to
be fast (preferably close to instant) and easy to run during
development to do a short check whether there are obvious typos or
other errors in local changes.
If we can keep these tests to machine-local requirements, if we
avoid any sleeps or heavy computations, then this will remain a
convenient test-suite to run locally without having to wait for
30min. In other words: We should be able to keep this under 10s (and
for the long term under 1min) easily.
* Add `make test-runtime` to run all osbuild pipeline executions. This
is not meant to be fast, but thorough. This will require external
sources (preferably limited to a suitable container image with
everything embedded). This will very likely not be run during
development, but rather by the CI.
* Add `make test-all` to run all tests. Very handy for shy people when
the chance of embarrassing copy-paste mistakes is too high to push
publicly.
Additionally to these new targets, this PR introduces 2 new directories
in ./test: ./test/mod/ and ./test/run/
These are meant as equivalent to `test-module` and `test-runtime`. The
reason is that preferably we stick to the auto-discovery of `unittest`
to enumerate tests, rather than enrolling our own or having to enumerate
them explicitly somewhere.
However, we need some way to tell `unittest` which test belongs into
which group. The easiest setup is likely to just use sub-directories.
Note that `test-all` picks all tests independently of where they are
put, even if they are in further different sub-modules under ./test.
For now, no tests are moved into the new directories. I expect this to
take a bit, since there are several out-standing PRs that modify ./test.
I intend to do the final move once we agreed on this and we synchronized
our test-modifications.
This changes the sources module to explicitly cleanup event-loops.
Additionally, the implementation is protected against re-entrency which
we do not support (and do not need).
We did occasionally get the following exception when running
source-servers:
/usr/lib/python3.8/asyncio/base_events.py:654: ResourceWarning: unclosed event loop <_UnixSelectorEventLoop running=False closed=False debug=False>
_warn(f"unclosed event loop {self!r}", ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
Exception ignored in: <function BaseEventLoop.__del__ at 0x7f92589d14c0>
Traceback (most recent call last):
File "/usr/lib/python3.8/asyncio/base_events.py", line 656, in __del__
self.close()
File "/usr/lib/python3.8/asyncio/unix_events.py", line 58, in close
super().close()
File "/usr/lib/python3.8/asyncio/selector_events.py", line 92, in close
self._close_self_pipe()
File "/usr/lib/python3.8/asyncio/selector_events.py", line 99, in _close_self_pipe
self._remove_reader(self._ssock.fileno())
File "/usr/lib/python3.8/asyncio/selector_events.py", line 274, in _remove_reader
key = self._selector.get_key(fd)
File "/usr/lib/python3.8/selectors.py", line 190, in get_key
return mapping[fileobj]
File "/usr/lib/python3.8/selectors.py", line 71, in __getitem__
fd = self._selector._fileobj_lookup(fileobj)
File "/usr/lib/python3.8/selectors.py", line 225, in _fileobj_lookup
return _fileobj_to_fd(fileobj)
File "/usr/lib/python3.8/selectors.py", line 42, in _fileobj_to_fd
raise ValueError("Invalid file descriptor: {}".format(fd))
ValueError: Invalid file descriptor: -1
This is triggered when an event-loop is not closed explicitly via
`event_loop.close()`. It then tries to cleanup explicitly. The problem
here is that python has no knowledge of in which order it should
collect GC'ed objects. This might end up more or less random. Therefore,
file-descriptors might be closed in arbitrary order, leading to the
event-loop being unable to unregister its internal objects.
I am not entirely sure whether this is the case here. However, the error
definitely triggers on the internal event-loop socketpair, which there
is no other external access to. Furthermore, this socketpair is only set
to -1 in its own __del__ function. So unless we have a memory
corruption, I see nothing else that could trigger this.
With this fix in place, I can run `test_sources.py` in a loop without
triggering the bug.
It is quite likely that our other `*Server` classes need the same fix. I
did not verify, yet.
Convert the bash script to an ansible playbook so we can gracefully
handle testing failures and gather logs reliably. Colorful output
is nice, too.
This brings osbuild's testing script in line with the script for
osbuild-composer.
Signed-off-by: Major Hayden <major@redhat.com>
This adds one more flags to `systemd-nspawn`:
--keep-unit
This prevents nspawn from creating its own scope unit and
instead uses the scope of the caller. Since we want nspawn to
run with the privileges of the caller, this is fitting for our
use case.
Furthermore, this makes nspawn work without a running system
bus, since it no longer needs to talk to systemd pid1.
(introduced with systemd-v209)
With this in place, osbuild can be run from within docker containers (or
other containers without systemd as pid1). This still requires some
extra setup, but this can all be done in the container manager.
Two cleanups for the context-managers we use:
* Use `contextlib.AbstractContextManager` if possible. This class
simply provides a default `__enter__` implementation which just
returns `self`. So use it where applicable.
Additionally, it provides an abstract `__exit__` method and thus
allows static checks for an existance of `__exit__` in the dependent
class. We might use that everywhere, but this is a separate
decision, so not included here.
* Explicitly return `None` from `__exit__`. The python docs state:
If an exception is supplied, and the method wishes to suppress
the exception (i.e., prevent it from being propagated), it
should return a true value. Otherwise, the exception will be
processed normally upon exit from this method.
That is, unless we want exceptions to be suppressed, we should
never return a `truthy` value. The python contextlib suggest using
`None` as a default return value, so lets just do that.
In particular, the explicit `return exc_type is None` that we use
has no effect at all, since it only returns `True` if no exception
was raised.
This commit cleans this up and just follows what the `contextlib`
module does and returns None everywhere (well, it returns nothing
which apparently is the same as returning `None` in python). It is
unlikely that we ever want to suppress any exceptions, anyway.
Make use of the new immutable-flag ioctl helpers. While at it, move the
`chmod` to `fchmod` and re-use the open file-descriptor. Document the
behavior and move the `fchmod` into its own try-block for the same
reasons as the `ioctl` call: We rely on the following unlink() to catch
any errors. Errors in the fixperms() step are non-consequential.
The FS_IOC_{GET,SET}FLAGS ioctl numbers are not stable across different
architectures. Most of them use the asm-generic versions, but ALPHA and
SPARC in particular use completely different IOC number setups (see the
definition of _IOC, _IOR, _IOW, etc. in the kernel).
This commit moves the helpers for `FS_IMMUTABLE_FL` into
`osbuild/util/` and adds explicit tests. This will make sure that we
catch any ioctl mismatches as soon as possible when we run the osbuild
test-suite on other architectures. Until then, we will have to live with
this mismatch.
Move remove_tree() into its own module in `osbuild.util.rmrf`. This way
we can use it in other modules as well, without cross-referencing
internal helpers.
Add a new module that implements a simple JSON communication channel.
This is meant to replace all our hard-coded SOCK_DGRAM code that is
copied all over the place.
This is intentionally left simple. It only supports synchronous
operations, trivial JSON encoding and decoding, and uses a message-based
transport mode.
For historical and occult reasons the grubenv file is, according
to its documentation[1] a 'preallocated 1024-byte file'. The
unused space in the file needs to be filled with '#' as padding,
which tools will count as "free space"[2] and there must not be a
trailing new-line.
Fix our code to do as they say to make grub2-editenv work and in
turn greenboot.
[1] https://www.gnu.org/software/grub/manual/grub/html_node/Environment-block.html
[2] grub-core/lib/envblk.c#L105 (commit 0f102b9844f852d48501d231d32a17e1cc24062d)
We want to run stages and other scripts inside of the nspawn containers
we use to build pipelines. Since our pipelines are meant to be
self-contained, this should imply that the build-root must have osbuild
installed. However, this has not been the case so far for several
reasons including:
1. OSBuild is not packaged for all the build-roots we want to support
and thus we have the chicken-and-egg problem.
2. During testing and development, we want to support using a local
`libdir`.
3. We already provide an API to the container. Importing scripts from
the outside just makes this API bigger, but does not change the
fact that build-roots are not self-contained. Same is true for the
running kernel, and probably much more..
With all this in mind, our strategy probably still is to eventually
package osbuild for the build-root. This would significantly reduce our
API exposure, points-of-failure, and host-reliance. However, this switch
might still be some weeks out.
With this in mind, though, we can expect the ideal setup to have a full
osbuild available in the build-root. Hence, any script we import so far
should be able to access the entire `libdir`. This commit unifies the
libdir handling by installing the symlinks into `libdir` and providing
a single bind-mount of the module-path into `libdir`.
We can always decide to scratch that in the future when we scratch the
libdir-import from the host-root. Until then, I believe this commit
nicely unifies the way we import the module both in a local checkout as
well as in the container.
The mknod() method currently allows passing no dir_fd, in which case an
internal one is opened. This FD is then never closed, though.
Fix this by simply making the dir_fd mandatory. All callers pass it
(there is actually only a single caller), so no need for the fallback.
Allow file systems to be identified via there label in addition to
their uuid; i.e. either `uuid` or `label` must be specified, which
results in either `UUID=<uuid>` or `LABEL=<label>` to end up in the
"fs_spec" field. See also fstab(5).
In addition to support for identifying file-systems via their uuids,
they now can be identified via their label as well. Two new options
are introduce for this: `rootfs` and `bootfs` for the root and boot
file system. The latter is option in the case a separated partition
is used for /boot. Both options are an object that can either have
`uuid` or `label` set. The old uuid based options, `root_fs_uuid` &
`boot_fs_uuid` are still supported for now.
Additionally, remove the `GRUB2_ROOT_FS_UUID` option from the
grubenv file and directly write the root file system identifier into
the grub config file.
Make sure we always pylint all python sources. We currently skip tests
as well as osbuild/util. Fix this by always recursively looking for all
python sources and then linting them.
A few adjustments to the AST check:
* Avoid local imports. Move the AST import to the head of the file.
We will get warnings if it is unused, so it should not get stale
there once we drop the AST hack.
* Avoid `TODO:` since pylint will parse it (parsing comments, yay!)
* Use `isinstance()` for type-checks.
This fixes a bunch of minor pylint warnings:
* Drop unused imports.
* Fix "inline-JSON" formatting.
* Fix space before/after brackets.
* Use `_` for unused variables.
* Break overlong lines.
* Mark unittest as `no-self-use` if applicable.
* Drop spurious newline at end of file.
This fixes 3 things:
* Drop an unused argument from the http server handler.
* Break an overlong `with:` statement.
* Fix indentation where it is wrong.
This drops the `server` alias for `http.server`. There is only a single
caller, so lets just be explicit so the callsite is easier to
understand.
As a side effect, this unifies all the imports, no cherrypicking
anymore.
By default, xz only uses one CPU core even if multiple cores are
available. If xz compression is chosen, allow xz to use all of the
cores available.
Signed-off-by: Major Hayden <major@redhat.com>
This stage has been replaced by the org.osbuild.rpm stage. The
latter does not need access to network due inside the container
due to its use of the osbuild sources API.
This converts all fedora links in our samples to `mirrors.kernel.org`.
This mirror works best from around the world, so lets avoid the wild
mix of local mirrors and instead use kernel.org.
This mirror is also well-managed and properly funded, so we should not
run into too many problems with it.
Comments must not be indented in makefiles, otherwise they might end up
being interpreted as rules. Simply drop the indentation and move it into
the comments itself.
