Show config generation for all services in response"

2 files changed, 69 insertions, 63 deletions

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 91cb5891e84..0f507624188 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
@@ -23,7 +23,9 @@ import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -66,8 +68,9 @@ public class ConfigConvergenceChecker extends AbstractComponent {
                                         .filter(service -> serviceTypesToCheck.contains(service.getServiceType()))
                                         .forEach(service -> getStatePort(service).ifPresent(port -> servicesToCheck.add(service))));
 
-        long currentGeneration = getServiceGeneration(servicesToCheck, timeoutPerService);
-        return new ServiceListResponse(200, servicesToCheck, requestUrl, application.getApplicationGeneration(),
+        Map<ServiceInfo, Long> currentGenerations = getServiceGenerations(servicesToCheck, timeoutPerService);
+        long currentGeneration = currentGenerations.values().stream().mapToLong(Long::longValue).min().orElse(-1);
+        return new ServiceListResponse(200, currentGenerations, requestUrl, application.getApplicationGeneration(),
                                        currentGeneration);
     }
 
@@ -100,23 +103,21 @@ public class ConfigConvergenceChecker extends AbstractComponent {
     }
 
     /** Get service generation for a list of services. Returns the minimum generation of all services */
-    private long getServiceGeneration(List<ServiceInfo> services, Duration timeout) {
-        List<URI> serviceUris = services.stream()
-                                        .map(s -> "http://" + s.getHostName() + ":" + getStatePort(s).get())
-                                        .map(URI::create)
-                                        .collect(Collectors.toList());
-        long generation = -1;
-        for (URI uri : serviceUris) {
-            try {
-                long serviceGeneration = getServiceGeneration(uri, timeout);
-                if (generation == -1 || serviceGeneration < generation) {
-                    generation = serviceGeneration;
-                }
-            } catch (ProcessingException e) { // Cannot connect to service to determine service generation
-                return -1;
-            }
-        }
-        return generation;
+    private Map<ServiceInfo, Long> getServiceGenerations(List<ServiceInfo> services, Duration timeout) {
+        return services.stream()
+                       .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
+                                                ));
     }
 
     /** Get service generation of service at given URL */
@@ -160,7 +161,7 @@ public class ConfigConvergenceChecker extends AbstractComponent {
     }
 
     private static long generationFromContainerState(JsonNode state) {
-        return state.get("config").get("generation").asLong();
+        return state.get("config").get("generation").asLong(-1);
     }
 
     private static StateApi createStateApi(Client client, URI uri) {
@@ -171,19 +172,20 @@ public class ConfigConvergenceChecker extends AbstractComponent {
     private static class ServiceListResponse extends JSONResponse {
 
         // Pre-condition: servicesToCheck has a state port
-        private ServiceListResponse(int status, List<ServiceInfo> servicesToCheck, URI uri, long wantedGeneration,
+        private ServiceListResponse(int status, Map<ServiceInfo, Long> servicesToCheck, URI uri, long wantedGeneration,
                                     long currentGeneration) {
             super(status);
             Cursor serviceArray = object.setArray("services");
-            for (ServiceInfo s : servicesToCheck) {
-                Cursor service = serviceArray.addObject();
-                String hostName = s.getHostName();
-                int statePort = getStatePort(s).get();
-                service.setString("host", hostName);
-                service.setLong("port", statePort);
-                service.setString("type", s.getServiceType());
-                service.setString("url", uri.toString() + "/" + hostName + ":" + statePort);
-            }
+            servicesToCheck.forEach((service, generation) -> {
+                Cursor serviceObject = serviceArray.addObject();
+                String hostName = service.getHostName();
+                int statePort = getStatePort(service).get();
+                serviceObject.setString("host", hostName);
+                serviceObject.setLong("port", statePort);
+                serviceObject.setString("type", service.getServiceType());
+                serviceObject.setString("url", uri.toString() + "/" + hostName + ":" + statePort);
+                serviceObject.setLong("currentGeneration", generation);
+            });
             object.setString("url", uri.toString());
             object.setLong("currentGeneration", currentGeneration);
             object.setLong("wantedGeneration", wantedGeneration);
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 487e96f17b2..fdea0fced67 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,7 +2,6 @@
 package com.yahoo.vespa.config.server.application;
 
 import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import com.github.tomakehurst.wiremock.stubbing.Scenario;
 import com.yahoo.config.model.api.Model;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
@@ -47,16 +46,20 @@ public class ConfigConvergenceCheckerTest {
     private Application application;
     private ConfigConvergenceChecker checker;
     private URI service;
+    private URI service2;
 
     @Rule
     public TemporaryFolder folder = new TemporaryFolder();
 
     @Rule
     public final WireMockRule wireMock = new WireMockRule(options().dynamicPort(), true);
+    @Rule
+    public final WireMockRule wireMock2 = new WireMockRule(options().dynamicPort(), true);
 
     @Before
     public void setup() {
         service = testServer();
+        service2 = testServer(wireMock2);
         Model mockModel = MockModel.createContainer(service.getHost(), service.getPort());
         application = new Application(mockModel,
                                       new ServerCache(),
@@ -115,7 +118,8 @@ public class ConfigConvergenceCheckerTest {
                            "      \"host\": \"" + serviceUrl.getHost() + "\",\n" +
                            "      \"port\": " + serviceUrl.getPort() + ",\n" +
                            "      \"type\": \"container\",\n" +
-                           "      \"url\": \"" + serviceUrl.toString() + "\"\n" +
+                           "      \"url\": \"" + serviceUrl.toString() + "\",\n" +
+                           "      \"currentGeneration\":" + 3 + "\n" +
                            "    }\n" +
                            "  ],\n" +
                            "  \"url\": \"" + requestUrl.toString() + "\",\n" +
@@ -130,49 +134,45 @@ public class ConfigConvergenceCheckerTest {
 
         { // Model with two hosts on different generations
             MockModel model = new MockModel(Arrays.asList(
-                    // Reuse hostname and port to avoid the need for two WireMock servers
                     MockModel.createContainerHost(service.getHost(), service.getPort()),
-                    MockModel.createContainerHost(service.getHost(), service.getPort()))
+                    MockModel.createContainerHost(service2.getHost(), service2.getPort()))
             );
             Application application = new Application(model, new ServerCache(), 4,
                                                       false,
                                                       Version.fromIntValues(0, 0, 0),
                                                       MetricUpdater.createTestUpdater(), appId);
 
-            String host2 = "host2";
-            wireMock.stubFor(get(urlEqualTo("/state/v1/config")).inScenario("config request")
-                                                                .whenScenarioStateIs(Scenario.STARTED)
-                                                                .willReturn(okJson("{\"config\":{\"generation\":4}}"))
-                                                                .willSetStateTo(host2));
-            wireMock.stubFor(get(urlEqualTo("/state/v1/config")).inScenario("config request")
-                                                                .whenScenarioStateIs(host2)
-                                                                .willReturn(okJson("{\"config\":{\"generation\":3}}")));
+            wireMock.stubFor(get(urlEqualTo("/state/v1/config")).willReturn(okJson("{\"config\":{\"generation\":4}}")));
+            wireMock2.stubFor(get(urlEqualTo("/state/v1/config")).willReturn(okJson("{\"config\":{\"generation\":3}}")));
 
             URI requestUrl = testServer().resolve("/serviceconverge");
             URI serviceUrl = testServer().resolve("/serviceconverge/" + hostAndPort(service));
+            URI serviceUrl2 = testServer().resolve("/serviceconverge/" + hostAndPort(service2));
             HttpResponse response = checker.servicesToCheck(application, requestUrl, Duration.ofSeconds(5));
             assertResponse("{\n" +
-                             "  \"services\": [\n" +
-                             "    {\n" +
-                             "      \"host\": \"" + service.getHost() + "\",\n" +
-                             "      \"port\": " + service.getPort() + ",\n" +
-                             "      \"type\": \"container\",\n" +
-                             "      \"url\": \"" + serviceUrl.toString() + "\"\n" +
-                             "    },\n" +
-                             "    {\n" +
-                             "      \"host\": \"" + service.getHost() + "\",\n" +
-                             "      \"port\": " + service.getPort() + ",\n" +
-                             "      \"type\": \"container\",\n" +
-                             "      \"url\": \"" + serviceUrl.toString() + "\"\n" +
-                             "    }\n" +
-                             "  ],\n" +
-                             "  \"url\": \"" + requestUrl.toString() + "\",\n" +
-                             "  \"currentGeneration\": 3,\n" +
-                             "  \"wantedGeneration\": 4,\n" +
-                             "  \"converged\": false\n" +
-                             "}",
-                             200,
-                             response);
+                           "  \"services\": [\n" +
+                           "    {\n" +
+                           "      \"host\": \"" + service.getHost() + "\",\n" +
+                           "      \"port\": " + service.getPort() + ",\n" +
+                           "      \"type\": \"container\",\n" +
+                           "      \"url\": \"" + serviceUrl.toString() + "\",\n" +
+                           "      \"currentGeneration\":" + 4 + "\n" +
+                           "    },\n" +
+                           "    {\n" +
+                           "      \"host\": \"" + service2.getHost() + "\",\n" +
+                           "      \"port\": " + service2.getPort() + ",\n" +
+                           "      \"type\": \"container\",\n" +
+                           "      \"url\": \"" + serviceUrl2.toString() + "\",\n" +
+                           "      \"currentGeneration\":" + 3 + "\n" +
+                           "    }\n" +
+                           "  ],\n" +
+                           "  \"url\": \"" + requestUrl.toString() + "\",\n" +
+                           "  \"currentGeneration\": 3,\n" +
+                           "  \"wantedGeneration\": 4,\n" +
+                           "  \"converged\": false\n" +
+                           "}",
+                           200,
+                           response);
         }
     }
 
@@ -193,6 +193,10 @@ public class ConfigConvergenceCheckerTest {
     }
 
     private URI testServer() {
+        return testServer(wireMock);
+    }
+
+    private URI testServer(WireMockRule wireMock) {
         return URI.create("http://127.0.0.1:" + wireMock.port());
     }