Merge pull request #11325 from vespa-engine/revert-11323-revert-11303-hakonhall/increase-client-side-node-admin-configserver-timeouts-from-15s-to-30s

Reduce timeouts for non-suspend ConfigServer REST API calls from node admin, try 2

6 files changed, 165 insertions, 80 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java
index 9b496526804..90768facf34 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java
@@ -1,6 +1,7 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.configserver;
 
+import java.time.Duration;
 import java.util.Optional;
 
 /**
@@ -9,16 +10,42 @@ import java.util.Optional;
  * @author freva
  */
 public interface ConfigServerApi extends AutoCloseable {
-
-    <T> T get(String path, Class<T> wantedReturnType);
-
-    <T> T post(String path, Object bodyJsonPojo, Class<T> wantedReturnType);
-
-    <T> T put(String path, Optional<Object> bodyJsonPojo, Class<T> wantedReturnType);
-
-    <T> T patch(String path, Object bodyJsonPojo, Class<T> wantedReturnType);
-
-    <T> T delete(String path, Class<T> wantedReturnType);
+    class Params {
+        private Optional<Duration> connectionTimeout;
+
+        /** Set the socket connect and read timeouts. */
+        public Params setConnectionTimeout(Duration connectionTimeout) {
+            this.connectionTimeout = Optional.of(connectionTimeout);
+            return this;
+        }
+
+        public Optional<Duration> getConnectionTimeout() { return connectionTimeout; }
+    }
+
+    <T> T get(String path, Class<T> wantedReturnType, Params params);
+    default <T> T get(String path, Class<T> wantedReturnType) {
+        return get(path, wantedReturnType, null);
+    }
+
+    <T> T post(String path, Object bodyJsonPojo, Class<T> wantedReturnType, Params params);
+    default <T> T post(String path, Object bodyJsonPojo, Class<T> wantedReturnType) {
+        return post(path, bodyJsonPojo, wantedReturnType, null);
+    }
+
+    <T> T put(String path, Optional<Object> bodyJsonPojo, Class<T> wantedReturnType, Params params);
+    default <T> T put(String path, Optional<Object> bodyJsonPojo, Class<T> wantedReturnType) {
+        return put(path, bodyJsonPojo, wantedReturnType, null);
+    }
+
+    <T> T patch(String path, Object bodyJsonPojo, Class<T> wantedReturnType, Params params);
+    default <T> T patch(String path, Object bodyJsonPojo, Class<T> wantedReturnType) {
+        return patch(path, bodyJsonPojo, wantedReturnType, null);
+    }
+
+    <T> T delete(String path, Class<T> wantedReturnType, Params params);
+    default <T> T delete(String path, Class<T> wantedReturnType) {
+        return delete(path, wantedReturnType, null);
+    }
 
     /** Close the underlying HTTP client and any threads this class might have started. */
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
index 4dadcb359ea..a066375ce5c 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
@@ -49,6 +49,11 @@ import java.util.logging.Logger;
 public class ConfigServerApiImpl implements ConfigServerApi {
 
     private static final Logger logger = Logger.getLogger(ConfigServerApiImpl.class.getName());
+    private static final RequestConfig DEFAULT_REQUEST_CONFIG = RequestConfig.custom()
+            .setConnectionRequestTimeout(1_000) // connection from connection manager
+            .setConnectTimeout(10_000) // establishment of connection
+            .setSocketTimeout(10_000) // waiting for data
+            .build();
 
     private final ObjectMapper mapper = new ObjectMapper();
 
@@ -134,9 +139,11 @@ public class ConfigServerApiImpl implements ConfigServerApi {
     }
 
     @Override
-    public <T> T put(String path, Optional<Object> bodyJsonPojo, Class<T> wantedReturnType) {
+    public <T> T put(String path, Optional<Object> bodyJsonPojo, Class<T> wantedReturnType, Params paramsOrNull) {
+        Optional<RequestConfig> requestConfigOverride = getRequestConfigOverride(paramsOrNull);
         return tryAllConfigServers(configServer -> {
             HttpPut put = new HttpPut(configServer.resolve(path));
+            requestConfigOverride.ifPresent(put::setConfig);
             setContentTypeToApplicationJson(put);
             if (bodyJsonPojo.isPresent()) {
                 put.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo.get())));
@@ -146,9 +153,11 @@ public class ConfigServerApiImpl implements ConfigServerApi {
     }
 
     @Override
-    public <T> T patch(String path, Object bodyJsonPojo, Class<T> wantedReturnType) {
+    public <T> T patch(String path, Object bodyJsonPojo, Class<T> wantedReturnType, Params paramsOrNull) {
+        Optional<RequestConfig> requestConfigOverride = getRequestConfigOverride(paramsOrNull);
         return tryAllConfigServers(configServer -> {
             HttpPatch patch = new HttpPatch(configServer.resolve(path));
+            requestConfigOverride.ifPresent(patch::setConfig);
             setContentTypeToApplicationJson(patch);
             patch.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo)));
             return patch;
@@ -156,21 +165,31 @@ public class ConfigServerApiImpl implements ConfigServerApi {
     }
 
     @Override
-    public <T> T delete(String path, Class<T> wantedReturnType) {
-        return tryAllConfigServers(configServer ->
-                new HttpDelete(configServer.resolve(path)), wantedReturnType);
+    public <T> T delete(String path, Class<T> wantedReturnType, Params paramsOrNull) {
+        Optional<RequestConfig> requestConfigOverride = getRequestConfigOverride(paramsOrNull);
+        return tryAllConfigServers(configServer -> {
+            HttpDelete delete = new HttpDelete(configServer.resolve(path));
+            requestConfigOverride.ifPresent(delete::setConfig);
+            return delete;
+        }, wantedReturnType);
     }
 
     @Override
-    public <T> T get(String path, Class<T> wantedReturnType) {
-        return tryAllConfigServers(configServer ->
-                new HttpGet(configServer.resolve(path)), wantedReturnType);
+    public <T> T get(String path, Class<T> wantedReturnType, Params paramsOrNull) {
+        Optional<RequestConfig> requestConfig = getRequestConfigOverride(paramsOrNull);
+        return tryAllConfigServers(configServer -> {
+            HttpGet get = new HttpGet(configServer.resolve(path));
+            requestConfig.ifPresent(get::setConfig);
+            return get;
+        }, wantedReturnType);
     }
 
     @Override
-    public <T> T post(String path, Object bodyJsonPojo, Class<T> wantedReturnType) {
+    public <T> T post(String path, Object bodyJsonPojo, Class<T> wantedReturnType, Params paramsOrNull) {
+        Optional<RequestConfig> requestConfigOverride = getRequestConfigOverride(paramsOrNull);
         return tryAllConfigServers(configServer -> {
             HttpPost post = new HttpPost(configServer.resolve(path));
+            requestConfigOverride.ifPresent(post::setConfig);
             setContentTypeToApplicationJson(post);
             post.setEntity(new StringEntity(mapper.writeValueAsString(bodyJsonPojo)));
             return post;
@@ -203,23 +222,28 @@ public class ConfigServerApiImpl implements ConfigServerApi {
         cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough
 
         // Have experienced hang in socket read, which may have been because of
-        // system defaults, therefore set explicit timeouts. Set arbitrarily to
-        // 15s > 10s used by Orchestrator lock timeout.
-        int timeoutMs = 15_000;
-        RequestConfig requestBuilder = RequestConfig.custom()
-                .setConnectTimeout(timeoutMs) // establishment of connection
-                .setConnectionRequestTimeout(timeoutMs) // connection from connection manager
-                .setSocketTimeout(timeoutMs) // waiting for data
-                .build();
-
+        // system defaults, therefore set explicit timeouts.
         return HttpClientBuilder.create()
-                .setDefaultRequestConfig(requestBuilder)
+                .setDefaultRequestConfig(DEFAULT_REQUEST_CONFIG)
                 .disableAutomaticRetries()
                 .setUserAgent("node-admin")
                 .setConnectionManager(cm)
                 .build();
     }
 
+    private static Optional<RequestConfig> getRequestConfigOverride(Params paramsOrNull) {
+        if (paramsOrNull == null) return Optional.empty();
+
+        RequestConfig.Builder builder = RequestConfig.copy(DEFAULT_REQUEST_CONFIG);
+
+        paramsOrNull.getConnectionTimeout().ifPresent(connectionTimeout -> {
+            builder.setConnectTimeout((int) connectionTimeout.toMillis());
+            builder.setSocketTimeout((int) connectionTimeout.toMillis());
+        });
+
+        return Optional.of(builder.build());
+    }
+
     // Shuffle config server URIs to balance load
     private static List<URI> randomizeConfigServerUris(Collection<URI> configServerUris) {
         List<URI> shuffledConfigServerHosts = new ArrayList<>(configServerUris);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConnectionException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConnectionException.java
index 7e860bfb66b..ef91e9bf81b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConnectionException.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConnectionException.java
@@ -14,8 +14,8 @@ import java.net.SocketTimeoutException;
 @SuppressWarnings("serial")
 public class ConnectionException extends ConvergenceException {
 
-    private ConnectionException(String message) {
-        super(message);
+    private ConnectionException(String message, Throwable cause) {
+        super(message, cause);
     }
 
     /**
@@ -24,7 +24,7 @@ public class ConnectionException extends ConvergenceException {
      */
     public static RuntimeException handleException(String prefix, Throwable t) {
         if (isKnownConnectionException(t))
-            return new ConnectionException(prefix + t.getMessage());
+            return new ConnectionException(prefix + t.getMessage(), t);
 
         return new RuntimeException(prefix, t);
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java
index 353abd64778..20c0604b5dc 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImpl.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.orchestrator.restapi.HostSuspensionApi;
 import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult;
 import com.yahoo.vespa.orchestrator.restapi.wire.UpdateHostResponse;
 
+import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
 
@@ -19,6 +20,15 @@ import java.util.Optional;
  * @author dybis
  */
 public class OrchestratorImpl implements Orchestrator {
+    // The server-side Orchestrator has an internal timeout of 10s.
+    //
+    // Note: A 409 has been observed to be returned after 33s in a case possibly involving
+    // zk leader election (which is unfortunate as it is difficult to differentiate between
+    // transient timeouts (do not allow suspend on timeout) and the config server being
+    // permanently down (allow suspend)). For now we'd like to investigate such long
+    // requests so keep the timeout low(er).
+    private static final Duration CONNECTION_TIMEOUT = Duration.ofSeconds(15);
+
     // TODO: Find a way to avoid duplicating this (present in orchestrator's services.xml also).
     private static final String ORCHESTRATOR_PATH_PREFIX = "/orchestrator";
     static final String ORCHESTRATOR_PATH_PREFIX_HOST_API
@@ -36,9 +46,8 @@ public class OrchestratorImpl implements Orchestrator {
     public void suspend(final String hostName) {
         UpdateHostResponse response;
         try {
-            response = configServerApi.put(getSuspendPath(hostName),
-                    Optional.empty(), /* body */
-                    UpdateHostResponse.class);
+            var params = new ConfigServerApi.Params().setConnectionTimeout(CONNECTION_TIMEOUT);
+            response = configServerApi.put(getSuspendPath(hostName), Optional.empty(), UpdateHostResponse.class, params);
         } catch (HttpException.NotFoundException n) {
             throw new OrchestratorNotFoundException("Failed to suspend " + hostName + ", host not found");
         } catch (HttpException e) {
@@ -58,10 +67,11 @@ public class OrchestratorImpl implements Orchestrator {
     public void suspend(String parentHostName, List<String> hostNames) {
         final BatchOperationResult batchOperationResult;
         try {
-            String params = String.join("&hostname=", hostNames);
+            var params = new ConfigServerApi.Params().setConnectionTimeout(CONNECTION_TIMEOUT);
+            String hostnames = String.join("&hostname=", hostNames);
             String url = String.format("%s/%s?hostname=%s", ORCHESTRATOR_PATH_PREFIX_HOST_SUSPENSION_API,
-                                       parentHostName, params);
-            batchOperationResult = configServerApi.put(url, Optional.empty(), BatchOperationResult.class);
+                                       parentHostName, hostnames);
+            batchOperationResult = configServerApi.put(url, Optional.empty(), BatchOperationResult.class, params);
         } catch (HttpException e) {
             throw new OrchestratorException("Failed to batch suspend for " + parentHostName + ": " + e.toString());
         } catch (ConnectionException e) {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
index 1ed3e5729e5..0909a03749e 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
@@ -15,15 +15,19 @@ import org.junit.Test;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.SocketTimeoutException;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
@@ -37,6 +41,9 @@ import static org.mockito.Mockito.when;
  */
 public class ConfigServerApiImplTest {
 
+    private static final int FAIL_RETURN_CODE = 100000;
+    private static final int TIMEOUT_RETURN_CODE = 100001;
+
     @JsonIgnoreProperties(ignoreUnknown = true)
     public static class TestPojo {
         @JsonProperty("foo")
@@ -50,7 +57,7 @@ public class ConfigServerApiImplTest {
     private final List<URI> configServers = Arrays.asList(URI.create(uri1), URI.create(uri2));
     private final StringBuilder mockLog = new StringBuilder();
 
-    private ConfigServerApiImpl executor;
+    private ConfigServerApiImpl configServerApi;
     private int mockReturnCode = 200;
 
     @Before
@@ -59,7 +66,11 @@ public class ConfigServerApiImplTest {
         when(httpMock.execute(any())).thenAnswer(invocationOnMock -> {
             HttpGet get = (HttpGet) invocationOnMock.getArguments()[0];
             mockLog.append(get.getMethod()).append(" ").append(get.getURI()).append("  ");
-            if (mockReturnCode == 100000) throw new RuntimeException("FAIL");
+
+            switch (mockReturnCode) {
+                case FAIL_RETURN_CODE: throw new RuntimeException("FAIL");
+                case TIMEOUT_RETURN_CODE: throw new SocketTimeoutException("read timed out");
+            }
 
             BasicStatusLine statusLine = new BasicStatusLine(HttpVersion.HTTP_1_1, mockReturnCode, null);
             BasicHttpEntity entity = new BasicHttpEntity();
@@ -73,12 +84,12 @@ public class ConfigServerApiImplTest {
 
             return response;
         });
-        executor = ConfigServerApiImpl.createForTestingWithClient(configServers, httpMock);
+        configServerApi = ConfigServerApiImpl.createForTestingWithClient(configServers, httpMock);
     }
 
     @Test
     public void testBasicParsingSingleServer() {
-        TestPojo answer = executor.get("/path", TestPojo.class);
+        TestPojo answer = configServerApi.get("/path", TestPojo.class);
         assertThat(answer.foo, is("bar"));
         assertLogStringContainsGETForAHost();
     }
@@ -88,7 +99,7 @@ public class ConfigServerApiImplTest {
         // Server is returning 400, no retries.
         mockReturnCode = 400;
 
-        TestPojo testPojo = executor.get("/path", TestPojo.class);
+        TestPojo testPojo = configServerApi.get("/path", TestPojo.class);
         assertEquals(testPojo.errorCode.intValue(), mockReturnCode);
         assertLogStringContainsGETForAHost();
     }
@@ -98,17 +109,33 @@ public class ConfigServerApiImplTest {
         // Server is returning 201, no retries.
         mockReturnCode = 201;
 
-        TestPojo testPojo = executor.get("/path", TestPojo.class);
+        TestPojo testPojo = configServerApi.get("/path", TestPojo.class);
         assertEquals(testPojo.errorCode.intValue(), mockReturnCode);
         assertLogStringContainsGETForAHost();
     }
 
     @Test
+    public void testBasicSuccessWithCustomTimeouts() {
+        mockReturnCode = TIMEOUT_RETURN_CODE;
+
+        var params = new ConfigServerApi.Params();
+        params.setConnectionTimeout(Duration.ofSeconds(3));
+
+        try {
+            TestPojo testPojo = configServerApi.get("/path", TestPojo.class, params);
+            fail();
+        } catch (ConnectionException e) {
+            assertNotNull(e.getCause());
+            assertEquals("read timed out", e.getCause().getMessage());
+        }
+    }
+
+    @Test
     public void testRetries() {
         // Client is throwing exception, should be retries.
-        mockReturnCode = 100000;
+        mockReturnCode = FAIL_RETURN_CODE;
         try {
-            executor.get("/path", TestPojo.class);
+            configServerApi.get("/path", TestPojo.class);
             fail("Expected failure");
         } catch (Exception e) {
             // ignore
@@ -123,7 +150,7 @@ public class ConfigServerApiImplTest {
         // Client is throwing exception, should be retries.
         mockReturnCode = 503;
         try {
-            executor.get("/path", TestPojo.class);
+            configServerApi.get("/path", TestPojo.class);
             fail("Expected failure");
         } catch (Exception e) {
             // ignore
@@ -136,7 +163,7 @@ public class ConfigServerApiImplTest {
     public void testForbidden() {
         mockReturnCode = 403;
         try {
-            executor.get("/path", TestPojo.class);
+            configServerApi.get("/path", TestPojo.class);
             fail("Expected exception");
         } catch (HttpException.ForbiddenException e) {
             // ignore
@@ -149,7 +176,7 @@ public class ConfigServerApiImplTest {
         // Server is returning 404, special exception is thrown.
         mockReturnCode = 404;
         try {
-            executor.get("/path", TestPojo.class);
+            configServerApi.get("/path", TestPojo.class);
             fail("Expected exception");
         } catch (HttpException.NotFoundException e) {
             // ignore
@@ -161,7 +188,7 @@ public class ConfigServerApiImplTest {
     public void testConflict() {
         // Server is returning 409, no exception is thrown.
         mockReturnCode = 409;
-        executor.get("/path", TestPojo.class);
+        configServerApi.get("/path", TestPojo.class);
         assertLogStringContainsGETForAHost();
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImplTest.java
index e4c46c504be..936a7bb224d 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorImplTest.java
@@ -12,6 +12,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -29,9 +30,10 @@ public class OrchestratorImplTest {
     @Test
     public void testSuspendCall() {
         when(configServerApi.put(
-                OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended",
-                Optional.empty(),
-                UpdateHostResponse.class
+                eq(OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended"),
+                eq(Optional.empty()),
+                eq(UpdateHostResponse.class),
+                any()
         )).thenReturn(new UpdateHostResponse(hostName, null));
 
         orchestrator.suspend(hostName);
@@ -40,9 +42,10 @@ public class OrchestratorImplTest {
     @Test(expected=OrchestratorException.class)
     public void testSuspendCallWithFailureReason() {
         when(configServerApi.put(
-                OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended",
-                Optional.empty(),
-                UpdateHostResponse.class
+                eq(OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended"),
+                eq(Optional.empty()),
+                eq(UpdateHostResponse.class),
+                any()
         )).thenReturn(new UpdateHostResponse(hostName, new HostStateChangeDenialReason("hostname", "fail")));
 
         orchestrator.suspend(hostName);
@@ -50,22 +53,16 @@ public class OrchestratorImplTest {
 
     @Test(expected=OrchestratorNotFoundException.class)
     public void testSuspendCallWithNotFound() {
-        when(configServerApi.put(
-                any(String.class),
-                any(),
-                any()
-        )).thenThrow(new HttpException.NotFoundException("Not Found"));
+        when(configServerApi.put(any(String.class), any(), any(), any()))
+                .thenThrow(new HttpException.NotFoundException("Not Found"));
 
         orchestrator.suspend(hostName);
     }
 
     @Test(expected=RuntimeException.class)
     public void testSuspendCallWithSomeOtherException() {
-        when(configServerApi.put(
-                any(String.class),
-                any(),
-                any()
-        )).thenThrow(new RuntimeException("Some parameter was wrong"));
+        when(configServerApi.put(any(String.class), any(), any(), any()))
+                .thenThrow(new RuntimeException("Some parameter was wrong"));
 
         orchestrator.suspend(hostName);
     }
@@ -103,11 +100,8 @@ public class OrchestratorImplTest {
 
     @Test(expected=RuntimeException.class)
     public void testResumeCallWithSomeOtherException() {
-        when(configServerApi.put(
-                any(String.class),
-                any(),
-                any()
-        )).thenThrow(new RuntimeException("Some parameter was wrong"));
+        when(configServerApi.put(any(String.class), any(), any(), any()))
+                .thenThrow(new RuntimeException("Some parameter was wrong"));
 
         orchestrator.suspend(hostName);
     }
@@ -118,9 +112,10 @@ public class OrchestratorImplTest {
         List<String> hostNames = Arrays.asList("a1.host1.test.yahoo.com", "a2.host1.test.yahoo.com");
 
         when(configServerApi.put(
-                "/orchestrator/v1/suspensions/hosts/host1.test.yahoo.com?hostname=a1.host1.test.yahoo.com&hostname=a2.host1.test.yahoo.com",
-                Optional.empty(),
-                BatchOperationResult.class
+                eq("/orchestrator/v1/suspensions/hosts/host1.test.yahoo.com?hostname=a1.host1.test.yahoo.com&hostname=a2.host1.test.yahoo.com"),
+                eq(Optional.empty()),
+                eq(BatchOperationResult.class),
+                any()
         )).thenReturn(BatchOperationResult.successResult());
 
         orchestrator.suspend(parentHostName, hostNames);
@@ -133,9 +128,10 @@ public class OrchestratorImplTest {
         String failureReason = "Failed to suspend";
 
         when(configServerApi.put(
-                "/orchestrator/v1/suspensions/hosts/host1.test.yahoo.com?hostname=a1.host1.test.yahoo.com&hostname=a2.host1.test.yahoo.com",
-                Optional.empty(),
-                BatchOperationResult.class
+                eq("/orchestrator/v1/suspensions/hosts/host1.test.yahoo.com?hostname=a1.host1.test.yahoo.com&hostname=a2.host1.test.yahoo.com"),
+                eq(Optional.empty()),
+                eq(BatchOperationResult.class),
+                any()
         )).thenReturn(new BatchOperationResult(failureReason));
 
         orchestrator.suspend(parentHostName, hostNames);
@@ -148,9 +144,10 @@ public class OrchestratorImplTest {
         String exceptionMessage = "Exception: Something crashed!";
 
         when(configServerApi.put(
-                "/orchestrator/v1/suspensions/hosts/host1.test.yahoo.com?hostname=a1.host1.test.yahoo.com&hostname=a2.host1.test.yahoo.com",
-                Optional.empty(),
-                BatchOperationResult.class
+                eq("/orchestrator/v1/suspensions/hosts/host1.test.yahoo.com?hostname=a1.host1.test.yahoo.com&hostname=a2.host1.test.yahoo.com"),
+                eq(Optional.empty()),
+                eq(BatchOperationResult.class),
+                any()
         )).thenThrow(new RuntimeException(exceptionMessage));
 
         orchestrator.suspend(parentHostName, hostNames);