Project

General

Profile

Bug #8727

Discovered hosts with same MAC address are reported as DHCP conflicts

Added by Lukas Zapletal almost 7 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Category:
Orchestration
Target version:
Difficulty:
Triaged:
Bugzilla link:

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

Related to Foreman - Bug #9776: foreman does not show a warning if a dhcp reservation already exists and simply failsRejected2015-03-16
Related to Foreman - Bug #12637: Improve DHCP lease unit testClosed2015-11-30
Related to Foreman - Bug #13725: Orchestration fails with 409 when there is a MAC DHCP conflictClosed2016-02-16
Related to Smart Proxy - Bug #8538: ISC DHCP provider does not respect expired and free leasesClosed2014-12-01

Associated revisions

Revision 37bb1b44 (diff)
Added by Lukas Zapletal over 6 years ago

Fixes #8727 - DHCP validation does not fail on discovered leases

Revision 5ce30897 (diff)
Added by Lukas Zapletal over 6 years ago

Fixes #8727 - DHCP validation does not fail on discovered leases

(cherry picked from commit 37bb1b44a9753382c6858e22ac55031e9662c4e4)

History

#1 Updated by Lukas Zapletal almost 7 years ago

  • Bugzilla link set to 1170518

#2 Updated by Greg Sutcliffe almost 7 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?

#3 Updated by The Foreman Bot almost 7 years ago

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

#4 Updated by Lukas Zapletal over 6 years ago

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

#5 Updated by Dominic Cleal over 6 years ago

  • Legacy Backlogs Release (now unused) set to 28

#6 Updated by Lukas Zapletal over 6 years ago

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

#7 Updated by The Foreman Bot about 6 years ago

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

#8 Updated by Lukas Zapletal about 6 years ago

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

#9 Updated by Ohad Levy almost 6 years ago

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

#10 Updated by Lukas Zapletal almost 6 years ago

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

Also available in: Atom PDF