WIP/RFC: Add support for modifying sysroots without a daemon #3006
Conversation
|
Skipping CI for Draft Pull Request. |
On that topic, although there isn't any C code being converted to Rust here, I think this would help the cause because those |
There was a problem hiding this comment.
One problem here is that I'd like to make increasing use of systemd in the daemon. We already added isolation.rs which goes in that direction, and if we go this way we will need to make a hard decision: do we enhance that code (like the livefs bits) to understand non-default sysroots, or do we skip using systemd entirely? I think for use in the initramfs, we can clearly still make use of systemd. But some of this work is also related to eventually supporting running in a podman/docker container, where we won't have systemd.
| @@ -418,12 +418,12 @@ rpmostree_run_script_in_bwrap_container (int rootfs_fd, | |||
| { | |||
| stdout_fd = sd_journal_stream_fd (id, LOG_INFO, 0); | |||
| if (stdout_fd < 0) | |||
| return glnx_prefix_error (error, "While creating stdout stream fd"); | |||
| return glnx_throw (error, "While creating stdout stream fd: %s", g_strerror (-stdout_fd)); | |||
There was a problem hiding this comment.
Looks like this can be split out now.
| if (progress) | ||
| ostree_async_progress_finish (progress); |
There was a problem hiding this comment.
Hmm. Shouldn't we include the _finish() call elsewhere though? Is this related?
There was a problem hiding this comment.
We _finish() at a higher level. We need to be consistent about calling _finish() at the same scope level as ostree_async_progress_new() so it's clear they all match up. Have to split out that patch.
|
Tangentially related to this...I've mentioned this elsewhere before but basically I've been thinking that the direction we should go in is changing the daemon to fork off the transaction as a subprocess. It'd start us down the road to solving a whole pile of issues, of which the biggest is making cancellation reliably work - we can just kill the subprocess while keeping the DBus proxy up. If we go that route, it would make implementing this easier as we could fork off in the same way outside of DBus. |
Yeah, that's actually what I was hitting when trying diff --cc rust/src/isolation.rs
index 18c3916b,18c3916b..cc81d155
--- a/rust/src/isolation.rs
+++ b/rust/src/isolation.rs
@@@ -33,7 -33,7 +33,14 @@@ pub(crate) struct UnitConfig<'a>
#[context("Running systemd worker")]
pub(crate) fn run_systemd_worker_sync(cfg: &UnitConfig) -> Result<()> {
if !crate::utils::running_in_systemd() {
-- return Err(anyhow!("Not running under systemd"));
++ // XXX
++ let mut cmd = Command::new(cfg.exec_args[0]);
++ cmd.args(&cfg.exec_args[1..]);
++ let status = cmd.status()?;
++ if !status.success() {
++ return Err(anyhow!("{}", status));
++ }
++ return Ok(())
}
let mut cmd = Command::new("systemd-run");
cmd.args(BASE_ARGS);But really, we don't actually need the Overall, the approach I was aiming for was basically rpm-ostree would have two modes:
So yeah, this indeed would require adapting various code bits to handle both cases, which adds complexity.
Yeah, that's an interesting idea. Related to this, I did briefly look at using the transaction API even in the "direct" mode, but didn't want to deal with dropping the |
|
So how do we feel about this approach? Worth cleaning up and pursuing? We'll incur some churn in splitting out the |
|
What if we reworked things such that the single DBus invocation was |
|
@jlebon: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Hmm, so you mean e.g. fold |
A papercut right now is that when using Tang, one must use kernel arguments to specify network configuration because network is required on every boot. This breaks the UX we're trying to emphasize, which is towards specifying networking settings using NM keyfiles *once* and having it apply everywhere. Let's fix this by automatically detecting this case and enabling `initramfs-etc` tracking of NM keyfiles on first boot. This will allow users to stay on the NM keyfile path and have it transparently work even when using Tang. The new service does this from the real root. Long-term, I'd like to investigate doing it earlier in the initramfs (this is related to coreos/rpm-ostree#3006). Modifications to NM keyfiles in the real root are automatically synced on every new deployment, or explicitly when using `rpm-ostree ex initramfs-etc --force-sync`.
|
@jlebon: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Given #4972, I'm not sure this makes sense anymore. |
Hack day project. This is really rough but functional. Came out of discussions in #2854. The way this works is we add a new "mux" layer which handles either calling through D-Bus or running the transaction code directly. On the "transaction" side, we factor out
callimplfunctions which just take in simpler parameters to untie it a bit from the daemon stuff. It's actually not that much new code. It's more a lot of reshuffling code we already have.Some demos. For example, adding config files needed for the rootfs at installation time:
Another example is running
rpm-ostree installfrom the initramfs and rebooting so that the packages are installed from first boot:(Obviously,
apply-livewould be nicer to avoid the reboot, though need to unwind more things there first.)There's a bunch of details I'm glossing over here and maybe that initramfs example isn't feasible, though I found it interesting as a POC to show the possibilities of supporting sysroots like this.