* [pve-devel] [PATCH installer v2 1/4] auto-install-assistant: add new parameter to specify partition label
2024-11-08 13:05 [pve-devel] [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Christoph Heiss
@ 2024-11-08 13:05 ` Christoph Heiss
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 2/4] fetch-answer: refactor cli argument parsing Christoph Heiss
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-11-08 13:05 UTC (permalink / raw)
To: pve-devel
.. for the 'partition' fetch method.
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* dropped useless "proxmox-ais" explanation from --prepare-iso
command description
proxmox-auto-install-assistant/src/main.rs | 12 ++++++++++--
proxmox-auto-installer/src/utils.rs | 1 +
proxmox-fetch-answer/src/main.rs | 1 +
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 1447175..5d11882 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -94,8 +94,7 @@ struct CommandValidateAnswer {
/// The behavior of how to fetch an answer file must be set with the '--fetch-from' parameter. The
/// answer file can be:{n}
/// * integrated into the ISO itself ('iso'){n}
-/// * present on a partition / file-system with the label 'PROXMOX-AIS' (Proxmox
-/// Automated Installer Source) ('partition'){n}
+/// * present on a partition / file-system, matched by its label ('partition'){n}
/// * requested via an HTTP Post request ('http').
///
/// The URL for the HTTP mode can be defined for the ISO with the '--url' argument. If not present,
@@ -110,6 +109,9 @@ struct CommandValidateAnswer {
/// to retrieve the URL. For example, the DNS TXT record for the fingerprint will only be used, if
/// no one was configured with the '--cert-fingerprint' parameter and if the URL was retrieved via
/// the DNS TXT record.
+///
+/// If the 'partition' mode is used, the '--partition-label' parameter can be used to set the
+/// partition label the auto-installer should search for. This defaults to 'proxmox-ais'.
#[derive(Args, Debug)]
struct CommandPrepareISO {
/// Path to the source ISO to prepare
@@ -141,6 +143,11 @@ struct CommandPrepareISO {
/// input ISO file.
#[arg(long)]
tmp: Option<String>,
+
+ /// Can be used in combination with `--fetch-from partition` to set the partition label
+ /// the auto-installer will search for.
+ #[arg(long, default_value_t = { "proxmox-ais".to_owned() } )]
+ partition_label: String,
}
/// Show the system information that can be used to identify a host.
@@ -323,6 +330,7 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
println!("Preparing ISO...");
let config = AutoInstSettings {
mode: args.fetch_from.clone(),
+ partition_label: args.partition_label.clone(),
http: HttpOptions {
url: args.url.clone(),
cert_fingerprint: args.cert_fingerprint.clone(),
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 45ad222..8ff8134 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -86,6 +86,7 @@ pub struct HttpOptions {
#[serde(rename_all = "lowercase", deny_unknown_fields)]
pub struct AutoInstSettings {
pub mode: FetchAnswerFrom,
+ pub partition_label: String,
#[serde(default)]
pub http: HttpOptions,
}
diff --git a/proxmox-fetch-answer/src/main.rs b/proxmox-fetch-answer/src/main.rs
index 660dc51..86c3270 100644
--- a/proxmox-fetch-answer/src/main.rs
+++ b/proxmox-fetch-answer/src/main.rs
@@ -63,6 +63,7 @@ fn settings_from_cli_args(args: &[String]) -> Result<AutoInstSettings> {
}
Ok(AutoInstSettings {
mode,
+ partition_label: "proxmox-ais".to_owned(),
http: HttpOptions {
url: args.get(2).cloned(),
cert_fingerprint: args.get(3).cloned(),
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 2/4] fetch-answer: refactor cli argument parsing
2024-11-08 13:05 [pve-devel] [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Christoph Heiss
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 1/4] auto-install-assistant: add new parameter to specify " Christoph Heiss
@ 2024-11-08 13:05 ` Christoph Heiss
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 3/4] fetch-answer: partition: also search for exact-matching partition label Christoph Heiss
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-11-08 13:05 UTC (permalink / raw)
To: pve-devel
Clean up the actual parsing a bit to make it more easily extensible, and
add a proper help menu.
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
proxmox-fetch-answer/src/main.rs | 40 +++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/proxmox-fetch-answer/src/main.rs b/proxmox-fetch-answer/src/main.rs
index 86c3270..753ab29 100644
--- a/proxmox-fetch-answer/src/main.rs
+++ b/proxmox-fetch-answer/src/main.rs
@@ -16,6 +16,22 @@ mod fetch_plugins;
static LOGGER: AutoInstLogger = AutoInstLogger;
static AUTOINST_MODE_FILE: &str = "/cdrom/auto-installer-mode.toml";
+const CLI_USAGE_HELPTEXT: &str = concat!(
+ "Usage: ",
+ env!("CARGO_BIN_NAME"),
+ " <command> <additional parameters..>
+
+Commands:
+ iso Fetch the builtin answer file from the ISO
+ http Fetch the answer file via HTTP(S)
+ Additional parameters: [<http-url>] [<tls-cert-fingerprint>]
+ partition Fetch the answer file from a mountable partition
+
+Options:
+ -h, --help Print this help menu
+"
+);
+
pub fn init_log() -> Result<()> {
AutoInstLogger::init("/tmp/fetch_answer.log")?;
log::set_logger(&LOGGER)
@@ -46,21 +62,27 @@ fn fetch_answer(install_settings: &AutoInstSettings) -> Result<String> {
}
fn settings_from_cli_args(args: &[String]) -> Result<AutoInstSettings> {
- // TODO: this was done in a bit of a hurry, needs tidying up
let mode = match args[1].to_lowercase().as_str() {
"iso" => FetchAnswerFrom::Iso,
"http" => FetchAnswerFrom::Http,
"partition" => FetchAnswerFrom::Partition,
- "-h" | "--help" => bail!(
- "usage: {} <http|iso|partition> [<http-url>] [<tls-cert-fingerprint>]",
- args[0]
- ),
+ "-h" | "--help" => {
+ eprintln!("{}", CLI_USAGE_HELPTEXT);
+ bail!("invalid usage");
+ }
_ => bail!("failed to parse fetch-from argument, not one of 'http', 'iso', or 'partition'"),
};
- if args.len() > 4 {
- } else if args.len() > 2 && mode != FetchAnswerFrom::Http {
- bail!("only 'http' fetch-from mode supports additional url and cert-fingerprint mode");
- }
+
+ match mode {
+ FetchAnswerFrom::Iso | FetchAnswerFrom::Partition if args.len() > 2 => {
+ bail!("'iso' and 'partition' modes do not take any additional arguments")
+ }
+ FetchAnswerFrom::Http if args.len() > 4 => {
+ bail!("'http' mode takes at most 2 additional arguments")
+ }
+ _ => {}
+ };
+
Ok(AutoInstSettings {
mode,
partition_label: "proxmox-ais".to_owned(),
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 3/4] fetch-answer: partition: also search for exact-matching partition label
2024-11-08 13:05 [pve-devel] [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Christoph Heiss
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 1/4] auto-install-assistant: add new parameter to specify " Christoph Heiss
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 2/4] fetch-answer: refactor cli argument parsing Christoph Heiss
@ 2024-11-08 13:05 ` Christoph Heiss
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 4/4] fetch-answer: use partition label from fetch config instead of hardcoded Christoph Heiss
2024-11-10 18:23 ` [pve-devel] applied-series: [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Thomas Lamprecht
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-11-08 13:05 UTC (permalink / raw)
To: pve-devel
While some filesystems - such as FAT(32) - might not supported/allow
mixed-case labels, some implementations still handle them correctly,
such as Linux. Thus, also search for that variant.
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* no changes
.../src/fetch_plugins/partition.rs | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index 4472922..cbfe2d5 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -43,12 +43,22 @@ fn path_exists_logged(file_name: &str, search_path: &str) -> Option<PathBuf> {
}
}
-/// Searches for upper and lower case existence of the partlabel in the search_path
+/// Searches for the exact case, upper and finally lower case existence of the partlabel in the
+/// search_path, in that order.
+///
+/// While some filesystems - such as FAT(32) - might not supported/allow mixed-case labels, some
+/// implementations still handle them correctly, such as Linux. Thus, also search for that variant
+/// first.
///
/// # Arguments
-/// * `partlabel_source` - Partition Label, used as upper and lower case
+/// * `partlabel_source` - Partition Label, used for matching, in the exact, upper and lower case
/// * `search_path` - Path where to search for the partition label
fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
+ if let Some(path) = path_exists_logged(partlabel, search_path) {
+ info!("Found partition with label '{partlabel}'");
+ return Ok(path);
+ }
+
let partlabel_upper_case = partlabel.to_uppercase();
if let Some(path) = path_exists_logged(&partlabel_upper_case, search_path) {
info!("Found partition with label '{partlabel_upper_case}'");
@@ -61,7 +71,7 @@ fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
return Ok(path);
}
- bail!("Could not detect upper or lower case labels for '{partlabel}'");
+ bail!("Could not find partition for label '{partlabel}'");
}
/// Will search and mount a partition/FS labeled PARTLABEL (proxmox-ais) in lower or uppercase
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] [PATCH installer v2 4/4] fetch-answer: use partition label from fetch config instead of hardcoded
2024-11-08 13:05 [pve-devel] [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Christoph Heiss
` (2 preceding siblings ...)
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 3/4] fetch-answer: partition: also search for exact-matching partition label Christoph Heiss
@ 2024-11-08 13:05 ` Christoph Heiss
2024-11-10 18:23 ` [pve-devel] applied-series: [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Thomas Lamprecht
4 siblings, 0 replies; 6+ messages in thread
From: Christoph Heiss @ 2024-11-08 13:05 UTC (permalink / raw)
To: pve-devel
This has been requested by at least one user one user [0] and definitely
makes sense, esp. for BMCs/IPMIs where one might not be able to control
the partition label.
[0] https://forum.proxmox.com/threads/proxmox-ais-question-request.153043/post-695689
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
* addressed suggestions made by Aaron
proxmox-auto-install-assistant/src/main.rs | 2 ++
.../src/fetch_plugins/partition.rs | 16 ++++++-------
proxmox-fetch-answer/src/main.rs | 23 +++++++++++++------
3 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index 5d11882..9f0fbec 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -146,6 +146,8 @@ struct CommandPrepareISO {
/// Can be used in combination with `--fetch-from partition` to set the partition label
/// the auto-installer will search for.
+ // FAT can only handle 11 characters (per specification at least, drivers might allow more),
+ // so shorten "Automated Installer Source" to "AIS" to be safe.
#[arg(long, default_value_t = { "proxmox-ais".to_owned() } )]
partition_label: String,
}
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index cbfe2d5..f4f8bcc 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -8,18 +8,16 @@ use std::{
static ANSWER_FILE: &str = "answer.toml";
static ANSWER_MP: &str = "/mnt/answer";
-// FAT can only handle 11 characters, so shorten Automated Installer Source to AIS
-static PARTLABEL: &str = "proxmox-ais";
static DISK_BY_ID_PATH: &str = "/dev/disk/by-label";
pub struct FetchFromPartition;
impl FetchFromPartition {
/// Returns the contents of the answer file
- pub fn get_answer() -> Result<String> {
+ pub fn get_answer(part_label: &str) -> Result<String> {
info!("Checking for answer file on partition.");
- let mut mount_path = PathBuf::from(mount_proxmoxinst_part()?);
+ let mut mount_path = PathBuf::from(mount_proxmoxinst_part(part_label)?);
mount_path.push(ANSWER_FILE);
let answer = fs::read_to_string(mount_path)
.map_err(|err| format_err!("failed to read answer file - {err}"))?;
@@ -74,14 +72,16 @@ fn scan_partlabels(partlabel: &str, search_path: &str) -> Result<PathBuf> {
bail!("Could not find partition for label '{partlabel}'");
}
-/// Will search and mount a partition/FS labeled PARTLABEL (proxmox-ais) in lower or uppercase
-/// to ANSWER_MP
-fn mount_proxmoxinst_part() -> Result<String> {
+/// Searches for a partition/filesystem labeled `part_label` mounts it to `ANSWER_MP`, if found.
+///
+/// # Arguments
+/// * `partlabel` - Partition Label, used for matching, in the exact, upper and lower case
+fn mount_proxmoxinst_part(part_label: &str) -> Result<String> {
if let Ok(true) = check_if_mounted(ANSWER_MP) {
info!("Skipping: '{ANSWER_MP}' is already mounted.");
return Ok(ANSWER_MP.into());
}
- let part_path = scan_partlabels(PARTLABEL, DISK_BY_ID_PATH)?;
+ let part_path = scan_partlabels(part_label, DISK_BY_ID_PATH)?;
info!("Mounting partition at {ANSWER_MP}");
// create dir for mountpoint
create_dir_all(ANSWER_MP)?;
diff --git a/proxmox-fetch-answer/src/main.rs b/proxmox-fetch-answer/src/main.rs
index 753ab29..d56f8b9 100644
--- a/proxmox-fetch-answer/src/main.rs
+++ b/proxmox-fetch-answer/src/main.rs
@@ -26,6 +26,7 @@ Commands:
http Fetch the answer file via HTTP(S)
Additional parameters: [<http-url>] [<tls-cert-fingerprint>]
partition Fetch the answer file from a mountable partition
+ Additional parameters: [<partition-label>]
Options:
-h, --help Print this help menu
@@ -49,10 +50,12 @@ fn fetch_answer(install_settings: &AutoInstSettings) -> Result<String> {
Err(err) => info!("Fetching answer file from ISO failed: {err}"),
}
}
- FetchAnswerFrom::Partition => match FetchFromPartition::get_answer() {
- Ok(answer) => return Ok(answer),
- Err(err) => info!("Fetching answer file from partition failed: {err}"),
- },
+ FetchAnswerFrom::Partition => {
+ match FetchFromPartition::get_answer(&install_settings.partition_label) {
+ Ok(answer) => return Ok(answer),
+ Err(err) => info!("Fetching answer file from partition failed: {err}"),
+ }
+ }
FetchAnswerFrom::Http => match FetchFromHTTP::get_answer(&install_settings.http) {
Ok(answer) => return Ok(answer),
Err(err) => info!("Fetching answer file via HTTP failed: {err}"),
@@ -74,18 +77,24 @@ fn settings_from_cli_args(args: &[String]) -> Result<AutoInstSettings> {
};
match mode {
- FetchAnswerFrom::Iso | FetchAnswerFrom::Partition if args.len() > 2 => {
- bail!("'iso' and 'partition' modes do not take any additional arguments")
+ FetchAnswerFrom::Iso if args.len() > 2 => {
+ bail!("'iso' mode does not take any additional arguments")
}
FetchAnswerFrom::Http if args.len() > 4 => {
bail!("'http' mode takes at most 2 additional arguments")
}
+ FetchAnswerFrom::Partition if args.len() > 3 => {
+ bail!("'partition' mode takes at most 1 additional argument")
+ }
_ => {}
};
Ok(AutoInstSettings {
mode,
- partition_label: "proxmox-ais".to_owned(),
+ partition_label: args
+ .get(2)
+ .ok_or(format_err!("partition label expected"))
+ .cloned()?,
http: HttpOptions {
url: args.get(2).cloned(),
cert_fingerprint: args.get(3).cloned(),
--
2.47.0
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* [pve-devel] applied-series: [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label
2024-11-08 13:05 [pve-devel] [PATCH installer v2 0/4] auto-install-assistant: allow specifying 'fetch-from' partition label Christoph Heiss
` (3 preceding siblings ...)
2024-11-08 13:05 ` [pve-devel] [PATCH installer v2 4/4] fetch-answer: use partition label from fetch config instead of hardcoded Christoph Heiss
@ 2024-11-10 18:23 ` Thomas Lamprecht
4 siblings, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2024-11-10 18:23 UTC (permalink / raw)
To: Proxmox VE development discussion, Christoph Heiss
Am 08.11.24 um 14:05 schrieb Christoph Heiss:
> This series allow specifying the partition label the
> `proxmox-fetch-answer` tool will search when trying to retrieve an
> answer file in the 'partition' fetch mode.
>
> This has been requested by at least one user one user [0] and definitely
> makes sense, esp. for BMCs/IPMIs where one might not be able to control
> the partition label.
>
> Patch #1 implements the new CLI parameter in the
> `proxmox-auto-install-assistant` tool, patch #4 then integrates it into
> the `proxmox-fetch-answer` itself. Patch #2 is just a small (but needed)
> refactor of the CLI parsing code.
>
> Patch #3 implements also searching for the partition label in the exact
> case it was specified by the user. This is something I stumbled over
> while testing it - see the patch itself for some more information.
>
> [0] https://forum.proxmox.com/threads/proxmox-ais-question-request.153043/post-695689
>
> History
> =======
> v1: https://lore.proxmox.com/pve-devel/20241018115943.813243-1-c.heiss@proxmox.com/
>
> Notable Changes v1 -> v2:
> * addressed suggestions by Aaron
> * added Aaron's T-b & R-b tags, thanks!
> * dropped useless "proxmox-ais" explanation from --prepare-iso
> description
>
> Diffstat
> ========
>
> Christoph Heiss (4):
> auto-install-assistant: add new parameter to specify partition label
> fetch-answer: refactor cli argument parsing
> fetch-answer: partition: also search for exact-matching partition
> label
> fetch-answer: use partition label from fetch config instead of
> hardcoded
>
> proxmox-auto-install-assistant/src/main.rs | 15 ++++-
> proxmox-auto-installer/src/utils.rs | 1 +
> .../src/fetch_plugins/partition.rs | 32 ++++++----
> proxmox-fetch-answer/src/main.rs | 58 ++++++++++++++-----
> 4 files changed, 80 insertions(+), 26 deletions(-)
>
applied series, thanks!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 6+ messages in thread