New path for suspend all API

This is required to allow authorization of these requests.

5 files changed, 59 insertions, 34 deletions

diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java
index ad0f3e094eb..1c4d138acef 100644
--- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java
+++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostApi.java
@@ -14,9 +14,7 @@ import javax.ws.rs.PUT;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
-import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
-import javax.ws.rs.core.UriInfo;
 
 /**
  * Definition of Orchestrator's REST API for hosts.
diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java
index a5ca15d2d15..a9846134eff 100644
--- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java
+++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/HostSuspensionApi.java
@@ -6,9 +6,16 @@ import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult;
 
 import javax.ws.rs.Consumes;
 import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
 import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
 import javax.ws.rs.core.MediaType;
+import java.util.List;
 
+/**
+ * @author hakonhall
+ */
 public interface HostSuspensionApi {
     /**
      * Path prefix for this api. Resources implementing this API should use this with a @Path annotation.
@@ -27,5 +34,13 @@ public interface HostSuspensionApi {
     @PUT
     @Produces(MediaType.APPLICATION_JSON)
     @Consumes(MediaType.APPLICATION_JSON)
+    @Deprecated // TODO: Remove after 2018-04-01
     BatchOperationResult suspendAll(BatchHostSuspendRequest request);
+
+    @PUT
+    @Path("/{hostname}")
+    @Produces(MediaType.APPLICATION_JSON)
+    @Consumes(MediaType.APPLICATION_JSON)
+    BatchOperationResult suspendAll(@PathParam("hostname") String parentHostname,
+                                    @QueryParam("hostname") List<String> hostnames);
 }
diff --git a/orchestrator/pom.xml b/orchestrator/pom.xml
index ae22b6718f9..3559e4282c3 100644
--- a/orchestrator/pom.xml
+++ b/orchestrator/pom.xml
@@ -131,6 +131,16 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <compilerArgs>
+            <arg>-Xlint:all</arg>
+            <arg>-Xlint:-deprecation</arg>
+            <arg>-Xlint:-serial</arg>
+	    <arg>-Xlint:-varargs</arg>
+            <arg>-Xlint:-try</arg>
+            <arg>-Werror</arg>
+          </compilerArgs>
+        </configuration>
       </plugin>
     </plugins>
   </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 85166ecc741..c1e312ddee6 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
@@ -21,6 +21,9 @@ import java.util.List;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
+/**
+ * @author hakonhall
+ */
 @Path(HostSuspensionApi.PATH_PREFIX)
 public class HostSuspensionResource implements HostSuspensionApi {
 
@@ -47,26 +50,28 @@ public class HostSuspensionResource implements HostSuspensionApi {
             log.log(LogLevel.DEBUG, message);
             throw createWebApplicationException(message, Response.Status.BAD_REQUEST);
         }
+        return suspendAll(parentHostnameString, hostnamesAsStrings);
+    }
 
+    @Override
+    public BatchOperationResult suspendAll(String parentHostnameString, List<String> hostnamesAsStrings) {
         HostName parentHostname = new HostName(parentHostnameString);
-        List<HostName> hostNames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList());
-
+        List<HostName> hostnames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList());
         try {
-            orchestrator.suspendAll(parentHostname, hostNames);
+            orchestrator.suspendAll(parentHostname, hostnames);
         } catch (BatchHostStateChangeDeniedException e) {
-            log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostNames + " with parent host " + parentHostname, e);
+            log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e);
             throw createWebApplicationException(e.getMessage(), Response.Status.CONFLICT);
         } catch (BatchHostNameNotFoundException e) {
-            log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostNames + " with parent host " + parentHostname, 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
             // by the URL path was found. It's one of the hostnames in the request it failed to find.
             throw createWebApplicationException(e.getMessage(), Response.Status.BAD_REQUEST);
         } catch (BatchInternalErrorException e) {
-            log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostNames + " with parent host " + parentHostname, e);
+            log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e);
             throw createWebApplicationException(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR);
         }
-
-        log.log(LogLevel.DEBUG, "Suspended " + hostNames + " with parent " + parentHostname);
+        log.log(LogLevel.DEBUG, "Suspended " + hostnames + " with parent " + parentHostname);
         return BatchOperationResult.successResult();
     }
 
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 65309440aee..2c7db25ae30 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
@@ -21,7 +21,6 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMoc
 import com.yahoo.vespa.orchestrator.model.ApplicationApi;
 import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException;
 import com.yahoo.vespa.orchestrator.policy.Policy;
-import com.yahoo.vespa.orchestrator.restapi.wire.BatchHostSuspendRequest;
 import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult;
 import com.yahoo.vespa.orchestrator.restapi.wire.GetHostResponse;
 import com.yahoo.vespa.orchestrator.restapi.wire.PatchHostRequest;
@@ -56,7 +55,11 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+/**
+ * @author hakonhall
+ */
 public class HostResourceTest {
+
     private static final int SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS = 0;
     private static final TenantId TENANT_ID = new TenantId("tenantId");
     private static final ApplicationInstanceId APPLICATION_INSTANCE_ID = new ApplicationInstanceId("applicationId");
@@ -110,20 +113,20 @@ public class HostResourceTest {
         public void grantSuspensionRequest(
                 ApplicationInstance applicationInstance,
                 HostName hostName,
-                MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException {
+                MutableStatusRegistry hostStatusService) {
 
         }
 
         @Override
-        public void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException {
+        public void grantSuspensionRequest(ApplicationApi applicationApi) {
         }
 
         @Override
-        public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException {
+        public void releaseSuspensionGrant(ApplicationApi application) {
         }
 
         @Override
-        public void acquirePermissionToRemove(ApplicationApi applicationApi) throws HostStateChangeDeniedException {
+        public void acquirePermissionToRemove(ApplicationApi applicationApi) {
         }
 
         @Override
@@ -151,7 +154,7 @@ public class HostResourceTest {
     private final UriInfo uriInfo = mock(UriInfo.class);
 
     @Test
-    public void returns_200_on_success() throws Exception {
+    public void returns_200_on_success() {
         HostResource hostResource =
                 new HostResource(alwaysAllowOrchestrator, uriInfo);
 
@@ -163,25 +166,23 @@ public class HostResourceTest {
     }
 
     @Test
-    public void returns_200_on_success_batch() throws Exception {
+    public void returns_200_on_success_batch() {
         HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysAllowOrchestrator);
-        BatchHostSuspendRequest request =
-                new BatchHostSuspendRequest("parentHostname", Arrays.asList("hostname1", "hostname2"));
-        BatchOperationResult response = hostSuspensionResource.suspendAll(request);
+        BatchOperationResult response = hostSuspensionResource.suspendAll("parentHostname",
+                                                                          Arrays.asList("hostname1", "hostname2"));
         assertThat(response.success());
     }
 
     @Test
-    public void returns_200_empty_batch() throws Exception {
+    public void returns_200_empty_batch() {
         HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysAllowOrchestrator);
-        BatchHostSuspendRequest request =
-                new BatchHostSuspendRequest("parentHostname", Collections.emptyList());
-        BatchOperationResult response = hostSuspensionResource.suspendAll(request);
+        BatchOperationResult response = hostSuspensionResource.suspendAll("parentHostname",
+                                                                          Collections.emptyList());;
         assertThat(response.success());
     }
 
     @Test
-    public void throws_404_when_host_unknown() throws Exception {
+    public void throws_404_when_host_unknown() {
         try {
             HostResource hostResource =
                     new HostResource(hostNotFoundOrchestrator, uriInfo);
@@ -196,12 +197,10 @@ public class HostResourceTest {
     // This is so because the hostname is part of the URL path for single-host, while the
     // hostnames are part of the request body for multi-host.
     @Test
-    public void throws_400_when_host_unknown_for_batch() throws Exception {
+    public void throws_400_when_host_unknown_for_batch() {
         try {
             HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(hostNotFoundOrchestrator);
-            BatchHostSuspendRequest request =
-                    new BatchHostSuspendRequest("parentHostname", Arrays.asList("hostname1", "hostname2"));
-            hostSuspensionResource.suspendAll(request);
+            hostSuspensionResource.suspendAll("parentHostname", Arrays.asList("hostname1", "hostname2"));
             fail();
         } catch (WebApplicationException w) {
             assertThat(w.getResponse().getStatus()).isEqualTo(400);
@@ -249,7 +248,7 @@ public class HostResourceTest {
     }
 
     @Test
-    public void throws_409_when_request_rejected_by_policies() throws Exception {
+    public void throws_409_when_request_rejected_by_policies() {
         final OrchestratorImpl alwaysRejectResolver = new OrchestratorImpl(
                 new AlwaysFailPolicy(),
                 new ClusterControllerClientFactoryMock(),
@@ -266,7 +265,7 @@ public class HostResourceTest {
     }
 
     @Test
-    public void throws_409_when_request_rejected_by_policies_for_batch() throws Exception {
+    public void throws_409_when_request_rejected_by_policies_for_batch() {
         final OrchestratorImpl alwaysRejectResolver = new OrchestratorImpl(
                 new AlwaysFailPolicy(),
                 new ClusterControllerClientFactoryMock(),
@@ -276,9 +275,7 @@ public class HostResourceTest {
 
         try {
             HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysRejectResolver);
-            BatchHostSuspendRequest request =
-                    new BatchHostSuspendRequest("parentHostname", Arrays.asList("hostname1", "hostname2"));
-            hostSuspensionResource.suspendAll(request);
+            hostSuspensionResource.suspendAll("parentHostname", Arrays.asList("hostname1", "hostname2"));
             fail();
         } catch (WebApplicationException w) {
             assertThat(w.getResponse().getStatus()).isEqualTo(409);