Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement contructable support for opennebula inventory plugin: keyed… #4524

Conversation

bsanders
Copy link
Contributor

@bsanders bsanders commented Apr 18, 2022

…_groups, compose, groups

SUMMARY

#4497 noted that the opennebula inventory plugin inherits Constructable, and the doc string also includes the macro(?) for that, however the code does not implement the standard dynamic inventory features such as keyed_groups, etc. This PR aims to implement that functionality.

I have included a new unit test, as well as new test-fixture data and re-worked some of the helper logic in the tests. Hopefully @feldsam is ok with the changes. I would be happy to refactor the pre-existing test and move the pre-existing sample data to the external file to make it match what I did.

Note, the PR is incomplete as not all of the testing logic passes, see below.

Fixes #4497

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

opennebula inventory plugin

ADDITIONAL INFORMATION

The code currently produces the output I would expect, however the unit test I added fails due to a traceback. I looked briefly into this, but wasn't quite sure why it was failing. The two test_ functions I added are identical except that one of them has commented out portions that are currently failing here:

[gw1] linux -- Python 3.8.0 /usr/bin/python3.8
inventory = <ansible_collections.community.general.plugins.inventory.opennebula.InventoryModule object at 0x7f1ceb5ce5b0>
mocker = <pytest_mock.plugin.MockerFixture object at 0x7f1ceb5ce6d0>

    def test_populate_constructable_templating(inventory, mocker):
        # bypass API fetch call
        inventory._get_vm_pool = mocker.MagicMock(side_effect=get_vm_pool_json)
        inventory.get_option = mocker.MagicMock(side_effect=mk_get_options(options_constructable_test))
>       inventory._populate()

tests/unit/plugins/inventory/test_opennebula.py:288: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
plugins/inventory/opennebula.py:239: in _populate
    self._add_host_to_composed_groups(self.get_option('groups'), server, hostname, strict=strict)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <ansible_collections.community.general.plugins.inventory.opennebula.InventoryModule object at 0x7f1ceb5ce5b0>
groups = {'benchmark_clients': "TGROUP.endswith('clients')", 'lin': 'is_linux == True'}
variables = {'GUEST_OS': 'linux', 'INPUTS_ORDER': '', 'LABELS': ['foo', 'bench'], 'LOGO': 'images/logos/linux.png', ...}
host = 'terraform_demo_00', strict = False, fetch_hostvars = True

    def _add_host_to_composed_groups(self, groups, variables, host, strict=False, fetch_hostvars=True):
        ''' helper to create complex groups for plugins based on jinja2 conditionals, hosts that meet the conditional are added to group'''
        # process each 'group entry'
        if groups and isinstance(groups, dict):
            if fetch_hostvars:
                variables = combine_vars(variables, self.inventory.get_host(host).get_vars())
>           self.templar.available_variables = variables
E           AttributeError: 'InventoryModule' object has no attribute 'templar'

/root/ansible/lib/ansible/plugins/inventory/__init__.py:369: AttributeError

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added bug This issue/PR relates to a bug cloud inventory inventory plugin needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) tests tests unit tests/unit labels Apr 18, 2022
@felixfontein
Copy link
Collaborator

templar isn't available in the unit tests because nobody created it. The inventory plugin for that test is created by

@pytest.fixture(scope="module")
def inventory():
    r = InventoryModule()
    r.inventory = InventoryData()
    return r

and that doesn't set templar.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-4 labels Apr 18, 2022
@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Apr 19, 2022
@bsanders
Copy link
Contributor Author

I noticed a few things while developing this. Tags/Attributes in opennebula are forced to upper case. Meaning if you use some other process to create them (Terraform, or the webui), create them as lower case, then do not check what they actually are now, they will be upper case. Labels, on the other hand are case-preserving. Also the variables we create and add to inventory as host variables are not forced uppercase. Not sure if it's worth noting that in the plugin docs, now that you can use them there. I'm new to open nebula, so I'm not sure what the surprise factor would be here.

I was surprised to see Ansible still supporting Python 2.x, let alone python 2.6. It should probably be noted that this module is unlikely to work on 2.6 - the underlying PyONE library doesn't support it. Again, dunno if it's appropriate to note that in the docs somewhere or not.

@feldsam do you want me to refactor the older test data into a file in the fixtures folder? You kept it as python, but the OrderedDict is the only non-json-able bit, and it doesn't seem super important here. Then all the data would be in one place, and the test code would be shorter/clearer.

…o-match-documentation.yaml

Co-authored-by: Felix Fontein <[email protected]>
@bsanders
Copy link
Contributor Author

Thought I'd drop in some before and after output. Here's a plugin config file:

% cat opennebula.yml
plugin: community.general.opennebula
api_url: "https://opennebula:2634/RPC2"
# specify these here, or in the form username:password in ~/.one/one_auth
#api_username:
#api_password:
group_by_label: 'foo'
filter_by_label: 'bench'
keyed_groups:
  - prefix: tgroup
    key: TGROUP
groups:
  benchmark_clients: TGROUP.endswith('clients')
  lin: is_linux == True
compose:
  is_linux: GUEST_OS == 'linux'
% ansible-inventory --list -i opennebula.yml
{
    "_meta": {
        "hostvars": {
            "terraform_demo_00": {
                "GUEST_OS": "linux",
                "INPUTS_ORDER": "",
                "LABELS": [
                    "foo",
                    "bench"
                ],
                "LOGO": "images/logos/linux.png",
                "MEMORY_UNIT_COST": "MB",
                "SCHED_REQUIREMENTS": "ARCH=\"x86_64\"",
                "TGROUP": "bench_clients",
                "ansible_host": "192.168.11.248",
                "name": "terraform_demo_00",
                "v4_first_ip": "192.168.11.248",
                "v6_first_ip": false
            },
            "terraform_demo_01": {
                "GUEST_OS": "linux",
                "INPUTS_ORDER": "",
                "LABELS": [
                    "foo",
                    "bench"
                ],
                "LOGO": "images/logos/linux.png",
                "MEMORY_UNIT_COST": "MB",
                "SCHED_REQUIREMENTS": "ARCH=\"x86_64\"",
                "TESTATTR": "testvar",
                "TGROUP": "bench_clients",
                "ansible_host": "192.168.11.241",
                "name": "terraform_demo_01",
                "v4_first_ip": "192.168.11.241",
                "v6_first_ip": false
            },
            "terraform_demo_srv_00": {
                "GUEST_OS": "linux",
                "INPUTS_ORDER": "",
                "LABELS": [
                    "serv",
                    "bench"
                ],
                "LOGO": "images/logos/linux.png",
                "MEMORY_UNIT_COST": "MB",
                "SCHED_REQUIREMENTS": "ARCH=\"x86_64\"",
                "TGROUP": "bench_server",
                "ansible_host": "192.168.11.247",
                "name": "terraform_demo_srv_00",
                "v4_first_ip": "192.168.11.247",
                "v6_first_ip": false
            }
        }
    },
    "all": {
        "children": [
            "bench",
            "foo",
            "serv",
            "ungrouped"
        ]
    },
    "bench": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01",
            "terraform_demo_srv_00"
        ]
    },
    "foo": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01"
        ]
    },
    "serv": {
        "hosts": [
            "terraform_demo_srv_00"
        ]
    }
}

After changes

% ansible-inventory --list -i opennebula.yml
{
    "_meta": {
        "hostvars": {
            "terraform_demo_00": {
                "GUEST_OS": "linux",
                "INPUTS_ORDER": "",
                "LABELS": [
                    "foo",
                    "bench"
                ],
                "LOGO": "images/logos/linux.png",
                "MEMORY_UNIT_COST": "MB",
                "SCHED_REQUIREMENTS": "ARCH=\"x86_64\"",
                "TGROUP": "bench_clients",
                "ansible_host": "192.168.11.248",
                "is_linux": true,
                "name": "terraform_demo_00",
                "v4_first_ip": "192.168.11.248",
                "v6_first_ip": false
            },
            "terraform_demo_01": {
                "GUEST_OS": "linux",
                "INPUTS_ORDER": "",
                "LABELS": [
                    "foo",
                    "bench"
                ],
                "LOGO": "images/logos/linux.png",
                "MEMORY_UNIT_COST": "MB",
                "SCHED_REQUIREMENTS": "ARCH=\"x86_64\"",
                "TESTATTR": "testvar",
                "TGROUP": "bench_clients",
                "ansible_host": "192.168.11.241",
                "is_linux": true,
                "name": "terraform_demo_01",
                "v4_first_ip": "192.168.11.241",
                "v6_first_ip": false
            },
            "terraform_demo_srv_00": {
                "GUEST_OS": "linux",
                "INPUTS_ORDER": "",
                "LABELS": [
                    "serv",
                    "bench"
                ],
                "LOGO": "images/logos/linux.png",
                "MEMORY_UNIT_COST": "MB",
                "SCHED_REQUIREMENTS": "ARCH=\"x86_64\"",
                "TGROUP": "bench_server",
                "ansible_host": "192.168.11.247",
                "is_linux": true,
                "name": "terraform_demo_srv_00",
                "v4_first_ip": "192.168.11.247",
                "v6_first_ip": false
            }
        }
    },
    "all": {
        "children": [
            "bench",
            "benchmark_clients",
            "foo",
            "lin",
            "serv",
            "tgroup_bench_clients",
            "tgroup_bench_server",
            "ungrouped"
        ]
    },
    "bench": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01",
            "terraform_demo_srv_00"
        ]
    },
    "benchmark_clients": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01"
        ]
    },
    "foo": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01"
        ]
    },
    "lin": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01",
            "terraform_demo_srv_00"
        ]
    },
    "serv": {
        "hosts": [
            "terraform_demo_srv_00"
        ]
    },
    "tgroup_bench_clients": {
        "hosts": [
            "terraform_demo_00",
            "terraform_demo_01"
        ]
    },
    "tgroup_bench_server": {
        "hosts": [
            "terraform_demo_srv_00"
        ]
    }
}

@felixfontein
Copy link
Collaborator

@feldsam does this look good to you?

@feldsam
Copy link
Contributor

feldsam commented Apr 21, 2022

Hello, thank you for contribution!

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Apr 21, 2022
@felixfontein felixfontein merged commit 8e72e98 into ansible-collections:main Apr 21, 2022
@patchback
Copy link

patchback bot commented Apr 21, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/8e72e98adbf9ad0b16523605ca6c1d0deff170c6/pr-4524

Backported as #4549

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Apr 21, 2022
#4524)

* Implement contructable support for opennebula inventory plugin: keyed_groups, compose, groups

* Fixed templating mock issues in unit tests, corrected some linting errors

* trying to make the linter happy

* Now trying to make python2.7 happy

* Added changelog fragment

* changelog fragment needs pluralization

* Update changelogs/fragments/4524-update-opennebula-inventory-plugin-to-match-documentation.yaml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 8e72e98)
@felixfontein
Copy link
Collaborator

@bsanders thanks a lot for implementing this!
@feldsam thanks a lot for reviewing!

felixfontein pushed a commit that referenced this pull request Apr 21, 2022
#4524) (#4549)

* Implement contructable support for opennebula inventory plugin: keyed_groups, compose, groups

* Fixed templating mock issues in unit tests, corrected some linting errors

* trying to make the linter happy

* Now trying to make python2.7 happy

* Added changelog fragment

* changelog fragment needs pluralization

* Update changelogs/fragments/4524-update-opennebula-inventory-plugin-to-match-documentation.yaml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 8e72e98)

Co-authored-by: Bill Sanders <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug cloud inventory inventory plugin new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OpenNebula dynamic inventory plugin does not support keyed_groups
4 participants