Fixes #29378 - DHCP conflict on hardware_type #753
Conversation
| @@ -80,6 +80,8 @@ def to_reservation(parsed_host) | |||
| name = parsed_host.name | |||
| ip_address = parsed_host.node_attributes.delete(:fixed_address) | |||
| mac_address = parsed_host.node_attributes.delete(:hardware_address) | |||
| # delete unwanted attributes which would break equality and cause conflicts | |||
| parsed_host.node_attributes.delete(:hardware_type) # only ethernet is supported | |||
There was a problem hiding this comment.
Not too familiar with it, but is infiniband the same hardware type? I believe it is also ethernet, but having difficulty finding a good source on this. @ananace perhaps you know?
Just thinking out loud and I don't know the answer to this: if hardware type is not ethernet, should we just ignore it and return nil since we're certain it won't be a conflict?
There was a problem hiding this comment.
InfiniBand is a bit magical in that regard, the hardware runs as RDMA by default, but many IB cards can be run as regular high-speed ethernet adapters as well if you specify the correct boot options - or in some cases if you simply attach them to an ethernet switch.
InfiniBand running as RDMA has no concept of DHCP or IPs - unless you add an IP-over-InfiniBand extension, which is done as a statically configured SDN layer - and IB cards running as regular ethernet adapters are basically identical to any other high-speed ethernet adapter in regards to the network layer.
In this case though I don't know if any of that is relevant, as the ISC documentation seems to not note any particular knowledge of IB or other such things;
Currently, only the ethernet and token-ring types are recognized, although support for a fddi hardware type (and others) would also be desirable.
Personally - from quickly glancing over the code and docs - I'd assume that the most reasonable thing to do here is to just filter out anything that's not hardware_type ethernet and then remove the attribute.
There was a problem hiding this comment.
Thanks for opinions. So you are saying that we should remove setting omcmd "set hardware-type = 1" # This is ethernet completely? I am not sure about this one because this can be quite a challenge in regard to the upgrade process. We'd need to migrate reservations.
Or are you saying that my proposal is what we want? This seems to me a bit easier - we just ignore this flag completely when loading this from configuration files.
There was a problem hiding this comment.
Rather than deleting the attribute, I was suggesting that you return nil if the type isn't ethernet.
Edit: and as @ananace said, then delete the attribute.
lzap
force-pushed
the
dhcp-options-equality-29378
branch
from
45bfbea
to
cd6ee3c
Compare
|
Ok, I have rewritten the patch. All reservations which are not type of "ethernet" are ignored now. |
Equality of DHCP records is poorly defined as arbitrary hash named "options" is used. Various implementations parse/fetch various DHCP options into the hash while SmartProxy DHCP API only defines few options (hostname, filename, ip, mac). In the reported problem, ISC DHCP configuration file parser also adds "hardware_type" option (set to "ethernet") while newly created DHCP record don't have this option. Comparsion of the two same DHCP records fails which lead to DHCP conflict.
The solution is either to add hardware_type to all DHCP Record instances for ISC DHCP or to remove it when data is parsed, because only "ethernet" hardware type is supported at the moment. This is hardcoded in the code, therefore I lean towards the latter solution.