[pve-devel] [PATCH pve-installer v2 1/6] auto: add early answer file sanity check for RAID configurations

["Michael Köppl" <m.koeppl@proxmox.com>]

The GUI and TUI installers already implement checks to ensure systems
have the minimum required number of disks available for the various RAID
configurations (min 2 disks for RAID1, min 4 disks for RAID10, etc).
This change adds an early check of the answer file to the
auto-installer, improving the user experience by avoiding failure during
the actual installation.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 proxmox-auto-install-assistant/src/main.rs    |  7 ++--
 proxmox-auto-installer/src/utils.rs           | 18 ++++++++++-
 proxmox-auto-installer/tests/parse-answer.rs  |  4 ++-
 .../btrfs_raid_single_disk.json               |  3 ++
 .../btrfs_raid_single_disk.toml               | 15 +++++++++
 .../zfs_raid_single_disk.json                 |  3 ++
 .../zfs_raid_single_disk.toml                 | 15 +++++++++
 proxmox-installer-common/src/options.rs       | 32 +++++++++++++++++++
 8 files changed, 92 insertions(+), 5 deletions(-)
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.toml
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
 create mode 100644 proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.toml

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index b64623b..3cf3abc 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -14,9 +14,9 @@ use proxmox_auto_installer::{
     answer::{Answer, FilterMatch},
     sysinfo::SysInfo,
     utils::{
-        AutoInstSettings, FetchAnswerFrom, HttpOptions, get_matched_udev_indexes, get_nic_list,
-        get_single_udev_index, verify_email_and_root_password_settings, verify_first_boot_settings,
-        verify_locale_settings,
+        get_matched_udev_indexes, get_nic_list, get_single_udev_index, verify_disks_settings,
+        verify_email_and_root_password_settings, verify_first_boot_settings,
+        verify_locale_settings, AutoInstSettings, FetchAnswerFrom, HttpOptions,
     },
 };
 use proxmox_installer_common::{FIRST_BOOT_EXEC_MAX_SIZE, FIRST_BOOT_EXEC_NAME};
@@ -597,6 +597,7 @@ fn parse_answer(path: impl AsRef<Path> + fmt::Debug) -> Result<Answer> {
     match toml::from_str(&contents) {
         Ok(answer) => {
             verify_locale_settings(&answer, &serde_json::from_str(LOCALE_INFO)?)?;
+            verify_disks_settings(&answer)?;
             verify_first_boot_settings(&answer)?;
             verify_email_and_root_password_settings(&answer)?;
             println!("The answer file was parsed successfully, no errors found!");
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 365e01a..d6bc6e3 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -6,7 +6,8 @@ use std::{collections::BTreeMap, process::Command};
 
 use crate::{
     answer::{
-        self, Answer, FirstBootHookSourceMode, FqdnConfig, FqdnExtendedConfig, FqdnSourceMode,
+        self, Answer, DiskSelection, FirstBootHookSourceMode, FqdnConfig, FqdnExtendedConfig,
+        FqdnSourceMode,
     },
     udevinfo::UdevInfo,
 };
@@ -384,6 +385,20 @@ pub fn verify_email_and_root_password_settings(answer: &Answer) -> Result<()> {
     }
 }
 
+pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
+    if let DiskSelection::Selection(selection) = &answer.disks.disk_selection {
+        let min_disks = answer.disks.fs_type.get_min_disks();
+        if selection.len() < min_disks {
+            bail!(
+                "{} requires at least {} disks",
+                answer.disks.fs_type,
+                min_disks
+            );
+        }
+    }
+    Ok(())
+}
+
 pub fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
     info!("Verifying first boot settings");
 
@@ -414,6 +429,7 @@ pub fn parse_answer(
     let network_settings = get_network_settings(answer, udev_info, runtime_info, setup_info)?;
 
     verify_locale_settings(answer, locales)?;
+    verify_disks_settings(answer)?;
     verify_email_and_root_password_settings(answer)?;
     verify_first_boot_settings(answer)?;
 
diff --git a/proxmox-auto-installer/tests/parse-answer.rs b/proxmox-auto-installer/tests/parse-answer.rs
index 34bc969..92dba63 100644
--- a/proxmox-auto-installer/tests/parse-answer.rs
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -142,11 +142,13 @@ mod tests {
             run_named_fail_parse_test,
             // Keep below entries alphabetically sorted
             both_password_and_hashed_set,
+            btrfs_raid_single_disk,
             fqdn_from_dhcp_no_default_domain,
             fqdn_hostname_only,
             no_fqdn_from_dhcp,
             no_root_password_set,
-            short_password
+            short_password,
+            zfs_raid_single_disk
         );
     }
 }
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
new file mode 100644
index 0000000..37f35fe
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.json
@@ -0,0 +1,3 @@
+{
+  "error": "BTRFS (RAID1) requires at least 2 disks"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.toml
new file mode 100644
index 0000000..e96fb16
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/btrfs_raid_single_disk.toml
@@ -0,0 +1,15 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "btrfs-raid-single-disk.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "btrfs"
+btrfs.raid = "raid1"
+disk-list = ["sda"]
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json b/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
new file mode 100644
index 0000000..9a8cc90
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.json
@@ -0,0 +1,3 @@
+{
+  "error": "ZFS (RAID10) requires at least 4 disks"
+}
diff --git a/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.toml b/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.toml
new file mode 100644
index 0000000..94ae748
--- /dev/null
+++ b/proxmox-auto-installer/tests/resources/parse_answer_fail/zfs_raid_single_disk.toml
@@ -0,0 +1,15 @@
+[global]
+keyboard = "de"
+country = "at"
+fqdn = "zfs-raid-single-disk.fail.testinstall"
+mailto = "mail@no.invalid"
+timezone = "Europe/Vienna"
+root-password = "12345678"
+
+[network]
+source = "from-dhcp"
+
+[disk-setup]
+filesystem = "zfs"
+zfs.raid = "raid10"
+disk-list = ["sda"]
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 9cc4ee0..9271b8b 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -20,6 +20,16 @@ pub enum BtrfsRaidLevel {
     Raid10,
 }
 
+impl BtrfsRaidLevel {
+    pub fn get_min_disks(&self) -> usize {
+        match self {
+            BtrfsRaidLevel::Raid0 => 1,
+            BtrfsRaidLevel::Raid1 => 2,
+            BtrfsRaidLevel::Raid10 => 4,
+        }
+    }
+}
+
 serde_plain::derive_display_from_serialize!(BtrfsRaidLevel);
 
 #[derive(Copy, Clone, Debug, Deserialize, Serialize, Eq, PartialEq)]
@@ -48,6 +58,19 @@ pub enum ZfsRaidLevel {
     RaidZ3,
 }
 
+impl ZfsRaidLevel {
+    pub fn get_min_disks(&self) -> usize {
+        match self {
+            ZfsRaidLevel::Raid0 => 1,
+            ZfsRaidLevel::Raid1 => 2,
+            ZfsRaidLevel::Raid10 => 4,
+            ZfsRaidLevel::RaidZ => 3,
+            ZfsRaidLevel::RaidZ2 => 4,
+            ZfsRaidLevel::RaidZ3 => 5,
+        }
+    }
+}
+
 serde_plain::derive_display_from_serialize!(ZfsRaidLevel);
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
@@ -67,6 +90,15 @@ impl FsType {
     pub fn is_lvm(&self) -> bool {
         matches!(self, FsType::Ext4 | FsType::Xfs)
     }
+
+    pub fn get_min_disks(&self) -> usize {
+        match self {
+            FsType::Ext4 => 1,
+            FsType::Xfs => 1,
+            FsType::Zfs(level) => level.get_min_disks(),
+            FsType::Btrfs(level) => level.get_min_disks(),
+        }
+    }
 }
 
 impl fmt::Display for FsType {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel