From 0c1d41ed78ff6c1135d823b3b24b6eb8f63a5a5b Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Sun, 16 Jun 2019 16:30:15 +0200 Subject: Remove system from ZoneId --- .../com/yahoo/config/provision/zone/ZoneApi.java | 4 --- .../com/yahoo/config/provision/zone/ZoneId.java | 32 +++------------------- .../com/yahoo/config/provision/ZoneIdTest.java | 17 ------------ .../api/integration/deployment/JobType.java | 2 +- .../controller/maintenance/SystemUpgrader.java | 4 +-- .../controller/integration/ZoneFilterMock.java | 2 +- .../controller/maintenance/OsUpgraderTest.java | 26 +++++++++--------- .../controller/maintenance/SystemUpgraderTest.java | 10 +++---- .../hosted/controller/restapi/os/OsApiTest.java | 4 +-- 9 files changed, 28 insertions(+), 73 deletions(-) diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java index 9f6ba29d8de..fd76dc10bdb 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneApi.java @@ -17,8 +17,4 @@ public interface ZoneApi { default RegionName getRegionName() { return getId().region(); } CloudName getCloudName(); - - default ZoneId toDeprecatedId() { - return ZoneId.from(getEnvironment(), getRegionName(), getCloudName(), getSystemName()); - } } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java index 5e664e00b4c..b0ac59718aa 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java @@ -20,30 +20,16 @@ public class ZoneId { private final Environment environment; private final RegionName region; - private final SystemName system; - private ZoneId(Environment environment, RegionName region, CloudName cloud, SystemName system) { + private ZoneId(Environment environment, RegionName region) { this.environment = Objects.requireNonNull(environment, "environment must be non-null"); this.region = Objects.requireNonNull(region, "region must be non-null"); - this.system = Objects.requireNonNull(system, "system must be non-null"); - } - - private ZoneId(Environment environment, RegionName region) { - this(environment, region, CloudName.defaultName(), SystemName.defaultSystem()); } public static ZoneId from(Environment environment, RegionName region) { return new ZoneId(environment, region); } - public static ZoneId from(SystemName system, Environment environment, RegionName region) { - return new ZoneId(environment, region, CloudName.defaultName(), system); - } - - public static ZoneId from(Environment environment, RegionName region, CloudName cloud, SystemName system) { - return new ZoneId(environment, region, cloud, system); - } - public static ZoneId from(String environment, String region) { return from(Environment.from(environment), RegionName.from(region)); } @@ -55,20 +41,14 @@ public class ZoneId { case 2: return from(parts[0], parts[1]); case 4: - return from(parts[2], parts[3], parts[0], parts[1]); + // Deprecated: parts[0] == cloud, parts[1] == system + // TODO: Figure out whether this can be removed + return from(parts[2], parts[3]); default: throw new IllegalArgumentException("Cannot deserialize zone id '" + value + "'"); } } - public static ZoneId from(Environment environment, RegionName region, CloudName cloud) { - return new ZoneId(environment, region, cloud, SystemName.defaultSystem()); - } - - public static ZoneId from(String environment, String region, String cloud, String system) { - return new ZoneId(Environment.from(environment), RegionName.from(region), CloudName.from(cloud), SystemName.from(system)); - } - public static ZoneId defaultId() { return new ZoneId(Environment.defaultEnvironment(), RegionName.defaultName()); } @@ -81,10 +61,6 @@ public class ZoneId { return region; } - public SystemName system() { - return system; - } - /** Returns the serialised value of this. Inverse of {@code ZoneId.from(String value)}. */ public String value() { return environment + "." + region; diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java index 27d45ba7d7d..434badbe9bf 100644 --- a/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java +++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ZoneIdTest.java @@ -26,17 +26,6 @@ public class ZoneIdTest { ZoneId zoneId = ZoneId.from(environment, region); assertEquals(region, zoneId.region()); assertEquals(environment, zoneId.environment()); - assertEquals(SystemName.defaultSystem(), zoneId.system()); - - ZoneId zoneIdWithSystem = ZoneId.from(system, environment, region); - assertEquals(region, zoneIdWithSystem.region()); - assertEquals(environment, zoneIdWithSystem.environment()); - assertEquals(system, zoneIdWithSystem.system()); - - ZoneId zoneIdWithCloudAndSystem = ZoneId.from(environment, region, cloud, system); - assertEquals(region, zoneIdWithCloudAndSystem.region()); - assertEquals(environment, zoneIdWithCloudAndSystem.environment()); - assertEquals(system, zoneIdWithCloudAndSystem.system()); } @Test @@ -45,12 +34,6 @@ public class ZoneIdTest { assertEquals(environment.value() + "." + region.value(), zoneId.value()); assertEquals(ZoneId.from(zoneId.value()), zoneId); - ZoneId zoneIdWithCloudAndSystem = ZoneId.from(environment, region, cloud, system); - assertEquals(environment.value() + "." + region.value(), zoneIdWithCloudAndSystem.value()); - assertEquals(ZoneId.from(zoneIdWithCloudAndSystem.value()), zoneIdWithCloudAndSystem); - // TODO: Expect cloud and system to be part of deserialized value when the new format is supported everywhere - //assertEquals(cloud.value() + "." + system.name() + "." + environment.value() + "." + region.value() , zoneId.value()); - String serializedZoneId = "some.illegal.value"; expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Cannot deserialize zone id '" + serializedZoneId + "'"); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java index 94e111455ac..a1beef23dbb 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/JobType.java @@ -154,7 +154,7 @@ public enum JobType { case test: return Optional.of(systemTest); case staging: return Optional.of(stagingTest); } - return from(system, ZoneId.from(system, environment, region)); + return from(system, ZoneId.from(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 5a40dd591fd..156e8d0d242 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 @@ -34,7 +34,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { if (minVersion(zone, application, Node::wantedVersion).map(target::isAfter) .orElse(true)) { log.info(String.format("Deploying %s version %s in %s", application.id(), target, zone.getId())); - controller().applications().deploy(application, zone.toDeprecatedId(), target); + controller().applications().deploy(application, zone.getId(), target); } } @@ -45,7 +45,7 @@ public class SystemUpgrader extends InfrastructureUpgrader { if (minVersion.isEmpty()) return true; return minVersion.get().equals(target) - && application.configConvergedIn(zone.toDeprecatedId(), controller(), Optional.of(target)); + && application.configConvergedIn(zone.getId(), controller(), Optional.of(target)); } @Override 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 700e6e9cb42..57f29fb72af 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 @@ -83,7 +83,7 @@ public class ZoneFilterMock implements ZoneList { @Override public List ids() { - return List.copyOf(zones.stream().map(ZoneApi::toDeprecatedId).collect(Collectors.toList())); + return List.copyOf(zones.stream().map(ZoneApi::getId).collect(Collectors.toList())); } @Override diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java index 39ff29f4ae0..38aa4af4756 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java @@ -57,16 +57,16 @@ public class OsUpgraderTest { ); // Bootstrap system - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId(), zone5.toDeprecatedId()), + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()), List.of(SystemApplication.tenantHost)); // Add system applications that exist in a real system, but are currently not upgraded - tester.configServer().addNodes(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId(), zone5.toDeprecatedId()), + tester.configServer().addNodes(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId(), zone5.getId()), List.of(SystemApplication.configServer)); // Fail a few nodes. Failed nodes should not affect versions - failNodeIn(zone1.toDeprecatedId(), SystemApplication.tenantHost); - failNodeIn(zone3.toDeprecatedId(), SystemApplication.tenantHost); + failNodeIn(zone1.getId(), SystemApplication.tenantHost); + failNodeIn(zone3.getId(), SystemApplication.tenantHost); // New OS version released Version version1 = Version.fromString("7.1"); @@ -78,37 +78,37 @@ public class OsUpgraderTest { // zone 1: begins upgrading osUpgrader.maintain(); - assertWanted(version1, SystemApplication.tenantHost, zone1.toDeprecatedId()); + assertWanted(version1, SystemApplication.tenantHost, zone1.getId()); // Other zones remain on previous version (none) - assertWanted(Version.emptyVersion, SystemApplication.proxy, zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()); + assertWanted(Version.emptyVersion, SystemApplication.proxy, zone2.getId(), zone3.getId(), zone4.getId()); // zone 1: completes upgrade - completeUpgrade(version1, SystemApplication.tenantHost, zone1.toDeprecatedId()); + completeUpgrade(version1, SystemApplication.tenantHost, zone1.getId()); statusUpdater.maintain(); assertEquals(2, nodesOn(version1).size()); assertEquals(11, nodesOn(Version.emptyVersion).size()); // zone 2 and 3: begins upgrading osUpgrader.maintain(); - assertWanted(version1, SystemApplication.proxy, zone2.toDeprecatedId(), zone3.toDeprecatedId()); + assertWanted(version1, SystemApplication.proxy, zone2.getId(), zone3.getId()); // zone 4: still on previous version - assertWanted(Version.emptyVersion, SystemApplication.tenantHost, zone4.toDeprecatedId()); + assertWanted(Version.emptyVersion, SystemApplication.tenantHost, zone4.getId()); // zone 2 and 3: completes upgrade - completeUpgrade(version1, SystemApplication.tenantHost, zone2.toDeprecatedId(), zone3.toDeprecatedId()); + completeUpgrade(version1, SystemApplication.tenantHost, zone2.getId(), zone3.getId()); // zone 4: begins upgrading osUpgrader.maintain(); - assertWanted(version1, SystemApplication.tenantHost, zone4.toDeprecatedId()); + assertWanted(version1, SystemApplication.tenantHost, zone4.getId()); // zone 4: completes upgrade - completeUpgrade(version1, SystemApplication.tenantHost, zone4.toDeprecatedId()); + completeUpgrade(version1, SystemApplication.tenantHost, zone4.getId()); // Next run does nothing as all zones are upgraded osUpgrader.maintain(); - assertWanted(version1, SystemApplication.tenantHost, zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()); + assertWanted(version1, SystemApplication.tenantHost, zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()); statusUpdater.maintain(); assertTrue("All nodes on target version", tester.controller().osVersionStatus().nodesIn(cloud).stream() .allMatch(node -> node.version().equals(version1))); 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 cb5e7cc90a1..7b817c175b8 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 @@ -50,7 +50,7 @@ public class SystemUpgraderTest { Version version1 = Version.fromString("6.5"); // Bootstrap a system without host applications - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()), + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), SystemApplication.configServer, SystemApplication.proxy); // Fail a few nodes. Failed nodes should not affect versions failNodeIn(zone1, SystemApplication.configServer); @@ -144,7 +144,7 @@ public class SystemUpgraderTest { SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); // Bootstrap system - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId()), SystemApplication.configServer, + tester.configServer().bootstrap(List.of(zone1.getId()), SystemApplication.configServer, SystemApplication.proxy); Version version1 = Version.fromString("6.5"); tester.upgradeSystem(version1); @@ -184,7 +184,7 @@ public class SystemUpgraderTest { ); Version version1 = Version.fromString("6.5"); - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId(), zone2.toDeprecatedId(), zone3.toDeprecatedId(), zone4.toDeprecatedId()), SystemApplication.all()); + tester.configServer().bootstrap(List.of(zone1.getId(), zone2.getId(), zone3.getId(), zone4.getId()), SystemApplication.all()); tester.upgradeSystem(version1); systemUpgrader.maintain(); assertCurrentVersion(SystemApplication.all(), version1, zone1, zone2, zone3, zone4); @@ -282,7 +282,7 @@ public class SystemUpgraderTest { public void does_not_deploy_proxy_app_in_zones_without_proxy() { List applications = List.of( SystemApplication.configServerHost, SystemApplication.configServer, SystemApplication.tenantHost); - tester.configServer().bootstrap(List.of(zone1.toDeprecatedId()), applications); + tester.configServer().bootstrap(List.of(zone1.getId()), applications); tester.configServer().disallowConvergenceCheck(SystemApplication.proxy.id()); SystemUpgrader systemUpgrader = systemUpgrader(UpgradePolicy.create().upgrade(zone1)); @@ -309,7 +309,7 @@ public class SystemUpgraderTest { private void convergeServices(SystemApplication application, ZoneApi... zones) { for (ZoneApi zone : zones) { - tester.controllerTester().configServer().convergeServices(application.id(), zone.toDeprecatedId()); + tester.controllerTester().configServer().convergeServices(application.id(), zone.getId()); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java index b2dfd7b4cb6..745a7af203b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/OsApiTest.java @@ -81,12 +81,12 @@ public class OsApiTest extends ControllerContainerTest { // Status is updated after some zones are upgraded upgradeAndUpdateStatus(); - completeUpgrade(zone1.toDeprecatedId()); + completeUpgrade(zone1.getId()); assertFile(new Request("http://localhost:8080/os/v1/"), "versions-partially-upgraded.json"); // All zones are upgraded upgradeAndUpdateStatus(); - completeUpgrade(zone2.toDeprecatedId(), zone3.toDeprecatedId()); + completeUpgrade(zone2.getId(), zone3.getId()); assertFile(new Request("http://localhost:8080/os/v1/"), "versions-all-upgraded.json"); // Downgrade with force is permitted -- cgit v1.2.3