Fix flaky IP assignment

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

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

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