Merge pull request #12046 from vespa-engine/hmusum/remove-support-for-using-tester-endpoint

Remove support for using tester endpoint

7 files changed, 22 insertions, 161 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 a64ee8876b2..a6ebea7fbdf 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
@@ -11,18 +11,13 @@ 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;
 import com.yahoo.security.KeyAlgorithm;
 import com.yahoo.security.KeyUtils;
 import com.yahoo.security.SignatureAlgorithm;
 import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.security.X509CertificateUtils;
-import com.yahoo.vespa.flags.BooleanFlag;
-import com.yahoo.vespa.flags.FetchVector;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.Instance;
@@ -413,17 +408,10 @@ public class InternalStepRunner implements StepRunner {
         logger.log(nodeList.asList().stream()
                            .flatMap(node -> nodeDetails(node, false))
                            .collect(toList()));
-        if (nodeList.summary().converged()) {
-            if (endpointsAvailable(testerId, zone, logger)) {
-                if (testerContainersAreUp(testerId, zone, logger)) {
-                    logger.log("Tester container successfully installed!");
-                    return Optional.of(running);
-                }
-            }
-            else if (run.stepInfo(installTester).get().startTime().get().plus(endpointTimeout).isBefore(controller.clock().instant())) {
-                logger.log(WARNING, "Tester failed to show up within " + endpointTimeout.toMinutes() + " minutes!");
-                return Optional.of(error);
-            }
+
+        if (nodeList.summary().converged() && testerContainersAreUp(testerId, zone, logger)) {
+            logger.log("Tester container successfully installed!");
+            return Optional.of(running);
         }
 
         if (run.stepInfo(installTester).get().startTime().get().plus(testerTimeout).isBefore(controller.clock().instant())) {
@@ -435,16 +423,13 @@ public class InternalStepRunner implements StepRunner {
     }
 
     /** Returns true iff all containers in the deployment give 100 consecutive 200 OK responses on /status.html. */
-    // TODO: Change implementation to only be used for real deployments when useConfigServerForTesterAPI() always returns true
     private boolean containersAreUp(ApplicationId id, ZoneId zoneId, DualLogger logger) {
         var endpoints = controller.routingController().zoneEndpointsOf(Set.of(new DeploymentId(id, zoneId)));
         if ( ! endpoints.containsKey(zoneId))
             return false;
 
         for (URI endpoint : endpoints.get(zoneId).values()) {
-            boolean ready = id.instance().isTester() ? controller.jobController().cloud().testerReady(endpoint)
-                                                     : controller.jobController().cloud().ready(endpoint);
-
+            boolean ready = controller.jobController().cloud().ready(endpoint);
             if (!ready) {
                 logger.log("Failed to get 100 consecutive OKs from " + endpoint);
                 return false;
@@ -456,23 +441,16 @@ public class InternalStepRunner implements StepRunner {
 
     /** Returns true iff all containers in the tester deployment give 100 consecutive 200 OK responses on /status.html. */
     private boolean testerContainersAreUp(ApplicationId id, ZoneId zoneId, DualLogger logger) {
-        if (useConfigServerForTesterAPI(zoneId)) {
-            DeploymentId deploymentId = new DeploymentId(id, zoneId);
-            if (controller.jobController().cloud().testerReady(deploymentId)) {
-                return true;
-            } else {
-                logger.log("Failed to get 100 consecutive OKs from tester container for " + deploymentId);
-                return false;
-            }
+        DeploymentId deploymentId = new DeploymentId(id, zoneId);
+        if (controller.jobController().cloud().testerReady(deploymentId)) {
+            return true;
         } else {
-            return containersAreUp(id, zoneId, logger);
+            logger.log("Failed to get 100 consecutive OKs from tester container for " + deploymentId);
+            return false;
         }
     }
 
-
     private boolean endpointsAvailable(ApplicationId id, ZoneId zone, DualLogger logger) {
-        if (useConfigServerForTesterAPI(zone) && id.instance().isTester()) return true; // Endpoints not used in this case, always return true
-
         var endpoints = controller.routingController().zoneEndpointsOf(Set.of(new DeploymentId(id, zone)));
         if ( ! endpoints.containsKey(zone)) {
             logger.log("Endpoints not yet ready.");
@@ -553,23 +531,9 @@ public class InternalStepRunner implements StepRunner {
         }
         logEndpoints(endpoints, logger);
 
-        Optional<URI> testerEndpoint = Optional.empty();
-        if (useConfigServerForTesterAPI(zoneId)) {
-            if ( ! controller.jobController().cloud().testerReady(getTesterDeploymentId(id))) {
-                logger.log(WARNING, "Tester container went bad!");
-                return Optional.of(error);
-            }
-        } else {
-            testerEndpoint = controller.jobController().testerEndpoint(id);
-            if (testerEndpoint.isEmpty()) {
-                logger.log(WARNING, "Endpoints for the tester container vanished again, while it was still active!");
-                return Optional.of(error);
-            }
-
-            if ( ! controller.jobController().cloud().testerReady(testerEndpoint.get())) {
-                logger.log(WARNING, "Tester container went bad!");
-                return Optional.of(error);
-            }
+        if (!controller.jobController().cloud().testerReady(getTesterDeploymentId(id))) {
+            logger.log(WARNING, "Tester container went bad!");
+            return Optional.of(error);
         }
 
         logger.log("Starting tests ...");
@@ -579,11 +543,7 @@ public class InternalStepRunner implements StepRunner {
                                                         true,
                                                         endpoints,
                                                         controller.applications().contentClustersByZone(deployments));
-        if (useConfigServerForTesterAPI(zoneId)) {
-            controller.jobController().cloud().startTests(getTesterDeploymentId(id), suite, config);
-        } else {
-            controller.jobController().cloud().startTests(testerEndpoint.get(), suite, config);
-        }
+        controller.jobController().cloud().startTests(getTesterDeploymentId(id), suite, config);
         return Optional.of(running);
     }
 
@@ -606,18 +566,7 @@ public class InternalStepRunner implements StepRunner {
 
         controller.jobController().updateTestLog(id);
 
-        TesterCloud.Status testStatus;
-        if (useConfigServerForTesterAPI(id.type().zone(controller.system()))) {
-            testStatus = controller.jobController().cloud().getStatus(getTesterDeploymentId(id));
-        } else {
-            Optional<URI> testerEndpoint = controller.jobController().testerEndpoint(id);
-            if (testerEndpoint.isEmpty()) {
-                logger.log("Endpoints for tester not found -- trying again later.");
-                return Optional.empty();
-            }
-            testStatus = controller.jobController().cloud().getStatus(testerEndpoint.get());
-        }
-
+        TesterCloud.Status testStatus = controller.jobController().cloud().getStatus(getTesterDeploymentId(id));
         switch (testStatus) {
             case NOT_STARTED:
                 throw new IllegalStateException("Tester reports tests not started, even though they should have!");
@@ -806,14 +755,6 @@ public class InternalStepRunner implements StepRunner {
         return new DeploymentId(runId.tester().id(), zoneId);
     }
 
-    private boolean useConfigServerForTesterAPI(ZoneId zoneId) {
-        BooleanFlag useConfigServerForTesterAPI = Flags.USE_CONFIG_SERVER_FOR_TESTER_API_CALLS.bindTo(controller.flagSource());
-        boolean useConfigServer = useConfigServerForTesterAPI.with(FetchVector.Dimension.ZONE_ID, zoneId.value()).value();
-        InternalStepRunner.logger.log(LogLevel.INFO, Flags.USE_CONFIG_SERVER_FOR_TESTER_API_CALLS.id().toString() +
-                                                     " has value " + useConfigServer + " in zone " + zoneId.value());
-        return useConfigServer;
-    }
-
     static NodeResources testerResourcesFor(ZoneId zone, DeploymentInstanceSpec spec) {
         return spec.steps().stream()
                    .filter(step -> step.concerns(zone.environment()))
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 10021441943..d8d9431dc22 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -7,9 +7,6 @@ import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.curator.Lock;
-import com.yahoo.vespa.flags.BooleanFlag;
-import com.yahoo.vespa.flags.FetchVector;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.Instance;
@@ -183,18 +180,8 @@ public class JobController {
             if (step.isEmpty())
                 return run;
 
-            List<LogEntry> entries;
-            ZoneId zone = id.type().zone(controller.system());
-            if (useConfigServerForTesterAPI(zone)) {
-                var testerId = new DeploymentId(id.tester().id(), zone);
-                entries = cloud.getLog(testerId, run.lastTestLogEntry());
-            } else {
-                Optional<URI> testerEndpoint = testerEndpoint(id);
-                if (testerEndpoint.isEmpty())
-                    return run;
-
-                entries = cloud.getLog(testerEndpoint.get(), run.lastTestLogEntry());
-            }
+            List<LogEntry> entries = cloud.getLog(new DeploymentId(id.tester().id(), id.type().zone(controller.system())),
+                                                  run.lastTestLogEntry());
             if (entries.isEmpty())
                 return run;
 
@@ -595,9 +582,4 @@ public class JobController {
         }
     }
 
-    private boolean useConfigServerForTesterAPI(ZoneId zoneId) {
-        BooleanFlag useConfigServerForTesterAPI = Flags.USE_CONFIG_SERVER_FOR_TESTER_API_CALLS.bindTo(controller.flagSource());
-        return useConfigServerForTesterAPI.with(FetchVector.Dimension.ZONE_ID, zoneId.value()).value();
-    }
-
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
index 2e74574f3c3..3e1f468691c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentContext.java
@@ -409,11 +409,6 @@ public class DeploymentContext {
         return this;
     }
 
-    /** Sets a single endpoint in the routing layer for the instance in this */
-    public DeploymentContext setEndpoints(ZoneId zone) {
-        return setEndpoints(zone, false);
-    }
-
     /** Deploy default application package, start a run for that change and return its ID */
     public RunId newRun(JobType type) {
         submit();
@@ -439,7 +434,6 @@ public class DeploymentContext {
         configServer().convergeServices(instanceId, JobType.systemTest.zone(tester.controller().system()));
         configServer().convergeServices(testerId.id(), JobType.systemTest.zone(tester.controller().system()));
         setEndpoints(JobType.systemTest.zone(tester.controller().system()));
-        setTesterEndpoints(JobType.systemTest.zone(tester.controller().system()));
         runner.run();
         assertEquals(unfinished, jobs.run(id).get().stepStatuses().get(Step.endTests));
         assertTrue(jobs.run(id).get().steps().get(Step.endTests).startTime().isPresent());
@@ -464,15 +458,6 @@ public class DeploymentContext {
 
         // Provision load balancers in directly routed zones, unless explicitly deferred
         if (provisionLoadBalancerIn(zone)) {
-            if (job.type().isTest()) {
-                var testerDeployment = new DeploymentId(testerId.id(), zone);
-                configServer().putLoadBalancers(zone, List.of(new LoadBalancer(testerDeployment.toString(),
-                                                                               testerDeployment.applicationId(),
-                                                                               ClusterSpec.Id.from("default"),
-                                                                               HostName.from("lb-0--" + testerId.id().serializedForm() + "--" + zone.toString()),
-                                                                               LoadBalancer.State.active,
-                                                                               Optional.of("dns-zone-1"))));
-            }
             configServer().putLoadBalancers(zone, List.of(new LoadBalancer(deployment.toString(),
                                                                            deployment.applicationId(),
                                                                            ClusterSpec.Id.from("default"),
@@ -529,18 +514,10 @@ public class DeploymentContext {
         return run;
     }
 
-    /** Sets a single endpoint in the routing layer for the tester instance in this */
-    private DeploymentContext setTesterEndpoints(ZoneId zone) {
-        return setEndpoints(zone, true);
-    }
-
-    /** Sets a single endpoint in the routing layer; this matches that required for the tester */
-    private DeploymentContext setEndpoints(ZoneId zone, boolean tester) {
+    /** Sets a single endpoint in the routing layer */
+    DeploymentContext setEndpoints(ZoneId zone) {
         if (!supportsRoutingMethod(RoutingMethod.shared, zone)) return this;
         var id = instanceId;
-        if (tester) {
-            id = testerId.id();
-        }
         routing.putEndpoints(new DeploymentId(id, zone),
                              Collections.singletonList(new RoutingEndpoint(String.format("https://%s--%s--%s.%s.%s.vespa:43",
                                                                                          id.instance().value(),
@@ -586,12 +563,7 @@ public class DeploymentContext {
         assertEquals(unfinished, jobs.run(id).get().stepStatuses().get(Step.installTester));
         configServer().convergeServices(TesterId.of(id.application()).id(), zone);
         runner.advance(currentRun(job));
-        if (provisionLoadBalancerIn(zone)) { // Endpoints are available immediately after deployment in directly routed zones
-            assertEquals(succeeded, jobs.run(id).get().stepStatuses().get(Step.installTester));
-        } else {
-            assertEquals(unfinished, jobs.run(id).get().stepStatuses().get(Step.installTester));
-            setTesterEndpoints(zone);
-        }
+        assertEquals(succeeded, jobs.run(id).get().stepStatuses().get(Step.installTester));
         runner.advance(currentRun(job));
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 93359ead8e8..938b801b88d 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzDbMock;
-import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId;
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGeneratorMock;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockTesterCloud;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
@@ -47,7 +46,6 @@ public class DeploymentTester {
 
     public static final TenantAndApplicationId appId = TenantAndApplicationId.from("tenant", "application");
     public static final ApplicationId instanceId = appId.defaultInstance();
-    public static final TesterId testerId = TesterId.of(instanceId);
 
     private final ControllerTester tester;
     private final JobController jobs;
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 d7af46ce8f4..8ecdd63fa8f 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
@@ -187,7 +187,6 @@ public class InternalStepRunnerTest {
         tester.clock().advance(InternalStepRunner.endpointTimeout.plus(Duration.ofSeconds(1)));
         tester.runner().run();
         assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.systemTest).get().stepStatuses().get(Step.installReal));
-        assertEquals(failed, tester.jobs().last(app.instanceId(), JobType.stagingTest).get().stepStatuses().get(Step.installTester));
     }
 
     @Test
@@ -316,8 +315,6 @@ public class InternalStepRunnerTest {
         RunId id = app.startSystemTestTests();
         tester.runner().run();
         assertEquals(unfinished, tester.jobs().run(id).get().stepStatuses().get(Step.endTests));
-        assertEquals(URI.create(tester.routing().endpoints(new DeploymentId(app.testerId().id(), JobType.systemTest.zone(system()))).get(0).endpoint()),
-                     tester.cloud().testerUrl());
         Inspector configObject = SlimeUtils.jsonToSlime(tester.cloud().config()).get();
         assertEquals(app.instanceId().serializedForm(), configObject.field("application").asString());
         assertEquals(JobType.systemTest.zone(system()).value(), configObject.field("zone").asString());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json
index dd3d16fc721..9e7eeba8420 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/system-test-details.json
@@ -83,36 +83,6 @@
       {
         "at": "(ignore)",
         "type": "info",
-        "message": "Endpoints not yet ready."
-      },
-      {
-        "at": "(ignore)",
-        "type": "info",
-        "message": "host-tenant1:application1:instance1-t-test.us-east-1: unorchestrated"
-      },
-      {
-        "at": "(ignore)",
-        "type": "info",
-        "message": "--- platform 6.1"
-      },
-      {
-        "at": "(ignore)",
-        "type": "info",
-        "message": "Found endpoints:"
-      },
-      {
-        "at": "(ignore)",
-        "type": "info",
-        "message": "- test.us-east-1"
-      },
-      {
-        "at": "(ignore)",
-        "type": "info",
-        "message": " |-- https://instance1-t--application1--tenant1.us-east-1.test.vespa:43 (cluster 'default')"
-      },
-      {
-        "at": "(ignore)",
-        "type": "info",
         "message": "Tester container successfully installed!"
       }
     ],
@@ -224,7 +194,7 @@
       }
     ]
   },
-  "lastId": 40,
+  "lastId": 34,
   "steps": {
     "deployTester": {
       "status": "succeeded",
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
index f0caa358cb8..1500154aa07 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java
@@ -72,6 +72,7 @@ public class LoadBalancerProvisioner {
     public void prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) {
         if (requestedNodes.type() != NodeType.tenant) return; // Nothing to provision for this node type
         if (!cluster.type().isContainer()) return; // Nothing to provision for this cluster type
+        if (application.instance().isTester()) return; // Do not provision for tester instances
         try (var loadBalancersLock = db.lockLoadBalancers()) {
             provision(application, cluster.id(), false, loadBalancersLock);
         }