GabrielBrascher opened a new pull request #4864: URL: https://github.com/apache/cloudstack/pull/4864
### Description This PR adds a missing break at the HTTP request re-try loop. It is a minor bug, but it can result in double re-try requests where one should be enough. Also, talking about corner cases, there is a small possibility of the first re-try request be successful, and the second fails, which could result in _false error_. This PR added the break and also Test cases that cover such scenarios to ensure that now we have it all covered. #### **DISCLAIMER** This PR has fixed the log issue caused by the misplaced `String.format` attributes at https://github.com/apache/cloudstack/pull/4846/commits/cd5fe4bf8df66893740fe3e0e507aa290d4e3b0b; merged in the context of #4846 and correctly reverted at #4861. To make sure all is good I re-build with the fix, and also executed all unit tests. ``` $ mvn clean install ..... [INFO] Apache CloudStack Console Proxy .................... SUCCESS [ 0.009 s] [INFO] Apache CloudStack Console Proxy - RDP Client ....... SUCCESS [ 6.635 s] [INFO] Apache CloudStack Console Proxy - Server ........... SUCCESS [ 4.458 s] [INFO] Apache CloudStack Framework - QuickCloud ........... SUCCESS [ 0.043 s] [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 11:00 min [INFO] Finished at: 2021-03-24T15:56:20-03:00 [INFO] ------------------------------------------------------------------------ ``` Additionally, All the 19 tests at RedfishClientTest have passed after commit https://github.com/apache/cloudstack/commit/b9dd504e0d56776cb36803ad57b04c25b6e52b75. ``` 1. RedfishClientTest.validateAddressAndPrepareForUrlTestIpv4 1.02 s passed 2. RedfishClientTest.validateAddressAndPrepareForUrlTestIpv6 10 ms passed 3. RedfishClientTest.retryHttpRequestExceptionAfterOneRetry 2.02 s passed 4. RedfishClientTest.validateAddressAndPrepareForUrlTestExpect 20 ms passed 5. RedfishClientTest.retryHttpRequestSuccessAtTheSecondRetry 4.02 s passed 6. RedfishClientTest.buildRequestUrlTestHttpsComputerSystemReset 61 ms passed 7. RedfishClientTest.validateAddressAndPrepareForUrlTestDomainName 1 ms passed 8. RedfishClientTest.buildRequestUrlTestGetSystemId 1 ms passed 9. RedfishClientTest.getSystemIdTestHttpStatusNotOk 53 ms passed 10. RedfishClientTest.buildRequestUrlTestComputerSystemReset 1 ms passed 11. RedfishClientTest.getSystemPowerStateTest 4 ms passed 12. RedfishClientTest.getSystemIdTest 2 ms passed 13. RedfishClientTest.retryHttpRequestNoException 6.01 s passed 14. RedfishClientTest.buildRequestUrlTestHttpsGetPowerState 3 ms passed 15. RedfishClientTest.getSystemPowerStateTestHttpStatusNotOk 10 ms passed 16. RedfishClientTest.buildRequestUrlTestGetPowerState 4 ms passed 17. RedfishClientTest.buildRequestUrlTestHttpsGetSystemId 3 ms passed 18. RedfishClientTest.retryHttpRequestNoRetries 3 ms passed 19. RedfishClientTest.retryHttpRequestExceptionAfterTwoRetries 4.00 s passed ``` <!--- ********************************************************************************* --> <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ********************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [X] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [X] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> <!-- see how your change affects other areas of the code, etc. --> A - Test steps with 4.15.0.0: connect at the JVM via remote debug place breakpoints and tail logs verify that it keeps attempting new HTTP requests even with the previous not failing until it reaches 3 attempts B - Test this implementation (4.15.1.0-SNAPSHOT): build packages update mgmt server connect on JVM to remote debug place breakpoints and tail logs verify that the break indeed works if the HTTP request has not failed <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
