diff options
author | Harald Musum <musum@oath.com> | 2018-06-04 20:48:42 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-06-04 20:48:42 +0200 |
commit | 053183610c2d91cc064f6a6ddddd5057cf52cd70 (patch) | |
tree | 569f248e8548f9e748b566f2829839e096c977d8 | |
parent | 185c19a44c0288c37e9075c35a87d9f0d816d66a (diff) |
Revert "Add converged field to service list response"
5 files changed, 70 insertions, 160 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 766229a45af..a80ecffd94a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -110,15 +110,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye public ApplicationRepository(TenantRepository tenantRepository, Provisioner hostProvisioner, Clock clock) { - this(tenantRepository, new ApplicationConvergenceChecker(), hostProvisioner, clock); - } - - public ApplicationRepository(TenantRepository tenantRepository, - ApplicationConvergenceChecker convergenceChecker, - Provisioner hostProvisioner, - Clock clock) { this(tenantRepository, Optional.of(hostProvisioner), - convergenceChecker, new HttpProxy(new SimpleHttpFetcher()), + new ApplicationConvergenceChecker(), new HttpProxy(new SimpleHttpFetcher()), new ConfigserverConfig(new ConfigserverConfig.Builder()), clock, new FileDistributionStatus()); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceChecker.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceChecker.java index 9d7ff60f43a..22a5450b184 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceChecker.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceChecker.java @@ -17,14 +17,9 @@ import javax.ws.rs.ProcessingException; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.client.WebTarget; +import java.io.IOException; import java.net.URI; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Comparator; -import java.util.HashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; +import java.util.*; /** * Checks for convergence of config generation for a given application. @@ -33,9 +28,11 @@ import java.util.Set; * @author hmusum */ public class ApplicationConvergenceChecker extends AbstractComponent { - private static final String statePath = "/state/v1/"; private static final String configSubPath = "config"; + private final StateApiFactory stateApiFactory; + private final Client client = ClientBuilder.newClient(); + private final static Set<String> serviceTypesToCheck = new HashSet<>(Arrays.asList( "container", "qrserver", @@ -45,9 +42,6 @@ public class ApplicationConvergenceChecker extends AbstractComponent { "distributor" )); - private final StateApiFactory stateApiFactory; - private final Client client = ClientBuilder.newClient(); - @Inject public ApplicationConvergenceChecker() { this(ApplicationConvergenceChecker::createStateApi); @@ -63,15 +57,7 @@ public class ApplicationConvergenceChecker extends AbstractComponent { .forEach(host -> host.getServices().stream() .filter(service -> serviceTypesToCheck.contains(service.getServiceType())) .forEach(service -> getStatePort(service).ifPresent(port -> servicesToCheck.add(service)))); - - long currentGeneration = servicesToCheck.stream() - .map(s -> "http://" + s.getHostName() + ":" + getStatePort(s).get()) - .map(URI::create) - .map(this::getServiceGeneration) - .min(Comparator.naturalOrder()) - .orElse(0L); - return new ServiceListResponse(200, servicesToCheck, uri, application.getApplicationGeneration(), - currentGeneration); + return new ServiceListResponse(200, servicesToCheck, uri, application.getApplicationGeneration()); } public ServiceResponse serviceConvergenceCheck(Application application, String hostAndPortToCheck, URI uri) { @@ -127,7 +113,7 @@ public class ApplicationConvergenceChecker extends AbstractComponent { return generationFromContainerState(state.config()); } - private boolean hostInApplication(Application application, String hostPort) { + private boolean hostInApplication(Application application, String hostPort) throws IOException { for (HostInfo host : application.getModel().getHosts()) { if (hostPort.startsWith(host.getHostname())) { for (ServiceInfo service : host.getServices()) { @@ -146,8 +132,7 @@ public class ApplicationConvergenceChecker extends AbstractComponent { final Cursor debug; // Pre-condition: servicesToCheck has a state port - private ServiceListResponse(int status, List<ServiceInfo> servicesToCheck, URI uri, long wantedGeneration, - long currentGeneration) { + private ServiceListResponse(int status, List<ServiceInfo> servicesToCheck, URI uri, Long wantedGeneration) { super(status); Cursor serviceArray = object.setArray("services"); for (ServiceInfo s : servicesToCheck) { @@ -160,9 +145,7 @@ public class ApplicationConvergenceChecker extends AbstractComponent { service.setString("url", uri.toString() + "/" + hostName + ":" + statePort); } object.setString("url", uri.toString()); - object.setLong("currentGeneration", currentGeneration); object.setLong("wantedGeneration", wantedGeneration); - object.setBool("converged", currentGeneration >= wantedGeneration); // TODO: Remove debug when clients are not using it anymore debug = object.setObject("debug"); debug.setLong("wantedGeneration", wantedGeneration); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java index 399169c122a..3c3edae7914 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ApplicationConvergenceCheckerTest.java @@ -17,29 +17,25 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.xml.sax.SAXException; import java.io.IOException; -import java.io.UncheckedIOException; import java.net.URI; -import java.util.Arrays; -import java.util.HashMap; -import java.util.Map; -import static com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker.ServiceResponse; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static com.yahoo.vespa.config.server.application.ApplicationConvergenceChecker.ServiceResponse; /** * @author Ulf Lilleengen */ public class ApplicationConvergenceCheckerTest { - private static final ObjectMapper mapper = new ObjectMapper(); - - private final TenantName tenant = TenantName.from("mytenant"); - private final ApplicationId appId = ApplicationId.from(tenant, ApplicationName.from("myapp"), InstanceName.from("myinstance")); + private TenantName tenant = TenantName.from("mytenant"); + private ApplicationId appId = ApplicationId.from(tenant, ApplicationName.from("myapp"), InstanceName.from("myinstance")); + private ObjectMapper mapper = new ObjectMapper(); private Application application; - private Map<URI, Long> currentGeneration; - private ApplicationConvergenceChecker checker; @Rule public TemporaryFolder folder = new TemporaryFolder(); @@ -53,122 +49,65 @@ public class ApplicationConvergenceCheckerTest { false, Version.fromIntValues(0, 0, 0), MetricUpdater.createTestUpdater(), appId); - currentGeneration = new HashMap<>(); - checker = new ApplicationConvergenceChecker( - (client, serviceUri) -> () -> asJson("{\"config\":{\"generation\":" - + currentGeneration.getOrDefault(serviceUri, 3L) - + "}}")); } @Test - public void service_convergence() throws Exception { + public void converge() throws IOException { + ApplicationConvergenceChecker checker = new ApplicationConvergenceChecker( + (client, serviceUri) -> () -> string2json("{\"config\":{\"generation\":3}}")); + HttpResponse serviceListResponse = checker.serviceListToCheckForConfigConvergence(application, + URI.create("http://foo:234/serviceconverge")); + assertThat(serviceListResponse.getStatus(), is(200)); + assertEquals(//"\"wantedGeneration\":3," + + // "\"debug\":{\"wantedGeneration\":3}," + + "{\"services\":[" + + "{" + + "\"host\":\"localhost\"," + + "\"port\":1337," + + "\"type\":\"container\"," + + "\"url\":\"http://foo:234/serviceconverge/localhost:1337\"}]," + + "\"url\":\"http://foo:234/serviceconverge\"," + + "\"wantedGeneration\":3," + + "\"debug\":{\"wantedGeneration\":3}}", + SessionHandlerTest.getRenderedString(serviceListResponse)); + ServiceResponse serviceResponse = checker.serviceConvergenceCheck(application, "localhost:1337", URI.create("http://foo:234/serviceconverge/localhost:1337")); - assertEquals(200, serviceResponse.getStatus()); - assertJsonEquals("{\n" + - " \"url\": \"http://foo:234/serviceconverge/localhost:1337\",\n" + - " \"host\": \"localhost:1337\",\n" + - " \"wantedGeneration\": 3,\n" + - " \"debug\": {\n" + - " \"host\": \"localhost:1337\",\n" + - " \"wantedGeneration\": 3,\n" + - " \"currentGeneration\": 3\n" + - " },\n" + - " \"converged\": true,\n" + - " \"currentGeneration\": 3\n" + - "}", - SessionHandlerTest.getRenderedString(serviceResponse)); + assertThat(serviceResponse.getStatus(), is(200)); + assertEquals("{" + + "\"url\":\"http://foo:234/serviceconverge/localhost:1337\"," + + "\"host\":\"localhost:1337\"," + + "\"wantedGeneration\":3," + + "\"debug\":{" + + "\"host\":\"localhost:1337\"," + + "\"wantedGeneration\":3," + + "\"currentGeneration\":3}," + + "\"converged\":true," + + "\"currentGeneration\":3}", + SessionHandlerTest.getRenderedString(serviceResponse)); ServiceResponse hostMissingResponse = checker.serviceConvergenceCheck(application, "notPresent:1337", URI.create("http://foo:234/serviceconverge/notPresent:1337")); - assertEquals(410, hostMissingResponse.getStatus()); - assertJsonEquals("{\n" + - " \"url\": \"http://foo:234/serviceconverge/notPresent:1337\",\n" + - " \"host\": \"notPresent:1337\",\n" + - " \"wantedGeneration\": 3,\n" + - " \"debug\": {\n" + - " \"host\": \"notPresent:1337\",\n" + - " \"wantedGeneration\": 3,\n" + - " \"problem\": \"Host:port (service) no longer part of application, refetch list of services.\"\n" + - " },\n" + - " \"problem\": \"Host:port (service) no longer part of application, refetch list of services.\"\n" + - "}", + assertThat(hostMissingResponse.getStatus(), is(410)); + assertEquals("{" + + "\"url\":\"http://foo:234/serviceconverge/notPresent:1337\"," + + "\"host\":\"notPresent:1337\"," + + "\"wantedGeneration\":3," + + "\"debug\":{" + + "\"host\":\"notPresent:1337\"," + + "\"wantedGeneration\":3," + + "\"problem\":\"Host:port (service) no longer part of application, refetch list of services.\"}," + + "\"problem\":\"Host:port (service) no longer part of application, refetch list of services.\"}", SessionHandlerTest.getRenderedString(hostMissingResponse)); } - @Test - public void service_list_convergence() throws Exception { - HttpResponse serviceListResponse = checker.serviceListToCheckForConfigConvergence(application, - URI.create("http://foo:234/serviceconverge")); - assertEquals(200, serviceListResponse.getStatus()); - assertJsonEquals("{\n" + - " \"services\": [\n" + - " {\n" + - " \"host\": \"localhost\",\n" + - " \"port\": 1337,\n" + - " \"type\": \"container\",\n" + - " \"url\": \"http://foo:234/serviceconverge/localhost:1337\"\n" + - " }\n" + - " ],\n" + - " \"url\": \"http://foo:234/serviceconverge\",\n" + - " \"currentGeneration\": 3,\n" + - " \"wantedGeneration\": 3,\n" + - " \"converged\": true,\n" + - " \"debug\": {\n" + - " \"wantedGeneration\": 3\n" + - " }\n" + - "}", - SessionHandlerTest.getRenderedString(serviceListResponse)); - - // Model with two hosts on different generations - MockModel model = new MockModel(Arrays.asList( - MockModel.createContainerHost("host1", 1234), - MockModel.createContainerHost("host2", 1234)) - ); - Application application = new Application(model, new ServerCache(), 4, - false, - Version.fromIntValues(0, 0, 0), - MetricUpdater.createTestUpdater(), appId); - currentGeneration.put(URI.create("http://host2:1234"), 4L); - serviceListResponse = checker.serviceListToCheckForConfigConvergence(application, URI.create("http://foo:234/serviceconverge")); - assertEquals(200, serviceListResponse.getStatus()); - assertJsonEquals("{\n" + - " \"services\": [\n" + - " {\n" + - " \"host\": \"host1\",\n" + - " \"port\": 1234,\n" + - " \"type\": \"container\",\n" + - " \"url\": \"http://foo:234/serviceconverge/host1:1234\"\n" + - " },\n" + - " {\n" + - " \"host\": \"host2\",\n" + - " \"port\": 1234,\n" + - " \"type\": \"container\",\n" + - " \"url\": \"http://foo:234/serviceconverge/host2:1234\"\n" + - " }\n" + - " ],\n" + - " \"url\": \"http://foo:234/serviceconverge\",\n" + - " \"currentGeneration\": 3,\n" + - " \"wantedGeneration\": 4,\n" + - " \"converged\": false,\n" + - " \"debug\": {\n" + - " \"wantedGeneration\": 4\n" + - " }\n" + - "}", - SessionHandlerTest.getRenderedString(serviceListResponse)); - } - - private static void assertJsonEquals(String expected, String actual) { - assertEquals(asJson(expected), asJson(actual)); - } - - private static JsonNode asJson(String data) { + private JsonNode string2json(String data) { try { return mapper.readTree(data); } catch (IOException e) { - throw new UncheckedIOException(e); + throw new RuntimeException(e); } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java index dcc60de0a3a..65727a8b989 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MockModel.java @@ -23,24 +23,17 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -/** - * Model with two services, one that does not have a state port - * - * @author hakonhall - */ -public class MockModel implements Model { +// Model with two services, one that does not have a state port +class MockModel implements Model { private final Collection<HostInfo> hosts; static MockModel createContainer(String hostname, int statePort) { - return new MockModel(Collections.singleton(createContainerHost(hostname, statePort))); - } - - static HostInfo createContainerHost(String hostname, int statePort) { ServiceInfo container = createServiceInfo(hostname, "container", "container", - ClusterSpec.Type.container, statePort, "state"); + ClusterSpec.Type.container, statePort, "state"); ServiceInfo serviceNoStatePort = createServiceInfo(hostname, "logserver", "logserver", - ClusterSpec.Type.admin, 1234, "logtp"); - return new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort)); + ClusterSpec.Type.admin, 1234, "logtp"); + HostInfo hostInfo = new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort)); + return new MockModel(Collections.singleton(hostInfo)); } static MockModel createClusterController(String hostname, int statePort) { @@ -58,6 +51,10 @@ public class MockModel implements Model { return new MockModel(Collections.singleton(hostInfo)); } + static MockModel createConfigProxy(String hostname, int rpcPort) { + return createConfigProxies(Collections.singletonList(hostname), rpcPort); + } + static MockModel createConfigProxies(List<String> hostnames, int rpcPort) { Set<HostInfo> hostInfos = new HashSet<>(); hostnames.forEach(hostname -> { 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 2d39efb9013..ce84cf4c280 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 @@ -68,9 +68,7 @@ public class ApplicationHandlerTest { tenantRepository.addTenant(TenantBuilder.create(componentRegistry, mytenantName)); tenantRepository.addTenant(TenantBuilder.create(componentRegistry, foobar)); provisioner = new SessionHandlerTest.MockProvisioner(); - applicationRepository = new ApplicationRepository(tenantRepository, - new ApplicationConvergenceChecker(stateApiFactory), - provisioner, Clock.systemUTC()); + applicationRepository = new ApplicationRepository(tenantRepository, provisioner, Clock.systemUTC()); listApplicationsHandler = new ListApplicationsHandler(ListApplicationsHandler.testOnlyContext(), tenantRepository, Zone.defaultZone()); |