diff options
author | HÃ¥kon Hallingstad <hakon.hallingstad@gmail.com> | 2023-09-14 09:33:56 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-14 09:33:56 +0200 |
commit | 4d486ca78615bbae3f0d10124f345998b14de7cf (patch) | |
tree | 1044153e88d3880fe611e92a0b9755099233e698 /node-repository | |
parent | 655cef075f7870f0760d7194ad32a47f754b7dab (diff) |
Revert "Revert "Handle IPv4 address not found from resolving hostname for GCP exclave""
Diffstat (limited to 'node-repository')
8 files changed, 234 insertions, 126 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java index 2fa4fc82867..ea5ef276b95 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java @@ -24,6 +24,7 @@ import com.yahoo.vespa.hosted.provision.NodeMutex; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.node.History; +import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisionRequest; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; @@ -278,9 +279,12 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { .build(); NodeSpec nodeSpec = NodeSpec.from(clusterCapacity.count(), 1, nodeResources, false, true, nodeRepository().zone().cloud().account(), Duration.ZERO); + var allocationContext = IP.Allocation.Context.from(nodeRepository().zone().cloud().name(), + nodeSpec.cloudAccount().isExclave(nodeRepository().zone()), + nodeRepository().nameResolver()); NodePrioritizer prioritizer = new NodePrioritizer(allNodes, applicationId, clusterSpec, nodeSpec, - true, nodeRepository().nameResolver(), nodeRepository().nodes(), nodeRepository().resourcesCalculator(), - nodeRepository().spareCount(), nodeSpec.cloudAccount().isExclave(nodeRepository().zone())); + true, allocationContext, nodeRepository().nodes(), nodeRepository().resourcesCalculator(), + nodeRepository().spareCount()); List<NodeCandidate> nodeCandidates = prioritizer.collect(); MutableInteger index = new MutableInteger(0); return nodeCandidates diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisioner.java index f78d717e010..c1407929cc9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisioner.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeMutex; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.node.Dns; import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.provisioning.FatalProvisioningException; import com.yahoo.vespa.hosted.provision.provisioning.HostIpConfig; @@ -98,7 +99,7 @@ public class HostResumeProvisioner extends NodeRepositoryMaintainer { /** Verify DNS configuration of given node */ private void verifyDns(String hostname, NodeType hostType, CloudAccount cloudAccount, IP.Config ipConfig) { for (String ipAddress : ipConfig.primary()) { - IP.verifyDns(hostname, ipAddress, hostType, nodeRepository().nameResolver(), cloudAccount, nodeRepository().zone()); + Dns.verify(hostname, ipAddress, hostType, nodeRepository().nameResolver(), cloudAccount, nodeRepository().zone()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Dns.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Dns.java new file mode 100644 index 00000000000..2c2239e4538 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Dns.java @@ -0,0 +1,73 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.node; + +import com.yahoo.config.provision.CloudAccount; +import com.yahoo.config.provision.CloudName; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.Zone; +import com.yahoo.vespa.hosted.provision.persistence.NameResolver; + +import java.util.EnumSet; +import java.util.Optional; +import java.util.Set; + +import static com.yahoo.config.provision.NodeType.confighost; + +/** + * DNS configuration for a node. + * + * @author hakonhall + */ +public class Dns { + private Dns() {} + + public enum RecordType { FORWARD, PUBLIC_FORWARD, REVERSE } + + /** Returns the set of DNS record types for a host and its children and the given version (ipv6), host type, etc. */ + public static Set<RecordType> recordTypesFor(IP.Version ipVersion, NodeType hostType, CloudName cloudName, boolean exclave) { + if (cloudName == CloudName.AWS) + return exclave ? + EnumSet.of(RecordType.FORWARD, RecordType.PUBLIC_FORWARD) : + EnumSet.of(RecordType.FORWARD, RecordType.PUBLIC_FORWARD, RecordType.REVERSE); + + if (cloudName == CloudName.GCP) { + if (exclave) { + return ipVersion.is6() ? + EnumSet.of(RecordType.FORWARD, RecordType.PUBLIC_FORWARD) : + EnumSet.noneOf(RecordType.class); + } else { + return hostType == confighost && ipVersion.is6() ? + EnumSet.of(RecordType.FORWARD, RecordType.REVERSE, RecordType.PUBLIC_FORWARD) : + EnumSet.of(RecordType.FORWARD, RecordType.REVERSE); + } + } + + throw new IllegalArgumentException("Does not manage DNS for cloud " + cloudName); + } + + /** Verify DNS configuration of given hostname and IP address */ + public static void verify(String hostname, String ipAddress, NodeType nodeType, NameResolver resolver, + CloudAccount cloudAccount, Zone zone) { + IP.Version version = IP.Version.fromIpAddress(ipAddress); + Set<RecordType> recordTypes = recordTypesFor(version, nodeType, zone.cloud().name(), cloudAccount.isExclave(zone)); + + if (recordTypes.contains(RecordType.FORWARD)) { + NameResolver.RecordType recordType = version.is6() ? NameResolver.RecordType.AAAA : NameResolver.RecordType.A; + Set<String> addresses = resolver.resolve(hostname, recordType); + if (!addresses.equals(java.util.Set.of(ipAddress))) + throw new IllegalArgumentException("Expected " + hostname + " to resolve to " + ipAddress + + ", but got " + addresses); + } + + if (recordTypes.contains(RecordType.REVERSE)) { + Optional<String> reverseHostname = resolver.resolveHostname(ipAddress); + if (reverseHostname.isEmpty()) + throw new IllegalArgumentException(ipAddress + " did not resolve to a hostname"); + + if (!reverseHostname.get().equals(hostname)) + throw new IllegalArgumentException(ipAddress + " resolved to " + reverseHostname.get() + + ", which does not match expected hostname " + hostname); + } + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java index 0927ffe3ce2..ca4b2c99ef0 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/IP.java @@ -6,7 +6,6 @@ import com.google.common.primitives.UnsignedBytes; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.HostName; -import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; @@ -17,7 +16,6 @@ import com.yahoo.vespa.hosted.provision.persistence.NameResolver.RecordType; import java.net.InetAddress; import java.util.Collections; import java.util.Comparator; -import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -38,6 +36,43 @@ import static com.yahoo.config.provision.NodeType.proxyhost; */ public record IP() { + /** IP version. Can be compared with ==, !=, and equals(). */ + public static class Version { + public static final Version v4 = new Version(4); + public static final Version v6 = new Version(6); + + private final int version; + + public static Version fromIpAddress(String ipAddress) { + if (ipAddress.contains(":")) return v6; + if (ipAddress.contains(".")) return v4; + throw new IllegalArgumentException("Failed to deduce the IP version from the textual representation of the IP address: " + ipAddress); + } + + public static Version fromIsIpv6(boolean isIpv6) { return isIpv6 ? v6 : v4; } + + private Version(int version) { this.version = version; } + + public boolean is4() { return version == 4; } + public boolean is6() { return version == 6; } + + public RecordType toForwardRecordType() { return is4() ? RecordType.A : RecordType.AAAA; } + + @Override + public String toString() { return "IPv" + version; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Version version1 = (Version) o; + return version == version1.version; + } + + @Override + public int hashCode() { return Objects.hash(version); } + } + /** Comparator for sorting IP addresses by their natural order */ public static final Comparator<InetAddress> NATURAL_ORDER = (ip1, ip2) -> { byte[] address1 = ip1.getAddress(); @@ -156,28 +191,28 @@ public record IP() { } /** A list of IP addresses and their protocol */ - record IpAddresses(Set<String> addresses, Protocol protocol) { + record IpAddresses(Set<String> addresses, Stack stack) { - public IpAddresses(Set<String> addresses, Protocol protocol) { + public IpAddresses(Set<String> addresses, Stack stack) { this.addresses = Collections.unmodifiableSet(new LinkedHashSet<>(Objects.requireNonNull(addresses, "addresses must be non-null"))); - this.protocol = Objects.requireNonNull(protocol, "type must be non-null"); + this.stack = Objects.requireNonNull(stack, "type must be non-null"); } /** Create addresses of the given set */ private static IpAddresses of(Set<String> addresses) { long ipv6AddrCount = addresses.stream().filter(IP::isV6).count(); if (ipv6AddrCount == addresses.size()) { // IPv6-only - return new IpAddresses(addresses, Protocol.ipv6); + return new IpAddresses(addresses, Stack.ipv6); } long ipv4AddrCount = addresses.stream().filter(IP::isV4).count(); if (ipv4AddrCount == addresses.size()) { // IPv4-only - return new IpAddresses(addresses, Protocol.ipv4); + return new IpAddresses(addresses, Stack.ipv4); } // If we're dual-stacked, we must have an equal number of addresses of each protocol. if (ipv4AddrCount == ipv6AddrCount) { - return new IpAddresses(addresses, Protocol.dualStack); + return new IpAddresses(addresses, Stack.dual); } throw new IllegalArgumentException(String.format("Dual-stacked IP address list must have an " + @@ -186,16 +221,21 @@ public record IP() { ipv6AddrCount, ipv4AddrCount)); } - public enum Protocol { + public enum Stack { - dualStack("dual-stack"), - ipv4("IPv4-only"), - ipv6("IPv6-only"); + dual("dual-stack", Version.v4, Version.v6), + ipv4("IPv4-only", Version.v4), + ipv6("IPv6-only", Version.v6); private final String description; + private final Set<Version> versions; - Protocol(String description) { this.description = description; } + Stack(String description, Version... versions) { + this.description = description; + this.versions = Set.of(versions); + } + public boolean supports(Version version) { return versions.contains(version); } } } @@ -227,34 +267,34 @@ public record IP() { /** * Find a free allocation in this pool. Note that the allocation is not final until it is assigned to a node * - * @param nodes a locked list of all nodes in the repository + * @param context allocation context + * @param nodes a locked list of all nodes in the repository * @return an allocation from the pool, if any can be made */ - public Optional<Allocation> findAllocation(LockedNodeList nodes, NameResolver resolver, boolean hasPtr) { + public Optional<Allocation> findAllocation(Allocation.Context context, LockedNodeList nodes) { if (ipAddresses.addresses.isEmpty()) { // IP addresses have not yet been resolved and should be done later. return findUnusedHostnames(nodes).map(Allocation::ofHostname) .findFirst(); } - if (!hasPtr) { - // Without PTR records (reverse IP mapping): Ensure only forward resolving from hostnames. - return findUnusedHostnames(nodes).findFirst().map(hostname -> Allocation.fromHostname(hostname, resolver, ipAddresses.protocol)); - } + Set<String> unusedIps = findUnusedIpAddresses(nodes); - if (ipAddresses.protocol == IpAddresses.Protocol.ipv4) { - return findUnusedIpAddresses(nodes).stream() - .findFirst() - .map(addr -> Allocation.ofIpv4(addr, resolver)); + if (context.allocateFromUnusedHostname()) + return findUnusedHostnames(nodes).findFirst().map(hostname -> Allocation.fromHostname(context, hostname, ipAddresses.stack, unusedIps)); + + if (ipAddresses.stack == IpAddresses.Stack.ipv4) { + return unusedIps.stream() + .findFirst() + .map(addr -> Allocation.ofIpv4(addr, context.resolver())); } - var unusedAddresses = findUnusedIpAddresses(nodes); - var allocation = unusedAddresses.stream() - .filter(IP::isV6) - .findFirst() - .map(addr -> Allocation.ofIpv6(addr, resolver)); + var allocation = unusedIps.stream() + .filter(IP::isV6) + .findFirst() + .map(addr -> Allocation.ofIpv6(addr, context.resolver())); allocation.flatMap(Allocation::ipv4Address).ifPresent(ipv4Address -> { - if (!unusedAddresses.contains(ipv4Address)) { + if (!unusedIps.contains(ipv4Address)) { throw new IllegalArgumentException("Allocation resolved " + ipv4Address + " from hostname " + allocation.get().hostname + ", but that address is not owned by this node"); @@ -299,6 +339,36 @@ public record IP() { Objects.requireNonNull(ipv6Address, "ipv6Address must be non-null"); } + public static class Context { + private final CloudName cloudName; + private final boolean exclave; + private final NameResolver resolver; + + private Context(CloudName cloudName, boolean exclave, NameResolver resolver) { + this.cloudName = cloudName; + this.exclave = exclave; + this.resolver = resolver; + } + + public static Context from(CloudName cloudName, boolean exclave, NameResolver resolver) { + return new Context(cloudName, exclave, resolver); + } + + public NameResolver resolver() { return resolver; } + + public boolean allocateFromUnusedHostname() { return exclave; } + + public boolean hasIpNotInDns(Version version) { + if (exclave && cloudName == CloudName.GCP && version.is4()) { + // Exclave nodes in GCP have IPv4, because load balancers backends are required to be IPv4, + // but it's private (10.x). The hostname only resolves to the public IPv6 address. + return true; + } + + return false; + } + } + /** * Allocate an IPv6 address. * @@ -353,22 +423,30 @@ public record IP() { return new Allocation(hostname4, Optional.of(addresses.get(0)), Optional.empty()); } - private static Allocation fromHostname(HostName hostname, NameResolver resolver, IpAddresses.Protocol protocol) { - // Resolve both A and AAAA to verify they match the protocol and to avoid surprises later on. + private static Allocation fromHostname(Context context, HostName hostname, IpAddresses.Stack stack, Set<String> unusedIps) { + Optional<String> ipv4Address = resolveAndVerify(context, hostname, stack, IP.Version.v4, unusedIps); + Optional<String> ipv6Address = resolveAndVerify(context, hostname, stack, IP.Version.v6, unusedIps); + return new Allocation(hostname.value(), ipv4Address, ipv6Address); + } - Optional<String> ipv4Address = resolveOptional(hostname.value(), resolver, RecordType.A); - if (protocol != IpAddresses.Protocol.ipv6 && ipv4Address.isEmpty()) - throw new IllegalArgumentException(protocol.description + " hostname " + hostname.value() + " did not resolve to an IPv4 address"); - if (protocol == IpAddresses.Protocol.ipv6 && ipv4Address.isPresent()) - throw new IllegalArgumentException(protocol.description + " hostname " + hostname.value() + " has an IPv4 address: " + ipv4Address.get()); + private static Optional<String> resolveAndVerify(Context context, HostName hostname, IpAddresses.Stack stack, Version version, Set<String> unusedIps) { + if (context.hasIpNotInDns(version)) { + List<String> candidates = unusedIps.stream() + .filter(a -> IP.Version.fromIpAddress(a).equals(version)) + .toList(); + if (candidates.size() != 1) { + throw new IllegalStateException("Unable to find a unique child IP address of " + hostname + ": Found " + candidates); + } + return candidates.stream().findFirst(); + } - Optional<String> ipv6Address = resolveOptional(hostname.value(), resolver, RecordType.AAAA); - if (protocol != IpAddresses.Protocol.ipv4 && ipv6Address.isEmpty()) - throw new IllegalArgumentException(protocol.description + " hostname " + hostname.value() + " did not resolve to an IPv6 address"); - if (protocol == IpAddresses.Protocol.ipv4 && ipv6Address.isPresent()) - throw new IllegalArgumentException(protocol.description + " hostname " + hostname.value() + " has an IPv6 address: " + ipv6Address.get()); + Optional<String> address = resolveOptional(hostname.value(), context.resolver(), version.toForwardRecordType()); + if (stack.supports(version) && address.isEmpty()) + throw new IllegalArgumentException(stack.description + " hostname " + hostname.value() + " did not resolve to an " + version + " address"); + if (!stack.supports(version) && address.isPresent()) + throw new IllegalArgumentException(stack.description + " hostname " + hostname.value() + " has an " + version + " address: " + address.get()); - return new Allocation(hostname.value(), ipv4Address, ipv6Address); + return address; } private static Optional<String> resolveOptional(String hostname, NameResolver resolver, RecordType recordType) { @@ -401,55 +479,6 @@ public record IP() { } } - public enum DnsRecordType { FORWARD, PUBLIC_FORWARD, REVERSE } - - /** Returns the set of DNS record types for a host and its children and the given version (ipv6), host type, etc. */ - public static Set<DnsRecordType> dnsRecordTypesFor(boolean ipv6, NodeType hostType, CloudName cloudName, boolean exclave) { - if (cloudName == CloudName.AWS) - return exclave ? - EnumSet.of(DnsRecordType.FORWARD, DnsRecordType.PUBLIC_FORWARD) : - EnumSet.of(DnsRecordType.FORWARD, DnsRecordType.PUBLIC_FORWARD, DnsRecordType.REVERSE); - - if (cloudName == CloudName.GCP) { - if (exclave) { - return ipv6 ? - EnumSet.of(DnsRecordType.FORWARD, DnsRecordType.PUBLIC_FORWARD) : - EnumSet.noneOf(DnsRecordType.class); - } else { - return hostType == confighost && ipv6 ? - EnumSet.of(DnsRecordType.FORWARD, DnsRecordType.REVERSE, DnsRecordType.PUBLIC_FORWARD) : - EnumSet.of(DnsRecordType.FORWARD, DnsRecordType.REVERSE); - } - } - - throw new IllegalArgumentException("Does not manage DNS for cloud " + cloudName); - } - - /** Verify DNS configuration of given hostname and IP address */ - public static void verifyDns(String hostname, String ipAddress, NodeType nodeType, NameResolver resolver, - CloudAccount cloudAccount, Zone zone) { - boolean ipv6 = isV6(ipAddress); - Set<DnsRecordType> recordTypes = dnsRecordTypesFor(ipv6, nodeType, zone.cloud().name(), cloudAccount.isExclave(zone)); - - if (recordTypes.contains(DnsRecordType.FORWARD)) { - RecordType recordType = ipv6 ? RecordType.AAAA : RecordType.A; - Set<String> addresses = resolver.resolve(hostname, recordType); - if (!addresses.equals(Set.of(ipAddress))) - throw new IllegalArgumentException("Expected " + hostname + " to resolve to " + ipAddress + - ", but got " + addresses); - } - - if (recordTypes.contains(DnsRecordType.REVERSE)) { - Optional<String> reverseHostname = resolver.resolveHostname(ipAddress); - if (reverseHostname.isEmpty()) - throw new IllegalArgumentException(ipAddress + " did not resolve to a hostname"); - - if (!reverseHostname.get().equals(hostname)) - throw new IllegalArgumentException(ipAddress + " resolved to " + reverseHostname.get() + - ", which does not match expected hostname " + hostname); - } - } - /** Convert IP address to string. This uses :: for zero compression in IPv6 addresses. */ public static String asString(InetAddress inetAddress) { return InetAddresses.toAddrString(inetAddress); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java index adc04c491e2..27136811fa1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.Nodelike; import com.yahoo.vespa.hosted.provision.node.Allocation; import com.yahoo.vespa.hosted.provision.node.IP; -import com.yahoo.vespa.hosted.provision.persistence.NameResolver; import com.yahoo.yolean.Exceptions; import java.time.Instant; @@ -276,9 +275,8 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat Node parent, boolean violatesSpares, LockedNodeList allNodes, - NameResolver nameResolver, - boolean hasPtrRecord) { - return new VirtualNodeCandidate(resources, freeParentCapacity, parent, violatesSpares, true, allNodes, nameResolver, hasPtrRecord); + IP.Allocation.Context ipAllocationContext) { + return new VirtualNodeCandidate(resources, freeParentCapacity, parent, violatesSpares, true, allNodes, ipAllocationContext); } public static NodeCandidate createNewExclusiveChild(Node node, Node parent) { @@ -382,8 +380,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat /** Needed to construct the node */ private final LockedNodeList allNodes; - private final NameResolver nameResolver; - private final boolean hasPtrRecord; + private final IP.Allocation.Context ipAllocationContext; private VirtualNodeCandidate(NodeResources resources, NodeResources freeParentCapacity, @@ -391,13 +388,11 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat boolean violatesSpares, boolean exclusiveSwitch, LockedNodeList allNodes, - NameResolver nameResolver, - boolean hasPtrRecord) { + IP.Allocation.Context ipAllocationContext) { super(freeParentCapacity, Optional.of(parent), violatesSpares, exclusiveSwitch, false, true, false); this.resources = resources; this.allNodes = allNodes; - this.nameResolver = nameResolver; - this.hasPtrRecord = hasPtrRecord; + this.ipAllocationContext = ipAllocationContext; } @Override @@ -436,7 +431,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat public NodeCandidate withNode() { Optional<IP.Allocation> allocation; try { - allocation = parent.get().ipConfig().pool().findAllocation(allNodes, nameResolver, hasPtrRecord); + allocation = parent.get().ipConfig().pool().findAllocation(ipAllocationContext, allNodes); if (allocation.isEmpty()) return new InvalidNodeCandidate(resources, freeParentCapacity, parent.get(), "No addresses available on parent host"); } catch (Exception e) { @@ -460,7 +455,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat @Override public NodeCandidate withExclusiveSwitch(boolean exclusiveSwitch) { - return new VirtualNodeCandidate(resources, freeParentCapacity, parent.get(), violatesSpares, exclusiveSwitch, allNodes, nameResolver, hasPtrRecord); + return new VirtualNodeCandidate(resources, freeParentCapacity, parent.get(), violatesSpares, exclusiveSwitch, allNodes, ipAllocationContext); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 24cf86e8a25..fe9f3443299 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.node.Nodes; import com.yahoo.vespa.hosted.provision.persistence.NameResolver; @@ -36,18 +37,17 @@ public class NodePrioritizer { private final NodeSpec requested; private final ApplicationId application; private final ClusterSpec clusterSpec; - private final NameResolver nameResolver; + private final IP.Allocation.Context ipAllocationContext; private final Nodes nodes; private final boolean dynamicProvisioning; private final boolean canAllocateToSpareHosts; private final boolean topologyChange; private final int currentClusterSize; private final Set<Node> spareHosts; - private final boolean enclave; public NodePrioritizer(LockedNodeList allNodes, ApplicationId application, ClusterSpec clusterSpec, NodeSpec nodeSpec, - boolean dynamicProvisioning, NameResolver nameResolver, Nodes nodes, - HostResourcesCalculator hostResourcesCalculator, int spareCount, boolean enclave) { + boolean dynamicProvisioning, IP.Allocation.Context ipAllocationContext, Nodes nodes, + HostResourcesCalculator hostResourcesCalculator, int spareCount) { this.allNodes = allNodes; this.calculator = hostResourcesCalculator; this.capacity = new HostCapacity(this.allNodes, hostResourcesCalculator); @@ -58,9 +58,8 @@ public class NodePrioritizer { this.spareHosts = dynamicProvisioning ? capacity.findSpareHostsInDynamicallyProvisionedZones(this.allNodes.asList()) : capacity.findSpareHosts(this.allNodes.asList(), spareCount); - this.nameResolver = nameResolver; + this.ipAllocationContext = ipAllocationContext; this.nodes = nodes; - this.enclave = enclave; NodeList nodesInCluster = this.allNodes.owner(application).type(clusterSpec.type()).cluster(clusterSpec.id()); NodeList nonRetiredNodesInCluster = nodesInCluster.not().retired(); @@ -135,8 +134,7 @@ public class NodePrioritizer { host, spareHosts.contains(host), allNodes, - nameResolver, - !enclave)); + ipAllocationContext)); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 78538af6c3c..67dae48cff7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.node.IP; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; import java.util.LinkedHashSet; @@ -167,16 +168,18 @@ public class Preparer { Supplier<Integer> nextIndex, LockedNodeList allNodes) { validateAccount(requested.cloudAccount(), application, allNodes); NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requested, nextIndex, nodeRepository); + var allocationContext = IP.Allocation.Context.from(nodeRepository.zone().cloud().name(), + requested.cloudAccount().isExclave(nodeRepository.zone()), + nodeRepository.nameResolver()); NodePrioritizer prioritizer = new NodePrioritizer(allNodes, application, cluster, requested, nodeRepository.zone().cloud().dynamicProvisioning(), - nodeRepository.nameResolver(), + allocationContext, nodeRepository.nodes(), nodeRepository.resourcesCalculator(), - nodeRepository.spareCount(), - requested.cloudAccount().isExclave(nodeRepository.zone())); + nodeRepository.spareCount()); allocation.offer(prioritizer.collect()); return allocation; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java index 88fe88dbaad..ecf7697f781 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/node/IPTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.provision.node; import com.google.common.collect.ImmutableSet; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeType; @@ -29,9 +30,12 @@ public class IPTest { private static final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default"); private static final LockedNodeList emptyList = new LockedNodeList(List.of(), () -> {}); - private final MockNameResolver resolver = new MockNameResolver().explicitReverseRecords(); + private IP.Allocation.Context contextOf(boolean exclave) { + return IP.Allocation.Context.from(CloudName.AWS, exclave, resolver); + } + @Test public void test_natural_order() { Set<String> ipAddresses = Set.of( @@ -85,7 +89,8 @@ public class IPTest { resolver.addReverseRecord("::1", "host3"); resolver.addReverseRecord("::2", "host1"); - Optional<IP.Allocation> allocation = pool.findAllocation(emptyList, resolver, true); + var context = contextOf(false); + Optional<IP.Allocation> allocation = pool.findAllocation(context, emptyList); assertEquals(Optional.of("::1"), allocation.get().ipv6Address()); assertFalse(allocation.get().ipv4Address().isPresent()); assertEquals("host3", allocation.get().hostname()); @@ -93,7 +98,7 @@ public class IPTest { // Allocation fails if DNS record is missing resolver.removeRecord("host3"); try { - pool.findAllocation(emptyList, resolver, true); + pool.findAllocation(context, emptyList); fail("Expected exception"); } catch (Exception e) { assertEquals("java.net.UnknownHostException: Could not resolve: host3", e.getMessage()); @@ -103,7 +108,7 @@ public class IPTest { @Test public void test_find_allocation_ipv4_only() { var pool = testPool(false); - var allocation = pool.findAllocation(emptyList, resolver, true); + var allocation = pool.findAllocation(contextOf(false), emptyList); assertFalse("Found allocation", allocation.isEmpty()); assertEquals(Optional.of("127.0.0.1"), allocation.get().ipv4Address()); assertTrue("No IPv6 address", allocation.get().ipv6Address().isEmpty()); @@ -112,7 +117,7 @@ public class IPTest { @Test public void test_find_allocation_dual_stack() { IP.Pool pool = testPool(true); - Optional<IP.Allocation> allocation = pool.findAllocation(emptyList, resolver, true); + Optional<IP.Allocation> allocation = pool.findAllocation(contextOf(false), emptyList); assertEquals(Optional.of("::1"), allocation.get().ipv6Address()); assertEquals("127.0.0.2", allocation.get().ipv4Address().get()); assertEquals("host3", allocation.get().hostname()); @@ -123,7 +128,7 @@ public class IPTest { IP.Pool pool = testPool(true); resolver.addRecord("host3", "127.0.0.127"); try { - pool.findAllocation(emptyList, resolver, true); + pool.findAllocation(contextOf(false), emptyList); fail("Expected exception"); } catch (IllegalArgumentException e) { assertEquals("Hostname host3 resolved to more than 1 IPv4 address: [127.0.0.2, 127.0.0.127]", @@ -137,7 +142,7 @@ public class IPTest { resolver.removeRecord("127.0.0.2") .addReverseRecord("127.0.0.2", "host5"); try { - pool.findAllocation(emptyList, resolver, true); + pool.findAllocation(contextOf(false), emptyList); fail("Expected exception"); } catch (IllegalArgumentException e) { assertEquals("Hostnames resolved from each IP address do not point to the same hostname " + @@ -158,7 +163,7 @@ public class IPTest { Set.of("2600:1f10:::2", "2600:1f10:::3"), List.of(HostName.of("node1"), HostName.of("node2"))); IP.Pool pool = config.pool(); - Optional<IP.Allocation> allocation = pool.findAllocation(emptyList, resolver, false); + Optional<IP.Allocation> allocation = pool.findAllocation(contextOf(true), emptyList); } private IP.Pool testPool(boolean dualStack) { @@ -193,7 +198,7 @@ public class IPTest { } IP.Pool pool = node.ipConfig().pool(); - assertNotEquals(dualStack, pool.ipAddresses().protocol() == IP.IpAddresses.Protocol.ipv4); + assertNotEquals(dualStack, pool.ipAddresses().stack() == IP.IpAddresses.Stack.ipv4); return pool; } |