Merge pull request #4421 from vespa-engine/jvenstad/zone-cleanup-3

Jvenstad/zone cleanup 3

9 files changed, 65 insertions, 74 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 44e87d258df..ec1051a3674 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -667,7 +667,7 @@ public class ApplicationController {
         deploymentSpec.zones().stream()
                 .filter(zone -> zone.environment() == Environment.prod)
                 .forEach(zone -> {
-                    if ( ! controller.zoneRegistry().getZone(zone.environment(), zone.region().orElse(null)).isPresent())
+                    if ( ! controller.zoneRegistry().hasZone(ZoneId.from(zone.environment(), zone.region().orElse(null))))
                         throw new IllegalArgumentException("Zone " + zone + " in deployment spec was not found in this system!");
                 });
     }
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 71a0a7f6297..f50958f0e66 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
@@ -6,9 +6,8 @@ import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.component.Version;
 import com.yahoo.component.Vtag;
-import com.yahoo.config.provision.Environment;
-import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.ZoneId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
@@ -153,31 +152,16 @@ public class Controller extends AbstractComponent {
 
     public Clock clock() { return clock; }
 
-    public URI getElkUri(DeploymentId deploymentId) {
-        return elkUrl(zoneRegistry.getLogServerUri(deploymentId.zone().environment(), deploymentId.zone().region()), deploymentId);
+    public Optional<URI> getLogServerUrl(DeploymentId deploymentId) {
+        return zoneRegistry.getLogServerUri(deploymentId);
     }
 
-    public List<URI> getConfigServerUris(Environment environment, RegionName region) {
-        return zoneRegistry.getConfigServerUris(environment, region);
+    public List<URI> getConfigServerUris(ZoneId zoneId) {
+        return zoneRegistry.getConfigServerUris(zoneId);
     }
     
     public ZoneRegistry zoneRegistry() { return zoneRegistry; }
 
-    private URI elkUrl(Optional<URI> kibanaHost, DeploymentId deploymentId) {
-        String kibanaQuery = "/#/discover?_g=()&_a=(columns:!(_source)," +
-                             "index:'logstash-*',interval:auto," +
-                             "query:(query_string:(analyze_wildcard:!t,query:'" +
-                             "HV-tenant:%22" + deploymentId.applicationId().tenant().value() + "%22%20" +
-                             "AND%20HV-application:%22" + deploymentId.applicationId().application().value() + "%22%20" +
-                             "AND%20HV-region:%22" + deploymentId.zone().region().value() + "%22%20" +
-                             "AND%20HV-instance:%22" + deploymentId.applicationId().instance().value() + "%22%20" +
-                             "AND%20HV-environment:%22" + deploymentId.zone().environment().value() + "%22'))," +
-                             "sort:!('@timestamp',desc))";
-
-        URI kibanaPath = URI.create(kibanaQuery);
-        return kibanaHost.map(uri -> uri.resolve(kibanaPath)).orElse(null);
-    }
-
     public Map<String, RotationStatus> getHealthStatus(String hostname) {
         return globalRoutingService.getHealthStatus(hostname);
     }
@@ -202,7 +186,9 @@ public class Controller extends AbstractComponent {
         return configServerClient.grabLog(deploymentId);
     }
 
-    public GitHub gitHub() { return gitHub; }
+    public GitHub gitHub() {
+        return gitHub;
+    }
 
     /** Replace the current version status by a new one */
     public void updateVersionStatus(VersionStatus newStatus) {
@@ -225,7 +211,9 @@ public class Controller extends AbstractComponent {
                 .orElse(Vtag.currentVersion);
     }
 
-    public MetricsService metricsService() { return metricsService; }
+    public MetricsService metricsService() {
+        return metricsService;
+    }
 
     public SystemName system() {
         return zoneRegistry.system();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java
index eb44229e790..5638ff28904 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentExpirer.java
@@ -53,14 +53,9 @@ public class DeploymentExpirer extends Maintainer {
     }
 
     public static boolean hasExpired(ZoneRegistry zoneRegistry, Deployment deployment, Instant now) {
-        return zoneRegistry.getDeploymentTimeToLive(deployment.zone().environment(), deployment.zone().region())
-                .map(duration -> getExpiration(deployment, duration))
-                .map(now::isAfter)
+        return zoneRegistry.getDeploymentTimeToLive(deployment.zone())
+                .map(timeToLive -> deployment.at().plus(timeToLive).isBefore(now))
                 .orElse(false);
     }
 
-    private static Instant getExpiration(Deployment instance, Duration ttl) {
-        return instance.at().plus(ttl);
-    }
-
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java
index c9ce0b76520..e67b96c22ad 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java
@@ -54,11 +54,10 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor {
             return createDiscoveryResponse(proxyRequest);
         }
 
-        Environment environment = Environment.from(proxyRequest.getEnvironment());
-        RegionName region = RegionName.from(proxyRequest.getRegion());
+        ZoneId zoneId = ZoneId.from(proxyRequest.getEnvironment(), proxyRequest.getRegion());
 
         // Make a local copy of the list as we want to manipulate it in case of ping problems.
-        final List<URI> allServers = new ArrayList<>(zoneRegistry.getConfigServerUris(environment, region));
+        List<URI> allServers = new ArrayList<>(zoneRegistry.getConfigServerUris(zoneId));
 
         StringBuilder errorBuilder = new StringBuilder();
         if (queueFirstServerIfDown(allServers)) {
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 2e5113908ab..d15686077c6 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
@@ -429,9 +429,9 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         DeploymentId deploymentId = new DeploymentId(application.id(),
                                                      ZoneId.from(environment, region));
 
-        Deployment deployment = application.deployments().get(deploymentId.zone());
+        Deployment deployment = application.deployments().get(deploymentId.zoneId());
         if (deployment == null)
-            throw new NotExistsException(application + " is not deployed in " + deploymentId.zone());
+            throw new NotExistsException(application + " is not deployed in " + deploymentId.zoneId());
 
         Slime slime = new Slime();
         toSlime(slime.setObject(), deploymentId, deployment, request);
@@ -447,18 +447,17 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
                 serviceUrlArray.addString(uri.toString());
         }
 
-        response.setString("nodes", withPath("/zone/v2/" + deploymentId.zone().environment() + "/" + deploymentId.zone().region() + "/nodes/v2/node/?&recursive=true&application=" + deploymentId.applicationId().tenant() + "." + deploymentId.applicationId().application() + "." + deploymentId.applicationId().instance(), request.getUri()).toString());
+        response.setString("nodes", withPath("/zone/v2/" + deploymentId.zoneId().environment() + "/" + deploymentId.zoneId().region() + "/nodes/v2/node/?&recursive=true&application=" + deploymentId.applicationId().tenant() + "." + deploymentId.applicationId().application() + "." + deploymentId.applicationId().instance(), request.getUri()).toString());
 
-        URI elkUrl = controller.getElkUri(deploymentId);
-        if (elkUrl != null)
-            response.setString("elkUrl", elkUrl.toString());
+        controller.getLogServerUrl(deploymentId)
+                .ifPresent(elkUrl -> response.setString("elkUrl", elkUrl.toString()));
 
         response.setString("yamasUrl", monitoringSystemUri(deploymentId).toString());
         response.setString("version", deployment.version().toFullString());
         response.setString("revision", deployment.revision().id());
         response.setLong("deployTimeEpochMs", deployment.at().toEpochMilli());
-        Optional<Duration> deploymentTimeToLive = controller.zoneRegistry().getDeploymentTimeToLive(deploymentId.zone().environment(), deploymentId.zone().region());
-        deploymentTimeToLive.ifPresent(duration -> response.setLong("expiryTimeEpochMs", deployment.at().plus(duration).toEpochMilli()));
+        controller.zoneRegistry().getDeploymentTimeToLive(deploymentId.zoneId())
+                .ifPresent(deploymentTimeToLive -> response.setLong("expiryTimeEpochMs", deployment.at().plus(deploymentTimeToLive).toEpochMilli()));
 
         controller.applications().get(deploymentId.applicationId()).flatMap(application -> application.deploymentJobs().projectId())
                 .ifPresent(i -> response.setString("screwdriverId", String.valueOf(i)));
@@ -493,9 +492,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
     }
 
     private URI monitoringSystemUri(DeploymentId deploymentId) {
-        return controller.zoneRegistry().getMonitoringSystemUri(deploymentId.zone().environment(),
-                                                                deploymentId.zone().region(),
-                                                                deploymentId.applicationId());
+        return controller.zoneRegistry().getMonitoringSystemUri(deploymentId);
     }
 
     private HttpResponse setGlobalRotationOverride(String tenantName, String applicationName, String instanceName, String environment, String region, boolean inService, HttpRequest request) {
@@ -583,7 +580,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         ApplicationView applicationView = controller.getApplicationView(tenantName, applicationName, instanceName, environment, region);
         ServiceApiResponse response = new ServiceApiResponse(ZoneId.from(environment, region),
                                                              new ApplicationId.Builder().tenant(tenantName).applicationName(applicationName).instanceName(instanceName).build(),
-                                                             controller.getConfigServerUris(Environment.from(environment), RegionName.from(region)),
+                                                             controller.getConfigServerUris(ZoneId.from(environment, region)),
                                                              request.getUri());
         response.setResponse(applicationView);
         return response;
@@ -593,7 +590,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         Map<?,?> result = controller.getServiceApiResponse(tenantName, applicationName, instanceName, environment, region, serviceName, restPath);
         ServiceApiResponse response = new ServiceApiResponse(ZoneId.from(environment, region),
                                                              new ApplicationId.Builder().tenant(tenantName).applicationName(applicationName).instanceName(instanceName).build(),
-                                                             controller.getConfigServerUris(Environment.from(environment), RegionName.from(region)),
+                                                             controller.getConfigServerUris(ZoneId.from(environment, region)),
                                                              request.getUri());
         response.setResponse(result, serviceName, restPath);
         return response;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java
index aecd3847653..83b725ae4c4 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java
@@ -109,9 +109,7 @@ public class ZoneApiHandler extends LoggingRequestHandler {
 
     private HttpResponse defaultRegion(HttpRequest request, Environment environment) {
         RegionName region = zoneRegistry.getDefaultRegion(environment)
-                                        .orElseThrow(() -> new IllegalArgumentException(
-                                                "No default region for environment: " + environment
-                                        ));
+                .orElseThrow(() -> new IllegalArgumentException("No default region for environment: " + environment));
         Slime slime = new Slime();
         Cursor root = slime.setObject();
         root.setString("name", region.value());
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java
index 3f85b0116ad..772dd1f6cb1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java
@@ -76,14 +76,12 @@ public class ZoneApiHandler extends LoggingRequestHandler {
 
     private HttpResponse proxy(HttpRequest request) {
         Path path = new Path(request.getUri().getPath());
-        if (!path.matches("/zone/v2/{environment}/{region}/{*}")) {
+        if ( ! path.matches("/zone/v2/{environment}/{region}/{*}")) {
             return notFound(path);
         }
-        Environment environment = Environment.from(path.get("environment"));
-        RegionName region = RegionName.from(path.get("region"));
-        Optional<ZoneId> zone = zoneRegistry.getZone(environment, region);
-        if (!zone.isPresent()) {
-            throw new IllegalArgumentException("No such zone: " + environment.value() + "." + region.value());
+        ZoneId zoneId = ZoneId.from(path.get("environment"), path.get("region"));
+        if ( ! zoneRegistry.hasZone(zoneId)) {
+            throw new IllegalArgumentException("No such zone: " + zoneId.value());
         }
         try {
             return proxy.handle(new ProxyRequest(request, "/zone/v2/"));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index 0e07f7b7589..876bd5fe029 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -120,8 +120,9 @@ public class VersionStatus {
 
     private static ListMap<Version, String> findConfigServerVersions(Controller controller) {
         List<URI> configServers = controller.zoneRegistry().zones().stream()
+                // TODO: Filter properly.
                 .filter(zone -> ! zone.region().equals(RegionName.from("us-east-2a")))
-                .flatMap(zone -> controller.getConfigServerUris(zone.environment(), zone.region()).stream())
+                .flatMap(zone -> controller.getConfigServerUris(zone).stream())
                 .collect(Collectors.toList());
 
         ListMap<Version, String> versions = new ListMap<>();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java
index 53af74bf542..2317b7bc6f1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ZoneRegistryMock.java
@@ -3,15 +3,16 @@ package com.yahoo.vespa.hosted.controller;
 
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
-import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.ZoneId;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
 
 import java.net.URI;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -67,28 +68,42 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry
     }
 
     @Override
-    public Optional<ZoneId> getZone(Environment environment, RegionName region) {
-        return zones().stream().filter(z -> z.environment().equals(environment) &&
-                                            z.region().equals(region)).findFirst();
+    public boolean hasZone(ZoneId zoneId) {
+        return zones.contains(zoneId);
     }
 
     @Override
-    public List<URI> getConfigServerUris(Environment environment, RegionName region) {
-        return getZone(environment, region)
-                .map(z -> URI.create(String.format("http://cfg.%s.%s.test", environment.value(), region.value())))
-                .map(Collections::singletonList)
-                .orElse(Collections.emptyList());
+    public List<URI> getConfigServerUris(ZoneId zoneId) {
+        return Collections.singletonList(URI.create(String.format("http://cfg.%s.test", zoneId.value())));
     }
 
     @Override
-    public Optional<URI> getLogServerUri(Environment environment, RegionName region) {
-        return getZone(environment, region)
-                .map(z -> URI.create(String.format("http://log.%s.%s.test", environment.value(), region.value())));
+    public List<URI> getConfigServerSecureUris(ZoneId zoneId) {
+        return Collections.singletonList(URI.create(String.format("https://cfg.%s.test:4443", zoneId.value())));
     }
 
     @Override
-    public Optional<Duration> getDeploymentTimeToLive(Environment environment, RegionName region) {
-        return Optional.ofNullable(deploymentTimeToLive.get(ZoneId.from(environment, region)));
+    public Optional<URI> getLogServerUri(DeploymentId deploymentId) {
+        if ( ! hasZone(deploymentId.zoneId()))
+            return Optional.empty();
+
+        String kibanaQuery = "/#/discover?_g=()&_a=(columns:!(_source)," +
+                             "index:'logstash-*',interval:auto," +
+                             "query:(query_string:(analyze_wildcard:!t,query:'" +
+                             "HV-tenant:%22" + deploymentId.applicationId().tenant().value() + "%22%20" +
+                             "AND%20HV-application:%22" + deploymentId.applicationId().application().value() + "%22%20" +
+                             "AND%20HV-region:%22" + deploymentId.zoneId().region().value() + "%22%20" +
+                             "AND%20HV-instance:%22" + deploymentId.applicationId().instance().value() + "%22%20" +
+                             "AND%20HV-environment:%22" + deploymentId.zoneId().environment().value() + "%22'))," +
+                             "sort:!('@timestamp',desc))";
+
+        URI kibanaPath = URI.create(kibanaQuery);
+        return Optional.of(URI.create(String.format("http://log.%s.test", deploymentId.zoneId().value())).resolve(kibanaPath));
+    }
+
+    @Override
+    public Optional<Duration> getDeploymentTimeToLive(ZoneId zoneId) {
+        return Optional.ofNullable(deploymentTimeToLive.get(zoneId));
     }
 
     @Override
@@ -97,9 +112,9 @@ public class ZoneRegistryMock extends AbstractComponent implements ZoneRegistry
     }
 
     @Override
-    public URI getMonitoringSystemUri(Environment environment, RegionName name, ApplicationId application) {
-        return URI.create("http://monitoring-system.test/?environment=" + environment.value() + "&region="
-                                  + name.value() + "&application=" + application.toShortString());
+    public URI getMonitoringSystemUri(DeploymentId deploymentId) {
+        return URI.create("http://monitoring-system.test/?environment=" + deploymentId.zoneId().environment().value() + "&region="
+                          + deploymentId.zoneId().region().value() + "&application=" + deploymentId.applicationId().toShortString());
     }
 
     @Override