CiscoDevNet / ansible-aci

Cisco ACI Ansible Collection
https://galaxy.ansible.com/cisco/aci
GNU General Public License v3.0
140 stars 96 forks source link

`aci_domain_to_vlan_pool` "absent" state removes the whole domain and not only the binding (DCNE-207) #694

Open edudppaz opened 1 week ago

edudppaz commented 1 week ago

Community Note

Description

Setting state "absent" on a aci_domain_to_vlan_pool task removes the whole domain and not only the binding to the vlan pool

Affected Module Name(s):

APIC version and APIC Platform

Collection versions

Output/ Error message

*

Expected Behavior

Actual Behavior

Playbook tasks to Reproduce

First:

- name: Bind a physical domain to VLAN pool
  cisco.aci.aci_domain_to_vlan_pool:
    host: apic
    username: admin
    password: SomeSecretPassword
    domain: phys_dom
    domain_type: phys
    pool: phys_pool
    pool_allocation_mode: static
    state: present
  delegate_to: localhost

Then:

- name: Remove binding from physical domain to VLAN pool
  cisco.aci.aci_domain_to_vlan_pool:
    host: apic
    username: admin
    password: SomeSecretPassword
    domain: phys_dom
    domain_type: phys
    pool: phys_pool
    pool_allocation_mode: static
    state: absent
  delegate_to: localhost

Important Factoids

From the code i can see that on the module, the url endpoint is being build using the whole domain as an object:

    aci.construct_url(
        root_class=dict(
            aci_class=domain_class,
            aci_rn=domain_rn,
            module_object=domain_mo,
            target_filter={"name": domain},
        ),
        child_classes=["infraRsVlanNs"],
    )

So it makes sense that setting "absent" and running aci.delete_config() deletes the whole domain The state absent should be a new payload that removes the binding, something like this (just writing it on the fly, must be checked and tested:

    elif state == "absent":
        aci.payload(
            aci_class=domain_class,
            class_config=dict(name=domain),
            child_configs=[
                {"infraRsVlanNs": {"status": "deleted" }},
            ],
        )
        aci.post_config()

This is based on a test done directly on the GUI with the API inspector:

method: POST
url: https://10.10.10.10/api/node/mo/uni/phys-phydom_ep_test/rsvlanNs.json
payload{"infraRsVlanNs":{"attributes":{"dn":"uni/phys-phydom_ep_test/rsvlanNs","status":"deleted"},"children":[]}}

References

akinross commented 1 week ago

Hi @edudppaz,

Thank you for raising this issue, I have added it to the backlog. If you would like to work on it yourself and push a PR please let me know.

edudppaz commented 1 week ago

Hi @akinross , i have created this pr #695 , tested on my local environment and seems to be working as expected.