search

ansible-collections / cisco.ios

Ansible Network Collection for Cisco IOS
GNU General Public License v3.0
262 stars 161 forks source link

Add VRF support for ios_ntp #291

Closed lvrfrc87 closed 2 years ago

lvrfrc87 commented 3 years ago
SUMMARY

Based on request https://github.com/ansible-collections/cisco.ios/issues/143, this PR add VRF support to NTP configuration

ISSUE TYPE
COMPONENT NAME

ios_ntp

feisa commented 3 years ago

sure let me checkout your branch

feisa commented 3 years ago

Error

The full traceback is: Traceback (most recent call last): File "/home/feisa/.ansible/tmp/ansible-local-11981194qw7x9qq/ansible-tmp-1617812569.869788-1198381-231450917832261/AnsiballZ_ios_ntp.py", line 102, in _ansiballz_main() File "/home/feisa/.ansible/tmp/ansible-local-11981194qw7x9qq/ansible-tmp-1617812569.869788-1198381-231450917832261/AnsiballZ_ios_ntp.py", line 94, in _ansiballz_main invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS) File "/home/feisa/.ansible/tmp/ansible-local-11981194qw7x9qq/ansible-tmp-1617812569.869788-1198381-231450917832261/AnsiballZ_ios_ntp.py", line 40, in invoke_module runpy.run_module(mod_name='ansible_collections.cisco.ios.plugins.modules.ios_ntp', init_globals=None, run_name='main', alter_sys=True) File "/usr/lib64/python3.6/runpy.py", line 205, in run_module return _run_module_code(code, init_globals, run_name, mod_spec) File "/usr/lib64/python3.6/runpy.py", line 96, in _run_module_code mod_name, mod_spec, pkg_name, script_name) File "/usr/lib64/python3.6/runpy.py", line 85, in _run_code exec(code, run_globals) File "/tmp/ansible_cisco.ios.ios_ntp_payload_wxi6vm9k/ansible_cisco.ios.ios_ntp_payload.zip/ansible_collections/cisco/ios/plugins/modules/ios_ntp.py", line 392, in File "/tmp/ansible_cisco.ios.ios_ntp_payload_wxi6vm9k/ansible_cisco.ios.ios_ntp_payload.zip/ansible_collections/cisco/ios/plugins/modules/ios_ntp.py", line 378, in main File "/tmp/ansible_cisco.ios.ios_ntp_payload_wxi6vm9k/ansible_cisco.ios.ios_ntp_payload.zip/ansible_collections/cisco/ios/plugins/modules/ios_ntp.py", line 198, in map_config_to_obj TypeError: 'NoneType' object is not iterable failed: [dc3-edgec001] (item=10.255.255.2) => { "ansible_loop_var": "item", "changed": false, "item": "10.255.255.2", "module_stderr": "Traceback (most recent call last):\n File \"/home/feisa/.ansible/tmp/ansible-local-11981194qw7x9qq/ansible-tmp-1617812569.869788-1198381-231450917832261/AnsiballZ_ios_ntp.py\", line 102, in \n _ansiballz_main()\n File \"/home/feisa/.ansible/tmp/ansible-local-11981194qw7x9qq/ansible-tmp-1617812569.869788-1198381-231450917832261/AnsiballZ_ios_ntp.py\", line 94, in _ansiballz_main\n invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n File \"/home/feisa/.ansible/tmp/ansible-local-11981194qw7x9qq/ansible-tmp-1617812569.869788-1198381-231450917832261/AnsiballZ_ios_ntp.py\", line 40, in invoke_module\n runpy.run_module(mod_name='ansible_collections.cisco.ios.plugins.modules.ios_ntp', init_globals=None, run_name='main', alter_sys=True)\n File \"/usr/lib64/python3.6/runpy.py\", line 205, in run_module\n return _run_module_code(code, init_globals, run_name, mod_spec)\n File \"/usr/lib64/python3.6/runpy.py\", line 96, in _run_module_code\n mod_name, mod_spec, pkg_name, script_name)\n File \"/usr/lib64/python3.6/runpy.py\", line 85, in _run_code\n exec(code, run_globals)\n File \"/tmp/ansible_cisco.ios.ios_ntp_payload_wxi6vm9k/ansible_cisco.ios.ios_ntp_payload.zip/ansible_collections/cisco/ios/plugins/modules/ios_ntp.py\", line 392, in \n File \"/tmp/ansible_cisco.ios.ios_ntp_payload_wxi6vm9k/ansible_cisco.ios.ios_ntp_payload.zip/ansible_collections/cisco/ios/plugins/modules/ios_ntp.py\", line 378, in main\n File \"/tmp/ansible_cisco.ios.ios_ntp_payload_wxi6vm9k/ansible_cisco.ios.ios_ntp_payload.zip/ansible_collections/cisco/ios/plugins/modules/ios_ntp.py\", line 198, in map_config_to_obj\nTypeError: 'NoneType' object is not iterable\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1 }

lvrfrc87 commented 3 years ago

@feisa I have pushed a new commit. Would you mind clone the latest and give another try?

feisa commented 3 years ago

output provided

JoanieAda commented 3 years ago

@lvrfrc87 Ran the current version in this PR and here is the traceback I get

The full traceback is: Traceback (most recent call last): File "/home/jsylvain/.ansible/tmp/ansible-local-1700163pvdw1_ul/ansible-tmp-1619600900.2945037-6527951318193/AnsiballZ_ios_ntp_lvrfrc87.py", line 102, in _ansiballz_main() File "/home/jsylvain/.ansible/tmp/ansible-local-1700163pvdw1_ul/ansible-tmp-1619600900.2945037-6527951318193/AnsiballZ_ios_ntp_lvrfrc87.py", line 94, in _ansiballz_main invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS) File "/home/jsylvain/.ansible/tmp/ansible-local-1700163pvdw1_ul/ansible-tmp-1619600900.2945037-6527951318193/AnsiballZ_ios_ntp_lvrfrc87.py", line 40, in invoke_module runpy.run_module(mod_name='ansible.modules.ios_ntp_lvrfrc87', init_globals=None, run_name='main', alter_sys=True) File "/usr/lib/python3.8/runpy.py", line 207, in run_module return _run_module_code(code, init_globals, run_name, mod_spec) File "/usr/lib/python3.8/runpy.py", line 97, in _run_module_code _run_code(code, mod_globals, init_globals, File "/usr/lib/python3.8/runpy.py", line 87, in _run_code exec(code, run_globals) File "/tmp/ansible_ios_ntp_lvrfrc87_payload_uiqh_jop/ansible_ios_ntp_lvrfrc87_payload.zip/ansible/modules/ios_ntp_lvrfrc87.py", line 396, in File "/tmp/ansible_ios_ntp_lvrfrc87_payload_uiqh_jop/ansible_ios_ntp_lvrfrc87_payload.zip/ansible/modules/ios_ntp_lvrfrc87.py", line 382, in main File "/tmp/ansible_ios_ntp_lvrfrc87_payload_uiqh_jop/ansible_ios_ntp_lvrfrc87_payload.zip/ansible/modules/ios_ntp_lvrfrc87.py", line 202, in map_config_to_obj TypeError: cannot unpack non-iterable NoneType object failed: [s1-core01] (item={'server': '172.16.255.2', 'vrf': 'mgmt'}) => { "ansible_facts": { "discovered_interpreter_python": "/usr/bin/python3" }, "ansible_loop_var": "item", "changed": false, "item": { "server": "172.16.255.2", "vrf": "mgmt" }, "module_stderr": "Traceback (most recent call last):\n File \"/home/jsylvain/.ansible/tmp/ansible-local-1700163pvdw1_ul/ansible-tmp-1619600900.2945037-6527951318193/AnsiballZ_ios_ntp_lvrfrc87.py\", line 102, in \n _ansiballz_main()\n File \"/home/jsylvain/.ansible/tmp/ansible-local-1700163pvdw1_ul/ansible-tmp-1619600900.2945037-6527951318193/AnsiballZ_ios_ntp_lvrfrc87.py\", line 94, in _ansiballz_main\n invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n File \"/home/jsylvain/.ansible/tmp/ansible-local-1700163pvdw1_ul/ansible-tmp-1619600900.2945037-6527951318193/AnsiballZ_ios_ntp_lvrfrc87.py\", line 40, in invoke_module\n runpy.run_module(mod_name='ansible.modules.ios_ntp_lvrfrc87', init_globals=None, run_name='main', alter_sys=True)\n File \"/usr/lib/python3.8/runpy.py\", line 207, in run_module\n return _run_module_code(code, init_globals, run_name, mod_spec)\n File \"/usr/lib/python3.8/runpy.py\", line 97, in _run_module_code\n _run_code(code, mod_globals, init_globals,\n File \"/usr/lib/python3.8/runpy.py\", line 87, in _run_code\n exec(code, run_globals)\n File \"/tmp/ansible_ios_ntp_lvrfrc87_payload_uiqh_jop/ansible_ios_ntp_lvrfrc87_payload.zip/ansible/modules/ios_ntp_lvrfrc87.py\", line 396, in \n File \"/tmp/ansible_ios_ntp_lvrfrc87_payload_uiqh_jop/ansible_ios_ntp_lvrfrc87_payload.zip/ansible/modules/ios_ntp_lvrfrc87.py\", line 382, in main\n File \"/tmp/ansible_ios_ntp_lvrfrc87_payload_uiqh_jop/ansible_ios_ntp_lvrfrc87_payload.zip/ansible/modules/ios_ntp_lvrfrc87.py\", line 202, in map_config_to_obj\nTypeError: cannot unpack non-iterable NoneType object\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1 }

JoanieAda commented 3 years ago

@lvrfrc87 Digging around a little bit that traceback seem to occur when unpacking into 2 variables on line 205

If I change it to the following instead(using statically set vrf right now)

server = parse_server(line, dest)
vrf = "vrf mgmt"

Then it passes that traceback, the next one that occur is the append method which is currently being passed 2 elements when it should only get one. Not sure if that is of any use or no.

On that point however, the combination of vrf and server should be added to the server list as either a tuple or list as they are tied with one another.

Here is a scenario where keeping them separate would result in a failure, so say I have the following configuration already:

ntp server vrf mgmt 172.16.1.1
ntp server vrf prod 10.1.1.1

If my goal is to add server 172.16.1.1 in the prod vrf during the validation of server_have and vrf_have it would find that both the server and vrf are already part of the list and wouldn't add the new line which should be ntp server vrf prod 172.16.1.1

JoanieAda commented 3 years ago

@lvrfrc87 I've update my PR in your repo to reflect the changes you had in the current version (regex optimization for one), this version has a lot less changes to your baseline than my original push

I've tested it against the same setup I originally used and it all seems to work (tested with both the present and absent states)

KB-perByte commented 2 years ago

Hey @lvrfrc87, Sorry for getting back late, we are planning an ios_ntp Resource Module in the next few months. That being out by end of September'21 the current module would be deprecated. We do not recommend adding features to modules that are/to be deprecated. Please check our Roadmap for the same https://github.com/ansible/community/wiki/Network%3A2021-Till-Dec-Roadmap Thank you!

lvrfrc87 commented 2 years ago

@KB-perByte would be possible to have this merged? Thanks!

KB-perByte commented 2 years ago

Hey @lvrfrc87, We can get this merged the changes do look good to me. Can you please consider the CI checks that are failing. let us know if you need any help. Thank you.

lvrfrc87 commented 2 years ago

@KB-perByte I can see there is one error remaining related to ios_logging.py shall I fix that in this PR or can we merge anyway?

KB-perByte commented 2 years ago

Hi @lvrfrc87, What is the issue? we have an ios_logging_global resource module out, can you check if the issue is present for the Resource module too?

lvrfrc87 commented 2 years ago

@lvrfrc87 For your tests you'll want to update the following file tests/unit/modules/network/ios/fixtures/ios_ntp_config.cfg Line 7 should actually be ntp server vrf my_mgmt_vrf 10.75.32.5

And in the file tests/unit/modules/network/ios/test_ios_ntp.py Line 105 should be "no ntp server vrf my_mgmt_vrf 10.75.32.5",

Essentially just need to add the vrf keyword following ntp server If you prefer I can also make the changes and issue a PR

@JoanieAda Thanks! That has already be done

KB-perByte commented 2 years ago

Hey @JoanieAda, You were right the test cases were missing out on the word vrf in the command. @lvrfrc87 everything else LGTM!

KB-perByte commented 2 years ago

recheck

lvrfrc87 commented 2 years ago

@KB-perByte I can still some error in sanity test not related to the module. Is it something you can look at?

KB-perByte commented 2 years ago

@lvrfrc87 I think that is a recent change in ansible/ansible causing the sanity to break, I'll monitor it.

KB-perByte commented 2 years ago

recheck

KB-perByte commented 2 years ago

Hey @lvrfrc87, https://github.com/ansible-collections/cisco.ios/pull/396 Merging this PR should fix the sanity issues we can rebase and merge this PR then,

KB-perByte commented 2 years ago

Hey @lvrfrc87 can you please rebase your repo with main, to fetch the changes that solve the CI breaking issue. We can merge it right after that. Thanks.

lvrfrc87 commented 2 years ago

@KB-perByte I can now see 1 integration test failing

KB-perByte commented 2 years ago

recheck

KB-perByte commented 2 years ago

Hey @lvrfrc87 Gate'd it, Thanks for the contribution. Do check out our roadmap for future contributions.