Changed primary key of 'tool netwatch' #247

tim427 · 2024-01-04T19:31:50Z

SUMMARY

I've changed the primary key for tool netwatch from name to host, because name isn't used at all.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

When adding name to the playbook:

TASK [common : (routeros) Set netwatch] *********************************************************************************************************************************************
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: 'name'
fatal: [routeroshostname -> localhost]: FAILED! => {"changed": false, "module_stderr": "Traceback (most recent call last):\n  File \"/home/username/.ansible/tmp/ansible-tmp-1704395855.7240071-1606677-122124317821788/AnsiballZ_api_modify.py\", line 107, in <module>\n    _ansiballz_main()\n  File \"/home/username/.ansible/tmp/ansible-tmp-1704395855.7240071-1606677-122124317821788/AnsiballZ_api_modify.py\", line 99, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/username/.ansible/tmp/ansible-tmp-1704395855.7240071-1606677-122124317821788/AnsiballZ_api_modify.py\", line 47, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.community.routeros.plugins.modules.api_modify', init_globals=dict(_module_fqn='ansible_collections.community.routeros.plugins.modules.api_modify', _modlib_path=modlib_path),\n  File \"<frozen runpy>\", line 226, in run_module\n  File \"<frozen runpy>\", line 98, in _run_module_code\n  File \"<frozen runpy>\", line 88, in _run_code\n  File \"/tmp/ansible_community.routeros.api_modify_payload_rbt_phd5/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 1148, in <module>\n  File \"/tmp/ansible_community.routeros.api_modify_payload_rbt_phd5/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 1144, in main\n  File \"/tmp/ansible_community.routeros.api_modify_payload_rbt_phd5/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 870, in sync_with_primary_keys\n  File \"/tmp/ansible_community.routeros.api_modify_payload_rbt_phd5/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 870, in <genexpr>\nKeyError: 'name'\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

When removing the name:

TASK [common : (routeros) Set netwatch] *********************************************************************************************************************************************
fatal: [routerhostname -> localhost]: FAILED! => {"changed": false, "msg": "Every element in data must contain \"name\". For example, the element at index #1 does not provide it."}

codecov · 2024-01-04T21:17:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d6feda) 82.97% compared to head (8403268) 82.97%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #247   +/-   ##
=======================================
  Coverage   82.97%   82.97%           
=======================================
  Files          32       32           
  Lines        4046     4046           
  Branches      871      871           
=======================================
  Hits         3357     3357           
  Misses        506      506           
  Partials      183      183

Flag	Coverage Δ
integration	`66.86% <ø> (ø)`
sanity	`22.10% <ø> (ø)`
units	`82.91% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

felixfontein · 2024-01-05T07:24:11Z

This was added by @hansmi in #216.

Also please note that any change must come with a changelog fragment.

derdeagle · 2024-01-05T07:44:50Z

I think this is a bit trickier than it appears at first.

Setting the primary key to the host parameter would make it impossible to have one netwatch entry for an ICMP probe and one netwatch entry for an HTTP probe for this host where each netwatch entry runs a different script or produces a different log message. The following would not be possible.

/tool netwatch
add host=10.0.0.1 interval=30s type=simple up-script=":log info \"Ping to 10.0.0.1 successful\""
add host=10.0.0.1 interval=30s type=http-get up-script=":log info \"HTTP to 10.0.0.1 port 80 successful\""

Nevertheless I see that name is not mandatory in RouterOS and having it set as the primary key would make it mandatory for using it in a playbook.

As the tool netwatch path has quite some parameters depending on other parameters this does not seem as an easy task to me. Maybe that is why name was set as a primary key initially.

tim427 · 2024-01-05T11:04:47Z

I think this is a bit trickier than it appears at first.

Setting the primary key to the host parameter would make it impossible to have one netwatch entry for an ICMP probe and one netwatch entry for an HTTP probe for this host where each netwatch entry runs a different script or produces a different log message. The following would not be possible.
/tool netwatch
add host=10.0.0.1 interval=30s type=simple up-script=":log info \"Ping to 10.0.0.1 successful\""
add host=10.0.0.1 interval=30s type=http-get up-script=":log info \"HTTP to 10.0.0.1 port 80 successful\""
Nevertheless I see that name is not mandatory in RouterOS and having it set as the primary key would make it mandatory for using it in a playbook.

As the tool netwatch path has quite some parameters depending on other parameters this does not seem as an easy task to me. Maybe that is why name was set as a primary key initially.

I see. I've mistakenly interpreted the output and concluded that the name parameter isn't used in RouterOS. However, the host parameter is always mandatory.

When using the following task:

    - name: Set netwatch
      community.routeros.api_modify:
        path: "tool netwatch"
        handle_absent_entries: remove
        handle_entries_content: remove_as_much_as_possible
        ensure_order: true
        data:
          - comment: "testcomment"
            name: "testname"
            disabled: "no"
            host: "123.123.123.123"
            interval: "1m"
            timeout: "1s"
            type: "simple"

Results in the following error;

TASK [common : Set netwatch] *********************************************************************************************************************************************
task path: /home/username/ansible/roles/common/tasks/routeros.yml:119
The full traceback is:
Traceback (most recent call last):
  File "/home/username/.ansible/tmp/ansible-tmp-1704452225.7768626-2416518-25623699886920/AnsiballZ_api_modify.py", line 107, in <module>
    _ansiballz_main()
  File "/home/username/.ansible/tmp/ansible-tmp-1704452225.7768626-2416518-25623699886920/AnsiballZ_api_modify.py", line 99, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/home/username/.ansible/tmp/ansible-tmp-1704452225.7768626-2416518-25623699886920/AnsiballZ_api_modify.py", line 47, in invoke_module
    runpy.run_module(mod_name='ansible_collections.community.routeros.plugins.modules.api_modify', init_globals=dict(_module_fqn='ansible_collections.community.routeros.plugins.modules.api_modify', _modlib_path=modlib_path),
  File "<frozen runpy>", line 226, in run_module
  File "<frozen runpy>", line 98, in _run_module_code
  File "<frozen runpy>", line 88, in _run_code
  File "/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py", line 1148, in <module>
  File "/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py", line 1144, in main
  File "/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py", line 870, in sync_with_primary_keys
  File "/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py", line 870, in <genexpr>
KeyError: 'name'
fatal: [routerhostname -> localhost]: FAILED! => {
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"/home/username/.ansible/tmp/ansible-tmp-1704452225.7768626-2416518-25623699886920/AnsiballZ_api_modify.py\", line 107, in <module>\n    _ansiballz_main()\n  File \"/home/username/.ansible/tmp/ansible-tmp-1704452225.7768626-2416518-25623699886920/AnsiballZ_api_modify.py\", line 99, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/home/username/.ansible/tmp/ansible-tmp-1704452225.7768626-2416518-25623699886920/AnsiballZ_api_modify.py\", line 47, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.community.routeros.plugins.modules.api_modify', init_globals=dict(_module_fqn='ansible_collections.community.routeros.plugins.modules.api_modify', _modlib_path=modlib_path),\n  File \"<frozen runpy>\", line 226, in run_module\n  File \"<frozen runpy>\", line 98, in _run_module_code\n  File \"<frozen runpy>\", line 88, in _run_code\n  File \"/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 1148, in <module>\n  File \"/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 1144, in main\n  File \"/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 870, in sync_with_primary_keys\n  File \"/tmp/ansible_community.routeros.api_modify_payload_qhzv0hbb/ansible_community.routeros.api_modify_payload.zip/ansible_collections/community/routeros/plugins/modules/api_modify.py\", line 870, in <genexpr>\nKeyError: 'name'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}

derdeagle · 2024-01-05T11:11:18Z

@tim427 Which version of RouterOS are you running? Not that I currently have evidence but only a suspicion. Could you please verify that you actually can use the name parameter by logging into your device and typing /tool netwatch add <TAB> for autocomplete and check if name is listed there?
I am not that very firm with the error messages tbh so as I said, it is only a suspicion.

I am running RouterOS version 7.13 currently (most recent) and having the following created via Ansible works fine.

/tool netwatch
add host=8.8.8.8 interval=30s name=testping1 type=simple
add host=8.8.4.4 interval=30s name=testping2 type=simple

tim427 · 2024-01-05T11:20:17Z

[user@routerhostname] > /tool netwatch
[user@routerhostname] /tool/netwatch> add host=8.8.8.8 interval=30s name=testping1 type=simple
[user@routerhostname] /tool/netwatch> add host=8.8.4.4 interval=30s name=testping2 type=simple
[user@routerhostname] /tool/netwatch> print
Columns: TYPE, HOST, TIMEOUT, INTERVAL, STATUS, SINCE
# TYPE    HOST            TIMEOUT  INTERVAL  STATUS  SINCE
0 simple  8.8.8.8                  30s       up      2024-01-05 12:15:05
1 simple  8.8.4.4                  30s       up      2024-01-05 12:15:05
[user@routerhostname] /tool/netwatch> /system/package/print
Columns: NAME, VERSION
# NAME      VERSION
0 routeros  7.13rc4
1 wireless  7.13rc4

Version: 7.13rc4

Tab completion:

[user@routerhostname] /tool/netwatch> add <TAB>
certificate           copy-from       host           name                packet-size     start-delay       thr-avg           thr-loss-count       thr-stdev             type
check-certificate     disabled        http-codes     packet-count        port            startup-delay     thr-http-time     thr-loss-percent     thr-tcp-conn-time     up-script
comment               down-script     interval       packet-interval     src-address     test-script       thr-jitter        thr-max              timeout

By simply executing /tool/netwatch/add it asks for host only:

[user@routerhostname] /tool/netwatch> add
host: 1.1.1.1
[user@routerhostname] /tool/netwatch>

So host is a required parameter, but I understand the community.routeros logic to have name as the primary key.
(but the name parameter fails at least with my test-setup)

tim427 · 2024-01-05T11:24:42Z

Same results for version: 7.14beta4

[user@routerhostname] > /system/package/print
Columns: NAME, VERSION
# NAME      VERSION
0 routeros  7.14beta4
1 wireless  7.14beta4

derdeagle · 2024-01-05T11:26:23Z

Unfortunately I am unable to reproduce this on the fly. Later on I will spawn a VM with RouterOS 7.14beta4 and try to provoke and reproduce this error.
For the sake of completeness: which version of this collection are you currently using? (small hint which I am always grateful for: ansible-galaxy collection list)

tim427 · 2024-01-05T11:28:00Z

Unfortunately I am unable to reproduce this on the fly. Later on I will spawn a VM with RouterOS 7.14beta4 and try to provoke and reproduce this error. For the sake of completeness: which version of this collection are you currently using? (small hint which I am always grateful for: ansible-galaxy collection list)

# /home/user/.ansible/collections/ansible_collections
Collection         Version
------------------ -------
ansible.netcommon  6.0.0
ansible.utils      3.0.0
community.routeros 2.11.0

tim427 · 2024-01-05T11:30:15Z

Interesting... This version actually works... 7.13

[user@routerhostname] > /system/package/print
Columns: NAME, VERSION
# NAME      VERSION
0 wireless  7.13
1 routeros  7.13

So it was broken in the latest ReleaseCandiate (7.13rc4), it works on the stable version (7.13) and is again broken on the new testing channel (7.14beta4).

tim427 · 2024-01-05T11:31:37Z

Also; the whole name parameter is only visible in CLI/API, not in WebFig, nor in WinBox.

So name is a "hidden" parameter?

tim427 · 2024-01-05T13:40:01Z

I found something interesting:

With the following config:

        path: "tool netwatch"
        handle_absent_entries: remove
        handle_entries_content: remove_as_much_as_possible
        ensure_order: true
        data:
          - name: "ams0.signal.blue"

And already some netwatch entries in place on the RouterOS without name parameter -> the playbook fails with the earlier specified error.

To conclude, it's exactly the otherway around:
KeyError: 'name' is a missing parameter on the RouterOS itself, not in community.routeros nor in the playbook.

Solution: remove everywhere the old netwatch entries and manage them with Ansible.

Would it be wise to make a "default" for the name parameter?

derdeagle · 2024-01-05T16:05:50Z

I just spawned a VM with RouterOS version 7.14beta4 and did some parallel testing with 7.13. It just got a bit more complicated as even the primary key set to the name parameter is not the best way as it is not enforced to be unique by RouterOS.

Below is the output of /tool/netwatch/export.

/tool netwatch
add host=8.8.8.8 name=test type=simple
add host=8.8.4.4 name=test type=simple

IMHO we should dig deeper here and find a suitable general approach.

I found something interesting:

With the following config:
        path: "tool netwatch"
        handle_absent_entries: remove
        handle_entries_content: remove_as_much_as_possible
        ensure_order: true
        data:
          - name: "ams0.signal.blue"
And already some netwatch entries in place on the RouterOS without name parameter -> the playbook fails with the earlier specified error.

To conclude, it's exactly the otherway around: KeyError: 'name' is a missing parameter on the RouterOS itself, not in community.routeros nor in the playbook.

Solution: remove everywhere the old netwatch entries and manage them with Ansible.

Would it be wise to make a "default" for the name parameter?

Good catch!
Probably because it is trying to look up the key name from the currently existing config on RouterOS but doesn't find it as it isn't set.

felixfontein · 2024-01-05T16:38:44Z

plugins/module_utils/_api_data.py

@@ -3652,7 +3652,7 @@ def join_path(path):
        versioned=[
            ('7', '>=', VersionedAPIData(
                fully_understood=True,
-                primary_keys=('name', ),
+                primary_keys=('host', ),


You could also simply use

Suggested change

primary_keys=('host', ),

Then the module doesn't use any kind of identifier. (You should best use such a path with handle_absent_entries=remove and handle_entries_content=remove_as_much_as_possible. Otherwise you don't modify entries, but only add new ones...)

felixfontein · 2024-01-06T08:28:04Z

This still needs a changelog fragment before it can be merged :)

github-actions · 2024-01-07T10:25:41Z

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.routeros/branch/main

felixfontein · 2024-01-07T13:04:30Z

@tim427 @derdeagle thanks a lot!

Changed primary key of 'tool netwatch'

09b0669

felixfontein reviewed Jan 5, 2024

View reviewed changes

Removed the primary key after discussion on PR ansible-collections#247

4da2e76

derdeagle approved these changes Jan 5, 2024

View reviewed changes

Added changelog fragment

8403268

felixfontein approved these changes Jan 7, 2024

View reviewed changes

felixfontein merged commit d5a2686 into ansible-collections:main Jan 7, 2024
44 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changed primary key of 'tool netwatch' #247

Changed primary key of 'tool netwatch' #247

tim427 commented Jan 4, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

felixfontein commented Jan 5, 2024

derdeagle commented Jan 5, 2024

tim427 commented Jan 5, 2024

derdeagle commented Jan 5, 2024

tim427 commented Jan 5, 2024 •

edited

Loading

tim427 commented Jan 5, 2024

derdeagle commented Jan 5, 2024

tim427 commented Jan 5, 2024

tim427 commented Jan 5, 2024

tim427 commented Jan 5, 2024 •

edited

Loading

tim427 commented Jan 5, 2024 •

edited

Loading

derdeagle commented Jan 5, 2024

felixfontein Jan 5, 2024

felixfontein commented Jan 6, 2024

github-actions bot commented Jan 7, 2024 •

edited

Loading

felixfontein commented Jan 7, 2024

Changed primary key of 'tool netwatch' #247

Changed primary key of 'tool netwatch' #247

Conversation

tim427 commented Jan 4, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

codecov bot commented Jan 4, 2024 • edited Loading

Codecov Report

felixfontein commented Jan 5, 2024

derdeagle commented Jan 5, 2024

tim427 commented Jan 5, 2024

derdeagle commented Jan 5, 2024

tim427 commented Jan 5, 2024 • edited Loading

tim427 commented Jan 5, 2024

derdeagle commented Jan 5, 2024

tim427 commented Jan 5, 2024

tim427 commented Jan 5, 2024

tim427 commented Jan 5, 2024 • edited Loading

tim427 commented Jan 5, 2024 • edited Loading

derdeagle commented Jan 5, 2024

felixfontein Jan 5, 2024

Choose a reason for hiding this comment

felixfontein commented Jan 6, 2024

github-actions bot commented Jan 7, 2024 • edited Loading

Docs Build 📝

felixfontein commented Jan 7, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

tim427 commented Jan 5, 2024 •

edited

Loading

tim427 commented Jan 5, 2024 •

edited

Loading

tim427 commented Jan 5, 2024 •

edited

Loading

github-actions bot commented Jan 7, 2024 •

edited

Loading