From e045db0172c9d9149b1edfb58542e0e8f9575601 Mon Sep 17 00:00:00 2001 From: Omer Tuchfeld Date: Tue, 21 Feb 2023 22:29:38 +0100 Subject: [PATCH] Proposal: install subcommand should allow just postprocess # Intro This is more a proposal than a pull request. Since the change is simple, it's best to demonstrate it with a PR rather than just words. # Proposal My proposal is that coreos-installer should allow optionally performing just the post-processing portion of the installation. # Why This would be useful to improve the installation time in the context of single-node OCP bootstrap-in-place installations (a method also used by the OCP Assisted Installer). # How / Background In that kind of installation, we create a bootstrap OpenShift control-plane running on a live ISO. When that control-plane is done forming we store the state of etcd among other things in an ignition file, install RHCOS to disk with said ignition then reboot to form a working OCP single node cluster. Setting up a control plane already takes a long time, and writing the image to disk, especially when using metal machines which usually have to read the osmet data from a slow virtual media, also takes a long time. Currently we have to perform these two actions serially - as we only have the ignition once the control plane is done forming. If this proposal were to be merged, we could instead perform the image writing to disk, without any ignition, parallel to the bootstrap control-plane formation. Then once we have the ignition we could use this new `--postprocess-only` installation flag to have coreos-installer just quickly write the ignition to disk without having to do a lengthy image installation. # Alternatives We currently have a workaround to do this without any changes to `coreos-installer` where we emulate the ignition writing behavior of this installer using simple bash commands: ```bash boot_partition_device=$(sudo lsblk --paths --output-all --json | jq --arg device $TARGET_DEVICE ' .blockdevices[] | select(.name == $device).children[] | select(.label == "boot").name ' -r) MOUNT_POINT=/tmp/sno-boot-mount-ignition IGNITION_DIR="${MOUNT_POINT}"/ignition SRC_IGNITION=/opt/openshift/master.ign DST_IGNITION="${IGNITION_DIR}"/config.ign sudo umount "${MOUNT_POINT}" || true sudo mkdir -p "${MOUNT_POINT}" sudo mount $boot_partition_device "${MOUNT_POINT}" sudo mkdir -p "${IGNITION_DIR}" sudo chmod 700 "${IGNITION_DIR}" sudo cp "${SRC_IGNITION}" "${DST_IGNITION}" sudo chmod 600 "${DST_IGNITION}" sudo umount "${MOUNT_POINT}" ``` But it is hacky / far from ideal and bound to break if the exact mechanisms of how ignition is written are ever to change. It's best if we could rely on coreos-installer to do this work. --- src/cmdline/install.rs | 5 +++++ src/install.rs | 44 ++++++++++++++++++++++-------------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/cmdline/install.rs b/src/cmdline/install.rs index e191d4756..ab1729ff0 100644 --- a/src/cmdline/install.rs +++ b/src/cmdline/install.rs @@ -111,6 +111,11 @@ pub struct InstallConfig { /// formatted as -. can be sha256 or sha512. #[clap(long, value_name = "digest")] pub ignition_hash: Option, + /// Only perform post-processing (ignition, network copy, kernel args, etc.) without writing + /// the image to disk + #[clap(long)] + #[clap(conflicts_with_all = &["image-file", "image-url"])] + pub postprocess_only: bool, /// Target CPU architecture /// /// Create an install disk for a different CPU architecture than the diff --git a/src/install.rs b/src/install.rs index 591e4b952..fce37cf2b 100644 --- a/src/install.rs +++ b/src/install.rs @@ -366,27 +366,29 @@ fn write_disk( ) -> Result<()> { let device = config.dest_device.as_deref().expect("device missing"); - // Get sector size of destination, for comparing with image - let sector_size = get_sector_size(dest)?; - - // copy the image - #[allow(clippy::match_bool, clippy::match_single_binding)] - let image_copy = match is_dasd(device, Some(dest))? { - #[cfg(target_arch = "s390x")] - true => s390x::image_copy_s390x, - _ => image_copy_default, - }; - write_image( - source, - dest, - Path::new(device), - image_copy, - true, - Some(saved), - Some(sector_size), - VerifyKeys::Production, - )?; - table.reread()?; + if !config.postprocess_only { + // Get sector size of destination, for comparing with image + let sector_size = get_sector_size(dest)?; + + // copy the image + #[allow(clippy::match_bool, clippy::match_single_binding)] + let image_copy = match is_dasd(device, Some(dest))? { + #[cfg(target_arch = "s390x")] + true => s390x::image_copy_s390x, + _ => image_copy_default, + }; + write_image( + source, + dest, + Path::new(device), + image_copy, + true, + Some(saved), + Some(sector_size), + VerifyKeys::Production, + )?; + table.reread()?; + } // postprocess if ignition.is_some()