community.routeros icon indicating copy to clipboard operation
community.routeros copied to clipboard

interface bridge vlan only works for non-existing vlans

Open gardar opened this issue 1 year ago • 7 comments

SUMMARY

It's only possible to use the interface bridge vlanpath with the api_modify module if the vlan does not already exist. It errors out if the vlan exists, both when attempting to modify the vlan but also when the config matches the existing vlan and it should not be attempting to modify it.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

api_modify interface bridge vlan

ANSIBLE VERSION
ansible [core 2.17.4]
  config file = /home/g/.ansible.cfg
  configured module search path = ['/home/g/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.12/site-packages/ansible
  ansible collection location = /home/g/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.12.6 (main, Sep  8 2024, 13:18:56) [GCC 14.2.1 20240805] (/usr/bin/python)
  jinja version = 3.1.4
  libyaml = True
COLLECTION VERSION
# /usr/lib/python3.12/site-packages/ansible_collections
Collection         Version
------------------ -------
community.routeros 2.19.0
CONFIGURATION
ANSIBLE_FORCE_COLOR(env: ANSIBLE_FORCE_COLOR) = True
ANSIBLE_PIPELINING(/home/g/.ansible.cfg) = True
CONFIG_FILE() = /home/g/.ansible.cfg
EDITOR(env: EDITOR) = vim
PAGER(env: PAGER) = less
OS / ENVIRONMENT
RouterOS 7.16.1
STEPS TO REPRODUCE
---
- community.routeros.api_modify:
  path: interface bridge vlan
  data:
    - vlan-ids: 2
      comment: test
      bridge: bridge
      tagged: ether2
EXPECTED RESULTS

The vlan should be modified if the data is different than the configuration on the device. If it's the same then it should not change the interface and give ok status.

ACTUAL RESULTS
 "msg": "Error while creating entry for bridge=\"bridge\", vlan-ids=\"2\": failure: vlan already added"

gardar avatar Oct 27 '24 13:10 gardar

Looks like I should perhaps use api_find_and_modify instead of api_modify for this. It is not working as expected though when I try to use it in a loop, as it's not finding any matches. More specifically the issue seems to be with the vlan-ids field.

This gives no matches:

- name: Configure vlans on bridge  # noqa args[module]
  community.routeros.api_find_and_modify:
    path: "interface bridge vlan"
    find:
      bridge: "{{ item.bridge }}"
      vlan-ids: "{{ item.vlan_id }}"
    values:
      vlan-ids: "{{ item.vlan_id }}"
      bridge: "{{ item.bridge }}"
      tagged: "{{ item.tagged }}"
  loop: "{{ __vlans }}"
  vars:
    __vlans:
      - bridge: bridge0
        vlan_id: 2
        tagged: ether2

whereas this gives a match:

- name: Configure vlans on bridge  # noqa args[module]
  community.routeros.api_find_and_modify:
    path: "interface bridge vlan"
    find:
      bridge: "{{ item.bridge }}"
      vlan-ids: 2
    values:
      vlan-ids: 2
      bridge: "{{ item.bridge }}"
      tagged: "{{ item.tagged }}"
  loop: "{{ __vlans }}"
  vars:
    __vlans:
      - bridge: bridge0
        vlan_id: 2
        tagged: ether2

I thought perhaps it might be that the vlan ids in the loop were interpreted as a string and not int but forcing the int data type with vlan-ids: "{{ item.vlan_id | int }}" does not help either.

Also, using variables does not seem to be the issue, as this works just fine:

- name: Configure vlans on bridge  # noqa args[module]
  community.routeros.api_find_and_modify:
    path: "interface bridge vlan"
    find:
      bridge: "{{ bridge }}"
      vlan-ids: "{{ vlan_id }}"
    values:
      vlan-ids: "{{ vlan_id }}"
      bridge: "{{ bridge }}"
      tagged: "{{ tagged }}"
  vars:
    bridge: bridge0
    vlan_id: 2
    tagged: ether2

Looking at the verbose output I notice that vlan-ids shows up as a quoted string under invocation.module_args.find when using a loop.

 {
    "ansible_loop_var": "item",
    "changed": false,
    "invocation": {
        "module_args": {
            "allow_no_matches": true,
            "ca_path": null,
            "encoding": "UTF-8",
            "find": {
                "bridge": "bridge0",
                "vlan-ids": "2"
            },

But when not using a loop it is unquoted.

 {
    "ansible_loop_var": "item",
    "changed": false,
    "invocation": {
        "module_args": {
            "allow_no_matches": true,
            "ca_path": null,
            "encoding": "UTF-8",
            "find": {
                "bridge": "bridge0",
                "vlan-ids": 2
            },

gardar avatar Oct 27 '24 19:10 gardar

Looks like this could be the same issue with the loops: https://github.com/ansible-collections/community.routeros/issues/169

gardar avatar Oct 27 '24 19:10 gardar

Found a workaround for the api_find_and_modify data type issue when using loops by using the dict() function.

- name: Configure vlans on bridge  # noqa args[module]
  community.routeros.api_find_and_modify:
    path: "interface bridge vlan"
    find: >-
      {{
        dict([
          ['vlan-ids', item.vlan_id],
          ['bridge', item.bridge]
        ])
      }}
    values:
      vlan-ids: "{{ item.vlan_id }}"
      bridge: "{{ item.bridge }}"
      tagged: "{{ item.tagged }}"
  loop: "{{ __vlans }}"
  vars:
    __vlans:
      - bridge: bridge0
        vlan_id: 2
        tagged: ether2

The same data type issue also affects the values field of the api_find_and_modify when using a loop and the task is not immutable and always shows as changed even though the old_data and new_data shown in the return values is identical. But using dict() as a workaround resolves that issue as well.

- name: Configure vlans on bridge  # noqa args[module]
  community.routeros.api_find_and_modify:
    path: "interface bridge vlan"
    find: >-
      {{
        dict([
          ['vlan-ids', item.vlan_id],
          ['bridge', item.bridge]
        ])
      }}
    values: >-
      {{
        dict([
          ['vlan-ids', item.vlan_id],
          ['bridge', item.bridge],
          ['tagged', item.tagged]
        ])
      }}
  loop: "{{ __vlans }}"
  vars:
    __vlans:
      - bridge: bridge0
        vlan_id: 2
        tagged: ether2

I tried few approaches, using the dict filter, using to/from json/yaml filters, creating the dict with inline jinja2, using block scalar to avoid the quotes around the variable, etc. but only dict() seems to return the correct data type.

I'm not sure if this issue is tied directly to the api_find_and_modify module as I'm able to replicate the issue with ansible.builtin.debug. But perhaps some workarounds can be integrated in the module.

- name: "Debug vlans"
  ansible.builtin.debug:
    msg: { vlan-id: "{{ item.vlan_id }}" }
  loop: "{{ __vlans }}"
  vars:
    vlan_id: 2
    __vlans:
      - bridge: bridge0
        vlan_id: 2
        tagged: ether3

Incorrectly returns a string:

 {
    "msg": {
        "vlan-id": "2"
    }
}
- name: "Debug vlans"
  ansible.builtin.debug:
    msg: { vlan-id: "{{ vlan_id }}" }
  loop: "{{ __vlans }}"
  vars:
    vlan_id: 2
    __vlans:
      - bridge: bridge0
        vlan_id: 2
        tagged: ether3

Correctly returns a int:

 {
    "msg": {
        "vlan-id": 2
    }
}

gardar avatar Oct 28 '24 02:10 gardar

Maybe it's best to do all value comparisons by first converting both values to strings. The Python library we use to communicate with the API does some conversions: when receiving values from the API, it changes yes/no and true/false to booleans, values that looks like integers to integers, and keeps the rest. When sending values to the API, it converts booleans to yes/no, converts everything else directly to strings with str(value): https://github.com/luqasz/librouteros/blob/62700c2532daba29fde3ea350c6130f77954e420/librouteros/protocol.py#L17-L41

So if we do a similar value to string conversion before comparison, these problems should go away.

felixfontein avatar Nov 23 '24 12:11 felixfontein

Yes that would be logical, but I'm still baffled why the behavior is different between a loop and a non-loop

gardar avatar Nov 23 '24 12:11 gardar

That's a result of how templating works in Ansible. IMO the main problem is that it's hard (or next to impossible) to distinguish between templating and expression evaluation. I've once wrote some ideas down here: https://forum.ansible.com/t/4386 Apparently everything will get better with data tagging - assuming that ever happens. (It's now supposed to come in ansible-core 2.19, but then it was already supposed to be there in several previous versions, so :shrug: It will likely also break a lot of existing playbooks/roles if it is supposed to fix things like the above, since you can bet on that someone actually depends on the behavior that we think is wrong...)

felixfontein avatar Nov 23 '24 13:11 felixfontein

Found a workaround for the api_find_and_modify data type issue when using loops by using the dict() function.

  • name: Configure vlans on bridge # noqa args[module] community.routeros.api_find_and_modify: path: "interface bridge vlan" find: >- {{ dict([ ['vlan-ids', item.vlan_id], ['bridge', item.bridge] ]) }} values: vlan-ids: "{{ item.vlan_id }}" bridge: "{{ item.bridge }}" tagged: "{{ item.tagged }}" loop: "{{ __vlans }}" vars: __vlans: - bridge: bridge0 vlan_id: 2 tagged: ether2 The same data type issue also affects the values field of the api_find_and_modify when using a loop and the task is not immutable and always shows as changed even though the old_data and new_data shown in the return values is identical. But using dict() as a workaround resolves that issue as well.

  • name: Configure vlans on bridge # noqa args[module] community.routeros.api_find_and_modify: path: "interface bridge vlan" find: >- {{ dict([ ['vlan-ids', item.vlan_id], ['bridge', item.bridge] ]) }} values: >- {{ dict([ ['vlan-ids', item.vlan_id], ['bridge', item.bridge], ['tagged', item.tagged] ]) }} loop: "{{ __vlans }}" vars: __vlans: - bridge: bridge0 vlan_id: 2 tagged: ether2 I tried few approaches, using the dict filter, using to/from json/yaml filters, creating the dict with inline jinja2, using block scalar to avoid the quotes around the variable, etc. but only dict() seems to return the correct data type.

I'm not sure if this issue is tied directly to the api_find_and_modify module as I'm able to replicate the issue with ansible.builtin.debug. But perhaps some workarounds can be integrated in the module.

  • name: "Debug vlans" ansible.builtin.debug: msg: { vlan-id: "{{ item.vlan_id }}" } loop: "{{ __vlans }}" vars: vlan_id: 2 __vlans: - bridge: bridge0 vlan_id: 2 tagged: ether3 Incorrectly returns a string:

{ "msg": { "vlan-id": "2" } }

  • name: "Debug vlans" ansible.builtin.debug: msg: { vlan-id: "{{ vlan_id }}" } loop: "{{ __vlans }}" vars: vlan_id: 2 __vlans: - bridge: bridge0 vlan_id: 2 tagged: ether3 Correctly returns a int:

{ "msg": { "vlan-id": 2 } }

thanks for the workaround - was messing around at the same location and nearly went crazy

michaelwdombek avatar Feb 08 '25 13:02 michaelwdombek

Yes, thanks for the work-around! I too was getting jammed up on the same path.

@felixfontein I think your suggestion to add string conversion before the comparison makes a lot of sense, even if only for applying the restrictions in api_modify.

And for anyone else looking for a full upsert solution: since api_find_and_modify only works if there's an existing record found, you can kludge together two calls to get the job done:

- name: Update VLAN on bridge if exists
  community.routeros.api_find_and_modify:
    path: interface bridge vlan
    find: >-
      {{
        dict([
          ['bridge', bridge_name],
          ['vlan-ids', vlan.id],
        ])
      }}
    values:
      bridge: "{{ bridge_name }}"
      tagged: "{{ trunks | join(',') }}"
      vlan-ids: "{{ vlan.id }}"
      comment: "{{ label }}"
  register: update_on_bridge

- name: Add VLAN to bridge
  community.routeros.api_modify:
    path: interface bridge vlan
    data:
      - bridge: "{{ bridge_name }}"
        tagged: "{{ trunks | join(',') }}"
        vlan-ids: "{{ vlan.id }}"
        comment: "{{ label }}"
  when: update_on_bridge.match_count == 0

schwartzie avatar Aug 14 '25 13:08 schwartzie

I've created #397 which compares values as strings. I hope this fixes the problems reported here.

felixfontein avatar Sep 13 '25 19:09 felixfontein