plugins: Add 'break' at RedifshClient request re-try loop (fixed issue from 4846) (#4864)

* Break loop if no exception on http request

* Add new tests ensuring the correct execution flow of the RedfishClient retry

* Log retry as "retry attempt %d/%d"

* Fix string.format parameters order at RedfishClient.retryHttpRequest

Author: GabrielBrascher

utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java

@@ -231,8 +231,9 @@ protected HttpResponse retryHttpRequest(String url, HttpRequestBase httpReq, Htt
         for (int attempt = 1; attempt < redfishRequestMaxRetries + 1; attempt++) {
             try {
                 TimeUnit.SECONDS.sleep(WAIT_FOR_REQUEST_RETRY);
-                LOGGER.debug(String.format("Retry HTTP %s request [URL: %s], attempt %d/%d.", httpReq.getMethod(), url, attempt, redfishRequestMaxRetries));
+                LOGGER.debug(String.format("HTTP %s request retry attempt %d/%d [URL: %s].", httpReq.getMethod(), attempt, redfishRequestMaxRetries, url));
                 response = client.execute(httpReq);
+                break;
             } catch (IOException | InterruptedException e) {
                 if (attempt == redfishRequestMaxRetries) {
                     throw new RedfishException(String.format("Failed to execute HTTP %s request retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(), attempt, redfishRequestMaxRetries,url, e));

utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java

@@ -207,4 +207,26 @@ public void retryHttpRequestNoException() throws IOException {
         Mockito.verify(newRedfishClientspy, Mockito.times(1)).retryHttpRequest(Mockito.anyString(), Mockito.any(), Mockito.any());
         Mockito.verify(client, Mockito.times(3)).execute(Mockito.any());
     }
+
+    @Test(expected = RedfishException.class)
+    public void retryHttpRequestExceptionAfterTwoRetries() throws IOException {
+        Mockito.when(client.execute(httpReq)).thenThrow(IOException.class).thenThrow(IOException.class);
+
+        RedfishClient newRedfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true, REDFISHT_REQUEST_RETRIES));
+        newRedfishClientspy.retryHttpRequest(url, httpReq, client);
+
+        Mockito.verify(newRedfishClientspy, Mockito.never()).retryHttpRequest(Mockito.anyString(), Mockito.any(), Mockito.any());
+        Mockito.verify(client, Mockito.never()).execute(Mockito.any());
+    }
+
+    @Test
+    public void retryHttpRequestSuccessAtTheSecondRetry() throws IOException {
+        Mockito.when(client.execute(httpReq)).thenThrow(IOException.class).thenReturn(httpResponse);
+
+        RedfishClient newRedfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true, REDFISHT_REQUEST_RETRIES));
+        newRedfishClientspy.retryHttpRequest(url, httpReq, client);
+
+        Mockito.verify(newRedfishClientspy, Mockito.times(1)).retryHttpRequest(Mockito.anyString(), Mockito.any(), Mockito.any());
+        Mockito.verify(client, Mockito.times(REDFISHT_REQUEST_RETRIES)).execute(Mockito.any());
+    }
 }