This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 0f3f2a0 oobm: Retry redfish requests (#4352)
0f3f2a0 is described below
commit 0f3f2a09370a18301db28ec3d28efe746b6437c9
Author: Gabriel Beims Bräscher <[email protected]>
AuthorDate: Wed Sep 30 08:05:17 2020 -0300
oobm: Retry redfish requests (#4352)
It is not common, but HTTP requests can fail due to connection issues. In
order to mitigate such situations and also improve logging, this PR enhances
the Redfish request handling by adding an execution flow for re-trying HTTP
requests; the retry happens only if the global settings redfish.retries is set
to 1 or more retries; default is of 2 (two). One can disable the retries by
setting redfish.retries to 0 (zero).
---
.../redfish/RedfishOutOfBandManagementDriver.java | 8 ++-
.../cloudstack/utils/redfish/RedfishClient.java | 39 +++++++++++++-
.../utils/redfish/RedfishClientTest.java | 63 +++++++++++++++++++---
3 files changed, 98 insertions(+), 12 deletions(-)
diff --git
a/plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java
b/plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java
index 3e86334..14a41e1 100644
---
a/plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java
+++
b/plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java
@@ -51,6 +51,10 @@ public class RedfishOutOfBandManagementDriver extends
AdapterBase implements Out
public static final ConfigKey<Boolean> USE_HTTPS = new
ConfigKey<Boolean>("Advanced", Boolean.class, "redfish.use.https", "true",
"Use HTTPS/SSL for all connections.", true,
ConfigKey.Scope.Global);
+ public static final ConfigKey<Integer> REDFISHT_REQUEST_MAX_RETRIES = new
ConfigKey<Integer>("Advanced", Integer.class, "redfish.retries", "2",
+ "Number of retries allowed if a Redfish REST request experiment
connection issues. If set to 0 (zero) there will be no retries.", true,
ConfigKey.Scope.Global);
+
+
private static final String HTTP_STATUS_OK =
String.valueOf(HttpStatus.SC_OK);
@Override
@@ -74,7 +78,7 @@ public class RedfishOutOfBandManagementDriver extends
AdapterBase implements Out
String username =
outOfBandOptions.get(OutOfBandManagement.Option.USERNAME);
String password =
outOfBandOptions.get(OutOfBandManagement.Option.PASSWORD);
String hostAddress =
outOfBandOptions.get(OutOfBandManagement.Option.ADDRESS);
- RedfishClient redfishClient = new RedfishClient(username, password,
USE_HTTPS.value(), IGNORE_SSL_CERTIFICATE.value());
+ RedfishClient redfishClient = new RedfishClient(username, password,
USE_HTTPS.value(), IGNORE_SSL_CERTIFICATE.value(),
REDFISHT_REQUEST_MAX_RETRIES.value());
RedfishClient.RedfishPowerState powerState = null;
if (cmd.getPowerOperation() ==
OutOfBandManagement.PowerOperation.STATUS) {
@@ -114,7 +118,7 @@ public class RedfishOutOfBandManagementDriver extends
AdapterBase implements Out
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey<?>[] {IGNORE_SSL_CERTIFICATE, USE_HTTPS};
+ return new ConfigKey<?>[] {IGNORE_SSL_CERTIFICATE, USE_HTTPS,
REDFISHT_REQUEST_MAX_RETRIES};
}
}
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 8166b70..8b211c0 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
@@ -28,6 +28,7 @@ import java.nio.charset.StandardCharsets;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
import java.util.Base64;
+import java.util.concurrent.TimeUnit;
import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection;
@@ -71,6 +72,7 @@ public class RedfishClient {
private String password;
private boolean useHttps;
private boolean ignoreSsl;
+ private int redfishRequestMaxRetries;
private final static String SYSTEMS_URL_PATH = "redfish/v1/Systems/";
private final static String COMPUTER_SYSTEM_RESET_URL_PATH =
"/Actions/ComputerSystem.Reset";
@@ -81,6 +83,8 @@ public class RedfishClient {
private final static String ODATA_ID = "@odata.id";
private final static String MEMBERS = "Members";
private final static String EXPECTED_HTTP_STATUS = "2XX";
+ private final static int WAIT_FOR_REQUEST_RETRY = 2;
+
/**
* Redfish Command type: </br>
@@ -126,11 +130,12 @@ public class RedfishClient {
ForceOff, ForceOn, ForceRestart, GracefulRestart, GracefulShutdown,
Nmi, On, PowerCycle, PushPowerButton
}
- public RedfishClient(String username, String password, boolean useHttps,
boolean ignoreSsl) {
+ public RedfishClient(String username, String password, boolean useHttps,
boolean ignoreSsl, int redfishRequestRetries) {
this.username = username;
this.password = password;
this.useHttps = useHttps;
this.ignoreSsl = ignoreSsl;
+ this.redfishRequestMaxRetries = redfishRequestRetries;
}
protected String buildRequestUrl(String hostAddress, RedfishCmdType cmd,
String resourceId) {
@@ -213,8 +218,38 @@ public class RedfishClient {
try {
return client.execute(httpReq);
} catch (IOException e) {
- throw new RedfishException(String.format("Failed to execute POST
request [URL: %s] due to exception.", url, e));
+ if (redfishRequestMaxRetries == 0) {
+ throw new RedfishException(String.format("Failed to execute
HTTP %s request [URL: %s] due to exception %s.", httpReq.getMethod(), url, e),
e);
+ }
+ return retryHttpRequest(url, httpReq, client);
+ }
+ }
+
+ protected HttpResponse retryHttpRequest(String url, HttpRequestBase
httpReq, HttpClient client) {
+ LOGGER.warn(String.format("Failed to execute HTTP %s request [URL:
%s]. Executing the request again.", httpReq.getMethod(), url));
+ HttpResponse response = null;
+ 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));
+ response = client.execute(httpReq);
+ } 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));
+ } else {
+ LOGGER.warn(
+ String.format("Failed to execute HTTP %s request
retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(),
attempt, redfishRequestMaxRetries,
+ url, e));
+ }
+ }
}
+
+ if (response == null) {
+ throw new RedfishException(String.format("Failed to execute HTTP
%s request [URL: %s].", httpReq.getMethod(), url));
+ }
+
+ LOGGER.debug(String.format("Successfully executed HTTP %s request
[URL: %s].", httpReq.getMethod(), url));
+ return response;
}
/**
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 552f187..15a75ba 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
@@ -19,23 +19,40 @@
package org.apache.cloudstack.utils.redfish;
import org.apache.commons.httpclient.HttpStatus;
+import org.apache.http.HttpResponse;
import org.apache.http.StatusLine;
+import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpRequestBase;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
-@RunWith(MockitoJUnitRunner.class) public class RedfishClientTest {
+import java.io.IOException;
+
+@RunWith(MockitoJUnitRunner.class)
+public class RedfishClientTest {
private static final String USERNAME = "user";
private static final String PASSWORD = "password";
private static final String oobAddress = "oob.host.address";
private static final String systemId = "SystemID.1";
private final static String COMPUTER_SYSTEM_RESET_URL_PATH =
"/Actions/ComputerSystem.Reset";
+ private final static Integer REDFISHT_REQUEST_RETRIES = Integer.valueOf(2);
+ private static final String url =
"https://address.system.net/redfish/v1/Systems/";;
+ private static final HttpRequestBase httpReq = new HttpGet(url);
+
+ @Mock
+ HttpClient client;
- RedfishClient redfishClientspy = Mockito.spy(new RedfishClient(USERNAME,
PASSWORD, true, true));
+ @Mock
+ HttpResponse httpResponse;
+
+ RedfishClient redfishClientspy = Mockito.spy(new RedfishClient(USERNAME,
PASSWORD, true, true, REDFISHT_REQUEST_RETRIES));
@Test(expected = RedfishException.class)
public void validateAddressAndPrepareForUrlTestExpect() {
@@ -68,7 +85,7 @@ import org.mockito.junit.MockitoJUnitRunner;
@Test
public void buildRequestUrlTestHttpsGetSystemId() {
- RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
true, false);
+ RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
true, false, REDFISHT_REQUEST_RETRIES);
String result = redfishclient.buildRequestUrl(oobAddress,
RedfishClient.RedfishCmdType.GetSystemId, systemId);
String expected = String.format("https://%s/redfish/v1/Systems/";,
oobAddress, systemId);
Assert.assertEquals(expected, result);
@@ -76,7 +93,7 @@ import org.mockito.junit.MockitoJUnitRunner;
@Test
public void buildRequestUrlTestGetSystemId() {
- RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
false, false);
+ RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
false, false, REDFISHT_REQUEST_RETRIES);
String result = redfishclient.buildRequestUrl(oobAddress,
RedfishClient.RedfishCmdType.GetSystemId, systemId);
String expected = String.format("http://%s/redfish/v1/Systems/";,
oobAddress, systemId);
Assert.assertEquals(expected, result);
@@ -84,7 +101,7 @@ import org.mockito.junit.MockitoJUnitRunner;
@Test
public void buildRequestUrlTestHttpsComputerSystemReset() {
- RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
true, false);
+ RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
true, false, REDFISHT_REQUEST_RETRIES);
String result = redfishclient.buildRequestUrl(oobAddress,
RedfishClient.RedfishCmdType.ComputerSystemReset, systemId);
String expected = String.format("https://%s/redfish/v1/Systems/%s%s";,
oobAddress, systemId, COMPUTER_SYSTEM_RESET_URL_PATH);
Assert.assertEquals(expected, result);
@@ -92,7 +109,7 @@ import org.mockito.junit.MockitoJUnitRunner;
@Test
public void buildRequestUrlTestComputerSystemReset() {
- RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
false, false);
+ RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
false, false, REDFISHT_REQUEST_RETRIES);
String result = redfishclient.buildRequestUrl(oobAddress,
RedfishClient.RedfishCmdType.ComputerSystemReset, systemId);
String expected = String.format("http://%s/redfish/v1/Systems/%s%s";,
oobAddress, systemId, COMPUTER_SYSTEM_RESET_URL_PATH);
Assert.assertEquals(expected, result);
@@ -100,7 +117,7 @@ import org.mockito.junit.MockitoJUnitRunner;
@Test
public void buildRequestUrlTestHttpsGetPowerState() {
- RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
true, false);
+ RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
true, false, REDFISHT_REQUEST_RETRIES);
String result = redfishclient.buildRequestUrl(oobAddress,
RedfishClient.RedfishCmdType.GetPowerState, systemId);
String expected = String.format("https://%s/redfish/v1/Systems/%s";,
oobAddress, systemId);
Assert.assertEquals(expected, result);
@@ -108,7 +125,7 @@ import org.mockito.junit.MockitoJUnitRunner;
@Test
public void buildRequestUrlTestGetPowerState() {
- RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
false, false);
+ RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD,
false, false, REDFISHT_REQUEST_RETRIES);
String result = redfishclient.buildRequestUrl(oobAddress,
RedfishClient.RedfishCmdType.GetPowerState, systemId);
String expected = String.format("http://%s/redfish/v1/Systems/%s";,
oobAddress, systemId);
Assert.assertEquals(expected, result);
@@ -160,4 +177,34 @@ import org.mockito.junit.MockitoJUnitRunner;
redfishClientspy.getSystemId(oobAddress);
}
+ @Test(expected = RedfishException.class)
+ public void retryHttpRequestNoRetries() throws IOException {
+ RedfishClient newRedfishClientspy = Mockito.spy(new
RedfishClient(USERNAME, PASSWORD, true, true, Integer.valueOf(0)));
+ 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(expected = RedfishException.class)
+ public void retryHttpRequestExceptionAfterOneRetry() throws IOException {
+
Mockito.when(client.execute(httpReq)).thenThrow(IOException.class).thenReturn(httpResponse);
+
+ RedfishClient newRedfishClientspy = Mockito.spy(new
RedfishClient(USERNAME, PASSWORD, true, true, Integer.valueOf(1)));
+ 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 retryHttpRequestNoException() throws IOException {
+
Mockito.when(client.execute(httpReq)).thenThrow(IOException.class).thenThrow(IOException.class).thenReturn(httpResponse);
+
+ RedfishClient newRedfishClientspy = Mockito.spy(new
RedfishClient(USERNAME, PASSWORD, true, true, Integer.valueOf(3)));
+ newRedfishClientspy.retryHttpRequest(url, httpReq, client);
+
+ Mockito.verify(newRedfishClientspy,
Mockito.times(1)).retryHttpRequest(Mockito.anyString(), Mockito.any(),
Mockito.any());
+ Mockito.verify(client, Mockito.times(3)).execute(Mockito.any());
+ }
}
