Activation: remove old objects after update done
The update on PSUs is one-by-one, after each PSU is updated, notify
ItemUpdater by onUpdateDone() so that the old object for the PSU is
removed.
Tested: With dummy PSU image and update service, verify the old
objects are removed on Witherspoon.
Change-Id: I212b8cba9570ad96083d362bf57691fdabb4e42f
diff --git a/src/activation.cpp b/src/activation.cpp
index cab9acd..ca8c7df 100644
--- a/src/activation.cpp
+++ b/src/activation.cpp
@@ -127,9 +127,11 @@
auto assocs = associations();
assocs.emplace_back(ACTIVATION_FWD_ASSOCIATION, ACTIVATION_REV_ASSOCIATION,
currentUpdatingPsu);
- currentUpdatingPsu.clear();
associations(assocs);
+ activationListener->onUpdateDone(versionId, currentUpdatingPsu);
+ currentUpdatingPsu.clear();
+
psuQueue.pop();
doUpdate(); // Update the next psu
}
@@ -218,7 +220,6 @@
storeImage();
activationProgress->progress(100);
- // TODO: delete the old software object
deleteImageManagerObject();
associationInterface->createActiveAssociation(objPath);
diff --git a/src/activation.hpp b/src/activation.hpp
index 669686e..3b03803 100644
--- a/src/activation.hpp
+++ b/src/activation.hpp
@@ -2,6 +2,7 @@
#include "config.h"
+#include "activation_listener.hpp"
#include "association_interface.hpp"
#include "types.hpp"
#include "version.hpp"
@@ -129,8 +130,9 @@
Activation(sdbusplus::bus::bus& bus, const std::string& objPath,
const std::string& versionId, const std::string& extVersion,
Status activationStatus, const AssociationList& assocs,
+ const std::string& filePath,
AssociationInterface* associationInterface,
- const std::string& filePath) :
+ ActivationListener* activationListener) :
ActivationInherit(bus, objPath.c_str(), true),
bus(bus), objPath(objPath), versionId(versionId),
systemdSignals(
@@ -140,7 +142,8 @@
sdbusRule::interface("org.freedesktop.systemd1.Manager"),
std::bind(&Activation::unitStateChange, this,
std::placeholders::_1)),
- associationInterface(associationInterface)
+ associationInterface(associationInterface),
+ activationListener(activationListener)
{
// Set Properties.
extendedVersion(extVersion);
@@ -278,6 +281,9 @@
/** @brief The AssociationInterface pointer */
AssociationInterface* associationInterface;
+ /** @brief The activationListener pointer */
+ ActivationListener* activationListener;
+
/** @brief The PSU manufacturer of the software */
std::string manufacturer;
diff --git a/src/activation_listener.hpp b/src/activation_listener.hpp
new file mode 100644
index 0000000..5437232
--- /dev/null
+++ b/src/activation_listener.hpp
@@ -0,0 +1,17 @@
+#pragma once
+
+#include <string>
+
+class ActivationListener
+{
+ public:
+ virtual ~ActivationListener() = default;
+
+ /** @brief Notify a PSU is updated
+ *
+ * @param[in] versionId - The versionId of the activation
+ * @param[in] psuInventoryPath - The PSU inventory path that is updated
+ */
+ virtual void onUpdateDone(const std::string& versionId,
+ const std::string& psuInventoryPath) = 0;
+};
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index acbc943..6d4e4c2 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -179,14 +179,29 @@
}
}
+void ItemUpdater::onUpdateDone(const std::string& versionId,
+ const std::string& psuInventoryPath)
+{
+ // After update is done, remove old activation objects
+ for (auto it = activations.begin(); it != activations.end(); ++it)
+ {
+ if (it->second->getVersionId() != versionId &&
+ utils::isAssociated(psuInventoryPath, it->second->associations()))
+ {
+ removePsuObject(psuInventoryPath);
+ break;
+ }
+ }
+}
+
std::unique_ptr<Activation> ItemUpdater::createActivationObject(
const std::string& path, const std::string& versionId,
const std::string& extVersion, Activation::Status activationStatus,
const AssociationList& assocs, const std::string& filePath)
{
return std::make_unique<Activation>(bus, path, versionId, extVersion,
- activationStatus, assocs, this,
- filePath);
+ activationStatus, assocs, filePath,
+ this, this);
}
void ItemUpdater::createPsuObject(const std::string& psuInventoryPath,
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index b713454..c3da8f9 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -35,7 +35,9 @@
/** @class ItemUpdater
* @brief Manages the activation of the PSU version items.
*/
-class ItemUpdater : public ItemUpdaterInherit, public AssociationInterface
+class ItemUpdater : public ItemUpdaterInherit,
+ public AssociationInterface,
+ public ActivationListener
{
friend class ::TestItemUpdater;
@@ -84,6 +86,14 @@
*/
void removeAssociation(const std::string& path) override;
+ /** @brief Notify a PSU is updated
+ *
+ * @param[in] versionId - The versionId of the activation
+ * @param[in] psuInventoryPath - The PSU inventory path that is updated
+ */
+ void onUpdateDone(const std::string& versionId,
+ const std::string& psuInventoryPath) override;
+
private:
/** @brief Callback function for Software.Version match.
* @details Creates an Activation D-Bus object.
diff --git a/test/mocked_activation_listener.hpp b/test/mocked_activation_listener.hpp
new file mode 100644
index 0000000..cbef46d
--- /dev/null
+++ b/test/mocked_activation_listener.hpp
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "activation_listener.hpp"
+
+#include <gmock/gmock.h>
+
+class MockedActivationListener : public ActivationListener
+{
+ public:
+ virtual ~MockedActivationListener() = default;
+
+ MOCK_METHOD2(onUpdateDone, void(const std::string& versionId,
+ const std::string& psuInventoryPath));
+};
diff --git a/test/test_activation.cpp b/test/test_activation.cpp
index 97efe79..a7ce542 100644
--- a/test/test_activation.cpp
+++ b/test/test_activation.cpp
@@ -1,4 +1,5 @@
#include "activation.hpp"
+#include "mocked_activation_listener.hpp"
#include "mocked_association_interface.hpp"
#include "mocked_utils.hpp"
@@ -62,6 +63,7 @@
sdbusplus::bus::bus mockedBus = sdbusplus::get_mocked_new(&sdbusMock);
const utils::MockedUtils& mockedUtils;
MockedAssociationInterface mockedAssociationInterface;
+ MockedActivationListener mockedActivationListener;
std::unique_ptr<Activation> activation;
std::string versionId = "abcdefgh";
std::string extVersion = "manufacturer=TestManu,model=TestModel";
@@ -75,7 +77,7 @@
{
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
}
TEST_F(TestActivation, ctorWithInvalidExtVersion)
@@ -83,7 +85,7 @@
extVersion = "invalid text";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
}
TEST_F(TestActivation, getUpdateService)
@@ -96,7 +98,7 @@
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
auto service = getUpdateService(psuInventoryPath);
EXPECT_EQ(toCompare, service);
@@ -106,7 +108,7 @@
{
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({}))); // No PSU inventory
@@ -116,6 +118,7 @@
.Times(0);
EXPECT_CALL(mockedAssociationInterface, addFunctionalAssociation(dBusPath))
.Times(0);
+ EXPECT_CALL(mockedActivationListener, onUpdateDone(_, _)).Times(0);
EXPECT_EQ(Status::Failed, activation->activation());
}
@@ -124,7 +127,7 @@
constexpr auto psu0 = "/com/example/inventory/psu0";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({psu0}))); // One PSU inventory
@@ -136,6 +139,9 @@
.Times(1);
EXPECT_CALL(mockedAssociationInterface, addFunctionalAssociation(dBusPath))
.Times(1);
+ EXPECT_CALL(mockedActivationListener,
+ onUpdateDone(StrEq(versionId), StrEq(psu0)))
+ .Times(1);
onUpdateDone();
EXPECT_EQ(Status::Active, activation->activation());
}
@@ -148,7 +154,7 @@
constexpr auto psu3 = "/com/example/inventory/psu3";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(
std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
@@ -157,14 +163,23 @@
EXPECT_EQ(Status::Activating, activation->activation());
EXPECT_EQ(10, getProgress());
+ EXPECT_CALL(mockedActivationListener,
+ onUpdateDone(StrEq(versionId), StrEq(psu0)))
+ .Times(1);
onUpdateDone();
EXPECT_EQ(Status::Activating, activation->activation());
EXPECT_EQ(30, getProgress());
+ EXPECT_CALL(mockedActivationListener,
+ onUpdateDone(StrEq(versionId), StrEq(psu1)))
+ .Times(1);
onUpdateDone();
EXPECT_EQ(Status::Activating, activation->activation());
EXPECT_EQ(50, getProgress());
+ EXPECT_CALL(mockedActivationListener,
+ onUpdateDone(StrEq(versionId), StrEq(psu2)))
+ .Times(1);
onUpdateDone();
EXPECT_EQ(Status::Activating, activation->activation());
EXPECT_EQ(70, getProgress());
@@ -174,6 +189,9 @@
EXPECT_CALL(mockedAssociationInterface, addFunctionalAssociation(dBusPath))
.Times(1);
+ EXPECT_CALL(mockedActivationListener,
+ onUpdateDone(StrEq(versionId), StrEq(psu3)))
+ .Times(1);
onUpdateDone();
EXPECT_EQ(Status::Active, activation->activation());
}
@@ -186,7 +204,7 @@
constexpr auto psu3 = "/com/example/inventory/psu3";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(
std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
@@ -195,6 +213,9 @@
EXPECT_EQ(Status::Activating, activation->activation());
EXPECT_EQ(10, getProgress());
+ EXPECT_CALL(mockedActivationListener,
+ onUpdateDone(StrEq(versionId), StrEq(psu0)))
+ .Times(1);
onUpdateDone();
EXPECT_EQ(Status::Activating, activation->activation());
EXPECT_EQ(30, getProgress());
@@ -203,6 +224,7 @@
.Times(0);
EXPECT_CALL(mockedAssociationInterface, addFunctionalAssociation(dBusPath))
.Times(0);
+ EXPECT_CALL(mockedActivationListener, onUpdateDone(_, _)).Times(0);
onUpdateFailed();
EXPECT_EQ(Status::Failed, activation->activation());
}
@@ -212,7 +234,7 @@
constexpr auto psu0 = "/com/example/inventory/psu0";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({psu0}))); // One PSU inventory
@@ -229,7 +251,7 @@
extVersion = "manufacturer=TestManu,model=DifferentModel";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(std::vector<std::string>({psu0})));
activation->requestedActivation(RequestedStatus::Active);
@@ -243,7 +265,7 @@
extVersion = "manufacturer=DifferentManu,model=TestModel";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(std::vector<std::string>({psu0})));
activation->requestedActivation(RequestedStatus::Active);
@@ -260,7 +282,7 @@
constexpr auto psu0 = "/com/example/inventory/psu0";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
Return(std::vector<std::string>({psu0}))); // One PSU inventory
@@ -287,7 +309,7 @@
Return(any(PropertyType(std::string("DifferentModel")))));
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(Return(
std::vector<std::string>({psu0, psu1, psu2, psu3}))); // 4 PSUs
@@ -325,7 +347,7 @@
constexpr auto psu0 = "/com/example/inventory/psu0";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
@@ -348,7 +370,7 @@
constexpr auto psu0 = "/com/example/inventory/psu0";
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
@@ -369,7 +391,7 @@
status = Status::Active; // Typically, a running PSU software is associated
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
- &mockedAssociationInterface, filePath);
+ filePath, &mockedAssociationInterface, &mockedActivationListener);
ON_CALL(mockedUtils, getPSUInventoryPath(_))
.WillByDefault(
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
index b4ecafc..cbea53c 100644
--- a/test/test_item_updater.cpp
+++ b/test/test_item_updater.cpp
@@ -490,3 +490,111 @@
.Times(1);
scanDirectory("./psu-images-valid-version0");
}
+
+TEST_F(TestItemUpdater, OnUpdateDoneOnTwoPSUsWithSameVersion)
+{
+ // Simulate there are two PSUs with same version, and updated to a new
+ // version
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto service = "com.example.Software.Psu";
+ auto version0 = std::string("version0");
+ auto version1 = std::string("version0");
+ auto objPath0 = getObjPath(version0);
+ auto objPath1 = getObjPath(version1);
+
+ EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillOnce(Return(std::vector<std::string>({psu0, psu1})));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu0), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu1), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psu0)))
+ .WillOnce(Return(std::string(version0)));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
+ .WillOnce(Return(std::string(version1)));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+
+ // Now there is one activation and it has two associations
+ auto& activations = GetActivations();
+ auto& activation = activations.find(version0)->second;
+ auto assocs = activation->associations();
+ EXPECT_EQ(2u, assocs.size());
+ EXPECT_EQ(psu0, std::get<2>(assocs[0]));
+ EXPECT_EQ(psu1, std::get<2>(assocs[1]));
+
+ EXPECT_CALL(mockedUtils, isAssociated(StrEq(psu0), _))
+ .WillOnce(Return(true));
+ itemUpdater->onUpdateDone("A new version", psu0);
+
+ // Now the activation should have one assoiation
+ assocs = activation->associations();
+ EXPECT_EQ(1u, assocs.size());
+ EXPECT_EQ(psu1, std::get<2>(assocs[0]));
+
+ EXPECT_CALL(mockedUtils, isAssociated(StrEq(psu1), _))
+ .WillOnce(Return(true));
+ itemUpdater->onUpdateDone("A new version", psu1);
+
+ // Now the activation shall be erased
+ EXPECT_EQ(0u, activations.size());
+}
+
+TEST_F(TestItemUpdater, OnUpdateDoneOnTwoPSUsWithDifferentVersion)
+{
+ // Simulate there are two PSUs with different version, and updated to a new
+ // version
+ constexpr auto psu0 = "/com/example/inventory/psu0";
+ constexpr auto psu1 = "/com/example/inventory/psu1";
+ constexpr auto service = "com.example.Software.Psu";
+ auto version0 = std::string("version0");
+ auto version1 = std::string("version1");
+ auto objPath0 = getObjPath(version0);
+ auto objPath1 = getObjPath(version1);
+
+ EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillOnce(Return(std::vector<std::string>({psu0, psu1})));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu0), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getService(_, StrEq(psu1), _))
+ .WillOnce(Return(service));
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psu0)))
+ .WillOnce(Return(std::string(version0)));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
+ .WillOnce(Return(std::string(version1)));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
+ StrEq(PRESENT)))
+ .WillOnce(Return(any(PropertyType(true)))); // present
+
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+
+ // There are two activations and each with one association
+ const auto& activations = GetActivations();
+ EXPECT_EQ(2u, activations.size());
+
+ EXPECT_CALL(mockedUtils, isAssociated(StrEq(psu0), _))
+ .WillOnce(Return(true));
+ // After psu0 is done, only one activation should be left
+ itemUpdater->onUpdateDone("A new version", psu0);
+ EXPECT_EQ(1u, activations.size());
+ const auto& activation1 = activations.find(version1)->second;
+ const auto& assocs1 = activation1->associations();
+ EXPECT_EQ(1u, assocs1.size());
+ EXPECT_EQ(psu1, std::get<2>(assocs1[0]));
+
+ EXPECT_CALL(mockedUtils, isAssociated(StrEq(psu1), _))
+ .WillOnce(Return(true));
+ // After psu1 is done, no activation should be left
+ itemUpdater->onUpdateDone("A new version", psu1);
+ EXPECT_EQ(0u, activations.size());
+}