Skip to content

Retry redfish requests #4352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rohityadavcloud merged 3 commits into from
Sep 30, 2020
Merged

Retry redfish requests #4352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b766ef4
Re-try Redfish HTTP request
GabrielBrascher Sep 1, 2020
294338c
Code enhancements and tests
GabrielBrascher Sep 25, 2020
1bd2254
Longer wait for request retry
GabrielBrascher Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions ...pache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -74,7 +78,7 @@ private OutOfBandManagementDriverResponse execute(final OutOfBandManagementDrive
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) {
Expand Down Expand Up @@ -114,7 +118,7 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {IGNORE_SSL_CERTIFICATE, USE_HTTPS};
return new ConfigKey<?>[] {IGNORE_SSL_CERTIFICATE, USE_HTTPS, REDFISHT_REQUEST_MAX_RETRIES};
}

}
39 changes: 37 additions & 2 deletions utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;
Expand Down Expand Up @@ -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";
Expand All @@ -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>
Expand Down Expand Up @@ -126,11 +130,12 @@ public enum RedfishResetCmd {
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) {
Expand Down Expand Up @@ -213,8 +218,38 @@ private HttpResponse executeHttpRequest(String url, HttpRequestBase httpReq) {
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;
}

/**
Expand Down
63 changes: 55 additions & 8 deletions utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -68,47 +85,47 @@ public void validateAddressAndPrepareForUrlTestIpv6() {

@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);
}

@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);
}

@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);
}

@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);
}

@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);
}

@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);
Expand Down Expand Up @@ -160,4 +177,34 @@ public void getSystemIdTestHttpStatusNotOk() {
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());
}
}