Use ResourceAllocation less. NodeResources does the job

4 files changed, 40 insertions, 28 deletions

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 d47b82a231f..059986072f3 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
@@ -120,7 +120,7 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
                     Map<ZoneId, Double> deploymentCosts = snapshotsByInstance.getOrDefault(instanceName, List.of()).stream()
                             .collect(Collectors.toUnmodifiableMap(
                                     ResourceSnapshot::getZoneId,
-                                    snapshot -> cost(snapshot.allocation(), systemName),
+                                    snapshot -> cost(snapshot.resources(), systemName),
                                     Double::sum));
                     locked = locked.with(instanceName, i -> i.withDeploymentCosts(deploymentCosts));
                     updateCostMetrics(tenantAndApplication.instance(instanceName), deploymentCosts);
@@ -234,10 +234,14 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
     }
 
     private static double cost(ResourceAllocation allocation, SystemName systemName) {
+        var resources = new NodeResources(allocation.getCpuCores(), allocation.getMemoryGb(), allocation.getDiskGb(), 0);
+        return cost(resources, systemName);
+    }
+
+    private static double cost(NodeResources resources, SystemName systemName) {
         // Divide cost by 3 in non-public zones to show approx. AWS equivalent cost
         double costDivisor = systemName.isPublic() ? 1.0 : 3.0;
-        double cost = new NodeResources(allocation.getCpuCores(), allocation.getMemoryGb(), allocation.getDiskGb(), 0).cost();
-        return Math.round(cost * 100.0 / costDivisor) / 100.0;
+        return Math.round(resources.cost() * 100.0 / costDivisor) / 100.0;
     }
 
     private void updateMeteringMetrics(Collection<ResourceSnapshot> resourceSnapshots) {
@@ -245,14 +249,14 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
         // total metered resource usage, for alerting on drastic changes
         metric.set(METERING_TOTAL_REPORTED,
                 resourceSnapshots.stream()
-                        .mapToDouble(r -> r.getCpuCores() + r.getMemoryGb() + r.getDiskGb()).sum(),
+                        .mapToDouble(r -> r.resources().vcpu() + r.resources().memoryGb() + r.resources().diskGb()).sum(),
                 metric.createContext(Collections.emptyMap()));
 
         resourceSnapshots.forEach(snapshot -> {
             var context = getMetricContext(snapshot);
-            metric.set("metering.vcpu", snapshot.getCpuCores(), context);
-            metric.set("metering.memoryGB", snapshot.getMemoryGb(), context);
-            metric.set("metering.diskGB", snapshot.getDiskGb(), context);
+            metric.set("metering.vcpu", snapshot.resources().vcpu(), context);
+            metric.set("metering.memoryGB", snapshot.resources().memoryGb(), context);
+            metric.set("metering.diskGB", snapshot.resources().diskGb(), context);
         });
     }
 
@@ -276,7 +280,7 @@ public class ResourceMeterMaintainer extends ControllerMaintainer {
                 "tenant", snapshot.getApplicationId().tenant().value(),
                 "applicationId", snapshot.getApplicationId().toFullString(),
                 "zoneId", snapshot.getZoneId().value(),
-                "architecture", snapshot.getArchitecture()
+                "architecture", snapshot.resources().architecture()
         ));
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/MeteringResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/MeteringResponse.java
index 17461aafd02..7df216d6c9c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/MeteringResponse.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/MeteringResponse.java
@@ -29,10 +29,10 @@ public class MeteringResponse extends SlimeJsonResponse {
             object.setString("applicationId", snapshot.getApplicationId().toShortString());
             object.setLong("timestamp", snapshot.getTimestamp().toEpochMilli());
             object.setString("zoneId", snapshot.getZoneId().value());
-            object.setDouble("cpu", snapshot.getCpuCores());
-            object.setDouble("memory", snapshot.getMemoryGb());
-            object.setDouble("disk", snapshot.getDiskGb());
-            object.setString("architecture", snapshot.getArchitecture().name());
+            object.setDouble("cpu", snapshot.resources().vcpu());
+            object.setDouble("memory", snapshot.resources().memoryGb());
+            object.setDouble("disk", snapshot.resources().diskGb());
+            object.setString("architecture", snapshot.resources().architecture().name());
         });
         return slime;
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
index cecf0b1a2cf..3034c93e593 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
@@ -61,18 +61,20 @@ public class ResourceMeterMaintainerTest {
                         .collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().cost().getAsDouble())));
 
         List<ResourceSnapshot> resourceSnapshots = List.of(
-                new ResourceSnapshot(app1, 12, 34, 56, NodeResources.Architecture.getDefault(), Instant.EPOCH, z1, Version.emptyVersion),
-                new ResourceSnapshot(app1, 23, 45, 67, NodeResources.Architecture.getDefault(), Instant.EPOCH, z2, Version.emptyVersion),
-                new ResourceSnapshot(app2, 34, 56, 78, NodeResources.Architecture.getDefault(), Instant.EPOCH, z1, Version.emptyVersion));
+                new ResourceSnapshot(app1, resources(12, 34, 56), Instant.EPOCH, z1, Version.emptyVersion),
+                new ResourceSnapshot(app1, resources(23, 45, 67), Instant.EPOCH, z2, Version.emptyVersion),
+                new ResourceSnapshot(app2, resources(34, 56, 78), Instant.EPOCH, z1, Version.emptyVersion));
+
         maintainer.updateDeploymentCost(resourceSnapshots);
         assertCost.accept(app1, Map.of(z1, 1.72, z2, 3.05));
         assertCost.accept(app2, Map.of(z1, 4.39));
 
         // Remove a region from app1 and add region to app2
         resourceSnapshots = List.of(
-                new ResourceSnapshot(app1, 23, 45, 67, NodeResources.Architecture.getDefault(), Instant.EPOCH, z2, Version.emptyVersion),
-                new ResourceSnapshot(app2, 34, 56, 78, NodeResources.Architecture.getDefault(), Instant.EPOCH, z1, Version.emptyVersion),
-                new ResourceSnapshot(app2, 45, 67, 89, NodeResources.Architecture.getDefault(), Instant.EPOCH, z2, Version.emptyVersion));
+                new ResourceSnapshot(app1, resources(23, 45, 67), Instant.EPOCH, z2, Version.emptyVersion),
+                new ResourceSnapshot(app2, resources(34, 56, 78), Instant.EPOCH, z1, Version.emptyVersion),
+                new ResourceSnapshot(app2, resources(45, 67, 89), Instant.EPOCH, z2, Version.emptyVersion));
+
         maintainer.updateDeploymentCost(resourceSnapshots);
         assertCost.accept(app1, Map.of(z2, 3.05));
         assertCost.accept(app2, Map.of(z1, 4.39, z2, 5.72));
@@ -97,13 +99,13 @@ public class ResourceMeterMaintainerTest {
         ResourceSnapshot app1 = consumedResources.stream().filter(snapshot -> snapshot.getApplicationId().equals(ApplicationId.from("tenant1", "app1", "default"))).findFirst().orElseThrow();
         ResourceSnapshot app2 = consumedResources.stream().filter(snapshot -> snapshot.getApplicationId().equals(ApplicationId.from("tenant2", "app2", "default"))).findFirst().orElseThrow();
 
-        assertEquals(24, app1.getCpuCores(), Double.MIN_VALUE);
-        assertEquals(24, app1.getMemoryGb(), Double.MIN_VALUE);
-        assertEquals(500, app1.getDiskGb(), Double.MIN_VALUE);
+        assertEquals(24, app1.resources().vcpu(), Double.MIN_VALUE);
+        assertEquals(24, app1.resources().memoryGb(), Double.MIN_VALUE);
+        assertEquals(500, app1.resources().diskGb(), Double.MIN_VALUE);
 
-        assertEquals(40, app2.getCpuCores(), Double.MIN_VALUE);
-        assertEquals(24, app2.getMemoryGb(), Double.MIN_VALUE);
-        assertEquals(500, app2.getDiskGb(), Double.MIN_VALUE);
+        assertEquals(40, app2.resources().vcpu(), Double.MIN_VALUE);
+        assertEquals(24, app2.resources().memoryGb(), Double.MIN_VALUE);
+        assertEquals(500, app2.resources().diskGb(), Double.MIN_VALUE);
 
         assertEquals(tester.clock().millis() / 1000, metrics.getMetric("metering_last_reported"));
         assertEquals(2224.0d, (Double) metrics.getMetric("metering_total_reported"), Double.MIN_VALUE);
@@ -154,4 +156,8 @@ public class ResourceMeterMaintainerTest {
                                        .build())
                      .toList();
     }
+
+    private NodeResources resources(double cpu, double ram, double disk) {
+        return new NodeResources(cpu, ram, disk, 0, NodeResources.DiskSpeed.getDefault(), NodeResources.StorageType.getDefault(), NodeResources.Architecture.getDefault(), NodeResources.GpuResources.getDefault());
+    }
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
index 9adfc83da5c..ac7287b7e27 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java
@@ -158,10 +158,12 @@ public class ControllerApiTest extends ControllerContainerTest {
         ApplicationId applicationId = ApplicationId.from("tenant", "app", "instance");
         Instant timestamp = Instant.ofEpochMilli(123456789);
         ZoneId zoneId = ZoneId.defaultId();
-        List<ResourceSnapshot> snapshots = List.of(
-                new ResourceSnapshot(applicationId, 12, 48, 1200, NodeResources.Architecture.arm64, timestamp, zoneId, Version.emptyVersion),
-                new ResourceSnapshot(applicationId, 24, 96, 2400,  NodeResources.Architecture.x86_64, timestamp, zoneId, Version.emptyVersion)
-        );
+        var resources = List.of(
+                new NodeResources(12, 48, 1200, 0, NodeResources.DiskSpeed.any, NodeResources.StorageType.any, NodeResources.Architecture.arm64),
+                new NodeResources(24, 96, 2400, 0, NodeResources.DiskSpeed.any, NodeResources.StorageType.any, NodeResources.Architecture.x86_64));
+
+        var snapshots = resources.stream().map(x -> new ResourceSnapshot(applicationId, x, timestamp, zoneId, Version.emptyVersion)).toList();
+
         tester.controller().serviceRegistry().resourceDatabase().writeResourceSnapshots(snapshots);
         tester.assertResponse(
                 operatorRequest("http://localhost:8080/controller/v1/metering/tenant/tenantName/month/2020-02", "", Request.Method.GET),