Merge pull request #9823 from vespa-engine/hakonhall/remove-system-from-zoneid

Remove system from ZoneId

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<ZoneId> 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<SystemApplication> 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