Add '/queue simple' path #260

samburney · 2024-03-30T11:16:12Z

The API does require the 'target' value, however it has some caveats.

A value of 0.0.0.0/0 will be returned as '' from the API, resulting in a change every time:

ok: [testrtr1] => { "queue_simple_api_info": { "changed": false, "failed": false, "result": [ { "!comment": null, "!dst": null, "!time": null, ".id": "*2D", "name": "Test queue with just defaults", "target": "" <----- This is with "target: 0.0.0.0/0" } ] } }

A value of '' will be accepted, but the API will return this as null (As below), again always resulting in a change:

ok: [testrtr1] => { "queue_simple_api_info": { "changed": false, "failed": false, "result": [ { "!comment": null, "!dst": null, "!target": null, <----- This is with "target: ''" "!time": null, ".id": "*29", "name": "Test queue with just defaults" } ] } }

Other values, such as IP addresses and interface names are accepted and behave as expected.

Does the module contain any logic to account for these kinds of edge cases?

derdeagle · 2024-02-10T13:15:59Z

I am not sure if setting the default for disabled to False is practical here. Even when setting it to no directly on a device it is neither shown in the output of /queue simple export nor in the output of /queue simple print detail.
Setting the default to False here looks like it would lead to changes on every execution. Can you confirm please?

I can confirm that setting to the disabled default to 'False' does not result in a change every time.

You're correct in that it does not appear in /export or api_info, I can remove this default if you like.

ok: [60brh-dc2-rtr1] => { "show_interface_queues": { "changed": false, "failed": false, "result": [ { "!dst": null, "!time": null, ".id": "*1", "bucket-size": "0.1/0.1", "burst-limit": "0/0", "burst-threshold": "0/0", "burst-time": "0s/0s", "comment": "Core: 172msa-dc1-core1:bonding1-vlan318 {HU-xxx-xxx} [1G]", "limit-at": "0/0", "max-limit": "0/1000000000", "name": "queue-bonding1-vlan318", "priority": "8/8", "queue": "default-small/default-small", "target": "bonding1-vlan318" } ] } }

Thank you for testing. If it doesn't lead to constant changes I am fine with it 👍

derdeagle · 2024-02-10T13:20:18Z

Are you sure about those values? Unfortunately I cannot test it on my own right now via Ansible but manually set I get the error message expected value of download-priority (line 1 column 39). The value 8/8 is the default for me.

The following is the output of /queue simple print detail for me on RouterOS 7.13 where I only specified the target manually which you already correctly set as required.

name="queue1" target=1.1.1.12/32 parent=none packet-marks="" priority=8/8 queue=default-small/default-small limit-at=0/0 max-limit=0/0 burst-limit=0/0 burst-threshold=0/0 burst-time=0s/0s bucket-size=0.1/0.1.

I took most of the default keys from the existing implementation of 'queue tree'. These values work as expected in an environment running 7.13.4.

The returned data does include the '/'s but they do not appear to be required as a part of the data set.

--- before +++ after @@ -1,3 +1,21 @@ { - "data": [] + "data": [ + { + ".id": "*1", + "bucket-size": "0.1/0.1", + "burst-limit": "0/0", + "burst-threshold": "0/0", + "burst-time": "0s/0s", + "comment": "Core: 172msa-dc1-core1:bonding1-vlan318 {HU-xxx-xxx} [1G]", + "disabled": false, + "limit-at": "0/0", + "max-limit": "0/1000000000", + "name": "queue-bonding1-vlan318", + "packet-marks": "", + "parent": "none", + "priority": "8/8", + "queue": "default-small/default-small", + "target": "bonding1-vlan318" + } + ] }

I'm happy to update and test the defaults to be include splitting the upload/download values if you like but it does work as-is.

Mhh interesting fact that the API accepts it but CLI doesn't. I then guess there are also no changes when rolled out again with only one value defined.
It seems it is working and it is not documented by MikroTik so 👍

I just spun up a test and it shows changes for me, please see below.

"data": [ { ".id": "*7", - "bucket-size": "0.1/0.1", - "burst-limit": "0/0", - "burst-threshold": "0/0", - "burst-time": "0s/0s", + "bucket-size": "0.1", + "burst-limit": 0, + "burst-threshold": 0, + "burst-time": "0s", "disabled": true, - "limit-at": "0/0", - "max-limit": "7000000/2000000", + "limit-at": 0, + "max-limit": "7M/2M", "name": "Limit wifi speed", "packet-marks": "", "parent": "none", - "priority": "8/8", + "priority": 8, "queue": "pcq-upload-default/pcq-download-default", "target": "10.10.2.0/24"

The only data I have provided is the following.

- disabled: True max-limit: 7M/2M name: Limit wifi speed queue: pcq-upload-default/pcq-download-default target: 10.10.2.0/24

Could you please recheck?

Can you please try max-limit: 7000000/2000000? The API will accept k/M/G/T etc... but converts it to bits on output. This causes it to always trigger a change.

I created a filter that does this conversion, allowing me to still use the abbreviated format.

Tbh. I could live with that but what I wanted to point out is the change from e.g. "priority": "8/8" to "priority": 8 without manually defining it. So the defaults you set do lead to constant changes (at least for me). The diff you posted only shows a new entry. Would you mind applying this change, then do a --check run again and post the output, please?

I've tested with your exact data with the results I've come to expect.

Playbook

- name: Test queue simple API hosts: mikrotik gather_facts: false module_defaults: community.routeros.api_modify: hostname: "{{ ansible_host }}" username: "{{ ansible_ssh_user }}" password: "{{ ansible_ssh_password }}" tasks: - name: Remove simple queues community.routeros.api_modify: path: queue simple handle_absent_entries: remove data: [] - name: Add simple queue (Create) community.routeros.api_modify: path: queue simple data: - disabled: True max-limit: 7M/2M name: Limit wifi speed queue: pcq-upload-default/pcq-download-default target: 10.10.2.0/24 - name: Add simple queue (Test for change with 7M/2M) community.routeros.api_modify: path: queue simple data: - disabled: True max-limit: 7M/2M name: Limit wifi speed queue: pcq-upload-default/pcq-download-default target: 10.10.2.0/24 - name: Add simple queue (Test for change with 7000000/2000000) community.routeros.api_modify: path: queue simple data: - disabled: True max-limit: 7000000/2000000 name: Limit wifi speed queue: pcq-upload-default/pcq-download-default target: 10.10.2.0/24

With --check

$ ansible-playbook playbooks/queue_simple_test.yml -l gns3-lab-core1 --diff --check PLAY [Test queue simple API] ********************************************************************************************************************************* TASK [Remove simple queues] ********************************************************************************************************************************** --- before +++ after @@ -1,20 +1,3 @@ { - "data": [ - { - ".id": "*9", - "bucket-size": "0.1/0.1", - "burst-limit": "0/0", - "burst-threshold": "0/0", - "burst-time": "0s/0s", - "disabled": true, - "limit-at": "0/0", - "max-limit": "7000000/2000000", - "name": "Limit wifi speed", - "packet-marks": "", - "parent": "none", - "priority": "8/8", - "queue": "pcq-upload-default/pcq-download-default", - "target": "10.10.2.0/24" - } - ] + "data": [] } changed: [gns3-lab-core1] TASK [Add simple queue (Create)] ***************************************************************************************************************************** --- before +++ after @@ -8,7 +8,7 @@ "burst-time": "0s/0s", "disabled": true, "limit-at": "0/0", - "max-limit": "7000000/2000000", + "max-limit": "7M/2M", "name": "Limit wifi speed", "packet-marks": "", "parent": "none", changed: [gns3-lab-core1] TASK [Add simple queue (Test for change with 7M/2M)] ********************************************************************************************************* --- before +++ after @@ -8,7 +8,7 @@ "burst-time": "0s/0s", "disabled": true, "limit-at": "0/0", - "max-limit": "7000000/2000000", + "max-limit": "7M/2M", "name": "Limit wifi speed", "packet-marks": "", "parent": "none", changed: [gns3-lab-core1] TASK [Add simple queue (Test for change with 7000000/2000000)] *********************************************************************************************** ok: [gns3-lab-core1] PLAY RECAP *************************************************************************************************************************************************** gns3-lab-core1 : ok=4 changed=3 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0

Without --check

$ ansible-playbook playbooks/queue_simple_test.yml -l gns3-lab-core1 --diff PLAY [Test queue simple API] ********************************************************************************************************************************* TASK [Remove simple queues] ********************************************************************************************************************************** --- before +++ after @@ -1,20 +1,3 @@ { - "data": [ - { - ".id": "*9", - "bucket-size": "0.1/0.1", - "burst-limit": "0/0", - "burst-threshold": "0/0", - "burst-time": "0s/0s", - "disabled": true, - "limit-at": "0/0", - "max-limit": "7000000/2000000", - "name": "Limit wifi speed", - "packet-marks": "", - "parent": "none", - "priority": "8/8", - "queue": "pcq-upload-default/pcq-download-default", - "target": "10.10.2.0/24" - } - ] + "data": [] } changed: [gns3-lab-core1] TASK [Add simple queue (Create)] ***************************************************************************************************************************** --- before +++ after @@ -1,3 +1,20 @@ { - "data": [] + "data": [ + { + ".id": "*A", + "bucket-size": "0.1/0.1", + "burst-limit": "0/0", + "burst-threshold": "0/0", + "burst-time": "0s/0s", + "disabled": true, + "limit-at": "0/0", + "max-limit": "7000000/2000000", + "name": "Limit wifi speed", + "packet-marks": "", + "parent": "none", + "priority": "8/8", + "queue": "pcq-upload-default/pcq-download-default", + "target": "10.10.2.0/24" + } + ] } changed: [gns3-lab-core1] TASK [Add simple queue (Test for change with 7M/2M)] ********************************************************************************************************* changed: [gns3-lab-core1] TASK [Add simple queue (Test for change with 7000000/2000000)] *********************************************************************************************** ok: [gns3-lab-core1] PLAY RECAP *************************************************************************************************************************************************** gns3-lab-core1 : ok=4 changed=3 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0

Sorry it took me so long to respond. I tried it with version 7.13 and 7.13.4 but I still do get other results than you it seems. With your version I always get changes regardless of using --check or not. When using --diff --check I do get the differences shown but with only --diff I just see the following

PLAY [mikrotik_devices] *********************************************************************************************** TASK [Manage /queue/simple] ******************************************************************************************* changed: [dev1.example.com] changed: [dev2.example.com] changed: [dev3.example.com] changed: [dev4.example.com] changed: [dev5.example.com] changed: [dev6.example.com] changed: [dev7.example.com] changed: [dev8.example.com] PLAY RECAP ************************************************************************************************************ dev3.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev4.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev5.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev1.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev2.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev7.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev6.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev8.example.com : ok=1 changed=1 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0

So the API changes something but RouterOS transformed it so that when checking back for the result it is the same as before. That is not desirable IMHO.
When defining the defaults as MikroTik uses them (in _api_data.py) I get the following (IMHO desirable) result:

PLAY [mikrotik_devices] *********************************************************************************************** TASK [Manage /queue/simple] ******************************************************************************************* ok: [dev1.example.com] ok: [dev2.example.com] ok: [dev3.example.com] ok: [dev4.example.com] ok: [dev5.example.com] ok: [dev6.example.com] ok: [dev7.example.com] ok: [dev8.example.com] PLAY RECAP ************************************************************************************************************ dev4.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev3.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev5.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev1.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev2.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev7.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev6.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0 dev8.example.com : ok=1 changed=0 unreachable=0 failed=0 skipped=0 rescued=0 ignored=0

Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		minor_changes:
		- api_info, api_modify - add missing path ``/queue simple`` (https://github.com/ansible-collections/community.routeros/pull/260).

-Original file line number
+Diff line change
@@ Expand Up / @@ -3829,6 +3829,30 @@ def join_path(path): @@
                 },
             ),
         ),
+        ('queue', 'simple'): APIData(
+            unversioned=VersionedAPIData(
+                primary_keys=('name', ),
+                fully_understood=True,
+                fields={
+                    'bucket-size': KeyInfo(default='0.1'),
+                    'burst-limit': KeyInfo(default=0),
+                    'burst-threshold': KeyInfo(default=0),
+                    'burst-time': KeyInfo(default='0s'),
+                    'comment': KeyInfo(can_disable=True, remove_value=''),
+                    'disabled': KeyInfo(default=False),
+                    'limit-at': KeyInfo(default=0),
+                    'max-limit': KeyInfo(default=0),
+                    'name': KeyInfo(),
+                    'packet-marks': KeyInfo(default=''),
+                    'parent': KeyInfo(default='none'),
+                    'priority': KeyInfo(default=8),
+                    'queue': KeyInfo(default='default-small'),
+                    'target': KeyInfo(required=True),
+                    'dst': KeyInfo(can_disable=True, remove_value=''),
+                    'time': KeyInfo(can_disable=True, remove_value=''),
+                },
+            ),
+        ),
         ('queue', 'tree'): APIData(
             unversioned=VersionedAPIData(
                 primary_keys=('name', ),
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -167,6 +167,7 @@ @@
             - ppp aaa
             - ppp profile
             - queue interface
+            - queue simple
             - queue tree
             - radius
             - radius incoming
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -176,6 +176,7 @@ @@
             - ppp aaa
             - ppp profile
             - queue interface
+            - queue simple
             - queue tree
             - radius
             - radius incoming
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add '/queue simple' path #260

Diff view

Diff view

There are no files selected for viewing

samburney Mar 30, 2024

derdeagle Feb 10, 2024

samburney Feb 11, 2024 •

edited

Loading

derdeagle Feb 12, 2024

derdeagle Feb 10, 2024

samburney Feb 11, 2024

derdeagle Feb 12, 2024

derdeagle Feb 12, 2024

samburney Feb 12, 2024

derdeagle Feb 12, 2024

samburney Feb 12, 2024

derdeagle Feb 17, 2024

Add '/queue simple' path #260

Add '/queue simple' path #260

Diff view

Diff view

There are no files selected for viewing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samburney Feb 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Playbook

With --check

Without --check

Choose a reason for hiding this comment

samburney Feb 11, 2024 •

edited

Loading