From a4b26febcb1cb756640a88aaaee5ded3bf4e987e Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 6 Nov 2018 19:33:48 +0100 Subject: [PATCH] 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. --- .../azure/AzureRepositoryPlugin.java | 1 + .../azure/AzureStorageSettings.java | 2 +- .../azure/AzureStorageServiceTests.java | 51 ++++++++++++------- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepositoryPlugin.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepositoryPlugin.java index 72b62a930ae..c6e8335bd5a 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepositoryPlugin.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepositoryPlugin.java @@ -59,6 +59,7 @@ public class AzureRepositoryPlugin extends Plugin implements RepositoryPlugin, R 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 diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java index c4e4c1439e4..1c90f97a437 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageSettings.java @@ -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 MAX_RETRIES_SETTING = + public static final Setting 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); diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java index 3b3793f22ba..f7b49bd24ad 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java +++ b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java @@ -25,9 +25,11 @@ import com.microsoft.azure.storage.core.Base64; 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; @@ -35,6 +37,7 @@ 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; @@ -60,10 +63,24 @@ public class AzureStorageServiceTests extends ESTestCase { 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")); @@ -85,7 +102,7 @@ public class AzureStorageServiceTests extends ESTestCase { 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")); @@ -117,7 +134,7 @@ public class AzureStorageServiceTests extends ESTestCase { 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")); @@ -141,7 +158,7 @@ public class AzureStorageServiceTests extends ESTestCase { 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")); @@ -154,7 +171,7 @@ public class AzureStorageServiceTests extends ESTestCase { } 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]")); } @@ -164,7 +181,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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(); @@ -172,13 +189,13 @@ public class AzureStorageServiceTests extends ESTestCase { } 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)); @@ -190,7 +207,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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)); @@ -200,7 +217,7 @@ public class AzureStorageServiceTests extends ESTestCase { 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()); @@ -213,7 +230,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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()); @@ -233,7 +250,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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)); @@ -252,7 +269,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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)); @@ -267,7 +284,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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()); } @@ -278,7 +295,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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()); } @@ -289,7 +306,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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()); } @@ -301,7 +318,7 @@ public class AzureStorageServiceTests extends ESTestCase { .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()); }