From 5874134d07c865cb65a4a5976dbc0e597bb104d6 Mon Sep 17 00:00:00 2001
From: Siegfried Weber <mail@siegfriedweber.net>
Date: Fri, 27 Aug 2021 16:51:33 +0200
Subject: [PATCH] Improve error message

Error message improved which is logged if a systemd unit file cannot be
created.
---
 CHANGELOG.adoc                              |  5 ++++-
 src/provider/states/pod/creating_service.rs | 21 +++++++++------------
 src/provider/systemdmanager/manager.rs      | 18 +++++++-----------
 3 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index ae11179..9173c28 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -7,9 +7,10 @@
 :270: https://github.com/stackabletech/agent/pull/270[#270]
 :273: https://github.com/stackabletech/agent/pull/273[#273]
 :274: https://github.com/stackabletech/agent/pull/274[#274]
+:276: https://github.com/stackabletech/agent/pull/276[#276]
 
 === Added
-* Prints self-diagnostic information on startup ({270})
+* Prints self-diagnostic information on startup ({270}).
 * Check added on startup if the configured directories exist and are
   writable by the Stackable agent ({273}).
 * Missing directories are created ({274}).
@@ -20,6 +21,8 @@
 * `certificates.k8s.io/v1` used instead of `certificates.k8s.io/v1beta1`
   so that the Stackable Agent is now compatible with Kubernetes v1.22
   but not any longer with versions prior to v1.19 ({267}).
+* Error message improved which is logged if a systemd unit file cannot
+  be created ({276}).
 
 == 0.5.0 - 2021-07-26
 
diff --git a/src/provider/states/pod/creating_service.rs b/src/provider/states/pod/creating_service.rs
index d8166d6..0cc7a3f 100644
--- a/src/provider/states/pod/creating_service.rs
+++ b/src/provider/states/pod/creating_service.rs
@@ -9,7 +9,7 @@ use super::setup_failed::SetupFailed;
 use super::starting::Starting;
 use crate::provider::systemdmanager::systemdunit::SystemDUnit;
 use crate::provider::{ContainerHandle, PodState, ProviderState};
-use anyhow::Error;
+use anyhow::{Context, Error};
 use dirs::home_dir;
 use std::env;
 use std::fs::create_dir_all;
@@ -113,17 +113,14 @@ impl State<PodState> for CreatingService {
             // Create the service
             // As per ADR005 we currently write the unit files directly in the systemd
             // unit directory (by passing None as [unit_file_path]).
-            match systemd_manager.create_unit(&unit, None, true, true).await {
-                Ok(()) => {}
-                Err(e) => {
-                    // TODO: We need to discuss what to do here, in theory we could have loaded
-                    // other services already, do we want to stop those?
-                    error!(
-                        "Failed to create systemd unit for service [{}]",
-                        service_name
-                    );
-                    return Transition::Complete(Err(e));
-                }
+            if let Err(e) = systemd_manager
+                .create_unit(&unit, None, true, true)
+                .await
+                .with_context(|| format!("Unit file [{}] could not be created", unit))
+            {
+                // TODO: We need to discuss what to do here, in theory we could have loaded
+                // other services already, do we want to stop those?
+                return Transition::Complete(Err(e));
             }
 
             let systemd_service = match systemd_manager
diff --git a/src/provider/systemdmanager/manager.rs b/src/provider/systemdmanager/manager.rs
index eab37b9..2217899 100644
--- a/src/provider/systemdmanager/manager.rs
+++ b/src/provider/systemdmanager/manager.rs
@@ -11,7 +11,7 @@ use super::systemd1_api::{
 use crate::provider::systemdmanager::systemdunit::SystemDUnit;
 use crate::provider::StackableError;
 use crate::provider::StackableError::RuntimeError;
-use anyhow::anyhow;
+use anyhow::{anyhow, Context};
 use futures_util::{future, stream::StreamExt};
 use log::debug;
 use std::fs;
@@ -190,16 +190,12 @@ impl SystemdManager {
         if !target_file.exists() {
             // Write unit file, no matter where
             // TODO: implement check for content equality
-            let mut unit_file = match File::create(&target_file) {
-                Ok(file) => file,
-                Err(e) => {
-                    debug!(
-                        "Error occurred when creating unit file [{}]: [{}]",
-                        unit_name, e
-                    );
-                    return Err(anyhow::Error::from(e));
-                }
-            };
+            let mut unit_file = File::create(&target_file).with_context(|| {
+                format!(
+                    "File [{}] could not be created",
+                    target_file.to_string_lossy()
+                )
+            })?;
             unit_file.write_all(unit.get_unit_file_content().as_bytes())?;
             unit_file.flush()?;
         }