Skip to content

Commit

Permalink
Pass env properties to merge function
Browse files Browse the repository at this point in the history
Required for containers#401

Adds a `EnvProperties` struct to allow for conditional merging of kargs
depending on whether the archiecture matches. Future properties such as
`platform.id` can be added in the future.

Signed-off-by: Luke Yang <luyang@redhat.com>
  • Loading branch information
lukewarmtemp committed May 27, 2024
1 parent d5efa17 commit 674a7e9
Showing 1 changed file with 60 additions and 21 deletions.
81 changes: 60 additions & 21 deletions lib/src/install/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ use serde::{Deserialize, Serialize};

use super::baseline::BlockSetup;

/// Properties of the environment, such as the system architecture
/// Left open for future properties such as `platform.id`
pub(crate) struct EnvProperties {
pub(crate) sys_arch: String,
}

/// The toplevel config entry for installation configs stored
/// in bootc/install (e.g. /etc/bootc/install/05-custom.toml)
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
Expand Down Expand Up @@ -47,16 +53,18 @@ pub(crate) struct InstallConfiguration {
/// Kernel arguments, applied at installation time
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) kargs: Option<Vec<String>>,
/// Supported architectures for this configuration
pub(crate) supported_arch: Option<Vec<String>>,
}

fn merge_basic<T>(s: &mut Option<T>, o: Option<T>) {
fn merge_basic<T>(s: &mut Option<T>, o: Option<T>, env: &EnvProperties) {
if let Some(o) = o {
*s = Some(o);
}
}

trait Mergeable {
fn merge(&mut self, other: Self)
fn merge(&mut self, other: Self, env: &EnvProperties)
where
Self: Sized;
}
Expand All @@ -65,13 +73,13 @@ impl<T> Mergeable for Option<T>
where
T: Mergeable,
{
fn merge(&mut self, other: Self)
fn merge(&mut self, other: Self, env: &EnvProperties)
where
Self: Sized,
{
if let Some(other) = other {
if let Some(s) = self.as_mut() {
s.merge(other)
s.merge(other, env)
} else {
*self = Some(other);
}
Expand All @@ -81,28 +89,38 @@ where

impl Mergeable for RootFS {
/// Apply any values in other, overriding any existing values in `self`.
fn merge(&mut self, other: Self) {
merge_basic(&mut self.fstype, other.fstype)
fn merge(&mut self, other: Self, env: &EnvProperties) {
merge_basic(&mut self.fstype, other.fstype, env)
}
}

impl Mergeable for BasicFilesystems {
/// Apply any values in other, overriding any existing values in `self`.
fn merge(&mut self, other: Self) {
self.root.merge(other.root)
fn merge(&mut self, other: Self, env: &EnvProperties) {
self.root.merge(other.root, env)
}
}

impl Mergeable for InstallConfiguration {
/// Apply any values in other, overriding any existing values in `self`.
fn merge(&mut self, other: Self) {
merge_basic(&mut self.root_fs_type, other.root_fs_type);
merge_basic(&mut self.block, other.block);
self.filesystem.merge(other.filesystem);
fn merge(&mut self, other: Self, env: &EnvProperties) {
merge_basic(&mut self.root_fs_type, other.root_fs_type, env);
merge_basic(&mut self.block, other.block, env);
self.filesystem.merge(other.filesystem, env);
if let Some(other_kargs) = other.kargs {
self.kargs
.get_or_insert_with(Default::default)
.extend(other_kargs)
// if arch is specified, only apply kargs if it matches the current arch
// if arch is not specified, apply kargs unconditionally
if let Some(other_arch) = other.supported_arch {
if other_arch.contains(&env.sys_arch) {
self.kargs
.get_or_insert_with(Default::default)
.extend(other_kargs.clone());
}
} else {
self.kargs
.get_or_insert_with(Default::default)
.extend(other_kargs)
}
}
}
}
Expand Down Expand Up @@ -154,14 +172,17 @@ impl InstallConfiguration {
#[context("Loading configuration")]
/// Load the install configuration, merging all found configuration files.
pub(crate) fn load_config() -> Result<Option<InstallConfiguration>> {
let env = EnvProperties {
sys_arch: std::env::consts::ARCH.to_string(),
};
const SYSTEMD_CONVENTIONAL_BASES: &[&str] = &["/usr/lib", "/usr/local/lib", "/etc", "/run"];
let fragments = liboverdrop::scan(SYSTEMD_CONVENTIONAL_BASES, "bootc/install", &["toml"], true);
let mut config: Option<InstallConfiguration> = None;
for (_name, path) in fragments {
let buf = std::fs::read_to_string(&path)?;
let mut unused = std::collections::HashSet::new();
let de = toml::Deserializer::new(&buf);
let c: InstallConfigurationToplevel = serde_ignored::deserialize(de, |path| {
let mut c: InstallConfigurationToplevel = serde_ignored::deserialize(de, |path| {
unused.insert(path.to_string());
})
.with_context(|| format!("Parsing {path:?}"))?;
Expand All @@ -171,9 +192,18 @@ pub(crate) fn load_config() -> Result<Option<InstallConfiguration>> {
if let Some(config) = config.as_mut() {
if let Some(install) = c.install {
tracing::debug!("Merging install config: {install:?}");
config.merge(install);
config.merge(install, &env);
}
} else {
// If arch is specified and doesn't match the current arch, remove kargs
// We need to add this since the merge function isn't applied for the first config file
if let Some(ref mut install) = c.install {
if let Some(arch) = install.supported_arch.as_ref() {
if !arch.contains(&env.sys_arch) {
install.kargs = None;
}
}
}
config = c.install;
}
}
Expand All @@ -188,6 +218,9 @@ pub(crate) fn load_config() -> Result<Option<InstallConfiguration>> {
fn test_parse_config() {
use super::baseline::Filesystem;

let env = EnvProperties {
sys_arch: "x86_64".to_string(),
};
let c: InstallConfigurationToplevel = toml::from_str(
r##"[install]
root-fs-type = "xfs"
Expand All @@ -202,7 +235,7 @@ root-fs-type = "xfs"
..Default::default()
}),
};
install.merge(other.install.unwrap());
install.merge(other.install.unwrap(), &env);
assert_eq!(
install.root_fs_type.as_ref().copied().unwrap(),
Filesystem::Ext4
Expand Down Expand Up @@ -236,7 +269,7 @@ kargs = ["console=ttyS0", "foo=bar"]
..Default::default()
}),
};
install.merge(other.install.unwrap());
install.merge(other.install.unwrap(), &env);
assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4);
assert_eq!(
install.kargs,
Expand All @@ -252,6 +285,9 @@ kargs = ["console=ttyS0", "foo=bar"]
#[test]
fn test_parse_filesystems() {
use super::baseline::Filesystem;
let env = EnvProperties {
sys_arch: "x86_64".to_string(),
};
let c: InstallConfigurationToplevel = toml::from_str(
r##"[install.filesystem.root]
type = "xfs"
Expand All @@ -273,7 +309,7 @@ type = "xfs"
..Default::default()
}),
};
install.merge(other.install.unwrap());
install.merge(other.install.unwrap(), &env);
assert_eq!(
install.filesystem_root().unwrap().fstype.unwrap(),
Filesystem::Ext4
Expand All @@ -282,6 +318,9 @@ type = "xfs"

#[test]
fn test_parse_block() {
let env = EnvProperties {
sys_arch: "x86_64".to_string(),
};
let c: InstallConfigurationToplevel = toml::from_str(
r##"[install.filesystem.root]
type = "xfs"
Expand All @@ -301,7 +340,7 @@ type = "xfs"
..Default::default()
}),
};
install.merge(other.install.unwrap());
install.merge(other.install.unwrap(), &env);
// Should be set, but zero length
assert_eq!(install.block.as_ref().unwrap().len(), 0);
assert!(install.get_block_setup(None).is_err());
Expand Down

0 comments on commit 674a7e9

Please sign in to comment.