diff options
4 files changed, 218 insertions, 10 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java index e272067a025..7c095c7a907 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImpl.java @@ -25,16 +25,16 @@ import java.util.Set; public class OrchestratorImpl implements Orchestrator { private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(OrchestratorImpl.class); // TODO: Figure out the port dynamically. - private static final int HARDCODED_ORCHESTRATOR_PORT = 19071; + static final int HARDCODED_ORCHESTRATOR_PORT = 19071; // TODO: Find a way to avoid duplicating this (present in orchestrator's services.xml also). private static final String ORCHESTRATOR_PATH_PREFIX = "/orchestrator"; - private static final String ORCHESTRATOR_PATH_PREFIX_HOST_API + static final String ORCHESTRATOR_PATH_PREFIX_HOST_API = ORCHESTRATOR_PATH_PREFIX + HostApi.PATH_PREFIX; - private final ConfigServerHttpRequestExecutor requestExecutor; - - private static final String ORCHESTRATOR_PATH_PREFIX_HOST_SUSPENSION_API + static final String ORCHESTRATOR_PATH_PREFIX_HOST_SUSPENSION_API = ORCHESTRATOR_PATH_PREFIX + HostSuspensionApi.PATH_PREFIX; + private final ConfigServerHttpRequestExecutor requestExecutor; + public OrchestratorImpl(ConfigServerHttpRequestExecutor requestExecutor) { this.requestExecutor = requestExecutor; } @@ -53,7 +53,7 @@ public class OrchestratorImpl implements Orchestrator { return updateHostResponse.reason() == null; } catch (ConfigServerHttpRequestExecutor.NotFoundException n) { // Orchestrator doesn't care about this node, so don't let that stop us. - logger.info("Got not found on delete, resuming"); + logger.info("Got not found on delete, suspending"); return true; } catch (Exception e) { logger.info("Got error on suspend " + hostName, e); @@ -71,6 +71,7 @@ public class OrchestratorImpl implements Orchestrator { BatchOperationResult.class); return batchOperationResult.getFailureReason(); } catch (Exception e) { + NODE_ADMIN_LOGGER.info("Got error on batch suspend for " + parentHostName + ", with nodes " + hostNames, e); return Optional.of(e.getMessage()); } } @@ -89,8 +90,8 @@ public class OrchestratorImpl implements Orchestrator { // Orchestrator doesn't care about this node, so don't let that stop us. logger.info("Got not found on delete, resuming"); return true; - } - catch (Exception e) { + } catch (Exception e) { + logger.info("Got error on resume " + hostName, e); return false; } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java index 78562ee2d15..1952b54f5fc 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java @@ -78,8 +78,10 @@ public class ConfigServerHttpRequestExecutor { throw new NotFoundException("Not found returned from " + configServer); } if (response.getStatusLine().getStatusCode() != Response.Status.OK.getStatusCode()) { - NODE_ADMIN_LOGGER.info("Non 200 received:\n" + read(response.getEntity())); - throw new RuntimeException("Did not get 200, but " + response.getStatusLine().getStatusCode()); + String entity = read(response.getEntity()); + NODE_ADMIN_LOGGER.info("Non 200 received:\n" + entity); + throw new RuntimeException("Did not get 200, but " + response.getStatusLine().getStatusCode() + + entity); } try { return mapper.readValue(response.getEntity().getContent(), wantedReturnType); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImplTest.java new file mode 100644 index 00000000000..9b2dcb07eb3 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/orchestrator/OrchestratorImplTest.java @@ -0,0 +1,185 @@ +package com.yahoo.vespa.hosted.node.admin.orchestrator; + +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.hosted.node.admin.util.ConfigServerHttpRequestExecutor; +import com.yahoo.vespa.orchestrator.restapi.wire.BatchHostSuspendRequest; +import com.yahoo.vespa.orchestrator.restapi.wire.BatchOperationResult; +import com.yahoo.vespa.orchestrator.restapi.wire.HostStateChangeDenialReason; +import com.yahoo.vespa.orchestrator.restapi.wire.UpdateHostResponse; +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +/** + * @author valerijf + */ +public class OrchestratorImplTest { + private static final HostName hostName = new HostName("host123.yahoo.com"); + private ConfigServerHttpRequestExecutor requestExecutor; + private OrchestratorImpl orchestrator; + + @Before + public void setup() { + requestExecutor = mock(ConfigServerHttpRequestExecutor.class); + orchestrator = new OrchestratorImpl(requestExecutor); + } + + @Test + public void testSuspendCall() { + when(requestExecutor.put( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended", + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + Optional.empty(), + UpdateHostResponse.class + )).thenReturn(new UpdateHostResponse(hostName.s(), null)); + + boolean response = orchestrator.suspend(hostName); + assertTrue("Expected Orchestrator to approve", response); + } + + @Test + public void testSuspendCallWithFailureReason() { + when(requestExecutor.put( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended", + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + Optional.empty(), + UpdateHostResponse.class + )).thenReturn(new UpdateHostResponse(hostName.s(), new HostStateChangeDenialReason("hostname", "service", "fail"))); + + boolean response = orchestrator.suspend(hostName); + assertFalse("Expected Orchestrator to deny when presented with HostChangeDenialReason", response); + } + + @Test + public void testSuspendCallWithNotFound() { + when(requestExecutor.put( + any(String.class), + any(Integer.class), + any(), + any() + )).thenThrow(requestExecutor.new NotFoundException("Not Found")); + + boolean response = orchestrator.suspend(hostName); + assertTrue("Expected Orchestrator to respond with true even when NotFoundException is thrown", response); + } + + @Test + public void testSuspendCallWithSomeOtherException() { + when(requestExecutor.put( + any(String.class), + any(Integer.class), + any(), + any() + )).thenThrow(new RuntimeException("Some parameter was wrong")); + + boolean response = orchestrator.suspend(hostName); + assertFalse("Expected Orchestrator to respond with false when some other exception is thrown", response); + } + + + @Test + public void testResumeCall() { + when(requestExecutor.delete( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended", + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + UpdateHostResponse.class + )).thenReturn(new UpdateHostResponse(hostName.s(), null)); + + boolean response = orchestrator.resume(hostName); + assertTrue("Expected Orchestrator to approve", response); + } + + @Test + public void testResumeCallWithFailureReason() { + when(requestExecutor.delete( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_API + "/" + hostName+ "/suspended", + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + UpdateHostResponse.class + )).thenReturn(new UpdateHostResponse(hostName.s(), new HostStateChangeDenialReason("hostname", "service", "fail"))); + + boolean response = orchestrator.resume(hostName); + assertFalse("Expected Orchestrator to deny when presented with HostChangeDenialReason", response); + } + + @Test + public void testResumeCallWithNotFound() { + when(requestExecutor.delete( + any(String.class), + any(Integer.class), + any() + )).thenThrow(requestExecutor.new NotFoundException("Not Found")); + + boolean response = orchestrator.resume(hostName); + assertTrue("Expected Orchestrator to respond with true even when NotFoundException is thrown", response); + } + + @Test + public void testResumeCallWithSomeOtherException() { + when(requestExecutor.put( + any(String.class), + any(Integer.class), + any(), + any() + )).thenThrow(new RuntimeException("Some parameter was wrong")); + + boolean response = orchestrator.suspend(hostName); + assertFalse("Expected Orchestrator to respond with false when some other exception is thrown", response); + } + + + @Test + public void testBatchSuspendCall() { + String parentHostName = "host1.test.yahoo.com"; + List<String> hostNames = Arrays.asList("a1.host1.test.yahoo.com", "a2.host1.test.yahoo.com"); + + when(requestExecutor.put( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_SUSPENSION_API, + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + Optional.of(new BatchHostSuspendRequest(parentHostName, hostNames)), + BatchOperationResult.class + )).thenReturn(BatchOperationResult.successResult()); + + Optional<String> response = orchestrator.suspend(parentHostName, hostNames); + assertFalse("Expected failureReason to be empty", response.isPresent()); + } + + @Test + public void testBatchSuspendCallWithFailureReason() { + String parentHostName = "host1.test.yahoo.com"; + List<String> hostNames = Arrays.asList("a1.host1.test.yahoo.com", "a2.host1.test.yahoo.com"); + String failureReason = "Failed to suspend"; + + when(requestExecutor.put( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_SUSPENSION_API, + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + Optional.of(new BatchHostSuspendRequest(parentHostName, hostNames)), + BatchOperationResult.class + )).thenReturn(new BatchOperationResult(failureReason)); + + Optional<String> response = orchestrator.suspend(parentHostName, hostNames); + assertEquals("Expected failureReason to be empty", response, Optional.of(failureReason)); + } + + @Test + public void testBatchSuspendCallWithSomeException() { + String parentHostName = "host1.test.yahoo.com"; + List<String> hostNames = Arrays.asList("a1.host1.test.yahoo.com", "a2.host1.test.yahoo.com"); + String exceptionMessage = "Exception: Something crashed!"; + + when(requestExecutor.put( + OrchestratorImpl.ORCHESTRATOR_PATH_PREFIX_HOST_SUSPENSION_API, + OrchestratorImpl.HARDCODED_ORCHESTRATOR_PORT, + Optional.of(new BatchHostSuspendRequest(parentHostName, hostNames)), + BatchOperationResult.class + )).thenThrow(new RuntimeException(exceptionMessage)); + + Optional<String> response = orchestrator.suspend(parentHostName, hostNames); + assertEquals("Expected failureReason to be empty", response, Optional.of(exceptionMessage)); + } +} diff --git a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/BatchHostSuspendRequest.java b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/BatchHostSuspendRequest.java index 8311967e1bb..739799fc37a 100644 --- a/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/BatchHostSuspendRequest.java +++ b/orchestrator-restapi/src/main/java/com/yahoo/vespa/orchestrator/restapi/wire/BatchHostSuspendRequest.java @@ -38,6 +38,26 @@ public class BatchHostSuspendRequest { } @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + BatchHostSuspendRequest that = (BatchHostSuspendRequest) o; + + if (parentHostname != null ? !parentHostname.equals(that.parentHostname) : that.parentHostname != null) + return false; + return hostnames != null ? hostnames.equals(that.hostnames) : that.hostnames == null; + + } + + @Override + public int hashCode() { + int result = parentHostname != null ? parentHostname.hashCode() : 0; + result = 31 * result + (hostnames != null ? hostnames.hashCode() : 0); + return result; + } + + @Override public String toString() { return "BatchHostSuspendRequest{" + "parentHostname=" + parentHostname + |