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());