Foreman » Smart Proxy

Foreman core now contains randomized IPAM DB implementation, we need similar for Smart Proxy. Ideally this would be an implementation that Ivan this is customer-facing, can you put this on your scrum?

module IPAM
  # Internal DB IPAM returning all IPs in random order to minimize race conditions
  class RandomDb < Base
    def generator
       @generator ||= Random.new(mac ? mac.gsub(':', '').to_i(16) : Random.new_seed)
    end

    def random_ip
      IPAddr.new(generator.rand(subnet_range.first.to_i..subnet_range.last.to_i), subnet.family)
    end

    # Safety check not to spend much CPU time when there are no many free IPs left. This gives up
    # in about a second on Ryzen 1700 running with Ruby 2.4.
    MAX_ITERATIONS = 100_000
    def suggest_ip
      iterations = 0
      loop do
        # next random IP from the sequence generated by MAC seed
        candidate = random_ip
        iterations += 1
        break if iterations >= MAX_ITERATIONS
        # try to match it
        ip = candidate.to_s
        if !excluded_ips.include?(ip) && !subnet.known_ips.include?(ip)
          logger.debug("Found #{ip} in #{iterations} iterations")
          return ip
        end
      end
      logger.debug("Not suggesting IP Address for #{subnet} as no free IP found in reasonable time (#{iterations} iterations)")
      errors.add(:subnet, _('no random free IP could be found in our DB, enlarge subnet range'))
      nil
    end
  end
end

The following unit tests simulates the race condition:

  def test_unused_ip_with_concurrent_record_add
    @subnet.stubs(:icmp_pingable?)
    @subnet.stubs(:tcp_pingable?)
    records = []
    records << Proxy::DHCP::Reservation.new('test', "192.168.0.1", "aa:bb:cc:dd:ee:01", @subnet, :hostname =>'test_01')
    assert_equal "192.168.0.2", @subnet.unused_ip(records)
    assert_equal "192.168.0.3", @subnet.unused_ip(records)
    records << Proxy::DHCP::Reservation.new('test', "192.168.0.2", "aa:bb:cc:dd:ee:02", @subnet, :hostname =>'test_02')
    assert_equal "192.168.0.4", @subnet.unused_ip(records) # this fails - returns 192.168.0.5
  ensure
    File.delete('test/tmp/foreman-proxy_192.168.0.0_24.tmp')
  end

  def test_unused_ip_with_concurrent_record_delete
    @subnet.stubs(:icmp_pingable?)
    @subnet.stubs(:tcp_pingable?)
    records = []
    records << Proxy::DHCP::Reservation.new('test', "192.168.0.1", "aa:bb:cc:dd:ee:01", @subnet, :hostname =>'test_01')
    records << Proxy::DHCP::Reservation.new('test', "192.168.0.2", "aa:bb:cc:dd:ee:02", @subnet, :hostname =>'test_02')
    assert_equal "192.168.0.3", @subnet.unused_ip(records)
    records.pop
    assert_equal "192.168.0.4", @subnet.unused_ip(records)  # this fails - returns 192.168.0.3
  ensure
    File.delete('test/tmp/foreman-proxy_192.168.0.0_24.tmp')
  end

And the following unit test never returns (CPU spikes at 100%) because Ruby is busy with creating 16 million records array in memory. The randomized can solve that:

  def test_unused_aclass_ip_beware_this_never_finishes
    @network = "10.0.0.0" 
    @netmask = "255.0.0.0" 
    @subnet = Proxy::DHCP::Subnet.new @network, @netmask
    @subnet.stubs(:icmp_pingable?)
    @subnet.stubs(:tcp_pingable?)
    r = Proxy::DHCP::Reservation.new('test', "10.0.0.1", "aa:bb:cc:dd:ee:ff", @subnet, :hostname =>'test1')
    assert_equal "10.0.0.2", @subnet.unused_ip([r])
  ensure
    File.delete('test/tmp/foreman-proxy_10.0.0.0_8.tmp')
  end

Ideally the code is refactored so unused_ip has it's own providers injected into subnet/server and we provide two implementations: the current one (perhaps fixed so it uses less memory) and random which I believe is very easy to implement (see the code bit from core) and workarounds the issue nicely.