From bbb9394df5c2ab1c15c769028ddb720d26d98522 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 31 Oct 2024 09:02:10 -0700 Subject: [PATCH] Fix bootout errors on macOS once and for all (#1257) * ConfigureInitService: remove unnecessary `systemctl daemon-reload` * ConfigureInitService: don't remove files that were already removed * ConfigureDeterminateNixdInitService: remove unused error enum * ConfigureInitService: don't check if units already exist If they do, we'll replace them in the execute step anyways. * ConfigureInitService: fixup tracing log line outside of guard * fixup: rename restore_shell variable to repair * ConfigureInitService: bootout failure is (probably) not fatal, just collect it and move on * fixup: const-ify selinux policy path for FHS systems * retry_bootout: use service identifier for bootout instead of path Hopefully fixes the "Boot-out failed: 5: Input/output error" error on all future installs. * fixup: deduplicate service_name --- .../mod.rs | 4 - src/action/common/configure_init_service.rs | 84 ++++++------------- .../macos/bootstrap_launchctl_service.rs | 2 +- .../create_determinate_volume_service.rs | 2 +- src/action/macos/create_nix_hook_service.rs | 2 +- src/action/macos/create_volume_service.rs | 2 +- src/action/macos/mod.rs | 13 ++- src/cli/mod.rs | 2 +- src/planner/linux.rs | 4 +- 9 files changed, 40 insertions(+), 75 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..d8475807d 100644 --- a/src/action/common/configure_determinate_nixd_init_service/mod.rs +++ b/src/action/common/configure_determinate_nixd_init_service/mod.rs @@ -180,10 +180,6 @@ impl Action for ConfigureDeterminateNixdInitService { } } -#[non_exhaustive] -#[derive(Debug, thiserror::Error)] -pub enum ConfigureDeterminateNixDaemonServiceError {} - #[derive(Deserialize, Clone, Debug, Serialize, PartialEq)] #[serde(rename_all = "PascalCase")] pub struct DeterminateNixDaemonPlist { diff --git a/src/action/common/configure_init_service.rs b/src/action/common/configure_init_service.rs index 7daba3d07..2714b0d75 100644 --- a/src/action/common/configure_init_service.rs +++ b/src/action/common/configure_init_service.rs @@ -116,23 +116,6 @@ impl ConfigureInitService { if which::which("systemctl").is_err() { return Err(Self::error(ActionErrorKind::SystemdMissing)); } - - if let (Some(service_src), Some(service_dest)) = - (service_src.as_ref(), service_dest.as_ref()) - { - Self::check_if_systemd_unit_exists( - &UnitSrc::Path(service_src.to_path_buf()), - service_dest, - ) - .await - .map_err(Self::error)?; - } - - for SocketFile { src, dest, .. } in socket_files.iter() { - Self::check_if_systemd_unit_exists(src, dest) - .await - .map_err(Self::error)?; - } }, InitSystem::None => { // Nothing here, no init system @@ -312,17 +295,6 @@ impl Action for ConfigureInitService { .as_ref() .expect("service_dest should be defined for systemd"); - if *start_daemon { - execute_command( - Command::new("systemctl") - .process_group(0) - .arg("daemon-reload") - .stdin(std::process::Stdio::null()), - ) - .await - .map_err(Self::error)?; - } - // The goal state is the `socket` enabled and active, the service not enabled and stopped (it activates via socket activation) let mut any_socket_was_active = false; for SocketFile { name, .. } in socket_files.iter() { @@ -354,8 +326,8 @@ impl Action for ConfigureInitService { }; } - tracing::trace!(src = TMPFILES_SRC, dest = TMPFILES_DEST, "Symlinking"); if !Path::new(TMPFILES_DEST).exists() { + tracing::trace!(src = TMPFILES_SRC, dest = TMPFILES_DEST, "Symlinking"); tokio::fs::symlink(TMPFILES_SRC, TMPFILES_DEST) .await .map_err(|e| { @@ -531,31 +503,25 @@ impl Action for ConfigureInitService { match self.init { InitSystem::Launchd => { - crate::action::macos::retry_bootout( - DARWIN_LAUNCHD_DOMAIN, - self.service_name - .as_ref() - .expect("service_name should be set for launchd"), - self.service_dest - .as_ref() - .expect("service_dest should be defined for launchd"), - ) - .await - .map_err(Self::error)?; + let service_name = self + .service_name + .as_ref() + .expect("service_name should be set for launchd"); + + if let Err(e) = + crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, service_name).await + { + errors.push(e); + } // check if the daemon is down up to 99 times, with 100ms of delay between each attempt for attempt in 1..100 { tracing::trace!(attempt, "Checking to see if the daemon is down yet"); if execute_command( - Command::new("launchctl").process_group(0).arg("print").arg( - [ - DARWIN_LAUNCHD_DOMAIN, - self.service_name - .as_ref() - .expect("service_name should be defined for launchd"), - ] - .join("/"), - ), + Command::new("launchctl") + .process_group(0) + .arg("print") + .arg([DARWIN_LAUNCHD_DOMAIN, service_name].join("/")), ) .await .is_err() @@ -645,20 +611,24 @@ impl Action for ConfigureInitService { } for socket in self.socket_files.iter() { - if let UnitSrc::Literal(_) = socket.src { + if socket.dest.exists() { tracing::trace!(path = %socket.dest.display(), "Removing"); - tokio::fs::remove_file(&socket.dest) + if let Err(err) = tokio::fs::remove_file(&socket.dest) .await .map_err(|e| ActionErrorKind::Remove(socket.dest.to_path_buf(), e)) - .map_err(Self::error)?; + { + errors.push(err); + } } } - if let Err(err) = tokio::fs::remove_file(TMPFILES_DEST) - .await - .map_err(|e| ActionErrorKind::Remove(PathBuf::from(TMPFILES_DEST), e)) - { - errors.push(err); + if Path::new(TMPFILES_DEST).exists() { + if let Err(err) = tokio::fs::remove_file(TMPFILES_DEST) + .await + .map_err(|e| ActionErrorKind::Remove(PathBuf::from(TMPFILES_DEST), e)) + { + errors.push(err); + } } if let Err(err) = execute_command( diff --git a/src/action/macos/bootstrap_launchctl_service.rs b/src/action/macos/bootstrap_launchctl_service.rs index 8882b3a21..4d34ba44e 100644 --- a/src/action/macos/bootstrap_launchctl_service.rs +++ b/src/action/macos/bootstrap_launchctl_service.rs @@ -139,7 +139,7 @@ impl Action for BootstrapLaunchctlService { #[tracing::instrument(level = "debug", skip_all)] async fn revert(&mut self) -> Result<(), ActionError> { - crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, &self.service, &self.path) + crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, &self.service) .await .map_err(Self::error)?; diff --git a/src/action/macos/create_determinate_volume_service.rs b/src/action/macos/create_determinate_volume_service.rs index 15b76028f..13956d31e 100644 --- a/src/action/macos/create_determinate_volume_service.rs +++ b/src/action/macos/create_determinate_volume_service.rs @@ -139,7 +139,7 @@ impl Action for CreateDeterminateVolumeService { } = self; if *needs_bootout { - crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, mount_service_label, path) + crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, mount_service_label) .await .map_err(Self::error)?; } diff --git a/src/action/macos/create_nix_hook_service.rs b/src/action/macos/create_nix_hook_service.rs index 3a8a1a646..e4799b38b 100644 --- a/src/action/macos/create_nix_hook_service.rs +++ b/src/action/macos/create_nix_hook_service.rs @@ -127,7 +127,7 @@ impl Action for CreateNixHookService { } = self; if *needs_bootout { - crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, service_label, path) + crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, service_label) .await .map_err(Self::error)?; } diff --git a/src/action/macos/create_volume_service.rs b/src/action/macos/create_volume_service.rs index e4c97681f..9b610cb9b 100644 --- a/src/action/macos/create_volume_service.rs +++ b/src/action/macos/create_volume_service.rs @@ -186,7 +186,7 @@ impl Action for CreateVolumeService { } = self; if *needs_bootout { - crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, mount_service_label, path) + crate::action::macos::retry_bootout(DARWIN_LAUNCHD_DOMAIN, mount_service_label) .await .map_err(Self::error)?; } diff --git a/src/action/macos/mod.rs b/src/action/macos/mod.rs index fe1366c08..a7f352c23 100644 --- a/src/action/macos/mod.rs +++ b/src/action/macos/mod.rs @@ -199,16 +199,14 @@ pub(crate) async fn retry_bootstrap( /// Wait for `launchctl bootout {domain} {service_path}` to succeed up to `retry_tokens * 500ms` amount /// of time. #[tracing::instrument] -pub(crate) async fn retry_bootout( - domain: &str, - service_name: &str, - service_path: &Path, -) -> Result<(), ActionErrorKind> { +pub(crate) async fn retry_bootout(domain: &str, service_name: &str) -> Result<(), ActionErrorKind> { + let service_identifier = [domain, service_name].join("/"); + let check_service_running = execute_command( Command::new("launchctl") .process_group(0) .arg("print") - .arg([domain, service_name].join("/")) + .arg(&service_identifier) .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()), @@ -226,8 +224,7 @@ pub(crate) async fn retry_bootout( let mut command = Command::new("launchctl"); command.process_group(0); command.arg("bootout"); - command.arg(domain); - command.arg(service_path); + command.arg(&service_identifier); command.stdin(std::process::Stdio::null()); command.stderr(std::process::Stdio::null()); command.stdout(std::process::Stdio::null()); diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 3738daf50..315c3ee44 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -47,7 +47,7 @@ impl CommandExecute for NixInstallerCli { NixInstallerSubcommand::Plan(plan) => plan.execute().await, NixInstallerSubcommand::SelfTest(self_test) => self_test.execute().await, NixInstallerSubcommand::Install(install) => install.execute().await, - NixInstallerSubcommand::Repair(restore_shell) => restore_shell.execute().await, + NixInstallerSubcommand::Repair(repair) => repair.execute().await, NixInstallerSubcommand::Uninstall(revert) => revert.execute().await, } } diff --git a/src/planner/linux.rs b/src/planner/linux.rs index 0fe2dc594..c4a0dd766 100644 --- a/src/planner/linux.rs +++ b/src/planner/linux.rs @@ -25,6 +25,8 @@ use crate::{ Action, BuiltinPlanner, }; +pub const FHS_SELINUX_POLICY_PATH: &str = "/usr/share/selinux/packages/nix.pp"; + /// A planner for traditional, mutable Linux systems like Debian, RHEL, or Arch #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "cli", derive(clap::Parser))] @@ -92,7 +94,7 @@ impl Planner for Linux { if has_selinux { plan.push( ProvisionSelinux::plan( - "/usr/share/selinux/packages/nix.pp".into(), + FHS_SELINUX_POLICY_PATH.into(), if self.settings.determinate_nix { DETERMINATE_SELINUX_POLICY_PP_CONTENT } else {