diff options
author | Valerij Fredriksen <valerijf@oath.com> | 2018-01-05 14:50:54 +0100 |
---|---|---|
committer | Valerij Fredriksen <valerijf@oath.com> | 2018-01-09 15:03:36 +0100 |
commit | c604803f8c2374980bcb721a93430b22e557c86e (patch) | |
tree | d9b6d639f56a4b0e7e88b63afca88dc6c9c257ba /node-admin | |
parent | e152b313428c6acc2ccefa00f8d01013c9b4f7f2 (diff) |
Use self-closeable HTTP client in node-admin request executor
Diffstat (limited to 'node-admin')
3 files changed, 59 insertions, 59 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java index 94ad94d9a65..130c2aa1535 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java @@ -3,8 +3,8 @@ package com.yahoo.vespa.hosted.node.admin.util; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.concurrent.ThreadFactoryFactory; import org.apache.http.HttpHeaders; -import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; @@ -13,27 +13,27 @@ import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpRequestBase; import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.config.Registry; -import org.apache.http.config.RegistryBuilder; -import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.conn.socket.PlainConnectionSocketFactory; import org.apache.http.conn.ssl.NoopHostnameVerifier; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.ssl.SSLContextBuilder; +import javax.net.ssl.SSLContext; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URI; -import java.security.GeneralSecurityException; +import java.security.KeyManagementException; +import java.security.NoSuchAlgorithmException; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; /** * Retries request on config server a few times before giving up. Assumes that all requests should be sent with @@ -43,60 +43,29 @@ import java.util.Optional; */ public class ConfigServerHttpRequestExecutor { private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerHttpRequestExecutor.class); + private static final Duration CLIENT_REFRESH_INTERVAL = Duration.ofHours(1); private final ObjectMapper mapper = new ObjectMapper(); - private final CloseableHttpClient client; + private final ScheduledExecutorService clientRefresherScheduler = + Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("http-client-refresher")); private final List<URI> configServerHosts; - @Override - public void finalize() throws Throwable { - try { - client.close(); - } catch (Exception e) { - NODE_ADMIN_LOGGER.warning("Ignoring exception thrown when closing client against " + configServerHosts, e); - } + private SelfCloseableHttpClient client; - super.finalize(); - } - - public static ConfigServerHttpRequestExecutor create(Collection<URI> configServerUris) { - PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(getConnectionSocketFactoryRegistry()); - cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough - - // Have experienced hang in socket read, which may have been because of - // system defaults, therefore set explicit timeouts. Set arbitrarily to - // 15s > 10s used by Orchestrator lock timeout. - int timeoutMs = 15_000; - RequestConfig requestBuilder = RequestConfig.custom() - .setConnectTimeout(timeoutMs) // establishment of connection - .setConnectionRequestTimeout(timeoutMs) // connection from connection manager - .setSocketTimeout(timeoutMs) // waiting for data - .build(); - - return new ConfigServerHttpRequestExecutor(randomizeConfigServerUris(configServerUris), - HttpClientBuilder.create() - .setDefaultRequestConfig(requestBuilder) - .disableAutomaticRetries() - .setUserAgent("node-admin") - .setConnectionManager(cm).build()); - } + public static ConfigServerHttpRequestExecutor create( + Collection<URI> configServerUris, Optional<KeyStoreOptions> keyStoreOptions, Optional<KeyStoreOptions> trustStoreOptions) { + Supplier<SelfCloseableHttpClient> clientSupplier = () -> createHttpClient(keyStoreOptions, trustStoreOptions); + ConfigServerHttpRequestExecutor requestExecutor = new ConfigServerHttpRequestExecutor( + randomizeConfigServerUris(configServerUris), clientSupplier.get()); - private static Registry<ConnectionSocketFactory> getConnectionSocketFactoryRegistry() { - try { - SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory( - new SSLContextBuilder().loadTrustMaterial(null, (arg0, arg1) -> true).build(), - NoopHostnameVerifier.INSTANCE); - - return RegistryBuilder.<ConnectionSocketFactory>create() - .register("http", PlainConnectionSocketFactory.getSocketFactory()) - .register("https", sslSocketFactory) - .build(); - } catch (GeneralSecurityException e) { - throw new RuntimeException("Failed to create SSL context", e); + if (keyStoreOptions.isPresent() || trustStoreOptions.isPresent()) { + requestExecutor.clientRefresherScheduler.scheduleAtFixedRate(() -> requestExecutor.client = clientSupplier.get(), + CLIENT_REFRESH_INTERVAL.toMillis(), CLIENT_REFRESH_INTERVAL.toMillis(), TimeUnit.MILLISECONDS); } + return requestExecutor; } - ConfigServerHttpRequestExecutor(List<URI> configServerHosts, CloseableHttpClient client) { + ConfigServerHttpRequestExecutor(List<URI> configServerHosts, SelfCloseableHttpClient client) { this.configServerHosts = configServerHosts; this.client = client; } @@ -200,4 +169,38 @@ public class ConfigServerHttpRequestExecutor { return shuffledConfigServerHosts; } + private static SelfCloseableHttpClient createHttpClient(Optional<KeyStoreOptions> keyStoreOptions, + Optional<KeyStoreOptions> trustStoreOptions) { + NODE_ADMIN_LOGGER.info("Creating new HTTP client"); + try { + SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory( + makeSslContext(keyStoreOptions, trustStoreOptions), NoopHostnameVerifier.INSTANCE); + return new SelfCloseableHttpClient(sslSocketFactory); + } catch (Exception e) { + NODE_ADMIN_LOGGER.error("Failed to create HTTP client with custom SSL Context, proceeding with default"); + return new SelfCloseableHttpClient(); + } + } + + private static SSLContext makeSslContext(Optional<KeyStoreOptions> keyStoreOptions, Optional<KeyStoreOptions> trustStoreOptions) + throws KeyManagementException, NoSuchAlgorithmException { + SSLContextBuilder sslContextBuilder = new SSLContextBuilder(); + keyStoreOptions.ifPresent(options -> { + try { + sslContextBuilder.loadKeyMaterial(options.getKeyStore(), options.password); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + trustStoreOptions.ifPresent(options -> { + try { + sslContextBuilder.loadTrustMaterial(options.getKeyStore(), null); + } catch (Exception e) { + throw new RuntimeException(e); + } + }); + + return sslContextBuilder.build(); + } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java index 45f5de3d1a5..c6e4447a606 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java @@ -57,7 +57,7 @@ public class NodeRepositoryImplTest { public void startContainer() throws Exception { final int port = findRandomOpenPort(); requestExecutor = ConfigServerHttpRequestExecutor.create( - Collections.singleton(URI.create("http://127.0.0.1:" + port))); + Collections.singleton(URI.create("http://127.0.0.1:" + port)), Optional.empty(), Optional.empty()); container = JDisc.fromServicesXml(ContainerConfig.servicesXmlV2(port), Networking.enable); } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java index 799f8a72fd9..175d3a9a051 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java @@ -7,7 +7,6 @@ import org.apache.http.HttpVersion; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.entity.BasicHttpEntity; -import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.message.BasicStatusLine; import org.junit.Before; import org.junit.Test; @@ -27,7 +26,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -56,7 +54,7 @@ public class ConfigServerHttpRequestExecutorTest { @Before public void initExecutor() throws IOException { - CloseableHttpClient httpMock = mock(CloseableHttpClient.class); + SelfCloseableHttpClient httpMock = mock(SelfCloseableHttpClient.class); when(httpMock.execute(any())).thenAnswer(invocationOnMock -> { HttpGet get = (HttpGet) invocationOnMock.getArguments()[0]; mockLog.append(get.getMethod()).append(" ").append(get.getURI()).append(" "); @@ -74,7 +72,6 @@ public class ConfigServerHttpRequestExecutorTest { return response; }); - doNothing().when(httpMock).close(); executor = new ConfigServerHttpRequestExecutor(configServers, httpMock); } |