Add converged field to service list response

5 files changed, 160 insertions, 70 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 f8e1bcec0c3..1aa7097630e 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,8 +110,15 @@ 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),
-             new ApplicationConvergenceChecker(), new HttpProxy(new SimpleHttpFetcher()),
+             convergenceChecker, 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 22a5450b184..9d7ff60f43a 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,9 +17,14 @@ 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.*;
+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;
 
 /**
  * Checks for convergence of config generation for a given application.
@@ -28,11 +33,9 @@ import java.util.*;
  * @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",
@@ -42,6 +45,9 @@ public class ApplicationConvergenceChecker extends AbstractComponent {
             "distributor"
     ));
 
+    private final StateApiFactory stateApiFactory;
+    private final Client client = ClientBuilder.newClient();
+
     @Inject
     public ApplicationConvergenceChecker() {
         this(ApplicationConvergenceChecker::createStateApi);
@@ -57,7 +63,15 @@ 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))));
-        return new ServiceListResponse(200, servicesToCheck, uri, application.getApplicationGeneration());
+
+        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);
     }
 
     public ServiceResponse serviceConvergenceCheck(Application application, String hostAndPortToCheck, URI uri) {
@@ -113,7 +127,7 @@ public class ApplicationConvergenceChecker extends AbstractComponent {
         return generationFromContainerState(state.config());
     }
 
-    private boolean hostInApplication(Application application, String hostPort) throws IOException {
+    private boolean hostInApplication(Application application, String hostPort) {
         for (HostInfo host : application.getModel().getHosts()) {
             if (hostPort.startsWith(host.getHostname())) {
                 for (ServiceInfo service : host.getServices()) {
@@ -132,7 +146,8 @@ 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) {
+        private ServiceListResponse(int status, List<ServiceInfo> servicesToCheck, URI uri, long wantedGeneration,
+                                    long currentGeneration) {
             super(status);
             Cursor serviceArray = object.setArray("services");
             for (ServiceInfo s : servicesToCheck) {
@@ -145,7 +160,9 @@ 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 3c3edae7914..399169c122a 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,25 +17,29 @@ 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 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;
+import static org.junit.Assert.assertEquals;
 
 /**
  * @author Ulf Lilleengen
  */
 public class ApplicationConvergenceCheckerTest {
 
-    private TenantName tenant = TenantName.from("mytenant");
-    private ApplicationId appId = ApplicationId.from(tenant, ApplicationName.from("myapp"), InstanceName.from("myinstance"));
-    private ObjectMapper mapper = new ObjectMapper();
+    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 Application application;
+    private Map<URI, Long> currentGeneration;
+    private ApplicationConvergenceChecker checker;
 
     @Rule
     public TemporaryFolder folder = new TemporaryFolder();
@@ -49,65 +53,122 @@ 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 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));
-
+    public void service_convergence() throws Exception {
         ServiceResponse serviceResponse = checker.serviceConvergenceCheck(application,
                                                                           "localhost:1337",
                                                                           URI.create("http://foo:234/serviceconverge/localhost:1337"));
-        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));
+        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));
 
         ServiceResponse hostMissingResponse = checker.serviceConvergenceCheck(application,
                                                                               "notPresent:1337",
                                                                               URI.create("http://foo:234/serviceconverge/notPresent:1337"));
-        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.\"}",
+        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" +
+                         "}",
                      SessionHandlerTest.getRenderedString(hostMissingResponse));
     }
 
-    private JsonNode string2json(String data) {
+    @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) {
         try {
             return mapper.readTree(data);
         } catch (IOException e) {
-            throw new RuntimeException(e);
+            throw new UncheckedIOException(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 65727a8b989..dcc60de0a3a 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,17 +23,24 @@ 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
-class MockModel implements Model {
+/**
+ * Model with two services, one that does not have a state port
+ *
+ * @author hakonhall
+ */
+public 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");
-        HostInfo hostInfo = new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort));
-        return new MockModel(Collections.singleton(hostInfo));
+                                                           ClusterSpec.Type.admin, 1234, "logtp");
+        return new HostInfo(hostname, Arrays.asList(container, serviceNoStatePort));
     }
 
     static MockModel createClusterController(String hostname, int statePort) {
@@ -51,10 +58,6 @@ 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 ce84cf4c280..2d39efb9013 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,7 +68,9 @@ public class ApplicationHandlerTest {
         tenantRepository.addTenant(TenantBuilder.create(componentRegistry, mytenantName));
         tenantRepository.addTenant(TenantBuilder.create(componentRegistry, foobar));
         provisioner = new SessionHandlerTest.MockProvisioner();
-        applicationRepository = new ApplicationRepository(tenantRepository, provisioner, Clock.systemUTC());
+        applicationRepository = new ApplicationRepository(tenantRepository,
+                                                          new ApplicationConvergenceChecker(stateApiFactory),
+                                                          provisioner, Clock.systemUTC());
         listApplicationsHandler = new ListApplicationsHandler(ListApplicationsHandler.testOnlyContext(),
                                                               tenantRepository,
                                                               Zone.defaultZone());