From 4a0e31e5692010edc348b761f831ebb10bff4615 Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Fri, 19 Mar 2021 03:46:05 -0300 Subject: [PATCH 1/4] Break loop if no exception on http request --- .../java/org/apache/cloudstack/utils/redfish/RedfishClient.java | 1 + 1 file changed, 1 insertion(+) diff --git a/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java b/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java index 8b211c026060..ec5afd12703e 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java @@ -233,6 +233,7 @@ protected HttpResponse retryHttpRequest(String url, HttpRequestBase httpReq, Htt 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)); 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)); From c622d353b046795c5771151d730e13e0329e71ad Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Fri, 19 Mar 2021 11:12:52 -0300 Subject: [PATCH 2/4] Add new tests ensuring the correct execution flow of the RedfishClient retry --- .../utils/redfish/RedfishClientTest.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java b/utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java index 15a75bab2121..674700bd4121 100644 --- a/utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java +++ b/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()); + } } From cd5fe4bf8df66893740fe3e0e507aa290d4e3b0b Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Tue, 23 Mar 2021 10:38:31 -0300 Subject: [PATCH 3/4] Log retry as "retry attempt %d/%d" --- .../java/org/apache/cloudstack/utils/redfish/RedfishClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java b/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java index ec5afd12703e..32b4f383e94c 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java @@ -231,7 +231,7 @@ 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(), url, attempt, redfishRequestMaxRetries)); response = client.execute(httpReq); break; } catch (IOException | InterruptedException e) { From b9dd504e0d56776cb36803ad57b04c25b6e52b75 Mon Sep 17 00:00:00 2001 From: Gabriel Brascher Date: Wed, 24 Mar 2021 15:38:35 -0300 Subject: [PATCH 4/4] Fix string.format parameters order at RedfishClient.retryHttpRequest --- .../java/org/apache/cloudstack/utils/redfish/RedfishClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java b/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java index 32b4f383e94c..51d6b7cb2518 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java @@ -231,7 +231,7 @@ 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("HTTP %s request retry attempt %d/%d [URL: %s].", 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) {