diff options
33 files changed, 688 insertions, 404 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java index daa237d90c1..02bb0b412d1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/AbstractService.java @@ -42,7 +42,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon private int id = 0; /** The actual base port for this Service. */ - private int basePort; + private int basePort = 0; /** The ports allocated to this Service. */ private List<Integer> ports = new ArrayList<>(); @@ -483,8 +483,7 @@ public abstract class AbstractService extends AbstractConfigProducer<AbstractCon * currently uses the first port as container http port. */ public void reservePortPrepended(int port, String suffix) { - hostResource.reservePort(this, port, suffix); - ports.add(0, port); + ports.add(0, hostResource.ports().requireNetworkPort(port, this, suffix)); } public void setHostResource(HostResource hostResource) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostPorts.java b/config-model/src/main/java/com/yahoo/vespa/model/HostPorts.java new file mode 100644 index 00000000000..7d4b1916eaa --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/HostPorts.java @@ -0,0 +1,221 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.model; + +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.provision.NetworkPorts; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.logging.Level; + +/** + * Allocator for network ports on a host + * @author arnej + */ +public class HostPorts { + + public HostPorts(String hostname) { + this.hostname = hostname; + } + + final String hostname; + public final static int BASE_PORT = 19100; + final static int MAX_PORTS = 799; + + private DeployLogger deployLogger = new DeployLogger() { + public void log(Level level, String message) { + System.err.println("deploy log["+level+"]: "+message); + } + }; + + private final Map<Integer, NetworkPortRequestor> portDB = new LinkedHashMap<>(); + + private int allocatedPorts = 0; + + private PortFinder portFinder = new PortFinder(Collections.emptyList()); + + private Optional<NetworkPorts> networkPortsList = Optional.empty(); + + /** + * Get the allocated network ports. + * Should be called after allocation is complete and flushPortReservations has been called + **/ + public Optional<NetworkPorts> networkPorts() { return networkPortsList; } + + /** + * Add port allocation from previous deployments. + * Call this before starting port allocations, to re-use existing ports where possible + **/ + public void addNetworkPorts(NetworkPorts ports) { + this.networkPortsList = Optional.of(ports); + this.portFinder = new PortFinder(ports.allocations()); + } + + /** + * Setup logging in order to send warnings back to the user. + **/ + public void useLogger(DeployLogger logger) { + this.deployLogger = logger; + } + + /** + * Returns the baseport of the first available port range of length numPorts, + * or 0 if there is no range of that length available. + * + * @param numPorts The length of the desired port range. + * @return The baseport of the first available range, or 0 if no range is available. + */ + public int nextAvailableBaseport(int numPorts) { + int range = 0; + int port = BASE_PORT; + for (; port < BASE_PORT + MAX_PORTS && (range < numPorts); port++) { + if (portDB.containsKey(port)) { + range = 0; + continue; + } + range++; + } + return range == numPorts ? port - range : 0; + } + + private int nextAvailableNetworkPort() { + int port = BASE_PORT; + for (; port < BASE_PORT + MAX_PORTS; port++) { + if (!portDB.containsKey(port)) return port; + } + return 0; + } + + /** Allocate a specific port number for a service */ + public int requireNetworkPort(int port, NetworkPortRequestor service, String suffix) { + reservePort(service, port, suffix); + String servType = service.getServiceType(); + String configId = service.getConfigId(); + portFinder.use(new NetworkPorts.Allocation(port, servType, configId, suffix)); + return port; + } + + /** Allocate a preferred port number for a service, fall back to using any dynamic port */ + public int wantNetworkPort(int port, NetworkPortRequestor service, String suffix) { + if (portDB.containsKey(port)) { + int fallback = nextAvailableNetworkPort(); + NetworkPortRequestor s = portDB.get(port); + deployLogger.log(Level.WARNING, + service.getServiceName() +" cannot reserve port " + port + " on " + + hostname + ": Already reserved for " + s.getServiceName() + + ". Using default port range from " + fallback); + return allocateNetworkPort(service, suffix); + } + return requireNetworkPort(port, service, suffix); + } + + /** Convenience method to allocate a preferred or required port number for a service */ + public int wantNetworkPort(int port, NetworkPortRequestor service, String suffix, boolean forceRequired) { + return forceRequired ? requireNetworkPort(port, service, suffix) : wantNetworkPort(port, service, suffix); + } + + /** Allocate a dynamic port number for a service */ + public int allocateNetworkPort(NetworkPortRequestor service, String suffix) { + String servType = service.getServiceType(); + String configId = service.getConfigId(); + int fallback = nextAvailableNetworkPort(); + int port = portFinder.findPort(new NetworkPorts.Allocation(fallback, servType, configId, suffix)); + reservePort(service, port, suffix); + portFinder.use(new NetworkPorts.Allocation(port, servType, configId, suffix)); + return port; + } + + /** Allocate all ports for a service */ + List<Integer> allocatePorts(NetworkPortRequestor service, int wantedPort) { + List<Integer> ports = new ArrayList<>(); + final int count = service.getPortCount(); + if (count < 1) + return ports; + + String[] suffixes = service.getPortSuffixes(); + if (suffixes.length != count) { + throw new IllegalArgumentException("service "+service+" had "+suffixes.length+" port suffixes, but port count "+count+", mismatch"); + } + + if (wantedPort > 0) { + boolean force = service.requiresWantedPort(); + if (service.requiresConsecutivePorts()) { + for (int i = 0; i < count; i++) { + ports.add(wantNetworkPort(wantedPort+i, service, suffixes[i], force)); + } + } else { + ports.add(wantNetworkPort(wantedPort, service, suffixes[0], force)); + for (int i = 1; i < count; i++) { + ports.add(allocateNetworkPort(service, suffixes[i])); + } + } + } else { + for (int i = 0; i < count; i++) { + ports.add(allocateNetworkPort(service, suffixes[i])); + } + } + return ports; + } + + public void flushPortReservations() { + this.networkPortsList = Optional.of(new NetworkPorts(portFinder.allocations())); + } + + /** + * Reserves the desired port for the given service, or throws as exception if the port + * is not available. + * + * @param service the service that wishes to reserve the port. + * @param port the port to be reserved. + */ + void reservePort(NetworkPortRequestor service, int port, String suffix) { + if (portDB.containsKey(port)) { + portAlreadyReserved(service, port); + } + if (inVespasPortRange(port)) { + allocatedPorts++; + if (allocatedPorts > MAX_PORTS) { + noMoreAvailablePorts(); + } + } + portDB.put(port, service); + } + + private boolean inVespasPortRange(int port) { + return port >= BASE_PORT && + port < BASE_PORT + MAX_PORTS; + } + + private void portAlreadyReserved(NetworkPortRequestor service, int port) { + NetworkPortRequestor otherService = portDB.get(port); + int nextAvailablePort = nextAvailableBaseport(service.getPortCount()); + if (nextAvailablePort == 0) { + noMoreAvailablePorts(); + } + String msg = (service.getClass().equals(otherService.getClass()) && service.requiresWantedPort()) + ? "You must set port explicitly for all instances of this service type, except the first one. " + : ""; + throw new RuntimeException(service.getServiceName() + " cannot reserve port " + port + + " on " + hostname + ": Already reserved for " + otherService.getServiceName() + + ". " + msg + "Next available port is: " + nextAvailablePort + " ports used: " + portDB); + } + + private void noMoreAvailablePorts() { + throw new RuntimeException + ("Too many ports are reserved in Vespa's port range (" + + BASE_PORT + ".." + (BASE_PORT+MAX_PORTS) + ") on " + hostname + + ". Move one or more services to another host, or outside this port range."); + } + + @Override + public String toString() { + return "HostPorts{"+hostname+"}"; + } + +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java b/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java index 3a8cc5c2e4c..2d091ad7008 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/HostResource.java @@ -28,32 +28,17 @@ import java.util.stream.Collectors; * * @author Ulf Lilleengen */ -public class HostResource implements Comparable<HostResource> { +public class HostResource implements Comparable<HostResource> +{ + private final HostPorts hostPorts; + + public HostPorts ports() { return hostPorts; } - public final static int BASE_PORT = 19100; - final static int MAX_PORTS = 799; private final Host host; /** Map from "sentinel name" to service */ private final Map<String, Service> services = new LinkedHashMap<>(); - private final Map<Integer, NetworkPortRequestor> portDB = new LinkedHashMap<>(); - - private int allocatedPorts = 0; - - static class PortReservation { - int gotPort; - NetworkPortRequestor service; - String suffix; - PortReservation(int port, NetworkPortRequestor svc, String suf) { - this.gotPort = port; - this.service = svc; - this.suffix = suf; - } - } - - private List<PortReservation> portReservations = new ArrayList<>(); - private Set<ClusterMembership> clusterMemberships = new LinkedHashSet<>(); // Empty for self-hosted Vespa. @@ -62,12 +47,6 @@ public class HostResource implements Comparable<HostResource> { /** The current Vespa version running on this node, or empty if not known */ private final Optional<Version> version; - private Optional<NetworkPorts> networkPortsList = Optional.empty(); - - public Optional<NetworkPorts> networkPorts() { return networkPortsList; } - - public void addNetworkPorts(NetworkPorts ports) { this.networkPortsList = Optional.of(ports); } - /** * Create a new {@link HostResource} bound to a specific {@link com.yahoo.vespa.model.Host}. * @@ -78,6 +57,7 @@ public class HostResource implements Comparable<HostResource> { } public HostResource(Host host, Optional<Version> version) { + this.hostPorts = new HostPorts(host.getHostname()); this.host = host; this.version = version; } @@ -92,26 +72,6 @@ public class HostResource implements Comparable<HostResource> { public Optional<Version> version() { return version; } /** - * Returns the baseport of the first available port range of length numPorts, - * or 0 if there is no range of that length available. - * - * @param numPorts The length of the desired port range. - * @return The baseport of the first available range, or 0 if no range is available. - */ - public int nextAvailableBaseport(int numPorts) { - int range = 0; - int port = BASE_PORT; - for (; port < BASE_PORT + MAX_PORTS && (range < numPorts); port++) { - if (portDB.containsKey(port)) { - range = 0; - continue; - } - range++; - } - return range == numPorts ? port - range : 0; - } - - /** * Adds service and allocates resources for it. * * @param service The Service to allocate resources for @@ -119,7 +79,8 @@ public class HostResource implements Comparable<HostResource> { * @return The allocated ports for the Service. */ List<Integer> allocateService(DeployLogger deployLogger, AbstractService service, int wantedPort) { - List<Integer> ports = allocatePorts(deployLogger, service, wantedPort); + ports().useLogger(deployLogger); + List<Integer> ports = hostPorts.allocatePorts(service, wantedPort); assert (getService(service.getServiceName()) == null) : ("There is already a service with name '" + service.getServiceName() + "' registered on " + this + ". Most likely a programming error - all service classes must have unique names, even in different packages!"); @@ -128,111 +89,6 @@ public class HostResource implements Comparable<HostResource> { return ports; } - private List<Integer> allocatePorts(DeployLogger deployLogger, NetworkPortRequestor service, int wantedPort) { - List<Integer> ports = new ArrayList<>(); - if (service.getPortCount() < 1) - return ports; - - int serviceBasePort = BASE_PORT + allocatedPorts; - if (wantedPort > 0) { - if (service.requiresWantedPort() || canUseWantedPort(deployLogger, service, wantedPort, serviceBasePort)) - serviceBasePort = wantedPort; - } - String[] suffixes = service.getPortSuffixes(); - if (suffixes.length != service.getPortCount()) { - throw new IllegalArgumentException("service "+service+" had "+suffixes.length+" port suffixes, but port count "+service.getPortCount()+", mismatch"); - } - - reservePort(service, serviceBasePort, suffixes[0]); - ports.add(serviceBasePort); - - int remainingPortsStart = service.requiresConsecutivePorts() ? - serviceBasePort + 1: - BASE_PORT + allocatedPorts; - for (int i = 0; i < service.getPortCount() - 1; i++) { - int port = remainingPortsStart + i; - reservePort(service, port, suffixes[i+1]); - ports.add(port); - } - if (suffixes.length != service.getPortCount()) { - throw new IllegalArgumentException("service "+service+" had "+suffixes.length+" port suffixes, but port count "+service.getPortCount()+", mismatch"); - } - return ports; - } - - public void flushPortReservations() { - List<NetworkPorts.Allocation> list = new ArrayList<>(); - for (PortReservation pr : portReservations) { - String servType = pr.service.getServiceType(); - String configId = pr.service.getConfigId(); - list.add(new NetworkPorts.Allocation(pr.gotPort, servType, configId, pr.suffix)); - } - this.networkPortsList = Optional.of(new NetworkPorts(list)); - } - - private boolean canUseWantedPort(DeployLogger deployLogger, NetworkPortRequestor service, int wantedPort, int serviceBasePort) { - for (int i = 0; i < service.getPortCount(); i++) { - int port = wantedPort + i; - if (portDB.containsKey(port)) { - NetworkPortRequestor s = portDB.get(port); - deployLogger.log(Level.WARNING, service.getServiceName() +" cannot reserve port " + port + " on " + - this + ": Already reserved for " + s.getServiceName() + - ". Using default port range from " + serviceBasePort); - return false; - } - if (!service.requiresConsecutivePorts()) break; - } - return true; - } - - /** - * Reserves the desired port for the given service, or throws as exception if the port - * is not available. - * - * @param service the service that wishes to reserve the port. - * @param port the port to be reserved. - */ - void reservePort(NetworkPortRequestor service, int port, String suffix) { - if (portDB.containsKey(port)) { - portAlreadyReserved(service, port); - } else { - if (inVespasPortRange(port)) { - allocatedPorts++; - if (allocatedPorts > MAX_PORTS) { - noMoreAvailablePorts(); - } - } - portDB.put(port, service); - portReservations.add(new PortReservation(port, service, suffix)); - } - } - - private boolean inVespasPortRange(int port) { - return port >= BASE_PORT && - port < BASE_PORT + MAX_PORTS; - } - - private void portAlreadyReserved(NetworkPortRequestor service, int port) { - NetworkPortRequestor otherService = portDB.get(port); - int nextAvailablePort = nextAvailableBaseport(service.getPortCount()); - if (nextAvailablePort == 0) { - noMoreAvailablePorts(); - } - String msg = (service.getClass().equals(otherService.getClass()) && service.requiresWantedPort()) - ? "You must set port explicitly for all instances of this service type, except the first one. " - : ""; - throw new RuntimeException(service.getServiceName() + " cannot reserve port " + port + - " on " + this + ": Already reserved for " + otherService.getServiceName() + - ". " + msg + "Next available port is: " + nextAvailablePort + " ports used: " + portDB); - } - - private void noMoreAvailablePorts() { - throw new RuntimeException - ("Too many ports are reserved in Vespa's port range (" + - BASE_PORT + ".." + (BASE_PORT+MAX_PORTS) + ") on " + this + - ". Move one or more services to another host, or outside this port range."); - } - /** * Returns the service with the given "sentinel name" on this Host, * or null if the name does not match any service. diff --git a/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java b/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java index a1b030ffc61..9e0bbc395df 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/HostSystem.java @@ -131,7 +131,7 @@ public class HostSystem extends AbstractConfigProducer<Host> { hostSpec.version()); hostResource.setFlavor(hostSpec.flavor()); hostSpec.membership().ifPresent(hostResource::addClusterMembership); - hostSpec.networkPorts().ifPresent(hostResource::addNetworkPorts); + hostSpec.networkPorts().ifPresent(np -> hostResource.ports().addNetworkPorts(np)); hostname2host.put(host.getHostname(), hostResource); log.log(DEBUG, () -> "Added new host resource for " + host.getHostname() + " with flavor " + hostResource.getFlavor()); return hostResource; @@ -146,7 +146,7 @@ public class HostSystem extends AbstractConfigProducer<Host> { public void dumpPortAllocations() { for (HostResource hr : getHosts()) { - hr.flushPortReservations(); + hr.ports().flushPortReservations(); /* System.out.println("port allocations for: "+hr.getHostname()); NetworkPorts ports = hr.networkPorts().get(); @@ -193,7 +193,7 @@ public class HostSystem extends AbstractConfigProducer<Host> { Set<HostSpec> getHostSpecs() { return getHosts().stream() .map(host -> new HostSpec(host.getHostname(), Collections.emptyList(), - host.getFlavor(), host.primaryClusterMembership(), host.version(), host.networkPorts())) + host.getFlavor(), host.primaryClusterMembership(), host.version(), host.ports().networkPorts())) .collect(Collectors.toCollection(LinkedHashSet::new)); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/PortFinder.java b/config-model/src/main/java/com/yahoo/vespa/model/PortFinder.java new file mode 100644 index 00000000000..78d3a7670c7 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/PortFinder.java @@ -0,0 +1,64 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +package com.yahoo.vespa.model; + +import static com.yahoo.config.provision.NetworkPorts.Allocation; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.TreeMap; + +import java.util.logging.Logger; +import java.util.logging.Level; + +public class PortFinder { + + private static final Logger log = Logger.getLogger(PortFinder.class.getName()); + + private final Map<String, Allocation> byKeys = new HashMap<>(); + private final Map<Integer, Allocation> byPorts = new TreeMap<>(); + + /** force add the given allocation, removing any conflicting ones */ + public void use(Allocation allocation) { + String key = allocation.key(); + if (byKeys.containsKey(key)) { + if (byKeys.get(key).port == allocation.port) { + return; // already OK + } + Allocation toRemove = byKeys.remove(key); + byPorts.remove(toRemove.port); + } + if (byPorts.containsKey(allocation.port)) { + Allocation toRemove = byPorts.remove(allocation.port); + byKeys.remove(toRemove.key()); + } + byPorts.put(allocation.port, allocation); + byKeys.put(key, allocation); + } + + public int findPort(Allocation request) { + String key = request.key(); + if (byKeys.containsKey(key)) { + int port = byKeys.get(key).port; + log.log(Level.INFO, "Re-using port "+port+" for allocation "+request); + return port; + } + int port = request.port; + while (byPorts.containsKey(port)) { + ++port; + } + return port; + } + + public PortFinder(Collection<Allocation> allocations) { + for (Allocation a : allocations) { + use(a); + } + } + + public Collection<Allocation> allocations() { + return byPorts.values(); + } + +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java index 99738c13d4a..2c4fa83d02b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Slobrok.java @@ -15,7 +15,7 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc @Override public void getConfig(StateserverConfig.Builder builder) { - builder.httpport(getStatePort()); + builder.httpport(getHealthPort()); } /** @@ -46,7 +46,7 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc } public String getStartupCommand() { - return "exec $ROOT/sbin/vespa-slobrok -p " + getPort() + " -c " + getConfigId(); + return "exec $ROOT/sbin/vespa-slobrok -p " + getRpcPort() + " -c " + getConfigId(); } /** @@ -62,16 +62,17 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc } /** - * @return The port on which this slobrok should respond, as a String. + * @return The port on which this slobrok should respond */ - private String getPort() { - return String.valueOf(getRelativePort(0)); + private int getRpcPort() { + return getRelativePort(0); } /** * @return The port on which the state server should respond */ - public int getStatePort() { + @Override + public int getHealthPort() { return getRelativePort(1); } @@ -79,7 +80,7 @@ public class Slobrok extends AbstractService implements StateserverConfig.Produc * @return The connection spec to this Slobrok */ public String getConnectionSpec() { - return "tcp/" + getHostName() + ":" + getPort(); + return "tcp/" + getHostName() + ":" + getRpcPort(); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java index 44de30acbb4..af2ecc3c92e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java @@ -267,7 +267,7 @@ public abstract class Container extends AbstractService implements * @return the actual search port * TODO: Remove. Use {@link #getPortsMeta()} and check tags in conjunction with {@link #getRelativePort(int)}. */ - public int getSearchPort(){ + public int getSearchPort() { if (getHttp() != null) throw new AssertionError("getSearchPort must not be used when http section is present."); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java index 9f1673f0270..12e4aaeca0e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java @@ -327,7 +327,7 @@ public class Content extends ConfigModel { Container docprocService = new ContainerImpl(indexingCluster, containerName, index, modelContext.getDeployState().isHosted()); index++; - docprocService.setBasePort(host.nextAvailableBaseport(docprocService.getPortCount())); + docprocService.setBasePort(host.ports().nextAvailableBaseport(docprocService.getPortCount())); docprocService.setHostResource(host); docprocService.initService(modelContext.getDeployLogger()); nodes.add(docprocService); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/package-info.java b/config-model/src/main/java/com/yahoo/vespa/model/package-info.java index f88eda4e7a2..99fe0519855 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/package-info.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/package-info.java @@ -142,8 +142,8 @@ com.yahoo.config.model.producer.AbstractConfigProducer <p>Each {@link com.yahoo.vespa.model.Host Host} has an available dynamic port range running from {@link - com.yahoo.vespa.model.HostResource#BASE_PORT BASE_PORT} (currently 19100) - with {@link com.yahoo.vespa.model.HostResource#MAX_PORTS MAX_PORTS} + com.yahoo.vespa.model.HostPorts#BASE_PORT BASE_PORT} (currently 19100) + with {@link com.yahoo.vespa.model.HostPorts#MAX_PORTS MAX_PORTS} (currently 799) ports upwards. When an instance of a subclass of {@link com.yahoo.vespa.model.AbstractService AbstractService} is assigned to a host, it is given the lowest available base port in diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java b/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java index 9b4fe93d6ea..c50f772d8d0 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/Dispatch.java @@ -89,7 +89,7 @@ public class Dispatch extends AbstractService implements SearchInterface, return "exec sbin/vespa-dispatch -c $VESPA_CONFIG_ID"; } - public int getFrtPort() { return getRelativePort(0); } + private int getFrtPort() { return getRelativePort(0); } public int getDispatchPort() { return getRelativePort(1); } @Override public int getHealthPort() { return getRelativePort(2); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java index 934184d5972..e255da5b487 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/SearchNode.java @@ -202,7 +202,7 @@ public class SearchNode extends AbstractService implements return getRelativePort(FS4_PORT); } - public int getHttpPort() { + private int getHttpPort() { return getRelativePort(HEALTH_PORT); } @@ -300,7 +300,13 @@ public class SearchNode extends AbstractService implements @Override public Optional<String> getPreShutdownCommand() { - return Optional.ofNullable(flushOnShutdown ? getDefaults().underVespaHome("bin/vespa-proton-cmd ") + getRpcPort() + " prepareRestart" : null); + if (flushOnShutdown) { + int port = getRpcPort(); + String cmd = getDefaults().underVespaHome("bin/vespa-proton-cmd ") + port + " prepareRestart"; + return Optional.of(cmd); + } else { + return Optional.empty(); + } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/TransactionLogServer.java b/config-model/src/main/java/com/yahoo/vespa/model/search/TransactionLogServer.java index c42579085a5..4f854800c05 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/TransactionLogServer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/TransactionLogServer.java @@ -48,7 +48,7 @@ public class TransactionLogServer extends AbstractService { * * @return The port. */ - public int getTlsPort() { + int getTlsPort() { return getRelativePort(0); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/HostPortsTest.java b/config-model/src/test/java/com/yahoo/vespa/model/HostPortsTest.java new file mode 100644 index 00000000000..e2834291c0d --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/HostPortsTest.java @@ -0,0 +1,153 @@ +// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.model; + +import com.yahoo.config.model.producer.AbstractConfigProducer; +import com.yahoo.config.model.test.MockRoot; +import com.yahoo.config.provision.NetworkPorts; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +/** + * @author arnej + */ +public class HostPortsTest { + + @Test + public void next_available_baseport_is_BASE_PORT_when_no_ports_have_been_reserved() { + HostPorts host = new HostPorts("myhostname"); + assertThat(host.nextAvailableBaseport(1), is(HostPorts.BASE_PORT)); + } + + @Test + public void next_available_baseport_is_BASE_PORT_plus_one_when_one_port_has_been_reserved() { + HostPorts host = new HostPorts("myhostname"); + MockRoot root = new MockRoot(); + host.reservePort(new TestService(root, 1), HostPorts.BASE_PORT, "foo"); + assertThat(host.nextAvailableBaseport(1), is(HostPorts.BASE_PORT + 1)); + } + + @Test + public void no_available_baseport_when_service_requires_more_consecutive_ports_than_available() { + HostPorts host = new HostPorts("myhostname"); + MockRoot root = new MockRoot(); + + for (int p = HostPorts.BASE_PORT; p < HostPorts.BASE_PORT + HostPorts.MAX_PORTS; p += 2) { + host.reservePort(new TestService(root, 1), p, "foo"); + } + assertThat(host.nextAvailableBaseport(2), is(0)); + + try { + host.reservePort(new TestService(root, 2), HostPorts.BASE_PORT, "bar"); + } catch (RuntimeException e) { + assertThat(e.getMessage(), containsString("Too many ports are reserved")); + } + } + + @Test + public void port_above_vespas_port_range_can_be_reserved() { + HostPorts host = new HostPorts("myhostname"); + MockRoot root = new MockRoot(); + host.allocatePorts(new TestService(root, 1), HostPorts.BASE_PORT + HostPorts.MAX_PORTS + 1); + } + + @Test(expected = RuntimeException.class) + public void allocating_same_port_throws_exception() { + HostPorts host = new HostPorts("myhostname"); + MockRoot root = new MockRoot(); + TestService service1 = new TestService(root, 1); + TestService service2 = new TestService(root, 1); + + host.allocatePorts(service1, HostPorts.BASE_PORT); + host.allocatePorts(service2, HostPorts.BASE_PORT); + } + + @Test(expected = RuntimeException.class) + public void allocating_overlapping_ports_throws_exception() { + HostPorts host = new HostPorts("myhostname"); + MockRoot root = new MockRoot(); + TestService service2 = new TestService(root, 2); + TestService service1 = new TestService(root, 1); + + host.allocatePorts(service2, HostPorts.BASE_PORT); + host.allocatePorts(service1, HostPorts.BASE_PORT + 1); + } + + NetworkPorts emulOldPorts() { + List<NetworkPorts.Allocation> list = new ArrayList<>(); + list.add(new NetworkPorts.Allocation(8080, "qrs", "foo", "http")); + list.add(new NetworkPorts.Allocation(19101, "slobrok", "slobrok.0", "http")); + return new NetworkPorts(list); + } + + @Test + public void use_old_port_when_available() { + HostPorts host = new HostPorts("myhostname"); + host.addNetworkPorts(emulOldPorts()); + + MockRoot root = new MockRoot(); + Service service = new MockSlobrok(root, 0); + assertThat(service.getConfigId(), is("slobrok.0")); + + // check that matching service get port from saved allocations + List<Integer> ports = host.allocatePorts(service, 0); + assertThat(ports.size(), is(1)); + assertThat(ports.get(0), is(19101)); + + // check that new service get next free port + ports = host.allocatePorts(new MockSlobrok(root, 1), 0); + assertThat(ports.get(0), is(19100)); + + // check that new service get next free port, skipping the allocated one + ports = host.allocatePorts(new MockSlobrok(root, 2), 0); + assertThat(ports.get(0), is(19102)); + } + + private static class MockSlobrok extends AbstractService { + MockSlobrok(AbstractConfigProducer parent, int number) { + super(parent, "slobrok."+number); + } + @Override public int getPortCount() { return 1; } + @Override public String[] getPortSuffixes() { return new String[]{"http"}; } + @Override public String getServiceType() { return "slobrok"; } + } + + private static int counter = 0; + int getCounter() { return ++counter; } + + private class TestService extends AbstractService { + private final int portCount; + + TestService(AbstractConfigProducer parent, int portCount) { + super(parent, "testService" + getCounter()); + this.portCount = portCount; + } + + @Override + public boolean requiresWantedPort() { + return true; + } + + @Override + public int getPortCount() { return portCount; } + + @Override + public String[] getPortSuffixes() { + String[] suffixes = new String[portCount]; + for (int i = 0; i < portCount; i++) { + suffixes[i] = "generic." + i; + } + return suffixes; + } + } +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java b/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java index d16bbe72a95..2d116dde472 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/HostResourceTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.model; import com.yahoo.component.Version; +import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.model.test.MockRoot; import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterSpec; @@ -27,41 +28,9 @@ import static org.junit.Assert.assertTrue; public class HostResourceTest { @Test - public void next_available_baseport_is_BASE_PORT_when_no_ports_have_been_reserved() { - MockRoot root = new MockRoot(); - HostResource host = mockHostResource(root); - assertThat(host.nextAvailableBaseport(1), is(HostResource.BASE_PORT)); - } - - @Test - public void next_available_baseport_is_BASE_PORT_plus_one_when_one_port_has_been_reserved() { - MockRoot root = new MockRoot(); - HostResource host = mockHostResource(root); - host.reservePort(new TestService(1), HostResource.BASE_PORT, "foo"); - assertThat(host.nextAvailableBaseport(1), is(HostResource.BASE_PORT + 1)); - } - - @Test - public void no_available_baseport_when_service_requires_more_consecutive_ports_than_available() { - MockRoot root = new MockRoot(); - HostResource host = mockHostResource(root); - - for (int p = HostResource.BASE_PORT; p < HostResource.BASE_PORT + HostResource.MAX_PORTS; p += 2) { - host.reservePort(new TestService(1), p, "foo"); - } - assertThat(host.nextAvailableBaseport(2), is(0)); - - try { - host.reservePort(new TestService(2), HostResource.BASE_PORT, "bar"); - } catch (RuntimeException e) { - assertThat(e.getMessage(), containsString("Too many ports are reserved")); - } - } - - @Test public void require_exception_when_no_matching_hostalias() { - TestService service = new TestService(1); MockRoot root = new MockRoot(); + TestService service = new TestService(root, 1); try { service.initService(root.deployLogger()); @@ -72,36 +41,6 @@ public class HostResourceTest { } @Test - public void port_above_vespas_port_range_can_be_reserved() { - MockRoot root = new MockRoot(); - HostResource host = mockHostResource(root); - - host.allocateService(root.deployLogger(), new TestService(1), HostResource.BASE_PORT + HostResource.MAX_PORTS + 1); - } - - @Test(expected = RuntimeException.class) - public void allocating_same_port_throws_exception() { - MockRoot root = new MockRoot(); - HostResource host = mockHostResource(root); - TestService service1 = new TestService(1); - TestService service2 = new TestService(1); - - host.allocateService(root.deployLogger(), service1, HostResource.BASE_PORT); - host.allocateService(root.deployLogger(), service2, HostResource.BASE_PORT); - } - - @Test(expected = RuntimeException.class) - public void allocating_overlapping_ports_throws_exception() { - MockRoot root = new MockRoot(); - HostResource host = mockHostResource(root); - TestService service2 = new TestService(2); - TestService service1 = new TestService(1); - - host.allocateService(root.deployLogger(), service2, HostResource.BASE_PORT); - host.allocateService(root.deployLogger(), service1, HostResource.BASE_PORT + 1); - } - - @Test public void no_clusters_yields_no_primary_cluster_membership() { HostResource host = hostResourceWithMemberships(); assertTrue(host.clusterMemberships().isEmpty()); @@ -166,11 +105,14 @@ public class HostResourceTest { return host; } + private static int counter = 0; + int getCounter() { return ++counter; } + private class TestService extends AbstractService { private final int portCount; - TestService(int portCount) { - super("testService"); + TestService(AbstractConfigProducer parent, int portCount) { + super(parent, "testService" + getCounter()); this.portCount = portCount; } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index 5ef0ba37c22..8786f2cbd5e 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -279,7 +279,8 @@ public class ContainerClusterTest { private static ContainerCluster newContainerCluster() { DeployState deployState = DeployState.createTestState(); - ContainerCluster cluster = new ContainerCluster(null, "subId", "name", deployState); + MockRoot root = new MockRoot("foo", deployState); + ContainerCluster cluster = new ContainerCluster(root, "subId", "name", deployState); addContainer(deployState.getDeployLogger(), cluster, "c1", "host-c1"); addContainer(deployState.getDeployLogger(), cluster, "c2", "host-c2"); return cluster; diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java index e0dcd3fcc47..90fb5e17078 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java @@ -11,7 +11,7 @@ import com.yahoo.container.core.ChainsConfig; import com.yahoo.container.jdisc.ContainerMbusConfig; import com.yahoo.document.config.DocumentmanagerConfig; import com.yahoo.search.config.QrStartConfig; -import com.yahoo.vespa.model.HostResource; +import com.yahoo.vespa.model.HostPorts; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.ContainerModel; @@ -120,7 +120,7 @@ public class DocprocBuilderTest extends DomBuilderTest { @Test public void testContainerMbusConfig() { assertThat(containerMbusConfig.enabled(), is(true)); - assertTrue(containerMbusConfig.port() >= HostResource.BASE_PORT); + assertTrue(containerMbusConfig.port() >= HostPorts.BASE_PORT); assertThat(containerMbusConfig.maxpendingcount(), is(300)); assertThat(containerMbusConfig.maxpendingsize(), is(100)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index 3d94dedf651..90cd80604ca 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -7,6 +7,7 @@ import com.google.common.io.Files; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; import com.yahoo.component.Vtag; +import com.yahoo.config.application.api.ApplicationMetaData; import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; @@ -25,6 +26,7 @@ import com.yahoo.vespa.config.server.http.SessionHandlerTest; import com.yahoo.vespa.config.server.http.v2.PrepareResult; import com.yahoo.vespa.config.server.session.LocalSession; import com.yahoo.vespa.config.server.session.PrepareParams; +import com.yahoo.vespa.config.server.session.SilentDeployLogger; import com.yahoo.vespa.config.server.tenant.Tenant; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; @@ -121,6 +123,24 @@ public class ApplicationRepositoryTest { } @Test + public void createFromActiveSession() { + PrepareResult result = deployApp(testApp); + long sessionId = applicationRepository.createSessionFromExisting(applicationId(), + new SilentDeployLogger(), + false, + timeoutBudget); + long originalSessionId = result.sessionId(); + ApplicationMetaData originalApplicationMetaData = getApplicationMetaData(applicationId(), originalSessionId); + ApplicationMetaData applicationMetaData = getApplicationMetaData(applicationId(), sessionId); + + assertNotEquals(sessionId, originalSessionId); + assertEquals(applicationMetaData.getApplicationName(), originalApplicationMetaData.getApplicationName()); + assertEquals(applicationMetaData.getPreviousActiveGeneration(), originalApplicationMetaData.getGeneration().longValue()); + assertNotEquals(applicationMetaData.getGeneration(), originalApplicationMetaData.getGeneration()); + assertEquals(applicationMetaData.getDeployedByUser(), originalApplicationMetaData.getDeployedByUser()); + } + + @Test public void testSuspension() { deployApp(testApp); assertFalse(applicationRepository.isSuspended(applicationId())); @@ -339,4 +359,8 @@ public class ApplicationRepositoryTest { return ApplicationId.from(tenantName, ApplicationName.from("testapp"), InstanceName.defaultName()); } + private ApplicationMetaData getApplicationMetaData(ApplicationId applicationId, long sessionId) { + Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); + return applicationRepository.getMetadataFromSession(tenant, sessionId); + } } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java deleted file mode 100644 index 6c8be2ac2f3..00000000000 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionFactoryTest.java +++ /dev/null @@ -1,96 +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.config.server.session; - -import com.google.common.io.Files; -import com.yahoo.config.application.api.ApplicationFile; -import com.yahoo.config.model.application.provider.BaseDeployLogger; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.InstanceName; -import com.yahoo.config.provision.TenantName; -import com.yahoo.io.IOUtils; -import com.yahoo.path.Path; -import com.yahoo.vespa.config.server.*; -import com.yahoo.vespa.config.server.http.CompressedApplicationInputStream; -import com.yahoo.vespa.config.server.http.CompressedApplicationInputStreamTest; - -import com.yahoo.vespa.config.server.http.v2.ApplicationApiHandler; -import com.yahoo.vespa.config.server.tenant.TenantRepository; -import com.yahoo.vespa.curator.mock.MockCurator; -import org.json.JSONException; -import org.json.JSONObject; -import org.junit.Before; -import org.junit.Test; - -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; - -import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -/** - * @author Ulf Lilleengen - */ -public class SessionFactoryTest { - private SessionFactory factory; - - @Before - public void setup_test() { - TenantRepository tenantRepository = new TenantRepository(new TestComponentRegistry.Builder().curator(new MockCurator()).build()); - factory = tenantRepository.defaultTenant().getSessionFactory(); - } - - @Test - public void require_that_session_can_be_created() throws IOException { - LocalSession session = getLocalSession(); - assertNotNull(session); - assertThat(session.getSessionId(), is(2l)); - assertTrue(session.getCreateTime() > 0); - } - - @Test - public void require_that_application_name_is_set_in_application_package() throws IOException, JSONException { - LocalSession session = getLocalSession("book"); - assertNotNull(session); - ApplicationFile meta = session.getApplicationFile(Path.createRoot().append(".applicationMetaData"), LocalSession.Mode.READ); - assertTrue(meta.exists()); - JSONObject json = new JSONObject(IOUtils.readAll(meta.createReader())); - assertThat(json.getJSONObject("application").getString("name"), is("book")); - } - - @Test - public void require_that_session_can_be_created_from_existing() throws IOException { - LocalSession session = getLocalSession(); - assertNotNull(session); - assertThat(session.getSessionId(), is(2L)); - LocalSession session2 = factory.createSessionFromExisting(session, - new BaseDeployLogger(), - false, - TimeoutBudgetTest.day()); - assertNotNull(session2); - assertThat(session2.getSessionId(), is(3L)); - } - - @Test(expected = RuntimeException.class) - public void require_that_invalid_app_dir_is_handled() { - createSession(new File("doesnotpointtoavaliddir"), "music"); - } - - private LocalSession getLocalSession() throws IOException { - return getLocalSession("music"); - } - - private LocalSession getLocalSession(String appName) throws IOException { - CompressedApplicationInputStream app = CompressedApplicationInputStream.createFromCompressedStream( - new FileInputStream(CompressedApplicationInputStreamTest.createTarFile()), ApplicationApiHandler.APPLICATION_X_GZIP); - return createSession(app.decompress(Files.createTempDir()), appName); - } - - private LocalSession createSession(File applicationDirectory, String appName) { - ApplicationId applicationId = ApplicationId.from(TenantName.defaultName(), ApplicationName.from(appName), InstanceName.defaultName()); - return factory.createSession(applicationDirectory, applicationId, TimeoutBudgetTest.day()); - } -} diff --git a/controller-server/pom.xml b/controller-server/pom.xml index 034f96b9445..c4cb66de3ec 100644 --- a/controller-server/pom.xml +++ b/controller-server/pom.xml @@ -177,15 +177,6 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> - <configuration> - <compilerArgs> - <arg>-Xlint:all</arg> - <arg>-Xlint:-serial</arg> - <arg>-Xlint:-deprecation</arg> - <arg>-Xlint:-try</arg> - <arg>-Werror</arg> - </compilerArgs> - </configuration> </plugin> </plugins> </build> diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java index 2f10dce0e0a..a75d0afbad0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java @@ -260,6 +260,7 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { return true; } + @SuppressWarnings("deprecation") private static CloseableHttpClient createHttpClient(RequestConfig config, ServiceIdentityProvider sslContextProvider, ZoneRegistry zoneRegistry, @@ -277,6 +278,7 @@ public class ConfigServerRestExecutorImpl implements ConfigServerRestExecutor { .build(); } + @SuppressWarnings("deprecation") private static class AthenzIdentityVerifierAdapter implements X509HostnameVerifier { private final AthenzIdentityVerifier verifier; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java index fe2394d872d..615fb017363 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java @@ -144,7 +144,7 @@ public final class ControllerTester { // Make root logger use time from manual clock configureDefaultLogHandler(handler -> handler.setFilter( record -> { - record.setMillis(clock.millis()); + record.setInstant(clock.instant()); return true; })); } diff --git a/documentapi/src/tests/loadtypes/CMakeLists.txt b/documentapi/src/tests/loadtypes/CMakeLists.txt index 39ee6816c5e..495c05c0e43 100644 --- a/documentapi/src/tests/loadtypes/CMakeLists.txt +++ b/documentapi/src/tests/loadtypes/CMakeLists.txt @@ -5,7 +5,6 @@ vespa_add_executable(documentapi_loadtype_test_app TEST DEPENDS documentapi vdstestlib - gmock gtest ) vespa_add_test(NAME documentapi_loadtype_test_app COMMAND documentapi_loadtype_test_app) diff --git a/documentapi/src/tests/loadtypes/loadtypetest.cpp b/documentapi/src/tests/loadtypes/loadtypetest.cpp index 20efb281300..178b9f9227b 100644 --- a/documentapi/src/tests/loadtypes/loadtypetest.cpp +++ b/documentapi/src/tests/loadtypes/loadtypetest.cpp @@ -4,9 +4,6 @@ #include <vespa/config/config.h> #include <vespa/config/common/exceptions.h> #include <vespa/vespalib/gtest/gtest.h> -#include <gmock/gmock.h> - -using namespace ::testing; namespace documentapi { @@ -17,7 +14,7 @@ assertConfigFailure(const vespalib::string &configId, const vespalib::string &ex LoadTypeSet createdFromConfigId(configId); FAIL() << "Config was expected to fail with error: " << expError; } catch (config::InvalidConfigException &e) { - EXPECT_THAT(e.getMessage(), HasSubstr(expError)); + EXPECT_TRUE(e.getMessage().find(expError) != std::string::npos); } } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index ac51e2d7ebc..7e093f2925c 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -72,7 +72,7 @@ public class Flags { HOSTNAME, APPLICATION_ID); public static final UnboundStringFlag TLS_INSECURE_MIXED_MODE = defineStringFlag( - "tls-insecure-mixed-mode", "plaintext_client_mixed_server", + "tls-insecure-mixed-mode", "tls_client_mixed_server", "TLS insecure mixed mode. Allowed values: ['plaintext_client_mixed_server', 'tls_client_mixed_server', 'tls_client_tls_server']", "Takes effect on restart of Docker container", NODE_TYPE, APPLICATION_ID, HOSTNAME); diff --git a/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java b/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java index 3338b631030..b3dbd8712c4 100644 --- a/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java +++ b/jdisc_core/src/test/java/com/yahoo/jdisc/core/OsgiLogHandlerTestCase.java @@ -5,6 +5,7 @@ import org.junit.Test; import org.osgi.framework.ServiceReference; import org.osgi.service.log.LogService; +import java.time.Instant; import java.util.Arrays; import java.util.Enumeration; import java.util.ResourceBundle; @@ -70,15 +71,13 @@ public class OsgiLogHandlerTestCase { } @Test - // TODO: Remove deprecation annotation and replace calls to LogRecord.setMillis() when we no longer have to support Java 8 - @SuppressWarnings("deprecation") public void requireThatJdk14PropertiesAreAvailableThroughServiceReference() { MyLogService logService = new MyLogService(); Logger log = newLogger(logService); LogRecord record = new LogRecord(Level.INFO, "message"); record.setLoggerName("loggerName"); - record.setMillis(69); + record.setInstant(Instant.ofEpochMilli(69)); Object[] parameters = new Object[0]; record.setParameters(parameters); ResourceBundle resouceBundle = new MyResourceBundle(); diff --git a/parent/pom.xml b/parent/pom.xml index 555da7c45af..8591bf40661 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -252,7 +252,7 @@ <plugin> <groupId>com.helger.maven</groupId> <artifactId>ph-javacc-maven-plugin</artifactId> - <version>4.1.1</version> + <version>4.1.2</version> <executions> <execution> <phase>generate-sources</phase> diff --git a/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp b/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp index f6da0c75d62..062586b1d0f 100644 --- a/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp +++ b/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp @@ -69,8 +69,11 @@ struct MyDocumentDBReference : public MockDocumentDBReference { } virtual std::shared_ptr<search::attribute::ReadableAttributeVector> getAttribute(vespalib::stringref name) override { auto itr = attributes.find(name); - ASSERT_TRUE(itr != attributes.end()); - return itr->second; + if (itr != attributes.end()) { + return itr->second; + } else { + return std::shared_ptr<search::attribute::ReadableAttributeVector>(); + } } void addIntAttribute(vespalib::stringref name) { attributes[name] = AttributeFactory::createAttribute(name, Config(BasicType::INT32)); @@ -82,6 +85,9 @@ struct MyDocumentDBReference : public MockDocumentDBReference { MockGidToLidChangeHandler &getGidToLidChangeHandler() { return *_gidToLidChangeHandler; } + void removeAttribute(vespalib::stringref name) { + attributes.erase(name); + } }; struct MyReferenceRegistry : public IDocumentDBReferenceRegistry { @@ -306,6 +312,15 @@ TEST_F("require that imported attributes are instantiated with search cache if v f.assertImportedAttribute("imported_b", "other_ref", "target_b", true, repo->get("imported_b")); } +TEST_F("require that missing target attribute prevents creation of imported attribute", Fixture) +{ + f.parentReference->removeAttribute("target_a"); + auto repo = f.resolve(); + EXPECT_EQUAL(1u, repo->size()); + EXPECT_FALSE(repo->get("imported_a")); + EXPECT_TRUE(repo->get("imported_b")); +} + TEST_F("require that listeners are added", Fixture) { f.resolve(); diff --git a/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp b/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp index 95da27cf3bd..0d83eea261a 100644 --- a/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp +++ b/searchcore/src/vespa/searchcore/proton/reference/document_db_reference_resolver.cpp @@ -146,8 +146,10 @@ DocumentDBReferenceResolver::createImportedAttributesRepo(const IAttributeManage auto targetDocumentDB = getTargetDocumentDB(refAttr->getName()); auto targetAttr = targetDocumentDB->getAttribute(attr.targetfield); auto targetDocumentMetaStore = targetDocumentDB->getDocumentMetaStore(); - auto importedAttr = ImportedAttributeVectorFactory::create(attr.name, refAttr, documentMetaStore, targetAttr, targetDocumentMetaStore, useSearchCache); - result->add(importedAttr->getName(), importedAttr); + if (targetAttr) { + auto importedAttr = ImportedAttributeVectorFactory::create(attr.name, refAttr, documentMetaStore, targetAttr, targetDocumentMetaStore, useSearchCache); + result->add(importedAttr->getName(), importedAttr); + } } } return result; diff --git a/security-utils/src/main/java/com/yahoo/security/tls/ReloadingTlsContext.java b/security-utils/src/main/java/com/yahoo/security/tls/ReloadingTlsContext.java index debf14a27f8..16f66f91da6 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/ReloadingTlsContext.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/ReloadingTlsContext.java @@ -15,6 +15,7 @@ import javax.net.ssl.SSLParameters; import javax.net.ssl.X509ExtendedTrustManager; import java.io.IOException; import java.io.UncheckedIOException; +import java.lang.ref.WeakReference; import java.nio.file.Files; import java.nio.file.Path; import java.security.KeyStore; @@ -22,6 +23,7 @@ import java.time.Duration; import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,37 +39,31 @@ public class ReloadingTlsContext implements TlsContext { private static final Logger log = Logger.getLogger(ReloadingTlsContext.class.getName()); - private final Path tlsOptionsConfigFile; private final TlsContext tlsContext; - private final MutableX509TrustManager trustManager = new MutableX509TrustManager(); - private final MutableX509KeyManager keyManager = new MutableX509KeyManager(); - private final ScheduledExecutorService scheduler = - Executors.newSingleThreadScheduledExecutor(runnable -> { - Thread thread = new Thread(runnable, "tls-context-reloader"); - thread.setDaemon(true); - return thread; - }); + private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(new ReloaderThreadFactory()); public ReloadingTlsContext(Path tlsOptionsConfigFile, AuthorizationMode mode) { - this.tlsOptionsConfigFile = tlsOptionsConfigFile; TransportSecurityOptions options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile); - reloadCryptoMaterial(options, trustManager, keyManager); + MutableX509TrustManager trustManager = new MutableX509TrustManager(); + MutableX509KeyManager keyManager = new MutableX509KeyManager(); + reloadTrustManager(options, trustManager); + reloadKeyManager(options, keyManager); this.tlsContext = createDefaultTlsContext(options, mode, trustManager, keyManager); - this.scheduler.scheduleAtFixedRate(new CryptoMaterialReloader(), + this.scheduler.scheduleAtFixedRate(new CryptoMaterialReloader(tlsOptionsConfigFile, scheduler, trustManager, keyManager), UPDATE_PERIOD.getSeconds()/*initial delay*/, UPDATE_PERIOD.getSeconds(), TimeUnit.SECONDS); } - private static void reloadCryptoMaterial(TransportSecurityOptions options, - MutableX509TrustManager trustManager, - MutableX509KeyManager keyManager) { + private static void reloadTrustManager(TransportSecurityOptions options, MutableX509TrustManager trustManager) { if (options.getCaCertificatesFile().isPresent()) { trustManager.updateTruststore(loadTruststore(options.getCaCertificatesFile().get())); } else { trustManager.useDefaultTruststore(); } + } + private static void reloadKeyManager(TransportSecurityOptions options, MutableX509KeyManager keyManager) { if (options.getPrivateKeyFile().isPresent() && options.getCertificatesFile().isPresent()) { keyManager.updateKeystore(loadKeystore(options.getPrivateKeyFile().get(), options.getCertificatesFile().get()), new char[0]); } else { @@ -122,21 +118,63 @@ public class ReloadingTlsContext implements TlsContext { public void close() { try { scheduler.shutdownNow(); - scheduler.awaitTermination(5, TimeUnit.SECONDS); + if (!scheduler.awaitTermination(10, TimeUnit.SECONDS)) { + throw new RuntimeException("Unable to shutdown executor before timeout"); + } } catch (InterruptedException e) { throw new RuntimeException(e); } } - private class CryptoMaterialReloader implements Runnable { + // Note: no reference to outer class (directly or indirectly) to ensure trust/key managers are eventually GCed once + // there are no more use of the outer class and the underlying SSLContext + private static class CryptoMaterialReloader implements Runnable { + + final Path tlsOptionsConfigFile; + final ScheduledExecutorService scheduler; + final WeakReference<MutableX509TrustManager> trustManager; + final WeakReference<MutableX509KeyManager> keyManager; + + CryptoMaterialReloader(Path tlsOptionsConfigFile, + ScheduledExecutorService scheduler, + MutableX509TrustManager trustManager, + MutableX509KeyManager keyManager) { + this.tlsOptionsConfigFile = tlsOptionsConfigFile; + this.scheduler = scheduler; + this.trustManager = new WeakReference<>(trustManager); + this.keyManager = new WeakReference<>(keyManager); + } + @Override public void run() { try { - reloadCryptoMaterial(TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile), trustManager, keyManager); + MutableX509TrustManager trustManager = this.trustManager.get(); + MutableX509KeyManager keyManager = this.keyManager.get(); + if (trustManager == null && keyManager == null) { + scheduler.shutdown(); + return; + } + TransportSecurityOptions options = TransportSecurityOptions.fromJsonFile(tlsOptionsConfigFile); + if (trustManager != null) { + reloadTrustManager(options, trustManager); + } + if (keyManager != null) { + reloadKeyManager(options, keyManager); + } } catch (Throwable t) { log.log(Level.SEVERE, String.format("Failed to reload crypto material (path='%s'): %s", tlsOptionsConfigFile, t.getMessage()), t); } } } + // Static class to ensure no reference to outer class is contained + private static class ReloaderThreadFactory implements ThreadFactory { + @Override + public Thread newThread(Runnable r) { + Thread thread = new Thread(r, "tls-context-reloader"); + thread.setDaemon(true); + return thread; + } + } + } diff --git a/security-utils/src/test/java/com/yahoo/security/tls/ReloadingTlsContextTest.java b/security-utils/src/test/java/com/yahoo/security/tls/ReloadingTlsContextTest.java new file mode 100644 index 00000000000..f991f86fdce --- /dev/null +++ b/security-utils/src/test/java/com/yahoo/security/tls/ReloadingTlsContextTest.java @@ -0,0 +1,70 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.security.tls; + +import com.yahoo.security.KeyUtils; +import com.yahoo.security.X509CertificateBuilder; +import com.yahoo.security.X509CertificateUtils; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import javax.net.ssl.SSLEngine; +import javax.security.auth.x500.X500Principal; +import java.io.IOException; +import java.math.BigInteger; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.KeyPair; +import java.security.cert.X509Certificate; + +import static com.yahoo.security.KeyAlgorithm.EC; +import static com.yahoo.security.SignatureAlgorithm.SHA256_WITH_ECDSA; +import static java.time.Instant.EPOCH; +import static java.time.temporal.ChronoUnit.DAYS; +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author bjorncs + */ +public class ReloadingTlsContextTest { + + @Rule + public TemporaryFolder tempDirectory = new TemporaryFolder(); + + @Test + public void can_create_sslcontext_from_credentials() throws IOException, InterruptedException { + KeyPair keyPair = KeyUtils.generateKeypair(EC); + Path privateKeyFile = tempDirectory.newFile().toPath(); + Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate())); + + X509Certificate certificate = X509CertificateBuilder + .fromKeypair(keyPair, new X500Principal("CN=dummy"), EPOCH, EPOCH.plus(1, DAYS), SHA256_WITH_ECDSA, BigInteger.ONE) + .build(); + Path certificateChainFile = tempDirectory.newFile().toPath(); + String certificatePem = X509CertificateUtils.toPem(certificate); + Files.writeString(certificateChainFile, certificatePem); + + Path caCertificatesFile = tempDirectory.newFile().toPath(); + Files.writeString(caCertificatesFile, certificatePem); + + TransportSecurityOptions options = new TransportSecurityOptions.Builder() + .withCertificates(certificateChainFile, privateKeyFile) + .withCaCertificates(caCertificatesFile) + .build(); + + Path optionsFile = tempDirectory.newFile().toPath(); + options.toJsonFile(optionsFile); + + try (TlsContext tlsContext = new ReloadingTlsContext(optionsFile, AuthorizationMode.ENFORCE)) { + SSLEngine sslEngine = tlsContext.createSslEngine(); + assertThat(sslEngine).isNotNull(); + String[] enabledCiphers = sslEngine.getEnabledCipherSuites(); + assertThat(enabledCiphers).isNotEmpty(); + assertThat(enabledCiphers).isSubsetOf(DefaultTlsContext.ALLOWED_CIPHER_SUITES.toArray(new String[0])); + + String[] enabledProtocols = sslEngine.getEnabledProtocols(); + assertThat(enabledProtocols).contains("TLSv1.2"); + } + } + +}
\ No newline at end of file diff --git a/vespalog/src/test/java/com/yahoo/log/LogSetupTestCase.java b/vespalog/src/test/java/com/yahoo/log/LogSetupTestCase.java index d476b111e4f..d0e2baf47c5 100644 --- a/vespalog/src/test/java/com/yahoo/log/LogSetupTestCase.java +++ b/vespalog/src/test/java/com/yahoo/log/LogSetupTestCase.java @@ -2,6 +2,7 @@ package com.yahoo.log; import java.io.IOException; +import java.time.Instant; import java.util.logging.LogRecord; import java.util.logging.Logger; import java.util.logging.Level; @@ -20,8 +21,6 @@ import static org.hamcrest.CoreMatchers.is; * * @author Bjorn Borud */ -// TODO: Remove annotation and replace setMillis with setInstant when we don't support Java 8 anymore. -@SuppressWarnings("deprecation") public class LogSetupTestCase { // For testing zookeeper log records protected static LogRecord zookeeperLogRecord; @@ -44,11 +43,11 @@ public class LogSetupTestCase { zookeeperLogRecord = new LogRecord(Level.WARNING, "zookeeper log record"); zookeeperLogRecord.setLoggerName("org.apache.zookeeper.server.NIOServerCnxn"); - zookeeperLogRecord.setMillis(1107011348029L); + zookeeperLogRecord.setInstant(Instant.ofEpochMilli(1107011348029L)); curatorLogRecord = new LogRecord(Level.WARNING, "curator log record"); curatorLogRecord.setLoggerName("org.apache.curator.utils.DefaultTracerDriver"); - curatorLogRecord.setMillis(1107011348029L); + curatorLogRecord.setInstant(Instant.ofEpochMilli(1107011348029L)); hostname = Util.getHostName(); pid = Util.getPID(); @@ -62,15 +61,15 @@ public class LogSetupTestCase { zookeeperLogRecordError = new LogRecord(Level.SEVERE, "zookeeper error"); zookeeperLogRecordError.setLoggerName("org.apache.zookeeper.server.NIOServerCnxn"); - zookeeperLogRecordError.setMillis(1107011348029L); + zookeeperLogRecordError.setInstant(Instant.ofEpochMilli(1107011348029L)); curatorLogRecordError = new LogRecord(Level.SEVERE, "curator log record"); curatorLogRecordError.setLoggerName("org.apache.curator.utils.DefaultTracerDriver"); - curatorLogRecordError.setMillis(1107011348029L); + curatorLogRecordError.setInstant(Instant.ofEpochMilli(1107011348029L)); notzookeeperLogRecord = new LogRecord(Level.WARNING, "not zookeeper log record"); notzookeeperLogRecord.setLoggerName("org.apache.foo.Bar"); - notzookeeperLogRecord.setMillis(1107011348029L); + notzookeeperLogRecord.setInstant(Instant.ofEpochMilli(1107011348029L)); } @Test diff --git a/vespalog/src/test/java/com/yahoo/log/VespaFormatterTestCase.java b/vespalog/src/test/java/com/yahoo/log/VespaFormatterTestCase.java index 9da71b2ad2e..cbfbd609f61 100644 --- a/vespalog/src/test/java/com/yahoo/log/VespaFormatterTestCase.java +++ b/vespalog/src/test/java/com/yahoo/log/VespaFormatterTestCase.java @@ -1,21 +1,22 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.log; -import java.util.logging.Level; -import java.util.logging.LogRecord; - -import org.junit.Test; import org.junit.Before; import org.junit.Ignore; +import org.junit.Test; -import static org.junit.Assert.*; -import static org.hamcrest.CoreMatchers.is; +import java.time.Instant; +import java.util.logging.Level; +import java.util.logging.LogRecord; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author Bjorn Borud */ -// TODO: Remove annotation and replace setMillis with setInstant when we don't support Java 8 anymore. -@SuppressWarnings("deprecation") public class VespaFormatterTestCase { private String hostname; @@ -36,7 +37,7 @@ public class VespaFormatterTestCase { pid = Util.getPID(); testRecord1 = new LogRecord(Level.INFO, "this is a test"); - testRecord1.setMillis(1098709021843L); + testRecord1.setInstant(Instant.ofEpochMilli(1098709021843L)); testRecord1.setThreadID(123); expected1 = "1098709021.843\t" @@ -59,7 +60,7 @@ public class VespaFormatterTestCase { testRecord2 = new LogRecord(Level.INFO, "this is a test"); - testRecord2.setMillis(1098709021843L); + testRecord2.setInstant(Instant.ofEpochMilli(1098709021843L)); testRecord2.setThreadID(123); testRecord2.setLoggerName("org.foo"); @@ -100,12 +101,12 @@ public class VespaFormatterTestCase { /** * test that {0} etc is replaced properly */ - @Test + @Test public void testTextFormatting () { VespaFormatter formatter = new VespaFormatter(serviceName, app); LogRecord testRecord = new LogRecord(Level.INFO, "this {1} is {0} test"); - testRecord.setMillis(1098709021843L); + testRecord.setInstant(Instant.ofEpochMilli(1098709021843L)); testRecord.setThreadID(123); testRecord.setLoggerName("org.foo"); Object[] params = { "a small", "message" }; diff --git a/vespalog/src/test/java/com/yahoo/log/VespaLogHandlerTestCase.java b/vespalog/src/test/java/com/yahoo/log/VespaLogHandlerTestCase.java index 5cbd130c05a..d18bce2f4ec 100644 --- a/vespalog/src/test/java/com/yahoo/log/VespaLogHandlerTestCase.java +++ b/vespalog/src/test/java/com/yahoo/log/VespaLogHandlerTestCase.java @@ -6,6 +6,7 @@ import org.junit.Before; import org.junit.Test; import java.io.*; +import java.time.Instant; import java.util.LinkedList; import java.util.List; import java.util.concurrent.BrokenBarrierException; @@ -14,14 +15,13 @@ import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; +import static java.time.Instant.ofEpochMilli; import static org.hamcrest.core.Is.is; import static org.junit.Assert.*; /** * @author Bjorn Borud */ -// TODO: Remove annotation and replace setMillis with setInstant when we don't support Java 8 anymore. -@SuppressWarnings("deprecation") public class VespaLogHandlerTestCase { protected static String hostname; protected static String pid; @@ -43,7 +43,7 @@ public class VespaLogHandlerTestCase { pid = Util.getPID(); record1 = new LogRecord(Level.INFO, "This is a test"); - record1.setMillis(1100011348029L); + record1.setInstant(ofEpochMilli(1100011348029L)); record1String = "1100011348.029\t" + hostname + "\t" @@ -53,7 +53,7 @@ public class VespaLogHandlerTestCase { + "\tmy-test-config-id\tTST\tinfo\tThis is a test"; record2 = new LogRecord(Level.FINE, "This is a test too"); - record2.setMillis(1100021348029L); + record2.setInstant(ofEpochMilli(1100021348029L)); record2.setLoggerName("com.yahoo.log.test"); record2String = "1100021348.029\t" + hostname @@ -63,7 +63,7 @@ public class VespaLogHandlerTestCase { record3 = new LogRecord(Level.WARNING, "another test"); record3.setLoggerName("com.yahoo.log.test"); - record3.setMillis(1107011348029L); + record3.setInstant(ofEpochMilli(1107011348029L)); record3String = "1107011348.029\t" + hostname + "\t" @@ -73,7 +73,7 @@ public class VespaLogHandlerTestCase { record4 = new LogRecord(Level.WARNING, "unicode \u00E6\u00F8\u00E5 test \u7881 unicode"); record4.setLoggerName("com.yahoo.log.test"); - record4.setMillis(1107011348029L); + record4.setInstant(ofEpochMilli(1107011348029L)); record4String = "1107011348.029\t" + hostname + "\t" |