Get model using command line tool
Get the PSU model using a command line tool specified in the meson
options. The default tool is 'psutils --get-model'. The tool should
get the model data directly from the PSU.
This is more accurate than getting the Model property of the Asset
interface on D-Bus. Inventory Manager saves its state to files. When
the BMC is booted, Inventory Manager initializes itself using the saved
state. This is necessary to handle the scenario where the BMC is
rebooted while the rest of the system is powered on (such as a
concurrent BMC code update).
However, if all power was removed from the system, a PSU may have been
added/removed/replaced while the BMC was offline. When the BMC boots,
the Inventory Manager saved state is not correct.
Eventually the PSU monitoring application will update the model on
D-Bus, but this can take a non-trivial amount of time. This is
especially true if EntityManager is used to provide the PSU bus and
address information to the PSU monitoring application.
Tested:
* Verified all automated tests build and run successfully
* Verified application uses command line tool to obtain model
* Verified command line tool was returning the correct model
* Test where command line tool fails with non-zero exit code
* Tested where all PSU information available when application starts
* Tested where PSU information is obtained after application starts
using the InterfacesAdded handler
* Tested where PSU presence changes and is obtained by the
PropertiesChanged handler
* Full test plan is available at
https://gist.github.com/smccarney/87bd821a6d317ec0915d1f162028ff01
Change-Id: Ia9d35850aa6ac27dd006679991272232d67390ff
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
diff --git a/README.md b/README.md
index feac5bf..7ae15cc 100644
--- a/README.md
+++ b/README.md
@@ -31,6 +31,7 @@
hardware:
- Get PSU firmware version
+- Get PSU model
- Compare the firmware version
- Update the PSU firmware
@@ -38,6 +39,8 @@
- `PSU_VERSION_UTIL`: It shall be defined as a command-line tool that accepts
the PSU inventory path as input, and outputs the PSU version string to stdout.
+- `PSU_MODEL_UTIL`: It shall be defined as a command-line tool that accepts the
+ PSU inventory path as input, and outputs the PSU model string to stdout.
- `PSU_VERSION_COMPARE_UTIL`: It shall be defined as a command-line tool that
accepts one or more PSU version strings, and outputs the latest version string
to stdout.
@@ -51,15 +54,16 @@
```text
meson -Dtests=disabled \
'-DPSU_VERSION_UTIL=/usr/bin/psutils --raw --get-version' \
+ '-DPSU_MODEL_UTIL=/usr/bin/psutils --raw --get-model' \
'-DPSU_VERSION_COMPARE_UTIL=/usr/bin/psutils --raw --compare' \
'-DPSU_UPDATE_SERVICE=psu-update@.service' \
build
```
The above configures the vendor-specific tools to use `psutils` from
-[phosphor-power][3] to get and compare the PSU versions, and use
-`psu-update@.service` to perform the PSU firmware update, where internally it
-invokes `psutils` as well.
+[phosphor-power][3] to get the PSU version and model, compare PSU versions, and
+use `psu-update@.service` to perform the PSU firmware update, where internally
+it invokes `psutils` as well.
## Usage
@@ -151,11 +155,11 @@
persistent storage defined by `IMG_DIR_PERSIST`. When a PSU is replaced, the
PSU's firmware version will be checked and updated if it's older than the one
stored in BMC.
-4. It is possible to put a PSU image and MANIFEST in the built-bin OpenBMC image
+4. It is possible to put a PSU image and MANIFEST in the built-in OpenBMC image
in BMC's read-only filesystem defined by `IMG_DIR_BUILTIN`. When the service
- starts, it will compare the versions of the built-in image, the stored image
- (after PSU update), and the existing PSUs, if there is any PSU that has an
- older firmware, it will be updated to the newest one.
+ starts, it will compare the versions of the built-in image and the existing
+ PSUs. If there is any PSU that has older firmware, it will be updated to the
+ new firmware.
[1]: https://github.com/openbmc/docs/blob/master/testing/local-ci-build.md
[2]:
diff --git a/meson.build b/meson.build
index b575366..12042d7 100644
--- a/meson.build
+++ b/meson.build
@@ -37,6 +37,7 @@
cdata.set_quoted('MANIFEST_FILE', get_option('MANIFEST_FILE'))
cdata.set_quoted('PSU_INVENTORY_PATH_BASE', get_option('PSU_INVENTORY_PATH_BASE'))
cdata.set_quoted('PSU_VERSION_UTIL', get_option('PSU_VERSION_UTIL'))
+cdata.set_quoted('PSU_MODEL_UTIL', get_option('PSU_MODEL_UTIL'))
cdata.set_quoted('PSU_VERSION_COMPARE_UTIL', get_option('PSU_VERSION_COMPARE_UTIL'))
cdata.set_quoted('PSU_UPDATE_SERVICE', get_option('PSU_UPDATE_SERVICE'))
cdata.set_quoted('IMG_DIR', get_option('IMG_DIR'))
diff --git a/meson.options b/meson.options
index b5877c9..68b3cfb 100644
--- a/meson.options
+++ b/meson.options
@@ -24,7 +24,7 @@
# The PSU_VERSION_UTIL specifies an executable that accepts the PSU
# inventory path as input, and output the version string, e.g
-# psutils get-version /xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0
+# psutils --get-version /xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0
# or in vendor-example
# get_version <some-psu-path>
option('PSU_VERSION_UTIL',
@@ -32,6 +32,15 @@
value: '/usr/bin/psutils --raw --get-version',
description: 'The command and arguments to get PSU version')
+# The PSU_MODEL_UTIL specifies an executable that accepts the PSU
+# inventory path as input and outputs the PSU model string.
+# For example:
+# psutils --get-model /xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0
+option('PSU_MODEL_UTIL',
+ type: 'string',
+ value: '/usr/bin/psutils --raw --get-model',
+ description: 'The command and arguments to get the PSU model')
+
# The PSU_VERSION_COMPARE_UTIL specifies an executable that accepts the PSU
# versions as input, and outputs which version is the newest, e.g.
# psutils get-version 0001 0002 0003 # May output 0003
diff --git a/src/activation.cpp b/src/activation.cpp
index bfc3b39..5c90860 100644
--- a/src/activation.cpp
+++ b/src/activation.cpp
@@ -280,8 +280,7 @@
auto psuManufacturer = utils::getProperty<std::string>(
bus, service.c_str(), psuInventoryPath.c_str(), ASSET_IFACE,
MANUFACTURER);
- auto psuModel = utils::getProperty<std::string>(
- bus, service.c_str(), psuInventoryPath.c_str(), ASSET_IFACE, MODEL);
+ auto psuModel = utils::getModel(psuInventoryPath);
if (psuModel != model)
{
// The model shall match
diff --git a/src/item_updater.cpp b/src/item_updater.cpp
index a545bf3..44612b3 100644
--- a/src/item_updater.cpp
+++ b/src/item_updater.cpp
@@ -10,12 +10,13 @@
#include <cassert>
#include <filesystem>
+#include <format>
+#include <set>
namespace
{
constexpr auto MANIFEST_VERSION = "version";
constexpr auto MANIFEST_EXTENDED_VERSION = "extended_version";
-constexpr auto TIMEOUT = 10;
} // namespace
namespace phosphor
@@ -306,15 +307,38 @@
{
psuStatusMap[psuPath] = {false, ""};
- // Add matches for PSU Inventory's property changes
+ // Add PropertiesChanged listener for Item interface so we are notified
+ // when Present property changes
psuMatches.emplace_back(
bus, MatchRules::propertiesChanged(psuPath, ITEM_IFACE),
std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
- std::placeholders::_1)); // For present
- psuMatches.emplace_back(
- bus, MatchRules::propertiesChanged(psuPath, ASSET_IFACE),
- std::bind(&ItemUpdater::onPsuInventoryChangedMsg, this,
- std::placeholders::_1)); // For model
+ std::placeholders::_1));
+ }
+}
+
+void ItemUpdater::handlePSUPresenceChanged(const std::string& psuPath)
+{
+ if (psuStatusMap.contains(psuPath))
+ {
+ if (psuStatusMap[psuPath].present)
+ {
+ // PSU is now present
+ psuStatusMap[psuPath].model = utils::getModel(psuPath);
+ auto version = utils::getVersion(psuPath);
+ if (!version.empty() && !psuPathActivationMap.contains(psuPath))
+ {
+ createPsuObject(psuPath, version);
+ }
+ }
+ else
+ {
+ // PSU is now missing
+ psuStatusMap[psuPath].model.clear();
+ if (psuPathActivationMap.contains(psuPath))
+ {
+ removePsuObject(psuPath);
+ }
+ }
}
}
@@ -345,94 +369,56 @@
void ItemUpdater::onPsuInventoryChanged(const std::string& psuPath,
const Properties& properties)
{
- std::optional<bool> present;
- std::optional<std::string> model;
-
- // 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())
+ try
{
- 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;
- }
-
- if (psuStatusMap[psuPath].present)
- {
- // If model is not updated, let's wait for it
- if (psuStatusMap[psuPath].model.empty())
+ if (psuStatusMap.contains(psuPath) && properties.contains(PRESENT))
{
- log<level::DEBUG>("Waiting for model to be updated");
- return;
- }
-
- auto version = utils::getVersion(psuPath);
- if (!version.empty())
- {
- if (psuPathActivationMap.find(psuPath) ==
- psuPathActivationMap.end())
+ psuStatusMap[psuPath].present =
+ std::get<bool>(properties.at(PRESENT));
+ handlePSUPresenceChanged(psuPath);
+ if (psuStatusMap[psuPath].present)
{
- createPsuObject(psuPath, version);
+ // Check if there are new PSU images to update
+ processStoredImage();
+ syncToLatestImage();
}
}
- else
- {
- // TODO: log an event
- log<level::ERR>("Failed to get PSU version",
- entry("PSU=%s", psuPath.c_str()));
- }
- // Check if there are new PSU images to update
- processStoredImage();
- syncToLatestImage();
}
- else
+ catch (const std::exception& e)
{
- 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;
- }
- psuStatusMap[psuPath].model = "";
-
- // Remove object or association
- removePsuObject(psuPath);
+ log<level::ERR>(
+ std::format(
+ "Unable to handle inventory PropertiesChanged event: {}",
+ e.what())
+ .c_str());
}
}
void ItemUpdater::processPSUImage()
{
- auto paths = utils::getPSUInventoryPath(bus);
- for (const auto& p : paths)
+ try
{
- addPsuToStatusMap(p);
-
- auto service = utils::getService(bus, p.c_str(), ITEM_IFACE);
- psuStatusMap[p].present = utils::getProperty<bool>(
- bus, service.c_str(), p.c_str(), ITEM_IFACE, PRESENT);
- psuStatusMap[p].model = utils::getProperty<std::string>(
- bus, service.c_str(), p.c_str(), ASSET_IFACE, MODEL);
- auto version = utils::getVersion(p);
- if ((psuPathActivationMap.find(p) == psuPathActivationMap.end()) &&
- psuStatusMap[p].present && !version.empty())
+ auto paths = utils::getPSUInventoryPath(bus);
+ for (const auto& p : paths)
{
- createPsuObject(p, version);
+ try
+ {
+ addPsuToStatusMap(p);
+ auto service = utils::getService(bus, p.c_str(), ITEM_IFACE);
+ psuStatusMap[p].present = utils::getProperty<bool>(
+ bus, service.c_str(), p.c_str(), ITEM_IFACE, PRESENT);
+ handlePSUPresenceChanged(p);
+ }
+ catch (const std::exception& e)
+ {
+ // Ignore errors; the information might not be available yet
+ }
}
}
+ catch (const std::exception& e)
+ {
+ // Ignore errors; the information might not be available yet
+ }
}
void ItemUpdater::processStoredImage()
@@ -583,15 +569,19 @@
auto paths = utils::getPSUInventoryPath(bus);
for (const auto& p : paths)
{
- // As long as there is a PSU is not associated with the latest
+ // If there is a present PSU that is not associated with the latest
// image, run the activation so that all PSUs are running the same
// latest image.
- if (!utils::isAssociated(p, assocs))
+ if (psuStatusMap.contains(p) && psuStatusMap[p].present)
{
- log<level::INFO>("Automatically update PSU",
- entry("VERSION_ID=%s", latestVersionId->c_str()));
- invokeActivation(activation);
- break;
+ if (!utils::isAssociated(p, assocs))
+ {
+ log<level::INFO>(
+ "Automatically update PSU",
+ entry("VERSION_ID=%s", latestVersionId->c_str()));
+ invokeActivation(activation);
+ break;
+ }
}
}
}
@@ -604,74 +594,48 @@
void ItemUpdater::onPSUInterfaceAdded(sdbusplus::message_t& msg)
{
- sdbusplus::message::object_path objPath;
- std::map<std::string,
- std::map<std::string, std::variant<bool, std::string>>>
- interfaces;
- msg.read(objPath, interfaces);
- std::string path = objPath.str;
+ // Maintain static set of valid PSU paths. This is needed if PSU interface
+ // comes in a separate InterfacesAdded message from Item interface.
+ static std::set<std::string> psuPaths{};
- if (interfaces.find(PSU_INVENTORY_IFACE) == interfaces.end())
+ try
{
- return;
- }
+ sdbusplus::message::object_path objPath;
+ std::map<std::string,
+ std::map<std::string, std::variant<bool, std::string>>>
+ interfaces;
+ msg.read(objPath, interfaces);
+ std::string path = objPath.str;
- addPsuToStatusMap(path);
-
- if (psuStatusMap[path].present && !psuStatusMap[path].model.empty())
- {
- return;
- }
-
- auto timeout = std::chrono::steady_clock::now() +
- std::chrono::seconds(TIMEOUT);
-
- // Poll the inventory item until it gets the present property
- // or the timeout is reached
- while (std::chrono::steady_clock::now() < timeout)
- {
- try
+ if (interfaces.contains(PSU_INVENTORY_IFACE))
{
- psuStatusMap[path].present = utils::getProperty<bool>(
- bus, msg.get_sender(), path.c_str(), ITEM_IFACE, PRESENT);
- break;
+ psuPaths.insert(path);
}
- catch (const std::exception& e)
+
+ if (interfaces.contains(ITEM_IFACE) && psuPaths.contains(path) &&
+ !psuStatusMap.contains(path))
{
- auto err = errno;
- log<level::INFO>(
- std::format("Failed to get Inventory Item Present. errno={}",
- err)
- .c_str());
- sleep(1);
+ auto interface = interfaces[ITEM_IFACE];
+ if (interface.contains(PRESENT))
+ {
+ addPsuToStatusMap(path);
+ psuStatusMap[path].present = std::get<bool>(interface[PRESENT]);
+ handlePSUPresenceChanged(path);
+ if (psuStatusMap[path].present)
+ {
+ // Check if there are new PSU images to update
+ processStoredImage();
+ syncToLatestImage();
+ }
+ }
}
}
-
- // Poll the inventory item until it retrieves model or the timeout is
- // reached. The model is the path trail of the firmware's and manifest
- // subdirectory. If the model not found the firmware and manifest
- // cannot be located.
- timeout = std::chrono::steady_clock::now() + std::chrono::seconds(TIMEOUT);
- while (std::chrono::steady_clock::now() < timeout &&
- psuStatusMap[path].present)
+ catch (const std::exception& e)
{
- try
- {
- psuStatusMap[path].model = utils::getProperty<std::string>(
- bus, msg.get_sender(), path.c_str(), ASSET_IFACE, MODEL);
- processPSUImageAndSyncToLatest();
- break;
- }
- catch (const std::exception& e)
- {
- auto err = errno;
- log<level::INFO>(
- std::format(
- "Failed to get Inventory Decorator Asset model. errno={}",
- err)
- .c_str());
- sleep(1);
- }
+ log<level::ERR>(
+ std::format("Unable to handle inventory InterfacesAdded event: {}",
+ e.what())
+ .c_str());
}
}
diff --git a/src/item_updater.hpp b/src/item_updater.hpp
index 885847a..e8a489c 100644
--- a/src/item_updater.hpp
+++ b/src/item_updater.hpp
@@ -164,14 +164,20 @@
void removePsuObject(const std::string& psuInventoryPath);
/** @brief Add PSU inventory path to the PSU status map
- * @details Also adds PropertiesChanged listeners for the inventory path so
- * we are notified when the present or model properties change.
+ * @details Also adds a PropertiesChanged listener for the inventory path
+ * so we are notified when the Present property changes.
* Does nothing if the inventory path already exists in the map.
*
* @param[in] psuPath - The PSU inventory path
*/
void addPsuToStatusMap(const std::string& psuPath);
+ /** @brief Handle a change in presence for a PSU.
+ *
+ * @param[in] psuPath - The PSU inventory path
+ */
+ void handlePSUPresenceChanged(const std::string& psuPath);
+
/**
* @brief Create and populate the active PSU Version.
*/
diff --git a/src/utils.cpp b/src/utils.cpp
index 3b611d7..316ef31 100644
--- a/src/utils.cpp
+++ b/src/utils.cpp
@@ -157,13 +157,22 @@
std::string Utils::getVersion(const std::string& inventoryPath) const
{
- // Invoke vendor-specify tool to get the version string, e.g.
- // psutils get-version
+ // Invoke vendor-specific tool to get the version string, e.g.
+ // psutils --get-version
// /xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0
auto [rc, r] = internal::exec(PSU_VERSION_UTIL, inventoryPath);
return (rc == 0) ? r : "";
}
+std::string Utils::getModel(const std::string& inventoryPath) const
+{
+ // Invoke vendor-specific tool to get the model string, e.g.
+ // psutils --get-model
+ // /xyz/openbmc_project/inventory/system/chassis/motherboard/powersupply0
+ auto [rc, r] = internal::exec(PSU_MODEL_UTIL, inventoryPath);
+ return (rc == 0) ? r : "";
+}
+
std::string Utils::getLatestVersion(const std::set<std::string>& versions) const
{
if (versions.empty())
diff --git a/src/utils.hpp b/src/utils.hpp
index be85494..3b033fc 100644
--- a/src/utils.hpp
+++ b/src/utils.hpp
@@ -80,10 +80,18 @@
*
* @param[in] inventoryPath - The PSU inventory object path
*
- * @return The version string, or empry string if it fails to get the version
+ * @return The version string, or empty string if it fails to get the version
*/
std::string getVersion(const std::string& inventoryPath);
+/** @brief Get model of PSU specified by the inventory path
+ *
+ * @param[in] inventoryPath - The PSU inventory object path
+ *
+ * @return The model string, or empty string if it fails to get the model
+ */
+std::string getModel(const std::string& inventoryPath);
+
/** @brief Get latest version from the PSU versions
*
* @param[in] versions - The list of the versions
@@ -133,6 +141,8 @@
virtual std::string getVersion(const std::string& inventoryPath) const = 0;
+ virtual std::string getModel(const std::string& inventoryPath) const = 0;
+
virtual std::string
getLatestVersion(const std::set<std::string>& versions) const = 0;
@@ -171,6 +181,8 @@
std::string getVersion(const std::string& inventoryPath) const override;
+ std::string getModel(const std::string& inventoryPath) const override;
+
std::string
getLatestVersion(const std::set<std::string>& versions) const override;
@@ -209,6 +221,11 @@
return getUtils().getVersion(inventoryPath);
}
+inline std::string getModel(const std::string& inventoryPath)
+{
+ return getUtils().getModel(inventoryPath);
+}
+
inline std::string getLatestVersion(const std::set<std::string>& versions)
{
return getUtils().getLatestVersion(versions);
diff --git a/test/mocked_utils.hpp b/test/mocked_utils.hpp
index ef4baa5..873c73b 100644
--- a/test/mocked_utils.hpp
+++ b/test/mocked_utils.hpp
@@ -33,6 +33,9 @@
MOCK_CONST_METHOD1(getVersion,
std::string(const std::string& psuInventoryPath));
+ MOCK_CONST_METHOD1(getModel,
+ std::string(const std::string& psuInventoryPath));
+
MOCK_CONST_METHOD1(getLatestVersion,
std::string(const std::set<std::string>& versions));
diff --git a/test/test_activation.cpp b/test/test_activation.cpp
index 27e0c66..4beffbc 100644
--- a/test/test_activation.cpp
+++ b/test/test_activation.cpp
@@ -35,8 +35,8 @@
// By default make it compatible with the test software
ON_CALL(mockedUtils, getPropertyImpl(_, _, _, _, StrEq(MANUFACTURER)))
.WillByDefault(Return(any(PropertyType(std::string("TestManu")))));
- ON_CALL(mockedUtils, getPropertyImpl(_, _, _, _, StrEq(MODEL)))
- .WillByDefault(Return(any(PropertyType(std::string("TestModel")))));
+ ON_CALL(mockedUtils, getModel(_))
+ .WillByDefault(Return(std::string("TestModel")));
ON_CALL(mockedUtils, isAssociated(_, _)).WillByDefault(Return(false));
}
~TestActivation() override
@@ -320,9 +320,8 @@
constexpr auto psu1 = "/com/example/inventory/psu1";
constexpr auto psu2 = "/com/example/inventory/psu2";
constexpr auto psu3 = "/com/example/inventory/psu3";
- ON_CALL(mockedUtils, getPropertyImpl(_, _, StrEq(psu1), _, StrEq(MODEL)))
- .WillByDefault(
- Return(any(PropertyType(std::string("DifferentModel")))));
+ ON_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillByDefault(Return(std::string("DifferentModel")));
activation = std::make_unique<Activation>(
mockedBus, dBusPath, versionId, extVersion, status, associations,
filePath, &mockedAssociationInterface, &mockedActivationListener);
diff --git a/test/test_item_updater.cpp b/test/test_item_updater.cpp
index 7fd1e7a..ad01f42 100644
--- a/test/test_item_updater.cpp
+++ b/test/test_item_updater.cpp
@@ -69,7 +69,6 @@
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)
@@ -88,14 +87,9 @@
.WillOnce(Return(std::vector<std::string>({psuPath})));
EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
.WillOnce(Return(service));
- EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
- .WillOnce(Return(std::string(version)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(false)))); // not present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -122,9 +116,8 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("dummyModel")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -157,17 +150,15 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu0)))
+ .WillOnce(Return(std::string("dummyModel0")));
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
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillOnce(Return(std::string("dummyModel1")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -209,17 +200,15 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu0)))
+ .WillOnce(Return(std::string("dummyModel0")));
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
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillOnce(Return(std::string("dummyModel1")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -261,9 +250,8 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("dummyModel")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -295,14 +283,9 @@
.WillOnce(Return(std::vector<std::string>({psuPath})));
EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
.WillOnce(Return(service));
- EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
- .WillOnce(Return(std::string(version)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(false)))); // not present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -314,14 +297,14 @@
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
// The PSU is present and version is added in a single call
- Properties propAddedAndModel{
- {PRESENT, PropertyType(true)},
- {MODEL, PropertyType(std::string("testModel"))}};
+ Properties propAdded{{PRESENT, PropertyType(true)}};
EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
.WillOnce(Return(std::string(version)));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("testModel")));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
.Times(2);
- onPsuInventoryChanged(psuPath, propAddedAndModel);
+ onPsuInventoryChanged(psuPath, propAdded);
}
TEST_F(TestItemUpdater, OnOnePSURemovedAndAddedWithLatestVersion)
@@ -339,9 +322,8 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("dummyModel")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -359,6 +341,8 @@
EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
.WillOnce(Return(std::string(version)));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("dummyModel")));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath)))
.Times(2);
@@ -372,7 +356,6 @@
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)))
@@ -403,17 +386,15 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu0)))
+ .WillOnce(Return(std::string("dummyModel0")));
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
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillOnce(Return(std::string("dummyModel1")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -449,16 +430,18 @@
objPath1 = getObjPath(version1);
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu0)))
.WillOnce(Return(std::string(version0)));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu0)))
+ .WillOnce(Return(std::string("dummyModel0")));
EXPECT_CALL(mockedUtils, getVersion(StrEq(psu1)))
.WillOnce(Return(std::string(version1)));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillOnce(Return(std::string("dummyModel1")));
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath0)))
.Times(2);
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(objPath1)))
.Times(2);
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)))
@@ -479,14 +462,9 @@
.WillOnce(Return(std::vector<std::string>({psuPath})));
EXPECT_CALL(mockedUtils, getService(_, StrEq(psuPath), _))
.WillOnce(Return(service));
- EXPECT_CALL(mockedUtils, getVersion(StrEq(psuPath)))
- .WillOnce(Return(std::string(version)));
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(false)))); // not present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("")))));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -523,9 +501,8 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
_, StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath),
- _, StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("dummyModel")));
// The item updater itself
EXPECT_CALL(sdbusMock, sd_bus_emit_object_added(_, StrEq(dBusPath)))
@@ -572,17 +549,15 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu0)))
+ .WillOnce(Return(std::string("dummyModel0")));
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
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillOnce(Return(std::string("dummyModel1")));
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
@@ -642,17 +617,15 @@
EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
StrEq(PRESENT)))
.WillOnce(Return(any(PropertyType(true)))); // present
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu0), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel0")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu0)))
+ .WillOnce(Return(std::string("dummyModel0")));
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
- EXPECT_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psu1), _,
- StrEq(MODEL)))
- .WillOnce(Return(any(PropertyType(std::string("dummyModel1")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psu1)))
+ .WillOnce(Return(std::string("dummyModel1")));
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
@@ -702,9 +675,9 @@
ON_CALL(mockedUtils, getPropertyImpl(_, StrEq(service), StrEq(psuPath), _,
StrEq(PRESENT)))
.WillByDefault(Return(any(PropertyType(true)))); // present
- ON_CALL(mockedUtils,
- getPropertyImpl(_, StrEq(service), StrEq(psuPath), _, StrEq(MODEL)))
- .WillByDefault(Return(any(PropertyType(std::string("dummyModel")))));
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("dummyModel")));
+
itemUpdater = std::make_unique<ItemUpdater>(mockedBus, dBusPath);
// Add an association to simulate that it has image in BMC filesystem
@@ -725,10 +698,9 @@
_, 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
+ EXPECT_CALL(mockedUtils, getModel(StrEq(psuPath)))
+ .WillOnce(Return(std::string("")))
+ .WillOnce(Return(std::string("")));
std::set<std::string> expectedVersions = {version, oldVersion};
EXPECT_CALL(mockedUtils, getLatestVersion(ContainerEq(expectedVersions)))
.WillOnce(Return(version));
@@ -739,5 +711,4 @@
.Times(3); // There are 3 systemd units are started, enable bmc reboot
// guard, start activation, and disable bmc reboot guard
onPsuInventoryChanged(psuPath, propAdded);
- onPsuInventoryChanged(psuPath, propModel);
}