Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dcnm_image_policy: overridden state should replace rather than merge image policy configurations #347

Closed
allenrobel opened this issue Nov 21, 2024 · 1 comment · Fixed by #348
Assignees
Labels
bug Something isn't working

Comments

@allenrobel
Copy link
Collaborator

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

Ansible Version and collection version

(.venv) arobel@AROBEL-M-G793 dcnm % ansible-galaxy collection list | grep dcnm
cisco.dcnm                               3.6.0
(.venv) arobel@AROBEL-M-G793 dcnm %
ansible [core 2.17.5]
  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/ndfc-python/.venv/lib/python3.12/site-packages/ansible
  ansible collection location = /Users/arobel/repos/ansible/collections
  executable location = /Users/arobel/repos/ndfc-python/.venv/bin/ansible
  python version = 3.12.4 (main, Jun  6 2024, 18:26:44) [Clang 15.0.0 (clang-1500.3.9.4)] (/Users/arobel/repos/ndfc-python/.venv/bin/python)
  jinja version = 3.1.4
  libyaml = True

DCNM version

  • V 3.6.0

Affected module(s)

  • dcnm_image_policy

Ansible Playbook

---
- name: dcnm_image_policy override
  hosts: ndfc
  tasks:
    - name: override image policy KR5M
      cisco.dcnm.dcnm_image_policy:
        state: overridden
        config:
        - name: KR5M
          agnostic: false
          description: "KR5M overridden"
          epld_image: n9000-epld.10.2.5.M.img
          platform: N9K
          release: 10.2.5_nxos64-cs_64bit
          type: PLATFORM
      register: result

    - debug:
        var: result

Debug Output

Expected Behavior

The above playbook should replace the image policy configuration to match the playbook.

Actual Behavior

After running the above playbook, the image policy configuration is not changed.

Image policy before and after running the above playbook (notice it contains package configuration).

            {
                "agnostic": false,
                "epldImgName": "n9000-epld.10.2.5.M.img",
                "fabricPolicyName": null,
                "imagePresent": "Present",
                "nxosVersion": "10.2.5_nxos64-cs_64bit",
                "packageName": "cfg_cmp-0.3.1.0-1.x86_64.rpm",
                "platform": "N9K",
                "policyDescr": "KR5M overridden",
                "policyName": "KR5M",
                "policyType": "PLATFORM",
                "role": null,
                "rpmimages": "mtx-grpctunnel-2.1.0.0-10.4.1.lib32_64_n9000",
                "sequence_number": 2,
                "unInstall": false
            }

The package configuration should be removed by the playbook.

Steps to Reproduce

  1. Create an image policy that includes package install information. The following playbook should work.
---
- name: dcnm_image_policy override
  hosts: ndfc
  tasks:
    - name: Create policy KR5M with package install information
      cisco.dcnm.dcnm_image_policy:
        state: merged
        config:
        - name: KR5M
          agnostic: false
          description: "KR5M"
          epld_image: n9000-epld.10.2.5.M.img
          packages:
            install:
            - cfg_cmp-0.3.1.0-1.x86_64.rpm
            uninstall:
            - mtx-grpctunnel-2.1.0.0-10.4.1.lib32_64_n9000
          platform: N9K
          release: 10.2.5_nxos64-cs_64bit
          type: PLATFORM
  1. override the above with the playbook provided for overridden state.

The package information is not removed.

References

@allenrobel allenrobel added the bug Something isn't working label Nov 21, 2024
@allenrobel allenrobel self-assigned this Nov 21, 2024
@allenrobel
Copy link
Collaborator Author

This issue is due to Merged().commit() -- rather than Replaced().commit() -- being called from within Overridden() in plugins/modules/dcnm_image_policy.py.

Will open a PR with the fix shortly.

@allenrobel allenrobel linked a pull request Nov 21, 2024 that will close this issue
mikewiebe pushed a commit that referenced this issue Dec 3, 2024
* Fix for issue 347

Manually tested this to verify.

Still need to update integration and unit tests.

* dcnm_image_policy: Update integration test

Update integration test for overridden state.

1. playbooks/roles/dcnm_image_policy/dcnm_tests.yaml

- Add vars
    - install_package_1
    - uninstall_package_1

2. test/integration/targets/dcnm_image_policy/tests/dcnm_image_policy_overridden.yaml

- Add packages.install and packages.uninstall configuration
- Verify that merged state adds these packages to the controller config
- Verify that overridden state removes packages.install and packages.uninstall
- Verify that overridden state metadata.action is "replace" instead of "update"
allenrobel added a commit that referenced this issue Dec 3, 2024
* Fix for issue 347

Manually tested this to verify.

Still need to update integration and unit tests.

* dcnm_image_policy: Update integration test

Update integration test for overridden state.

1. playbooks/roles/dcnm_image_policy/dcnm_tests.yaml

- Add vars
    - install_package_1
    - uninstall_package_1

2. test/integration/targets/dcnm_image_policy/tests/dcnm_image_policy_overridden.yaml

- Add packages.install and packages.uninstall configuration
- Verify that merged state adds these packages to the controller config
- Verify that overridden state removes packages.install and packages.uninstall
- Verify that overridden state metadata.action is "replace" instead of "update"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant