Skip to content

Commit fe7e74b

Browse files
committed
Register Azure max_retries setting (#35286)
This commit properly registers the Azure max_retries setting in the settings infrastructure, allowing this setting to be actually used.
1 parent a2e8b85 commit fe7e74b

File tree

3 files changed

+36
-18
lines changed

3 files changed

+36
-18
lines changed

plugins/repository-azure/src/main/java/org/elasticsearch/cloud/azure/storage/AzureStorageSettings.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public final class AzureStorageSettings {
5858
key -> SecureSetting.secureString(key, null));
5959

6060
/** max_retries: Number of retries in case of Azure errors. Defaults to 3 (RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT). */
61-
private static final Setting<Integer> MAX_RETRIES_SETTING =
61+
public static final Setting<Integer> MAX_RETRIES_SETTING =
6262
Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "max_retries",
6363
(key) -> Setting.intSetting(key, RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT, Setting.Property.NodeScope),
6464
ACCOUNT_SETTING, KEY_SETTING);

plugins/repository-azure/src/main/java/org/elasticsearch/plugin/repository/azure/AzureRepositoryPlugin.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public List<Setting<?>> getSettings() {
6464
AzureStorageSettings.KEY_SETTING,
6565
AzureStorageSettings.ENDPOINT_SUFFIX_SETTING,
6666
AzureStorageSettings.TIMEOUT_SETTING,
67+
AzureStorageSettings.MAX_RETRIES_SETTING,
6768
AzureStorageSettings.PROXY_TYPE_SETTING,
6869
AzureStorageSettings.PROXY_HOST_SETTING,
6970
AzureStorageSettings.PROXY_PORT_SETTING

plugins/repository-azure/src/test/java/org/elasticsearch/cloud/azure/storage/AzureStorageServiceTests.java

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,20 @@
2626
import org.elasticsearch.common.settings.Setting;
2727
import org.elasticsearch.common.settings.Settings;
2828
import org.elasticsearch.common.settings.SettingsException;
29+
import org.elasticsearch.common.settings.SettingsModule;
2930
import org.elasticsearch.plugin.repository.azure.AzureRepositoryPlugin;
3031
import org.elasticsearch.test.ESTestCase;
3132

3233
import java.io.IOException;
34+
import java.io.UncheckedIOException;
3335
import java.net.InetAddress;
3436
import java.net.InetSocketAddress;
3537
import java.net.Proxy;
3638
import java.net.URI;
3739
import java.net.URISyntaxException;
3840
import java.net.UnknownHostException;
3941
import java.nio.charset.StandardCharsets;
42+
import java.util.Collections;
4043
import java.util.Map;
4144

4245
import static org.elasticsearch.cloud.azure.storage.AzureStorageService.blobNameFromUri;
@@ -79,10 +82,24 @@ public void testReadSecuredSettings() {
7982
assertThat(loadedSettings.get("azure3").getEndpointSuffix(), equalTo("my_endpoint_suffix"));
8083
}
8184

85+
private AzureRepositoryPlugin pluginWithSettingsValidation(Settings settings) {
86+
final AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings);
87+
new SettingsModule(settings, plugin.getSettings(), Collections.emptyList(), Collections.emptySet());
88+
return plugin;
89+
}
90+
91+
private AzureStorageService storageServiceWithSettingsValidation(Settings settings) {
92+
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings)) {
93+
return plugin.azureStoreService;
94+
} catch (IOException e) {
95+
throw new UncheckedIOException(e);
96+
}
97+
}
98+
8299
public void testCreateClientWithEndpointSuffix() throws IOException {
83100
final Settings settings = Settings.builder().setSecureSettings(buildSecureSettings())
84101
.put("azure.client.azure1.endpoint_suffix", "my_endpoint_suffix").build();
85-
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings)) {
102+
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings)) {
86103
final AzureStorageService azureStorageService = plugin.azureStoreService;
87104
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
88105
assertThat(client1.getEndpoint().toString(), equalTo("https://myaccount1.blob.my_endpoint_suffix"));
@@ -104,7 +121,7 @@ public void testReinitClientSettings() throws IOException {
104121
secureSettings2.setString("azure.client.azure3.account", "myaccount23");
105122
secureSettings2.setString("azure.client.azure3.key", encodeKey("mykey23"));
106123
final Settings settings2 = Settings.builder().setSecureSettings(secureSettings2).build();
107-
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings1)) {
124+
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings1)) {
108125
final AzureStorageService azureStorageService = plugin.azureStoreService;
109126
final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
110127
assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount11.blob.core.windows.net"));
@@ -136,7 +153,7 @@ public void testReinitClientEmptySettings() throws IOException {
136153
secureSettings.setString("azure.client.azure1.account", "myaccount1");
137154
secureSettings.setString("azure.client.azure1.key", encodeKey("mykey11"));
138155
final Settings settings = Settings.builder().setSecureSettings(secureSettings).build();
139-
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings)) {
156+
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings)) {
140157
final AzureStorageService azureStorageService = plugin.azureStoreService;
141158
final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
142159
assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
@@ -160,7 +177,7 @@ public void testReinitClientWrongSettings() throws IOException {
160177
secureSettings2.setString("azure.client.azure1.account", "myaccount1");
161178
// missing key
162179
final Settings settings2 = Settings.builder().setSecureSettings(secureSettings2).build();
163-
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings1)) {
180+
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings1)) {
164181
final AzureStorageService azureStorageService = plugin.azureStoreService;
165182
final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
166183
assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
@@ -173,7 +190,7 @@ public void testReinitClientWrongSettings() throws IOException {
173190
}
174191

175192
public void testGetSelectedClientNonExisting() {
176-
final AzureStorageService azureStorageService = new AzureStorageService(buildSettings());
193+
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(buildSettings());
177194
final SettingsException e = expectThrows(SettingsException.class, () -> azureStorageService.client("azure4"));
178195
assertThat(e.getMessage(), is("Unable to find client with name [azure4]"));
179196
}
@@ -199,21 +216,21 @@ public void testGetSelectedClientDefaultTimeout() {
199216
.setSecureSettings(buildSecureSettings())
200217
.put("azure.client.azure3.timeout", "30s")
201218
.build();
202-
final AzureStorageService azureStorageService = new AzureStorageService(timeoutSettings);
219+
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(timeoutSettings);
203220
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
204221
assertThat(client1.getDefaultRequestOptions().getTimeoutIntervalInMs(), nullValue());
205222
final CloudBlobClient client3 = azureStorageService.client("azure3").v1();
206223
assertThat(client3.getDefaultRequestOptions().getTimeoutIntervalInMs(), is(30 * 1000));
207224
}
208225

209226
public void testGetSelectedClientNoTimeout() {
210-
final AzureStorageService azureStorageService = new AzureStorageService(buildSettings());
227+
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(buildSettings());
211228
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
212229
assertThat(client1.getDefaultRequestOptions().getTimeoutIntervalInMs(), is(nullValue()));
213230
}
214231

215232
public void testGetSelectedClientBackoffPolicy() {
216-
final AzureStorageService azureStorageService = new AzureStorageService(buildSettings());
233+
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(buildSettings());
217234
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
218235
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), is(notNullValue()));
219236
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), instanceOf(RetryExponentialRetry.class));
@@ -225,7 +242,7 @@ public void testGetSelectedClientBackoffPolicyNbRetries() {
225242
.put("azure.client.azure1.max_retries", 7)
226243
.build();
227244

228-
final AzureStorageService azureStorageService = new AzureStorageService(timeoutSettings);
245+
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(timeoutSettings);
229246
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
230247
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), is(notNullValue()));
231248
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), instanceOf(RetryExponentialRetry.class));
@@ -235,7 +252,7 @@ public void testNoProxy() {
235252
final Settings settings = Settings.builder()
236253
.setSecureSettings(buildSecureSettings())
237254
.build();
238-
final AzureStorageService mock = new AzureStorageService(settings);
255+
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
239256
assertThat(mock.storageSettings.get("azure1").getProxy(), nullValue());
240257
assertThat(mock.storageSettings.get("azure2").getProxy(), nullValue());
241258
assertThat(mock.storageSettings.get("azure3").getProxy(), nullValue());
@@ -248,7 +265,7 @@ public void testProxyHttp() throws UnknownHostException {
248265
.put("azure.client.azure1.proxy.port", 8080)
249266
.put("azure.client.azure1.proxy.type", "http")
250267
.build();
251-
final AzureStorageService mock = new AzureStorageService(settings);
268+
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
252269
final Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
253270

254271
assertThat(azure1Proxy, notNullValue());
@@ -268,7 +285,7 @@ public void testMultipleProxies() throws UnknownHostException {
268285
.put("azure.client.azure2.proxy.port", 8081)
269286
.put("azure.client.azure2.proxy.type", "http")
270287
.build();
271-
final AzureStorageService mock = new AzureStorageService(settings);
288+
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
272289
final Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
273290
assertThat(azure1Proxy, notNullValue());
274291
assertThat(azure1Proxy.type(), is(Proxy.Type.HTTP));
@@ -287,7 +304,7 @@ public void testProxySocks() throws UnknownHostException {
287304
.put("azure.client.azure1.proxy.port", 8080)
288305
.put("azure.client.azure1.proxy.type", "socks")
289306
.build();
290-
final AzureStorageService mock = new AzureStorageService(settings);
307+
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
291308
final Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
292309
assertThat(azure1Proxy, notNullValue());
293310
assertThat(azure1Proxy.type(), is(Proxy.Type.SOCKS));
@@ -302,7 +319,7 @@ public void testProxyNoHost() {
302319
.put("azure.client.azure1.proxy.port", 8080)
303320
.put("azure.client.azure1.proxy.type", randomFrom("socks", "http"))
304321
.build();
305-
final SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageService(settings));
322+
final SettingsException e = expectThrows(SettingsException.class, () -> storageServiceWithSettingsValidation(settings));
306323
assertEquals("Azure Proxy type has been set but proxy host or port is not defined.", e.getMessage());
307324
}
308325

@@ -313,7 +330,7 @@ public void testProxyNoPort() {
313330
.put("azure.client.azure1.proxy.type", randomFrom("socks", "http"))
314331
.build();
315332

316-
final SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageService(settings));
333+
final SettingsException e = expectThrows(SettingsException.class, () -> storageServiceWithSettingsValidation(settings));
317334
assertEquals("Azure Proxy type has been set but proxy host or port is not defined.", e.getMessage());
318335
}
319336

@@ -324,7 +341,7 @@ public void testProxyNoType() {
324341
.put("azure.client.azure1.proxy.port", 8080)
325342
.build();
326343

327-
final SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageService(settings));
344+
final SettingsException e = expectThrows(SettingsException.class, () -> storageServiceWithSettingsValidation(settings));
328345
assertEquals("Azure Proxy port or host have been set but proxy type is not defined.", e.getMessage());
329346
}
330347

@@ -336,7 +353,7 @@ public void testProxyWrongHost() {
336353
.put("azure.client.azure1.proxy.port", 8080)
337354
.build();
338355

339-
final SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageService(settings));
356+
final SettingsException e = expectThrows(SettingsException.class, () -> storageServiceWithSettingsValidation(settings));
340357
assertEquals("Azure proxy host is unknown.", e.getMessage());
341358
}
342359

0 commit comments

Comments
 (0)