CiscoDevNet / ansible-dcnm

Apache License 2.0
47 stars 37 forks source link

dcnm_fabric: replaced state implicit change handling. #305

Closed allenrobel closed 5 months ago

allenrobel commented 5 months ago

Summary

Submitting issue found by @mikewiebe where replaced state does not set a fabric parameter to default if the playbook does not include the parameter (implicit change). In Mike's case, the fabric is initially configured with NTP_SERVER_IP_LIST: <ip_address>. A playbook is then run, with dcnm_fabric task state set to replaced where the task does not include the NTP_SERVER_IP_LIST parameter. In this case, NTP_SERVER_IP_LIST should be set to the default value of "".

Community Note

Ansible Version and collection version

ansible [core 2.16.6]
  config file = /Users/arobel/.ansible.cfg
  configured module search path = ['/Users/arobel/repos/ansible_dev/dcnm_fabric/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/arobel/repos/ansible_dev/dcnm_fabric/ansible_collections/cisco/dcnm/.venv/lib/python3.12/site-packages/ansible
  ansible collection location = /Users/arobel/repos/ansible_dev/dcnm_fabric
  executable location = /Users/arobel/repos/ansible_dev/dcnm_fabric/ansible_collections/cisco/dcnm/.venv/bin/ansible
  python version = 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] (/Users/arobel/repos/ansible_dev/dcnm_fabric/ansible_collections/cisco/dcnm/.venv/bin/python)
  jinja version = 3.1.4
  libyaml = True

DCNM version

Affected module(s)

Ansible Playbook

Start with the following fabric.

---
-   hosts: ndfc
    check_mode: false
    gather_facts: false
    tasks:
        - cisco.dcnm.dcnm_fabric:
            state: merged
            config:
            -   FABRIC_NAME: FOO
                FABRIC_TYPE: VXLAN_EVPN
                BGP_AS: 65535
                NTP_SERVER_IP_LIST: "1.1.1.1"
                NTP_SERVER_VRF: "management"
                DEPLOY: true

Then send the following replaced state playbook.


-   hosts: ndfc
    check_mode: false
    gather_facts: false
    tasks:
        - cisco.dcnm.dcnm_fabric:
            state: replaced
            config:
            -   FABRIC_NAME: FOO
                FABRIC_TYPE: VXLAN_EVPN
                BGP_AS: 65535
                DEPLOY: true

Debug Output

The problem is understood, so no debugging output is needed.

Expected Behavior

The NTP server configuration for the fabric should be removed.

Actual Behavior

The NTP server configuration is not removed.

Steps to Reproduce

See Ansible Playbook section above.

References

None

Workaround

Include the fields in the playbook with the changes desired. For example, the following playbook will work to remove the NTP server configuration.

---
-   hosts: ndfc
    check_mode: false
    gather_facts: false
    tasks:
        - cisco.dcnm.dcnm_fabric:
            state: replaced
            config:
            -   FABRIC_NAME: FOO
                FABRIC_TYPE: VXLAN_EVPN
                BGP_AS: 65535
                NTP_SERVER_IP_LIST: ""
                NTP_SERVER_VRF: ""
                DEPLOY: true

Dev Notes

Need to look at the logic in dcnm_fabric/replaced.py for FabricReplacedCommon().update_replaced_payload() The values upon entering this method when the above replaced state playbook is run are:

FabricReplacedBulk.update_replaced_payload: parameter: NTP_SERVER_IP_LIST, playbook: None, controller: 1.1.1.1, default: None

Since playbook is None the if playbook is None: conditional is entered (see below).

Since default is None, we return None and this is the problem.

We need to check here if controller != default, and return {parameter: default} in this case.

    def update_replaced_payload(self, parameter, playbook, controller, default):
        if playbook is None:
            if default is None:
                return None
            if controller == default:
                return None
            if controller is None or controller == "":
                return None
            return {parameter: default}
        if playbook == controller:
            return None
        return {parameter: playbook}