Re: [pve-devel] [PATCH v3 15/30] auto-installer: add fetch answer binary
Two typos ;) And some small nits
On Thu, Mar 28, 2024 at 02:50:13PM +0100, Aaron Lauterer wrote:
[..]
> diff --git a/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
> b/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
> new file mode 100644
> index 000..9e89a3c
> --- /dev/null
> +++ b/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
[..]
> +fn main() -> ExitCode {
> +if let Err(err) = init_log() {
> +panic!("could not initilize logging: {err}");
^
typo
And for consistency sake, capitalize the first word here too as done
everywhere else?
> +}
> +
> +info!("Fetching answer file");
> +let answer = match fetch_answer() {
> +Ok(answer) => answer,
> +Err(err) => {
> +error!("Aborting: {}", err);
> +return ExitCode::FAILURE;
> +}
> +};
> +
> +let mut child = match Command::new("proxmox-auto-installer")
> +.stdout(Stdio::inherit())
> +.stdin(Stdio::piped())
> +.stderr(Stdio::null())
> +.spawn()
> +{
> +Ok(child) => child,
> +Err(err) => panic!("Failed to start automatic installation: {}",
> err),
Very much a nit; but above always inline format-strings (i.e. "{err}")
are used, from here on out mostly explicit parameters. Maybe unify them
and use the former consistenly where possible?
E.g. below in `utils.rs` are some more instances
> +};
> +
> +let mut stdin = child.stdin.take().expect("Failed to open stdin");
> +std::thread::spawn(move || {
> +stdin
> +.write_all(answer.as_bytes())
> +.expect("Failed to write to stdin");
> +});
[..]
> diff --git a/proxmox-auto-installer/src/fetch_plugins/utils.rs
> b/proxmox-auto-installer/src/fetch_plugins/utils.rs
> new file mode 100644
> index 000..82cd3e0
> --- /dev/null
> +++ b/proxmox-auto-installer/src/fetch_plugins/utils.rs
> @@ -0,0 +1,90 @@
> +use anyhow::{bail, Result};
> +use log::{info, warn};
> +use std::{
> +fs::create_dir_all,
> +path::{Path, PathBuf},
> +process::Command,
> +};
> +
> +/// Searches for upper and lower case existence of the partlabel in the
> search_path
> +///
> +/// # Arguemnts
> +/// * `partlabel_lower` - Partition Label in lower case
> +/// * `search_path` - Path where to search for the partiiton label
> +/// search_path: String
Stray last line?
> +pub fn scan_partlabels(partlabel_lower: &str, search_path: &str) ->
> Result {
> +let partlabel = partlabel_lower.to_uppercase();
> +let path = Path::new(search_path).join(partlabel.clone());
^
Can be `&partlabel`
> +match path.try_exists() {
> +Ok(true) => {
> +info!("Found partition with label '{}'", partlabel);
> +return Ok(path);
> +}
> +Ok(false) => info!("Did not detect partition with label '{}'",
> partlabel),
> +Err(err) => info!("Encountered issue, accessing '{}': {}",
> path.display(), err),
> +}
> +
> +let partlabel = partlabel_lower.to_lowercase();
Since you explicitly convert it to lowercase anyway here, I'd say name
`partlabel_lower` does not make much sense (anymore)?
> +let path = Path::new(search_path).join(partlabel.clone());
^
Same here
> +match path.try_exists() {
> +Ok(true) => {
> +info!("Found partition with label '{}'", partlabel);
> +return Ok(path);
> +}
> +Ok(false) => info!("Did not detect partition with label '{}'",
> partlabel),
> +Err(err) => info!("Encountered issue, accessing '{}': {}",
> path.display(), err),
> +}
> +bail!(
> +"Could not detect upper or lower case labels for '{}'",
> +partlabel_lower
> +);
> +}
> +
[..]
> +/// Tries to unmount the specified path. Will warn on errors, but not fail.
> +///
> +/// # Arguemnts
> +/// * `target_path` - path to unmount
> +pub fn umount_part(target_path: &str) -> Result<()> {
> +info!("Unmounting partitiona at {target_path}");
^^
typo
> +match Command::new("umount").arg(target_path).output() {
> +Ok(output) => {
> +if output.status.success() {
> +Ok(())
> +} else {
> +warn!("Error unmounting: {}",
> String::from_utf8(output.stderr)?);
> +Ok(())
> +}
> +}
> +Err(err) => {
> +warn!("Error unmounting: {}", err);
> +Ok(())
> +}
> +}
> +}
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 15/30] auto-installer: add fetch answer binary
it is supposed to be run first and fetch an answer file.
The initial implementation searches for a partition/filesystem called
'proxmoxinst' or 'PROXMOXINST' with an 'answer.toml' file in the root
directory.
Once it has an answer file, it will call the 'proxmox-auto-installer'
and pipe in the contents via stdin.
Signed-off-by: Aaron Lauterer
---
Makefile | 4 +-
.../src/bin/proxmox-fetch-answer.rs | 71 +++
.../src/fetch_plugins/mod.rs | 2 +
.../src/fetch_plugins/partition.rs| 32 +++
.../src/fetch_plugins/utils.rs| 90 +++
proxmox-auto-installer/src/lib.rs | 1 +
6 files changed, 199 insertions(+), 1 deletion(-)
create mode 100644 proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
create mode 100644 proxmox-auto-installer/src/fetch_plugins/mod.rs
create mode 100644 proxmox-auto-installer/src/fetch_plugins/partition.rs
create mode 100644 proxmox-auto-installer/src/fetch_plugins/utils.rs
diff --git a/Makefile b/Makefile
index 3ac5769..e44450e 100644
--- a/Makefile
+++ b/Makefile
@@ -20,6 +20,7 @@ PREFIX = /usr
BINDIR = $(PREFIX)/bin
USR_BIN := \
proxmox-tui-installer\
+ proxmox-fetch-answer\
proxmox-auto-installer
COMPILED_BINS := \
@@ -121,7 +122,8 @@ $(COMPILED_BINS): cargo-build
.PHONY: cargo-build
cargo-build:
$(CARGO) build --package proxmox-tui-installer --bin
proxmox-tui-installer \
- --package proxmox-auto-installer --bin proxmox-auto-installer
$(CARGO_BUILD_ARGS)
+ --package proxmox-auto-installer --bin proxmox-auto-installer \
+ --bin proxmox-fetch-answer $(CARGO_BUILD_ARGS)
%-banner.png: %-banner.svg
rsvg-convert -o $@ $<
diff --git a/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
b/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
new file mode 100644
index 000..9e89a3c
--- /dev/null
+++ b/proxmox-auto-installer/src/bin/proxmox-fetch-answer.rs
@@ -0,0 +1,71 @@
+use anyhow::{anyhow, Error, Result};
+use log::{error, info, LevelFilter};
+use proxmox_auto_installer::{fetch_plugins::partition::FetchFromPartition,
log::AutoInstLogger};
+use std::io::Write;
+use std::process::{Command, ExitCode, Stdio};
+
+static LOGGER: AutoInstLogger = AutoInstLogger;
+
+pub fn init_log() -> Result<()> {
+AutoInstLogger::init("/tmp/fetch_answer.log")?;
+log::set_logger(&LOGGER)
+.map(|()| log::set_max_level(LevelFilter::Info))
+.map_err(|err| anyhow!(err))
+}
+
+fn fetch_answer() -> Result {
+match FetchFromPartition::get_answer() {
+Ok(answer) => return Ok(answer),
+Err(err) => info!("Fetching answer file from partition failed: {err}"),
+}
+// TODO: add more options to get an answer file, e.g. download from url
where url could be
+// fetched via txt records on predefined subdomain, kernel param, dhcp
option, ...
+
+Err(Error::msg("Could not find any answer file!"))
+}
+
+fn main() -> ExitCode {
+if let Err(err) = init_log() {
+panic!("could not initilize logging: {err}");
+}
+
+info!("Fetching answer file");
+let answer = match fetch_answer() {
+Ok(answer) => answer,
+Err(err) => {
+error!("Aborting: {}", err);
+return ExitCode::FAILURE;
+}
+};
+
+let mut child = match Command::new("proxmox-auto-installer")
+.stdout(Stdio::inherit())
+.stdin(Stdio::piped())
+.stderr(Stdio::null())
+.spawn()
+{
+Ok(child) => child,
+Err(err) => panic!("Failed to start automatic installation: {}", err),
+};
+
+let mut stdin = child.stdin.take().expect("Failed to open stdin");
+std::thread::spawn(move || {
+stdin
+.write_all(answer.as_bytes())
+.expect("Failed to write to stdin");
+});
+
+match child.wait() {
+Ok(status) => {
+if status.success() {
+ExitCode::SUCCESS
+} else {
+ExitCode::FAILURE // Will be trapped
+}
+}
+Err(err) => {
+error!("Auto installer exited: {err}");
+ExitCode::FAILURE
+}
+}
+}
diff --git a/proxmox-auto-installer/src/fetch_plugins/mod.rs
b/proxmox-auto-installer/src/fetch_plugins/mod.rs
new file mode 100644
index 000..11d6937
--- /dev/null
+++ b/proxmox-auto-installer/src/fetch_plugins/mod.rs
@@ -0,0 +1,2 @@
+pub mod partition;
+mod utils;
diff --git a/proxmox-auto-installer/src/fetch_plugins/partition.rs
b/proxmox-auto-installer/src/fetch_plugins/partition.rs
new file mode 100644
index 000..0c47a62
--- /dev/null
+++ b/proxmox-auto-installer/src/fetch_plugins/partition.rs
@@ -0,0 +1,32 @@
+use anyhow::{Error, Result};
+use std::{fs::read_to_string, path::Path};
+use log::info;
+
+use crate::fetch_plugins::utils::mount_proxmoxinst_part;
+
+static ANSWER_FILE: &str =
