Check and sync PSU image when PSU is plugged in
When a PSU is plugged out and in, the service shall check the version
and do update if it is with an old software.
When the PSU is plugged in, the model is not available for a while, so
the service subscribe the model property as well, and only create the
software object when both present and model properties are set.
Tested: With dummy update service, verify on Witherspoon that when PSU
is plugged out (by setting "Present" property in inventory), the
software object is removed, and when it's pluggd int (by setting
"Present" property), the software object is created after the
model is got, and is upgraded by a newer image stored in BMC
filesystem.
Signed-off-by: Lei YU <mine260309@gmail.com>
Change-Id: Ia7516e5bc9c642158b216036bcddf404157f9204
diff --git a/src/activation.cpp b/src/activation.cpp
index ca8c7df..af56f28 100644
--- a/src/activation.cpp
+++ b/src/activation.cpp
@@ -225,6 +225,9 @@
associationInterface->createActiveAssociation(objPath);
associationInterface->addFunctionalAssociation(objPath);
+ // Reset RequestedActivations to none so that it could be activated in
+ // future
+ requestedActivation(SoftwareActivation::RequestedActivations::None);
activation(Status::Active);
}
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index 6d4e4c2..90ff62d 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -32,7 +32,6 @@
void ItemUpdater::createActivation(sdbusplus::message::message& m)
{
namespace msg = sdbusplus::message;
- namespace variant_ns = msg::variant_ns;
sdbusplus::message::object_path objPath;
std::map<std::string, std::map<std::string, msg::variant<std::string>>>
@@ -54,7 +53,7 @@
{
// Only process the PSU images
auto value = SVersion::convertVersionPurposeFromString(
- variant_ns::get<std::string>(propertyValue));
+ std::get<std::string>(propertyValue));
if (value == VersionPurpose::PSU)
{
@@ -63,7 +62,7 @@
}
else if (propertyName == VERSION)
{
- version = variant_ns::get<std::string>(propertyValue);
+ version = std::get<std::string>(propertyValue);
}
}
}
@@ -72,7 +71,7 @@
const auto& it = propertyMap.find("Path");
if (it != propertyMap.end())
{
- filePath = variant_ns::get<std::string>(it->second);
+ filePath = std::get<std::string>(it->second);
}
}
}
@@ -131,7 +130,8 @@
}
else
{
- versions.erase(versionId);
+ versionStrings.erase(it->second->getVersionString());
+ versions.erase(it);
}
// Removing entry in activations map
@@ -192,6 +192,10 @@
break;
}
}
+
+ auto it = activations.find(versionId);
+ assert(it != activations.end());
+ psuPathActivationMap.emplace(psuInventoryPath, it->second);
}
std::unique_ptr<Activation> ItemUpdater::createActivationObject(
@@ -247,6 +251,8 @@
void ItemUpdater::removePsuObject(const std::string& psuInventoryPath)
{
+ psuStatusMap[psuInventoryPath] = {false, ""};
+
auto it = psuPathActivationMap.find(psuInventoryPath);
if (it == psuPathActivationMap.end())
{
@@ -308,27 +314,63 @@
void ItemUpdater::onPsuInventoryChanged(const std::string& psuPath,
const Properties& properties)
{
- bool present;
- std::string version;
+ std::optional<bool> present;
+ std::optional<std::string> model;
- // Only present property is interested
+ // The code was expecting to get callback on multiple properties changed.
+ // But in practice, the callback is received one-by-one for each property.
+ // So it has to handle Present and Version property separately.
auto p = properties.find(PRESENT);
- if (p == properties.end())
+ if (p != properties.end())
+ {
+ present = std::get<bool>(p->second);
+ psuStatusMap[psuPath].present = *present;
+ }
+ p = properties.find(MODEL);
+ if (p != properties.end())
+ {
+ model = std::get<std::string>(p->second);
+ psuStatusMap[psuPath].model = *model;
+ }
+
+ // If present or model is not changed, ignore
+ if (!present.has_value() && !model.has_value())
{
return;
}
- present = sdbusplus::message::variant_ns::get<bool>(p->second);
- if (present)
+ if (psuStatusMap[psuPath].present)
{
- version = utils::getVersion(psuPath);
+ // If model is not updated, let's wait for it
+ if (psuStatusMap[psuPath].model.empty())
+ {
+ log<level::DEBUG>("Waiting for model to be updated");
+ return;
+ }
+
+ auto version = utils::getVersion(psuPath);
if (!version.empty())
{
createPsuObject(psuPath, version);
+ // Check if there is new PSU images to update
+ syncToLatestImage();
+ }
+ else
+ {
+ // TODO: log an event
+ log<level::ERR>("Failed to get PSU version",
+ entry("PSU=%s", psuPath.c_str()));
}
}
else
{
+ if (!present.has_value())
+ {
+ // If a PSU is plugged out, model property is update to empty as
+ // well, and we get callback here, but ignore that because it is
+ // handled by "Present" callback.
+ return;
+ }
// Remove object or association
removePsuObject(psuPath);
}
@@ -351,7 +393,11 @@
psuMatches.emplace_back(
bus, MatchRules::propertiesChanged(p, ITEM_IFACE),
std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
- std::placeholders::_1));
+ std::placeholders::_1)); // For present
+ psuMatches.emplace_back(
+ bus, MatchRules::propertiesChanged(p, ASSET_IFACE),
+ std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
+ std::placeholders::_1)); // For model
}
}
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index c3da8f9..0477e60 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -197,6 +197,19 @@
/** @brief A collection of the version strings */
std::set<std::string> versionStrings;
+
+ /** @brief A struct to hold the PSU present status and model */
+ struct psuStatus
+ {
+ bool present;
+ std::string model;
+ };
+
+ /** @brief The map of PSU inventory path and the psuStatus
+ *
+ * It is used to handle psu inventory changed event, that only create psu
+ * software object when a PSU is present and the model is retrieved */
+ std::map<std::string, psuStatus> psuStatusMap;
};
} // namespace updater
diff --git a/src/version.hpp b/src/version.hpp
index dbc48d7..e26bb31 100644
--- a/src/version.hpp
+++ b/src/version.hpp
@@ -149,6 +149,12 @@
/** @brief The temUpdater's erase callback. */
eraseFunc eraseCallback;
+ /** @brief Get the version string. */
+ const std::string& getVersionString() const
+ {
+ return versionStr;
+ }
+
private:
/** @brief Persistent sdbusplus DBus bus connection */
sdbusplus::bus::bus& bus;
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
index cbea53c..b2cb62a 100644
--- a/test/test_item_updater.cpp
+++ b/test/test_item_updater.cpp
@@ -8,6 +8,7 @@
using namespace phosphor::software::updater;
using ::testing::_;
+using ::testing::ContainerEq;
using ::testing::Pointee;
using ::testing::Return;
using ::testing::ReturnArg;
@@ -35,7 +36,7 @@
utils::freeUtils();
}
- const auto& GetActivations()
+ auto& GetActivations()
{
return itemUpdater->activations;
}
@@ -61,6 +62,9 @@
sdbusplus::bus::bus mockedBus = sdbusplus::get_mocked_new(&sdbusMock);
const utils::MockedUtils& mockedUtils;
std::unique_ptr<ItemUpdater> itemUpdater;
+ Properties propAdded{{PRESENT, PropertyType(true)}};
+ Properties propRemoved{{PRESENT, PropertyType(false)}};
+ Properties propModel{{MODEL, PropertyType(std::string("dummyModel"))}};
};
TEST_F(TestItemUpdater, ctordtor)
@@ -281,22 +285,24 @@
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
// The PSU is present and version is added in a single call
- Properties propAdded{{PRESENT, PropertyType(true)}};
+ Properties propAddedAndModel{
+ {PRESENT, PropertyType(true)},
+ {MODEL, PropertyType(std::string("testModel"))}};
EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
.WillOnce(Return(std::string(version)));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
.Times(2);
- onPsuInventoryChanged(psuPath, propAdded);
+ onPsuInventoryChanged(psuPath, propAddedAndModel);
}
-TEST_F(TestItemUpdater, OnOnePSURemovedAndAdded)
+TEST_F(TestItemUpdater, OnOnePSURemovedAndAddedWithLatestVersion)
{
constexpr auto psuPath = "/com/example/inventory/psu0";
constexpr auto service = "com.example.Software.Psu";
constexpr auto version = "version0";
std::string objPath = getObjPath(version);
- EXPECT_CALL(mockedUtils, getPSUInventoryPath(_))
- .WillOnce(Return(std::vector<std::string>({psuPath})));
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(std::vector<std::string>({psuPath})));
EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
.WillOnce(Return(service));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
@@ -315,17 +321,26 @@
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
// the activation and version object will be removed
- Properties propRemoved{{PRESENT, PropertyType(false)}};
EXPECT_CALL(sdbusMock, sd_bus_emit_object_removed(_, StrEq(objPath)))
.Times(2);
onPsuInventoryChanged(psuPath, propRemoved);
- Properties propAdded{{PRESENT, PropertyType(true)}};
EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
.WillOnce(Return(std::string(version)));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
.Times(2);
+
+ // On PSU inserted, it shall check if it's the latest version
+ std::set<std::string> expectedVersions = {version};
+ EXPECT_CALL(mockedUtils, getLatestVersion(ContainerEq(expectedVersions)))
+ .WillOnce(Return(version));
+ EXPECT_CALL(mockedUtils, isAssociated(StrEq(psuPath), _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(sdbusMock, sd_bus_message_new_method_call(_, _, _, _, _,
+ StrEq("StartUnit")))
+ .Times(0);
onPsuInventoryChanged(psuPath, propAdded);
+ onPsuInventoryChanged(psuPath, propModel);
// on exit, objects are removed
EXPECT_CALL(sdbusMock, sd_bus_emit_object_removed(_, StrEq(objPath)))
@@ -381,7 +396,6 @@
EXPECT_EQ(psu1, std::get<2>(assocs[1]));
// PSU0 is removed, only associations shall be updated
- Properties propRemoved{{PRESENT, PropertyType(false)}};
onPsuInventoryChanged(psu0, propRemoved);
assocs = activation->associations();
EXPECT_EQ(1u, assocs.size());
@@ -395,8 +409,6 @@
// Add PSU0 and PSU1 back, but PSU1 with a different version
version1 = "version1";
objPath1 = getObjPath(version1);
- Properties propAdded0{{PRESENT, PropertyType(true)}};
- Properties propAdded1{{PRESENT, PropertyType(true)}};
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu0)))
.WillOnce(Return(std::string(version0)));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
@@ -405,8 +417,10 @@
.Times(2);
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath1)))
.Times(2);
- onPsuInventoryChanged(psu0, propAdded0);
- onPsuInventoryChanged(psu1, propAdded1);
+ onPsuInventoryChanged(psu0, propAdded);
+ onPsuInventoryChanged(psu1, propModel);
+ onPsuInventoryChanged(psu1, propAdded);
+ onPsuInventoryChanged(psu0, propModel);
// on exit, objects are removed
EXPECT_CALL(sdbusMock, sd_bus_emit_object_removed(_, StrEq(objPath0)))
@@ -522,8 +536,15 @@
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+ std::string newVersionId = "NewVersionId";
+ AssociationList associations;
+ auto dummyActivation = std::make_unique<Activation>(
+ mockedBus, dBusPath, newVersionId, "", Activation::Status::Active,
+ associations, "", itemUpdater.get(), itemUpdater.get());
+
// Now there is one activation and it has two associations
auto& activations = GetActivations();
+ activations.emplace(newVersionId, std::move(dummyActivation));
auto& activation = activations.find(version0)->second;
auto assocs = activation->associations();
EXPECT_EQ(2u, assocs.size());
@@ -532,7 +553,7 @@
EXPECT_CALL(mockedUtils, isAssociated(StrEq(psu0), _))
.WillOnce(Return(true));
- itemUpdater->onUpdateDone("A new version", psu0);
+ itemUpdater->onUpdateDone(newVersionId, psu0);
// Now the activation should have one assoiation
assocs = activation->associations();
@@ -541,10 +562,11 @@
EXPECT_CALL(mockedUtils, isAssociated(StrEq(psu1), _))
.WillOnce(Return(true));
- itemUpdater->onUpdateDone("A new version", psu1);
+ itemUpdater->onUpdateDone(newVersionId, psu1);
- // Now the activation shall be erased
- EXPECT_EQ(0u, activations.size());
+ // Now the activation shall be erased and only the dummy one is left
+ EXPECT_EQ(1u, activations.size());
+ EXPECT_NE(activations.find(newVersionId), activations.end());
}
TEST_F(TestItemUpdater, OnUpdateDoneOnTwoPSUsWithDifferentVersion)
@@ -578,15 +600,21 @@
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());
+ std::string newVersionId = "NewVersionId";
+ AssociationList associations;
+ auto dummyActivation = std::make_unique<Activation>(
+ mockedBus, dBusPath, newVersionId, "", Activation::Status::Active,
+ associations, "", itemUpdater.get(), itemUpdater.get());
+
+ auto& activations = GetActivations();
+ activations.emplace(newVersionId, std::move(dummyActivation));
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());
+
+ // After psu0 is done, two activations should be left
+ itemUpdater->onUpdateDone(newVersionId, psu0);
+ EXPECT_EQ(2u, activations.size());
const auto& activation1 = activations.find(version1)->second;
const auto& assocs1 = activation1->associations();
EXPECT_EQ(1u, assocs1.size());
@@ -594,7 +622,62 @@
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());
+ // After psu1 is done, only the dummy activation should be left
+ itemUpdater->onUpdateDone(newVersionId, psu1);
+ EXPECT_EQ(1u, activations.size());
+ EXPECT_NE(activations.find(newVersionId), activations.end());
+}
+
+TEST_F(TestItemUpdater, OnOnePSURemovedAndAddedWithOldVersion)
+{
+ constexpr auto psuPath = "/com/example/inventory/psu0";
+ constexpr auto service = "com.example.Software.Psu";
+ constexpr auto version = "version0";
+ std::string versionId =
+ version; // In testing versionId is the same as version
+ std::string objPath = getObjPath(version);
+ ON_CALL(mockedUtils, getPSUInventoryPath(_))
+ .WillByDefault(Return(std::vector<std::string>({psuPath})));
+ ON_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
+ .WillByDefault(Return(service));
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
+ .WillOnce(Return(std::string(version)));
+ ON_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath), _,
+ StrEq(PRESENT)))
+ .WillByDefault(Return(any(PropertyType(true)))); // present
+
+ itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
+
+ // Add an association to simulate that it has image in BMC filesystem
+ auto& activation = GetActivations().find(versionId)->second;
+ auto assocs = activation->associations();
+ assocs.emplace_back(ACTIVATION_FWD_ASSOCIATION, ACTIVATION_REV_ASSOCIATION,
+ "SomePath");
+ activation->associations(assocs);
+ activation->path("SomeFilePath");
+
+ onPsuInventoryChanged(psuPath, propRemoved);
+
+ // On PSU inserted, it checks and finds a newer version
+ auto oldVersion = "old-version";
+ EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
+ .WillOnce(Return(std::string(oldVersion)));
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MANUFACTURER)))
+ .WillOnce(
+ Return(any(PropertyType(std::string(""))))); // Checking compatible
+ EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
+ _, StrEq(MODEL)))
+ .WillOnce(
+ Return(any(PropertyType(std::string(""))))); // Checking compatible
+ std::set<std::string> expectedVersions = {version, oldVersion};
+ EXPECT_CALL(mockedUtils, getLatestVersion(ContainerEq(expectedVersions)))
+ .WillOnce(Return(version));
+ ON_CALL(mockedUtils, isAssociated(StrEq(psuPath), _))
+ .WillByDefault(Return(false));
+ EXPECT_CALL(sdbusMock, sd_bus_message_new_method_call(_, _, _, _, _,
+ StrEq("StartUnit")))
+ .Times(1);
+ onPsuInventoryChanged(psuPath, propAdded);
+ onPsuInventoryChanged(psuPath, propModel);
}