Refactoring of fact parsers

Facts, one of some important things in The Foreman project. That piece of information that informs about facts of host. They can come from many programs, ie. Puppet, Ansible, Salt and so on.
Most integration with these programs are in plugins, however Puppet has historically exclusive place in The Foreman. Right now, there’s effort to make Puppet as option like others (Making Puppet optional). What is means for parsers? Well, since the parsers deal with the same thing (aka facts of the host), we decided to unify these parsers.

That means I opened PRs where I move parsers from Ansible plugin (fixes #32518 - Adding Ansible fact parser by domitea · Pull Request #8491 · theforeman/foreman · GitHub), Katello (Fix #32517 - Moving rhsm parser and importer to core by domitea · Pull Request #8508 · theforeman/foreman · GitHub), Salt plugin (Fixes 32548 - Moving salt parser and importer to core by domitea · Pull Request #8511 · theforeman/foreman · GitHub) and Chef plugin (Fix 32558 - Moving Chef parser and importer to Core by domitea · Pull Request #8512 · theforeman/foreman · GitHub). However, this is a small step for something bigger and more interesting - The Refactoring! If I move all parsers to one big pile then I can refactor them into much cleaner code. Some preparations started by @ekohl in Fixes #27906 - Always prefer modern facts in Facter by ekohl · Pull Request #8449 · theforeman/foreman · GitHub but it’s a beginning.

I wanted to start this thread because I need some sort of memo to avoid forget something important during refactoring. I know that @lzap has some great comments for that:

  1. Fictures, Fictures and Fictures. I mean the file that represents facts from some hosts for testing. This is very important point and I want to keep it in mind because it’s keeps integrity of parsers. @ekohl has similar thought and informed me about facterdb (GitHub - camptocamp/facterdb: A Database of OS facts provided by Facter) that’s ideal for that. I can test the parser for all facts in automatize way.
  2. Hooks. Discovery is a great plugin and it uses ImportHookService (Searching for the foreman_default_hostgroup plugin maintainer - #2 by lzap) to actually update the facts before they gets saved. I think It’s would be great addition because it could be used across plugins.

So, there It is, my thoughts about refactoring fact parsers. if It’s there anything I forgot, drop me a comment!

2 Likes

That facter db is awesome. If we are able to integrate that that would be dream. We will not need all of them, but some good picks would be great - I would be probably interested in different families, OS versions and network configurations (regular, VLAN, bridge, bond, team, bond with VLAN).

I am ready to do the review just let me know as long as PRs are ready, ideally off github :slight_smile:

One wish: If you could test everything with discovery plugin turned on. After each change, please do import a fake discovery host via extra/discover-host because discovery plugin is the first to break as it heavily relies on puppet parser and extends it quite a bit. I really hope this refactoring will help and we can even move discovery importer into core too (not the plugin yet).

Thanks for the writeup! Great stuff.