From f9ab680840a486f72b4a8a1176a6e86ba87283f7 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Wed, 8 Mar 2023 12:39:37 -0800 Subject: [PATCH] Cure existing systemd units (#313) * Cure existing systemd units * ActionError::Exists -> ActionError::DifferentContent To more accurately reflect its associated error message. * create_directory: use PathWasNotDir error instead * check if the service files and an override dir exists in plan and execute * fixup: target_os guarding * fixup: check if existing file is a symlink and see if they link to the same place * abstract systemd unit checking to function * fixup: logic error if the link_dest and unit_src were the same, we'd still error that the file exists --- src/action/base/create_directory.rs | 2 +- src/action/base/create_file.rs | 4 +- src/action/common/configure_init_service.rs | 90 ++++++++++++++++----- src/action/mod.rs | 15 +++- 4 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/action/base/create_directory.rs b/src/action/base/create_directory.rs index 883d5b9..97b96c8 100644 --- a/src/action/base/create_directory.rs +++ b/src/action/base/create_directory.rs @@ -42,7 +42,7 @@ impl CreateDirectory { .await .map_err(|e| ActionError::GettingMetadata(path.clone(), e))?; if !metadata.is_dir() { - return Err(ActionError::Exists(path.to_owned())); + return Err(ActionError::PathWasNotDirectory(path.to_owned())); } // Does it have the right user/group? diff --git a/src/action/base/create_file.rs b/src/action/base/create_file.rs index baffd1e..9bdec86 100644 --- a/src/action/base/create_file.rs +++ b/src/action/base/create_file.rs @@ -120,7 +120,7 @@ impl CreateFile { .map_err(|e| ActionError::Read(this.path.clone(), e))?; if discovered_buf != this.buf { - return Err(ActionError::Exists(this.path.clone())); + return Err(ActionError::DifferentContent(this.path.clone())); } tracing::debug!("Creating file `{}` already complete", this.path.display()); @@ -346,7 +346,7 @@ mod test { ) .await { - Err(ActionError::Exists(path)) => assert_eq!(path, test_file.as_path()), + Err(ActionError::DifferentContent(path)) => assert_eq!(path, test_file.as_path()), _ => return Err(eyre!("Should have returned an ActionError::Exists error")), } diff --git a/src/action/common/configure_init_service.rs b/src/action/common/configure_init_service.rs index 583a257..2c557d1 100644 --- a/src/action/common/configure_init_service.rs +++ b/src/action/common/configure_init_service.rs @@ -12,8 +12,12 @@ use crate::settings::InitSystem; #[cfg(target_os = "linux")] const SERVICE_SRC: &str = "/nix/var/nix/profiles/default/lib/systemd/system/nix-daemon.service"; #[cfg(target_os = "linux")] +const SERVICE_DEST: &str = "/etc/systemd/system/nix-daemon.service"; +#[cfg(target_os = "linux")] const SOCKET_SRC: &str = "/nix/var/nix/profiles/default/lib/systemd/system/nix-daemon.socket"; #[cfg(target_os = "linux")] +const SOCKET_DEST: &str = "/etc/systemd/system/nix-daemon.socket"; +#[cfg(target_os = "linux")] const TMPFILES_SRC: &str = "/nix/var/nix/profiles/default/lib/tmpfiles.d/nix-daemon.conf"; #[cfg(target_os = "linux")] const TMPFILES_DEST: &str = "/etc/tmpfiles.d/nix-daemon.conf"; @@ -32,11 +36,54 @@ pub struct ConfigureInitService { } impl ConfigureInitService { + #[cfg(target_os = "linux")] + async fn check_if_systemd_unit_exists(src: &str, dest: &str) -> Result<(), ActionError> { + // TODO: once we have a way to communicate interaction between the library and the cli, + // interactively ask for permission to remove the file + + let unit_src = PathBuf::from(src); + // NOTE: Check if the unit file already exists... + let unit_dest = PathBuf::from(dest); + if unit_dest.exists() { + if unit_dest.is_symlink() { + let link_dest = tokio::fs::read_link(&unit_dest) + .await + .map_err(|e| ActionError::ReadSymlink(unit_dest.clone(), e))?; + if link_dest != unit_src { + return Err(ActionError::SymlinkExists(unit_dest)); + } + } else { + return Err(ActionError::FileExists(unit_dest)); + } + } + // NOTE: ...and if there are any overrides in the most well-known places for systemd + if Path::new(&format!("{dest}.d")).exists() { + return Err(ActionError::DirExists(PathBuf::from(format!("{dest}.d")))); + } + + Ok(()) + } #[tracing::instrument(level = "debug", skip_all)] pub async fn plan( init: InitSystem, start_daemon: bool, ) -> Result, ActionError> { + match init { + #[cfg(target_os = "macos")] + InitSystem::Launchd => { + // No plan checks, yet + }, + #[cfg(target_os = "linux")] + InitSystem::Systemd => { + Self::check_if_systemd_unit_exists(SERVICE_SRC, SERVICE_DEST).await?; + Self::check_if_systemd_unit_exists(SOCKET_SRC, SOCKET_DEST).await?; + }, + #[cfg(not(target_os = "macos"))] + InitSystem::None => { + // Nothing here, no init system + }, + }; + Ok(Self { init, start_daemon }.into()) } } @@ -71,8 +118,8 @@ impl Action for ConfigureInitService { InitSystem::Systemd => { let mut explanation = vec![ "Run `systemd-tempfiles --create --prefix=/nix/var/nix`".to_string(), - format!("Run `systemctl link {SERVICE_SRC}`"), - format!("Run `systemctl link {SOCKET_SRC}`"), + format!("Symlink `{SERVICE_SRC}` to `{SERVICE_DEST}`"), + format!("Symlink `{SOCKET_SRC}` to `{SOCKET_DEST}`"), "Run `systemctl daemon-reload`".to_string(), ]; if self.start_daemon { @@ -159,23 +206,30 @@ impl Action for ConfigureInitService { ) .await?; - execute_command( - Command::new("systemctl") - .process_group(0) - .arg("link") - .arg(SERVICE_SRC) - .stdin(std::process::Stdio::null()), - ) - .await?; + // TODO: once we have a way to communicate interaction between the library and the + // cli, interactively ask for permission to remove the file - execute_command( - Command::new("systemctl") - .process_group(0) - .arg("link") - .arg(SOCKET_SRC) - .stdin(std::process::Stdio::null()), - ) - .await?; + Self::check_if_systemd_unit_exists(SERVICE_SRC, SERVICE_DEST).await?; + tokio::fs::symlink(SERVICE_SRC, SERVICE_DEST) + .await + .map_err(|e| { + ActionError::Symlink( + PathBuf::from(SERVICE_SRC), + PathBuf::from(SERVICE_DEST), + e, + ) + })?; + + Self::check_if_systemd_unit_exists(SOCKET_SRC, SOCKET_DEST).await?; + tokio::fs::symlink(SOCKET_SRC, SOCKET_DEST) + .await + .map_err(|e| { + ActionError::Symlink( + PathBuf::from(SOCKET_SRC), + PathBuf::from(SOCKET_DEST), + e, + ) + })?; if *start_daemon { execute_command( diff --git a/src/action/mod.rs b/src/action/mod.rs index 87795bb..833941b 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -298,11 +298,20 @@ pub enum ActionError { } }).collect::>().join(" & "))] Children(Vec>), - /// The path already exists + /// The path already exists with different content that expected #[error( "`{0}` exists with different content than planned, consider removing it with `rm {0}`" )] - Exists(std::path::PathBuf), + DifferentContent(std::path::PathBuf), + /// The file already exists + #[error("`{0}` already exists, consider removing it with `rm {0}`")] + FileExists(std::path::PathBuf), + /// The directory already exists + #[error("`{0}` already exists, consider removing it with `rm -r {0}`")] + DirExists(std::path::PathBuf), + /// The symlink already exists + #[error("`{0}` already exists, consider removing it with `rm {0}`")] + SymlinkExists(std::path::PathBuf), #[error("`{0}` exists with a different uid ({1}) than planned ({2}), consider updating it with `chown {2} {0}`")] PathUserMismatch(std::path::PathBuf, u32, u32), #[error("`{0}` exists with a different gid ({1}) than planned ({2}), consider updating it with `chgrp {2} {0}`")] @@ -345,6 +354,8 @@ pub enum ActionError { Read(std::path::PathBuf, #[source] std::io::Error), #[error("Reading directory `{0}`")] ReadDir(std::path::PathBuf, #[source] std::io::Error), + #[error("Reading symbolic link `{0}`")] + ReadSymlink(std::path::PathBuf, #[source] std::io::Error), #[error("Open path `{0}`")] Open(std::path::PathBuf, #[source] std::io::Error), #[error("Write path `{0}`")]