diff --git a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb index 42409784ea..9188b3fd7f 100644 --- a/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb +++ b/src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb @@ -116,7 +116,7 @@ def try_to_allocate_dynamic_ip(reservation, subnet) def find_next_available_ip(addresses_we_cant_allocate, first_range_address, prefix) # Remove IPs that are below subnet range - filtered_ips = addresses_we_cant_allocate.sort_by { |ip| ip.to_i }.reject { |ip| ip.to_i < first_range_address.to_i } + filtered_ips = addresses_we_cant_allocate.sort_by { |ip| [ip.to_i, ip.prefix] }.reject { |ip| ip.to_i < first_range_address.to_i } current_ip = to_ipaddr(first_range_address.to_i + 1) found = false diff --git a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb index 27fe9a90e9..90d2927e2b 100644 --- a/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb +++ b/src/bosh-director/lib/bosh/director/ip_addr_or_cidr.rb @@ -43,12 +43,32 @@ def each_base_addr(prefix_length) end end + # Structural equality for use by +Set+ and +Hash+. Two instances are equal + # only if they share the same base address integer, prefix length, and + # address family. For example, +10.0.11.32/30+ and +10.0.11.32/32+ are NOT + # equal + # + # @param other [Object] the object to compare with + # @return [Boolean] +true+ if +other+ is an +IpAddrOrCidr+ with identical + # address, prefix, and address family; +false+ otherwise def eql?(other) - self == other + other.is_a?(IpAddrOrCidr) && + @ipaddr.to_i == other.to_i && + @ipaddr.prefix == other.prefix && + @ipaddr.ipv4? == other.ipv4? end + # Returns an integer hash value derived from the address integer, prefix + # length, and address family - the same three fields used by {#eql?}. + # Satisfies Ruby's contract: if +a.eql?(b)+ then +a.hash == b.hash+. + # + # @return [Integer] hash value for use by +Set+ and +Hash+ def hash - @ipaddr.hash + [ + @ipaddr.to_i, + @ipaddr.prefix, + @ipaddr.ipv4?, + ].hash end def count diff --git a/src/bosh-director/spec/unit/bosh/director/ip_addr_or_cidr_spec.rb b/src/bosh-director/spec/unit/bosh/director/ip_addr_or_cidr_spec.rb index 6808b2173b..9d902e78b8 100644 --- a/src/bosh-director/spec/unit/bosh/director/ip_addr_or_cidr_spec.rb +++ b/src/bosh-director/spec/unit/bosh/director/ip_addr_or_cidr_spec.rb @@ -228,5 +228,96 @@ module Bosh::Director end end end + + describe '#eql? and #hash' do + context 'when two objects have the same base address and prefix' do + it 'eql? returns true' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.32/30') + expect(a.eql?(b)).to be true + end + + it 'hash values are equal' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.32/30') + expect(a.hash).to eq(b.hash) + end + + it 'can be used as the same Set element' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.32/30') + expect(Set.new([a, b]).size).to eq(1) + end + end + + context 'when two objects have the same base address but different prefixes' do + it 'eql? returns false' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.32/32') + expect(a.eql?(b)).to be false + end + + it 'hash values are different' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.32/32') + expect(a.hash).not_to eq(b.hash) + end + + it 'are stored as distinct elements in a Set' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.32/32') + expect(Set.new([a, b]).size).to eq(2) + end + end + + context 'when using objects with different base addresses' do + it 'eql? returns false' do + a = IpAddrOrCidr.new('10.0.11.32/30') + b = IpAddrOrCidr.new('10.0.11.36/30') + expect(a.eql?(b)).to be false + end + end + + context 'when compared to a non-IpAddrOrCidr object' do + it 'eql? returns false for nil' do + a = IpAddrOrCidr.new('10.0.11.32/30') + expect(a.eql?(nil)).to be false + end + + it 'eql? returns false for a String' do + a = IpAddrOrCidr.new('10.0.11.32/30') + expect(a.eql?('10.0.11.32/30')).to be false + end + end + + context 'when an IPv4 and IPv6 address have the same integer value and prefix' do + it 'eql? returns false' do + ipv4 = IpAddrOrCidr.new('0.0.0.0/32') + ipv6 = IpAddrOrCidr.new('::/32') + expect(ipv4.eql?(ipv6)).to be false + end + + it 'are stored as distinct elements in a Set' do + ipv4 = IpAddrOrCidr.new('0.0.0.0/32') + ipv6 = IpAddrOrCidr.new('::/32') + expect(Set.new([ipv4, ipv6]).size).to eq(2) + end + end + + context 'hash contract (eql? implies equal hash)' do + it 'is satisfied for equal objects' do + a = IpAddrOrCidr.new('192.168.1.0/24') + b = IpAddrOrCidr.new('192.168.1.0/24') + expect(a.eql?(b)).to be true + expect(a.hash).to eq(b.hash) + end + + it 'is satisfied for unequal objects (different prefix)' do + a = IpAddrOrCidr.new('192.168.1.0/24') + b = IpAddrOrCidr.new('192.168.1.0/32') + expect(a.eql?(b)).to be false + end + end + end end end