Merge pull request vespa-engine#9883 from vespa-engine/hakonhall/avoi…

…d-too-much-use-of-zonelistids-now-that-zones-is-available

Avoid too much use of ZoneList.ids() now that zones() is available

config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java

@@ -1,10 +1,13 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.provision.zone;
 
+import com.google.common.collect.ImmutableList;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.Zone;
 
 import java.util.List;
+import java.util.stream.Collectors;
 
 /**
  * Provides filters for and access to a list of ZoneIds.
@@ -32,7 +35,9 @@ public interface ZoneList extends ZoneFilter {
     /** Returns the ZoneApi of all zones in this list. */
     List<? extends ZoneApi> zones();
 
-    /** Returns the id of all zones in this list as — you guessed it — a list. */
-    List<ZoneId> ids();
+    /** Returns the ZoneIds of all zones in this list. */
+    default List<ZoneId> ids() {
+        return zones().stream().map(ZoneApi::getId).collect(Collectors.toList());
+    }
 
 }

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java

@@ -3,6 +3,7 @@
 
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.config.provision.ApplicationId;
@@ -81,8 +82,8 @@ protected void maintain() {
     private List<NodeRepositoryNode> getNodes() {
         return controller().zoneRegistry().zones()
                 .ofCloud(CloudName.from("aws"))
-                .reachable().ids().stream()
-                .flatMap(zoneId -> uncheck(() -> nodeRepository.listNodes(zoneId, true).nodes().stream()))
+                .reachable().zones().stream()
+                .flatMap(zone -> uncheck(() -> nodeRepository.listNodes(zone.getId(), true).nodes().stream()))
                 .filter(node -> node.getOwner() != null && !node.getOwner().getTenant().equals("hosted-vespa"))
                 .filter(node -> node.getState() == NodeState.active)
                 .collect(Collectors.toList());

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java

@@ -5,6 +5,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.inject.Inject;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.config.provision.zone.ZoneList;
 import com.yahoo.jdisc.http.HttpRequest.Method;
@@ -114,9 +115,9 @@ private ProxyResponse createDiscoveryResponse(ProxyRequest proxyRequest) {
         if ( ! environmentName.isEmpty())
             zones = zones.in(Environment.from(environmentName));
 
-        for (ZoneId zoneId : zones.ids()) {
+        for (ZoneApi zone : zones.zones()) {
             responseStructure.uris.add(proxyRequest.getScheme() + "://" + proxyRequest.getControllerPrefix() +
-                                       zoneId.environment().value() + "/" + zoneId.region().value());
+                                       zone.getEnvironment().value() + "/" + zone.getRegionName().value());
         }
         JsonNode node = mapper.valueToTree(responseStructure);
         return new ProxyResponse(proxyRequest, node.toString(), 200, Optional.empty(), "application/json");

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java

@@ -2,6 +2,7 @@
 
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeOwner;
@@ -34,8 +35,8 @@ public static String resourceShareByPropertyToCsv(NodeRepositoryClientInterface
         String date = LocalDate.now(clock).toString();
 
         List<NodeRepositoryNode> nodes = controller.zoneRegistry().zones()
-                .reachable().in(Environment.prod).ofCloud(cloudName).ids().stream()
-                .flatMap(zoneId -> uncheck(() -> nodeRepository.listNodes(zoneId, true).nodes().stream()))
+                .reachable().in(Environment.prod).ofCloud(cloudName).zones().stream()
+                .flatMap(zone -> uncheck(() -> nodeRepository.listNodes(zone.getId(), true).nodes().stream()))
                 .filter(node -> node.getOwner() != null && !node.getOwner().getTenant().equals("hosted-vespa"))
                 .collect(Collectors.toList());
 

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java

@@ -5,6 +5,7 @@
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.io.IOUtils;
@@ -30,6 +31,7 @@
 import java.util.Set;
 import java.util.StringJoiner;
 import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 /**
  * This implements the /os/v1 API which provides operators with information about, and scheduling of OS upgrades for
@@ -123,7 +125,7 @@ private List<ZoneId> zonesAt(Path path) {
         ZoneList zones = controller.zoneRegistry().zones().controllerUpgraded();
         if (path.get("region") != null) zones = zones.in(RegionName.from(path.get("region")));
         if (path.get("environment") != null) zones = zones.in(Environment.from(path.get("environment")));
-        return zones.ids();
+        return zones.zones().stream().map(ZoneApi::getId).collect(Collectors.toList());
     }
 
     private Slime setOsVersion(HttpRequest request) {

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v1/ZoneApiHandler.java

@@ -3,6 +3,8 @@
 
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.Zone;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
@@ -70,8 +72,8 @@ private HttpResponse get(HttpRequest request) {
     }
 
     private HttpResponse root(HttpRequest request) {
-        List<Environment> environments = zoneRegistry.zones().all().ids().stream()
-                                                     .map(ZoneId::environment)
+        List<Environment> environments = zoneRegistry.zones().all().zones().stream()
+                                                     .map(ZoneApi::getEnvironment)
                                                      .distinct()
                                                      .sorted(Comparator.comparing(Environment::value))
                                                      .collect(Collectors.toList());
@@ -90,17 +92,16 @@ private HttpResponse root(HttpRequest request) {
     }
 
     private HttpResponse environment(HttpRequest request, Environment environment) {
-        List<ZoneId> zones = zoneRegistry.zones().all().in(environment).ids();
         Slime slime = new Slime();
         Cursor root = slime.setArray();
-        zones.forEach(zone -> {
+        zoneRegistry.zones().all().in(environment).zones().forEach(zone -> {
             Cursor object = root.addObject();
-            object.setString("name", zone.region().value());
+            object.setString("name", zone.getRegionName().value());
             object.setString("url", request.getUri()
                                            .resolve("/zone/v2/environment/")
                                            .resolve(environment.value() + "/")
                                            .resolve("region/")
-                                           .resolve(zone.region().value())
+                                           .resolve(zone.getRegionName().value())
                                            .toString());
         });
         return new SlimeJsonResponse(slime);

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java

@@ -94,16 +94,16 @@ private HttpResponse root(HttpRequest request) {
         Cursor root = slime.setObject();
         Cursor uris = root.setArray("uris");
         ZoneList zoneList = zoneRegistry.zones().reachable();
-        zoneList.ids().forEach(zoneId -> uris.addString(request.getUri()
+        zoneList.zones().forEach(zone -> uris.addString(request.getUri()
                                                                .resolve("/zone/v2/")
-                                                               .resolve(zoneId.environment().value() + "/")
-                                                               .resolve(zoneId.region().value())
+                                                               .resolve(zone.getEnvironment().value() + "/")
+                                                               .resolve(zone.getRegionName().value())
                                                                .toString()));
         Cursor zones = root.setArray("zones");
-        zoneList.ids().forEach(zoneId -> {
+        zoneList.zones().forEach(zone -> {
             Cursor object = zones.addObject();
-            object.setString("environment", zoneId.environment().value());
-            object.setString("region", zoneId.region().value());
+            object.setString("environment", zone.getEnvironment().value());
+            object.setString("region", zone.getRegionName().value());
         });
         return new SlimeJsonResponse(slime);
     }

controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java

@@ -6,6 +6,7 @@
 import com.yahoo.component.Version;
 import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.HostName;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -155,20 +156,17 @@ public static VersionStatus compute(Controller controller) {
     }
 
     private static ListMap<Version, HostName> findSystemApplicationVersions(Controller controller) {
-        List<ZoneId> zones = controller.zoneRegistry().zones()
-                                       .controllerUpgraded()
-                                       .ids();
         ListMap<Version, HostName> versions = new ListMap<>();
-        for (ZoneId zone : zones) {
+        for (ZoneApi zone : controller.zoneRegistry().zones().controllerUpgraded().zones()) {
             for (SystemApplication application : SystemApplication.all()) {
                 List<Node> eligibleForUpgradeApplicationNodes = controller.configServer().nodeRepository()
-                        .list(zone, application.id()).stream()
+                        .list(zone.getId(), application.id()).stream()
                         .filter(SystemUpgrader::eligibleForUpgrade)
                         .collect(Collectors.toList());
                 if (eligibleForUpgradeApplicationNodes.isEmpty())
                     continue;
 
-                boolean configConverged = application.configConvergedIn(zone, controller, Optional.empty());
+                boolean configConverged = application.configConvergedIn(zone.getId(), controller, Optional.empty());
                 if (!configConverged) {
                     log.log(LogLevel.WARNING, "Config for " + application.id() + " in " + zone + " has not converged");
                 }

controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java

@@ -5,6 +5,7 @@
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.TenantName;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -123,10 +124,10 @@ public void upgradeController(Version version) {
 
     /** Upgrade system applications in all zones to given version */
     public void upgradeSystemApplications(Version version) {
-        for (ZoneId zone : tester.zoneRegistry().zones().all().ids()) {
+        for (ZoneApi zone : tester.zoneRegistry().zones().all().zones()) {
             for (SystemApplication application : SystemApplication.all()) {
-                tester.configServer().setVersion(application.id(), zone, version);
-                tester.configServer().convergeServices(application.id(), zone);
+                tester.configServer().setVersion(application.id(), zone.getId(), version);
+                tester.configServer().convergeServices(application.id(), zone.getId());
             }
         }
         computeVersionStatus();

controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java

@@ -81,11 +81,6 @@ public List<? extends ZoneApi> zones() {
         return List.copyOf(zones);
     }
 
-    @Override
-    public List<ZoneId> ids() {
-        return List.copyOf(zones.stream().map(ZoneApi::getId).collect(Collectors.toList()));
-    }
-
     @Override
     public ZoneList ofCloud(CloudName cloud) {
         return filter(zone -> zone.getCloudName().equals(cloud));

controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java

@@ -6,6 +6,7 @@
 import com.yahoo.application.container.handler.Response;
 import com.yahoo.component.ComponentSpecification;
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.container.http.filter.FilterChainRepository;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilter;
@@ -59,10 +60,10 @@ public void computeVersionStatus() {
 
     public void upgradeSystem(Version version) {
         controller().curator().writeControllerVersion(controller().hostname(), version);
-        for (ZoneId zone : controller().zoneRegistry().zones().all().ids()) {
+        for (ZoneApi zone : controller().zoneRegistry().zones().all().zones()) {
             for (SystemApplication application : SystemApplication.all()) {
-                configServer().setVersion(application.id(), zone, version);
-                configServer().convergeServices(application.id(), zone);
+                configServer().setVersion(application.id(), zone.getId(), version);
+                configServer().convergeServices(application.id(), zone.getId());
             }
         }
         computeVersionStatus();

controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java

@@ -6,6 +6,7 @@
 import com.yahoo.component.Vtag;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.HostName;
+import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
@@ -60,10 +61,10 @@ public void testSystemVersionIsVersionOfOldestConfigServer() {
         Version version0 = Version.fromString("6.1");
         Version version1 = Version.fromString("6.5");
         // Upgrade some config servers
-        for (ZoneId zone : tester.zoneRegistry().zones().all().ids()) {
-            for (Node node : tester.configServer().nodeRepository().list(zone, SystemApplication.configServer.id())) {
-                tester.configServer().nodeRepository().putByHostname(zone, new Node(node.hostname(), node.state(), node.type(),
-                                                                                    node.owner(), version1, node.wantedVersion()));
+        for (ZoneApi zone : tester.zoneRegistry().zones().all().zones()) {
+            for (Node node : tester.configServer().nodeRepository().list(zone.getId(), SystemApplication.configServer.id())) {
+                Node upgradedNode = new Node(node.hostname(), node.state(), node.type(), node.owner(), version1, node.wantedVersion());
+                tester.configServer().nodeRepository().putByHostname(zone.getId(), upgradedNode);
                 break;
             }
         }
@@ -105,10 +106,10 @@ public void testSystemVersionNeverShrinks() {
 
         // Downgrade one config server in each zone
         Version ancientVersion = Version.fromString("5.1");
-        for (ZoneId zone : tester.controller().zoneRegistry().zones().all().ids()) {
-            for (Node node : tester.configServer().nodeRepository().list(zone, SystemApplication.configServer.id())) {
-                tester.configServer().nodeRepository().putByHostname(zone, new Node(node.hostname(), node.state(), node.type(),
-                                                                                    node.owner(), ancientVersion, node.wantedVersion()));
+        for (ZoneApi zone : tester.controller().zoneRegistry().zones().all().zones()) {
+            for (Node node : tester.configServer().nodeRepository().list(zone.getId(), SystemApplication.configServer.id())) {
+                Node downgradedNode = new Node(node.hostname(), node.state(), node.type(), node.owner(), ancientVersion, node.wantedVersion());
+                tester.configServer().nodeRepository().putByHostname(zone.getId(), downgradedNode);
                 break;
             }
         }