Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add install alongside #380

Closed
cgwalters opened this issue Mar 7, 2024 · 9 comments
Closed

Add install alongside #380

cgwalters opened this issue Mar 7, 2024 · 9 comments
Labels
area/install Issues related to `bootc install` enhancement New feature or request

Comments

@cgwalters
Copy link
Collaborator

cgwalters commented Mar 7, 2024

Right now we have

podman run --rm --privileged --pid=host -v /:/target --security-opt label=type:unconfined_t "${image}" bootc install-to-filesystem --karg=console=ttyS0,115200n8 --replace=alongside /target

We can first consider simplifying like this by adding a new toplevel to-existing-root:

podman run --rm --privileged --pid=host --security-opt label=type:unconfined_t -v /:/target "${image}" bootc install to-existing-root

or so, which would assume installing to /target. This consolidates --replace=alongside and /target to just to-existing-root.

I don't see a reason to support installing alongside to not the rootfs.

Now, if we can switch to using dynamic mounts then we can also drop the -v /:/target - I'd like to test this out as it could simplify a lot of our code.

Handling kargs in alongside

When doing an alongside install, there are a ton of use cases for picking up things from the existing environment:

  • root ssh keys
  • console= kargs

Or in general, picking up "configuration".

I am a bit uncertain if we try to add a lot of sugar to bootc in the near term for this versus having higher level tools do it, but...

We could in to-existing-root also have something like --import-kargs=console=* or so to fetch the existing console kernel args (match by glob).

@cgwalters cgwalters added area/install Issues related to `bootc install` enhancement New feature or request labels Mar 7, 2024
@cgwalters
Copy link
Collaborator Author

cgwalters commented Mar 7, 2024

Bigger picture though, while "alongside" installs were easy to implement (it's just small bits of code on top of to-filesystem I do think what we really want to encourage people to do is takeover installs because that ends up fixing several things:

  • We don't "leak" the entire existing prior root
  • We can switch filesystem types (including supporting initializing LUKS in the cloud, etc.)

@cgwalters
Copy link
Collaborator Author

This all said I think in practice people should be automating these invocations, not typing them by hand. It's already very doable by scripting in e.g. cloud-init and that's the intended design.

@rhatdan
Copy link
Member

rhatdan commented Mar 7, 2024

I agree on takeover installs, this is what most people will want, since I don't believe there is any easy way back to the previous root OS. Being able to maintain /home would be nice, but in most cases we expect people to just boot an existing image and convert it to an Bootable OCI Image.

@rhatdan
Copy link
Member

rhatdan commented Mar 7, 2024

I read about dynamic mounts but not sure how this can allow access to / from within the privileged container without Podman requiring a change?

@cgwalters
Copy link
Collaborator Author

cgwalters commented Mar 7, 2024

I read about dynamic mounts but not sure how this can allow access to / from within the privileged container without Podman requiring a change?

I believe we can get access to the root mount namespace at least because we have --pid=host so we inherently have /proc/1/ns/mnt. (Also in practice we're running with --privileged so we can do anything - we could inject a kernel module that does whatever we want, etc. But this is about doing it in a sane/supportable way)

@rhatdan
Copy link
Member

rhatdan commented Mar 8, 2024

Ok, good idea.

cgwalters added a commit to cgwalters/bootc that referenced this issue Mar 8, 2024
This is just mild sugar on top of the existing `to-filesystem`
right now.  But, I think we can do more with this later.

Partially addresses containers#380
cgwalters added a commit to cgwalters/bootc that referenced this issue Mar 8, 2024
This is just mild sugar on top of the existing `to-filesystem`
right now.  But, I think we can do more with this later.

Partially addresses containers#380

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Collaborator Author

One part in #382

@cgwalters
Copy link
Collaborator Author

One thing I wanted to sanity check here is; do we have open_tree in current c9s? And we appear to, as this works

diff --git a/lib/Cargo.toml b/lib/Cargo.toml
index 6491bc3..bd5ccc4 100644
--- a/lib/Cargo.toml
+++ b/lib/Cargo.toml
@@ -32,7 +32,7 @@ openssl = "^0.10.64"
 # TODO drop this in favor of rustix
 nix = { version = "0.28", features = ["ioctl", "sched"] }
 regex = "1.10.3"
-rustix = { "version" = "0.38", features = ["thread", "fs", "system", "process"] }
+rustix = { "version" = "0.38", features = ["thread", "fs", "system", "process", "mount"] }
 schemars = { version = "0.8.16", features = ["chrono"] }
 serde = { features = ["derive"], version = "1.0.197" }
 serde_ignored = "0.1.10"
diff --git a/lib/src/install.rs b/lib/src/install.rs
index c017821..38f7c19 100644
--- a/lib/src/install.rs
+++ b/lib/src/install.rs
@@ -1204,6 +1204,7 @@ fn clean_boot_directories(rootfs: &Dir) -> Result<()> {
 /// Implementation of the `bootc install to-filsystem` CLI command.
 #[context("Installing to filesystem")]
 pub(crate) async fn install_to_filesystem(opts: InstallToFilesystemOpts) -> Result<()> {
+    use rustix::mount::OpenTreeFlags;
     let fsopts = opts.filesystem_opts;
     let root_path = &fsopts.root_path;
 
@@ -1219,6 +1220,14 @@ pub(crate) async fn install_to_filesystem(opts: InstallToFilesystemOpts) -> Resu
         anyhow::bail!("Not a root mountpoint: {root_path}");
     }
 
+    let host_root = rustix::mount::open_tree(
+        rootfs_fd.as_fd(),
+        "/proc/1/ns/mnt",
+        OpenTreeFlags::OPEN_TREE_CLOEXEC,
+    )
+    .context("Opening host root")?;
+    dbg!(host_root);
+
     // Gather global state, destructuring the provided options
     let state = prepare_install(opts.config_opts, opts.source_opts, opts.target_opts).await?;
 

Also for my own reference, the man pages are over here https://github.com/brauner/man-pages-md (because groff stinks apparently, which I agree with).

cgwalters added a commit to cgwalters/bootc that referenced this issue Jun 22, 2024
We're really going to need to switch over to having the container
do dynamic mounts; cc containers#380 (comment)

Just noticed this missing in one place, and found others with
a grep.

Right now we do operate without, but it can be racier.

Signed-off-by: Colin Walters <[email protected]>
cgwalters added a commit to cgwalters/bootc that referenced this issue Jul 16, 2024
This works around an issue where `lsblk` ends up parsing
data cached from udev in `/run/udev`, but we don't have that
mounted by default.

Adding a new mount point hard requirement is logistically
complicated right now - we will eventually switch
to dynamic mounts with `open_tree` (cc
containers#380 )
but this is a relatively straightforward workaround.

Signed-off-by: Colin Walters <[email protected]>
cgwalters added a commit to cgwalters/bootc that referenced this issue Jul 16, 2024
This works around an issue where `lsblk` ends up parsing
data cached from udev in `/run/udev`, but we don't have that
mounted by default.

Adding a new mount point hard requirement is logistically
complicated right now - we will eventually switch
to dynamic mounts with `open_tree` (cc
containers#380 )
but this is a relatively straightforward workaround.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Collaborator Author

This issue ended up being a grab bag but as of right now we do have install to-existing-root. I split off the dynamic mounts to #826

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/install Issues related to `bootc install` enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants