bmc: merge update and prepare systemd

The update systemd and prepare systemd were effectively the same, and
this makes the systemd action generic.

Tested: Not tested beyond unit-tests continuing to pass.
Signed-off-by: Patrick Venture <venture@google.com>
Change-Id: I8070731e53bfd6cdafb1d74d9db652f20341985e

10 files changed, 108 insertions, 275 deletions

diff --git a/bmc/Makefile.am b/bmc/Makefile.am
index a33a63e..f8db30b 100644
--- a/bmc/Makefile.am
+++ b/bmc/Makefile.am
@@ -40,9 +40,7 @@ libfirmwareblob_common_la_SOURCES = \
 	fs.cpp \
 	firmware_handler.cpp \
 	file_handler.cpp \
-	prepare_systemd.cpp \
 	general_systemd.cpp \
-	update_systemd.cpp \
 	lpc_handler.cpp
 
 if ENABLE_ASPEED_LPC
diff --git a/bmc/buildjson.cpp b/bmc/buildjson.cpp
index 9de4b87..45796ca 100644
--- a/bmc/buildjson.cpp
+++ b/bmc/buildjson.cpp
@@ -18,8 +18,6 @@
 #include "file_handler.hpp"
 #include "fs.hpp"
 #include "general_systemd.hpp"
-#include "prepare_systemd.hpp"
-#include "update_systemd.hpp"
 
 #include <algorithm>
 #include <cstdio>
@@ -54,6 +52,24 @@ std::unique_ptr<TriggerableActionInterface>
         sdbusplus::bus::new_default(), path, unit, systemdMode);
 }
 
+std::unique_ptr<TriggerableActionInterface>
+    buildSystemd(const nlohmann::json& data)
+{
+    /* This type of action requires a unit, and optionally a mode. */
+    const auto& unit = data.at("unit");
+
+    /* the mode parameter is optional. */
+    std::string systemdMode = "replace";
+    const auto& mode = data.find("mode");
+    if (mode != data.end())
+    {
+        systemdMode = data.at("mode").get<std::string>();
+    }
+
+    return SystemdNoFile::CreateSystemdNoFile(sdbusplus::bus::new_default(),
+                                              unit, systemdMode);
+}
+
 std::vector<HandlerConfig> buildHandlerFromJson(const nlohmann::json& data)
 {
     std::vector<HandlerConfig> handlers;
@@ -103,18 +119,7 @@ std::vector<HandlerConfig> buildHandlerFromJson(const nlohmann::json& data)
             const std::string prepareType = prep.at("type");
             if (prepareType == "systemd")
             {
-                const auto& unit = prep.at("unit");
-
-                /* the mode parameter is optional. */
-                std::string systemdMode = "replace";
-                const auto& mode = prep.find("mode");
-                if (mode != prep.end())
-                {
-                    systemdMode = prep.at("mode").get<std::string>();
-                }
-
-                pack->preparation = SystemdPreparation::CreatePreparation(
-                    sdbusplus::bus::new_default(), unit, systemdMode);
+                pack->preparation = std::move(buildSystemd(prep));
             }
             else
             {
@@ -138,7 +143,7 @@ std::vector<HandlerConfig> buildHandlerFromJson(const nlohmann::json& data)
             const std::string updateType = update.at("type");
             if (updateType == "reboot")
             {
-                pack->update = SystemdUpdateMechanism::CreateSystemdUpdate(
+                pack->update = SystemdNoFile::CreateSystemdNoFile(
                     sdbusplus::bus::new_default(), "reboot.target",
                     "replace-irreversibly");
             }
@@ -148,18 +153,7 @@ std::vector<HandlerConfig> buildHandlerFromJson(const nlohmann::json& data)
             }
             else if (updateType == "systemd")
             {
-                const auto& unit = update.at("unit");
-
-                /* the mode parameter is optional. */
-                std::string systemdMode = "replace";
-                const auto& mode = update.find("mode");
-                if (mode != update.end())
-                {
-                    systemdMode = update.at("mode").get<std::string>();
-                }
-
-                pack->update = SystemdUpdateMechanism::CreateSystemdUpdate(
-                    sdbusplus::bus::new_default(), unit, systemdMode);
+                pack->update = std::move(buildSystemd(update));
             }
             else
             {
diff --git a/bmc/general_systemd.cpp b/bmc/general_systemd.cpp
index ff51f39..5b9fb87 100644
--- a/bmc/general_systemd.cpp
+++ b/bmc/general_systemd.cpp
@@ -103,4 +103,54 @@ const std::string SystemdWithStatusFile::getMode() const
     return mode;
 }
 
+std::unique_ptr<TriggerableActionInterface>
+    SystemdNoFile::CreateSystemdNoFile(sdbusplus::bus::bus&& bus,
+                                       const std::string& service,
+                                       const std::string& mode)
+{
+    return std::make_unique<SystemdNoFile>(std::move(bus), service, mode);
+}
+
+bool SystemdNoFile::trigger()
+{
+    static constexpr auto systemdService = "org.freedesktop.systemd1";
+    static constexpr auto systemdRoot = "/org/freedesktop/systemd1";
+    static constexpr auto systemdInterface = "org.freedesktop.systemd1.Manager";
+
+    auto method = bus.new_method_call(systemdService, systemdRoot,
+                                      systemdInterface, "StartUnit");
+    method.append(triggerService);
+    method.append(mode);
+
+    try
+    {
+        bus.call_noreply(method);
+        state = ActionStatus::running;
+        return true;
+    }
+    catch (const sdbusplus::exception::SdBusError& ex)
+    {
+        /* TODO: Once logging supports unit-tests, add a log message to test
+         * this failure.
+         */
+        state = ActionStatus::failed;
+        return false;
+    }
+}
+
+void SystemdNoFile::abort()
+{
+    return;
+}
+
+ActionStatus SystemdNoFile::status()
+{
+    return state;
+}
+
+const std::string SystemdNoFile::getMode() const
+{
+    return mode;
+}
+
 } // namespace ipmi_flash
diff --git a/bmc/general_systemd.hpp b/bmc/general_systemd.hpp
index 5c4c0e3..c7fe4cb 100644
--- a/bmc/general_systemd.hpp
+++ b/bmc/general_systemd.hpp
@@ -57,4 +57,39 @@ class SystemdWithStatusFile : public TriggerableActionInterface
     const std::string triggerService;
     const std::string mode;
 };
+
+class SystemdNoFile : public TriggerableActionInterface
+{
+  public:
+    static std::unique_ptr<TriggerableActionInterface>
+        CreateSystemdNoFile(sdbusplus::bus::bus&& bus,
+                            const std::string& service,
+                            const std::string& mode);
+
+    SystemdNoFile(sdbusplus::bus::bus&& bus, const std::string& service,
+                  const std::string& mode) :
+        bus(std::move(bus)),
+        triggerService(service), mode(mode)
+    {
+    }
+
+    ~SystemdNoFile() = default;
+    SystemdNoFile(const SystemdNoFile&) = delete;
+    SystemdNoFile& operator=(const SystemdNoFile&) = delete;
+    SystemdNoFile(SystemdNoFile&&) = default;
+    SystemdNoFile& operator=(SystemdNoFile&&) = default;
+
+    bool trigger() override;
+    void abort() override;
+    ActionStatus status() override;
+
+    const std::string getMode() const;
+
+  private:
+    sdbusplus::bus::bus bus;
+    const std::string triggerService;
+    const std::string mode;
+    ActionStatus state = ActionStatus::unknown;
+};
+
 } // namespace ipmi_flash
diff --git a/bmc/main.cpp b/bmc/main.cpp
index c0526de..ab7ece8 100644
--- a/bmc/main.cpp
+++ b/bmc/main.cpp
@@ -26,9 +26,7 @@
 #include "lpc_handler.hpp"
 #include "lpc_nuvoton.hpp"
 #include "pci_handler.hpp"
-#include "prepare_systemd.hpp"
 #include "status.hpp"
-#include "update_systemd.hpp"
 #include "util.hpp"
 
 #include <cstdint>
diff --git a/bmc/prepare_systemd.cpp b/bmc/prepare_systemd.cpp
deleted file mode 100644
index c08fdd1..0000000
--- a/bmc/prepare_systemd.cpp
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * Copyright 2019 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "prepare_systemd.hpp"
-
-#include "status.hpp"
-
-#include <memory>
-
-namespace ipmi_flash
-{
-
-std::unique_ptr<TriggerableActionInterface>
-    SystemdPreparation::CreatePreparation(sdbusplus::bus::bus&& bus,
-                                          const std::string& service,
-                                          const std::string& mode)
-{
-    return std::make_unique<SystemdPreparation>(std::move(bus), service, mode);
-}
-
-bool SystemdPreparation::trigger()
-{
-    static constexpr auto systemdService = "org.freedesktop.systemd1";
-    static constexpr auto systemdRoot = "/org/freedesktop/systemd1";
-    static constexpr auto systemdInterface = "org.freedesktop.systemd1.Manager";
-
-    auto method = bus.new_method_call(systemdService, systemdRoot,
-                                      systemdInterface, "StartUnit");
-    method.append(triggerService);
-    method.append(mode);
-
-    try
-    {
-        bus.call_noreply(method);
-    }
-    catch (const sdbusplus::exception::SdBusError& ex)
-    {
-        /* TODO: Once logging supports unit-tests, add a log message to test
-         * this failure.
-         */
-        state = ActionStatus::failed;
-        return false;
-    }
-
-    state = ActionStatus::success;
-    return true;
-}
-
-void SystemdPreparation::abort()
-{
-    return;
-}
-
-ActionStatus SystemdPreparation::status()
-{
-    return state;
-}
-
-} // namespace ipmi_flash
diff --git a/bmc/prepare_systemd.hpp b/bmc/prepare_systemd.hpp
deleted file mode 100644
index b8fe605..0000000
--- a/bmc/prepare_systemd.hpp
+++ /dev/null
@@ -1,43 +0,0 @@
-#pragma once
-
-#include "status.hpp"
-
-#include <memory>
-#include <sdbusplus/bus.hpp>
-#include <string>
-
-namespace ipmi_flash
-{
-
-class SystemdPreparation : public TriggerableActionInterface
-{
-  public:
-    static std::unique_ptr<TriggerableActionInterface>
-        CreatePreparation(sdbusplus::bus::bus&& bus, const std::string& service,
-                          const std::string& mode);
-
-    SystemdPreparation(sdbusplus::bus::bus&& bus, const std::string& service,
-                       const std::string& mode) :
-        bus(std::move(bus)),
-        triggerService(service), mode(mode)
-    {
-    }
-
-    ~SystemdPreparation() = default;
-    SystemdPreparation(const SystemdPreparation&) = delete;
-    SystemdPreparation& operator=(const SystemdPreparation&) = delete;
-    SystemdPreparation(SystemdPreparation&&) = default;
-    SystemdPreparation& operator=(SystemdPreparation&&) = default;
-
-    bool trigger() override;
-    void abort() override;
-    ActionStatus status() override;
-
-  private:
-    sdbusplus::bus::bus bus;
-    const std::string triggerService;
-    const std::string mode;
-    ActionStatus state = ActionStatus::unknown;
-};
-
-} // namespace ipmi_flash
diff --git a/bmc/test/firmware_json_unittest.cpp b/bmc/test/firmware_json_unittest.cpp
index bb2585d..d56a20a 100644
--- a/bmc/test/firmware_json_unittest.cpp
+++ b/bmc/test/firmware_json_unittest.cpp
@@ -1,6 +1,5 @@
 #include "buildjson.hpp"
 #include "general_systemd.hpp"
-#include "update_systemd.hpp"
 
 #include <nlohmann/json.hpp>
 
@@ -490,8 +489,7 @@ TEST(FirmwareJsonTest, VerifyValidSingleNonReboot)
         h[0].actions->verification.get());
     EXPECT_THAT(verifier->getMode(), "replace");
     EXPECT_FALSE(h[0].actions->update == nullptr);
-    auto updater =
-        reinterpret_cast<SystemdUpdateMechanism*>(h[0].actions->update.get());
+    auto updater = reinterpret_cast<SystemdNoFile*>(h[0].actions->update.get());
     EXPECT_THAT(updater->getMode(), "replace");
 }
 
@@ -534,8 +532,7 @@ TEST(FirmwareJsonTest, VerifyValidWithModes)
         h[0].actions->verification.get());
     EXPECT_THAT(verifier->getMode(), "replace-nope");
     EXPECT_FALSE(h[0].actions->update == nullptr);
-    auto updater =
-        reinterpret_cast<SystemdUpdateMechanism*>(h[0].actions->update.get());
+    auto updater = reinterpret_cast<SystemdNoFile*>(h[0].actions->update.get());
     EXPECT_THAT(updater->getMode(), "replace-fake");
 }
 
diff --git a/bmc/update_systemd.cpp b/bmc/update_systemd.cpp
deleted file mode 100644
index afd4316..0000000
--- a/bmc/update_systemd.cpp
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright 2019 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "update_systemd.hpp"
-
-#include "status.hpp"
-
-#include <memory>
-#include <sdbusplus/bus.hpp>
-#include <string>
-
-namespace ipmi_flash
-{
-
-std::unique_ptr<TriggerableActionInterface>
-    SystemdUpdateMechanism::CreateSystemdUpdate(sdbusplus::bus::bus&& bus,
-                                                const std::string& target,
-                                                const std::string& mode)
-{
-    return std::make_unique<SystemdUpdateMechanism>(std::move(bus), target,
-                                                    mode);
-}
-
-bool SystemdUpdateMechanism::trigger()
-{
-    /* TODO: Add a util method for triggering a service with optional additional
-     * parameter. */
-    static constexpr auto systemdService = "org.freedesktop.systemd1";
-    static constexpr auto systemdRoot = "/org/freedesktop/systemd1";
-    static constexpr auto systemdInterface = "org.freedesktop.systemd1.Manager";
-
-    auto method = bus.new_method_call(systemdService, systemdRoot,
-                                      systemdInterface, "StartUnit");
-    method.append(target);
-    method.append(mode);
-
-    try
-    {
-        bus.call_noreply(method);
-        return true;
-    }
-    catch (const sdbusplus::exception::SdBusError& ex)
-    {
-        return false;
-    }
-}
-
-void SystemdUpdateMechanism::abort()
-{
-    return;
-}
-
-ActionStatus SystemdUpdateMechanism::status()
-{
-    /* For now, don't check if the target failed. */
-    return ActionStatus::running;
-}
-
-const std::string SystemdUpdateMechanism::getMode() const
-{
-    return mode;
-}
-
-} // namespace ipmi_flash
diff --git a/bmc/update_systemd.hpp b/bmc/update_systemd.hpp
deleted file mode 100644
index a8dd8fc..0000000
--- a/bmc/update_systemd.hpp
+++ /dev/null
@@ -1,47 +0,0 @@
-#pragma once
-
-#include "status.hpp"
-
-#include <memory>
-#include <sdbusplus/bus.hpp>
-#include <string>
-
-namespace ipmi_flash
-{
-
-/**
- * Implements the update interface by simply triggering a systemd unit.
- */
-class SystemdUpdateMechanism : public TriggerableActionInterface
-{
-  public:
-    static std::unique_ptr<TriggerableActionInterface>
-        CreateSystemdUpdate(sdbusplus::bus::bus&& bus,
-                            const std::string& target, const std::string& mode);
-
-    SystemdUpdateMechanism(sdbusplus::bus::bus&& bus, const std::string& target,
-                           const std::string& mode) :
-        bus(std::move(bus)),
-        target(target), mode(mode)
-    {
-    }
-
-    ~SystemdUpdateMechanism() = default;
-    SystemdUpdateMechanism(const SystemdUpdateMechanism&) = delete;
-    SystemdUpdateMechanism& operator=(const SystemdUpdateMechanism&) = delete;
-    SystemdUpdateMechanism(SystemdUpdateMechanism&&) = default;
-    SystemdUpdateMechanism& operator=(SystemdUpdateMechanism&&) = default;
-
-    bool trigger() override;
-    void abort() override;
-    ActionStatus status() override;
-
-    const std::string getMode() const;
-
-  private:
-    sdbusplus::bus::bus bus;
-    const std::string target;
-    const std::string mode;
-};
-
-} // namespace ipmi_flash