From e37a674b61ca298092a333b2677f1c36cc061ec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Hallingstad?= Date: Mon, 24 Jun 2019 11:20:13 +0200 Subject: [PATCH] Avoid too much use of ZoneList.ids() now that zones() is available --- .../yahoo/config/provision/zone/ZoneList.java | 9 +++++++-- .../maintenance/ResourceMeterMaintainer.java | 5 +++-- .../proxy/ConfigServerRestExecutorImpl.java | 5 +++-- .../controller/restapi/cost/CostCalculator.java | 5 +++-- .../controller/restapi/os/OsApiHandler.java | 4 +++- .../restapi/zone/v1/ZoneApiHandler.java | 13 +++++++------ .../restapi/zone/v2/ZoneApiHandler.java | 12 ++++++------ .../controller/versions/VersionStatus.java | 10 ++++------ .../controller/deployment/DeploymentTester.java | 7 ++++--- .../controller/integration/ZoneFilterMock.java | 5 ----- .../controller/restapi/ContainerTester.java | 7 ++++--- .../controller/versions/VersionStatusTest.java | 17 +++++++++-------- 12 files changed, 53 insertions(+), 46 deletions(-) diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java index 5f3f2e10898c..776f925c4242 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneList.java +++ b/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 zones(); - /** Returns the id of all zones in this list as — you guessed it — a list. */ - List ids(); + /** Returns the ZoneIds of all zones in this list. */ + default List ids() { + return zones().stream().map(ZoneApi::getId).collect(Collectors.toList()); + } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java index 9302ecbe738a..c4f0597572ba 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java +++ b/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 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()); 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 a208249b4100..73a029ad3b35 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 @@ -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"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java index 18c00d69b623..c44a80f7a206 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/cost/CostCalculator.java +++ b/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 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()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java index 5454d71185af..bc360fe3c6fb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiHandler.java +++ b/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 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) { 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 b115e659c286..6cfaed93fa9d 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 @@ -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 environments = zoneRegistry.zones().all().ids().stream() - .map(ZoneId::environment) + List 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 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); 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 9d95383fbfb0..f0259fc4d51f 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 @@ -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); } 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 87f35d3b2c1c..ab5fd2714e59 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 @@ -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 findSystemApplicationVersions(Controller controller) { - List zones = controller.zoneRegistry().zones() - .controllerUpgraded() - .ids(); ListMap versions = new ListMap<>(); - for (ZoneId zone : zones) { + for (ZoneApi zone : controller.zoneRegistry().zones().controllerUpgraded().zones()) { for (SystemApplication application : SystemApplication.all()) { List 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"); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java index 3ce32347e356..887406ecba88 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java +++ b/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(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java index 57f29fb72af3..00e6162d5e5e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ZoneFilterMock.java @@ -81,11 +81,6 @@ public List zones() { return List.copyOf(zones); } - @Override - public List 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)); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java index ef86ffa125f9..c7be543dd00f 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java +++ b/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(); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 8e3dc24193f2..655c16ccceb0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/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; } }