vm-less and daemon-less ELBE build without mknod - #487
Conversation
t-8ch
left a comment
There was a problem hiding this comment.
I focused on step 1) for now.
You could also drop the other steps for now and we do one after the other.
| elif not xmlfile.endswith('.xml'): | ||
| args.parser.error('Unknown file ending (use either xml or iso)') | ||
|
|
||
| local_build_with_repodir_and_dl_result(xmlfile, cdrom, args.base_image, args) |
There was a problem hiding this comment.
This is duplicated with the initvm submit path. It should be in a helper.
There was a problem hiding this comment.
| cdrom = xmlfile | ||
| xmlfile = tmp.fname('source.xml') | ||
| elif not xmlfile.endswith('.xml'): | ||
| args.parser.error('Unknown file ending (use either xml or iso)') |
There was a problem hiding this comment.
Maybe we can handle this in the input arguments type.
There was a problem hiding this comment.
|
|
||
|
|
||
| def local_build_with_repodir_and_dl_result(xmlfile, cdrom, base_image, args): | ||
| os.makedirs(args.build_dir, exist_ok=True) |
There was a problem hiding this comment.
Switched to pathlib throughout the file https://github.com/Linutronix/elbe/pull/487/changes#diff-069babafd9ca08b3f8b23e08b0b493d5a2f570fade3f6e1a59e1e63da3a827e4R5
| # check, whether everything is ok. | ||
| prj = pm.db.get_project_data(prjdir) | ||
| if prj.status != 'build_done': | ||
| raise CliError(191, f'Project build was not successful, current status: {prj.status}') |
There was a problem hiding this comment.
The idea behind these explicit error numbers is that they are unique.
There was a problem hiding this comment.
Switched to 213 which should be the smallest number that is still free (hopefully...).
| @@ -0,0 +1,159 @@ | |||
| # ELBE - Debian Based Embedded Rootfilesystem Builder | |||
There was a problem hiding this comment.
This file contains a lot of duplicated code, can we reduce this?
For example with a shared interface.
There was a problem hiding this comment.
Created an abstract base class to reduce code duplication https://github.com/Linutronix/elbe/pull/487/changes#diff-6b59297358299c92fbc808d444c3339faef3e980efd8db93efe102b75bb299aeR120
| class BuildJob(AsyncWorkerJob): | ||
| def __init__(self, project, build_bin, build_src, skip_pbuilder, base_image_path): | ||
| def __init__(self, project, build_bin, build_src, skip_pbuilder, | ||
| base_image_path, exclude_initvm_pkgs=False): |
There was a problem hiding this comment.
If the argument to a function is always passed, please remove the default argument.
(Some other functions may duplicate all default arguments anyways, keep it there for consistency)
|
|
||
| pm.build_project(prjdir, args.build_bin, args.build_sources, bool(cdrom), | ||
| uploaded_base_image_path) | ||
| uploaded_base_image_path, args.exclude_initvm_pkgs) |
There was a problem hiding this comment.
This will always be True, no?
Maybe we can then also get rid of the ugly args.exclude_initvm_pkgs assignment.
There was a problem hiding this comment.
Passing it explicitly now, instead of the indirect way via args.
https://github.com/Linutronix/elbe/pull/487/changes#diff-069babafd9ca08b3f8b23e08b0b493d5a2f570fade3f6e1a59e1e63da3a827e4R44
| elif init_attr is not None: | ||
| self.attrs = [init_attr] | ||
| else: | ||
| self.attrs = [] |
There was a problem hiding this comment.
Can you split the changes to the repomanager into its own commit, explaining what is going on?
There was a problem hiding this comment.
Now in a separate commit
| self.gen_repo_conf() | ||
|
|
||
| def _reprepro(self, args): | ||
| do(['reprepro', *args], env_add={'GNUPGHOME': self.gnupg_home}) |
There was a problem hiding this comment.
Split the creation of the helper into its own commit.
| BINARIES_MAIN_DIR = f'{ELBE_CACHE_DIR}/binaries/main' | ||
| BINARIES_ADDED_DIR = f'{ELBE_CACHE_DIR}/binaries/added' | ||
| INITVM_BIN_REPO_DIR = f'{ELBE_CACHE_DIR}/initvm-bin-repo' | ||
| INITVM_GNUPG_HOME = f'{ELBE_CACHE_DIR}/gnupg' |
There was a problem hiding this comment.
Please move this commit earlier into the series.
There was a problem hiding this comment.
Now directly after the extract initvm packages commits.
d4cabf0 to
3b2caf8
Compare
Allow RepoBase/CdromInitRepo to be constructed without init attributes (init_codename=None), and to pick up an existing repo directory that does not yet have a conf/distributions file, treating it like a fresh repository in that case. This prepares the ground for skipping the initvm CDROM entirely on initvm-less builds. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Adding initvm packages to the CDROMs is not reasonable when there is no initvm. Therefore, exclude them. Also, provide this option as CLI parameter (default off) for initvm builds for consistency. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Introduce a reprepro helper that wraps the reprepro invocations which set GNUPGHOME, instead of repeating the env_add argument at every call site. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
The gnupg home directory used for signing and verifying repositories was hardcoded to /var/cache/elbe/gnupg throughout the signing helpers and repository classes. Pass it explicitly instead, so callers can point them at a project-specific keyring. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
More of the code from initvmaction.py can be moved to buildsubmitaction.py by introducing an abstract base class. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Add a new top-level "elbe build" command as alternative to "elbe initvm submit". It drives the project manager directly in-process, without any daemon or SOAP communication. It is meant to be used in an environment that already provides the required isolation. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
The rbind-based pseudo filesystem mounting for chroot() needs the _Mount helper, but imgutils.py cannot be imported from shellhelper.py without creating a cyclic dependency. Move _Mount there first. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
…cratch Several operations require a chroot with the pseudo filesystems (/proc, /sys and /dev) in it. Inside a container, the container runtime already protects parts of it (like /proc/kcore) by overlaying it with a null bind mount. Doing a fresh mount from scratch (e.g. mount -t proc None foo) inside a completely new mount and user namespace, would expose the content again, so the kernel has explicit procection against it (-> mount_too_revealing). But we do not need fresh pseudo filesystems. We can just rebind what we already have. The only issue with that is that we need to scope it more tightly around the operations that actually require this setup with chroot and pseudo filesystems, otherwise we would fill up the newly created filesystem with stuff from the pseudo filesystems we do not need. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Several actions require elevated privileges for losetup or mknod. Instead of starting the build and then later failing with an error message, check beforehand for critical elements in the XML and print a respective error message. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
With the recent additions, it is no longer necessary to spawn an initvm to build ELBE images. Therefore, provide a container that enables this use case and replace the existing legacy containerfile with it. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
/tmp is usually RAM-backed, so when putting build artifacts there, it will fill up very quickly and exhaust the RAM. Therefore, use /var/tmp per default. On Debian systems /var/tmp will still follow a retention policy, so any potential leftovers will be removed after 30 days (in the default configuration). Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Split the test_xml to only contain code that is not directly related to the initvm. This will enable us in the next step to add further tests that will use the container-based approach. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Provide a test driver to run the same tests as for the initvm in test_xml.py for the container-based build as well. Run all tests as root, skip if we are not root. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Build a simple image without a bootloader to demonstrate an ELBE build that does not require elevated privileges. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
If there is no udev running (like when we are inside a container), we do not need to wait for it. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
…without udev Setup UUID as needed e.g. for grub, because that is usually also done by udev. So without udev, we need a replacement for that functionality. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
By unsharing into a new user and mount namespace, we get sufficient permissions to perform (bind) mounts needed for the build process. With this, we can avoid the need for an explicit --cap-add CAP_SYS_ADMIN Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Add how to build rootless for ELBE XMLs with a reduced feature set. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Detect whether the XML under test actually needs a rootful container by checking xml_needs_rootful(). When it does not, run the container rootless via --userns auto instead of requiring elevated privileges, and only skip the test when a rootful container is required and the tests are not run as root. Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
3b2caf8 to
90dc4e3
Compare
This supersedes #482 by removing the manual mknod procedures with passing udev and /dev directly into the container for the rootful case.
Also reordered the commits as discussed:
fc80da4
54c6257
da13574
d532e23
9843820
d7f66a3
707b6c6
ea2994e
ec47114
b5b9d22
No additional commits, but requires to manually unshare if not done via container.
0095a8d
43a09d5
41e12bf
76aa59e
79d2fd0
d4cabf0