commit 3f5962877855937e32008aac96dd750d8264cd00
author Willy Tu <wltu@google.com> Fri Apr 12 16:27:17 2024 +0000
committer Willy Tu <wltu@google.com> Tue Apr 16 17:27:25 2024 +0000
tree 859c5c254a87d0cdfa69c473f6a482fb21a616dd
parent deb85ceebe045cc14028febf94e0c358aeaef171

handler: Support more retry logic

Add retry logic for more methods that may fail due to unstable
ipmi interfaces. Also added one seconds sleep in between retries to
allow some time for the interface to be initialized.

Tested: Updated BMC firmware with the new tool and didn't have any
regression.

Change-Id: Ifc4155dd895f1a654da9e03f17e4c1e8613c9133
Signed-off-by: Willy Tu <wltu@google.com>

tools/test/data_interface_mock.hpp

diff --git a/tools/test/data_interface_mock.hpp b/tools/test/data_interface_mock.hpp
index a533a32..4b7479b 100644
--- a/tools/test/data_interface_mock.hpp
+++ b/tools/test/data_interface_mock.hpp
@@ -14,6 +14,7 @@
 
     MOCK_METHOD(bool, sendContents, (const std::string&, std::uint16_t),
                 (override));
+    MOCK_METHOD(void, waitForRetry, (), (override));
     MOCK_METHOD(ipmi_flash::FirmwareFlags::UpdateFlags, supportedType, (),
                 (const, override));
 };

tools/test/tools_updater_unittest.cpp

diff --git a/tools/test/tools_updater_unittest.cpp b/tools/test/tools_updater_unittest.cpp
index ac21a1e..acaaee7 100644
--- a/tools/test/tools_updater_unittest.cpp
+++ b/tools/test/tools_updater_unittest.cpp
@@ -187,6 +187,37 @@
                  ToolException);
 }
 
+TEST_F(UpdateHandlerTest, VerifyFileToolException)
+{
+    /* On open, it can except and this is converted to a ToolException. */
+    EXPECT_CALL(blobMock, openBlob(ipmi_flash::verifyBlobId, _))
+        .Times(3)
+        .WillRepeatedly(Throw(ToolException("asdf")));
+    EXPECT_THROW(updater.verifyFile(ipmi_flash::verifyBlobId, false),
+                 ToolException);
+}
+
+TEST_F(UpdateHandlerTest, VerifyFileHandleReturnsTrueOnSuccessWithRetries)
+{
+    EXPECT_CALL(blobMock, openBlob(ipmi_flash::verifyBlobId, _))
+        .Times(3)
+        .WillRepeatedly(Return(session));
+    EXPECT_CALL(blobMock, commit(session, _))
+        .WillOnce(Throw(ToolException("asdf")))
+        .WillOnce(Throw(ToolException("asdf")))
+        .WillOnce(Return());
+    ipmiblob::StatResponse verificationResponse = {};
+    /* the other details of the response are ignored, and should be. */
+    verificationResponse.metadata.push_back(
+        static_cast<std::uint8_t>(ipmi_flash::ActionStatus::success));
+
+    EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
+        .WillOnce(Return(verificationResponse));
+    EXPECT_CALL(blobMock, closeBlob(session)).Times(3).WillRepeatedly(Return());
+
+    EXPECT_TRUE(updater.verifyFile(ipmi_flash::verifyBlobId, false));
+}
+
 TEST_F(UpdateHandlerTest, VerifyFileCommitExceptionForwards)
 {
     /* On commit, it can except. */
@@ -238,16 +269,41 @@
 {
     /* It can throw an error, when polling fails. */
     EXPECT_CALL(blobMock, openBlob(ipmi_flash::biosVersionBlobId, _))
-        .WillOnce(Return(session));
+        .Times(3)
+        .WillRepeatedly(Return(session));
     ipmiblob::StatResponse readVersionResponse = {};
     readVersionResponse.blob_state = blobs::StateFlags::commit_error;
     EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
-        .WillOnce(Return(readVersionResponse));
-    EXPECT_CALL(blobMock, closeBlob(session)).WillOnce(Return());
+        .Times(3)
+        .WillRepeatedly(Return(readVersionResponse));
+    EXPECT_CALL(blobMock, closeBlob(session)).Times(3).WillRepeatedly(Return());
     EXPECT_THROW(updater.readVersion(ipmi_flash::biosVersionBlobId),
                  ToolException);
 }
 
+TEST_F(UpdateHandlerTest, ReadVersionReturnExpectedWithRetries)
+{
+    /* It can return as expected, when polling and readBytes succeeds. */
+    EXPECT_CALL(blobMock, openBlob(ipmi_flash::biosVersionBlobId, _))
+        .Times(3)
+        .WillRepeatedly(Return(session));
+    ipmiblob::StatResponse readVersionResponse = {};
+    readVersionResponse.blob_state = blobs::StateFlags::open_read |
+                                     blobs::StateFlags::committed;
+    readVersionResponse.size = 10;
+    EXPECT_CALL(blobMock, getStat(TypedEq<std::uint16_t>(session)))
+        .Times(3)
+        .WillRepeatedly(Return(readVersionResponse));
+    std::vector<uint8_t> resp = {0x2d, 0xfe};
+    EXPECT_CALL(blobMock, readBytes(session, 0, _))
+        .WillOnce(Throw(ToolException("asdf")))
+        .WillOnce(Throw(ToolException("asdf")))
+        .WillOnce(Return(resp));
+
+    EXPECT_CALL(blobMock, closeBlob(session)).Times(3).WillRepeatedly(Return());
+    EXPECT_EQ(resp, updater.readVersion(ipmi_flash::biosVersionBlobId));
+}
+
 TEST_F(UpdateHandlerTest, ReadVersionCovertsOpenBlobExceptionToToolException)
 {
     /* On open, it can except and this is converted to a ToolException. */