RFC: Drop parsing NIC interfaces

Sorry again, I meant what data do they provide. Foreman does parse interface data provided by e.g. ansible, doesn’t it?

For example in Ansible, we only parse basic information: regular interface (IP and MAC address).

I am currently looking into how to get additional information about interfaces in Linux and I am afraid this can’t be easily done in Ruby:

http://man7.org/linux/man-pages/man7/netdevice.7.html

To get extra flags, ioctl must be done with union within struct as a parameter. I think it could be easier to write a simple command line application which would give required data and call that from facter.

So I was able to write a small utility which talks to kernel via netlink and acquires NIC link data. Two regular interfaces:

link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:dd:ee:ff
link.enp7s0.type => device
link.lo.type => device

A simple bond:

link.bond0.bond.mode => active-backup
link.bond0.mac => 52:54:00:aa:bb:cc
link.bond0.type => bond
link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.master => bond0
link.enp1s0.slave => bond
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:aa:bb:cc
link.enp7s0.master => bond0
link.enp7s0.slave => bond
link.enp7s0.type => device
link.lo.type => device

Team:

link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.master => team0
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:aa:bb:cc
link.enp7s0.master => team0
link.enp7s0.type => device
link.lo.type => device
link.team0.mac => 52:54:00:aa:bb:cc
link.team0.type => team

Bridge with two slaves:

link.br0.mac => 52:54:00:aa:bb:cc
link.br0.type => bridge
link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.master => br0
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:dd:ee:ff
link.enp7s0.master => br0
link.enp7s0.type => device
link.lo.type => device

Bridge and regular:

link.br0.mac => 52:54:00:dd:ee:ff
link.br0.type => bridge
link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:dd:ee:ff
link.enp7s0.master => br0
link.enp7s0.type => device
link.lo.type => device

VLAN (output is currently dot-separated JSON is planned):

link.enp1s0.10.mac => 52:54:00:aa:bb:cc
link.enp1s0.10.parent => enp1s0
link.enp1s0.10.type => vlan
link.enp1s0.10.vlan.id => 10
link.enp1s0.10.vlan.protocol => 802.1q
link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:dd:ee:ff
link.enp7s0.type => device
link.lo.type => device

Finally bond with VLAN:

link.bond0.10.mac => 52:54:00:aa:bb:cc
link.bond0.10.parent => bond0
link.bond0.10.type => vlan
link.bond0.10.vlan.id => 10
link.bond0.10.vlan.protocol => 802.1q
link.bond0.bond.mode => active-backup
link.bond0.mac => 52:54:00:aa:bb:cc
link.bond0.type => bond
link.enp1s0.mac => 52:54:00:aa:bb:cc
link.enp1s0.master => bond0
link.enp1s0.slave => bond
link.enp1s0.type => device
link.enp7s0.mac => 52:54:00:aa:bb:cc
link.enp7s0.master => bond0
link.enp7s0.slave => bond
link.enp7s0.type => device
link.lo.type => device

Please take a look on this output and let me know if we are able to reconstruct all the types from this data. To me, it looks like this is enough! When confirmed, I am able to write a custom fact that runs this tool and reports the data back via facter.

At first glance it looks correct to reconstruct the NIC setup. What I really like about this is that it allows users to set their own NIC names (external, lan) which are actually bonds or even a VLAN on top of a bond. As you correctly concluded the current heuristics would never allow this. Looking forward to seeing this.

Let me know if you need help with shaping this into a custom fact.

1 Like

Thanks, I tried pure ruby but the netlink wrapper library did not work and it was very low-level anyway (too much of an effort to implement this). Also not sure is this kind of dependency would be accepted upstream.

In this design, the custom fact can simply search for /usr/bin/link_tool_whatever and if it is installed, facts will be provided. If not, it can be simply skipped. That should work fine.

Perhaps it should be implemented as an external fact.

1 Like

I think this conversation is about different feature though. I agree we should be able to deploy. But what’s disucssed here is, whether we want to reverse engineer the datacenter. E.g. if I deploy Foreman to brownfield, do I just deploy facter and get the overview of my bonds and vlans? Or do I have to manually setup everything in Foreman forms.

It is and that’s why the code is so complicated. If there’s no simple way and the current implementation shows it’s cause more problems than good, let’s get rid of it.

I’m afraid this won’t solve the main issue. We’d import subset of interfaces, but they would still be bond/vlans or whatever you have without us being able to tell. Therefore all validations like MAC uniqueness would cause issues.

yeah I think that is a good idea, instead of “I see hundred irelevant NICs”, we’ll be getting “I don’t see my vlan called custom01” which is definitely better for the user

I believe this is what we should aim for. The tool looks great and could be integrated in all kinds of config management or fact reporting solutions. Would we package it as a rpm and deploy it in the foreman-client repo?

Yeah, since this looks as reliable way to detect all information we need, I wouldn’t be against keeping parsing in. We’d still need to offer whitelisting as @bagasse suggested. However I’d like to better understand how much work it is to build and support that tool in all platforms we support in foreman-client. I agree it would be nice to integrate it with all facters, so the logic does not have to be implemented in every facter.

Would this even work in subscription-manager?

I think it makes a lot of sense to build on top of facter, due to history of Foreman. Therefore we should:

  • make a decision to build on top of facter as the defacto standard for all facts in Foreman
  • extend facter 2, 3 and NG with network link information
  • define minimum set of facts Foreman understand and document it
  • create transformation core API so other fact parsers can easily turn their format into our stanard

Let me go one after the other points now.

Although I was a bit against facter, I think it makes a lot of sense to build on top of it. I think we all agree here.

Extending with network link info (bond, vlans) is already in the works, I have a prototype external facts, currently working on rough edges and want to start on Foreman part soon.

Defining minimum set of facts Foreman understand is the most important one I think. Even facter NG (next generation) is not perfect, I don’t like how it throws all facts into one heap - volatile ones (e.g. free memory, uptime) with stable ones (available memory, boot time). It also reports huge amount of facts, we should cherry pick them throwing irrelevant away (these could be enabled by user via settings perhaps - whitelisting mentioned above). We should define minimum set we support (based on facter 3/NG facts plus some extra - e.g. network link we discussed above), we should define which facts are considered stable and which volatile and then refactor our parsing code in a clean way.

A good API for plugin authors is viable to get this into the right direction. Huge advantage is that user is presented with just one set of facts e.g. networking.interfaces.eth0.bindings6.ipaddress and this remains the same no matter which client reports this: Ansible, Facter or RHSM.

Here is a question for you guys.

Consider a system with interface eth0 with IPv4 and the default IPv4 route and eth1 with IPv6 and the default IPv6 route.

Which interface should be considered as the primary?

Facter at the moment only reports “primary” interface by executing ip route get 1, therefore it ignores IPv6. That’s not future proof enough IMHO.

I’d check IPv6 first, IPv4 second.

2 Likes

So, I have the tool ready and if there are no objections, I will start a core PR that would remove heuristics we do over network identifiers and use the information from a custom fact:

Facter.add(:link) do
  setcode do
    json = `ufacter -json -modules link`
    JSON.parse(json)['link']
  end
end

We need to ship binary called ufacter which I wrote to provide netlink NIC information which then goes into facter. I tried to use external fact but it did not work out - these can only be simple key-value facts, not structured facts.

The idea behind micro facter is that it’s a tool that is used by facter to provide extra facts we need to reconstruct NICs. But users could also use it as a standalone tool if they want if we define good enough set of minimal viable facts which are needed to properly register a host. Then facter can take over once the host is (re)-provisioned.

1 Like

You should use Facter::Core::Execution.execute instead of backticks (see https://puppet.com/docs/facter/3.6/custom_facts.html#executing-shell-commands-in-facts) and probably confine it to the Linux kernel (confine :kernel => 'Linux')

1 Like

Nice, could this be integrated into subscription manager? If used as a standalone tool, how could a host send the data to Foreman?

Good idea, indeed. I think the first step would be to create a foreman client repository package (it’s written in GoLang with minimum dependencies so should be piece of cake) and then we can start integrating it as well. I think in case of RHSM it could be easier to just use a native library (https://pypi.org/project/pyroute2/) tho.

Certificate would need to be bootstrapped. Or discovered hosts API could be used to register the host as well, there is no certificate needed for this workflow.

Will do.

Is my assumption correct that dropping the custom fact into a directory and configuring /opt/puppetlabs/etc/facter/facter.conf would be enough to get the host reporting this with every fact upload triggered via puppet run?

Shall we configure this in the ufacter.rpm (possibly a subpackage or better something like foreman-facts.rpm) or via puppet itself? I mean, this could be chicken and egg problem.

I think that the best way would be to create a package called foreman-facts which would require /usr/bin/facter so it could be fullfilled with either puppetlabs facter or any other distribution of facter and would require ufacter tool as well. The contents of the package would be the custom fact file and post scriplet that would modify the puppet configuration (not owning the file). Once puppet takes over it can overwrite the configuration as needed (and rpm file update would not break it).

The same for debian.

I am thinking about completely new (from scratch) fact API. A module (plugin) would be responsible for:

  • Detecting if arbitrary JSON is in particular format (to find which module would provide parsing).
  • Transforming architecture, OS family, name into a common name (e.g. “redhat” into “Red Hat”).
  • Transforming arbitrary JSON into common model (CFM).

If the CFM is well defined, there is no need of separate fields for os, architecture or primary interface as this should all be defined by the CFM (based on puppet facter for the most part). For example, primary IPv4 interface in CFM would be cfm[:primary] (that’s from facter) and primary IPv6 interface would be simply cfm[:primary6].

Foreman could then store those CFM facts into the current model, it could also store the original facts alongside into text/json field (not searchable) and by simple iterating over cfm[:interfaces] it would create NIC models and the rest of the data stored separately (CPUs, memory).