[Feature]: netbox_device_interface - untagged_vlan can't be removed

[ltDan79]

ISSUE TYPE

• Bug Report

SOFTWARE VERSIONS

Ansible:

2.9.123

Netbox:

2.8.8

Collection:

1.0.1

SUMMARY

@FragmentedPacket
As disscussed on #322, I open an other issue to track the untagged removed vlan bug.
Thx

STEPS TO REPRODUCE

First run

- hosts: cmdb_devices_config
  connection: local
  tasks:
  - name: Configure Devices Interfaces cmdb groups vars
    collections:
      - netbox.netbox
    netbox_device_interface:
      netbox_url: "{{ netbox_url }}"
      netbox_token: "{{ netbox_token }}"
      data:
        device: am-2960-01
        name: GigabitEthernet1/0/3
        description: 802.1x Access Port
        mtu: 1600
        enabled: true
        mode: Access
        untagged_vlan:
          name: Vlan1403
          site: am
        tags: [ 802.1x ]
      state: present

second run

- hosts: cmdb_devices_config
  connection: local
  tasks:
  - name: Configure Devices Interfaces cmdb groups vars
    collections:
      - netbox.netbox
    netbox_device_interface:
      netbox_url: "{{ netbox_url }}"
      netbox_token: "{{ netbox_token }}"
      data:
        device: am-2960-01
        name: GigabitEthernet1/0/3
        description: 802.1x Access Port
        mtu: 1600
        enabled: true
        mode: Access
        untagged_vlan: null
        tags: [ 802.1x ]
      state: present

EXPECTED RESULTS

untagged Vlan removed

ACTUAL RESULTS

untagged Vlan not removed

[FragmentedPacket]

What I'm thinking of doing is having the user set something like the following from your example:

- hosts: cmdb_devices_config
  connection: local
  tasks:
  - name: Configure Devices Interfaces cmdb groups vars
    collections:
      - netbox.netbox
    netbox_device_interface:
      netbox_url: "{{ netbox_url }}"
      netbox_token: "{{ netbox_token }}"
      data:
        device: am-2960-01
        name: GigabitEthernet1/0/3
        description: 802.1x Access Port
        mtu: 1600
        enabled: true
        mode: Access
        untagged_vlan:
          nullify: True
        tags: [ 802.1x ]
      state: present

And then in netbox_utils

    def _normalize_data(self, data):
        """
        :returns data (dict): Normalized module data to formats accepted by Netbox searches
        such as changing from user specified value to slug
        ex. Test Rack -> test-rack
        :params data (dict): Original data from Netbox module
        """
        for k, v in data.items():
            if isinstance(v, dict):
                if v.get("id"):
                    try:
                        data[k] = int(v["id"])
                    except (ValueError, TypeError):
                        pass
                elif v.get("nullify"):
                    data[k] = None
                else:
                    for subk, subv in v.items():
                        sub_data_type = QUERY_TYPES.get(subk, "q")
                        if sub_data_type == "slug":
                            data[k][subk] = self._to_slug(subv)
            else:
                data_type = QUERY_TYPES.get(k, "q")
                if data_type == "slug":
                    data[k] = self._to_slug(v)
                elif data_type == "timezone":
                    if " " in v:
                        data[k] = v.replace(" ", "_")
            if k == "description":
                data[k] = v.strip()

            if k == "mac_address":
                data[k] = v.upper()

        return data

[ltDan79]

@FragmentedPacket that will works, however it's not so intuitive compared to the tagged_vlan parameter which is implecitly removed when not specified . It will therefore also require an update of the documentation.

[FragmentedPacket]

Let me play around with it, but we already use the pattern for converting a Jinja strong to ID but definitely needs to be documented

[FragmentedPacket]

Maybe having the value as nullify? I think the reason we remove the default args (they set it as None), it would remove the fields if they weren't specified, which puts the burden on the user to specify them every time.

[ltDan79]

@FragmentedPacket In my opinion infrastructure as Code must be deterministic. So it's not a problem to have this type of behaviour.
For example it's strange to have to set the tags fields to an empty list (tags: []) to remove tags, an other behaviour for tagged_vlan, and an other one for untagged_vlan.

[jvanderaa]

I think that this should follow what the other network modules are doing. There are modes of:

• Overridden: Full IAC mode that will overwrite anything that is not set with a null value
• Merge: Traditional behavior, where if a parameter is not set then nothing is done
• Deleted: Removal
• Gathered: Get information for resource module
• Replaced: sets information for any parameter set

[andybro19]

@FragmentedPacket what's the progress on adding the ability for users to set fields to None?

I have another use case that could be useful to be able to do. Currently, with the netbox.netbox.netbox_ip_address module when you set data.assigned_object: null it just produces an 'ok' result and doesn't remove the ip address from the interface. Similarly, when you set data.assigned_object: None it produces an error and doesn't remove the ip address from the interface. There's also many other fields that would be useful to be able to set to None, like removing a device from a rack or cluster, removing a tenant, etc.

[FragmentedPacket]

Hi @andybro19, I have not and this would be a pretty big change so I haven't really had time to implement this. I do have some thoughts and am trying to think the best way to cover all the use cases without it increasing the complexity of these modules.

This would cover all fields of an object rather than a single one.

To @jvanderaa's point, there are tons of different states to implement to cover the full range of use cases.

[sc68cal]

I agree with #330 (comment) - however that is going to take a lot of work to implement.

[jantari]

Just wanted to share that I ran into this problem as well, with prefixes.

Some of our prefixes that are not in any VLAN had erroneously been assigned a "VLAN 1" 🤦

So when updating these through ansible I had to make sure any VLAN information on these is removed.

Here's the workaround I use and it works very well and is idempotent too:

- name: Create prefixes with VLANs
  netbox.netbox.netbox_prefix:
    netbox_url: '{{ netbox_url }}'
    netbox_token: '{{ netbox_token }}'
    data:
      prefix: "{{ item.subnet | ansible.netcommon.ipaddr('network/prefix') }}"
      site: "{{ item.firewall_location_long_name }}"
      description: '{{ item.comment }}'
      status: Active
      tags: [Ansible]
      vlan:
        name: "{{ item.name }}"
        site: "{{ item.firewall_location_long_name }}"
    state: present
  loop: "{{ _address_objects | selectattr('vlanid', 'defined') | rejectattr('vlanid', 'none') }}"
  loop_control:
    label: "{{ item.firewall_location_short_name }}-{{ item.name }} ( {{ item.subnet }} )"

- name: Create prefixes without VLANs
  netbox.netbox.netbox_prefix:
    netbox_url: '{{ netbox_url }}'
    netbox_token: '{{ netbox_token }}'
    data:
      prefix: "{{ item.subnet | ansible.netcommon.ipaddr('network/prefix') }}"
      site: "{{ item.firewall_location_long_name }}"
      description: '{{ item.comment }}'
      status: Active
      tags: [Ansible]
      vlan: null # This doesn't unset any information, this is just ignored by the module, but explicit null is nicer to read for humans.
    state: present
  loop: >
    {{
      (_address_objects | selectattr('vlanid', 'undefined')) +
      (_address_objects | selectattr('vlanid', 'defined') | selectattr('vlanid', 'none'))
    }}
  loop_control:
    label: "{{ item.firewall_location_short_name }}-{{ item.name }} ( {{ item.subnet }} )"
  register: _netbox_prefixes_no_vlan

# Because unsetting / nulling a property with the netbox.netbox.netbox_prefix module
# is not possible right now ( https://github.com/netbox-community/ansible_modules/issues/330 )
# we do a "manual" API call to Netbox to make sure VLAN-less networks do not have a VLAN set.
# This can happen when someone manually edits a prefix, and does it wrong (e.g. no VLAN != VLAN 1)!
- name: Unset any VLAN information for prefixes without VLAN
  ansible.builtin.uri:
    url: 'https://netbox.hlb-intern.de/api/ipam/prefixes/{{ item.id }}/'
    method: PATCH
    headers:
      Content-Type: application/json
      Authorization: "Token {{ netbox_token }}"
    body_format: json
    body:
      vlan: null
  loop: "{{ _netbox_prefixes_no_vlan.results | map(attribute='prefix') }}"
  loop_control:
    label: "{{ item.prefix }} - {{ item.description }}"
  changed_when: true # This task only ever runs when a change needs to be made, so every run is a change
  when: item.vlan is not none # Only run when VLAN information has to be corrected

Basically, I have previously sourced all of my subnet/prefix information, transformed it into a data format I can work with easily and then put it into the variable _address_objects.

Then I split the those into the prefixes that are in a VLAN (vlanid defined and non-null) and the ones that are not (vlanid either undefined or null) and create them in separate tasks. By registering the output of the task creating the non-VLAN-prefixes, I can get a nice list of all their netbox-objects and thus their netbox ID. By looping over that in the last task shown here, I can easily issue a small PATCH API call with ansibles own ansible.builtin.uri module to remove any VLAN information from those prefixes if there is any. Because of the when: statement on that last task, it only ever runs when it needs to so is idempotent.

This can of course be adapted to your data schema / formats and to any other netbox objects other than prefix - hope it helps!