foreman.py will only import 250 systems when using host_filters

[rhjhunt]

ISSUE TYPE

• Bug Report

COMPONENT NAME

• API
• UI

SUMMARY

When using Satellite 6 as a dynamic inventory, and with the source varibles set as the following.

satellite6_want_hostcollections: True host_filters: not (host_collection="virt-who" or host_collection="decomm" or host_collection="unmanaged")

Only 250 of my systems out of 1000 will be imported into the inventory. If I don't set the want hostcollections and host_filters all 1000 systems are imported.

ENVIRONMENT

• AWX version: 3.3.2
• Ansible version: 2.7.2
• Operating System: RHEL 7.6
• Web Browser: Chrome 71

STEPS TO REPRODUCE

Configure Satellite 6 as a dynamic inventory Modify the source variables and add:

satellite6_want_hostcollections: True host_filters: not (host_collection="virt-who" or host_collection="decomm" or host_collection="unmanaged") Run the job to sync the dynamic inventory

EXPECTED RESULTS

All systems are imported

ACTUAL RESULTS

Only 250 systems are imported

ADDITIONAL INFORMATION

I suspect it is from this in the foreman.py, because if I change the number to 1000 all my hosts are imported.

   def _get_json(self, url, ignore_errors=None, params=None):
        if params is None:
            params = {}
        params['per_page'] = 250

        page = 1
        results = []
        s = self._get_session()
        while True:
            params['page'] = page
            ret = s.get(url, params=params)
            if ignore_errors and ret.status_code in ignore_errors:
                break
            ret.raise_for_status()
            json = ret.json()
            # /hosts/:id has not results key
            if 'results' not in json:
                return json
            # Facts are returned as dict in results not list
            if isinstance(json['results'], dict):
                return json['results']
            # List of all hosts is returned paginaged
            results = results + json['results']
            if len(results) >= json['subtotal']:
                break
            page += 1
            if len(json['results']) == 0:
                print("Did not make any progress during loop. "
                      "expected %d got %d" % (json['total'], len(results)),
                      file=sys.stderr)
                break
        return results

[kdelee]

@wenottingham do we want to file a bug in the ansible repo?

[ryanpetrello]

Yep, this looks like it might be a bug upstream in Ansible.

[awithrow9]

@unxfrek is this coming from a customer?

[rhjhunt]

@awithrow9 yes I was working with a customer on this and I was able to reproduce.

[rhjhunt]

@awithrow9 should I open this issue against ansible rather than AWX?

Also as a workaround, would it be viable to edit /var/lib/awx/venv/awx/lib/python2.7/site-packages/awx/plugins/inventory/foreman.py and change the '250' to the amount in their inventory? (e.g. 1000). That setting would need to be modified on each tower node if they are clustered correct?

[awithrow9]

@unxfrek, yes can you also open this up in Ansible.

@AlanCoding, @kdelee are we using the foreman plugin? If not, why?

[AlanCoding]

We are not using the foreman inventory plugin because we do not have a satellite6 server for integration testing.

In terms of our readiness to implement it, core merged this so we can use environment vars for credentials, so we're good for that part of the contract. The configuration options do not give any real reason for concern, it'll take some work. Right now, I don't see any parameters that have no equivalent in the plugin and we also allowed users to use in the script.

I can't say anything about the content it returns until I test it.

[ryanpetrello]

So regardless, we need this fixed upstream in ansible first before we can pull it into AWX. Do we have any evidence that the upstream foreman plugin fixes this issue? We should probably track this ticket as part of the work whenever we move to support Sat6 via plugins.

Given timing and that this would potentially need to land in Ansible first, I'd say this should be cut cc @wenottingham

[wenottingham]

The relevant code in the plugin looks pretty much the same.

[ryanpetrello]

@wenottingham I vote we kick this out and work on it as part of "Finish up inventory plugins for all sources" later(do we have a ticket for that?)

[wenottingham]

No ticket yet, but I can concur if we don't have a test nor fix.

[ryanpetrello]

@jladdjr if you're not tracking it already this is also something we can attempt to verify once we switch to the plugin

[blomquisg]

@jladdjr did you have time to look at this?

[jladdjr]

@blomquisg yep, will take a look 👍

[ryanpetrello]

@elyezer @squidboylan @kdelee is this something any of you and yours have bandwidth to test now that we've merged the plugin?

If it doesn't "just work" in the new plugin, we're unlikely to fix it right now.

cc @blomquisg

[kdelee]

Yeah I would say we need to check to see if plugin ever got the reports feature ported to it that the script had -- because that was the fix, and then we need to use it.

[kdelee]

I have not been able to reproduce. Given the age of this issue, and our move to the inventory plugins, I propose that this be closed and if anyone is able to reproduce with the plugin that they file a bug in https://github.com/theforeman/foreman-ansible-modules/ because now we rely on what is in https://github.com/theforeman/foreman-ansible-modules/blob/master/plugins/inventory/foreman.py