fortinet-ansible-dev / ansible-galaxy-fortios-collection

GNU General Public License v3.0
84 stars 48 forks source link

fortios_system_zone cannnot be state absent of member_path #247

Closed miyuk closed 4 months ago

miyuk commented 1 year ago

Overview

fortios_system_zone cannnot be absent state that already deleted params.

Environment

ansible: 2.13.3 fortinet.forios collection: 2.2.3 fortigate: 7.2.4

Reproduction steps

  1. Create zone test with no interfaces.
    config system zone
    edit "test"
    next
    end
  2. Run ansible playbook to delete zone interfaces such as port2.
    ---
    - hosts: fortigate
    gather_facts: false
    collections:
    - fortinet.fortios
    tasks:
    - name: delete zone interface
      fortios_system_zone:
        vdom: root
        state: present
        member_path: interface:interface_name
        member_state: absent
        system_zone:
          name: test
          interface:
            - interface_name: port2
  3. Raise exception
    fatal: [fortigate]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "access_token": null,
            "enable_log": false,
            "member_path": "interface:interface_name",
            "member_state": "absent",
            "state": "present",
            "system_zone": {
                "description": null,
                "interface": [
                    {
                        "interface_name": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER"
                    }
                ],
                "intrazone": null,
                "name": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
                "tagging": null
            },
            "vdom": "root"
        }
    },
    "meta": [
        {
            "build": 1396,
            "child_mkey": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "child_path": "interface",
            "http_method": "DELETE",
            "http_status": 404,
            "mkey": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "name": "zone",
            "object_path": "/system/zone/********/interface/********",
            "path": "system",
            "serial": "**********",
            "status": "error",
            "vdom": "root",
            "version": "v7.2.4"
        }
    ]
    }

    Expected behavior

No error, and zone interface port2 is absent (no changed)

miyuk commented 1 year ago

And this module couldn't use check_mode with member_path. This module is delete interfaces in spite of using --check option.

Maybe, because this modules changes member params at fos.do_member_operation . This function has been in used all fortios modules.

https://github.com/fortinet-ansible-dev/ansible-galaxy-fortios-collection/blob/main/plugins/modules/fortios_system_zone.py#L379

I think we shuld add check_mode options on FortiOSHandler.do_member_operation

MaxxLiu22 commented 1 year ago

Hi @miyuk ,

Thank you for raising this issue, your steps are trying to delete a non-exist interface, so it return 404 not found, but even I have created interfaces through CLI, Ansible still can't achieve delete function, I have reported it to the development team, there are some existing issues around check mode function, I have reported them. Thank you again for your debugging.

Thanks, Maxx

miyuk commented 1 year ago

Hi, @MaxxLiu22

Thank you for your reply. about non-exist interface, Ansible module should commit to be absent status by idempotency

When member_state: absent, I think 404 error is not expected behavior.

Thanks

JieX19 commented 5 months ago

Hi @miyuk

Thank you so much for your time! When deleting a non-existing object and receiving a 404 error in response, this behavior does align with Ansible's idempotence principle. Here's why:

Expected Outcome: If the object doesn't exist, the desired end state is already achieved (i.e., the object is not present). Subsequent attempts to delete the same non-existing object will still result in the same desired state – the object remains absent.

Consistent Behavior: Ansible's idempotence isn't just about successful operations; it's also about consistent behavior across runs. Even though a 404 error is returned when attempting to delete a non-existing object, this consistent behavior ensures that subsequent runs of the playbook will not introduce any changes.

No Side Effects: Since deleting a non-existing object and receiving a 404 error does not alter the system's state, there are no unintended side effects from running the task multiple times. This aligns with Ansible's goal of ensuring predictable and safe automation.

In summary, while receiving a 404 error when deleting a non-existing object might seem like an anomaly, it does align with Ansible's idempotence principle because it maintains the desired state and ensures consistent behavior across multiple playbook runs.

miyuk commented 4 months ago

@JieX19 I understood that this behavior is correct action. Close this issue.