Merge pull request #20392 from vespa-engine/mpolden/check-if-status-changed

Stop deploying on routing status change MERGEOK

2 files changed, 4 insertions, 97 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java
index 48f23a1f7bd..3aa69c6cedf 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandler.java
@@ -3,8 +3,6 @@ package com.yahoo.vespa.config.server.http.v1;
 
 import com.google.inject.Inject;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Deployer;
-import com.yahoo.config.provision.Deployment;
 import com.yahoo.jdisc.http.HttpRequest;
 import com.yahoo.path.Path;
 import com.yahoo.restapi.RestApi;
@@ -20,7 +18,6 @@ import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Clock;
-import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.List;
@@ -50,18 +47,16 @@ public class RoutingStatusApiHandler extends RestApiRequestHandler<RoutingStatus
 
     private final Curator curator;
     private final Clock clock;
-    private final Deployer deployer;
 
     @Inject
-    public RoutingStatusApiHandler(Context context, Curator curator, Deployer deployer) {
-        this(context, curator, Clock.systemUTC(), deployer);
+    public RoutingStatusApiHandler(Context context, Curator curator) {
+        this(context, curator, Clock.systemUTC());
     }
 
-    RoutingStatusApiHandler(Context context, Curator curator, Clock clock, Deployer deployer) {
+    RoutingStatusApiHandler(Context context, Curator curator, Clock clock) {
         super(context, RoutingStatusApiHandler::createRestApiDefinition);
         this.curator = Objects.requireNonNull(curator);
         this.clock = Objects.requireNonNull(clock);
-        this.deployer = Objects.requireNonNull(deployer);
 
         curator.create(DEPLOYMENT_STATUS_ROOT);
     }
@@ -126,21 +121,6 @@ public class RoutingStatusApiHandler extends RestApiRequestHandler<RoutingStatus
             return new SlimeJsonResponse(toSlime(wantedStatus));
         }
         changeStatus(upstreamNames, wantedStatus);
-        try {
-            Optional<Deployment> deployment = deployer.deployFromLocalActive(instance, Duration.ofMinutes(1));
-            if (deployment.isEmpty()) throw new IllegalArgumentException("No deployment of " + instance + " found");
-            deployment.get().activate();
-        } catch (Exception e) {
-            log.log(Level.SEVERE, "Failed to redeploy " + instance + ". Reverting routing status to " +
-                                  currentStatus.status(), e);
-            changeStatus(upstreamNames, currentStatus);
-            throw new RestApiException.InternalServerError("Failed to change status to " +
-                                                           wantedStatus.status() + ", reverting to "
-                                                           + currentStatus.status() +
-                                                           " because redeployment of " +
-                                                           instance + " failed: " +
-                                                           Exceptions.toMessageString(e));
-        }
         return new SlimeJsonResponse(toSlime(wantedStatus));
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java
index 8dd7cf4d6fc..e2b45d33cbc 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v1/RoutingStatusApiHandlerTest.java
@@ -2,9 +2,6 @@
 package com.yahoo.vespa.config.server.http.v1;
 
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.Deployer;
-import com.yahoo.config.provision.Deployment;
-import com.yahoo.config.provision.HostFilter;
 import com.yahoo.container.jdisc.HttpRequestBuilder;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.jdisc.http.HttpRequest.Method;
@@ -19,17 +16,11 @@ import org.junit.Test;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.nio.charset.StandardCharsets;
-import java.time.Clock;
-import java.time.Duration;
 import java.time.Instant;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author bjorncs
@@ -42,7 +33,6 @@ public class RoutingStatusApiHandlerTest {
 
     private final Curator curator = new MockCurator();
     private final ManualClock clock = new ManualClock();
-    private final MockDeployer deployer = new MockDeployer(clock);
 
     private RestApiTestDriver testDriver;
 
@@ -50,8 +40,7 @@ public class RoutingStatusApiHandlerTest {
     public void before() {
         RoutingStatusApiHandler requestHandler = new RoutingStatusApiHandler(RestApiTestDriver.createHandlerTestContext(),
                                                                              curator,
-                                                                             clock,
-                                                                             deployer);
+                                                                             clock);
         testDriver = RestApiTestDriver.newBuilder(requestHandler).build();
     }
 
@@ -77,14 +66,6 @@ public class RoutingStatusApiHandlerTest {
         String response = responseAsString(executeRequest(Method.PUT, "/routing/v1/status/" + upstreamName + "?application=" + instance.serializedForm(),
                                                           statusOut()));
         assertEquals(response("OUT", "issue-XXX", "operator", clock.instant()), response);
-        assertTrue("Re-deployed " + instance, deployer.lastDeployed.containsKey(instance));
-
-        // Status is reverted if redeployment fails
-        deployer.failNextDeployment(true);
-        response = responseAsString(executeRequest(Method.PUT, "/routing/v1/status/" + upstreamName + "?application=" + instance.serializedForm(),
-                                                   requestContent("IN", "all good")));
-        assertEquals("{\"error-code\":\"INTERNAL_SERVER_ERROR\",\"message\":\"Failed to change status to in, reverting to out because redeployment of t1.a1.i1 failed: Deployment failed\"}",
-                     response);
 
         // Read status stored in old format (path exists, but without content)
         curator.set(Path.fromString("/routing/v1/status/" + upstreamName), new byte[0]);
@@ -92,7 +73,6 @@ public class RoutingStatusApiHandlerTest {
         assertEquals(response("OUT", "", "", clock.instant()), response);
 
         // Change status of multiple upstreams
-        deployer.failNextDeployment(false);
         String upstreamName2 = "upstream2";
         String upstreams = upstreamName + "," + upstreamName2 + "," + upstreamName2;
         response = responseAsString(executeRequest(Method.PUT, "/routing/v1/status/" + upstreams + "?application=" + instance.serializedForm(),
@@ -172,57 +152,4 @@ public class RoutingStatusApiHandlerTest {
         return "{\"status\":\"" + status + "\",\"cause\":\"" + reason + "\",\"agent\":\"" + agent + "\",\"lastUpdate\":" + instant.getEpochSecond() + "}";
     }
 
-    private static class MockDeployer implements Deployer {
-
-        private final Map<ApplicationId, Instant> lastDeployed = new HashMap<>();
-        private final Clock clock;
-
-        private boolean failNextDeployment = false;
-
-        public MockDeployer(Clock clock) {
-            this.clock = clock;
-        }
-
-        public MockDeployer failNextDeployment(boolean fail) {
-            this.failNextDeployment = fail;
-            return this;
-        }
-
-        @Override
-        public Optional<Deployment> deployFromLocalActive(ApplicationId application, boolean bootstrap) {
-            return deployFromLocalActive(application, Duration.ZERO, false);
-        }
-
-        @Override
-        public Optional<Deployment> deployFromLocalActive(ApplicationId application, Duration timeout, boolean bootstrap) {
-            if (failNextDeployment) {
-                throw new RuntimeException("Deployment failed");
-            }
-            return Optional.of(new Deployment() {
-                @Override
-                public void prepare() {}
-
-                @Override
-                public long activate() {
-                    lastDeployed.put(application, clock.instant());
-                    return 1L;
-                }
-
-                @Override
-                public void restart(HostFilter filter) {}
-            });
-        }
-
-        @Override
-        public Optional<Instant> lastDeployTime(ApplicationId application) {
-            return Optional.ofNullable(lastDeployed.get(application));
-        }
-
-        @Override
-        public Duration serverDeployTimeout() {
-            return Duration.ZERO;
-        }
-
-    }
-
 }