Project

General

Profile

Bug #26041

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

Added by Arend Lapere 8 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Network
Target version:
-
Difficulty:
easy
Triaged:
No
Bugzilla link:
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?

Associated revisions

Revision f4e55598 (diff)
Added by UXabre 7 months ago

Fixes #26041 - Only validate MAC-address if subnet is set

History

#1 Updated by Lukas Zapletal 8 months 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.

#2 Updated by Arend Lapere 8 months 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

#3 Updated by Lukas Zapletal 8 months 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.

#4 Updated by The Foreman Bot 8 months ago

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

#5 Updated by Timo Goebel 7 months ago

  • Fixed in Releases 1.22.0 added

#6 Updated by Anonymous 7 months ago

  • Status changed from Ready For Testing to Closed

Also available in: Atom PDF