From 5d4a7acb4bb04a0a6dd0ae89267154428f3ea296 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 14 Jun 2019 21:04:20 +0200 Subject: Merge identical case --- .../com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 8c38b1bbd84..3c9073207d7 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -436,6 +436,7 @@ public class NodeAgentImpl implements NodeAgent { case reserved: case parked: case failed: + case inactive: removeContainerIfNeededUpdateContainerState(context, container); updateNodeRepoWithCurrentAttributes(context); break; @@ -481,10 +482,6 @@ public class NodeAgentImpl implements NodeAgent { context.log(logger, "Call resume against Orchestrator"); orchestrator.resume(context.hostname().value()); break; - case inactive: - removeContainerIfNeededUpdateContainerState(context, container); - updateNodeRepoWithCurrentAttributes(context); - break; case provisioned: nodeRepository.setNodeState(context.hostname().value(), NodeState.dirty); break; -- cgit v1.2.3 From 9d9f823be15f50a1881ded2b2a70826ea42a386f Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 14 Jun 2019 21:17:58 +0200 Subject: Do not ignore missing ACL --- .../noderepository/RealNodeRepository.java | 68 ++++++++++------------ 1 file changed, 31 insertions(+), 37 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index ca52eca13d2..d7ddc7656f5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -80,43 +80,37 @@ public class RealNodeRepository implements NodeRepository { */ @Override public Map getAcls(String hostName) { - try { - String path = String.format("/nodes/v2/acl/%s?children=true", hostName); - GetAclResponse response = configServerApi.get(path, GetAclResponse.class); - - // Group ports by container hostname that trusts them - Map> trustedPorts = response.trustedPorts.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Port::getTrustedBy, - Collectors.mapping(port -> port.port, Collectors.toSet()))); - - // Group node ip-addresses by container hostname that trusts them - Map> trustedNodes = response.trustedNodes.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Node::getTrustedBy, - Collectors.mapping( - node -> new Acl.Node(node.hostname, node.ipAddress), - Collectors.toSet()))); - - // Group trusted networks by container hostname that trusts them - Map> trustedNetworks = response.trustedNetworks.stream() - .collect(Collectors.groupingBy(GetAclResponse.Network::getTrustedBy, - Collectors.mapping(node -> node.network, Collectors.toSet()))); - - - // For each hostname create an ACL - return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) - .flatMap(Set::stream) - .distinct() - .collect(Collectors.toMap( - Function.identity(), - hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), - trustedNetworks.get(hostname)))); - } catch (HttpException.NotFoundException e) { - NODE_ADMIN_LOGGER.warning("Failed to fetch ACLs for " + hostName + " No ACL will be applied"); - } - - return Collections.emptyMap(); + String path = String.format("/nodes/v2/acl/%s?children=true", hostName); + GetAclResponse response = configServerApi.get(path, GetAclResponse.class); + + // Group ports by container hostname that trusts them + Map> trustedPorts = response.trustedPorts.stream() + .collect(Collectors.groupingBy( + GetAclResponse.Port::getTrustedBy, + Collectors.mapping(port -> port.port, Collectors.toSet()))); + + // Group node ip-addresses by container hostname that trusts them + Map> trustedNodes = response.trustedNodes.stream() + .collect(Collectors.groupingBy( + GetAclResponse.Node::getTrustedBy, + Collectors.mapping( + node -> new Acl.Node(node.hostname, node.ipAddress), + Collectors.toSet()))); + + // Group trusted networks by container hostname that trusts them + Map> trustedNetworks = response.trustedNetworks.stream() + .collect(Collectors.groupingBy(GetAclResponse.Network::getTrustedBy, + Collectors.mapping(node -> node.network, Collectors.toSet()))); + + + // For each hostname create an ACL + return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) + .flatMap(Set::stream) + .distinct() + .collect(Collectors.toMap( + Function.identity(), + hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), + trustedNetworks.get(hostname)))); } @Override -- cgit v1.2.3 From 42cc8bd0b0df30995102e4838a3212e6c840b42e Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 14 Jun 2019 21:19:27 +0200 Subject: Move more validation to NodeSpec --- .../configserver/noderepository/NodeSpec.java | 7 +++++ .../noderepository/RealNodeRepository.java | 30 +++++----------------- 2 files changed, 13 insertions(+), 24 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java index d402e75ff7b..52d6f16dd78 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeSpec.java @@ -91,6 +91,13 @@ public class NodeSpec { Set additionalIpAddresses, NodeReports reports, Optional parentHostname) { + if (state == NodeState.active) { + Objects.requireNonNull(wantedVespaVersion, "Unknown vespa version for active node"); + Objects.requireNonNull(wantedDockerImage, "Unknown docker image for active node"); + Objects.requireNonNull(wantedRestartGeneration, "Unknown restartGeneration for active node"); + Objects.requireNonNull(currentRestartGeneration, "Unknown currentRestartGeneration for active node"); + } + this.hostname = Objects.requireNonNull(hostname); this.wantedDockerImage = Objects.requireNonNull(wantedDockerImage); this.currentDockerImage = Objects.requireNonNull(currentDockerImage); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index d7ddc7656f5..3fa052a3a8a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -15,7 +15,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.No import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import java.time.Instant; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -143,30 +142,13 @@ public class RealNodeRepository implements NodeRepository { Objects.requireNonNull(node.state, "Unknown node state"); NodeState nodeState = NodeState.valueOf(node.state); - if (nodeState == NodeState.active) { - Objects.requireNonNull(node.wantedVespaVersion, "Unknown vespa version for active node"); - Objects.requireNonNull(node.wantedDockerImage, "Unknown docker image for active node"); - Objects.requireNonNull(node.restartGeneration, "Unknown restartGeneration for active node"); - Objects.requireNonNull(node.currentRestartGeneration, "Unknown currentRestartGeneration for active node"); - } - - String hostName = Objects.requireNonNull(node.hostname, "hostname is null"); - - NodeOwner owner = null; - if (node.owner != null) { - owner = new NodeOwner(node.owner.tenant, node.owner.application, node.owner.instance); - } - - NodeMembership membership = null; - if (node.membership != null) { - membership = new NodeMembership(node.membership.clusterType, node.membership.clusterId, - node.membership.group, node.membership.index, node.membership.retired); - } - NodeReports reports = NodeReports.fromMap(node.reports == null ? Collections.emptyMap() : node.reports); + Optional membership = Optional.ofNullable(node.membership) + .map(m -> new NodeMembership(m.clusterType, m.clusterId, m.group, m.index, m.retired)); + NodeReports reports = NodeReports.fromMap(Optional.ofNullable(node.reports).orElseGet(Map::of)); return new NodeSpec( - hostName, + node.hostname, Optional.ofNullable(node.wantedDockerImage).map(DockerImage::fromString), Optional.ofNullable(node.currentDockerImage).map(DockerImage::fromString), nodeState, @@ -179,8 +161,8 @@ public class RealNodeRepository implements NodeRepository { Optional.ofNullable(node.currentOsVersion).map(Version::fromString), Optional.ofNullable(node.allowedToBeDown), Optional.ofNullable(node.wantToDeprovision), - Optional.ofNullable(owner), - Optional.ofNullable(membership), + Optional.ofNullable(node.owner).map(o -> new NodeOwner(o.tenant, o.application, o.instance)), + membership, Optional.ofNullable(node.restartGeneration), Optional.ofNullable(node.currentRestartGeneration), node.rebootGeneration, -- cgit v1.2.3 From 00ef286aae9b1cfb63129a7771e5c2e49cb535fd Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 14 Jun 2019 21:30:31 +0200 Subject: Make NodeRepositoryException and OrchestratorException extend ConvergenceException --- .../noderepository/NodeRepositoryException.java | 4 +++- .../configserver/noderepository/RealNodeRepository.java | 6 +++--- .../configserver/orchestrator/OrchestratorException.java | 4 +++- .../admin/configserver/orchestrator/OrchestratorImpl.java | 13 +++++-------- .../vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java | 4 ++-- 5 files changed, 16 insertions(+), 15 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java index 6094518c3fc..10e61d373d2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/NodeRepositoryException.java @@ -1,7 +1,9 @@ // 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.noderepository; -public class NodeRepositoryException extends RuntimeException { +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; + +public class NodeRepositoryException extends ConvergenceException { public NodeRepositoryException(String message) { super(message); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index 3fa052a3a8a..9c41d70bff4 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -45,7 +45,7 @@ public class RealNodeRepository implements NodeRepository { NodeMessageResponse response = configServerApi.post("/nodes/v2/node", nodesToPost, NodeMessageResponse.class); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Failed to add nodes to node-repo: " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to add nodes: " + response.message + " " + response.errorCode); } @Override @@ -120,7 +120,7 @@ public class RealNodeRepository implements NodeRepository { NodeMessageResponse.class); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to update node attributes: " + response.message + " " + response.errorCode); } @Override @@ -133,7 +133,7 @@ public class RealNodeRepository implements NodeRepository { NODE_ADMIN_LOGGER.info(response.message); if (Strings.isNullOrEmpty(response.errorCode)) return; - throw new NodeRepositoryException("Unexpected message " + response.message + " " + response.errorCode); + throw new NodeRepositoryException("Failed to set node state: " + response.message + " " + response.errorCode); } private static NodeSpec createNodeSpec(NodeRepositoryNode node) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java index fe19da0c41c..8575bf7f655 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/orchestrator/OrchestratorException.java @@ -1,8 +1,10 @@ // Copyright 2017 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.orchestrator; +import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; + @SuppressWarnings("serial") -public class OrchestratorException extends RuntimeException { +public class OrchestratorException extends ConvergenceException { public OrchestratorException(String message) { super(message); } 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 64a67aa612a..6124e1bdc0e 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 @@ -40,8 +40,7 @@ public class OrchestratorImpl implements Orchestrator { } catch (HttpException.NotFoundException n) { throw new OrchestratorNotFoundException("Failed to suspend " + hostName + ", host not found"); } catch (HttpException e) { - throw new OrchestratorException("Failed to suspend " + hostName + ": " + - e.toString()); + throw new OrchestratorException("Failed to suspend " + hostName + ": " + e.toString()); } catch (RuntimeException e) { throw new RuntimeException("Got error on suspend", e); } @@ -60,9 +59,8 @@ public class OrchestratorImpl implements Orchestrator { parentHostName, params); batchOperationResult = configServerApi.put(url, Optional.empty(), BatchOperationResult.class); } catch (HttpException e) { - throw new OrchestratorException("Failed to batch suspend for " + - parentHostName + ": " + e.toString()); - } catch (Exception e) { + throw new OrchestratorException("Failed to batch suspend for " + parentHostName + ": " + e.toString()); + } catch (RuntimeException e) { throw new RuntimeException("Got error on batch suspend for " + parentHostName + ", with nodes " + hostNames, e); } @@ -80,9 +78,8 @@ public class OrchestratorImpl implements Orchestrator { } catch (HttpException.NotFoundException n) { throw new OrchestratorNotFoundException("Failed to resume " + hostName + ", host not found"); } catch (HttpException e) { - throw new OrchestratorException("Failed to suspend " + hostName + ": " + - e.toString()); - } catch (Exception e) { + throw new OrchestratorException("Failed to suspend " + hostName + ": " + e.toString()); + } catch (RuntimeException e) { throw new RuntimeException("Got error on resume", e); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 3c9073207d7..cfdcefec1e3 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -391,7 +391,7 @@ public class NodeAgentImpl implements NodeAgent { public void converge(NodeAgentContext context) { try { doConverge(context); - } catch (OrchestratorException | ConvergenceException e) { + } catch (ConvergenceException e) { context.log(logger, e.getMessage()); } catch (ContainerNotFoundException e) { containerState = ABSENT; @@ -494,7 +494,7 @@ public class NodeAgentImpl implements NodeAgent { nodeRepository.setNodeState(context.hostname().value(), NodeState.ready); break; default: - throw new RuntimeException("UNKNOWN STATE " + node.getState().name()); + throw new ConvergenceException("UNKNOWN STATE " + node.getState().name()); } } -- cgit v1.2.3 From ce3a75ab0a09cfc508f72ce26e0c9c9614650da3 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 14 Jun 2019 22:17:54 +0200 Subject: Handle well understood HTTP exceptions in config server client --- .../admin/configserver/ConfigServerApiImpl.java | 6 ++--- .../node/admin/configserver/HttpException.java | 31 ++++++++++++++++++++-- .../node/admin/configserver/state/StateImpl.java | 23 ++++------------ .../admin/configserver/state/StateImplTest.java | 3 ++- 4 files changed, 39 insertions(+), 24 deletions(-) (limited to 'node-admin') 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 0e6000c651b..3d1c49fc166 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 @@ -106,7 +106,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { try { return mapper.readValue(response.getEntity().getContent(), wantedReturnType); } catch (IOException e) { - throw new RuntimeException("Failed parse response from config server", e); + throw new UncheckedIOException("Failed parse response from config server", e); } } catch (HttpException e) { if (!e.isRetryable()) throw e; @@ -124,8 +124,8 @@ public class ConfigServerApiImpl implements ConfigServerApi { } } - throw new RuntimeException("All requests against the config servers (" - + configServers + ") failed, last as follows:", lastException); + throw HttpException.handleException( + "All requests against the config servers (" + configServers + ") failed, last as follows:", lastException); } @Override diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java index 256fe38ec68..3825107bfa6 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java @@ -1,13 +1,19 @@ // Copyright 2017 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 com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; +import org.apache.http.NoHttpResponseException; + import javax.ws.rs.core.Response; +import java.io.EOFException; +import java.net.SocketException; +import java.net.SocketTimeoutException; /** * @author hakonhall */ @SuppressWarnings("serial") -public class HttpException extends RuntimeException { +public class HttpException extends ConvergenceException { private final boolean isRetryable; @@ -21,7 +27,12 @@ public class HttpException extends RuntimeException { this.isRetryable = isRetryable; } - public boolean isRetryable() { + private HttpException(String message) { + super(message); + this.isRetryable = false; + } + + boolean isRetryable() { return isRetryable; } @@ -55,6 +66,22 @@ public class HttpException extends RuntimeException { throw new HttpException(status, message, true); } + /** + * Returns {@link HttpException} if the given Throwable is of a known and well understood error or + * a RuntimeException with the given exception as cause otherwise. + */ + public static RuntimeException handleException(String prefix, Throwable t) { + for (; t != null; t = t.getCause()) { + if (t instanceof SocketException || + t instanceof SocketTimeoutException || + t instanceof NoHttpResponseException || + t instanceof EOFException) + return new HttpException(prefix + t.getMessage()); + } + + return new RuntimeException(prefix, t); + } + public static class NotFoundException extends HttpException { public NotFoundException(String message) { super(Response.Status.NOT_FOUND, message, false); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java index efeb3039379..2fe8d4b4792 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.state; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse; /** @@ -16,26 +17,12 @@ public class StateImpl implements State { @Override public HealthCode getHealth() { - HealthResponse response; try { - response = configServerApi.get("/state/v1/health", HealthResponse.class); - } catch (RuntimeException e) { - if (causedByConnectionRefused(e)) { - return HealthCode.DOWN; - } - - throw e; + HealthResponse response = configServerApi.get("/state/v1/health", HealthResponse.class); + return HealthCode.fromString(response.status.code); + } catch (HttpException e) { + return HealthCode.DOWN; } - return HealthCode.fromString(response.status.code); } - private static boolean causedByConnectionRefused(Throwable throwable) { - for (Throwable cause = throwable; cause != null; cause = cause.getCause()) { - if (cause instanceof java.net.ConnectException) { - return true; - } - } - - return false; - } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java index 2604aa05367..14755ebf9cc 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.node.admin.configserver.state; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.HttpException; import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse; import org.junit.Test; @@ -28,7 +29,7 @@ public class StateImplTest { @Test public void connectException() { - RuntimeException exception = new RuntimeException(new ConnectException("connection refused")); + RuntimeException exception = HttpException.handleException("Error: ", new ConnectException("connection refused")); when(api.get(any(), any())).thenThrow(exception); HealthCode code = state.getHealth(); -- cgit v1.2.3 From 9c0424d37128f2bc97bc0ade122dab07b0bc2021 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 14 Jun 2019 22:19:15 +0200 Subject: Remove PrefixLogger --- .../admin/configserver/ConfigServerApiImpl.java | 8 +-- .../noderepository/RealNodeRepository.java | 6 +- .../vespa/hosted/node/admin/util/PrefixLogger.java | 66 ---------------------- 3 files changed, 7 insertions(+), 73 deletions(-) delete mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java (limited to 'node-admin') 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 3d1c49fc166..73c86fc8de1 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 @@ -7,7 +7,6 @@ import com.yahoo.config.provision.HostName; import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory; import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; -import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import org.apache.http.HttpHeaders; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; @@ -38,6 +37,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.logging.Logger; /** * Retries request on config server a few times before giving up. Assumes that all requests should be sent with @@ -47,7 +47,7 @@ import java.util.Optional; * @author bjorncs */ public class ConfigServerApiImpl implements ConfigServerApi { - private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class); + private static final Logger logger = Logger.getLogger(ConfigServerApiImpl.class.getName()); private final ObjectMapper mapper = new ObjectMapper(); @@ -117,9 +117,9 @@ public class ConfigServerApiImpl implements ConfigServerApi { // Failure to communicate with a config server is not abnormal during upgrades if (e.getMessage().contains("(Connection refused)")) { - NODE_ADMIN_LOGGER.info("Connection refused to " + configServer + " (upgrading?), will try next"); + logger.info("Connection refused to " + configServer + " (upgrading?), will try next"); } else { - NODE_ADMIN_LOGGER.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); + logger.warning("Failed to communicate with " + configServer + ", will try next: " + e.getMessage()); } } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index 9c41d70bff4..fe19b81614d 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.Ge import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.GetNodesResponse; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeMessageResponse; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.bindings.NodeRepositoryNode; -import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import java.time.Instant; import java.util.List; @@ -22,6 +21,7 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Function; +import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -29,7 +29,7 @@ import java.util.stream.Stream; * @author stiankri, dybis */ public class RealNodeRepository implements NodeRepository { - private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(RealNodeRepository.class); + private static final Logger logger = Logger.getLogger(RealNodeRepository.class.getName()); private final ConfigServerApi configServerApi; @@ -130,7 +130,7 @@ public class RealNodeRepository implements NodeRepository { "/nodes/v2/state/" + state + "/" + hostName, Optional.empty(), /* body */ NodeMessageResponse.class); - NODE_ADMIN_LOGGER.info(response.message); + logger.info(response.message); if (Strings.isNullOrEmpty(response.errorCode)) return; throw new NodeRepositoryException("Failed to set node state: " + response.message + " " + response.errorCode); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java deleted file mode 100644 index f4d85a19f6d..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/PrefixLogger.java +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2017 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.util; - -import com.yahoo.log.LogLevel; -import com.yahoo.vespa.hosted.dockerapi.ContainerName; - -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * @author freva - */ -public class PrefixLogger { - private final String prefix; - private final Logger logger; - - private PrefixLogger(Class clazz, String prefix) { - this.logger = Logger.getLogger(clazz.getName()); - this.prefix = prefix + ": "; - } - - public static PrefixLogger getNodeAdminLogger(Class clazz) { - return new PrefixLogger(clazz, "NodeAdmin"); - } - - public static PrefixLogger getNodeAgentLogger(Class clazz, ContainerName containerName) { - return new PrefixLogger(clazz, "NodeAgent-" + containerName.asString()); - } - - private void log(Level level, String message, Throwable thrown) { - logger.log(level, prefix + message, thrown); - } - - private void log(Level level, String message) { - logger.log(level, prefix + message); - } - - - public void debug(String message) { - log(LogLevel.DEBUG, message); - } - - public void info(String message) { - log(LogLevel.INFO, message); - } - - public void info(String message, Throwable thrown) { - log(LogLevel.INFO, message, thrown); - } - - public void error(String message) { - log(LogLevel.ERROR, message); - } - - public void error(String message, Throwable thrown) { - log(LogLevel.ERROR, message, thrown); - } - - public void warning(String message) { - log(LogLevel.WARNING, message); - } - - public void warning(String message, Throwable thrown) { - log(LogLevel.WARNING, message, thrown); - } -} -- cgit v1.2.3