ananace / fog-hyperv

Hyper-V provider for fog
MIT License
8 stars 1 forks source link

Case sensitive MAC Address #21

Closed zyronix closed 6 years ago

zyronix commented 6 years ago

Hi there,

Been running into a bug with Hyper-V, dhcp-isc and Foreman 1.16. (All running latest version on Debian 9 or Windows Server 2016). Creating a host from the foreman interface works just as intended, the VM is created and the boots correctly and I am able to start the Debian preseed for the host. However, during the final step of the unattended installation, a finish script will run: calling the foreman api and telling it that the build has finished. During this step, foreman finilises the machine. One of the step that it does is delete the DCHP lease and recreate it removing the requirement for provising. This is where the bug occurs.

Hyper-V output the MAC in uppercase, while ISC-DHCP records the in lowercase. So whenever you require Foreman for the lowcase MAC address, the tell you that the mac address does not exist:

$ curl -k https://foreman.example.com:8443/dhcp/192.168.0.0/mac/00:15:5D:11:73:0F --cert /etc/puppetlabs/puppet/ssl/certs/foreman.example.com.pem --key /etc/puppetlabs/puppet/ssl/private_keys/foreman.example.com.pem
$ No DHCP record for MAC 192.168.0.0/00:15:5D:11:73:0F 

And if we request it with lower case:

curl -k https://foreman.example.com:8443/dhcp/192.168.0.0/mac/00:15:5d:11:73:0f --cert /etc/puppetlabs/puppet/ssl/certs/foreman.example.com.pem --key /etc/puppetlabs/puppet/ssl/private_keys/foreman.example.com.pem
{"name":"boyd-chafin.example.com","ip":"192.168.0.54","mac":"00:15:5d:11:73:0f","subnet":"192.168.0.0/255.255.255.0","type":"reservation","hostname":"boyd-chafin.example.com","deleteable":true,"hardware_type":"ethernet","filename":"pxelinux.0","nextServer":"192.168.0.1"}

But what happens when you sent a delete for this record that 'does not excist'?

$ curl -k -XDELETE https://foreman.example.com:8443/dhcp/192.168.0.0/mac/00:15:5D:11:73:0F --cert /etc/puppetlabs/puppet/ssl/certs/foreman.example.com.pem --key /etc/puppetlabs/puppet/ssl/private_keys/foreman.example.com.pem
$ echo $?
$ 0

Looking at the log file of the foreman proxy, you will see just a simple http 200 response for this call, but no communication will be done to the DHCP server. This is because Foreman 'thinks' that the record isn't there and thus not delete it. The next call after that is recreating the DHCP lease, but this time ISC-DHCP will tell you: 'no gonna happen, this lease already exist'. Resulting in the API returning an error and making the provision fail.

The problem here is that during the creation of the VM, dynamic mac address is disabled and a static mac address is assigned. Hyper-V determines the new mac address and then fog-hyperv requests the mac address using the 'Get-VMNetworkAdapter' cmdlet. This cmdlet always returns the mac in upercase:

Get-VMNetworkAdapter -VmName boyd-chafin.example.com  | select Id,ComputerName,Connected,Dynamic
MacAddressEnabled,IpAddresses,IsExternalAdapter,IsLegacy,IsManagementOs,MacAddress,Name,SwitchId,SwitchName,VmId,VmName

Id                       : <REMOVED>
ComputerName             : <REMOVED>
Connected                : True
DynamicMacAddressEnabled : False
IPAddresses              : {}
IsExternalAdapter        : False
IsLegacy                 : True
IsManagementOs           : False
MacAddress               : 00155D11730F
Name                     : Legacy Network Adapter
SwitchId                 : <removed>
SwitchName               : Default Switch
VMId                     : <removed>
VMName                   : boyd-chafin.example.com

Now, I will also reports this bug at the foreman project, because a mac address is NOT case sensitive, thus the API should not handle it with case sensitivy. However, you could also help with this; If this plugin would convert the mac-address to lowercase; there wouldn't be a problem. Some dirty hack I did to make this work is the following code in the network_adapter request:

module Fog
  module Compute
    class Hyperv
      class Real
        def get_vm_network_adapter(options = {})
          requires_one options, :vm_name, :all, :management_os
          data = run_shell('Get-VMNetworkAdapter', options)
          data[:mac_address] = data[:mac_address].downcase
          data
        end
      end

      class Mock
        def get_vm_network_adapter(args = {})
          requires_one args, :vm_name, :all, :management_os

          data = handle_mock_response(args)
          if args[:all]
            data
          elsif args[:vm_name]
            data.find { |i| i[:vm_name].casecmp(args[:vm_name]).zero? }
          elsif args[:management_os]
            data.find { |i| i[:is_management_os] }
          end
        end
      end
    end
  end
end

Maybe you have a beter (cleaner) solution for this?

ananace commented 6 years ago

Pushed a commit to foreman_hyperv, since that's where the case-insensitivity ends up being a problem.

Please try that change out and see if it solves your issue in a suitable way.

I'm trying to keep from putting Foreman specific workarounds in the Fog provider, instead just sticking to thin wrappers around the PowerShell commandlets as best as I can.

zyronix commented 6 years ago

Awesome, test and works. Within 6 minutes a working fix, kudos 👍. Foreman now indeed sees the mac as lowercase (notable in the interface).

The only thing which I am now sure about, might this be a problem for other supported DHCP servers? Like Windows DHCP? I don't have an environment to test that case.

ananace commented 6 years ago

I only have access to Infoblox DHCP for testing, and MAC addresses are treated in a case-insensitive manner by it. So I honestly have no idea if other DHCP servers would also fail on this.

Either way, really nice to hear that the solution works for you. Thanks for helping find these issues. :heart: