cli: add `bootc-reinstall` binary #1063

omertuc · 2025-01-27T14:10:03Z

Background

The current usage instructions for bootc involve a long podman invocation.

Issue

It's hard to remember and type the long podman invocation, making the usage of bootc difficult for users.

See https://issues.redhat.com/browse/BIFROST-610 and https://issues.redhat.com/browse/BIFROST-611 (Epic https://issues.redhat.com/browse/BIFROST-594)

Solution

We want to make the usage of bootc easier by providing a new Fedora/RHEL subpackage that includes a new binary bootc-reinstall. This binary will simplify the usage of bootc by providing a simple command line interface (configured either through CLI flags or a configuration file) with an interactive prompt that allows users to reinstall the current system using bootc. The commandline will handle helping the user choose SSH keys / users and ensure and warn the user about the destructive nature of the operation, and eventually issues they might run into in the various clouds (e.g. missing cloud agent on the target image)

Implementation

Added new reinstall-cli crate that outputs the new bootc-reinstall binary. Modified the bootc.spec file to generate the new subpackage which includes this binary.

This new crate depends on the existing utils crate.

Refactored the tracing initialization from the bootc binary into the utils crate so that it can be reused by the new crate. The new CLI can either be configured through commandline flags or through a configuration file in a path set by the environment variable BOOTC_REINSTALL_CONFIG.

The configuration file is a YAML file.

Limitations

Only root SSH keys are supported. The multi user selection TUI is implemented, but if you choose anything other than root you will get an error.

Try

Try out instructions:

# Make srpm
cargo xtask package-srpm

# Mock group
sudo usermod -a -G mock $(whoami)
newgrp mock

# Build RPM for RHEL
mock --rebuild -r rhel+epel-9-x86_64 --rebuild target/bootc-*.src.rpm

Then install the RPM (/var/lib/mock/rhel+epel-9-x86_64/result/bootc-reinstall-2*.el9.x86_64.rpm) on a rhel9 gcp vm instance template

TODO

Missing docs, missing functionality. Everything is in alpha stage. User choice / SSH keys / prompt disabling should also eventually be supported to be configured through commandline arguments or the configuration file.

Demo

cgwalters

Nice, thanks for getting the ball rolling here!

reinstall-cli/Cargo.toml

reinstall-cli/src/main.rs

# Background The current usage instructions for bootc involve a long podman invocation. # Issue It's hard to remember and type the long podman invocation, making the usage of bootc difficult for users. See https://issues.redhat.com/browse/BIFROST-610 and https://issues.redhat.com/browse/BIFROST-611 (Epic https://issues.redhat.com/browse/BIFROST-594) # Solution We want to make the usage of bootc easier by providing a new Fedora/RHEL subpackage that includes a new binary `bootc-reinstall`. This binary will simplify the usage of bootc by providing a simple command line interface (configured either through CLI flags or a configuration file) with an interactive prompt that allows users to reinstall the current system using bootc. The commandline will handle helping the user choose SSH keys / users and ensure and warn the user about the destructive nature of the operation, and eventually issues they might run into in the various clouds (e.g. missing cloud agent on the target image) # Implementation Added new reinstall-cli crate that outputs the new bootc-reinstall binary. Modified the bootc.spec file to generate the new subpackage which includes this binary. This new crate depends on the existing utils crate. Refactored the tracing initialization from the bootc binary into the utils crate so that it can be reused by the new crate. The new CLI can either be configured through commandline flags or through a configuration file in a path set by the environment variable `BOOTC_REINSTALL_CONFIG`. The configuration file is a YAML file. # Limitations Only root SSH keys are supported. The multi user selection TUI is implemented, but if you choose anything other than root you will get an error. # Try Try out instructions: ```bash # Make srpm cargo xtask package-srpm # Mock group sudo usermod -a -G mock $(whoami) newgrp mock # Build RPM for RHEL mock --rebuild -r rhel+epel-9-x86_64 --rebuild target/bootc-*.src.rpm ``` Then install the RPM (`/var/lib/mock/rhel+epel-9-x86_64/result/bootc-reinstall-2*.el9.x86_64.rpm`) on [a rhel9 gcp vm](https://console.cloud.google.com/compute/instanceTemplates/details/rhel9-dev-1?project=bifrost-devel&authuser=1&inv=1&invt=Abn-jg) instance template # TODO Missing docs, missing functionality. Everything is in alpha stage. User choice / SSH keys / prompt disabling should also eventually be supported to be configured through commandline arguments or the configuration file. Signed-off-by: Omer Tuchfeld <[email protected]>

cgwalters

Awesome progress!! Various comments follow

cgwalters · 2025-01-30T13:27:20Z

reinstall-cli/src/podman.rs

+        let root_authorized_keys_path = root_key.authorized_keys_path.clone();
+
+        podman_command_and_args.push("-v".to_string());
+        podman_command_and_args.push(format!(


For later followup: I think we could support --root-ssh-authorized-keys-fd or so, and pass it via file descriptor. That'd avoid the bind dance here.

Or maybe we should learn --ssh-home-dir, and directly scrape all keys from there?

cgwalters · 2025-01-30T13:32:34Z

reinstall-cli/src/prompt.rs

+
+    Ok(selected_user_indices
+        .iter()
+        // Safe unwrap because we know the index is valid


I dug because I was curious what happens if MultiSelect was passed an empty set, looks like it's this: https://github.com/console-rs/dialoguer/blob/6244c77656e8f3e934a2981c5c7e9dacf938a492/src/prompts/multi_select.rs#L211

OK.

cgwalters · 2025-01-30T13:35:48Z

reinstall-cli/src/users.rs

+        }
+
+        let home_dir = user_info.home_dir();
+        let user_authorized_keys_path = home_dir.join(".ssh/authorized_keys");


It's a bit harder than this in the general case; in CoreOS derivatives we've pushed for https://github.com/coreos/ssh-key-dir for example.

cgwalters · 2025-01-30T13:38:03Z

reinstall-cli/src/users.rs

+    let mut result = Vec::new();
+
+    for user_info in iter {
+        if user_info.uid() < 1000 && user_info.uid() != 0 {


Note though 1000 is recommend, it's not required to be the barrier. See https://systemd.io/UIDS-GIDS/

But see below comment about loginctl

cgwalters · 2025-01-30T13:39:25Z

reinstall-cli/src/users.rs

+/// multi-threaded.
+pub(crate) fn get_all_users_keys() -> Result<Vec<UserKeys>> {
+    #[allow(unsafe_code)]
+    let iter = unsafe { uzers::all_users() };


It may be simpler and more reliable here to parse the output of loginctl -j. That will only give us logged in users, which in the common case of non-root user + sudo will give us both, and in the case of just logging in as root won't offer other users, which also seems right.

cgwalters · 2025-01-30T13:45:19Z

reinstall-cli/src/podman.rs

+    .concat()
+}
+
+pub(crate) fn run_podman(all_args: Vec<String>) -> Result<()> {


This isn't specific to running podman? See below

cgwalters · 2025-01-30T13:46:09Z

reinstall-cli/src/podman.rs

+        .status()
+        .context(format!(
+            "Failed to run the command \"{}\"",
+            all_args.join(" ")


I think the debug output of Command correctly handles shell quoting, which is pretty useful because it means you can copy-paste.

It looks like the main goal of this run_podman is really to add the error context from the command so we could just call it run_cmd or something?

But that said, I notice an immediate large problem here in that we're not checking the return code of the child...this type of thing is why I added the CommandRunExt helpers in the bootc-utils crate.

I think we could extend it with something like run_with_cmd_errcontext() or so?

cgwalters · 2025-01-30T13:51:49Z

reinstall-cli/src/main.rs

+    let all_args = podman::command(&config.bootc_image, prompt::get_root_key()?);
+    println!("Going to run command: {}", all_args.join(" "));
+    prompt::temporary_developer_protection_prompt()?;
+    podman::run_podman(all_args)?;


One thing that'd be cool I think is on failure, save all the provided interactive answers in e.g. /run/bootc/reinstall.yaml or so, and tell the user:

Installation answers saved in /run/bootc/reinstall.yaml error: Out of disk space

Then when we re-run we detect that and offer to resume from saved answers.

cgwalters · 2025-01-30T13:58:05Z

reinstall-cli/Cargo.toml

@@ -0,0 +1,31 @@
+[package]
+name = "bootc-reinstall"


By far the biggest thing we need to figure out before trying to land this is one of the hardest problems: naming the thing.

I am not opposed to bootc-reinstall...but I'm not in love with it either. One minor thing I mentioned elsewhere is that if we have both bootc and bootc-reinstall then shells will stop tab completion without adding a space.

I guess in most cases they'll be mutually exclusive? i.e. you only have bootc-reinstall on non-bootc hosts? But I'm sure people will ask us why we don't ship this on bootc images too...

Another option which I like a little better is system-reinstall-bootc - it's longer to type which is a good thing here, doesn't clash with tab completion for bootc, and it feels more "tightly scoped".

Having it not have the bootc prefix also makes clearer that logically it's a higher level tool that in theory could live separately or be maintained elsewhere.

IOW I think people would get legitimately confused if we have bootc install and bootc-reinstall. It seems easier to message system-reinstall-bootc is a standalone tool.

Open to other names too of course...

cgwalters · 2025-01-30T13:59:19Z

reinstall-cli/src/config/mod.rs

+
+        ensure!(
+            value.is_empty(),
+            "unknown keys {:?} in config file",


Confused why we aren't using #[derive(Deserialize)] on the struct and https://serde.rs/container-attrs.html#deny_unknown_fields

openshift-ci bot added the do-not-merge/work-in-progress label Jan 27, 2025

omertuc force-pushed the reinstallcli branch 2 times, most recently from 88934de to a645547 Compare January 27, 2025 14:11

cgwalters reviewed Jan 27, 2025

View reviewed changes

reinstall-cli/Cargo.toml Show resolved Hide resolved

reinstall-cli/Cargo.toml Outdated Show resolved Hide resolved

reinstall-cli/src/main.rs Outdated Show resolved Hide resolved

reinstall-cli/src/main.rs Outdated Show resolved Hide resolved

omertuc force-pushed the reinstallcli branch 11 times, most recently from f6f60e7 to 30b8271 Compare January 29, 2025 23:28

omertuc changed the title ~~WIP: bootc-reinstall CLI~~ cli: add bootc-reinstall binary Jan 29, 2025

omertuc marked this pull request as ready for review January 29, 2025 23:29

openshift-ci bot removed the do-not-merge/work-in-progress label Jan 29, 2025

omertuc force-pushed the reinstallcli branch 4 times, most recently from 6b037d9 to a4d4e6f Compare January 30, 2025 09:23

omertuc force-pushed the reinstallcli branch from a4d4e6f to 7498bdf Compare January 30, 2025 09:57

cgwalters reviewed Jan 30, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: add `bootc-reinstall` binary #1063

cli: add `bootc-reinstall` binary #1063

omertuc commented Jan 27, 2025 •

edited

Loading

cgwalters left a comment

cgwalters left a comment

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025 •

edited

Loading

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cgwalters Jan 30, 2025

cli: add bootc-reinstall binary #1063

Are you sure you want to change the base?

cli: add bootc-reinstall binary #1063

Conversation

omertuc commented Jan 27, 2025 • edited Loading

Background

Issue

Solution

Implementation

Limitations

Try

TODO

Demo

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgwalters Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cli: add `bootc-reinstall` binary #1063

cli: add `bootc-reinstall` binary #1063

omertuc commented Jan 27, 2025 •

edited

Loading

cgwalters Jan 30, 2025 •

edited

Loading