install feature is always on, add install-to-disk

I consider `bootc install to-filesystem` support a key feature of bootc.
In theory today one can still set up a system directly with `ostree`
and we will continue to support that.

But things like logically bound images we do want to be initialized
from the start and that's with `bootc install to-filesystem`.

Maintaining the feature just has a logistical annoyance any
time one touches the install code as we often end up needing
to carefully `#[cfg(feature = "install")]` in many places
in an infectious way.

Also as we head towards enabling factory reset
#404
we really do want some of the install code enabled there.

However, `to-disk` is much more of a "demo". I don't want
bootc to grow too much knowledge around block devices. Complex
setups (LVM, LUKS) etc. are the domain of external installers
and provisioning tools.

So the feature gate is now on that (which is still on by default).

We ended up with more `#[cfg(feature = "install-to-disk")]` than
I'd have liked, but some of that can be fixed subsequently.

Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

lib/Cargo.toml

@@ -53,9 +53,11 @@ similar-asserts = { workspace = true }
 static_assertions = { workspace = true }
 
 [features]
-default = ["install"]
-# This feature enables `bootc install`.  Disable if you always want to use an external installer.
-install = []
+default = ["install-to-disk"]
+# This feature enables `bootc install to-disk`, which is considered just a "demo"
+# or reference installer; we expect most nontrivial use cases to be using
+# `bootc install to-filesystem`.
+install-to-disk = []
 # This featuares enables `bootc internals publish-rhsm-facts` to integrate with
 # Red Hat Subscription Manager
 rhsm = []

lib/src/blockdev.rs

@@ -1,15 +1,20 @@
 use std::collections::HashMap;
+#[cfg(feature = "install-to-disk")]
 use std::env;
+#[cfg(feature = "install-to-disk")]
 use std::path::Path;
 use std::process::Command;
 use std::sync::OnceLock;
 
 use anyhow::{anyhow, Context, Result};
-use camino::{Utf8Path, Utf8PathBuf};
+use camino::Utf8Path;
+#[cfg(feature = "install-to-disk")]
+use camino::Utf8PathBuf;
 use fn_error_context::context;
 use regex::Regex;
 use serde::Deserialize;
 
+#[cfg(feature = "install-to-disk")]
 use crate::install::run_in_host_mountns;
 use crate::task::Task;
 use bootc_utils::CommandRunExt;
@@ -47,6 +52,7 @@ impl Device {
         self.path.clone().unwrap_or(format!("/dev/{}", &self.name))
     }
 
+    #[allow(dead_code)]
     pub(crate) fn has_children(&self) -> bool {
         self.children.as_ref().map_or(false, |v| !v.is_empty())
     }
@@ -86,6 +92,7 @@ impl Device {
 }
 
 #[context("Failed to wipe {dev}")]
+#[cfg(feature = "install-to-disk")]
 pub(crate) fn wipefs(dev: &Utf8Path) -> Result<()> {
     Task::new_and_run(
         format!("Wiping device {dev}"),
@@ -161,6 +168,7 @@ impl PartitionTable {
     }
 
     // Find the partition with the given offset (starting at 1)
+    #[allow(dead_code)]
     pub(crate) fn find_partno(&self, partno: u32) -> Result<&Partition> {
         let r = self
             .partitions
@@ -171,6 +179,7 @@ impl PartitionTable {
 }
 
 impl Partition {
+    #[allow(dead_code)]
     pub(crate) fn path(&self) -> &Utf8Path {
         self.node.as_str().into()
     }
@@ -185,10 +194,12 @@ pub(crate) fn partitions_of(dev: &Utf8Path) -> Result<PartitionTable> {
     Ok(o.partitiontable)
 }
 
+#[cfg(feature = "install-to-disk")]
 pub(crate) struct LoopbackDevice {
     pub(crate) dev: Option<Utf8PathBuf>,
 }
 
+#[cfg(feature = "install-to-disk")]
 impl LoopbackDevice {
     // Create a new loopback block device targeting the provided file path.
     pub(crate) fn new(path: &Path) -> Result<Self> {
@@ -243,13 +254,15 @@ impl LoopbackDevice {
     }
 }
 
+#[cfg(feature = "install-to-disk")]
 impl Drop for LoopbackDevice {
     fn drop(&mut self) {
         // Best effort to unmount if we're dropped without invoking `close`
         let _ = self.impl_close();
     }
 }
 
+#[cfg(feature = "install-to-disk")]
 pub(crate) fn udev_settle() -> Result<()> {
     // There's a potential window after rereading the partition table where
     // udevd hasn't yet received updates from the kernel, settle will return
@@ -311,6 +324,7 @@ pub(crate) fn find_parent_devices(device: &str) -> Result<Vec<String>> {
 }
 
 /// Parse a string into mibibytes
+#[cfg(feature = "install-to-disk")]
 pub(crate) fn parse_size_mib(mut s: &str) -> Result<u64> {
     let suffixes = [
         ("MiB", 1u64),

lib/src/bootloader.rs

@@ -7,8 +7,11 @@ use crate::task::Task;
 
 /// The name of the mountpoint for efi (as a subdirectory of /boot, or at the toplevel)
 pub(crate) const EFI_DIR: &str = "efi";
+#[cfg(feature = "install-to-disk")]
 pub(crate) const ESP_GUID: &str = "C12A7328-F81F-11D2-BA4B-00A0C93EC93B";
+#[cfg(feature = "install-to-disk")]
 pub(crate) const PREPBOOT_GUID: &str = "9E1A2D38-C612-4316-AA26-8B49521E5A8B";
+#[cfg(feature = "install-to-disk")]
 pub(crate) const PREPBOOT_LABEL: &str = "PowerPC-PReP-boot";
 #[cfg(target_arch = "powerpc64")]
 /// We make a best-effort to support MBR partitioning too.

lib/src/boundimage.rs

@@ -10,7 +10,6 @@ use camino::Utf8Path;
 use cap_std_ext::cap_std::fs::Dir;
 use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
-#[cfg(feature = "install")]
 use ostree_ext::containers_image_proxy;
 use ostree_ext::ostree::Deployment;
 
@@ -33,7 +32,6 @@ pub(crate) struct BoundImage {
 }
 
 #[derive(Debug, PartialEq, Eq)]
-#[cfg(feature = "install")]
 pub(crate) struct ResolvedBoundImage {
     pub(crate) image: String,
     pub(crate) digest: String,
@@ -104,7 +102,6 @@ pub(crate) fn query_bound_images(root: &Dir) -> Result<Vec<BoundImage>> {
     Ok(bound_images)
 }
 
-#[cfg(feature = "install")]
 impl ResolvedBoundImage {
     #[context("resolving bound image {}", src.image)]
     pub(crate) async fn from_image(src: &BoundImage) -> Result<Self> {

lib/src/cli.rs

@@ -184,7 +184,6 @@ pub(crate) struct StatusOpts {
     pub(crate) booted: bool,
 }
 
-#[cfg(feature = "install")]
 #[derive(Debug, clap::Subcommand, PartialEq, Eq)]
 pub(crate) enum InstallOpts {
     /// Install to the target block device.
@@ -197,6 +196,7 @@ pub(crate) enum InstallOpts {
     /// in the container image, alongside any required system partitions such as
     /// the EFI system partition. Use `install to-filesystem` for anything more
     /// complex such as RAID, LVM, LUKS etc.
+    #[cfg(feature = "install-to-disk")]
     ToDisk(crate::install::InstallToDiskOpts),
     /// Install to an externally created filesystem structure.
     ///
@@ -384,7 +384,6 @@ pub(crate) enum InternalsOpts {
         #[clap(allow_hyphen_values = true)]
         args: Vec<OsString>,
     },
-    #[cfg(feature = "install")]
     /// Invoked from ostree-ext to complete an installation.
     BootcInstallCompletion {
         /// Path to the sysroot
@@ -525,7 +524,6 @@ pub(crate) enum Opt {
     /// An installation is not simply a copy of the container filesystem, but includes
     /// other setup and metadata.
     #[clap(subcommand)]
-    #[cfg(feature = "install")]
     Install(InstallOpts),
     /// Operations which can be executed as part of a container build.
     #[clap(subcommand)]
@@ -537,7 +535,6 @@ pub(crate) enum Opt {
     #[clap(subcommand, hide = true)]
     Image(ImageOpts),
     /// Execute the given command in the host mount namespace
-    #[cfg(feature = "install")]
     #[clap(hide = true)]
     ExecInHostMountNamespace {
         #[clap(trailing_var_arg = true, allow_hyphen_values = true)]
@@ -1053,8 +1050,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
                 }
             }
         },
-        #[cfg(feature = "install")]
         Opt::Install(opts) => match opts {
+            #[cfg(feature = "install-to-disk")]
             InstallOpts::ToDisk(opts) => crate::install::install_to_disk(opts).await,
             InstallOpts::ToFilesystem(opts) => {
                 crate::install::install_to_filesystem(opts, false).await
@@ -1068,7 +1065,6 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
                 crate::install::completion::run_from_anaconda(rootfs).await
             }
         },
-        #[cfg(feature = "install")]
         Opt::ExecInHostMountNamespace { args } => {
             crate::install::exec_in_host_mountns(args.as_slice())
         }
@@ -1104,7 +1100,6 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
                 let sysroot = get_storage().await?;
                 crate::deploy::cleanup(&sysroot).await
             }
-            #[cfg(feature = "install")]
             InternalsOpts::BootcInstallCompletion { sysroot, stateroot } => {
                 let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
                 crate::install::completion::run_from_ostree(rootfs, &sysroot, &stateroot).await

lib/src/install.rs

@@ -6,6 +6,7 @@
 
 // This sub-module is the "basic" installer that handles creating basic block device
 // and filesystem setup.
+#[cfg(feature = "install-to-disk")]
 pub(crate) mod baseline;
 pub(crate) mod completion;
 pub(crate) mod config;
@@ -40,9 +41,12 @@ use ostree_ext::oci_spec;
 use ostree_ext::ostree;
 use ostree_ext::prelude::Cast;
 use ostree_ext::sysroot::SysrootLock;
-use rustix::fs::{FileTypeExt, MetadataExt as _};
+#[cfg(feature = "install-to-disk")]
+use rustix::fs::FileTypeExt;
+use rustix::fs::MetadataExt as _;
 use serde::{Deserialize, Serialize};
 
+#[cfg(feature = "install-to-disk")]
 use self::baseline::InstallBlockDeviceOpts;
 use crate::boundimage::{BoundImage, ResolvedBoundImage};
 use crate::containerenv::ContainerExecutionInfo;
@@ -57,6 +61,7 @@ use crate::utils::sigpolicy_from_opts;
 /// The toplevel boot directory
 const BOOT: &str = "boot";
 /// Directory for transient runtime state
+#[cfg(feature = "install-to-disk")]
 const RUN_BOOTC: &str = "/run/bootc";
 /// The default path for the host rootfs
 const ALONGSIDE_ROOT_MOUNT: &str = "/target";
@@ -65,10 +70,8 @@ const LOST_AND_FOUND: &str = "lost+found";
 /// The filename of the composefs EROFS superblock; TODO move this into ostree
 const OSTREE_COMPOSEFS_SUPER: &str = ".ostree.cfs";
 /// The mount path for selinux
-#[cfg(feature = "install")]
 const SELINUXFS: &str = "/sys/fs/selinux";
 /// The mount path for uefi
-#[cfg(feature = "install")]
 const EFIVARFS: &str = "/sys/firmware/efi/efivars";
 pub(crate) const ARCH_USES_EFI: bool = cfg!(any(target_arch = "x86_64", target_arch = "aarch64"));
 
@@ -202,6 +205,7 @@ pub(crate) struct InstallConfigOpts {
     pub(crate) stateroot: Option<String>,
 }
 
+#[cfg(feature = "install-to-disk")]
 #[derive(Debug, Clone, clap::Parser, Serialize, Deserialize, PartialEq, Eq)]
 pub(crate) struct InstallToDiskOpts {
     #[clap(flatten)]
@@ -375,6 +379,7 @@ impl State {
     }
 
     #[context("Finalizing state")]
+    #[allow(dead_code)]
     pub(crate) fn consume(self) -> Result<()> {
         self.tempdir.close()?;
         // If we had invoked `setenforce 0`, then let's re-enable it.
@@ -880,6 +885,7 @@ pub(crate) fn exec_in_host_mountns(args: &[std::ffi::OsString]) -> Result<()> {
 }
 
 pub(crate) struct RootSetup {
+    #[cfg(feature = "install-to-disk")]
     luks_device: Option<String>,
     device_info: crate::blockdev::PartitionTable,
     /// Absolute path to the location where we've mounted the physical
@@ -907,12 +913,14 @@ impl RootSetup {
     }
 
     // Drop any open file descriptors and return just the mount path and backing luks device, if any
+    #[cfg(feature = "install-to-disk")]
     fn into_storage(self) -> (Utf8PathBuf, Option<String>) {
         (self.physical_root_path, self.luks_device)
     }
 }
 
 #[derive(Debug)]
+#[allow(dead_code)]
 pub(crate) enum SELinuxFinalState {
     /// Host and target both have SELinux, but user forced it off for target
     ForceTargetDisabled,
@@ -1449,6 +1457,7 @@ fn installation_complete() {
 
 /// Implementation of the `bootc install to-disk` CLI command.
 #[context("Installing to disk")]
+#[cfg(feature = "install-to-disk")]
 pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
     let mut block_opts = opts.block_opts;
     let target_blockdev_meta = block_opts
@@ -1845,6 +1854,7 @@ pub(crate) async fn install_to_filesystem(
     let skip_finalize =
         matches!(fsopts.replace, Some(ReplaceMode::Alongside)) || fsopts.skip_finalize;
     let mut rootfs = RootSetup {
+        #[cfg(feature = "install-to-disk")]
         luks_device: None,
         device_info,
         physical_root_path: fsopts.root_path,

lib/src/install/baseline.rs

@@ -21,12 +21,14 @@ use clap::ValueEnum;
 use fn_error_context::context;
 use serde::{Deserialize, Serialize};
 
+use super::config::Filesystem;
 use super::MountSpec;
 use super::RootSetup;
 use super::State;
 use super::RUN_BOOTC;
 use super::RW_KARG;
 use crate::mount;
+#[cfg(feature = "install-to-disk")]
 use crate::mount::is_mounted_in_pid1_mountns;
 use crate::task::Task;
 
@@ -36,20 +38,6 @@ pub(crate) const EFIPN_SIZE_MB: u32 = 512;
 /// The GPT type for "linux"
 pub(crate) const LINUX_PARTTYPE: &str = "0FC63DAF-8483-4772-8E79-3D69D8477DE4";
 
-#[derive(clap::ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-pub(crate) enum Filesystem {
-    Xfs,
-    Ext4,
-    Btrfs,
-}
-
-impl Display for Filesystem {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        self.to_possible_value().unwrap().get_name().fmt(f)
-    }
-}
-
 #[derive(clap::ValueEnum, Default, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 pub(crate) enum BlockSetup {
@@ -104,6 +92,7 @@ impl BlockSetup {
     }
 }
 
+#[cfg(feature = "install-to-disk")]
 fn mkfs<'a>(
     dev: &str,
     fs: Filesystem,
@@ -143,6 +132,7 @@ fn mkfs<'a>(
 }
 
 #[context("Creating rootfs")]
+#[cfg(feature = "install-to-disk")]
 pub(crate) fn install_create_rootfs(
     state: &State,
     opts: InstallBlockDeviceOpts,