From 2b343d2e9fc10873f0ec02a8142e327885141157 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Fri, 18 Oct 2024 11:11:31 -0700 Subject: [PATCH] ConfigureInitService: support both literal and path service sources This makes it possible for ConfigureDeterminateNixdInitService to be an even thinner wrapper around ConfigureInitService. --- .../mod.rs | 77 +++--------- src/action/common/configure_init_service.rs | 114 +++++++++++++----- .../common/configure_upstream_init_service.rs | 12 +- 3 files changed, 106 insertions(+), 97 deletions(-) diff --git a/src/action/common/configure_determinate_nixd_init_service/mod.rs b/src/action/common/configure_determinate_nixd_init_service/mod.rs index 003b7ad71..11d9f5a20 100644 --- a/src/action/common/configure_determinate_nixd_init_service/mod.rs +++ b/src/action/common/configure_determinate_nixd_init_service/mod.rs @@ -1,13 +1,11 @@ use std::collections::HashMap; -use std::path::PathBuf; use serde::{Deserialize, Serialize}; -use tokio::io::AsyncWriteExt; use tracing::{span, Span}; use crate::action::common::configure_init_service::{SocketFile, UnitSrc}; use crate::action::{common::ConfigureInitService, Action, ActionDescription}; -use crate::action::{ActionError, ActionErrorKind, ActionTag, StatefulAction}; +use crate::action::{ActionError, ActionTag, StatefulAction}; use crate::settings::InitSystem; // Linux @@ -36,12 +34,26 @@ impl ConfigureDeterminateNixdInitService { init: InitSystem, start_daemon: bool, ) -> Result, ActionError> { - let service_dest: Option = match init { + let service_src = match init { + InitSystem::Systemd => Some(UnitSrc::Literal( + include_str!("./nix-daemon.determinate-nixd.service").to_string(), + )), + InitSystem::Launchd => { + let generated_plist = generate_plist(); + let mut buf = Vec::new(); + plist::to_writer_xml(&mut buf, &generated_plist).map_err(Self::error)?; + let plist = String::from_utf8_lossy(&buf); + + Some(UnitSrc::Literal(plist.to_string())) + }, + InitSystem::None => None, + }; + let service_dest = match init { InitSystem::Launchd => Some(DARWIN_NIXD_DAEMON_DEST.into()), InitSystem::Systemd => Some(LINUX_NIXD_DAEMON_DEST.into()), InitSystem::None => None, }; - let service_name: Option = match init { + let service_name = match init { InitSystem::Launchd => Some(DARWIN_NIXD_SERVICE_NAME.into()), _ => None, }; @@ -49,7 +61,7 @@ impl ConfigureDeterminateNixdInitService { let configure_init_service = ConfigureInitService::plan( init, start_daemon, - None, + service_src, service_dest, service_name, vec![ @@ -106,44 +118,7 @@ impl Action for ConfigureDeterminateNixdInitService { #[tracing::instrument(level = "debug", skip_all)] async fn execute(&mut self) -> Result<(), ActionError> { - let Self { - init, - configure_init_service, - } = self; - - if *init == InitSystem::Launchd { - let daemon_file = DARWIN_NIXD_DAEMON_DEST; - - // This is the only part that is actually different from configure_init_service, beyond variable parameters. - - let generated_plist = generate_plist(); - - let mut options = tokio::fs::OpenOptions::new(); - options.create(true).write(true).read(true); - - let mut file = options - .open(&daemon_file) - .await - .map_err(|e| Self::error(ActionErrorKind::Open(PathBuf::from(daemon_file), e)))?; - - let mut buf = Vec::new(); - plist::to_writer_xml(&mut buf, &generated_plist).map_err(Self::error)?; - file.write_all(&buf) - .await - .map_err(|e| Self::error(ActionErrorKind::Write(PathBuf::from(daemon_file), e)))?; - } else if *init == InitSystem::Systemd { - let daemon_file = PathBuf::from(LINUX_NIXD_DAEMON_DEST); - - tokio::fs::write( - &daemon_file, - include_str!("./nix-daemon.determinate-nixd.service"), - ) - .await - .map_err(|e| ActionErrorKind::Write(daemon_file.clone(), e)) - .map_err(Self::error)?; - } - - configure_init_service + self.configure_init_service .try_execute() .await .map_err(Self::error)?; @@ -162,20 +137,6 @@ impl Action for ConfigureDeterminateNixdInitService { async fn revert(&mut self) -> Result<(), ActionError> { self.configure_init_service.try_revert().await?; - let file_to_remove = match self.init { - InitSystem::Launchd => Some(DARWIN_NIXD_DAEMON_DEST), - InitSystem::Systemd => Some(LINUX_NIXD_DAEMON_DEST), - InitSystem::None => None, - }; - - if let Some(file_to_remove) = file_to_remove { - tracing::trace!(path = %file_to_remove, "Removing"); - tokio::fs::remove_file(file_to_remove) - .await - .map_err(|e| ActionErrorKind::Remove(file_to_remove.into(), e)) - .map_err(Self::error)?; - } - Ok(()) } } diff --git a/src/action/common/configure_init_service.rs b/src/action/common/configure_init_service.rs index 7daba3d07..b188ba828 100644 --- a/src/action/common/configure_init_service.rs +++ b/src/action/common/configure_init_service.rs @@ -27,6 +27,15 @@ pub enum UnitSrc { Literal(String), } +impl std::fmt::Display for UnitSrc { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + UnitSrc::Path(inner) => inner.display().fmt(f), + UnitSrc::Literal(inner) => inner.fmt(f), + } + } +} + /** Configure the init to run the Nix daemon */ @@ -36,7 +45,7 @@ pub struct ConfigureInitService { init: InitSystem, start_daemon: bool, // TODO(cole-h): make an enum so we can distinguish between "written out by another step" vs "actually there isn't one" - service_src: Option, + service_src: Option, service_name: Option, service_dest: Option, socket_files: Vec, @@ -93,7 +102,7 @@ impl ConfigureInitService { pub async fn plan( init: InitSystem, start_daemon: bool, - service_src: Option, + service_src: Option, service_dest: Option, service_name: Option, socket_files: Vec, @@ -181,8 +190,7 @@ impl Action for ConfigureInitService { "Symlink `{0}` to `{1}`", self.service_src .as_ref() - .expect("service_src should be defined for systemd") - .display(), + .expect("service_src should be defined for systemd"), self.service_dest .as_ref() .expect("service_src should be defined for systemd") @@ -218,7 +226,7 @@ impl Action for ConfigureInitService { if let Some(service_src) = self.service_src.as_ref() { explanation.push(format!( "Copy `{0}` to `{1}`", - service_src.display(), + service_src, self.service_dest .as_ref() .expect("service_dest should be defined for launchd") @@ -264,15 +272,25 @@ impl Action for ConfigureInitService { let domain = DARWIN_LAUNCHD_DOMAIN; if let Some(service_src) = service_src { - tokio::fs::copy(&service_src, service_dest) - .await - .map_err(|e| { - Self::error(ActionErrorKind::Copy( - service_src.clone(), - PathBuf::from(service_dest), - e, - )) - })?; + match service_src { + UnitSrc::Path(path) => { + tracing::trace!(src = %path.display(), dest = %service_dest.display(), "Copying"); + tokio::fs::copy(&path, service_dest).await.map_err(|e| { + Self::error(ActionErrorKind::Copy( + path.to_path_buf(), + PathBuf::from(service_dest), + e, + )) + })?; + }, + UnitSrc::Literal(contents) => { + tracing::trace!(dest = %service_dest.display(), "Writing"); + tokio::fs::write(service_dest, contents) + .await + .map_err(|e| ActionErrorKind::Write(service_dest.clone(), e)) + .map_err(Self::error)?; + }, + } } crate::action::macos::retry_bootstrap(domain, service, service_dest) @@ -382,12 +400,9 @@ impl Action for ConfigureInitService { // cli, interactively ask for permission to remove the file if let Some(service_src) = service_src.as_ref() { - Self::check_if_systemd_unit_exists( - &UnitSrc::Path(service_src.to_path_buf()), - service_dest, - ) - .await - .map_err(Self::error)?; + Self::check_if_systemd_unit_exists(service_src, service_dest) + .await + .map_err(Self::error)?; if Path::new(service_dest).exists() { tracing::trace!(path = %service_dest.display(), "Removing"); tokio::fs::remove_file(service_dest) @@ -395,17 +410,29 @@ impl Action for ConfigureInitService { .map_err(|e| ActionErrorKind::Remove(service_dest.into(), e)) .map_err(Self::error)?; } - tracing::trace!(src = %service_src.display(), dest = %service_dest.display(), "Symlinking"); - tokio::fs::symlink(service_src, service_dest) - .await - .map_err(|e| { - ActionErrorKind::Symlink( - service_src.clone(), - PathBuf::from(service_dest), - e, - ) - }) - .map_err(Self::error)?; + + match service_src { + UnitSrc::Path(path) => { + tracing::trace!(src = %path.display(), dest = %service_dest.display(), "Symlinking"); + tokio::fs::symlink(path, service_dest) + .await + .map_err(|e| { + ActionErrorKind::Symlink( + path.to_path_buf(), + PathBuf::from(service_dest), + e, + ) + }) + .map_err(Self::error)?; + }, + UnitSrc::Literal(contents) => { + tracing::trace!(dest = %service_dest.display(), "Writing"); + tokio::fs::write(service_dest, contents) + .await + .map_err(|e| ActionErrorKind::Write(service_dest.clone(), e)) + .map_err(Self::error)?; + }, + } } for SocketFile { src, dest, .. } in socket_files.iter() { @@ -500,7 +527,6 @@ impl Action for ConfigureInitService { self.service_src .as_ref() .expect("service_src should be defined for systemd") - .display() )); steps.push("Run `systemd-tempfiles --remove --prefix=/nix/var/nix`".to_string()); steps.push("Run `systemctl daemon-reload`".to_string()); @@ -565,6 +591,16 @@ impl Action for ConfigureInitService { } tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; } + + if service_dest.exists() { + tracing::trace!(path = %service_dest.display(), "Removing"); + if let Err(err) = tokio::fs::remove_file(&service_dest) + .await + .map_err(|e| ActionErrorKind::Remove(service_dest.to_path_buf(), e)) + { + errors.push(err); + } + } }, InitSystem::Systemd => { // We separate stop and disable (instead of using `--now`) to avoid cases where the service isn't started, but is enabled. @@ -644,6 +680,20 @@ impl Action for ConfigureInitService { errors.push(err); } + let service_dest = self + .service_dest + .as_ref() + .expect("service_dest should be defined for systemd"); + if service_dest.exists() { + tracing::trace!(path = %service_dest.display(), "Removing"); + if let Err(err) = tokio::fs::remove_file(&service_dest) + .await + .map_err(|e| ActionErrorKind::Remove(service_dest.to_path_buf(), e)) + { + errors.push(err); + } + } + for socket in self.socket_files.iter() { if let UnitSrc::Literal(_) = socket.src { tracing::trace!(path = %socket.dest.display(), "Removing"); diff --git a/src/action/common/configure_upstream_init_service.rs b/src/action/common/configure_upstream_init_service.rs index 10a67c5e4..3fc6af87a 100644 --- a/src/action/common/configure_upstream_init_service.rs +++ b/src/action/common/configure_upstream_init_service.rs @@ -1,5 +1,3 @@ -use std::path::PathBuf; - use tracing::{span, Span}; use crate::action::{ActionError, ActionTag, StatefulAction}; @@ -33,17 +31,17 @@ impl ConfigureUpstreamInitService { init: InitSystem, start_daemon: bool, ) -> Result, ActionError> { - let service_src: Option = match init { - InitSystem::Launchd => Some(DARWIN_NIX_DAEMON_SOURCE.into()), - InitSystem::Systemd => Some(SERVICE_SRC.into()), + let service_src = match init { + InitSystem::Launchd => Some(UnitSrc::Path(DARWIN_NIX_DAEMON_SOURCE.into())), + InitSystem::Systemd => Some(UnitSrc::Path(SERVICE_SRC.into())), InitSystem::None => None, }; - let service_dest: Option = match init { + let service_dest = match init { InitSystem::Launchd => Some(DARWIN_NIX_DAEMON_DEST.into()), InitSystem::Systemd => Some(SERVICE_DEST.into()), InitSystem::None => None, }; - let service_name: Option = match init { + let service_name = match init { InitSystem::Launchd => Some(DARWIN_LAUNCHD_SERVICE_NAME.into()), _ => None, };