Skip to content

Commit

Permalink
Register Azure max_retries setting (#35286)
Browse files Browse the repository at this point in the history 
This commit properly registers the Azure max_retries setting in the settings infrastructure,
allowing this setting to be actually used.
  • Loading branch information
@ywelsch
ywelsch authored Nov 6, 2018
1 parent 3c18aa0 commit a4b26fe
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 18 deletions.
1 change: 1 addition & 0 deletions ...itory-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepositoryPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public List<Setting<?>> getSettings() {
AzureStorageSettings.KEY_SETTING,
AzureStorageSettings.ENDPOINT_SUFFIX_SETTING,
AzureStorageSettings.TIMEOUT_SETTING,
AzureStorageSettings.MAX_RETRIES_SETTING,
AzureStorageSettings.PROXY_TYPE_SETTING,
AzureStorageSettings.PROXY_HOST_SETTING,
AzureStorageSettings.PROXY_PORT_SETTING
Expand Down
2 changes: 1 addition & 1 deletion ...sitory-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ final class AzureStorageSettings {
key -> SecureSetting.secureString(key, null));

/** max_retries: Number of retries in case of Azure errors. Defaults to 3 (RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT). */
private static final Setting<Integer> MAX_RETRIES_SETTING =
public static final Setting<Integer> MAX_RETRIES_SETTING =
Setting.affixKeySetting(AZURE_CLIENT_PREFIX_KEY, "max_retries",
(key) -> Setting.intSetting(key, RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT, Setting.Property.NodeScope),
ACCOUNT_SETTING, KEY_SETTING);
Expand Down
51 changes: 34 additions & 17 deletions ...ry-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,19 @@
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
import org.elasticsearch.common.settings.SettingsModule;
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Map;

import static org.elasticsearch.repositories.azure.AzureStorageService.blobNameFromUri;
Expand All @@ -60,10 +63,24 @@ public void testReadSecuredSettings() {
assertThat(loadedSettings.get("azure3").getEndpointSuffix(), equalTo("my_endpoint_suffix"));
}

private AzureRepositoryPlugin pluginWithSettingsValidation(Settings settings) {
final AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings);
new SettingsModule(settings, plugin.getSettings(), Collections.emptyList(), Collections.emptySet());
return plugin;
}

private AzureStorageService storageServiceWithSettingsValidation(Settings settings) {
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings)) {
return plugin.azureStoreService;
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

public void testCreateClientWithEndpointSuffix() throws IOException {
final Settings settings = Settings.builder().setSecureSettings(buildSecureSettings())
.put("azure.client.azure1.endpoint_suffix", "my_endpoint_suffix").build();
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings)) {
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings)) {
final AzureStorageService azureStorageService = plugin.azureStoreService;
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
assertThat(client1.getEndpoint().toString(), equalTo("https://myaccount1.blob.my_endpoint_suffix"));
Expand All @@ -85,7 +102,7 @@ public void testReinitClientSettings() throws IOException {
secureSettings2.setString("azure.client.azure3.account", "myaccount23");
secureSettings2.setString("azure.client.azure3.key", encodeKey("mykey23"));
final Settings settings2 = Settings.builder().setSecureSettings(secureSettings2).build();
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings1)) {
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings1)) {
final AzureStorageService azureStorageService = plugin.azureStoreService;
final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount11.blob.core.windows.net"));
Expand Down Expand Up @@ -117,7 +134,7 @@ public void testReinitClientEmptySettings() throws IOException {
secureSettings.setString("azure.client.azure1.account", "myaccount1");
secureSettings.setString("azure.client.azure1.key", encodeKey("mykey11"));
final Settings settings = Settings.builder().setSecureSettings(secureSettings).build();
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings)) {
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings)) {
final AzureStorageService azureStorageService = plugin.azureStoreService;
final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
Expand All @@ -141,7 +158,7 @@ public void testReinitClientWrongSettings() throws IOException {
secureSettings2.setString("azure.client.azure1.account", "myaccount1");
// missing key
final Settings settings2 = Settings.builder().setSecureSettings(secureSettings2).build();
try (AzureRepositoryPlugin plugin = new AzureRepositoryPlugin(settings1)) {
try (AzureRepositoryPlugin plugin = pluginWithSettingsValidation(settings1)) {
final AzureStorageService azureStorageService = plugin.azureStoreService;
final CloudBlobClient client11 = azureStorageService.client("azure1").v1();
assertThat(client11.getEndpoint().toString(), equalTo("https://myaccount1.blob.core.windows.net"));
Expand All @@ -154,7 +171,7 @@ public void testReinitClientWrongSettings() throws IOException {
}

public void testGetSelectedClientNonExisting() {
final AzureStorageService azureStorageService = new AzureStorageService(buildSettings());
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(buildSettings());
final SettingsException e = expectThrows(SettingsException.class, () -> azureStorageService.client("azure4"));
assertThat(e.getMessage(), is("Unable to find client with name [azure4]"));
}
Expand All @@ -164,21 +181,21 @@ public void testGetSelectedClientDefaultTimeout() {
.setSecureSettings(buildSecureSettings())
.put("azure.client.azure3.timeout", "30s")
.build();
final AzureStorageService azureStorageService = new AzureStorageService(timeoutSettings);
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(timeoutSettings);
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
assertThat(client1.getDefaultRequestOptions().getTimeoutIntervalInMs(), nullValue());
final CloudBlobClient client3 = azureStorageService.client("azure3").v1();
assertThat(client3.getDefaultRequestOptions().getTimeoutIntervalInMs(), is(30 * 1000));
}

public void testGetSelectedClientNoTimeout() {
final AzureStorageService azureStorageService = new AzureStorageService(buildSettings());
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(buildSettings());
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
assertThat(client1.getDefaultRequestOptions().getTimeoutIntervalInMs(), is(nullValue()));
}

public void testGetSelectedClientBackoffPolicy() {
final AzureStorageService azureStorageService = new AzureStorageService(buildSettings());
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(buildSettings());
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), is(notNullValue()));
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), instanceOf(RetryExponentialRetry.class));
Expand All @@ -190,7 +207,7 @@ public void testGetSelectedClientBackoffPolicyNbRetries() {
.put("azure.client.azure1.max_retries", 7)
.build();

final AzureStorageService azureStorageService = new AzureStorageService(timeoutSettings);
final AzureStorageService azureStorageService = storageServiceWithSettingsValidation(timeoutSettings);
final CloudBlobClient client1 = azureStorageService.client("azure1").v1();
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), is(notNullValue()));
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), instanceOf(RetryExponentialRetry.class));
Expand All @@ -200,7 +217,7 @@ public void testNoProxy() {
final Settings settings = Settings.builder()
.setSecureSettings(buildSecureSettings())
.build();
final AzureStorageService mock = new AzureStorageService(settings);
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
assertThat(mock.storageSettings.get("azure1").getProxy(), nullValue());
assertThat(mock.storageSettings.get("azure2").getProxy(), nullValue());
assertThat(mock.storageSettings.get("azure3").getProxy(), nullValue());
Expand All @@ -213,7 +230,7 @@ public void testProxyHttp() throws UnknownHostException {
.put("azure.client.azure1.proxy.port", 8080)
.put("azure.client.azure1.proxy.type", "http")
.build();
final AzureStorageService mock = new AzureStorageService(settings);
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
final Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();

assertThat(azure1Proxy, notNullValue());
Expand All @@ -233,7 +250,7 @@ public void testMultipleProxies() throws UnknownHostException {
.put("azure.client.azure2.proxy.port", 8081)
.put("azure.client.azure2.proxy.type", "http")
.build();
final AzureStorageService mock = new AzureStorageService(settings);
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
final Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
assertThat(azure1Proxy, notNullValue());
assertThat(azure1Proxy.type(), is(Proxy.Type.HTTP));
Expand All @@ -252,7 +269,7 @@ public void testProxySocks() throws UnknownHostException {
.put("azure.client.azure1.proxy.port", 8080)
.put("azure.client.azure1.proxy.type", "socks")
.build();
final AzureStorageService mock = new AzureStorageService(settings);
final AzureStorageService mock = storageServiceWithSettingsValidation(settings);
final Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
assertThat(azure1Proxy, notNullValue());
assertThat(azure1Proxy.type(), is(Proxy.Type.SOCKS));
Expand All @@ -267,7 +284,7 @@ public void testProxyNoHost() {
.put("azure.client.azure1.proxy.port", 8080)
.put("azure.client.azure1.proxy.type", randomFrom("socks", "http"))
.build();
final SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageService(settings));
final SettingsException e = expectThrows(SettingsException.class, () -> storageServiceWithSettingsValidation(settings));
assertEquals("Azure Proxy type has been set but proxy host or port is not defined.", e.getMessage());
}

Expand All @@ -278,7 +295,7 @@ public void testProxyNoPort() {
.put("azure.client.azure1.proxy.type", randomFrom("socks", "http"))
.build();

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

Expand All @@ -289,7 +306,7 @@ public void testProxyNoType() {
.put("azure.client.azure1.proxy.port", 8080)
.build();

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

Expand All @@ -301,7 +318,7 @@ public void testProxyWrongHost() {
.put("azure.client.azure1.proxy.port", 8080)
.build();

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

Expand Down

0 comments on commit a4b26fe

Please sign in to comment.