CiscoDevNet / ansible-meraki

Cisco Meraki Ansible Collection
GNU General Public License v3.0
60 stars 44 forks source link

meraki_mx_vlan idempotency issues #434

Open mystery-rabbit opened 1 year ago

mystery-rabbit commented 1 year ago

Two parameters of meraki_mx_vlan command have idempotency issues:

the following can be used to reproduce the first issue and with commenting, the second.:

---
- name: "Demo idempotency issue with VLAN"
  vars:
    site_cidr: 10.11.0.0/16
    vlans:
      - name: default
        vlan_id: 1
        subnet: "{{ site_cidr | ansible.utils.ipsubnet(24, 0) }}"
        appliance_ip: "{{ site_cidr | ansible.utils.ipsubnet(24, 0) | ansible.utils.nthhost(1) }}"
        vpn_nat_subnet: ""
        dns_nameservers: "upstream_dns"
        dhcp_handling: "Run a DHCP server"
        dhcp_relay_server_ips: ""
        dhcp_lease_time: "1 day"
        dhcp_boot_options_enabled: false
        dhcp_boot_next_server: ""
        dhcp_boot_filename: ""
        dhcp_options: []
        fixed_ip_assignments: []
        reserved_ip_range: []
  hosts: all
  tasks:

    - name: Delete VLANs
      cisco.meraki.meraki_mx_vlan:
        org_name: "{{ meraki_organisation.name }}"
        net_name: "{{ inventory_hostname }}"
        state: "absent"
        vlan_id: "{{ item.vlan_id }}"
      loop: "{{ vlans }}"

    - name: Create VLANs 1st time
      cisco.meraki.meraki_mx_vlan:
        org_name: "{{ meraki_organisation.name }}"
        net_name: "{{ inventory_hostname }}"
        state: "present"
        name: "{{ item.name }}"
        vlan_id: "{{ item.vlan_id }}"
        subnet: "{{ item.subnet }}"
        appliance_ip: "{{ item.appliance_ip }}"
        vpn_nat_subnet: "{{ item.vpn_nat_subnet }}"

        dns_nameservers: "{{ item.dns_nameservers }}"

        dhcp_handling: "{{ item.dhcp_handling }}"
        dhcp_relay_server_ips: "{{ item.dhcp_relay_server_ips }}"
        dhcp_lease_time: "{{ item.dhcp_lease_time }}"
        dhcp_boot_options_enabled: "{{ item.dhcp_boot_options_enabled }}"
        dhcp_boot_next_server: "{{ item.dhcp_boot_next_server }}"
        dhcp_boot_filename: "{{ item.dhcp_boot_filename }}"
        dhcp_options: "{{ item.dhcp_options }}"

        fixed_ip_assignments: "{{ item.fixed_ip_assignments }}"
        reserved_ip_range: "{{ item.reserved_ip_range }}"
      loop: "{{ vlans }}"

   ## Issue:: this is not idempotent; cannot be used to configure/reconfigure a VLAN.
    - name: Create VLANs 2nd time
      cisco.meraki.meraki_mx_vlan:
        org_name: "{{ meraki_organisation.name }}"
        net_name: "{{ inventory_hostname }}"
        state: "present"
        name: "{{ item.name }}"
        vlan_id: "{{ item.vlan_id }}"
        subnet: "{{ item.subnet }}"
        appliance_ip: "{{ item.appliance_ip }}"
        vpn_nat_subnet: "{{ item.vpn_nat_subnet }}"

        dns_nameservers: "{{ item.dns_nameservers }}"

        dhcp_handling: "{{ item.dhcp_handling }}"
        ## Issue ^^ idempotency issue.
        dhcp_relay_server_ips: "{{ item.dhcp_relay_server_ips }}"
        ## Issue ^^ NoneType object is not subscriptable
        dhcp_lease_time: "{{ item.dhcp_lease_time }}"
        dhcp_boot_options_enabled: "{{ item.dhcp_boot_options_enabled }}"
        dhcp_boot_next_server: "{{ item.dhcp_boot_next_server }}"
        dhcp_boot_filename: "{{ item.dhcp_boot_filename }}"
        dhcp_options: "{{ item.dhcp_options }}"

        fixed_ip_assignments: "{{ item.fixed_ip_assignments }}"
        reserved_ip_range: "{{ item.reserved_ip_range }}"
      loop: "{{ vlans }}"

example run:

(.venv) vagrant@terra01:~/meraki-as-code/ansible$ ansible-playbook demo_mx_vlan_bug.yml -i inv_xxx.yml -l "Mock*Branch*2"

PLAY [Demo idempotency issue with VLAN] ********************************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************************************************************************************************************************************
ok: [Mock Branch 2]

TASK [Delete VLANs] ****************************************************************************************************************************************************************************************************************************************************
changed: [Mock Branch 2] => (item={'name': 'default', 'vlan_id': 1, 'subnet': '10.11.0.0/24', 'appliance_ip': '10.11.0.1', 'vpn_nat_subnet': '', 'dns_nameservers': 'upstream_dns', 'dhcp_handling': 'Run a DHCP server', 'dhcp_relay_server_ips': '', 'dhcp_lease_time': '1 day', 'dhcp_boot_options_enabled': False, 'dhcp_boot_next_server': '', 'dhcp_boot_filename': '', 'dhcp_options': [], 'fixed_ip_assignments': [], 'reserved_ip_range': []})

TASK [Create VLANs 1st time] *******************************************************************************************************************************************************************************************************************************************
changed: [Mock Branch 2] => (item={'name': 'default', 'vlan_id': 1, 'subnet': '10.11.0.0/24', 'appliance_ip': '10.11.0.1', 'vpn_nat_subnet': '', 'dns_nameservers': 'upstream_dns', 'dhcp_handling': 'Run a DHCP server', 'dhcp_relay_server_ips': '', 'dhcp_lease_time': '1 day', 'dhcp_boot_options_enabled': False, 'dhcp_boot_next_server': '', 'dhcp_boot_filename': '', 'dhcp_options': [], 'fixed_ip_assignments': [], 'reserved_ip_range': []})

TASK [Create VLANs 2nd time] *******************************************************************************************************************************************************************************************************************************************
failed: [Mock Branch 2] (item={'name': 'default', 'vlan_id': 1, 'subnet': '10.11.0.0/24', 'appliance_ip': '10.11.0.1', 'vpn_nat_subnet': '', 'dns_nameservers': 'upstream_dns', 'dhcp_handling': 'Run a DHCP server', 'dhcp_relay_server_ips': '', 'dhcp_lease_time': '1 day', 'dhcp_boot_options_enabled': False, 'dhcp_boot_next_server': '', 'dhcp_boot_filename': '', 'dhcp_options': [], 'fixed_ip_assignments': [], 'reserved_ip_range': []}) => {"ansible_loop_var": "item", "changed": false, "item": {"appliance_ip": "10.11.0.1", "dhcp_boot_filename": "", "dhcp_boot_next_server": "", "dhcp_boot_options_enabled": false, "dhcp_handling": "Run a DHCP server", "dhcp_lease_time": "1 day", "dhcp_options": [], "dhcp_relay_server_ips": "", "dns_nameservers": "upstream_dns", "fixed_ip_assignments": [], "name": "default", "reserved_ip_range": [], "subnet": "10.11.0.0/24", "vlan_id": 1, "vpn_nat_subnet": ""}, "msg": "HTTP error 400 - https://api.meraki.com/api/v1/networks/L_690176642894537554/appliance/vlans/1 - 'dhcpHandling' must be a string", "response": "OK (unknown bytes)", "status": 400}

PLAY RECAP *************************************************************************************************************************************************************************************************************************************************************
Mock Branch 2              : ok=3    changed=2    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0 
mystery-rabbit commented 1 year ago

Version of module:

# /home/vagrant/meraki-as-code/ansible/roles/community/ansible_collections
Collection   Version
------------ -------
cisco.meraki 2.15.0 
kbreit commented 1 year ago

Thank you for reporting this. I'm looking into it now.

kbreit commented 1 year ago

Update. I've found the bug responsible for the first error. 1 line fix.

kbreit commented 1 year ago

Regarding the second one, this one is harder. I think this error comes from dhcp_relay_server_ips and dhcp_handling == "Run a DHCP server" my (perceived) fact that they don't make a lot of sense to run together. If a network is running a DHCP server, it won't have to have a DHCP relay setup. Meraki isn't including dhcp_relay_server_ips in the response so the diff comparison breaks and that's I think why it's not idempotent.

Do you have a use case where you need to specify both?

mystery-rabbit commented 1 year ago

I agree with your analysis - i only stumbled across it trying to work out which attribute was throwing the original error. As for use case, no - nothing that couldn't be solved with some playbook logic - multiple calls and some "when x is defined" sort of thing.

kbreit commented 1 year ago

I think I could also have it strip that from the payload since it's incompatible. Would that create any problems you could imagine?

mystery-rabbit commented 1 year ago

No, it wouldn't, they are mutually exclusive in application. Is that compatible with the general approach and philosophy used in the rest of the project? are the modules in general acting as idempotent interfaces to the API - thus allowing me to malform a request should i wish to - or is including that sanity check part of the overall philosophy of the project? if it doesn't clash with your approach, that would make sense (and it should be set when dhcp_handling: "Relay DHCP to another server". )

ppodgorsek commented 1 year ago

I am facing the same issue with a much shorter declaration:

- name: Create VLAN
  cisco.meraki.meraki_mx_vlan:
    auth_key: "{{ meraki_authentication_key }}"
    org_id: "{{ meraki_organisation_details.data.id }}"
    net_id: "{{ meraki_network_details.data.id }}"
    state: present
    vlan_id: "{{ meraki_vlan_id }}"
    name: "{{ meraki_vlan_name }}"
    subnet: "192.168.10.1/24"
    appliance_ip: "192.168.10.1"
    dns_nameservers: google_dns
  delegate_to: localhost

Here's what the module error contains:

The full traceback is: Traceback (most recent call last): File "/.ansible/tmp/ansible-tmp-1686661857.357004-7648-85579569599896/AnsiballZ_meraki_mx_vlan.py", line 107, in _ansiballz_main() File "/.ansible/tmp/ansible-tmp-1686661857.357004-7648-85579569599896/AnsiballZ_meraki_mx_vlan.py", line 99, in _ansiballz_main invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS) File "/.ansible/tmp/ansible-tmp-1686661857.357004-7648-85579569599896/AnsiballZ_meraki_mx_vlan.py", line 47, in invoke_module runpy.run_module(mod_name='ansible_collections.cisco.meraki.plugins.modules.meraki_mx_vlan', init_globals=dict(_module_fqn='ansible_collections.cisco.meraki.plugins.modules.meraki_mx_vlan', _modlib_path=modlib_path), File "", line 226, in run_module File "", line 98, in _run_module_code File "", line 88, in _run_code File "/tmp/ansible_cisco.meraki.meraki_mx_vlan_payload_ysykjkpk/ansible_cisco.meraki.meraki_mx_vlan_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_mx_vlan.py", line 585, in File "/tmp/ansible_cisco.meraki.meraki_mx_vlan_payload_ysykjkpk/ansible_cisco.meraki.meraki_mx_vlan_payload.zip/ansible_collections/cisco/meraki/plugins/modules/meraki_mx_vlan.py", line 562, in main File "/tmp/ansible_cisco.meraki.meraki_mx_vlan_payload_ysykjkpk/ansible_cisco.meraki.meraki_mx_vlan_payload.zip/ansible_collections/cisco/meraki/plugins/module_utils/network/meraki/meraki.py", line 205, in generate_diff TypeError: 'NoneType' object is not subscriptable

ppodgorsek commented 1 year ago

Found the cause of my problem: the subnet was 192.168.10.1/24 instead of 192.168.10.0/24. Meraki stores the CIDR using the first IP of the range (192.168.10.0/24) so it was always highlighted as a change but there was nothing to apply when the Ansible module was trying to update the resource.

kbreit commented 1 year ago

Thanks for the update. I'll take a look at the original bug still though.