Execute Post Fail Action if VPD parsing fails #425

souvik1914581 · 2024-10-16T11:27:53Z

This commit adds changes to execute Post Fail Action for a FRU if Pre Action for the FRU has passed and VPD parsing has failed.

Test:

Create a malformed VPD file by modifying copy of 7-0051/eeprom, name the copy as 7-0051_eeprom_noVTOC.dat
Create a copy of 50001001.json, name the copy as 50001001_custom.json. Replace "/sys/bus/i2c/drivers/at24/7-0051/eeprom" string in the custom JSON with "7-0051_eeprom_noVTOC.dat".
Run ./vpd-parser -f /tmp/7-0051_eeprom_noVTOC.dat -c ./50001001_custom.json.
Observe the logs to see "preAction" is successful, VPD parsing fails and "postFailAction" is executed as specified in the custom JSON.
Now run /vpd-parser -f /sys/bus/i2c/drivers/at24/7-0051/eeprom -c ./50001001.json.
Observe the logs to see "preAction" is successful, VPD parsing is successful and "postFailAction" is not executed.

This commit adds changes to execute Post Fail Action for a FRU if Pre Action for the FRU has passed and VPD parsing has failed. Test: 1. Create a malformed VPD file by modifying copy of 7-0051/eeprom, name the copy as 7-0051_eeprom_noVTOC.dat 2. Create a copy of 50001001.json, name the copy as 50001001_custom.json. Replace "/sys/bus/i2c/drivers/at24/7-0051/eeprom" string in the custom JSON with "7-0051_eeprom_noVTOC.dat". 3. Run ./vpd-parser -f /tmp/7-0051_eeprom_noVTOC.dat -c ./50001001_custom.json. 4. Observe the logs to see "preAction" is successful, VPD parsing fails and "postFailAction" is executed as specified in the custom JSON. 5. Now run /vpd-parser -f /sys/bus/i2c/drivers/at24/7-0051/eeprom -c ./50001001.json. 6. Observe the logs to see "preAction" is successful, VPD parsing is successful and "postFailAction" is not executed. Signed-off-by: Souvik Roy <[email protected]>

This commit adds changes to execute Post Fail Action for a FRU if Pre Action for the FRU has passed and VPD parsing has failed. Test: 1. Create a malformed VPD file by modifying copy of 7-0051/eeprom, name the copy as 7-0051_eeprom_noVTOC.dat 2. Create a copy of 50001001.json, name the copy as 50001001_custom.json. Replace "/sys/bus/i2c/drivers/at24/7-0051/eeprom" string in the custom JSON with "7-0051_eeprom_noVTOC.dat". 3. Run ./vpd-parser -f /tmp/7-0051_eeprom_noVTOC.dat -c ./50001001_custom.json. 4. Observe the logs to see "preAction" is successful, VPD parsing fails and "postFailAction" is executed as specified in the custom JSON. 5. Now run /vpd-parser -f /sys/bus/i2c/drivers/at24/7-0051/eeprom -c ./50001001.json. 6. Observe the logs to see "preAction" is successful, VPD parsing is successful and "postFailAction" is not executed. 7. Create a copy of 50001001_custom.json 50001001_custom_noPostFail.json with "PostFailAction" tag deleted. 8. Run ./vpd-parser -f /tmp/7-0051_eeprom_noVTOC.dat -c 50001001_custom_noPostFail.json 9. Observe the logs to see "preAction" is successful, VPD parsing fails and "postFailAction" also fails. Signed-off-by: Souvik Roy <[email protected]>

branupama · 2024-10-17T07:29:15Z

src/worker.cpp

- // after collection for FRU is successfully done.
- if (jsonUtility::isActionRequired(m_parsedJson, i_vpdFilePath, "postAction",
- "collection"))
+ try


Should we extend try block for Parser object creation as well ? so we can run "executePostFailAction" .
As we are introducing throwing an exception in Parser constructor.

Parser constructor does not throw any exception right now. Are we planning to add any throw in the Parser constructor later?

#380
raising exception in case of error paths.

@branupama That can be extended once we introduce that change in parser class.

branupama · 2024-10-17T07:32:09Z

src/worker.cpp

+ {
+ throw std::runtime_error(
+ "Required post action failed for path " + i_vpdFilePath +
+ " Aborting collection for this FRU");


Why we are aborting the collection if "postAction" fails ?
why we are not populating the collected VPD ?

I didn't change this logic. I just moved it under try block.

@branupama Why do you think that a FRU should be populated if there is a processing failure at any stage?

branupama · 2024-10-17T07:35:18Z

src/worker.cpp

 }
+ catch (std::exception& ex)


l_exception ?

We are not using l_ for the exception object reference throughout worker.cpp, so to maintain consistency, I followed the same convention.

@souvik1914581 The code of worker was written before we brought it the guidelines. Hennce, with new addition we are following that.
Please modify with l_ex.

branupama · 2024-10-17T07:36:38Z

src/worker.cpp

+ {
+ throw std::runtime_error("Post fail action failed for path " +
+ i_vpdFilePath +
+ " Aborting collection for this FRU");


as we are not aborting anything, is the message should be ?
"VPD collection failed for the FRU " + i_vpdFilePath

We end up in this catch block if

vpdParser->parse() throws when parsing fails.

processPostAction() throws if parsedVpd has no value.

Do you mean we should check for the type of exception caught and log accordingly here?
On a side note: I just noticed processPostAction() throws but it's doxygen doesn't mention @throws.

ok, this is fine.

@souvik1914581 In the case that you have added, in case "executePostFailAction" also fails then we loose the exception that was thrown by the parser path.
Could you please add that log as well to the log that you have added so that we don't miss the reason, why collection failed.

branupama · 2024-10-18T04:28:19Z

src/worker.cpp

- // after collection for FRU is successfully done.
- if (jsonUtility::isActionRequired(m_parsedJson, i_vpdFilePath, "postAction",
- "collection"))
+ try


#380
raising exception in case of error paths.

branupama · 2024-10-18T04:36:55Z

src/worker.cpp

+ {
+ throw std::runtime_error("Post fail action failed for path " +
+ i_vpdFilePath +
+ " Aborting collection for this FRU");


ok, this is fine.

SunnySrivastava1984 · 2024-10-18T04:33:02Z

src/worker.cpp

- // after collection for FRU is successfully done.
- if (jsonUtility::isActionRequired(m_parsedJson, i_vpdFilePath, "postAction",
- "collection"))
+ try


@branupama That can be extended once we introduce that change in parser class.

SunnySrivastava1984 · 2024-10-18T04:36:02Z

src/worker.cpp

+ {
+ throw std::runtime_error(
+ "Required post action failed for path " + i_vpdFilePath +
+ " Aborting collection for this FRU");


@branupama Why do you think that a FRU should be populated if there is a processing failure at any stage?

SunnySrivastava1984 · 2024-10-18T04:37:05Z

src/worker.cpp

 }
+ catch (std::exception& ex)


@souvik1914581 The code of worker was written before we brought it the guidelines. Hennce, with new addition we are following that.
Please modify with l_ex.

SunnySrivastava1984 · 2024-10-18T04:55:43Z

src/worker.cpp

+ {
+ throw std::runtime_error("Post fail action failed for path " +
+ i_vpdFilePath +
+ " Aborting collection for this FRU");


@souvik1914581 In the case that you have added, in case "executePostFailAction" also fails then we loose the exception that was thrown by the parser path.
Could you please add that log as well to the log that you have added so that we don't miss the reason, why collection failed.

This commit adds changes to execute Post Fail Action for a FRU if Pre Action for the FRU has passed and VPD parsing has failed. Test: 1. Create a malformed VPD file by modifying copy of 7-0051/eeprom, name the copy as 7-0051_eeprom_noVTOC.dat 2. Create a copy of 50001001.json, name the copy as 50001001_custom.json. Replace "/sys/bus/i2c/drivers/at24/7-0051/eeprom" string in the custom JSON with "7-0051_eeprom_noVTOC.dat". 3. Run ./vpd-parser -f /tmp/7-0051_eeprom_noVTOC.dat -c ./50001001_custom.json. 4. Observe the logs to see "preAction" is successful, VPD parsing fails and "postFailAction" is executed as specified in the custom JSON. 5. Now run /vpd-parser -f /sys/bus/i2c/drivers/at24/7-0051/eeprom -c ./50001001.json. 6. Observe the logs to see "preAction" is successful, VPD parsing is successful and "postFailAction" is not executed. 7. Create a copy of 50001001_custom.json 50001001_custom_noPostFail.json with "PostFailAction" tag deleted. 8. Run ./vpd-parser -f /tmp/7-0051_eeprom_noVTOC.dat -c 50001001_custom_noPostFail.json 9. Observe the logs to see "preAction" is successful, VPD parsing fails and "postFailAction" also fails. Signed-off-by: Souvik Roy <[email protected]>

souvik1914581 force-pushed the sr_exPostFailVpdParsingFails branch from 1c1f475 to b723db3 Compare October 16, 2024 11:28

souvik1914581 force-pushed the sr_exPostFailVpdParsingFails branch from b723db3 to eabd0a2 Compare October 16, 2024 16:42

souvik1914581 force-pushed the sr_exPostFailVpdParsingFails branch from eabd0a2 to 63f03b4 Compare October 16, 2024 17:00

souvik1914581 force-pushed the sr_exPostFailVpdParsingFails branch from 63f03b4 to a249b16 Compare October 16, 2024 17:49

souvik1914581 requested review from SunnySrivastava1984, branupama, jinuthomas, PriyangaRamasamy and RekhaAparna01 October 16, 2024 17:49

branupama suggested changes Oct 17, 2024

View reviewed changes

branupama suggested changes Oct 18, 2024

View reviewed changes

SunnySrivastava1984 requested changes Oct 18, 2024

View reviewed changes

souvik1914581 force-pushed the sr_exPostFailVpdParsingFails branch from a249b16 to c007e63 Compare October 18, 2024 05:20

souvik1914581 requested review from SunnySrivastava1984 and branupama October 18, 2024 05:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Execute Post Fail Action if VPD parsing fails #425

Execute Post Fail Action if VPD parsing fails #425

souvik1914581 commented Oct 16, 2024

branupama Oct 17, 2024

souvik1914581 Oct 17, 2024

branupama Oct 18, 2024

SunnySrivastava1984 Oct 18, 2024

branupama Oct 17, 2024

souvik1914581 Oct 17, 2024

SunnySrivastava1984 Oct 18, 2024

branupama Oct 17, 2024

souvik1914581 Oct 17, 2024

SunnySrivastava1984 Oct 18, 2024

branupama Oct 17, 2024

souvik1914581 Oct 17, 2024

branupama Oct 18, 2024

SunnySrivastava1984 Oct 18, 2024

branupama Oct 18, 2024

branupama Oct 18, 2024

SunnySrivastava1984 Oct 18, 2024

SunnySrivastava1984 Oct 18, 2024

SunnySrivastava1984 Oct 18, 2024

SunnySrivastava1984 Oct 18, 2024

Execute Post Fail Action if VPD parsing fails #425

Are you sure you want to change the base?

Execute Post Fail Action if VPD parsing fails #425

Conversation

souvik1914581 commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment