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 3e863342bad..14a41e131a0 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 USE_HTTPS = new ConfigKey("Advanced", Boolean.class, "redfish.use.https", "true", "Use HTTPS/SSL for all connections.", true, ConfigKey.Scope.Global); + public static final ConfigKey REDFISHT_REQUEST_MAX_RETRIES = new ConfigKey("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 8166b703a02..8b211c02606 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:
@@ -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,10 +218,40 @@ 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; + } + /** * Returns the proper URL path for the given Redfish command ({@link RedfishCmdType}). */ 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 552f1874a4f..15a75bab212 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); - RedfishClient redfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true)); + @Mock + HttpClient client; + + @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()); + } }