infoblox-ansible icon indicating copy to clipboard operation
infoblox-ansible copied to clipboard

Add modules to manipulate additional types of Name Server Groups

Open badnetmask opened this issue 3 years ago • 27 comments

This PR adds support for additional types of Name Server Groups.

The original nsgroup module only handles Authoritative type. This PR also modifies the description of the original nsgroup module to reflect that.

Each module's documentation contain examples.

nios_nsgroup_delegation – Configure Infoblox DNS Nameserver Delegation Groups
nios_nsgroup_forwardingmember – Configure Infoblox DNS Nameserver Forwarding Member Groups
nios_nsgroup_forwardstubserver – Configure Infoblox DNS Nameserver Forward/Stub Server Groups
nios_nsgroup_stubmember – Configure Infoblox DNS Nameserver Stub Member Groups```

badnetmask avatar Apr 13 '21 21:04 badnetmask

Hi @badnetmask, Thank you for your contribution to the Infoblox NIOS modules. We had to restructure the directory as per the requirements. As a result, there are some merge conflicts that have to be resolved. Can you please consider the same? Sorry for the inconvenience caused.

anagha-infoblox avatar Oct 11 '21 19:10 anagha-infoblox

Will be glad to review my changes as soon as I can find some available cycles.

badnetmask avatar Oct 11 '21 19:10 badnetmask

WIP -- I have fixed the merge conflicts, but I believe the new standards require integration tests, so I will work on these later.

badnetmask avatar Oct 18 '21 20:10 badnetmask

WIP -- I have fixed the merge conflicts, but I believe the new standards require integration tests, so I will work on these later.

Thank you! Yes, according to the new standards there's a requirement for unit and integration tests. I've suggested a few changes to fix the sanity error.

anagha-infoblox avatar Oct 19 '21 04:10 anagha-infoblox

@anagha-infoblox I need help figuring out how to create the tests. For whatever reason, when I run a playbook that uses all these new objects, against a real Infoblox device, all the objects are created just fine. However, running the collection in integration tests mode, they don't due to an error on the "infoblox-client" library. You can see the error on here on GitHub, on the checks tab.

So I'm not sure what's happening here, since the integration tests docker image uses the same version of the library that I have installed on my system (0.5.0). Any help here would be appreciated.

badnetmask avatar Oct 31 '21 15:10 badnetmask

Here's a quick example to demonstrate how it's working on a real Infoblox device:

❯ cat test_nsgroup.yml 
---
# playbook used to test: https://github.com/badnetmask/infoblox-ansible/tree/dtc
- hosts: localhost
  connection: local
  gather_facts: no
  vars:
    - nios_provider:
        host: <ip_address>
        username: admin
        password: infoblox
        wapi_version: "2.6"
  tasks:
    - name: configure a nsgroup delegation on the system
      infoblox.nios_modules.nios_nsgroup_delegation:
        name: ansible-nsgroup_delegation
        state: present
        provider: "{{ nios_provider }}"
        delegate_to:
          - name: ns1
            address: 192.168.0.1
      register: nsgroup_delegation_create1

❯ ansible-playbook -i localhost, test_nsgroup.yml

PLAY [localhost]

TASK [configure a nsgroup delegation on the system] 
changed: [localhost]

PLAY RECAP 
localhost                  : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

badnetmask avatar Oct 31 '21 16:10 badnetmask

@anagha-infoblox I need help figuring out how to create the tests. For whatever reason, when I run a playbook that uses all these new objects, against a real Infoblox device, all the objects are created just fine. However, running the collection in integration tests mode, they don't due to an error on the "infoblox-client" library. You can see the error on here on GitHub, on the checks tab.

So I'm not sure what's happening here, since the integration tests docker image uses the same version of the library that I have installed on my system (0.5.0). Any help here would be appreciated.

@badnetmask I see that the tests are failing because of the NoneType error. Before the tests were failing due to the error stated above for stable-2.12 and devel branches. It was fixed by adding this file under integration tests- https://github.com/infobloxopen/infoblox-ansible/blob/master/tests/integration/targets/prepare_nios_tests/tasks/prepare_nios_tests_idempotence.yml If that's an issue then, I would suggest adding

- pip:
    name: infoblox-client

under the tasks.

anagha-infoblox avatar Nov 01 '21 15:11 anagha-infoblox

Well, I basically copied the entire tree under the nios_zone, and modified the test tasks for this object, so I would expect all dependencies would be resolved. Nevertheless, I did add the pip task to my test, and it still fails the same way. I hope I am just missing something obvious, but I can't see what it is.

badnetmask avatar Nov 01 '21 15:11 badnetmask

@badnetmask Sorry for the delayed reply. The integration tests are failing because these objects are not added to the nios-simulator. The tests are being run with the help of a simulator that mimics the functionality of a NIOS grid. We will look into the procedure of adding new objects to the simulator. FYI, Code for nios-simulator- https://github.com/ansible/nios-test-container Link to the docker image of nios-simulator: https://quay.io/repository/ansible/nios-test-container?tag=1.3.0&tab=tags

anagha-infoblox avatar Nov 19 '21 07:11 anagha-infoblox

@anagha-infoblox I made all the necessary changes to the test container. All the tests pass when I run locally. Please review: https://github.com/ansible/nios-test-container/pull/8

badnetmask avatar Nov 21 '21 01:11 badnetmask

@anagha-infoblox - good and bad news. good news: the simulator image has been updated and is working (as you can see here: https://github.com/infobloxopen/infoblox-ansible/runs/4401477649?check_suite_focus=true). bad news: it only works for the devel branch, because that's where the image version has been updated. @mattclay any suggestions to help get us unstuck from here?

badnetmask avatar Dec 02 '21 22:12 badnetmask

@badnetmask The container used can be overridden by setting the ANSIBLE_NIOSSIM_CONTAINER environment variable before running ansible-test.

mattclay avatar Dec 02 '21 22:12 mattclay

@anagha-infoblox I suppose the container override needs to be configured on the repo level, which I do not have access. Please, let me know your thoughts.

badnetmask avatar Dec 02 '21 23:12 badnetmask

@anagha-infoblox I suppose the container override needs to be configured on the repo level, which I do not have access. Please, let me know your thoughts.

@badnetmask I have configured the ANSIBLE_NIOSSIM_CONTAINER environment variable in the PR#99

anagha-infoblox avatar Dec 10 '21 08:12 anagha-infoblox

Seems like the integration tests are not picking up the modified variable.

badnetmask avatar Dec 10 '21 14:12 badnetmask

Seems like the integration tests are not picking up the modified variable.

Yes. I will try this again and, the modules will be included in the next major release.

anagha-infoblox avatar Dec 13 '21 11:12 anagha-infoblox

@badnetmask, I have configured the ANSIBLE_NIOSSIM_CONTAINER environment variable in the PR#103. The tests are picking up the modified variable.

anagha-infoblox avatar Jan 07 '22 11:01 anagha-infoblox

@anagha-infoblox looks like all checks passed now \o/

badnetmask avatar Jan 14 '22 15:01 badnetmask

Thank you for your contribution to the Infoblox NIOS modules. According to the new standards, there is a requirement for unit and integration tests. The PR will be merged once apt unit tests for the modules are added.

anagha-infoblox avatar Jan 21 '22 15:01 anagha-infoblox

I'm having an issue here while trying to run the unit test locally (trying to avoid spamming GitHub checks).

This command:

$ ansible-test units --docker -v --color --coverage --python 3.9

Fails with this error:

ImportError while importing test module '/root/ansible_collections/infoblox/nios_modules/tests/unit/plugins/modules/test_nios_zone.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
/tmp/ansible-test-5dmd8vi7/ansible/utils/collection_loader/_collection_finder.py:434: in exec_module
    exec(code_obj, module.__dict__)
tests/unit/plugins/modules/test_nios_zone.py:25: in <module>
    from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
E   ModuleNotFoundError: No module named 'ansible_collections.infoblox.nios_modules.tests.unit.compat'
--------------------------------------------------------------------- generated xml file: /root/ansible_collections/infoblox/nios_modules/tests/output/junit/python3.9-modules-units.xml

What I don't understand is how these tests run just fine from GitHub if this module does not exist in the repo. Please advise.

badnetmask avatar Feb 05 '22 23:02 badnetmask

I'm having an issue here while trying to run the unit test locally (trying to avoid spamming GitHub checks).

This command:

$ ansible-test units --docker -v --color --coverage --python 3.9

Fails with this error:

ImportError while importing test module '/root/ansible_collections/infoblox/nios_modules/tests/unit/plugins/modules/test_nios_zone.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
/tmp/ansible-test-5dmd8vi7/ansible/utils/collection_loader/_collection_finder.py:434: in exec_module
    exec(code_obj, module.__dict__)
tests/unit/plugins/modules/test_nios_zone.py:25: in <module>
    from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
E   ModuleNotFoundError: No module named 'ansible_collections.infoblox.nios_modules.tests.unit.compat'
--------------------------------------------------------------------- generated xml file: /root/ansible_collections/infoblox/nios_modules/tests/output/junit/python3.9-modules-units.xml

What I don't understand is how these tests run just fine from GitHub if this module does not exist in the repo. Please advise.

@badnetmask, Sorry for the delayed reply. The CI workflow is failing because of the same issue. Will make the required changes soon.

anagha-infoblox avatar May 02 '22 12:05 anagha-infoblox

@badnetmask, I have raised a Pull Request with the changes required to pass the unit tests. Once the unit tests are added, the PR: Add modules to manipulate additional types of Name Server Groups will be merged. Thank you

anagha-infoblox avatar May 02 '22 13:05 anagha-infoblox

The Pull Request is merged. With the latest code

I'm having an issue here while trying to run the unit test locally (trying to avoid spamming GitHub checks).

This command:

$ ansible-test units --docker -v --color --coverage --python 3.9

Fails with this error:

ImportError while importing test module '/root/ansible_collections/infoblox/nios_modules/tests/unit/plugins/modules/test_nios_zone.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
/tmp/ansible-test-5dmd8vi7/ansible/utils/collection_loader/_collection_finder.py:434: in exec_module
    exec(code_obj, module.__dict__)
tests/unit/plugins/modules/test_nios_zone.py:25: in <module>
    from ansible_collections.infoblox.nios_modules.tests.unit.compat.mock import patch, MagicMock, Mock
E   ModuleNotFoundError: No module named 'ansible_collections.infoblox.nios_modules.tests.unit.compat'
--------------------------------------------------------------------- generated xml file: /root/ansible_collections/infoblox/nios_modules/tests/output/junit/python3.9-modules-units.xml

What I don't understand is how these tests run just fine from GitHub if this module does not exist in the repo. Please advise.

The Pull Request is merged. With the latest code, the above issue is fixed. Sorry for the delayed response.

anagha-infoblox avatar May 02 '22 14:05 anagha-infoblox

Seems like I still have issues running the unittests on my end, but I'll spend some time figuring it out before complaining. :smile:

badnetmask avatar May 03 '22 20:05 badnetmask

@badnetmask, can you please push the unit tests that you have written? We can verify the same if any issues persist.

anagha-infoblox avatar Jun 01 '22 05:06 anagha-infoblox

@JchhatbarInfoblox @JkhatriInfobox - this PR has been pending for a few years, and ended up forgotten because I didn't fix some pending issues. Now that they are fixed, would you mind giving it a review?

badnetmask avatar Apr 03 '24 03:04 badnetmask

@badnetmask - We’ve initiated the review process for your Pull Request. It appears that only Integration tests have been included, with Unit tests missing. Could you please commit the Unit test cases to the PR? This will ensure it passes both the Unit and Integration tests in the CI run.

Additionally, the modifications made to the file [tests/unit/compat/mock.py] have already been merged into the master branch. To maintain a clean commit history, please rebase your branch with the master. Thank you.

JkhatriInfobox avatar Jul 25 '24 09:07 JkhatriInfobox