Use correct zone for determining tester flavour

Tester flavour was incorrectly taken from the first zone of the first
instance with a zone deploying to the environment of the current run.

2 files changed, 47 insertions, 12 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
index ee97962ebe2..e5ab59b814a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.google.common.collect.ImmutableSet;
 import com.yahoo.component.Version;
+import com.yahoo.config.application.api.DeploymentInstanceSpec;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.Notifications;
 import com.yahoo.config.application.api.Notifications.When;
@@ -10,6 +11,7 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.AthenzDomain;
 import com.yahoo.config.provision.AthenzService;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.log.LogLevel;
@@ -106,11 +108,12 @@ import static java.util.stream.Collectors.toList;
 public class InternalStepRunner implements StepRunner {
 
     private static final Logger logger = Logger.getLogger(InternalStepRunner.class.getName());
-    private static final NodeResources DEFAULT_TESTER_RESOURCES =
+
+    static final NodeResources DEFAULT_TESTER_RESOURCES =
             new NodeResources(1, 4, 50, 0.3, NodeResources.DiskSpeed.any);
     // Must match exactly the advertised resources of an AWS instance type. Also consider that the container
     // will have ~1.8 GB less memory than equivalent resources in AWS (VESPA-16259).
-    private static final NodeResources DEFAULT_TESTER_RESOURCES_AWS =
+    static final NodeResources DEFAULT_TESTER_RESOURCES_AWS =
             new NodeResources(2, 8, 50, 0.3, NodeResources.DiskSpeed.any);
 
     static final Duration endpointTimeout = Duration.ofMinutes(15);
@@ -765,10 +768,7 @@ public class InternalStepRunner implements StepRunner {
         byte[] servicesXml = servicesXml(controller.zoneRegistry().accessControlDomain(),
                                          ! controller.system().isPublic(),
                                          useTesterCertificate,
-                                         testerFlavorFor(id, spec)
-                                                 .map(NodeResources::fromLegacyName)
-                                                 .orElse(zone.region().value().contains("aws-") ?
-                                                         DEFAULT_TESTER_RESOURCES_AWS : DEFAULT_TESTER_RESOURCES));
+                                         testerResourcesFor(zone, spec.requireInstance(id.application().instance())));
         byte[] testPackage = controller.applications().applicationStore().getTester(id.application().tenant(), id.application().application(), version);
         byte[] deploymentXml = deploymentXml(id.tester(),
                                              spec.athenzDomain(),
@@ -814,12 +814,14 @@ public class InternalStepRunner implements StepRunner {
         return useConfigServer;
     }
 
-    private static Optional<String> testerFlavorFor(RunId id, DeploymentSpec spec) {
-        for (DeploymentSpec.Step step : spec.steps())
-            if (step.concerns(id.type().environment()))
-                return step.zones().get(0).testerFlavor();
-
-        return Optional.empty();
+    static NodeResources testerResourcesFor(ZoneId zone, DeploymentInstanceSpec spec) {
+        return spec.steps().stream()
+                   .filter(step -> step.concerns(zone.environment()))
+                   .findFirst()
+                   .flatMap(step -> step.zones().get(0).testerFlavor())
+                   .map(NodeResources::fromLegacyName)
+                   .orElse(zone.region().value().contains("aws-") ?
+                           DEFAULT_TESTER_RESOURCES_AWS : DEFAULT_TESTER_RESOURCES);
     }
 
     /** Returns the generated services.xml content for the tester application. */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index 9688e98009a..cf5d341042c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -456,6 +456,39 @@ public class InternalStepRunnerTest {
                                            "3554970337.947845\t17491290-v6-1.ostk.bm2.prod.ne1.yahoo.com\t5480\tcontainer\tstderr\twarning\tjava.lang.NullPointerException\\n\\tat org.apache.felix.framework.BundleRevisionImpl.calculateContentPath(BundleRevisionImpl.java:438)\\n\\tat org.apache.felix.framework.BundleRevisionImpl.initializeContentPath(BundleRevisionImpl.java:371)";
 
     @Test
+    public void generates_correct_tester_flavor() {
+        DeploymentSpec spec = DeploymentSpec.fromXml("<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" +
+                                                     "    <instance id='first'>\n" +
+                                                     "        <test tester-flavor=\"d-6-16-100\" />\n" +
+                                                     "        <prod>\n" +
+                                                     "            <region active=\"true\">us-west-1</region>\n" +
+                                                     "            <test>us-west-1</test>\n" +
+                                                     "        </prod>\n" +
+                                                     "    </instance>\n" +
+                                                     "    <instance id='second'>\n" +
+                                                     "        <test />\n" +
+                                                     "        <staging />\n" +
+                                                     "        <prod tester-flavor=\"d-6-16-100\">\n" +
+                                                     "            <parallel>\n" +
+                                                     "                <region active=\"true\">us-east-3</region>\n" +
+                                                     "                <region active=\"true\">us-central-1</region>\n" +
+                                                     "            </parallel>\n" +
+                                                     "            <region active=\"true\">us-west-1</region>\n" +
+                                                     "            <test>us-west-1</test>\n" +
+                                                     "        </prod>\n" +
+                                                     "    </instance>\n" +
+                                                     "</deployment>\n");
+
+        NodeResources firstResources = InternalStepRunner.testerResourcesFor(ZoneId.from("prod", "us-west-1"), spec.requireInstance("first"));
+        assertEquals(InternalStepRunner.DEFAULT_TESTER_RESOURCES, firstResources);
+
+        NodeResources secondResources = InternalStepRunner.testerResourcesFor(ZoneId.from("prod", "us-west-1"), spec.requireInstance("second"));
+        assertEquals(6, secondResources.vcpu(), 1e-9);
+        assertEquals(16, secondResources.memoryGb(), 1e-9);
+        assertEquals(100, secondResources.diskGb(), 1e-9);
+    }
+
+    @Test
     public void generates_correct_services_xml_test() {
         assertFile("test_runner_services.xml-cd", new String(InternalStepRunner.servicesXml(AthenzDomain.from("vespa.vespa.cd"),
                                                                                             true,