From bfc9d5bf2effd4f782a493f8a60951328783d9cc Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Thu, 15 Feb 2018 12:51:41 +0100 Subject: ConfigServerHttpRequestExecutor -> ConfigServerApiImpl, also removed scheduler that refreshes SSLConnectionSocketFactory --- .../node/admin/configserver/ConfigServerApi.java | 27 +++ .../admin/configserver/ConfigServerApiImpl.java | 171 ++++++++++++++ .../node/admin/configserver/HttpException.java | 52 +++++ .../configserver/SelfCloseableHttpClient.java | 77 +++++++ .../util/ConfigServerHttpRequestExecutor.java | 249 --------------------- .../hosted/node/admin/util/HttpException.java | 52 ----- .../node/admin/util/SelfCloseableHttpClient.java | 77 ------- .../configserver/ConfigServerApiImplTest.java | 164 ++++++++++++++ .../util/ConfigServerHttpRequestExecutorTest.java | 164 -------------- 9 files changed, 491 insertions(+), 542 deletions(-) create mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java create mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java create mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java create mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java delete mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java delete mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/HttpException.java delete mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SelfCloseableHttpClient.java create mode 100644 node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java delete mode 100644 node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java new file mode 100644 index 00000000000..2de84bf10e6 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java @@ -0,0 +1,27 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.configserver; + +import java.util.Optional; + +/** + * Interface to execute basic HTTP request against config server(s) + * + * @author freva + */ +public interface ConfigServerApi { + + T get(String path, Class wantedReturnType); + + T post(String path, Object bodyJsonPojo, Class wantedReturnType); + + T put(String path, Optional bodyJsonPojo, Class wantedReturnType); + + T patch(String path, Object bodyJsonPojo, Class wantedReturnType); + + T delete(String path, Class wantedReturnType); + + /** + * Close the underlying HTTP client and any threads this class might have started. + */ + void stop(); +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java new file mode 100644 index 00000000000..c10340525da --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java @@ -0,0 +1,171 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.configserver; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; +import org.apache.http.HttpHeaders; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpPatch; +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.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.entity.StringEntity; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +/** + * Retries request on config server a few times before giving up. Assumes that all requests should be sent with + * content-type application/json + * + * @author dybdahl + */ +public class ConfigServerApiImpl implements ConfigServerApi { + private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class); + + private final ObjectMapper mapper = new ObjectMapper(); + + private final List configServerHosts; + + /** + * The 'client' will be periodically re-created by clientRefresherScheduler if we provide keyStoreOptions + * or trustStoreOptions. This is needed because the key/trust stores are updated outside of node-admin, + * but we want to use the most recent store. + * + * The 'client' reference must be volatile because it is set and read in different threads, and visibility + * of changes is only guaranteed for volatile variables. + */ + private volatile SelfCloseableHttpClient client; + + public ConfigServerApiImpl(Collection configServerUris) { + this(configServerUris, SSLConnectionSocketFactory.getSocketFactory()); + } + + ConfigServerApiImpl(Collection configServerUris, SSLConnectionSocketFactory sslConnectionSocketFactory) { + this(randomizeConfigServerUris(configServerUris), new SelfCloseableHttpClient(sslConnectionSocketFactory)); + } + + ConfigServerApiImpl(List configServerHosts, SelfCloseableHttpClient client) { + this.configServerHosts = configServerHosts; + this.client = client; + } + + interface CreateRequest { + HttpUriRequest createRequest(URI configServerUri) throws JsonProcessingException, UnsupportedEncodingException; + } + + private T tryAllConfigServers(CreateRequest requestFactory, Class wantedReturnType) { + Exception lastException = null; + for (URI configServer : configServerHosts) { + final CloseableHttpResponse response; + try { + response = client.execute(requestFactory.createRequest(configServer)); + } catch (Exception e) { + // Failure to communicate with a config server is not abnormal, as they are + // upgraded at the same time as Docker hosts. + if (e.getMessage().indexOf("(Connection refused)") > 0) { + NODE_ADMIN_LOGGER.info("Connection refused to " + configServer + " (upgrading?), will try next"); + } else { + NODE_ADMIN_LOGGER.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); + } + lastException = e; + continue; + } + + try { + Optional retryableException = HttpException.handleStatusCode( + response.getStatusLine().getStatusCode(), + "Config server " + configServer); + if (retryableException.isPresent()) { + lastException = retryableException.get(); + continue; + } + + try { + return mapper.readValue(response.getEntity().getContent(), wantedReturnType); + } catch (IOException e) { + throw new RuntimeException("Response didn't contain nodes element, failed parsing?", e); + } + } finally { + try { + response.close(); + } catch (IOException e) { + NODE_ADMIN_LOGGER.warning("Ignoring exception from closing response", e); + } + } + } + + throw new RuntimeException("All requests against the config servers (" + + configServerHosts + ") failed, last as follows:", lastException); + } + + public T put(String path, Optional bodyJsonPojo, Class wantedReturnType) { + return tryAllConfigServers(configServer -> { + HttpPut put = new HttpPut(configServer.resolve(path)); + setContentTypeToApplicationJson(put); + if (bodyJsonPojo.isPresent()) { + put.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo.get()))); + } + return put; + }, wantedReturnType); + } + + public T patch(String path, Object bodyJsonPojo, Class wantedReturnType) { + return tryAllConfigServers(configServer -> { + HttpPatch patch = new HttpPatch(configServer.resolve(path)); + setContentTypeToApplicationJson(patch); + patch.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo))); + return patch; + }, wantedReturnType); + } + + public T delete(String path, Class wantedReturnType) { + return tryAllConfigServers(configServer -> + new HttpDelete(configServer.resolve(path)), wantedReturnType); + } + + public T get(String path, Class wantedReturnType) { + return tryAllConfigServers(configServer -> + new HttpGet(configServer.resolve(path)), wantedReturnType); + } + + public T post(String path, Object bodyJsonPojo, Class wantedReturnType) { + return tryAllConfigServers(configServer -> { + HttpPost post = new HttpPost(configServer.resolve(path)); + setContentTypeToApplicationJson(post); + post.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo))); + return post; + }, wantedReturnType); + } + + private void setContentTypeToApplicationJson(HttpRequestBase request) { + request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json"); + } + + public void setSSLConnectionSocketFactory(SSLConnectionSocketFactory sslSocketFactory) { + this.client = new SelfCloseableHttpClient(sslSocketFactory); + } + + // Shuffle config server URIs to balance load + private static List randomizeConfigServerUris(Collection configServerUris) { + List shuffledConfigServerHosts = new ArrayList<>(configServerUris); + Collections.shuffle(shuffledConfigServerHosts); + return shuffledConfigServerHosts; + } + + @Override + public void stop() { + client.close(); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java new file mode 100644 index 00000000000..0a2ae1bd426 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java @@ -0,0 +1,52 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.configserver; + +import javax.ws.rs.core.Response; +import java.util.Optional; + +@SuppressWarnings("serial") +public class HttpException extends RuntimeException { + public static class NotFoundException extends HttpException { + private static final long serialVersionUID = 4791511887L; + public NotFoundException(String message) { + super(Response.Status.NOT_FOUND, message); + } + } + + /** + * Returns empty on success. + * Returns an exception if the error is retriable. + * Throws an exception on a non-retriable error, like 404 Not Found. + */ + static Optional handleStatusCode(int statusCode, String message) { + Response.Status status = Response.Status.fromStatusCode(statusCode); + if (status == null) { + return Optional.of(new HttpException(statusCode, message)); + } + + switch (status.getFamily()) { + case SUCCESSFUL: return Optional.empty(); + case CLIENT_ERROR: + switch (status) { + case NOT_FOUND: + throw new NotFoundException(message); + case CONFLICT: + // A response body is assumed to be present, and + // will later be interpreted as an error. + return Optional.empty(); + } + throw new HttpException(statusCode, message); + } + + // Other errors like server-side errors are assumed to be retryable. + return Optional.of(new HttpException(status, message)); + } + + private HttpException(int statusCode, String message) { + super("HTTP status code " + statusCode + ": " + message); + } + + private HttpException(Response.Status status, String message) { + super(status.toString() + " (" + status.getStatusCode() + "): " + message); + } +} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java new file mode 100644 index 00000000000..cead7816387 --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java @@ -0,0 +1,77 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.configserver; + +import com.yahoo.log.LogLevel; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +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.SSLConnectionSocketFactory; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; + +import java.io.IOException; +import java.util.logging.Logger; + +/** + * @author freva + */ +class SelfCloseableHttpClient implements AutoCloseable { + + private static final Logger log = Logger.getLogger(SelfCloseableHttpClient.class.getName()); + + private final CloseableHttpClient httpClient; + + SelfCloseableHttpClient() { + this(SSLConnectionSocketFactory.getSocketFactory()); + } + + SelfCloseableHttpClient(SSLConnectionSocketFactory sslConnectionSocketFactory) { + Registry socketFactoryRegistry = RegistryBuilder.create() + .register("http", PlainConnectionSocketFactory.getSocketFactory()) + .register("https", sslConnectionSocketFactory) + .build(); + + PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(socketFactoryRegistry); + 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(); + + this.httpClient = HttpClientBuilder.create() + .setDefaultRequestConfig(requestBuilder) + .disableAutomaticRetries() + .setUserAgent("node-admin") + .setConnectionManager(cm).build(); + } + + public CloseableHttpResponse execute(HttpUriRequest request) throws IOException { + return httpClient.execute(request); + } + + @Override + public void finalize() throws Throwable { + close(); + super.finalize(); + } + + @Override + public void close() { + try { + httpClient.close(); + } catch (Exception e) { + log.log(LogLevel.WARNING, "Ignoring exception thrown when closing http client", e); + } + } +} 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 deleted file mode 100644 index 13bfc949533..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java +++ /dev/null @@ -1,249 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -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 com.yahoo.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; -import com.yahoo.vespa.athenz.tls.AthenzSslContextBuilder; -import org.apache.http.HttpHeaders; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpDelete; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpPatch; -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.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.entity.StringEntity; -import org.bouncycastle.jce.provider.BouncyCastleProvider; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.UnsupportedEncodingException; -import java.net.URI; -import java.nio.file.Path; -import java.security.GeneralSecurityException; -import java.security.KeyStore; -import java.security.Security; -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 - * content-type application/json - * - * @author dybdahl - */ -public class ConfigServerHttpRequestExecutor implements AutoCloseable { - 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 ScheduledExecutorService clientRefresherScheduler = - Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("http-client-refresher")); - private final List configServerHosts; - - /** - * The 'client' will be periodically re-created by clientRefresherScheduler if we provide keyStoreOptions - * or trustStoreOptions. This is needed because the key/trust stores are updated outside of node-admin, - * but we want to use the most recent store. - * - * The 'client' reference must be volatile because it is set and read in different threads, and visibility - * of changes is only guaranteed for volatile variables. - */ - private volatile SelfCloseableHttpClient client; - - public static ConfigServerHttpRequestExecutor create( - Collection configServerUris, - Optional keyStoreOptions, - Optional trustStoreOptions, - Optional athenzIdentity) { - Security.addProvider(new BouncyCastleProvider()); - - Supplier clientSupplier = () -> createHttpClient(keyStoreOptions, trustStoreOptions, athenzIdentity); - ConfigServerHttpRequestExecutor requestExecutor = new ConfigServerHttpRequestExecutor( - randomizeConfigServerUris(configServerUris), clientSupplier.get()); - - 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 configServerHosts, SelfCloseableHttpClient client) { - this.configServerHosts = configServerHosts; - this.client = client; - } - - public interface CreateRequest { - HttpUriRequest createRequest(URI configServerUri) throws JsonProcessingException, UnsupportedEncodingException; - } - - private T tryAllConfigServers(CreateRequest requestFactory, Class wantedReturnType) { - Exception lastException = null; - for (URI configServer : configServerHosts) { - final CloseableHttpResponse response; - try { - response = client.execute(requestFactory.createRequest(configServer)); - } catch (Exception e) { - // Failure to communicate with a config server is not abnormal, as they are - // upgraded at the same time as Docker hosts. - if (e.getMessage().indexOf("(Connection refused)") > 0) { - NODE_ADMIN_LOGGER.info("Connection refused to " + configServer + " (upgrading?), will try next"); - } else { - NODE_ADMIN_LOGGER.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); - } - lastException = e; - continue; - } - - try { - Optional retryableException = HttpException.handleStatusCode( - response.getStatusLine().getStatusCode(), - "Config server " + configServer); - if (retryableException.isPresent()) { - lastException = retryableException.get(); - continue; - } - - try { - return mapper.readValue(response.getEntity().getContent(), wantedReturnType); - } catch (IOException e) { - throw new RuntimeException("Response didn't contain nodes element, failed parsing?", e); - } - } finally { - try { - response.close(); - } catch (IOException e) { - NODE_ADMIN_LOGGER.warning("Ignoring exception from closing response", e); - } - } - } - - throw new RuntimeException("All requests against the config servers (" - + configServerHosts + ") failed, last as follows:", lastException); - } - - public T put(String path, Optional bodyJsonPojo, Class wantedReturnType) { - return tryAllConfigServers(configServer -> { - HttpPut put = new HttpPut(configServer.resolve(path)); - setContentTypeToApplicationJson(put); - if (bodyJsonPojo.isPresent()) { - put.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo.get()))); - } - return put; - }, wantedReturnType); - } - - public T patch(String path, Object bodyJsonPojo, Class wantedReturnType) { - return tryAllConfigServers(configServer -> { - HttpPatch patch = new HttpPatch(configServer.resolve(path)); - setContentTypeToApplicationJson(patch); - patch.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo))); - return patch; - }, wantedReturnType); - } - - public T delete(String path, Class wantedReturnType) { - return tryAllConfigServers(configServer -> - new HttpDelete(configServer.resolve(path)), wantedReturnType); - } - - public T get(String path, Class wantedReturnType) { - return tryAllConfigServers(configServer -> - new HttpGet(configServer.resolve(path)), wantedReturnType); - } - - public T post(String path, Object bodyJsonPojo, Class wantedReturnType) { - return tryAllConfigServers(configServer -> { - HttpPost post = new HttpPost(configServer.resolve(path)); - setContentTypeToApplicationJson(post); - post.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo))); - return post; - }, wantedReturnType); - } - - private void setContentTypeToApplicationJson(HttpRequestBase request) { - request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json"); - } - - // Shuffle config server URIs to balance load - private static List randomizeConfigServerUris(Collection configServerUris) { - List shuffledConfigServerHosts = new ArrayList<>(configServerUris); - Collections.shuffle(shuffledConfigServerHosts); - return shuffledConfigServerHosts; - } - - private static SelfCloseableHttpClient createHttpClient(Optional keyStoreOptions, - Optional trustStoreOptions, - Optional athenzIdentity) { - NODE_ADMIN_LOGGER.info("Creating new HTTP client"); - try { - SSLContext sslContext = makeSslContext(keyStoreOptions, trustStoreOptions); - HostnameVerifier hostnameVerifier = makeHostnameVerifier(athenzIdentity); - SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory(sslContext, hostnameVerifier); - return new SelfCloseableHttpClient(sslSocketFactory); - } catch (Exception e) { - NODE_ADMIN_LOGGER.error("Failed to create HTTP client with custom SSL Context, proceeding with default", e); - return new SelfCloseableHttpClient(); - } - } - - private static SSLContext makeSslContext(Optional keyStoreOptions, Optional trustStoreOptions) { - AthenzSslContextBuilder sslContextBuilder = new AthenzSslContextBuilder(); - trustStoreOptions.ifPresent(options -> sslContextBuilder.withTrustStore(options.path.toFile(), options.type)); - keyStoreOptions.ifPresent(options -> { - try { - KeyStore keyStore = loadKeyStoreFromFileWithProvider(options.path, options.password, options.type, "BC"); - sslContextBuilder.withKeyStore(keyStore, options.password); - } catch (Exception e) { - throw new RuntimeException("Failed to read key store", e); - } - }); - - return sslContextBuilder.build(); - } - - private static HostnameVerifier makeHostnameVerifier(Optional athenzIdentity) { - return athenzIdentity - .map(identity -> (HostnameVerifier) new AthenzIdentityVerifier(Collections.singleton(identity))) - .orElse(SSLConnectionSocketFactory.getDefaultHostnameVerifier()); - } - - @Override - public void close() { - clientRefresherScheduler.shutdown(); - do { - try { - clientRefresherScheduler.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS); - } catch (InterruptedException e1) { - NODE_ADMIN_LOGGER.info("Interrupted while waiting for clientRefresherScheduler to shutdown"); - } - } while (!clientRefresherScheduler.isTerminated()); - - client.close(); - } - - private static KeyStore loadKeyStoreFromFileWithProvider(Path path, char[] password, String keyStoreType, String provider) - throws IOException, GeneralSecurityException { - KeyStore keyStore = KeyStore.getInstance(keyStoreType, provider); - try (FileInputStream in = new FileInputStream(path.toFile())) { - keyStore.load(in, password); - } - return keyStore; - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/HttpException.java deleted file mode 100644 index 55d3ecc4e60..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/HttpException.java +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.util; - -import javax.ws.rs.core.Response; -import java.util.Optional; - -@SuppressWarnings("serial") -public class HttpException extends RuntimeException { - public static class NotFoundException extends HttpException { - private static final long serialVersionUID = 4791511887L; - public NotFoundException(String message) { - super(Response.Status.NOT_FOUND, message); - } - } - - /** - * Returns empty on success. - * Returns an exception if the error is retriable. - * Throws an exception on a non-retriable error, like 404 Not Found. - */ - static Optional handleStatusCode(int statusCode, String message) { - Response.Status status = Response.Status.fromStatusCode(statusCode); - if (status == null) { - return Optional.of(new HttpException(statusCode, message)); - } - - switch (status.getFamily()) { - case SUCCESSFUL: return Optional.empty(); - case CLIENT_ERROR: - switch (status) { - case NOT_FOUND: - throw new NotFoundException(message); - case CONFLICT: - // A response body is assumed to be present, and - // will later be interpreted as an error. - return Optional.empty(); - } - throw new HttpException(statusCode, message); - } - - // Other errors like server-side errors are assumed to be retryable. - return Optional.of(new HttpException(status, message)); - } - - private HttpException(int statusCode, String message) { - super("HTTP status code " + statusCode + ": " + message); - } - - private HttpException(Response.Status status, String message) { - super(status.toString() + " (" + status.getStatusCode() + "): " + message); - } -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SelfCloseableHttpClient.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SelfCloseableHttpClient.java deleted file mode 100644 index 8e516729aff..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SelfCloseableHttpClient.java +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.util; - -import com.yahoo.log.LogLevel; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -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.SSLConnectionSocketFactory; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; - -import java.io.IOException; -import java.util.logging.Logger; - -/** - * @author freva - */ -class SelfCloseableHttpClient implements AutoCloseable { - - private static final Logger log = Logger.getLogger(SelfCloseableHttpClient.class.getName()); - - private final CloseableHttpClient httpClient; - - SelfCloseableHttpClient() { - this(SSLConnectionSocketFactory.getSocketFactory()); - } - - SelfCloseableHttpClient(SSLConnectionSocketFactory sslConnectionSocketFactory) { - Registry socketFactoryRegistry = RegistryBuilder.create() - .register("http", PlainConnectionSocketFactory.getSocketFactory()) - .register("https", sslConnectionSocketFactory) - .build(); - - PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(socketFactoryRegistry); - 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(); - - this.httpClient = HttpClientBuilder.create() - .setDefaultRequestConfig(requestBuilder) - .disableAutomaticRetries() - .setUserAgent("node-admin") - .setConnectionManager(cm).build(); - } - - public CloseableHttpResponse execute(HttpUriRequest request) throws IOException { - return httpClient.execute(request); - } - - @Override - public void finalize() throws Throwable { - close(); - super.finalize(); - } - - @Override - public void close() { - try { - httpClient.close(); - } catch (Exception e) { - log.log(LogLevel.WARNING, "Ignoring exception thrown when closing http client", e); - } - } -} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java new file mode 100644 index 00000000000..f39a64d2dee --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java @@ -0,0 +1,164 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.configserver; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +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.message.BasicStatusLine; +import org.junit.Before; +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.junit.MatcherAssert.assertThat; +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.mock; +import static org.mockito.Mockito.when; + +/** + * Basic testing of retry logic. + * + * @author dybis + */ +public class ConfigServerApiImplTest { + + @JsonIgnoreProperties(ignoreUnknown = true) + public static class TestPojo { + @JsonProperty("foo") + String foo; + @JsonProperty("error-code") + Integer errorCode; + } + + private final String uri1 = "http://host1:666"; + private final String uri2 = "http://host2:666"; + private final List configServers = Arrays.asList(URI.create(uri1), URI.create(uri2)); + private final StringBuilder mockLog = new StringBuilder(); + + private ConfigServerApiImpl executor; + private int mockReturnCode = 200; + + @Before + public void initExecutor() throws IOException { + 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(" "); + if (mockReturnCode == 100000) throw new RuntimeException("FAIL"); + + BasicStatusLine statusLine = new BasicStatusLine(HttpVersion.HTTP_1_1, mockReturnCode, null); + BasicHttpEntity entity = new BasicHttpEntity(); + String returnMessage = "{\"foo\":\"bar\", \"no\":3, \"error-code\": " + mockReturnCode + "}"; + InputStream stream = new ByteArrayInputStream(returnMessage.getBytes(StandardCharsets.UTF_8)); + entity.setContent(stream); + + CloseableHttpResponse response = mock(CloseableHttpResponse.class); + when(response.getEntity()).thenReturn(entity); + when(response.getStatusLine()).thenReturn(statusLine); + + return response; + }); + executor = new ConfigServerApiImpl(configServers, httpMock); + } + + @Test + public void testBasicParsingSingleServer() throws Exception { + TestPojo answer = executor.get("/path", TestPojo.class); + assertThat(answer.foo, is("bar")); + assertLogStringContainsGETForAHost(); + } + + @Test(expected = HttpException.class) + public void testBasicFailure() throws Exception { + // Server is returning 400, no retries. + mockReturnCode = 400; + + TestPojo testPojo = executor.get("/path", TestPojo.class); + assertEquals(testPojo.errorCode.intValue(), mockReturnCode); + assertLogStringContainsGETForAHost(); + } + + @Test + public void testBasicSuccessWithNoRetries() throws Exception { + // Server is returning 201, no retries. + mockReturnCode = 201; + + TestPojo testPojo = executor.get("/path", TestPojo.class); + assertEquals(testPojo.errorCode.intValue(), mockReturnCode); + assertLogStringContainsGETForAHost(); + } + + @Test + public void testRetries() throws Exception { + // Client is throwing exception, should be retries. + mockReturnCode = 100000; + try { + executor.get("/path", TestPojo.class); + fail("Expected failure"); + } catch (Exception e) { + // ignore + } + + String[] log = mockLog.toString().split(" "); + assertThat(log, arrayContainingInAnyOrder("GET http://host1:666/path", "GET http://host2:666/path")); + } + + @Test + public void testRetriesOnBadHttpResponseCode() throws Exception { + // Client is throwing exception, should be retries. + mockReturnCode = 503; + try { + executor.get("/path", TestPojo.class); + fail("Expected failure"); + } catch (Exception e) { + // ignore + } + + String[] log = mockLog.toString().split(" "); + assertThat(log, arrayContainingInAnyOrder( + "GET http://host1:666/path", "GET http://host2:666/path")); + } + + @Test + public void testNotFound() throws Exception { + // Server is returning 404, special exception is thrown. + mockReturnCode = 404; + try { + executor.get("/path", TestPojo.class); + fail("Expected exception"); + } catch (HttpException.NotFoundException e) { + // ignore + } + assertLogStringContainsGETForAHost(); + } + + @Test + public void testConflict() throws Exception { + // Server is returning 409, no exception is thrown. + mockReturnCode = 409; + executor.get("/path", TestPojo.class); + assertLogStringContainsGETForAHost(); + } + + private void assertLogStringContainsGETForAHost() { + String logString = mockLog.toString(); + //assertThat(logString, startsWith("GET http://host")); + //assertThat(logString, endsWith(":666/path ")); + assertTrue("log does not contain expected entries:" + logString, + (logString.equals("GET http://host1:666/path ") || logString.equals("GET http://host2:666/path "))); + } +} 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 deleted file mode 100644 index 175d3a9a051..00000000000 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java +++ /dev/null @@ -1,164 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.util; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonProperty; -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.message.BasicStatusLine; -import org.junit.Before; -import org.junit.Test; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.URI; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.List; - -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.Matchers.arrayContainingInAnyOrder; -import static org.hamcrest.junit.MatcherAssert.assertThat; -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.mock; -import static org.mockito.Mockito.when; - -/** - * Basic testing of retry logic. - * - * @author dybis - */ -public class ConfigServerHttpRequestExecutorTest { - - @JsonIgnoreProperties(ignoreUnknown = true) - public static class TestPojo { - @JsonProperty("foo") - String foo; - @JsonProperty("error-code") - Integer errorCode; - } - - private final String uri1 = "http://host1:666"; - private final String uri2 = "http://host2:666"; - private final List configServers = Arrays.asList(URI.create(uri1), URI.create(uri2)); - private final StringBuilder mockLog = new StringBuilder(); - - private ConfigServerHttpRequestExecutor executor; - private int mockReturnCode = 200; - - @Before - public void initExecutor() throws IOException { - 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(" "); - if (mockReturnCode == 100000) throw new RuntimeException("FAIL"); - - BasicStatusLine statusLine = new BasicStatusLine(HttpVersion.HTTP_1_1, mockReturnCode, null); - BasicHttpEntity entity = new BasicHttpEntity(); - String returnMessage = "{\"foo\":\"bar\", \"no\":3, \"error-code\": " + mockReturnCode + "}"; - InputStream stream = new ByteArrayInputStream(returnMessage.getBytes(StandardCharsets.UTF_8)); - entity.setContent(stream); - - CloseableHttpResponse response = mock(CloseableHttpResponse.class); - when(response.getEntity()).thenReturn(entity); - when(response.getStatusLine()).thenReturn(statusLine); - - return response; - }); - executor = new ConfigServerHttpRequestExecutor(configServers, httpMock); - } - - @Test - public void testBasicParsingSingleServer() throws Exception { - TestPojo answer = executor.get("/path", TestPojo.class); - assertThat(answer.foo, is("bar")); - assertLogStringContainsGETForAHost(); - } - - @Test(expected = HttpException.class) - public void testBasicFailure() throws Exception { - // Server is returning 400, no retries. - mockReturnCode = 400; - - TestPojo testPojo = executor.get("/path", TestPojo.class); - assertEquals(testPojo.errorCode.intValue(), mockReturnCode); - assertLogStringContainsGETForAHost(); - } - - @Test - public void testBasicSuccessWithNoRetries() throws Exception { - // Server is returning 201, no retries. - mockReturnCode = 201; - - TestPojo testPojo = executor.get("/path", TestPojo.class); - assertEquals(testPojo.errorCode.intValue(), mockReturnCode); - assertLogStringContainsGETForAHost(); - } - - @Test - public void testRetries() throws Exception { - // Client is throwing exception, should be retries. - mockReturnCode = 100000; - try { - executor.get("/path", TestPojo.class); - fail("Expected failure"); - } catch (Exception e) { - // ignore - } - - String[] log = mockLog.toString().split(" "); - assertThat(log, arrayContainingInAnyOrder("GET http://host1:666/path", "GET http://host2:666/path")); - } - - @Test - public void testRetriesOnBadHttpResponseCode() throws Exception { - // Client is throwing exception, should be retries. - mockReturnCode = 503; - try { - executor.get("/path", TestPojo.class); - fail("Expected failure"); - } catch (Exception e) { - // ignore - } - - String[] log = mockLog.toString().split(" "); - assertThat(log, arrayContainingInAnyOrder( - "GET http://host1:666/path", "GET http://host2:666/path")); - } - - @Test - public void testNotFound() throws Exception { - // Server is returning 404, special exception is thrown. - mockReturnCode = 404; - try { - executor.get("/path", TestPojo.class); - fail("Expected exception"); - } catch (HttpException.NotFoundException e) { - // ignore - } - assertLogStringContainsGETForAHost(); - } - - @Test - public void testConflict() throws Exception { - // Server is returning 409, no exception is thrown. - mockReturnCode = 409; - executor.get("/path", TestPojo.class); - assertLogStringContainsGETForAHost(); - } - - private void assertLogStringContainsGETForAHost() { - String logString = mockLog.toString(); - //assertThat(logString, startsWith("GET http://host")); - //assertThat(logString, endsWith(":666/path ")); - assertTrue("log does not contain expected entries:" + logString, - (logString.equals("GET http://host1:666/path ") || logString.equals("GET http://host2:666/path "))); - } -} -- cgit v1.2.3