search

nuagenetworks / vspk-ansible

An ansible module to manage Nuage VSP environments
BSD 3-Clause "New" or "Revised" License
7 stars 4 forks source link

required_if definition is not backwards compatible with Ansible < 2.3.0 #4

Closed networkop closed 7 years ago

networkop commented 7 years ago

The option to define is_one_of boolean flag has only appeared in Ansible 2.3.0 proof Therefore the following definition required_if=[ ['state', 'present', ['id', 'properties', 'match_filter'], True], results in the following error on any Ansible < 2.3.0 (e.g. 2.2.0):

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ValueError: too many values to unpack
fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "module_stderr": "Traceback (most recent call last): 
File \"/tmp/ansible_FfQhm4/ansible_module_nuage_vspk.py\", line 1044, in <module>
    main()
  File \"/tmp/ansible_FfQhm4/ansible_module_nuage_vspk.py\", line 1033, in main supports_check_mode=True
 File \"/tmp/ansible_FfQhm4/ansible_modlib.zip/ansible/module_utils/basic.py\", line 714, in __init__
  File \"/tmp/ansible_FfQhm4/ansible_modlib.zip/ansible/module_utils/basic.py\", line 1353, in _check_required_if\nValueError: too many values to unpack
", "module_stdout": "", "msg": "MODULE FAILURE"}

Can I suggest to do a conditional assignment? Something like this:

    from packaging import version
    from ansible import __version__ as ansible_version
    if version.parse(ansible_version) < version.parse('2.3.0'):
        required_if = []
    else:
        required_if=[
            ['state', 'present', ['id', 'properties', 'match_filter'], True],
            ['state', 'absent', ['id', 'properties', 'match_filter'], True],
            ['command', 'change_password', ['id', 'properties']],
            ['command', 'wait_for_job', ['id']]
        ]

and substitute the assignment inside AnsibleModule definition like this:

        required_one_of=[
            ['command', 'state']
        ],
        required_if=required_if,
        supports_check_mode=True

Let me know what you think. Happy to submit a pull request if you're happy

pdellaert commented 7 years ago

Hi @networkop,

You point out a valid issue in case Ansible 2.2.x is used. The main issue i see with your adaptation: If ansible 2.3 or older is being used, there are no checks at all on the validity of the input...

Also, the reason I've made the changes is because we've opened a PR against the official Ansible repository to get our module officially in the next Ansible release (2.4). From the feedback i've got from the Ansible maintainers, making this change will most likely not be accepted.

As i do understand your issue, and you might be running Ansible 2.2 instead of the latest release, i have tagged a release which can be used with Ansible 2.2, and i will update the readme to display the new requirements.

Release: https://github.com/nuagenetworks/vspk-ansible/releases/tag/v2.2-beta

I hope that helps?

networkop commented 7 years ago

Fair enough, if you think it's not going to be accepted, that's fine.