Project

General

Profile

Actions

Bug #26041

closed

Managed interface should not require a MAC address if no subnet selected

Added by Arend Lapere about 5 years ago. Updated about 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Network
Target version:
-
Difficulty:
easy
Triaged:
No
Fixed in Releases:
Found in Releases:

Description

So, in our scenario, we set up a smart proxy, which sets up an OpenVPN connection towards our foreman server.
This foreman server also manages this Smart-Proxy.

Currently our flow is like this:
- Provision smart proxy box with CentOS 7.6
- Run Ansible script to deploy foreman
- Ansible Callback tells foreman a "tun0"-interface was detected. (with no MAC address)
- Edit the smart proxy and set "tun0" as managed + REX & give it a name + domain

Expectation:
The A and AAAA record are created on the managed DNS, without having to provide a MAC-address

Reality:
The UI seems to complain that no MAC address has been selected. For now, I fill in 00:00:00:00:00:00 but that seems a bit rough on the edges, considering I don't have a subnet at all (which might sound absurd :p)
Also, since Ansible Callback created this, I can't select "Virtual NIC"; but again, that would not be correct either.

Pondering Moment:
From a reprovision point of view, it does not make sense to have a "tun"-interface as part of our provisioning scripts either; but that could IMO be fixed in the provisioning scripts to only create interfaces with a subnet and/or mac address?

Actions #1

Updated by Lukas Zapletal about 5 years ago

Arend, sup! While I agree that the task itself is easy, the problem is in our large codebase and templates. We currently expect that MAC address exists, there might be dragons. However you can start trying, just remove this line from nic/base.rb file:

    validates :mac, :presence => true,
              :if => Proc.new { |nic| nic.managed? && nic.host_managed? && !nic.host.compute? && !nic.virtual? }

And start testing this.

Actions #2

Updated by Arend Lapere about 5 years ago

Hey Lukas! Thanks for your input, it has proven to be very helpful! (sorry for getting back so late on this subject)
I've managed to play around a bit, and the following code seems to do the trick:

validates :mac, :presence => true,
              :if => Proc.new { |nic| nic.managed? && nic.host_managed? && !nic.host.compute? && !nic.virtual? && nic.provision? && (nic.subnet.present? || nic.subnet6.present?)}

The "beef" is in both checking for it being a provisioning interface (because, well, provisioning means DHCP, DHCP means MAC-address) but also checking if there's a subnet set at all (assuming that a subnet always needs a MAC-address... could probably be filtered down further in case no DHCP/IEU-64 is used)

However, seems like a lot of checks are happening for that property? Is this the way forward, or should I not split this up in a separate validation function?

Another thing, as you mentioned, the templates regarding networking (luckily, as far as i can tell, a snippet) needs to be adjusted to take into account checking for a MAC-address

(also, writing a bit before my turn because the tests are still running; the above is based on a manual test I've performed)

Kind regards,
Arend Lapere

Actions #3

Updated by Lukas Zapletal about 5 years ago

The change looks good, no need to split this into another method, just file a PR and maybe an extra test for this case would be required.

Templates need to be changed only when host.mac returns nil, which is only if the provisioning interface has mac set to nil. And you can go ahead and patch Foreman core first and only then make necessary changes in community-templates repo.

Actions #4

Updated by The Foreman Bot about 5 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/6540 added
Actions #5

Updated by Timo Goebel about 5 years ago

  • Fixed in Releases 1.22.0 added
Actions #6

Updated by Anonymous about 5 years ago

  • Status changed from Ready For Testing to Closed
Actions

Also available in: Atom PDF