Merge pull request #14085 from vespa-engine/hmusum/remove-use-of-flavors-from-allocated-hosts-serialization

Remove use of flavors from allocated hosts serialization

9 files changed, 47 insertions, 80 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
index 62b9cefab78..4c6dccebae4 100644
--- a/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
+++ b/config-model/src/main/java/com/yahoo/config/model/provision/InMemoryProvisioner.java
@@ -197,7 +197,7 @@ public class InMemoryProvisioner implements HostProvisioner {
 
         int nextIndex = nextIndexInCluster.getOrDefault(new Pair<>(clusterGroup.type(), clusterGroup.id()), startIndex);
         while (allocation.size() < nodesInGroup) {
-            // Find the smallest host that can fit the requested requested
+            // Find the smallest host that can fit the requested resources
             Optional<NodeResources> hostResources = freeNodes.keySet().stream()
                     .sorted(new MemoryDiskCpu())
                     .filter(resources -> requestedResources.isUnspecified() || resources.satisfies(requestedResources))
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java
index fa684ff6046..6bc3809bd54 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/HostSpec.java
@@ -45,15 +45,16 @@ public class HostSpec implements Comparable<HostSpec> {
     /** Create a host in a hosted system */
     public HostSpec(String hostname,
                     NodeResources realResources,
-                    NodeResources advertisedResurces,
+                    NodeResources advertisedResources,
                     NodeResources requestedResources,
                     ClusterMembership membership,
                     Optional<Version> version,
                     Optional<NetworkPorts> networkPorts,
                     Optional<DockerImage> dockerImageRepo) {
-        this(hostname, List.of(),
+        this(hostname,
+             List.of(),
              realResources,
-             advertisedResurces,
+             advertisedResources,
              requestedResources,
              Optional.of(membership),
              version,
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java
index 96b189cefe6..e4913643e8d 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializer.java
@@ -6,7 +6,6 @@ import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.HostSpec;
-import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Cursor;
@@ -16,7 +15,6 @@ import com.yahoo.slime.SlimeUtils;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Optional;
@@ -93,7 +91,7 @@ public class AllocatedHostsSerializer {
                 object.setString(hostSpecDockerImageRepoKey, repo.repository());
             });
         });
-        host.flavor().ifPresent(flavor -> toSlime(flavor, object)); // TODO: Remove this line after June 2020
+        host.flavor().ifPresent(flavor -> toSlime(flavor, object)); // TODO: Remove this line when 7.272 has been released
         toSlime(host.realResources(), object.setObject(realResourcesKey));
         toSlime(host.advertisedResources(), object.setObject(advertisedResourcesKey));
         host.requestedResources().ifPresent(resources -> toSlime(resources, object.setObject(requestedResourcesKey)));
@@ -124,25 +122,26 @@ public class AllocatedHostsSerializer {
         resourcesObject.setString(storageTypeKey, storageTypeToString(resources.storageType()));
     }
 
-    public static AllocatedHosts fromJson(byte[] json, Optional<NodeFlavors> nodeFlavors) {
-        return fromSlime(SlimeUtils.jsonToSlime(json).get(), nodeFlavors);
+    public static AllocatedHosts fromJson(byte[] json) {
+        return fromSlime(SlimeUtils.jsonToSlime(json).get());
     }
 
-    public static AllocatedHosts fromSlime(Inspector inspector, Optional<NodeFlavors> nodeFlavors) {
+    public static AllocatedHosts fromSlime(Inspector inspector) {
         Inspector array = inspector.field(mappingKey);
         Set<HostSpec> hosts = new LinkedHashSet<>();
         array.traverse((ArrayTraverser)(i, host) -> {
-            hosts.add(hostFromSlime(host.field(hostSpecKey), nodeFlavors));
+            hosts.add(hostFromSlime(host.field(hostSpecKey)));
         });
         return AllocatedHosts.withHosts(hosts);
     }
 
-    private static HostSpec hostFromSlime(Inspector object, Optional<NodeFlavors> nodeFlavors) {
+    private static HostSpec hostFromSlime(Inspector object) {
+
         if (object.field(hostSpecMembershipKey).valid()) { // Hosted
             return new HostSpec(object.field(hostSpecHostNameKey).asString(),
-                                nodeResourcesFromSlime(object.field(realResourcesKey), object, nodeFlavors),
-                                nodeResourcesFromSlime(object.field(advertisedResourcesKey), object, nodeFlavors),
-                                optionalNodeResourcesFromSlime(object.field(requestedResourcesKey)), // TODO: Make non-optional after June 2020
+                                nodeResourcesFromSlime(object.field(realResourcesKey)),
+                                nodeResourcesFromSlime(object.field(advertisedResourcesKey)),
+                                optionalNodeResourcesFromSlime(object.field(requestedResourcesKey)), // TODO: Make non-optional when we serialize NodeResources.unspecified()
                                 membershipFromSlime(object),
                                 optionalString(object.field(hostSpecCurrentVespaVersionKey)).map(com.yahoo.component.Version::new),
                                 NetworkPortsSerializer.fromSlime(object.field(hostSpecNetworkPortsKey)),
@@ -162,13 +161,6 @@ public class AllocatedHostsSerializer {
         return aliases;
     }
 
-    private static Optional<Flavor> flavorFromSlime(Inspector object, Optional<NodeFlavors> nodeFlavors) {
-        if (object.field(flavorKey).valid() && nodeFlavors.isPresent() && nodeFlavors.get().exists(object.field(flavorKey).asString()))
-            return nodeFlavors.get().getFlavor(object.field(flavorKey).asString());
-        else
-            return Optional.empty();
-    }
-
     private static NodeResources nodeResourcesFromSlime(Inspector resources) {
         return new NodeResources(resources.field(vcpuKey).asDouble(),
                                  resources.field(memoryKey).asDouble(),
@@ -183,13 +175,6 @@ public class AllocatedHostsSerializer {
         return nodeResourcesFromSlime(resources);
     }
 
-    private static NodeResources nodeResourcesFromSlime(Inspector resources, Inspector parent,
-                                                        Optional<NodeFlavors> nodeFlavors) {
-        if ( ! resources.valid()) // TODO: Remove the fallback using nodeFlavors after June 2020
-            return flavorFromSlime(parent, nodeFlavors).map(f -> f.resources()).orElse(NodeResources.unspecified);
-        return nodeResourcesFromSlime(resources);
-    }
-
     private static NodeResources.DiskSpeed diskSpeedFromSlime(Inspector diskSpeed) {
         switch (diskSpeed.asString()) {
             case "fast" : return NodeResources.DiskSpeed.fast;
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java
index 30810b79104..700bc389664 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/serialization/AllocatedHostsSerializerTest.java
@@ -5,12 +5,9 @@ import com.yahoo.component.Version;
 import com.yahoo.config.provision.AllocatedHosts;
 import com.yahoo.config.provision.ClusterMembership;
 import com.yahoo.config.provision.DockerImage;
-import com.yahoo.config.provision.Flavor;
 import com.yahoo.config.provision.HostSpec;
 import com.yahoo.config.provision.NetworkPorts;
-import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.NodeResources;
-import com.yahoo.config.provisioning.FlavorsConfig;
 import org.junit.Test;
 
 import java.io.IOException;
@@ -28,42 +25,45 @@ import static org.junit.Assert.assertEquals;
  */
 public class AllocatedHostsSerializerTest {
 
+    private static final NodeResources smallSlowDiskSpeedNode = new NodeResources(0.5, 3.1, 4, 1, NodeResources.DiskSpeed.slow);
+    private static final NodeResources bigSlowDiskSpeedNode = new NodeResources(1.0, 6.2, 8, 2, NodeResources.DiskSpeed.slow);
+    private static final NodeResources anyDiskSpeedNode = new NodeResources(0.5, 3.1, 4, 1, NodeResources.DiskSpeed.any);
+
     @Test
     public void testAllocatedHostsSerialization() throws IOException {
-        NodeFlavors configuredFlavors = configuredFlavorsFrom("C/12/45/100", 12, 45, 100, 50, Flavor.Type.BARE_METAL);
         Set<HostSpec> hosts = new LinkedHashSet<>();
         hosts.add(new HostSpec("empty", List.of(), Optional.empty()));
         hosts.add(new HostSpec("with-aliases", List.of("alias1", "alias2"), Optional.empty()));
         hosts.add(new HostSpec("allocated",
-                               NodeResources.unspecified(),
-                               NodeResources.unspecified(),
-                               NodeResources.unspecified(),
+                               smallSlowDiskSpeedNode,
+                               bigSlowDiskSpeedNode,
+                               anyDiskSpeedNode,
                                ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"),
                                                       Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"))),
                                Optional.empty(),
                                Optional.empty(),
                                Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"))));
         hosts.add(new HostSpec("flavor-from-resources-2",
-                               new NodeResources(0.5, 3.1, 4, 1, NodeResources.DiskSpeed.slow),
-                               new NodeResources(1.0, 6.2, 8, 2, NodeResources.DiskSpeed.slow),
-                               new NodeResources(0.5, 3.1, 4, 1, NodeResources.DiskSpeed.any),
+                               smallSlowDiskSpeedNode,
+                               bigSlowDiskSpeedNode,
+                               anyDiskSpeedNode,
                                ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"),
                                                       Optional.empty()),
                                Optional.empty(),
                                Optional.empty(),
                                Optional.empty()));
         hosts.add(new HostSpec("with-version",
-                               NodeResources.unspecified(),
-                               NodeResources.unspecified(),
-                               NodeResources.unspecified(),
+                               smallSlowDiskSpeedNode,
+                               bigSlowDiskSpeedNode,
+                               anyDiskSpeedNode,
                                ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"),
                                                       Optional.empty()),
                                Optional.of(Version.fromString("3.4.5")),
                                Optional.empty(), Optional.empty()));
         hosts.add(new HostSpec("with-ports",
-                               NodeResources.unspecified(),
-                               NodeResources.unspecified(),
-                               NodeResources.unspecified(),
+                               smallSlowDiskSpeedNode,
+                               bigSlowDiskSpeedNode,
+                               anyDiskSpeedNode,
                                ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"),
                                                       Optional.empty()),
                                Optional.empty(),
@@ -71,11 +71,11 @@ public class AllocatedHostsSerializerTest {
                                                                     new NetworkPorts.Allocation(4567, "service2", "configId2", "suffix2")))),
                                Optional.empty()));
 
-        assertAllocatedHosts(AllocatedHosts.withHosts(hosts), configuredFlavors);
+        assertAllocatedHosts(AllocatedHosts.withHosts(hosts));
     }
 
-    private void assertAllocatedHosts(AllocatedHosts expectedHosts, NodeFlavors configuredFlavors) throws IOException {
-        AllocatedHosts deserializedHosts = fromJson(toJson(expectedHosts), Optional.of(configuredFlavors));
+    private void assertAllocatedHosts(AllocatedHosts expectedHosts) throws IOException {
+        AllocatedHosts deserializedHosts = fromJson(toJson(expectedHosts));
 
         assertEquals(expectedHosts, deserializedHosts);
         for (HostSpec expectedHost : expectedHosts.getHosts()) {
@@ -99,17 +99,4 @@ public class AllocatedHostsSerializerTest {
         throw new IllegalArgumentException("No host " + hostname + " is present");
     }
 
-    private NodeFlavors configuredFlavorsFrom(String flavorName, double cpu, double mem, double disk, double bandwidth, Flavor.Type type) {
-        FlavorsConfig.Builder b = new FlavorsConfig.Builder();
-        FlavorsConfig.Flavor.Builder flavor = new FlavorsConfig.Flavor.Builder();
-        flavor.name(flavorName);
-        flavor.minDiskAvailableGb(disk);
-        flavor.minCpuCores(cpu);
-        flavor.minMainMemoryAvailableGb(mem);
-        flavor.bandwidth(bandwidth);
-        flavor.environment(type.name());
-        b.flavor(flavor);
-        return new NodeFlavors(b.build());
-    }
-
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
index b38d90d470f..6c4ef469be6 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
@@ -9,7 +9,6 @@ import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.application.provider.DeployData;
 import com.yahoo.config.model.application.provider.FilesApplicationPackage;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.io.IOUtils;
 import com.yahoo.path.Path;
@@ -653,9 +652,8 @@ public class SessionRepository {
 
     private SessionZooKeeperClient createSessionZooKeeperClient(long sessionId) {
         String serverId = componentRegistry.getConfigserverConfig().serverId();
-        Optional<NodeFlavors> nodeFlavors = componentRegistry.getZone().nodeFlavors();
         Path sessionPath = getSessionPath(sessionId);
-        return new SessionZooKeeperClient(curator, componentRegistry.getConfigCurator(), sessionPath, serverId, nodeFlavors);
+        return new SessionZooKeeperClient(curator, componentRegistry.getConfigCurator(), sessionPath, serverId);
     }
 
     private File getAndValidateExistingSessionAppDir(long sessionId) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java
index 27440b3765b..cf1e07788ff 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java
@@ -51,24 +51,21 @@ public class SessionZooKeeperClient {
     private final Path sessionPath;
     private final Path sessionStatusPath;
     private final String serverId;  // hostname
-    private final Optional<NodeFlavors> nodeFlavors;
 
     // Only for testing
     // TODO: Remove, use the constructor below
     public SessionZooKeeperClient(Curator curator, Path sessionPath) {
-        this(curator, ConfigCurator.create(curator), sessionPath, "1", Optional.empty());
+        this(curator, ConfigCurator.create(curator), sessionPath, "1");
     }
 
     public SessionZooKeeperClient(Curator curator,
                                   ConfigCurator configCurator,
                                   Path sessionPath,
-                                  String serverId,
-                                  Optional<NodeFlavors> nodeFlavors) {
+                                  String serverId) {
         this.curator = curator;
         this.configCurator = configCurator;
         this.sessionPath = sessionPath;
         this.serverId = serverId;
-        this.nodeFlavors = nodeFlavors;
         this.sessionStatusPath = sessionPath.append(ConfigCurator.SESSIONSTATE_ZK_SUBPATH);
     }
 
@@ -135,7 +132,7 @@ public class SessionZooKeeperClient {
     }
 
     public ApplicationPackage loadApplicationPackage() {
-        return new ZKApplicationPackage(configCurator, sessionPath, nodeFlavors);
+        return new ZKApplicationPackage(configCurator, sessionPath);
     }
 
     public ConfigDefinitionRepo getUserConfigDefinitions() {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java
index 11cec9efd95..665f37759b4 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackage.java
@@ -13,7 +13,6 @@ import com.yahoo.config.codegen.DefParser;
 import com.yahoo.config.model.application.provider.PreGeneratedFileRegistry;
 import com.yahoo.config.provision.AllocatedHosts;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.NodeFlavors;
 import com.yahoo.config.provision.serialization.AllocatedHostsSerializer;
 import com.yahoo.io.IOUtils;
 import com.yahoo.io.reader.NamedReader;
@@ -52,17 +51,17 @@ public class ZKApplicationPackage implements ApplicationPackage {
     public static final String allocatedHostsNode = "allocatedHosts";
     private final ApplicationMetaData metaData;
 
-    public ZKApplicationPackage(ConfigCurator zk, Path sessionPath, Optional<NodeFlavors> nodeFlavors) {
+    public ZKApplicationPackage(ConfigCurator zk, Path sessionPath) {
         verifyAppPath(zk, sessionPath);
         zkApplication = new ZKApplication(zk, sessionPath);
         metaData = readMetaDataFromLiveApp(zkApplication);
         importFileRegistries();
-        allocatedHosts = importAllocatedHosts(nodeFlavors);
+        allocatedHosts = importAllocatedHosts();
     }
 
-    private Optional<AllocatedHosts> importAllocatedHosts(Optional<NodeFlavors> nodeFlavors) {
+    private Optional<AllocatedHosts> importAllocatedHosts() {
         if ( ! zkApplication.exists(ZKApplicationPackage.allocatedHostsNode)) return Optional.empty();
-        return Optional.of(readAllocatedHosts(nodeFlavors));
+        return Optional.of(readAllocatedHosts());
     }
 
     /**
@@ -70,9 +69,9 @@ public class ZKApplicationPackage implements ApplicationPackage {
      *
      * @return the allocated hosts at this node or empty if there is no data at this path
      */
-    private AllocatedHosts readAllocatedHosts(Optional<NodeFlavors> nodeFlavors) {
+    private AllocatedHosts readAllocatedHosts() {
         try {
-            return AllocatedHostsSerializer.fromJson(zkApplication.getBytes(ZKApplicationPackage.allocatedHostsNode), nodeFlavors);
+            return AllocatedHostsSerializer.fromJson(zkApplication.getBytes(ZKApplicationPackage.allocatedHostsNode));
         } catch (Exception e) {
             throw new RuntimeException("Unable to read allocated hosts", e);
         }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java
index dfc5649433a..deff0aba376 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/ZooKeeperClientTest.java
@@ -165,7 +165,7 @@ public class ZooKeeperClientTest {
         Path hostsPath = app.append(ZKApplicationPackage.allocatedHostsNode);
         assertTrue(zk.exists(hostsPath.getAbsolute()));
         
-        AllocatedHosts deserialized = fromJson(zk.getBytes(hostsPath.getAbsolute()), Optional.empty());
+        AllocatedHosts deserialized = fromJson(zk.getBytes(hostsPath.getAbsolute()));
         assertEquals(hosts, deserialized.getHosts());
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java
index 4b4605cce7d..32d5d84f323 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/zookeeper/ZKApplicationPackageTest.java
@@ -48,7 +48,7 @@ public class ZKApplicationPackageTest {
             Collections.singleton(new HostSpec("foo.yahoo.com",
                                                TEST_FLAVOR.get().resources(),
                                                TEST_FLAVOR.get().resources(),
-                                               NodeResources.unspecified(),
+                                               TEST_FLAVOR.get().resources(),
                                                ClusterMembership.from("container/test/0/0", Version.fromString("6.73.1"),
                                                                       Optional.of(DockerImage.fromString("docker.foo.com:4443/vespa/bar"))),
                                                Optional.of(Version.fromString("6.0.1")), Optional.empty(),
@@ -67,7 +67,7 @@ public class ZKApplicationPackageTest {
     @Test
     public void testBasicZKFeed() throws IOException {
         feed(configCurator, new File(APP));
-        ZKApplicationPackage zkApp = new ZKApplicationPackage(configCurator, Path.fromString("/0"), Optional.of(new MockNodeFlavors()));
+        ZKApplicationPackage zkApp = new ZKApplicationPackage(configCurator, Path.fromString("/0"));
         assertTrue(Pattern.compile(".*<slobroks>.*",Pattern.MULTILINE+Pattern.DOTALL).matcher(IOUtils.readAll(zkApp.getServices())).matches());
         assertTrue(Pattern.compile(".*<alias>.*",Pattern.MULTILINE+Pattern.DOTALL).matcher(IOUtils.readAll(zkApp.getHosts())).matches());
         assertTrue(Pattern.compile(".*<slobroks>.*",Pattern.MULTILINE+Pattern.DOTALL).matcher(IOUtils.readAll(zkApp.getFile(Path.fromString("services.xml")).createReader())).matches());