Project

General

Profile

Actions

Bug #8727

closed

Discovered hosts with same MAC address are reported as DHCP conflicts

Added by Lukas Zapletal about 10 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Category:
Orchestration
Target version:
Fixed in Releases:
Found in Releases:

Description

In our DHCP validation code, we check for conflicts for each leases represented by both MAC and IP. We should not validate records based on MAC because this particular case is not valid reason we (me/Greg) think to present an interactive confirmation form to the user.


Related issues 4 (0 open4 closed)

Related to Foreman - Bug #9776: foreman does not show a warning if a dhcp reservation already exists and simply failsRejected03/16/2015Actions
Related to Foreman - Bug #12637: Improve DHCP lease unit testClosedLukas Zapletal11/30/2015Actions
Related to Foreman - Bug #13725: Orchestration fails with 409 when there is a MAC DHCP conflictClosedLukas Zapletal02/16/2016Actions
Related to Smart Proxy - Bug #8538: ISC DHCP provider does not respect expired and free leasesClosedLukas Zapletal12/01/2014Actions
Actions #1

Updated by Lukas Zapletal about 10 years ago

  • Bugzilla link set to 1170518
Actions #2

Updated by Greg Sutcliffe about 10 years ago

To expand a bit on Lukas' brief description :P

Current Situation

Currently we take the record we're proposing to create (a mac/ip pair) can then request all the records the proxy can find either by mac or ip. This returns an array, and we remove entries matching the proposed record. If there's anything left, we report it as a conflict. This is executed with this code:

conflicts = [proxy.record(network, mac), proxy.record(network, ip)].delete_if { |c| c  self }.compact

This causes an issue in the case of multiple-ip leases. If a host boots with a PXE UUID, and then gets a new lease from dhcpcd within the OS, we now have:

[mac1, ip1] - PXE lease
[mac1, ip2] - dhcpcd lease

Given the above code, if we provision this system and change nothing, we will still have an entry for [mac1, ip1] in the conflicts array. This is potentially reported to the user via an overwrite confirmation, but that wouldn't be available via the API (or any plugin which is hiding the host edit form). The same happens if you try to change the IP of a pre-existing host - you have to confirm the Overwrite when you save the host.

Analysis

We're struggling to come up with a scenario where we would care about leases with the same mac. Macs are a hardware identifier, and as such, if I see a lease for my mac with a different IP, I would interpret that as "My host is releasing the old IP and requesting a lease for the new IP". Since ISC's lease file is a last-lease-wins system, we don't even need to delete the old leases for this mac (although we should, ideally).

It's still true that we care about an IP conflict. In the opposite case to above, where we vary the mac and keep the same IP, we now have a record like [mac2, ip1]. I would interpret that as "My host (mac2) is requesting an IP that belongs to another host (mac1)". In this scenario, we do indeed need to flag this as a conflict, and raise it to the user for investigation.

Proposal

What we're proposing is to change the logic to (note that this can be written much more neatly, the below code is for illustration of the logic):

- conflicts = [proxy.record(network, mac), proxy.record(network, ip)].delete_if { |c| c  self }.compact
+ conflicts = [proxy.record(network, ip)].delete_if { |c| c == self }.compact

In other words, do not request leases-by-mac from the proxy during conflict detection, since we do not actually care about them. We might request leases-by-mac during the orchestration, since we would want to find and remove any leases not matching the current mac/ip pair.

Have we missed a usecase for mac conflicts, or does this sound sensible?

Actions #3

Updated by The Foreman Bot about 10 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/2022 added
  • Pull request deleted ()
Actions #4

Updated by Lukas Zapletal over 9 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100
Actions #5

Updated by Dominic Cleal over 9 years ago

  • Translation missing: en.field_release set to 28
Actions #6

Updated by Lukas Zapletal over 9 years ago

  • Related to Bug #9776: foreman does not show a warning if a dhcp reservation already exists and simply fails added
Actions #7

Updated by The Foreman Bot about 9 years ago

  • Pull request https://github.com/theforeman/foreman/pull/2941 added
Actions #8

Updated by Lukas Zapletal about 9 years ago

  • Related to Bug #12637: Improve DHCP lease unit test added
Actions #9

Updated by Ohad Levy almost 9 years ago

  • Related to Bug #13725: Orchestration fails with 409 when there is a MAC DHCP conflict added
Actions #10

Updated by Lukas Zapletal almost 9 years ago

  • Related to Bug #8538: ISC DHCP provider does not respect expired and free leases added
Actions

Also available in: Atom PDF