Merge with master

8 files changed, 43 insertions, 84 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index 8fb37ba2a15..b65d3bc0849 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -1,7 +1,6 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller;
 
-import com.fasterxml.jackson.databind.JsonNode;
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.Version;
@@ -10,7 +9,6 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.athenz.api.AthenzDomain;
 import com.yahoo.vespa.curator.Lock;
-import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
@@ -157,11 +155,6 @@ public class Controller extends AbstractComponent {
         return globalRoutingService.getHealthStatus(rotation.name());
     }
 
-    // TODO: Model the response properly
-    public JsonNode waitForConfigConvergence(DeploymentId deploymentId, long timeout) {
-        return configServer.waitForConfigConverge(deploymentId, timeout);
-    }
-
     public ApplicationView getApplicationView(String tenantName, String applicationName, String instanceName,
                                               String environment, String region) {
         return configServer.getApplicationView(tenantName, applicationName, instanceName, environment, region);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java
index 516cd52d710..ac0d08f5105 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgrader.java
@@ -11,7 +11,6 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
-import java.util.Arrays;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
@@ -38,12 +37,8 @@ public class SystemUpgrader extends Maintainer {
         if (!target.isPresent()) {
             return;
         }
-        // TODO: Change to SystemApplication.all() once host applications support upgrade
-        try {
-            deploy(Arrays.asList(SystemApplication.configServer, SystemApplication.zone), target.get());
-        } catch (Exception e) {
-            log.log(Level.WARNING, "Failed to upgrade system. Retrying in " + maintenanceInterval(), e);
-        }
+
+        deploy(SystemApplication.all(), target.get());
     }
 
     /** Deploy a list of system applications until they converge on the given version */
@@ -51,14 +46,14 @@ public class SystemUpgrader extends Maintainer {
         for (List<ZoneId> zones : controller().zoneRegistry().upgradePolicy().asList()) {
             boolean converged = true;
             for (ZoneId zone : zones) {
-                for (SystemApplication application : applications) {
-                    boolean dependenciesConverged = application.dependencies().stream()
-                                                               .filter(applications::contains) // TODO: Remove when all() is used.
-                                                               .allMatch(dependency -> currentVersion(zone, dependency.id()).equals(target));
-                    if (dependenciesConverged) {
-                        deploy(target, application, zone);
-                    }
-                    converged &= currentVersion(zone, application.id()).equals(target);
+                try {
+                    converged &= deployInZone(zone, applications, target);
+                } catch (UnreachableNodeRepositoryException e) {
+                    converged = false;
+                    log.log(Level.WARNING, e.getMessage() + ". Continuing to next parallel deployed zone");
+                } catch (Exception e) {
+                    converged = false;
+                    log.log(Level.WARNING, "Failed to upgrade " + zone + ". Continuing to next parallel deployed zone", e);
                 }
             }
             if (!converged) {
@@ -67,6 +62,18 @@ public class SystemUpgrader extends Maintainer {
         }
     }
 
+    /** @return true if all applications have converged to the target version in the zone */
+    private boolean deployInZone(ZoneId zone, List<SystemApplication> applications, Version target) {
+        boolean converged = true;
+        for (SystemApplication application : applications) {
+            if (convergedOn(target, application.dependencies(), zone)) {
+                deploy(target, application, zone);
+            }
+            converged &= convergedOn(target, application, zone);
+        }
+        return converged;
+    }
+
     /** Deploy application on given version idempotently */
     private void deploy(Version target, SystemApplication application, ZoneId zone) {
         if (!wantedVersion(zone, application.id(), target).equals(target)) {
@@ -75,12 +82,20 @@ public class SystemUpgrader extends Maintainer {
         }
     }
 
+    private boolean convergedOn(Version target, List<SystemApplication> applications, ZoneId zone) {
+        return applications.stream().allMatch(application -> convergedOn(target, application, zone));
+    }
+
+    private boolean convergedOn(Version target, SystemApplication application, ZoneId zone) {
+        return currentVersion(zone, application.id(), target).equals(target);
+    }
+
     private Version wantedVersion(ZoneId zone, ApplicationId application, Version defaultVersion) {
         return minVersion(zone, application, Node::wantedVersion).orElse(defaultVersion);
     }
 
-    private Version currentVersion(ZoneId zone, ApplicationId application) {
-        return minVersion(zone, application, Node::currentVersion).orElse(Version.emptyVersion);
+    private Version currentVersion(ZoneId zone, ApplicationId application, Version defaultVersion) {
+        return minVersion(zone, application, Node::currentVersion).orElse(defaultVersion);
     }
 
     private Optional<Version> minVersion(ZoneId zone, ApplicationId application, Function<Node, Version> versionField) {
@@ -92,9 +107,8 @@ public class SystemUpgrader extends Maintainer {
                                .map(versionField)
                                .min(Comparator.naturalOrder());
         } catch (Exception e) {
-            log.log(Level.WARNING, String.format("Failed to get version for %s in %s: %s", application, zone,
-                                                 Exceptions.toMessageString(e)));
-            return Optional.empty();
+            throw new UnreachableNodeRepositoryException(String.format("Failed to get version for %s in %s: %s",
+                    application, zone, Exceptions.toMessageString(e)));
         }
     }
 
@@ -105,4 +119,9 @@ public class SystemUpgrader extends Maintainer {
                            .map(VespaVersion::versionNumber);
     }
 
+    private class UnreachableNodeRepositoryException extends RuntimeException {
+        private UnreachableNodeRepositoryException(String reason) {
+            super(reason);
+        }
+    }
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index d9848577f0a..10088ba3fea 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -168,7 +168,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         if (path.matches("/application/v4/tenant/{tenant}/application")) return applications(path.get("tenant"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return application(path.get("tenant"), path.get("application"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}")) return deployment(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
-        if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/converge")) return waitForConvergence(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service")) return services(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/service/{service}/{*}")) return service(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"), path.get("service"), path.getRest(), request);
         if (path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation")) return rotationStatus(path.get("tenant"), path.get("application"), path.get("instance"), path.get("environment"), path.get("region"));
@@ -579,12 +578,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         return new SlimeJsonResponse(slime);
     }
 
-    private HttpResponse waitForConvergence(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
-        return new JacksonJsonResponse(controller.waitForConfigConvergence(new DeploymentId(ApplicationId.from(tenantName, applicationName, instanceName),
-                                                                                            ZoneId.from(environment, region)),
-                                                                           asLong(request.getProperty("timeout"), 1000)));
-    }
-
     private HttpResponse services(String tenantName, String applicationName, String instanceName, String environment, String region, HttpRequest request) {
         ApplicationView applicationView = controller.getApplicationView(tenantName, applicationName, instanceName, environment, region);
         ServiceApiResponse response = new ServiceApiResponse(ZoneId.from(environment, region),
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JacksonJsonResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JacksonJsonResponse.java
deleted file mode 100644
index cfd6feccf01..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JacksonJsonResponse.java
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.restapi.application;
-
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.yahoo.container.jdisc.HttpResponse;
-
-import java.io.IOException;
-import java.io.OutputStream;
-
-/**
- * @author bratseth
- */
-public class JacksonJsonResponse extends HttpResponse {
-
-    private final JsonNode node;
-
-    public JacksonJsonResponse(JsonNode node) {
-        super(200);
-        this.node = node;
-    }
-
-    @Override
-    public void render(OutputStream stream) throws IOException {
-        new ObjectMapper().writeValue(stream, node);
-    }
-
-    @Override
-    public String getContentType() { return "application/json"; }
-
-}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java
index 99b913709df..ac9afd9752a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerMock.java
@@ -1,9 +1,6 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller;
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.node.JsonNodeFactory;
-import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.Version;
@@ -186,13 +183,6 @@ public class ConfigServerMock extends AbstractComponent implements ConfigServer
         applications.remove(deployment.applicationId());
     }
 
-    @Override
-    public JsonNode waitForConfigConverge(DeploymentId applicationInstance, long timeoutInSeconds) {
-        ObjectNode root = new ObjectNode(JsonNodeFactory.instance);
-        root.put("generation", 1);
-        return root;
-    }
-
     // Returns a canned example response
     @Override
     public ApplicationView getApplicationView(String tenantName, String applicationName, String instanceName,
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java
index 1ec64f8d478..4b563ed203d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/SystemUpgraderTest.java
@@ -121,7 +121,6 @@ public class SystemUpgraderTest {
     }
 
     @Test
-    @Ignore // TODO: Unignore once host applications support upgrade
     public void upgrade_system_containing_host_applications() {
         tester.controllerTester().zoneRegistry().setUpgradePolicy(
                 UpgradePolicy.create()
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index 407263a7973..8d734ec549c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -9,7 +9,6 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.io.IOUtils;
 import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Slime;
@@ -294,14 +293,12 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/restart", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               "Requested restart of tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default");
+
         // POST a 'restart application' command with a host filter (other filters not supported yet)
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/restart?hostname=host1", POST)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               "Requested restart of tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default");
-        // GET (wait for) convergence
-        tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/converge", GET)
-                                      .userIdentity(USER_ID),
-                              new File("convergence.json"));
+
         // GET services
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default/service", GET)
                                       .userIdentity(USER_ID),
@@ -319,11 +316,13 @@ public class ApplicationApiTest extends ControllerContainerTest {
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default", DELETE)
                                       .userIdentity(USER_ID),
                               "Deactivated tenant/tenant1/application/application1/environment/dev/region/us-west-1/instance/default");
+
         // DELETE (deactivate) a deployment - prod
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", DELETE)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
                               "Deactivated tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default");
 
+
         // DELETE (deactivate) a deployment is idempotent
         tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/corp-us-east-1/instance/default", DELETE)
                                       .screwdriverIdentity(SCREWDRIVER_ID),
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/convergence.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/convergence.json
deleted file mode 100644
index acfb67b702b..00000000000
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/convergence.json
+++ /dev/null
@@ -1,3 +0,0 @@
-{
-  "generation": 1
-}
\ No newline at end of file