crit-service: create error on failed service
This enhances the existing code to support logging an error when a
monitored service fails. The same systemd event is triggered for a
target failure and a service failure so no new logic is needed in
that area.
Tested:
- Repeatedly killed the host-state service until its unit went into the
failed state. Verified this was detected and the expected log was
created.
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Change-Id: I459dd8c35ceddec986336fee635fdf691257f758
diff --git a/systemd_target_monitor.cpp b/systemd_target_monitor.cpp
index 0d8ae74..a02f693 100644
--- a/systemd_target_monitor.cpp
+++ b/systemd_target_monitor.cpp
@@ -85,7 +85,8 @@
dump_targets(targetData);
}
- phosphor::state::manager::SystemdTargetLogging targetMon(targetData, bus);
+ phosphor::state::manager::SystemdTargetLogging targetMon(targetData,
+ serviceData, bus);
// Subscribe to systemd D-bus signals indicating target completions
targetMon.subscribeToSystemdSignals();
diff --git a/systemd_target_signal.cpp b/systemd_target_signal.cpp
index 25089ef..9f778d5 100644
--- a/systemd_target_signal.cpp
+++ b/systemd_target_signal.cpp
@@ -42,8 +42,8 @@
}
}
-const std::string* SystemdTargetLogging::processError(const std::string& unit,
- const std::string& result)
+const std::string SystemdTargetLogging::processError(const std::string& unit,
+ const std::string& result)
{
auto targetEntry = this->targetData.find(unit);
if (targetEntry != this->targetData.end())
@@ -56,10 +56,25 @@
info(
"Monitored systemd unit has hit an error, unit:{UNIT}, result:{RESULT}",
"UNIT", unit, "RESULT", result);
- return (&targetEntry->second.errorToLog);
+ return (targetEntry->second.errorToLog);
}
}
- return nullptr;
+
+ // Check if it's in our list of services to monitor
+ if (std::find(this->serviceData.begin(), this->serviceData.end(), unit) !=
+ this->serviceData.end())
+ {
+ if (result == "failed")
+ {
+ info(
+ "Monitored systemd service has hit an error, unit:{UNIT}, result:{RESULT}",
+ "UNIT", unit, "RESULT", result);
+ return (std::string{
+ "xyz.openbmc_project.State.Error.CriticalServiceFailure"});
+ }
+ }
+
+ return (std::string{});
}
void SystemdTargetLogging::systemdUnitChange(sdbusplus::message::message& msg)
@@ -74,12 +89,12 @@
// In most cases it will just be success, in which case just return
if (result != "done")
{
- const std::string* error = processError(unit, result);
+ const std::string error = processError(unit, result);
// If this is a monitored error then log it
- if (error)
+ if (!error.empty())
{
- logError(*error, result);
+ logError(error, result);
}
}
return;
diff --git a/systemd_target_signal.hpp b/systemd_target_signal.hpp
index d9a58c7..8674ca5 100644
--- a/systemd_target_signal.hpp
+++ b/systemd_target_signal.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include "systemd_service_parser.hpp"
#include "systemd_target_parser.hpp"
#include <sdbusplus/bus.hpp>
@@ -28,9 +29,10 @@
virtual ~SystemdTargetLogging() = default;
SystemdTargetLogging(const TargetErrorData& targetData,
+ const ServiceMonitorData& serviceData,
sdbusplus::bus::bus& bus) :
targetData(targetData),
- bus(bus),
+ serviceData(serviceData), bus(bus),
systemdJobRemovedSignal(
bus,
sdbusplus::bus::match::rules::type::signal() +
@@ -66,8 +68,8 @@
*
* @return valid pointer to error to log, otherwise nullptr
*/
- const std::string* processError(const std::string& unit,
- const std::string& result);
+ const std::string processError(const std::string& unit,
+ const std::string& result);
private:
/** @brief Call phosphor-logging to create error
@@ -100,6 +102,9 @@
/** @brief Systemd targets to monitor and error logs to create */
const TargetErrorData& targetData;
+ /** @brief Systemd targets to monitor and error logs to create */
+ const ServiceMonitorData& serviceData;
+
/** @brief Persistent sdbusplus DBus bus connection. */
sdbusplus::bus::bus& bus;
diff --git a/test/systemd_signal.cpp b/test/systemd_signal.cpp
index cd4b980..7f7ce55 100644
--- a/test/systemd_signal.cpp
+++ b/test/systemd_signal.cpp
@@ -21,25 +21,29 @@
{"xyz.openbmc_project.State.Chassis.Error.PowerOnTargetFailure",
{"timeout", "failed"}}}};
+ ServiceMonitorData serviceData = {
+ "xyz.openbmc_project.biosconfig_manager.service",
+ "xyz.openbmc_project.Dump.Manager.service"};
+
auto bus = sdbusplus::bus::new_default();
auto event = sdeventplus::Event::get_default();
bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
- phosphor::state::manager::SystemdTargetLogging targetMon(targetData, bus);
+ phosphor::state::manager::SystemdTargetLogging targetMon(targetData,
+ serviceData, bus);
std::string invalidUnit = "invalid_unit";
std::string validError = "timeout";
- const std::string* errorToLog =
- targetMon.processError(invalidUnit, validError);
- EXPECT_EQ(errorToLog, nullptr);
+ std::string errorToLog = targetMon.processError(invalidUnit, validError);
+ EXPECT_TRUE(errorToLog.empty());
std::string validUnit = "obmc-chassis-poweron@0.target";
std::string invalidError = "invalid_error";
errorToLog = targetMon.processError(validUnit, invalidError);
- EXPECT_EQ(errorToLog, nullptr);
+ EXPECT_TRUE(errorToLog.empty());
errorToLog = targetMon.processError(validUnit, validError);
- EXPECT_NE(errorToLog, nullptr);
- EXPECT_EQ(*errorToLog,
+ EXPECT_FALSE(errorToLog.empty());
+ EXPECT_EQ(errorToLog,
"xyz.openbmc_project.State.Chassis.Error.PowerOnTargetFailure");
}