Skip to content

Commit 614b14f

Browse files
committed
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 <walters@verbum.org>
1 parent f95d43c commit 614b14f

File tree

15 files changed

+71
-81
lines changed

15 files changed

+71
-81
lines changed

lib/Cargo.toml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,11 @@ similar-asserts = { workspace = true }
5353
static_assertions = { workspace = true }
5454

5555
[features]
56-
default = ["install"]
57-
# This feature enables `bootc install`. Disable if you always want to use an external installer.
58-
install = []
56+
default = ["install-to-disk"]
57+
# This feature enables `bootc install to-disk`, which is considered just a "demo"
58+
# or reference installer; we expect most nontrivial use cases to be using
59+
# `bootc install to-filesystem`.
60+
install-to-disk = []
5961
# Implementation detail of man page generation.
6062
docgen = ["clap_mangen"]
6163

lib/src/blockdev.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
use std::collections::HashMap;
2+
#[cfg(feature = "install-to-disk")]
23
use std::env;
4+
#[cfg(feature = "install-to-disk")]
35
use std::path::Path;
46
use std::process::Command;
57
use std::sync::OnceLock;
68

79
use anyhow::{anyhow, Context, Result};
8-
use camino::{Utf8Path, Utf8PathBuf};
10+
use camino::Utf8Path;
11+
#[cfg(feature = "install-to-disk")]
12+
use camino::Utf8PathBuf;
913
use fn_error_context::context;
1014
use regex::Regex;
1115
use serde::Deserialize;
1216

17+
#[cfg(feature = "install-to-disk")]
1318
use crate::install::run_in_host_mountns;
1419
use crate::task::Task;
1520
use bootc_utils::CommandRunExt;
@@ -47,6 +52,7 @@ impl Device {
4752
self.path.clone().unwrap_or(format!("/dev/{}", &self.name))
4853
}
4954

55+
#[allow(dead_code)]
5056
pub(crate) fn has_children(&self) -> bool {
5157
self.children.as_ref().map_or(false, |v| !v.is_empty())
5258
}
@@ -86,6 +92,7 @@ impl Device {
8692
}
8793

8894
#[context("Failed to wipe {dev}")]
95+
#[cfg(feature = "install-to-disk")]
8996
pub(crate) fn wipefs(dev: &Utf8Path) -> Result<()> {
9097
Task::new_and_run(
9198
format!("Wiping device {dev}"),
@@ -161,6 +168,7 @@ impl PartitionTable {
161168
}
162169

163170
// Find the partition with the given offset (starting at 1)
171+
#[allow(dead_code)]
164172
pub(crate) fn find_partno(&self, partno: u32) -> Result<&Partition> {
165173
let r = self
166174
.partitions
@@ -171,6 +179,7 @@ impl PartitionTable {
171179
}
172180

173181
impl Partition {
182+
#[allow(dead_code)]
174183
pub(crate) fn path(&self) -> &Utf8Path {
175184
self.node.as_str().into()
176185
}
@@ -185,10 +194,12 @@ pub(crate) fn partitions_of(dev: &Utf8Path) -> Result<PartitionTable> {
185194
Ok(o.partitiontable)
186195
}
187196

197+
#[cfg(feature = "install-to-disk")]
188198
pub(crate) struct LoopbackDevice {
189199
pub(crate) dev: Option<Utf8PathBuf>,
190200
}
191201

202+
#[cfg(feature = "install-to-disk")]
192203
impl LoopbackDevice {
193204
// Create a new loopback block device targeting the provided file path.
194205
pub(crate) fn new(path: &Path) -> Result<Self> {
@@ -243,13 +254,15 @@ impl LoopbackDevice {
243254
}
244255
}
245256

257+
#[cfg(feature = "install-to-disk")]
246258
impl Drop for LoopbackDevice {
247259
fn drop(&mut self) {
248260
// Best effort to unmount if we're dropped without invoking `close`
249261
let _ = self.impl_close();
250262
}
251263
}
252264

265+
#[cfg(feature = "install-to-disk")]
253266
pub(crate) fn udev_settle() -> Result<()> {
254267
// There's a potential window after rereading the partition table where
255268
// 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>> {
311324
}
312325

313326
/// Parse a string into mibibytes
327+
#[cfg(feature = "install-to-disk")]
314328
pub(crate) fn parse_size_mib(mut s: &str) -> Result<u64> {
315329
let suffixes = [
316330
("MiB", 1u64),

lib/src/bootloader.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ use crate::task::Task;
77

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

lib/src/boundimage.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use camino::Utf8Path;
1010
use cap_std_ext::cap_std::fs::Dir;
1111
use cap_std_ext::dirext::CapStdExtDirExt;
1212
use fn_error_context::context;
13-
#[cfg(feature = "install")]
1413
use ostree_ext::containers_image_proxy;
1514
use ostree_ext::ostree::Deployment;
1615

@@ -33,7 +32,6 @@ pub(crate) struct BoundImage {
3332
}
3433

3534
#[derive(Debug, PartialEq, Eq)]
36-
#[cfg(feature = "install")]
3735
pub(crate) struct ResolvedBoundImage {
3836
pub(crate) image: String,
3937
pub(crate) digest: String,
@@ -104,7 +102,6 @@ pub(crate) fn query_bound_images(root: &Dir) -> Result<Vec<BoundImage>> {
104102
Ok(bound_images)
105103
}
106104

107-
#[cfg(feature = "install")]
108105
impl ResolvedBoundImage {
109106
#[context("resolving bound image {}", src.image)]
110107
pub(crate) async fn from_image(src: &BoundImage) -> Result<Self> {

lib/src/cli.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,6 @@ pub(crate) struct StatusOpts {
184184
pub(crate) booted: bool,
185185
}
186186

187-
#[cfg(feature = "install")]
188187
#[derive(Debug, clap::Subcommand, PartialEq, Eq)]
189188
pub(crate) enum InstallOpts {
190189
/// Install to the target block device.
@@ -197,6 +196,7 @@ pub(crate) enum InstallOpts {
197196
/// in the container image, alongside any required system partitions such as
198197
/// the EFI system partition. Use `install to-filesystem` for anything more
199198
/// complex such as RAID, LVM, LUKS etc.
199+
#[cfg(feature = "install-to-disk")]
200200
ToDisk(crate::install::InstallToDiskOpts),
201201
/// Install to an externally created filesystem structure.
202202
///
@@ -384,7 +384,6 @@ pub(crate) enum InternalsOpts {
384384
#[clap(allow_hyphen_values = true)]
385385
args: Vec<OsString>,
386386
},
387-
#[cfg(feature = "install")]
388387
/// Invoked from ostree-ext to complete an installation.
389388
BootcInstallCompletion {
390389
/// Path to the sysroot
@@ -522,7 +521,6 @@ pub(crate) enum Opt {
522521
/// An installation is not simply a copy of the container filesystem, but includes
523522
/// other setup and metadata.
524523
#[clap(subcommand)]
525-
#[cfg(feature = "install")]
526524
Install(InstallOpts),
527525
/// Operations which can be executed as part of a container build.
528526
#[clap(subcommand)]
@@ -534,7 +532,6 @@ pub(crate) enum Opt {
534532
#[clap(subcommand, hide = true)]
535533
Image(ImageOpts),
536534
/// Execute the given command in the host mount namespace
537-
#[cfg(feature = "install")]
538535
#[clap(hide = true)]
539536
ExecInHostMountNamespace {
540537
#[clap(trailing_var_arg = true, allow_hyphen_values = true)]
@@ -1044,8 +1041,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
10441041
}
10451042
}
10461043
},
1047-
#[cfg(feature = "install")]
10481044
Opt::Install(opts) => match opts {
1045+
#[cfg(feature = "install-to-disk")]
10491046
InstallOpts::ToDisk(opts) => crate::install::install_to_disk(opts).await,
10501047
InstallOpts::ToFilesystem(opts) => {
10511048
crate::install::install_to_filesystem(opts, false).await
@@ -1059,7 +1056,6 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
10591056
crate::install::completion::run_from_anaconda(rootfs).await
10601057
}
10611058
},
1062-
#[cfg(feature = "install")]
10631059
Opt::ExecInHostMountNamespace { args } => {
10641060
crate::install::exec_in_host_mountns(args.as_slice())
10651061
}
@@ -1095,7 +1091,6 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
10951091
let sysroot = get_storage().await?;
10961092
crate::deploy::cleanup(&sysroot).await
10971093
}
1098-
#[cfg(feature = "install")]
10991094
InternalsOpts::BootcInstallCompletion { sysroot, stateroot } => {
11001095
let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
11011096
crate::install::completion::run_from_ostree(rootfs, &sysroot, &stateroot).await

lib/src/install.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
77
// This sub-module is the "basic" installer that handles creating basic block device
88
// and filesystem setup.
9+
#[cfg(feature = "install-to-disk")]
910
pub(crate) mod baseline;
1011
pub(crate) mod completion;
1112
pub(crate) mod config;
@@ -40,9 +41,12 @@ use ostree_ext::oci_spec;
4041
use ostree_ext::ostree;
4142
use ostree_ext::prelude::Cast;
4243
use ostree_ext::sysroot::SysrootLock;
43-
use rustix::fs::{FileTypeExt, MetadataExt as _};
44+
#[cfg(feature = "install-to-disk")]
45+
use rustix::fs::FileTypeExt;
46+
use rustix::fs::MetadataExt as _;
4447
use serde::{Deserialize, Serialize};
4548

49+
#[cfg(feature = "install-to-disk")]
4650
use self::baseline::InstallBlockDeviceOpts;
4751
use crate::boundimage::{BoundImage, ResolvedBoundImage};
4852
use crate::containerenv::ContainerExecutionInfo;
@@ -57,6 +61,7 @@ use crate::utils::sigpolicy_from_opts;
5761
/// The toplevel boot directory
5862
const BOOT: &str = "boot";
5963
/// Directory for transient runtime state
64+
#[cfg(feature = "install-to-disk")]
6065
const RUN_BOOTC: &str = "/run/bootc";
6166
/// The default path for the host rootfs
6267
const ALONGSIDE_ROOT_MOUNT: &str = "/target";
@@ -65,10 +70,8 @@ const LOST_AND_FOUND: &str = "lost+found";
6570
/// The filename of the composefs EROFS superblock; TODO move this into ostree
6671
const OSTREE_COMPOSEFS_SUPER: &str = ".ostree.cfs";
6772
/// The mount path for selinux
68-
#[cfg(feature = "install")]
6973
const SELINUXFS: &str = "/sys/fs/selinux";
7074
/// The mount path for uefi
71-
#[cfg(feature = "install")]
7275
const EFIVARFS: &str = "/sys/firmware/efi/efivars";
7376
pub(crate) const ARCH_USES_EFI: bool = cfg!(any(target_arch = "x86_64", target_arch = "aarch64"));
7477

@@ -202,6 +205,7 @@ pub(crate) struct InstallConfigOpts {
202205
pub(crate) stateroot: Option<String>,
203206
}
204207

208+
#[cfg(feature = "install-to-disk")]
205209
#[derive(Debug, Clone, clap::Parser, Serialize, Deserialize, PartialEq, Eq)]
206210
pub(crate) struct InstallToDiskOpts {
207211
#[clap(flatten)]
@@ -375,6 +379,7 @@ impl State {
375379
}
376380

377381
#[context("Finalizing state")]
382+
#[allow(dead_code)]
378383
pub(crate) fn consume(self) -> Result<()> {
379384
self.tempdir.close()?;
380385
// 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<()> {
880885
}
881886

882887
pub(crate) struct RootSetup {
888+
#[cfg(feature = "install-to-disk")]
883889
luks_device: Option<String>,
884890
device_info: crate::blockdev::PartitionTable,
885891
/// Absolute path to the location where we've mounted the physical
@@ -907,12 +913,14 @@ impl RootSetup {
907913
}
908914

909915
// Drop any open file descriptors and return just the mount path and backing luks device, if any
916+
#[cfg(feature = "install-to-disk")]
910917
fn into_storage(self) -> (Utf8PathBuf, Option<String>) {
911918
(self.physical_root_path, self.luks_device)
912919
}
913920
}
914921

915922
#[derive(Debug)]
923+
#[allow(dead_code)]
916924
pub(crate) enum SELinuxFinalState {
917925
/// Host and target both have SELinux, but user forced it off for target
918926
ForceTargetDisabled,
@@ -1449,6 +1457,7 @@ fn installation_complete() {
14491457

14501458
/// Implementation of the `bootc install to-disk` CLI command.
14511459
#[context("Installing to disk")]
1460+
#[cfg(feature = "install-to-disk")]
14521461
pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
14531462
let mut block_opts = opts.block_opts;
14541463
let target_blockdev_meta = block_opts
@@ -1845,6 +1854,7 @@ pub(crate) async fn install_to_filesystem(
18451854
let skip_finalize =
18461855
matches!(fsopts.replace, Some(ReplaceMode::Alongside)) || fsopts.skip_finalize;
18471856
let mut rootfs = RootSetup {
1857+
#[cfg(feature = "install-to-disk")]
18481858
luks_device: None,
18491859
device_info,
18501860
physical_root_path: fsopts.root_path,

lib/src/install/baseline.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ use clap::ValueEnum;
2121
use fn_error_context::context;
2222
use serde::{Deserialize, Serialize};
2323

24+
use super::config::Filesystem;
2425
use super::MountSpec;
2526
use super::RootSetup;
2627
use super::State;
2728
use super::RUN_BOOTC;
2829
use super::RW_KARG;
2930
use crate::mount;
31+
#[cfg(feature = "install-to-disk")]
3032
use crate::mount::is_mounted_in_pid1_mountns;
3133
use crate::task::Task;
3234

@@ -36,20 +38,6 @@ pub(crate) const EFIPN_SIZE_MB: u32 = 512;
3638
/// The GPT type for "linux"
3739
pub(crate) const LINUX_PARTTYPE: &str = "0FC63DAF-8483-4772-8E79-3D69D8477DE4";
3840

39-
#[derive(clap::ValueEnum, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
40-
#[serde(rename_all = "kebab-case")]
41-
pub(crate) enum Filesystem {
42-
Xfs,
43-
Ext4,
44-
Btrfs,
45-
}
46-
47-
impl Display for Filesystem {
48-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
49-
self.to_possible_value().unwrap().get_name().fmt(f)
50-
}
51-
}
52-
5341
#[derive(clap::ValueEnum, Default, Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
5442
#[serde(rename_all = "kebab-case")]
5543
pub(crate) enum BlockSetup {
@@ -104,6 +92,7 @@ impl BlockSetup {
10492
}
10593
}
10694

95+
#[cfg(feature = "install-to-disk")]
10796
fn mkfs<'a>(
10897
dev: &str,
10998
fs: Filesystem,
@@ -143,6 +132,7 @@ fn mkfs<'a>(
143132
}
144133

145134
#[context("Creating rootfs")]
135+
#[cfg(feature = "install-to-disk")]
146136
pub(crate) fn install_create_rootfs(
147137
state: &State,
148138
opts: InstallBlockDeviceOpts,

0 commit comments

Comments
 (0)