Activation: Support to update multiple PSUs
Queue the update on multiple PSUs, and do the update one-by-one.
Tested: Write unit test cases and verify the cases pass.
On witherspoon, verify the dummy update services are run on all
PSUs.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: Icfe0f6e74623e54840df8d731d852d53d6071768
diff --git a/src/activation.cpp b/src/activation.cpp
index 0ba7e64..f63352a 100644
--- a/src/activation.cpp
+++ b/src/activation.cpp
@@ -54,7 +54,7 @@
{
if (value == Status::Activating)
{
- startActivation();
+ value = startActivation();
}
else
{
@@ -94,16 +94,67 @@
{
if (newStateResult == "done")
{
- finishActivation();
+ onUpdateDone();
}
if (newStateResult == "failed" || newStateResult == "dependency")
{
- activation(Status::Failed);
+ onUpdateFailed();
}
}
}
-void Activation::startActivation()
+bool Activation::doUpdate(const std::string& psuInventoryPath)
+{
+ psuUpdateUnit = internal::getUpdateService(psuInventoryPath, versionId);
+ try
+ {
+ auto method = bus.new_method_call(SYSTEMD_BUSNAME, SYSTEMD_PATH,
+ SYSTEMD_INTERFACE, "StartUnit");
+ method.append(psuUpdateUnit, "replace");
+ bus.call_noreply(method);
+ return true;
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error staring service", entry("ERROR=%s", e.what()));
+ onUpdateFailed();
+ return false;
+ }
+}
+
+bool Activation::doUpdate()
+{
+ // When the queue is empty, all updates are done
+ if (psuQueue.empty())
+ {
+ finishActivation();
+ return true;
+ }
+
+ // Do the update on a PSU
+ const auto& psu = psuQueue.front();
+ return doUpdate(psu);
+}
+
+void Activation::onUpdateDone()
+{
+ auto progress = activationProgress->progress() + progressStep;
+ activationProgress->progress(progress);
+
+ psuQueue.pop();
+ doUpdate(); // Update the next psu
+}
+
+void Activation::onUpdateFailed()
+{
+ // TODO: report an event
+ log<level::ERR>("Failed to udpate PSU",
+ entry("PSU=%s", psuQueue.front().c_str()));
+ std::queue<std::string>().swap(psuQueue); // Clear the queue
+ activation(Status::Failed);
+}
+
+Activation::Status Activation::startActivation()
{
if (!activationProgress)
{
@@ -115,29 +166,40 @@
std::make_unique<ActivationBlocksTransition>(bus, path);
}
- // TODO: for now only update one psu, future commits shall handle update
- // multiple psus
auto psuPaths = utils::getPSUInventoryPath(bus);
if (psuPaths.empty())
{
- return;
+ log<level::WARNING>("No PSU inventory found");
+ return Status::Failed;
}
- psuUpdateUnit = internal::getUpdateService(psuPaths[0], versionId);
+ for (const auto& p : psuPaths)
+ {
+ psuQueue.push(p);
+ }
- auto method = bus.new_method_call(SYSTEMD_BUSNAME, SYSTEMD_PATH,
- SYSTEMD_INTERFACE, "StartUnit");
- method.append(psuUpdateUnit, "replace");
- bus.call_noreply(method);
-
- activationProgress->progress(10);
+ // The progress to be increased for each successful update of PSU
+ // E.g. in case we have 4 PSUs:
+ // 1. Initial progrss is 10
+ // 2. Add 20 after each update is done, so we will see progress to be 30,
+ // 50, 70, 90
+ // 3. When all PSUs are updated, it will be 100 and the interface is
+ // removed.
+ progressStep = 80 / psuQueue.size();
+ if (doUpdate())
+ {
+ activationProgress->progress(10);
+ return Status::Activating;
+ }
+ else
+ {
+ return Status::Failed;
+ }
}
void Activation::finishActivation()
{
activationProgress->progress(100);
- activationBlocksTransition.reset();
- activationProgress.reset();
// TODO: delete the old software object
// TODO: create related associations
diff --git a/src/activation.hpp b/src/activation.hpp
index d7880f6..59364e4 100644
--- a/src/activation.hpp
+++ b/src/activation.hpp
@@ -4,6 +4,7 @@
#include "types.hpp"
+#include <queue>
#include <sdbusplus/server.hpp>
#include <xyz/openbmc_project/Association/Definitions/server.hpp>
#include <xyz/openbmc_project/Software/Activation/server.hpp>
@@ -11,6 +12,8 @@
#include <xyz/openbmc_project/Software/ActivationProgress/server.hpp>
#include <xyz/openbmc_project/Software/ExtendedVersion/server.hpp>
+class TestActivation;
+
namespace phosphor
{
namespace software
@@ -106,7 +109,9 @@
class Activation : public ActivationInherit
{
public:
+ friend class ::TestActivation;
using Status = Activations;
+
/** @brief Constructs Activation Software Manager
*
* @param[in] bus - The Dbus bus object
@@ -118,9 +123,7 @@
*/
Activation(sdbusplus::bus::bus& bus, const std::string& path,
const std::string& versionId, const std::string& extVersion,
- sdbusplus::xyz::openbmc_project::Software::server::Activation::
- Activations activationStatus,
- const AssociationList& assocs) :
+ Status activationStatus, const AssociationList& assocs) :
ActivationInherit(bus, path.c_str(), true),
versionId(versionId), bus(bus), path(path),
systemdSignals(
@@ -146,7 +149,7 @@
*
* @return Success or exception thrown
*/
- Activations activation(Activations value) override;
+ Status activation(Status value) override;
/** @brief Activation */
using ActivationInherit::activation;
@@ -180,8 +183,28 @@
*/
void deleteImageManagerObject();
+ /** @brief Invoke the update service for the PSU
+ *
+ * @param[in] psuInventoryPath - The PSU inventory to be updated.
+ *
+ * @return true if the update starts, and false if it fails.
+ */
+ bool doUpdate(const std::string& psuInventoryPath);
+
+ /** @brief Do PSU update one-by-one
+ *
+ * @return true if the update starts, and false if it fails.
+ */
+ bool doUpdate();
+
+ /** @brief Handle an update done event */
+ void onUpdateDone();
+
+ /** @brief Handle an update failure event */
+ void onUpdateFailed();
+
/** @brief Start PSU update */
- void startActivation();
+ Status startActivation();
/** @brief Finish PSU update */
void finishActivation();
@@ -195,6 +218,12 @@
/** @brief Used to subscribe to dbus systemd signals */
sdbusplus::bus::match_t systemdSignals;
+ /** @brief The queue of psu objects to be updated */
+ std::queue<std::string> psuQueue;
+
+ /** @brief The progress step for each PSU update is done */
+ uint32_t progressStep;
+
/** @brief The PSU update systemd unit */
std::string psuUpdateUnit;
diff --git a/test/mocked_utils.hpp b/test/mocked_utils.hpp
index 7afe07c..85f5ffc 100644
--- a/test/mocked_utils.hpp
+++ b/test/mocked_utils.hpp
@@ -33,7 +33,7 @@
const char* propertyName));
};
-const UtilsInterface& getUtils()
+inline const UtilsInterface& getUtils()
{
static MockedUtils utils;
return utils;
diff --git a/test/test_activation.cpp b/test/test_activation.cpp
index 0ea3640..338dc60 100644
--- a/test/test_activation.cpp
+++ b/test/test_activation.cpp
@@ -1,4 +1,5 @@
#include "activation.hpp"
+#include "mocked_utils.hpp"
#include <sdbusplus/test/sdbus_mock.hpp>
@@ -7,24 +8,43 @@
using namespace phosphor::software::updater;
+using ::testing::_;
+using ::testing::Return;
+
class TestActivation : public ::testing::Test
{
public:
- using ActivationStatus = sdbusplus::xyz::openbmc_project::Software::server::
- Activation::Activations;
- TestActivation()
+ using Status = Activation::Status;
+ using RequestedStatus = Activation::RequestedActivations;
+ TestActivation() :
+ mockedUtils(
+ reinterpret_cast<const utils::MockedUtils&>(utils::getUtils()))
{
}
~TestActivation()
{
}
+
+ void onUpdateDone()
+ {
+ activation->onUpdateDone();
+ }
+ void onUpdateFailed()
+ {
+ activation->onUpdateFailed();
+ }
+ int getProgress()
+ {
+ return activation->activationProgress->progress();
+ }
static constexpr auto dBusPath = SOFTWARE_OBJPATH;
sdbusplus::SdBusMock sdbusMock;
sdbusplus::bus::bus mockedBus = sdbusplus::get_mocked_new(&sdbusMock);
+ const utils::MockedUtils& mockedUtils;
std::unique_ptr<Activation> activation;
std::string versionId = "abcdefgh";
std::string extVersion = "Some Ext Version";
- ActivationStatus status = ActivationStatus::Active;
+ Status status = Status::Ready;
AssociationList associations;
};
@@ -51,3 +71,102 @@
psuInventoryPath, versionId);
EXPECT_EQ(toCompare, service);
}
+
+TEST_F(TestActivation, doUpdateWhenNoPSU)
+{
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(
+ Return(std::vector<std::string>({}))); // No PSU inventory
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Failed, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateOnePSUOK)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(
+ Return(std::vector<std::string>({psu0}))); // One PSU inventory
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Activating, activation->activation());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Active, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateFourPSUsOK)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto psu2 = "/com/example/inventory/psu2";
+ constexpr auto psu3 = "/com/example/inventory/psu3";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(
+ std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(10, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(30, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(50, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(70, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Active, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateFourPSUsFailonSecond)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto psu2 = "/com/example/inventory/psu2";
+ constexpr auto psu3 = "/com/example/inventory/psu3";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(
+ std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(10, getProgress());
+
+ onUpdateDone();
+ EXPECT_EQ(Status::Activating, activation->activation());
+ EXPECT_EQ(30, getProgress());
+
+ onUpdateFailed();
+ EXPECT_EQ(Status::Failed, activation->activation());
+}
+
+TEST_F(TestActivation, doUpdateOnExceptionFromDbus)
+{
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ activation = std::make_unique<Activation>(mockedBus, dBusPath, versionId,
+ extVersion, status, associations);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(
+ Return(std::vector<std::string>({psu0}))); // One PSU inventory
+ ON_CALL(sdbusMock, sd_bus_call(_, _, _, _, nullptr))
+ .WillByDefault(Return(-1)); // Make sdbus call failure
+ activation->requestedActivation(RequestedStatus::Active);
+
+ EXPECT_EQ(Status::Failed, activation->activation());
+}
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
index 9f05ed2..b020c28 100644
--- a/test/test_item_updater.cpp
+++ b/test/test_item_updater.cpp
@@ -25,6 +25,8 @@
reinterpret_cast<const utils::MockedUtils&>(utils::getUtils()))
{
ON_CALL(mockedUtils, getVersionId(_)).WillByDefault(ReturnArg<0>());
+ ON_CALL(mockedUtils, getPropertyImpl(_, _, _, _, StrEq(PRESENT)))
+ .WillByDefault(Return(any(PropertyType(true))));
}
~TestItemUpdater()