Foreman Inventory Plugin fails


#1

Problem:
Host parameters are not recognized by ansible and therefore defaults are set.

Expected outcome:
Host parameters define ansible variables.

Foreman and Proxy versions:
Both 1.19.1

Ansible version:
2.7.5

Other relevant data:
Since Ansible 2.6 there is a foreman inventory plugin which replaces the formerly used ansible inventory plugin from foreman.

It seems that the issue seems lines 222 - 227 and / or 165 - 170. The ‘set_variables’ function gets somehow wrong datatypes it seems. So the keys are corrupted and not recognized by ansible.
In Python the results from __get_json look like this:

[{u'comment': u'', u'subscription_global_status': 0, u'environment_id': None, u'managed': False, u'content_facet_attributes': {u'content_view_id': 22,....

So basically it’s a Python list containing json elements. My idea was that Python’s unicode format could be the problem here.

Mfg Martin


#2

It looks like unicode shouldn’t matter:

$ python2
>>> {'comment': True, u'comment': False}
{'comment': False}
$ python3
>>> {'comment': True, u'comment': False}
{'comment': False}

What I do find odd is this part:

It returns element 0 but according to API documentation the structure is:

{
  "all_parameters": [
    {
      "priority": null,
      "created_at": "2018-11-15 19:01:27 UTC",
      "updated_at": "2018-11-15 19:01:27 UTC",
      "id": 513706444,
      "name": "loc_param",
      "value": "abc"
    },
    {
      "priority": null,
      "created_at": "2018-11-15 19:01:27 UTC",
      "updated_at": "2018-11-15 19:01:27 UTC",
      "id": 32400255,
      "name": "org_param",
      "value": "xyz"
    },
    {
      "priority": null,
      "created_at": "2018-11-15 19:01:27 UTC",
      "updated_at": "2018-11-15 19:01:27 UTC",
      "id": 636252244,
      "name": "test",
      "value": "myvalue"
    }
  ]
}

To me that suggests that it’s getting the very first parameter and iterating over the properties of that parameter. Looks like something like this is needed:

def _get_all_params_by_id(self, hid):
    url = "%s/api/v2/hosts/%s" % (self.foreman_url, hid)
    ret = self._get_json(url, [404])
    if not ret or not isinstance(ret, MutableMapping) or not ret.get('all_parameters', []):
        ret = {'all_parameters': []} 
    return {parameter['name']: parameter['value'] for parameter in ret['all_parameters']}

Disclaimer: I only read documentation and source code. None of this has been tested.

Another note: I don’t understand why this script isn’t getting /hosts lists with include=all_parameters and avoid n API calls where n is the number of hosts. Probably a massive performance boost. Looks like this parameter was added in Foreman 1.14.


#3

The mentioned part was looking odd to me as well, it simply doesn’t make sense. The function _get_json is building a whole list with multiple API calls which results in a large structure and with our full setup in a running time about 3min.

My first approach was to return the whole structure which broke the items() Iteration in line 223. I tried this construct next:

for param in self._get_all_params_by_id(host['id']):
                    try:
                        self.inventory.set_variable(host['name'], param['name'], { param['value'] })

At first it seemed to work but not all Parameters fit into a dict here. Without coding it from scratch it would be the easiest theoretical way to fetch the whole host list and working with the Python script rather to make so many calls. So I agree with you on this point as well.

I’m not getting the final hint what data structure self.inventory.set_variable is expecting. It looks to me that the first parameter host[‘name’] identifies the sub-structure for the specific host in the inventory object and then simple key -> value parsing is done.


#4

Update: The changes I’ve made and your idea are matching. I was stuck on the next issue, that the script produces the following.

Target: “AllowUsers user1 user 2”
Acutal State: “AllowUsers u s e r 1 u s e r 2”


#5

That typically happens when you use ' '.join('user1 user2') instead of ' '.join(['user1', 'user2']). Both are iterables and it’s very easy run into this with Ansible is my experience because it’s not very strict with data types.


#6

That was my assumption too. This join is done in the Jinja templates as far as I know.
It was just weird that it worked before.


#7

It could be that before it was just empty and not executed at all or provided as a list via another way. The Jinja template filter join(string) is essentially string.join so it’s also very easy to trigger there.


#8

Thank you for your support, it gave me a bunch of ideas to solve this problem.

Basically I dropped the whole Inventory Script and wrote it from scratch using “all_parameters” and some value-parsing-magic to achieve the best results.

At the moment I’m polishing my code and extend it to work with Redis caching and fact caching. After that it would make sense to establish an direct contact to the Ansible team to get it asap into upstream.


#9

I did try to make some changes, but they’re still untested:


#10

Thank you for your suggestions, I used some ideas in my script. It is working now in our environment but cache is not implemented yet.

One question…the _meta object sppeds up the inventory because Ansible does not call every single host the documentation says. But I didn’t find any further information why and how it works.


#11

Sadly that’s out of my Ansible knowledge.