diff options
author | valerijf <valerijf@yahoo-inc.com> | 2017-04-25 11:23:39 +0200 |
---|---|---|
committer | valerijf <valerijf@yahoo-inc.com> | 2017-04-25 11:23:39 +0200 |
commit | f54f696a8d97c30f40665c535a394a8524ad2d02 (patch) | |
tree | 3d6903aab6303ea3749ba9439f83096382857ec4 /node-admin | |
parent | 646c9cd19bd29a9a6d69adf03f2aebb089869550 (diff) |
Do not throw RuntimeException on non-200 response
Diffstat (limited to 'node-admin')
6 files changed, 39 insertions, 38 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java index 35582c1bc2e..c3755fc8d5b 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImpl.java @@ -21,7 +21,6 @@ import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; /** @@ -33,10 +32,10 @@ public class NodeRepositoryImpl implements NodeRepository { private final int port; private final ConfigServerHttpRequestExecutor requestExecutor; - public NodeRepositoryImpl(Set<String> configServerHosts, int configPort, String baseHostName) { + public NodeRepositoryImpl(ConfigServerHttpRequestExecutor requestExecutor, int configPort, String baseHostName) { this.baseHostName = baseHostName; this.port = configPort; - this.requestExecutor = ConfigServerHttpRequestExecutor.create(configServerHosts); + this.requestExecutor = requestExecutor; } @Override @@ -154,21 +153,25 @@ public class NodeRepositoryImpl implements NodeRepository { @Override public void markAsDirty(String hostName) { - NodeMessageResponse response = requestExecutor.put( - "/nodes/v2/state/dirty/" + hostName, - port, - Optional.empty(), /* body */ - NodeMessageResponse.class); - NODE_ADMIN_LOGGER.info(response.message); + markNodeToState(hostName, Node.State.dirty); } @Override public void markAsReady(final String hostName) { + markNodeToState(hostName, Node.State.ready); + } + + private void markNodeToState(String hostName, Node.State state) { NodeMessageResponse response = requestExecutor.put( - "/nodes/v2/state/ready/" + hostName, + "/nodes/v2/state/" + state + "/" + hostName, port, Optional.empty(), /* body */ NodeMessageResponse.class); NODE_ADMIN_LOGGER.info(response.message); + + if (response.errorCode == null || response.errorCode.isEmpty()) { + return; + } + throw new RuntimeException("Unexpected message " + response.message + " " + response.errorCode); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/bindings/NodeMessageResponse.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/bindings/NodeMessageResponse.java index 929cf62ce74..8dd293013f3 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/bindings/NodeMessageResponse.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/noderepository/bindings/NodeMessageResponse.java @@ -13,4 +13,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; public class NodeMessageResponse { @JsonProperty("message") public String message; + @JsonProperty("error-code") + public String errorCode; } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java index 5d227b26eb4..e9e5e15712d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/ComponentsProviderImpl.java @@ -62,8 +62,9 @@ public class ComponentsProviderImpl implements ComponentsProvider { throw new IllegalStateException("Environment setting for config servers missing or empty."); } - Orchestrator orchestrator = new OrchestratorImpl(ConfigServerHttpRequestExecutor.create(configServerHosts)); - NodeRepository nodeRepository = new NodeRepositoryImpl(configServerHosts, WEB_SERVICE_PORT, baseHostName); + ConfigServerHttpRequestExecutor requestExecutor = ConfigServerHttpRequestExecutor.create(configServerHosts); + Orchestrator orchestrator = new OrchestratorImpl(requestExecutor); + NodeRepository nodeRepository = new NodeRepositoryImpl(requestExecutor, WEB_SERVICE_PORT, baseHostName); DockerOperations dockerOperations = new DockerOperationsImpl(docker, environment, metricReceiver); Optional<StorageMaintainer> storageMaintainer = isRunningLocally ? 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 239829bc094..87eb80c1837 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 @@ -96,12 +96,7 @@ public class ConfigServerHttpRequestExecutor { if (response.getStatusLine().getStatusCode() == Response.Status.NOT_FOUND.getStatusCode()) { throw new NotFoundException("Not found returned from " + configServer); } - if (response.getStatusLine().getStatusCode() != Response.Status.OK.getStatusCode()) { - String entity = read(response.getEntity()); - NODE_ADMIN_LOGGER.info("Non-200 HTTP response code received:\n" + entity); - throw new RuntimeException("Did not get response code 200, but " + response.getStatusLine().getStatusCode() + - entity); - } + try { return mapper.readValue(response.getEntity().getContent(), wantedReturnType); } catch (IOException e) { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java index 1e027bdd4a6..5c6717075dc 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/noderepository/NodeRepositoryImplTest.java @@ -2,12 +2,12 @@ package com.yahoo.vespa.hosted.node.admin.noderepository; -import com.google.common.collect.Sets; import com.yahoo.application.Networking; import com.yahoo.application.container.JDisc; import com.yahoo.vespa.hosted.node.admin.ContainerNodeSpec; import com.yahoo.vespa.hosted.dockerapi.DockerImage; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAttributes; +import com.yahoo.vespa.hosted.node.admin.util.ConfigServerHttpRequestExecutor; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig; @@ -18,9 +18,9 @@ import org.junit.Test; import java.io.IOException; import java.net.ServerSocket; import java.time.Instant; +import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.Set; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertFalse; @@ -36,7 +36,8 @@ import static org.junit.Assert.fail; public class NodeRepositoryImplTest { private JDisc container; private int port; - private final Set<String> configServerHosts = Sets.newHashSet("127.0.0.1"); + private final ConfigServerHttpRequestExecutor requestExecutor = ConfigServerHttpRequestExecutor.create( + Collections.singleton("127.0.0.1")); private int findRandomOpenPort() throws IOException { @@ -62,7 +63,7 @@ public class NodeRepositoryImplTest { private void waitForJdiscContainerToServe() throws InterruptedException { Instant start = Instant.now(); - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(Sets.newHashSet("127.0.0.1"), port, "foobar"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "foobar"); while (Instant.now().minusSeconds(120).isBefore(start)) { try { nodeRepositoryApi.getContainersToRun(); @@ -84,7 +85,7 @@ public class NodeRepositoryImplTest { @Test public void testGetContainersToRunAPi() throws IOException, InterruptedException { waitForJdiscContainerToServe(); - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(configServerHosts, port, "dockerhost4"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); final List<ContainerNodeSpec> containersToRun = nodeRepositoryApi.getContainersToRun(); assertThat(containersToRun.size(), is(1)); final ContainerNodeSpec nodeSpec = containersToRun.get(0); @@ -101,7 +102,7 @@ public class NodeRepositoryImplTest { @Test public void testGetContainer() throws InterruptedException, IOException { waitForJdiscContainerToServe(); - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(configServerHosts, port, "dockerhost4"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); String hostname = "host4.yahoo.com"; Optional<ContainerNodeSpec> nodeSpec = nodeRepositoryApi.getContainerNodeSpec(hostname); assertThat(nodeSpec.isPresent(), is(true)); @@ -111,7 +112,7 @@ public class NodeRepositoryImplTest { @Test public void testGetContainerForNonExistingNode() throws InterruptedException, IOException { waitForJdiscContainerToServe(); - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(configServerHosts, port, "dockerhost4"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); String hostname = "host-that-does-not-exist"; Optional<ContainerNodeSpec> nodeSpec = nodeRepositoryApi.getContainerNodeSpec(hostname); assertFalse(nodeSpec.isPresent()); @@ -120,7 +121,7 @@ public class NodeRepositoryImplTest { @Test public void testUpdateNodeAttributes() throws InterruptedException, IOException { waitForJdiscContainerToServe(); - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(configServerHosts, port, "dockerhost4"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); String hostname = "host4.yahoo.com"; nodeRepositoryApi.updateNodeAttributes( hostname, @@ -133,7 +134,7 @@ public class NodeRepositoryImplTest { @Test(expected = RuntimeException.class) public void testUpdateNodeAttributesWithBadValue() throws InterruptedException, IOException { waitForJdiscContainerToServe(); - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(configServerHosts, port, "dockerhost4"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); String hostname = "host4.yahoo.com"; nodeRepositoryApi.updateNodeAttributes( hostname, @@ -145,7 +146,7 @@ public class NodeRepositoryImplTest { @Test public void testMarkAsReady() throws InterruptedException, IOException { - NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(configServerHosts, port, "dockerhost4"); + NodeRepository nodeRepositoryApi = new NodeRepositoryImpl(requestExecutor, port, "dockerhost4"); waitForJdiscContainerToServe(); nodeRepositoryApi.markAsReady("host55.yahoo.com"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java index cc1db1eb3c7..7d6a95d8dc8 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java @@ -5,13 +5,11 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.yahoo.collections.ArraySet; import org.apache.http.HttpEntity; -import org.apache.http.HttpResponse; import org.apache.http.StatusLine; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.impl.client.CloseableHttpClient; import org.junit.Test; -import org.mockito.stubbing.Answer; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -22,6 +20,7 @@ import java.util.Set; 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.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; @@ -40,6 +39,8 @@ public class ConfigServerHttpRequestExecutorTest { public static class TestPojo { @JsonProperty("foo") public String foo; + @JsonProperty("error-code") + public Integer errorCode; } private final StringBuilder mockLog = new StringBuilder(); @@ -47,7 +48,7 @@ public class ConfigServerHttpRequestExecutorTest { private CloseableHttpClient createClientMock() throws IOException { CloseableHttpClient httpMock = mock(CloseableHttpClient.class); - when(httpMock.execute(any())).thenAnswer((Answer<HttpResponse>) invocationOnMock -> { + when(httpMock.execute(any())).thenAnswer(invocationOnMock -> { HttpGet get = (HttpGet) invocationOnMock.getArguments()[0]; mockLog.append(get.getMethod()).append(" ").append(get.getURI()).append(" "); CloseableHttpResponse response = mock(CloseableHttpResponse.class); @@ -57,7 +58,8 @@ public class ConfigServerHttpRequestExecutorTest { if (mockReturnCode == 100000) throw new RuntimeException("FAIL"); HttpEntity entity = mock(HttpEntity.class); when(response.getEntity()).thenReturn(entity); - InputStream stream = new ByteArrayInputStream("{ \"foo\":\"bar\", \"no\":3}".getBytes(StandardCharsets.UTF_8)); + String returnMessage = "{\"foo\":\"bar\", \"no\":3, \"error-code\": " + mockReturnCode + "}"; + InputStream stream = new ByteArrayInputStream(returnMessage.getBytes(StandardCharsets.UTF_8)); when(entity.getContent()).thenReturn(stream); return response; }); @@ -84,12 +86,9 @@ public class ConfigServerHttpRequestExecutorTest { // Server is returning 400, no retries. mockReturnCode = 400; ConfigServerHttpRequestExecutor executor = new ConfigServerHttpRequestExecutor(configServers, createClientMock()); - try { - executor.get("/path", 666, TestPojo.class); - fail("Expected failure"); - } catch (Exception e) { - // ignore - } + + TestPojo testPojo = executor.get("/path", 666, TestPojo.class); + assertEquals(testPojo.errorCode.intValue(), mockReturnCode); assertLogStringContainsGETForAHost(); } |