Return 504 Gateway Timeout on lock timeout from Orchestrator

5 files changed, 23 insertions, 12 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java
index cc8a401ed78..4bb93ffa3cb 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java
@@ -152,29 +152,34 @@ public class HostResource implements HostApi {
         return new UpdateHostResponse(hostName.s(), null);
     }
 
-    private static WebApplicationException webExceptionFromTimeout(String operationDescription, HostName hostName, UncheckedTimeoutException e) {
-        return createWebException(operationDescription, hostName, e, HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage());
+    private static WebApplicationException webExceptionFromTimeout(String operationDescription,
+                                                                   HostName hostName,
+                                                                   UncheckedTimeoutException e) {
+        return createWebException(operationDescription, hostName, e, HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage(),
+                Response.Status.GATEWAY_TIMEOUT);
     }
 
     private static WebApplicationException webExceptionWithDenialReason(
             String operationDescription,
             HostName hostName,
             HostStateChangeDeniedException e) {
-        return createWebException(operationDescription, hostName, e, e.getConstraintName(), e.getMessage());
+        return createWebException(operationDescription, hostName, e, e.getConstraintName(), e.getMessage(),
+                Response.Status.CONFLICT);
     }
 
     private static WebApplicationException createWebException(String operationDescription,
                                                               HostName hostname,
                                                               Exception e,
                                                               String constraint,
-                                                              String message) {
+                                                              String message,
+                                                              Response.Status status) {
         HostStateChangeDenialReason hostStateChangeDenialReason = new HostStateChangeDenialReason(
                 constraint, operationDescription + " failed: " + message);
         UpdateHostResponse response = new UpdateHostResponse(hostname.s(), hostStateChangeDenialReason);
         return new WebApplicationException(
                 hostStateChangeDenialReason.toString(),
                 e,
-                Response.status(Response.Status.CONFLICT)
+                Response.status(status)
                         .entity(response)
                         .type(MediaType.APPLICATION_JSON_TYPE)
                         .build());
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java
index 24d42264a51..79e0fc0f3e9 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java
@@ -18,6 +18,7 @@ import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import java.util.List;
+import java.util.concurrent.TimeoutException;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -42,9 +43,12 @@ public class HostSuspensionResource implements HostSuspensionApi {
         List<HostName> hostnames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList());
         try {
             orchestrator.suspendAll(parentHostname, hostnames);
-        } catch (BatchHostStateChangeDeniedException | UncheckedTimeoutException e) {
+        } catch (BatchHostStateChangeDeniedException e) {
             log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e);
             throw createWebApplicationException(e.getMessage(), Response.Status.CONFLICT);
+        } catch (UncheckedTimeoutException e) {
+            log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e);
+            throw createWebApplicationException(e.getMessage(), Response.Status.GATEWAY_TIMEOUT);
         } catch (BatchHostNameNotFoundException e) {
             log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e);
             // Note that we're returning BAD_REQUEST instead of NOT_FOUND because the resource identified
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java
index 9f91e08d344..993cddae2b3 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.status;
 
+import com.google.common.util.concurrent.UncheckedTimeoutException;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
 import com.yahoo.vespa.applicationmodel.HostName;
 import com.yahoo.vespa.orchestrator.OrchestratorContext;
@@ -45,7 +46,7 @@ public interface StatusService {
      */
     MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(
             OrchestratorContext context,
-            ApplicationInstanceReference applicationInstanceReference);
+            ApplicationInstanceReference applicationInstanceReference) throws UncheckedTimeoutException;
 
     /**
      * Returns all application instances that are allowed to be down. The intention is to use this
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
index f6e01a49ce8..e3d2a0827ed 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.orchestrator.status;
 
+import com.google.common.util.concurrent.UncheckedTimeoutException;
 import com.yahoo.container.jaxrs.annotation.Component;
 import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
@@ -102,7 +103,7 @@ public class ZookeeperStatusService implements StatusService {
     @Override
     public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(
             OrchestratorContext context,
-            ApplicationInstanceReference applicationInstanceReference) {
+            ApplicationInstanceReference applicationInstanceReference) throws UncheckedTimeoutException {
         Duration duration = context.getTimeLeft();
         String lockPath = applicationInstanceLock2Path(applicationInstanceReference);
         Lock lock = new Lock(lockPath, curator);
diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
index 55ac65c036c..2e97b3b7242 100644
--- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
+++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java
@@ -352,7 +352,7 @@ public class HostResourceTest {
     }
 
     @Test
-    public void throws_409_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException {
+    public void throws_504_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException {
         Orchestrator orchestrator = mock(Orchestrator.class);
         doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).resume(any(HostName.class));
 
@@ -361,13 +361,13 @@ public class HostResourceTest {
             hostResource.resume("hostname");
             fail();
         } catch (WebApplicationException w) {
-            assertThat(w.getResponse().getStatus()).isEqualTo(409);
+            assertThat(w.getResponse().getStatus()).isEqualTo(504);
             assertEquals("resume failed: Timeout Message [deadline]", w.getMessage());
         }
     }
 
     @Test
-    public void throws_409_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException {
+    public void throws_504_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException {
         Orchestrator orchestrator = mock(Orchestrator.class);
         doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).suspendAll(any(), any());
 
@@ -376,7 +376,7 @@ public class HostResourceTest {
             resource.suspendAll("parenthost", Arrays.asList("h1", "h2", "h3"));
             fail();
         } catch (WebApplicationException w) {
-            assertThat(w.getResponse().getStatus()).isEqualTo(409);
+            assertThat(w.getResponse().getStatus()).isEqualTo(504);
         }
     }
 }