diff options
4 files changed, 105 insertions, 92 deletions
diff --git a/configserver/pom.xml b/configserver/pom.xml index 8cd1b4b4254..43187038f97 100644 --- a/configserver/pom.xml +++ b/configserver/pom.xml @@ -220,6 +220,11 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.assertj</groupId> + <artifactId>assertj-core</artifactId> + <scope>test</scope> + </dependency> + <dependency> <groupId>com.yahoo.vespa</groupId> <artifactId>http-utils</artifactId> <version>${project.version}</version> diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java index 6b316c06b54..e89b9685a06 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java @@ -1,27 +1,28 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.application; -import ai.vespa.util.http.VespaClientBuilderFactory; +import ai.vespa.util.http.VespaHttpClientBuilder; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.config.model.api.HostInfo; import com.yahoo.config.model.api.PortInfo; import com.yahoo.config.model.api.ServiceInfo; -import java.util.logging.Level; +import com.yahoo.log.LogLevel; import com.yahoo.slime.Cursor; import com.yahoo.vespa.config.server.http.JSONResponse; -import org.glassfish.jersey.client.ClientProperties; -import org.glassfish.jersey.client.proxy.WebResourceFactory; - -import javax.ws.rs.GET; -import javax.ws.rs.Path; -import javax.ws.rs.ProcessingException; -import javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientRequestFilter; -import javax.ws.rs.client.WebTarget; -import javax.ws.rs.core.HttpHeaders; +import org.apache.http.HttpStatus; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.utils.URIBuilder; +import org.apache.http.impl.client.CloseableHttpClient; + +import java.io.IOException; +import java.io.UncheckedIOException; import java.net.URI; +import java.net.URISyntaxException; import java.time.Duration; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -29,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -42,12 +44,11 @@ import static com.yahoo.config.model.api.container.ContainerServiceType.QRSERVER * * @author Ulf Lilleengen * @author hmusum + * @author bjorncs */ public class ConfigConvergenceChecker extends AbstractComponent { private static final Logger log = Logger.getLogger(ConfigConvergenceChecker.class.getName()); - private static final String statePath = "/state/v1/"; - private static final String configSubPath = "config"; private final static Set<String> serviceTypesToCheck = Set.of( CONTAINER.serviceName, QRSERVER.serviceName, @@ -58,16 +59,12 @@ public class ConfigConvergenceChecker extends AbstractComponent { "distributor" ); - private final StateApiFactory stateApiFactory; - private final VespaClientBuilderFactory clientBuilderFactory = new VespaClientBuilderFactory(); + private final CloseableHttpClient httpClient; + private final ObjectMapper jsonMapper = new ObjectMapper(); @Inject public ConfigConvergenceChecker() { - this(ConfigConvergenceChecker::createStateApi); - } - - public ConfigConvergenceChecker(StateApiFactory stateApiFactory) { - this.stateApiFactory = stateApiFactory; + this.httpClient = createHttpClient(); } /** Fetches the active config generation for all services in the given application. */ @@ -99,7 +96,7 @@ public class ConfigConvergenceChecker extends AbstractComponent { long currentGeneration = getServiceGeneration(URI.create("http://" + hostAndPortToCheck), timeout); boolean converged = currentGeneration >= wantedGeneration; return ServiceResponse.createOkResponse(requestUrl, hostAndPortToCheck, wantedGeneration, currentGeneration, converged); - } catch (ProcessingException e) { // e.g. if we cannot connect to the service to find generation + } catch (NonSuccessStatusCodeException | IOException e) { // e.g. if we cannot connect to the service to find generation return ServiceResponse.createNotFoundResponse(requestUrl, hostAndPortToCheck, wantedGeneration, e.getMessage()); } catch (Exception e) { return ServiceResponse.createErrorResponse(requestUrl, hostAndPortToCheck, wantedGeneration, e.getMessage()); @@ -108,46 +105,47 @@ public class ConfigConvergenceChecker extends AbstractComponent { @Override public void deconstruct() { - clientBuilderFactory.close(); - } - - @Path(statePath) - public interface StateApi { - @Path(configSubPath) - @GET - JsonNode config(); - } - - public interface StateApiFactory { - StateApi createStateApi(Client client, URI serviceUri); + try { + httpClient.close(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } } /** Gets service generation for a list of services (in parallel). */ private Map<ServiceInfo, Long> getServiceGenerations(List<ServiceInfo> services, Duration timeout) { return services.parallelStream() - .collect(Collectors.toMap(service -> service, - service -> { - try { - return getServiceGeneration(URI.create("http://" + service.getHostName() - + ":" + getStatePort(service).get()), timeout); - } - catch (ProcessingException e) { // Cannot connect to service to determine service generation - return -1L; - } - }, - (v1, v2) -> { throw new IllegalStateException("Duplicate keys for values '" + v1 + "' and '" + v2 + "'."); }, - LinkedHashMap::new - )); + .collect(Collectors.toMap( + service -> service, + service -> { + try { + return getServiceGeneration(URI.create("http://" + service.getHostName() + + ":" + getStatePort(service).get()), timeout); + } catch (IOException | NonSuccessStatusCodeException e) { + return -1L; + } + }, + (v1, v2) -> { throw new IllegalStateException("Duplicate keys for values '" + v1 + "' and '" + v2 + "'."); }, + LinkedHashMap::new + )); } /** Get service generation of service at given URL */ - private long getServiceGeneration(URI serviceUrl, Duration timeout) { - Client client = createClient(timeout); - try { - StateApi state = stateApiFactory.createStateApi(client, serviceUrl); - return generationFromContainerState(state.config()); - } finally { - client.close(); + private long getServiceGeneration(URI serviceUrl, Duration timeout) throws IOException, NonSuccessStatusCodeException { + HttpGet request = new HttpGet(createApiUri(serviceUrl)); + request.setConfig(createRequestConfig(timeout)); + try (CloseableHttpResponse response = httpClient.execute(request)) { + int statusCode = response.getStatusLine().getStatusCode(); + if (statusCode != HttpStatus.SC_OK) throw new NonSuccessStatusCodeException(statusCode); + if (response.getEntity() == null) throw new IOException("Response has no content"); + JsonNode jsonContent = jsonMapper.readTree(response.getEntity().getContent()); + return generationFromContainerState(jsonContent); + } catch (Exception e) { + log.log( + LogLevel.DEBUG, + e, + () -> String.format("Failed to retrieve service config generation for '%s': %s", serviceUrl, e.getMessage())); + throw e; } } @@ -166,16 +164,6 @@ public class ConfigConvergenceChecker extends AbstractComponent { return false; } - private Client createClient(Duration timeout) { - return clientBuilderFactory.newBuilder() - .register( - (ClientRequestFilter) ctx -> - ctx.getHeaders().put(HttpHeaders.USER_AGENT, List.of("config-convergence-checker"))) - .property(ClientProperties.CONNECT_TIMEOUT, (int) timeout.toMillis()) - .property(ClientProperties.READ_TIMEOUT, (int) timeout.toMillis()) - .build(); - } - private static Optional<Integer> getStatePort(ServiceInfo service) { return service.getPorts().stream() .filter(port -> port.getTags().contains("state")) @@ -187,9 +175,43 @@ public class ConfigConvergenceChecker extends AbstractComponent { return state.get("config").get("generation").asLong(-1); } - private static StateApi createStateApi(Client client, URI uri) { - WebTarget target = client.target(uri); - return WebResourceFactory.newResource(StateApi.class, target); + private static URI createApiUri(URI serviceUrl) { + try { + return new URIBuilder(serviceUrl) + .setPath("/state/v1/config") + .build(); + } catch (URISyntaxException e) { + throw new RuntimeException(e); + } + } + + private static CloseableHttpClient createHttpClient() { + return VespaHttpClientBuilder + .create() + .setUserAgent("config-convergence-checker") + .setConnectionTimeToLive(20, TimeUnit.SECONDS) + .setMaxConnPerRoute(4) + .setMaxConnTotal(100) + .setDefaultRequestConfig(createRequestConfig(Duration.ofSeconds(10))) + .build(); + } + + private static RequestConfig createRequestConfig(Duration timeout) { + int timeoutMillis = (int)timeout.toMillis(); + return RequestConfig.custom() + .setConnectionRequestTimeout(timeoutMillis) + .setConnectTimeout(timeoutMillis) + .setSocketTimeout(timeoutMillis) + .build(); + } + + private static class NonSuccessStatusCodeException extends Exception { + final int statusCode; + + NonSuccessStatusCodeException(int statusCode) { + super("Expected status code 200, got " + statusCode); + this.statusCode = statusCode; + } } private static class ServiceListResponse extends JSONResponse { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java index 6aeb774d2b0..1630e965771 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java @@ -2,12 +2,12 @@ package com.yahoo.vespa.config.server.application; import com.github.tomakehurst.wiremock.junit.WireMockRule; +import com.yahoo.component.Version; import com.yahoo.config.model.api.Model; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; -import com.yahoo.component.Version; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; @@ -31,8 +31,8 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.okJson; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen @@ -184,10 +184,15 @@ public class ConfigConvergenceCheckerTest { .withBody("response too slow"))); HttpResponse response = checker.getServiceConfigGenerationResponse(application, hostAndPort(service), requestUrl, Duration.ofMillis(1)); // Message contained in a SocketTimeoutException may differ across platforms, so we do a partial match of the response here - assertResponse((responseBody) -> assertTrue("Response matches", responseBody.startsWith( - "{\"url\":\"" + requestUrl.toString() + "\",\"host\":\"" + hostAndPort(requestUrl) + - "\",\"wantedGeneration\":3,\"error\":\"java.net.SocketTimeoutException") && - responseBody.endsWith("\"}")), 404, response); + assertResponse( + responseBody -> + assertThat(responseBody) + .startsWith("{\"url\":\"" + requestUrl.toString() + "\",\"host\":\"" + hostAndPort(requestUrl) + + "\",\"wantedGeneration\":3,\"error\":\"") + .contains("timed out") + .endsWith("\"}"), + 404, + response); } private URI testServer() { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java index 7276091fed0..06a455954ac 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java @@ -1,7 +1,6 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http.v2; -import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; import com.yahoo.config.model.api.ModelFactory; @@ -18,7 +17,6 @@ import com.yahoo.vespa.config.server.MockLogRetriever; import com.yahoo.vespa.config.server.MockProvisioner; import com.yahoo.vespa.config.server.MockTesterClient; import com.yahoo.vespa.config.server.TestComponentRegistry; -import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; import com.yahoo.vespa.config.server.application.HttpProxy; import com.yahoo.vespa.config.server.application.OrchestratorMock; import com.yahoo.vespa.config.server.deploy.DeployTester; @@ -37,12 +35,10 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import javax.ws.rs.client.Client; import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.List; @@ -425,21 +421,6 @@ public class ApplicationHandlerTest { return createApplicationHandler().handle(HttpRequest.createTestRequest(restartUrl, GET)); } - private static class MockStateApiFactory implements ConfigConvergenceChecker.StateApiFactory { - boolean createdApi = false; - @Override - public ConfigConvergenceChecker.StateApi createStateApi(Client client, URI serviceUri) { - createdApi = true; - return () -> { - try { - return new ObjectMapper().readTree("{\"config\":{\"generation\":1}}"); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; - } - } - private ApplicationHandler createApplicationHandler() { return createApplicationHandler(applicationRepository); } |