BGP Password Idempotency Problem in aci_l3out_bgp_peer

aj-cruz commented 1 month ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

When setting a BGP password within the aci_l3out_bgp_peer module, each run of the playbook will report a change in the peer object.

Not sure if this is possible with passwords, but I noticed when I use aci_rest setting and changing passwords works properly

Affected Module Name(s):

aci_l3out_bgp_peer

APIC version and APIC Platform

ACI Simulator 5.3(2b)

Collection versions

cisco.aci 2.9.0

Output/ Error message

TASK [Configure Loopback BGP Peers] *****************************************************************************************************************************************
changed: [dc1_apic1 -> localhost]

PLAY RECAP ******************************************************************************************************************************************************************
dc1_apic1                  : ok=6    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Expected Behavior

With no changes to the variables file, subsequent runs of the playbook report no changes

Actual Behavior

With no changes to the variables file, subsequent runs of the playbook always report a change in the bgp peer module

Playbook tasks to Reproduce

- name: CONFIGURE ACI L3OUT
  hosts: dc1_apic1
  gather_facts: false
  collections:
    - cisco.aci

  tasks:
    - name: Create Login Anchor
      set_fact:
        aci_login: &aci_login
          host: "{{ apic_host }}"
          username: "{{ apic_user }}"
          certificate_name: "{{ certificate_name }}"
          private_key: "{{ private_key }}"
          use_proxy: "{{ apic_use_proxy }}"
          validate_certs: "{{ apic_validate_certs }}"

    - name: Configure Tenant
      aci_tenant:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
      delegate_to: localhost

    - name: Configure VRF
      aci_vrf:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
        vrf: Test-VRF
      delegate_to: localhost

    - name: Configure L3Out
      aci_l3out:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
        name: Test-L3Out
        vrf: Test-VRF
        domain: phys
        l3protocol: bgp
      delegate_to: localhost

    - name: Add Node Profile to L3Out
      cisco.aci.aci_l3out_logical_node_profile:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
        l3out: Test-L3Out
        node_profile: Test-NodePro
      delegate_to: localhost

    - name: Configure Loopback BGP Peers
      cisco.aci.aci_l3out_bgp_peer:
        <<: *aci_login
        tenant: Test-Tenant
        l3out: Test-L3Out
        node_profile: Test-NodePro
        peer_ip: 2.2.2.2
        bgp_password: mypassword
      delegate_to: localhost

Important Factoids

If I do it with aci_rest with the following example playbook, it works fine:

- name: CONFIGURE ACI L3OUT
  hosts: dc1_apic1
  gather_facts: false
  collections:
    - cisco.aci

  tasks:
    - name: Create Login Anchor
      set_fact:
        aci_login: &aci_login
          host: "{{ apic_host }}"
          username: "{{ apic_user }}"
          certificate_name: "{{ certificate_name }}"
          private_key: "{{ private_key }}"
          use_proxy: "{{ apic_use_proxy }}"
          validate_certs: "{{ apic_validate_certs }}"

    - name: Configure Tenant
      aci_tenant:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
      delegate_to: localhost

    - name: Configure VRF
      aci_vrf:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
        vrf: Test-VRF
      delegate_to: localhost

    - name: Configure L3Out
      aci_l3out:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
        name: Test-L3Out
        vrf: Test-VRF
        domain: phys
        l3protocol: bgp
      delegate_to: localhost

    - name: Add Node Profile to L3Out
      cisco.aci.aci_l3out_logical_node_profile:
        <<: *aci_login
        state: present
        tenant: Test-Tenant
        l3out: Test-L3Out
        node_profile: Test-NodePro
      delegate_to: localhost

    - name: Configure Loopback BGP Peers
      aci_rest:
        <<: *aci_login
        path: "/api/node/mo/uni/tn-Test-Tenant/out-Test-L3Out/lnodep-Test-NodePro/peerP-[2.2.2.2].json"
        method: post
        content:
          bgpPeerP:
            attributes:
              dn: "uni/tn-Test-Tenant/out-Test-L3Out/lnodep-Test-NodePro/peerP-[2.2.2.2]"
              addr: 2.2.2.2
              password: mybgppassword
      delegate_to: localhost

References

akinross commented 1 month ago

Hi @aj-cruz,

This is expected in the module, see issue https://github.com/CiscoDevNet/ansible-aci/issues/645 with a linked PR that updates the documentation for more details.

There is another PR open to address the documentation throughout the repository, see https://github.com/CiscoDevNet/ansible-aci/pull/655.

akinross commented 1 month ago

Hi @aj-cruz, did this clarify it for you?

aj-cruz commented 1 month ago

@akinross Yes thank you it can be closed. Though I am curious, what is different about the way aci_rest functions that it works properly with it?

akinross commented 1 month ago

Hi @aj-cruz,

If the request done in aci_rest is a POST/DELETE request we set the ?rsp-subtree=modified flag. This flag will return only modified attributes, and determines based on the return of the request if it has changed or not.

This is done because there is no initial GET request done like we do in regular modules to determine the difference and whether we should send changes to apic or not.

CiscoDevNet / ansible-aci