Skip to content

Commit 0f3f2a0

Browse files
GabrielBrascherGabrielBrascher
authored
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).
1 parent b3bafff commit 0f3f2a0

File tree

3 files changed

+98
-12
lines changed

3 files changed

+98
-12
lines changed

‎plugins/outofbandmanagement-drivers/redfish/src/main/java/org/apache/cloudstack/outofbandmanagement/driver/redfish/RedfishOutOfBandManagementDriver.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ public class RedfishOutOfBandManagementDriver extends AdapterBase implements Out
5151
public static final ConfigKey<Boolean> USE_HTTPS = new ConfigKey<Boolean>("Advanced", Boolean.class, "redfish.use.https", "true",
5252
"Use HTTPS/SSL for all connections.", true, ConfigKey.Scope.Global);
5353

54+
public static final ConfigKey<Integer> REDFISHT_REQUEST_MAX_RETRIES = new ConfigKey<Integer>("Advanced", Integer.class, "redfish.retries", "2",
55+
"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);
56+
57+
5458
private static final String HTTP_STATUS_OK = String.valueOf(HttpStatus.SC_OK);
5559

5660
@Override
@@ -74,7 +78,7 @@ private OutOfBandManagementDriverResponse execute(final OutOfBandManagementDrive
7478
String username = outOfBandOptions.get(OutOfBandManagement.Option.USERNAME);
7579
String password = outOfBandOptions.get(OutOfBandManagement.Option.PASSWORD);
7680
String hostAddress = outOfBandOptions.get(OutOfBandManagement.Option.ADDRESS);
77-
RedfishClient redfishClient = new RedfishClient(username, password, USE_HTTPS.value(), IGNORE_SSL_CERTIFICATE.value());
81+
RedfishClient redfishClient = new RedfishClient(username, password, USE_HTTPS.value(), IGNORE_SSL_CERTIFICATE.value(), REDFISHT_REQUEST_MAX_RETRIES.value());
7882

7983
RedfishClient.RedfishPowerState powerState = null;
8084
if (cmd.getPowerOperation() == OutOfBandManagement.PowerOperation.STATUS) {
@@ -114,7 +118,7 @@ public String getConfigComponentName() {
114118

115119
@Override
116120
public ConfigKey<?>[] getConfigKeys() {
117-
return new ConfigKey<?>[] {IGNORE_SSL_CERTIFICATE, USE_HTTPS};
121+
return new ConfigKey<?>[] {IGNORE_SSL_CERTIFICATE, USE_HTTPS, REDFISHT_REQUEST_MAX_RETRIES};
118122
}
119123

120124
}

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.security.KeyManagementException;
2929
import java.security.NoSuchAlgorithmException;
3030
import java.util.Base64;
31+
import java.util.concurrent.TimeUnit;
3132

3233
import javax.net.ssl.HostnameVerifier;
3334
import javax.net.ssl.HttpsURLConnection;
@@ -71,6 +72,7 @@ public class RedfishClient {
7172
private String password;
7273
private boolean useHttps;
7374
private boolean ignoreSsl;
75+
private int redfishRequestMaxRetries;
7476

7577
private final static String SYSTEMS_URL_PATH = "redfish/v1/Systems/";
7678
private final static String COMPUTER_SYSTEM_RESET_URL_PATH = "/Actions/ComputerSystem.Reset";
@@ -81,6 +83,8 @@ public class RedfishClient {
8183
private final static String ODATA_ID = "@odata.id";
8284
private final static String MEMBERS = "Members";
8385
private final static String EXPECTED_HTTP_STATUS = "2XX";
86+
private final static int WAIT_FOR_REQUEST_RETRY = 2;
87+
8488

8589
/**
8690
* Redfish Command type: </br>
@@ -126,11 +130,12 @@ public enum RedfishResetCmd {
126130
ForceOff, ForceOn, ForceRestart, GracefulRestart, GracefulShutdown, Nmi, On, PowerCycle, PushPowerButton
127131
}
128132

129-
public RedfishClient(String username, String password, boolean useHttps, boolean ignoreSsl) {
133+
public RedfishClient(String username, String password, boolean useHttps, boolean ignoreSsl, int redfishRequestRetries) {
130134
this.username = username;
131135
this.password = password;
132136
this.useHttps = useHttps;
133137
this.ignoreSsl = ignoreSsl;
138+
this.redfishRequestMaxRetries = redfishRequestRetries;
134139
}
135140

136141
protected String buildRequestUrl(String hostAddress, RedfishCmdType cmd, String resourceId) {
@@ -213,8 +218,38 @@ private HttpResponse executeHttpRequest(String url, HttpRequestBase httpReq) {
213218
try {
214219
return client.execute(httpReq);
215220
} catch (IOException e) {
216-
throw new RedfishException(String.format("Failed to execute POST request [URL: %s] due to exception.", url, e));
221+
if (redfishRequestMaxRetries == 0) {
222+
throw new RedfishException(String.format("Failed to execute HTTP %s request [URL: %s] due to exception %s.", httpReq.getMethod(), url, e), e);
223+
}
224+
return retryHttpRequest(url, httpReq, client);
225+
}
226+
}
227+
228+
protected HttpResponse retryHttpRequest(String url, HttpRequestBase httpReq, HttpClient client) {
229+
LOGGER.warn(String.format("Failed to execute HTTP %s request [URL: %s]. Executing the request again.", httpReq.getMethod(), url));
230+
HttpResponse response = null;
231+
for (int attempt = 1; attempt < redfishRequestMaxRetries + 1; attempt++) {
232+
try {
233+
TimeUnit.SECONDS.sleep(WAIT_FOR_REQUEST_RETRY);
234+
LOGGER.debug(String.format("Retry HTTP %s request [URL: %s], attempt %d/%d.", httpReq.getMethod(), url, attempt, redfishRequestMaxRetries));
235+
response = client.execute(httpReq);
236+
} catch (IOException | InterruptedException e) {
237+
if (attempt == redfishRequestMaxRetries) {
238+
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));
239+
} else {
240+
LOGGER.warn(
241+
String.format("Failed to execute HTTP %s request retry attempt %d/%d [URL: %s] due to exception %s", httpReq.getMethod(), attempt, redfishRequestMaxRetries,
242+
url, e));
243+
}
244+
}
217245
}
246+
247+
if (response == null) {
248+
throw new RedfishException(String.format("Failed to execute HTTP %s request [URL: %s].", httpReq.getMethod(), url));
249+
}
250+
251+
LOGGER.debug(String.format("Successfully executed HTTP %s request [URL: %s].", httpReq.getMethod(), url));
252+
return response;
218253
}
219254

220255
/**

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

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,40 @@
1919
package org.apache.cloudstack.utils.redfish;
2020

2121
import org.apache.commons.httpclient.HttpStatus;
22+
import org.apache.http.HttpResponse;
2223
import org.apache.http.StatusLine;
24+
import org.apache.http.client.HttpClient;
2325
import org.apache.http.client.methods.CloseableHttpResponse;
26+
import org.apache.http.client.methods.HttpGet;
27+
import org.apache.http.client.methods.HttpRequestBase;
2428
import org.junit.Assert;
2529
import org.junit.Test;
2630
import org.junit.runner.RunWith;
31+
import org.mockito.Mock;
2732
import org.mockito.Mockito;
2833
import org.mockito.junit.MockitoJUnitRunner;
2934

30-
@RunWith(MockitoJUnitRunner.class) public class RedfishClientTest {
35+
import java.io.IOException;
36+
37+
@RunWith(MockitoJUnitRunner.class)
38+
public class RedfishClientTest {
3139

3240
private static final String USERNAME = "user";
3341
private static final String PASSWORD = "password";
3442
private static final String oobAddress = "oob.host.address";
3543
private static final String systemId = "SystemID.1";
3644
private final static String COMPUTER_SYSTEM_RESET_URL_PATH = "/Actions/ComputerSystem.Reset";
45+
private final static Integer REDFISHT_REQUEST_RETRIES = Integer.valueOf(2);
46+
private static final String url = "https://address.system.net/redfish/v1/Systems/";
47+
private static final HttpRequestBase httpReq = new HttpGet(url);
48+
49+
@Mock
50+
HttpClient client;
3751

38-
RedfishClient redfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true));
52+
@Mock
53+
HttpResponse httpResponse;
54+
55+
RedfishClient redfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true, REDFISHT_REQUEST_RETRIES));
3956

4057
@Test(expected = RedfishException.class)
4158
public void validateAddressAndPrepareForUrlTestExpect() {
@@ -68,47 +85,47 @@ public void validateAddressAndPrepareForUrlTestIpv6() {
6885

6986
@Test
7087
public void buildRequestUrlTestHttpsGetSystemId() {
71-
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, true, false);
88+
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, true, false, REDFISHT_REQUEST_RETRIES);
7289
String result = redfishclient.buildRequestUrl(oobAddress, RedfishClient.RedfishCmdType.GetSystemId, systemId);
7390
String expected = String.format("https://%s/redfish/v1/Systems/", oobAddress, systemId);
7491
Assert.assertEquals(expected, result);
7592
}
7693

7794
@Test
7895
public void buildRequestUrlTestGetSystemId() {
79-
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, false, false);
96+
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, false, false, REDFISHT_REQUEST_RETRIES);
8097
String result = redfishclient.buildRequestUrl(oobAddress, RedfishClient.RedfishCmdType.GetSystemId, systemId);
8198
String expected = String.format("http://%s/redfish/v1/Systems/", oobAddress, systemId);
8299
Assert.assertEquals(expected, result);
83100
}
84101

85102
@Test
86103
public void buildRequestUrlTestHttpsComputerSystemReset() {
87-
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, true, false);
104+
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, true, false, REDFISHT_REQUEST_RETRIES);
88105
String result = redfishclient.buildRequestUrl(oobAddress, RedfishClient.RedfishCmdType.ComputerSystemReset, systemId);
89106
String expected = String.format("https://%s/redfish/v1/Systems/%s%s", oobAddress, systemId, COMPUTER_SYSTEM_RESET_URL_PATH);
90107
Assert.assertEquals(expected, result);
91108
}
92109

93110
@Test
94111
public void buildRequestUrlTestComputerSystemReset() {
95-
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, false, false);
112+
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, false, false, REDFISHT_REQUEST_RETRIES);
96113
String result = redfishclient.buildRequestUrl(oobAddress, RedfishClient.RedfishCmdType.ComputerSystemReset, systemId);
97114
String expected = String.format("http://%s/redfish/v1/Systems/%s%s", oobAddress, systemId, COMPUTER_SYSTEM_RESET_URL_PATH);
98115
Assert.assertEquals(expected, result);
99116
}
100117

101118
@Test
102119
public void buildRequestUrlTestHttpsGetPowerState() {
103-
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, true, false);
120+
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, true, false, REDFISHT_REQUEST_RETRIES);
104121
String result = redfishclient.buildRequestUrl(oobAddress, RedfishClient.RedfishCmdType.GetPowerState, systemId);
105122
String expected = String.format("https://%s/redfish/v1/Systems/%s", oobAddress, systemId);
106123
Assert.assertEquals(expected, result);
107124
}
108125

109126
@Test
110127
public void buildRequestUrlTestGetPowerState() {
111-
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, false, false);
128+
RedfishClient redfishclient = new RedfishClient(USERNAME, PASSWORD, false, false, REDFISHT_REQUEST_RETRIES);
112129
String result = redfishclient.buildRequestUrl(oobAddress, RedfishClient.RedfishCmdType.GetPowerState, systemId);
113130
String expected = String.format("http://%s/redfish/v1/Systems/%s", oobAddress, systemId);
114131
Assert.assertEquals(expected, result);
@@ -160,4 +177,34 @@ public void getSystemIdTestHttpStatusNotOk() {
160177
redfishClientspy.getSystemId(oobAddress);
161178
}
162179

180+
@Test(expected = RedfishException.class)
181+
public void retryHttpRequestNoRetries() throws IOException {
182+
RedfishClient newRedfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true, Integer.valueOf(0)));
183+
newRedfishClientspy.retryHttpRequest(url, httpReq, client);
184+
185+
Mockito.verify(newRedfishClientspy, Mockito.never()).retryHttpRequest(Mockito.anyString(), Mockito.any(), Mockito.any());
186+
Mockito.verify(client, Mockito.never()).execute(Mockito.any());
187+
}
188+
189+
@Test(expected = RedfishException.class)
190+
public void retryHttpRequestExceptionAfterOneRetry() throws IOException {
191+
Mockito.when(client.execute(httpReq)).thenThrow(IOException.class).thenReturn(httpResponse);
192+
193+
RedfishClient newRedfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true, Integer.valueOf(1)));
194+
newRedfishClientspy.retryHttpRequest(url, httpReq, client);
195+
196+
Mockito.verify(newRedfishClientspy, Mockito.never()).retryHttpRequest(Mockito.anyString(), Mockito.any(), Mockito.any());
197+
Mockito.verify(client, Mockito.never()).execute(Mockito.any());
198+
}
199+
200+
@Test
201+
public void retryHttpRequestNoException() throws IOException {
202+
Mockito.when(client.execute(httpReq)).thenThrow(IOException.class).thenThrow(IOException.class).thenReturn(httpResponse);
203+
204+
RedfishClient newRedfishClientspy = Mockito.spy(new RedfishClient(USERNAME, PASSWORD, true, true, Integer.valueOf(3)));
205+
newRedfishClientspy.retryHttpRequest(url, httpReq, client);
206+
207+
Mockito.verify(newRedfishClientspy, Mockito.times(1)).retryHttpRequest(Mockito.anyString(), Mockito.any(), Mockito.any());
208+
Mockito.verify(client, Mockito.times(3)).execute(Mockito.any());
209+
}
163210
}

0 commit comments

Comments
 (0)