fortinet / ansible-fortimanager-generic

8 stars 6 forks source link

Idempotency #10

Closed mooky31 closed 4 years ago

mooky31 commented 4 years ago

Hello,

I'm trying to declare a new device using this module. Here is the playbook:

- name: Test API
  hosts: FortiManager
  gather_facts: no
  connection: httpapi

  collections:
    - fortinet.fortimanager

  vars:
    ansible_httpapi_use_ssl: True
    ansible_httpapi_validate_certs: False
    ansible_httpapi_port: 443

  tasks:
    - name: Provisioning
      fmgr_generic:
         method: exec
         params:
            -  url: /dvm/cmd/add/device
               data:
                  adom: root
                  device:
                     desc: Provisioned by ansible
                     device action: add_model
                     mgmt_mode: fmg
                     mr: 2
                     name: fortiVM01
                     os_type: fos
                     os_ver: '6.0'
                     sn: FGVMEVHBR3TXNV04
                  flags:
                    - none
      register: provision
      failed_when: provision.rc != -20010

The task fails if the device already exists, whereas it should be OK.

But as this is a generic module I can understand this behavior. I tried to correct it myself, but the failed_when doesn't do anything, I don't know why

chillancezen commented 4 years ago

hi @mooky31,

it seems that you are using ansible 2.9.x, the syntax is not recognized correctly.

according to https://docs.ansible.com/ansible/latest/user_guide/playbooks_error_handling.html,

the syntax is a little different in old version of Ansible: we should add another task to detect the errors if we are going to override the way to check errors

- name: Test API
  hosts: fortimanager01
  gather_facts: no
  connection: httpapi

  collections:
    - fortinet.fortimanager

  vars:
    ansible_httpapi_use_ssl: True
    ansible_httpapi_validate_certs: False
    ansible_httpapi_port: 443

  tasks:
    - name: Provisioning
      fmgr_generic:
         method: exec
         params:
            -  url: /dvm/cmd/add/device
               data:
                  adom: root
                  device:
                     desc: Provisioned by ansible
                     device action: add_model
                     mgmt_mode: fmg
                     mr: 2
                     name: fortiVM01
                     os_type: fos
                     os_ver: '6.0'
                     sn: FGVMEVHBR3TXNV04
                  flags:
                    - none
      register: provision
      ignore_errors: yes

    - name: fail the play
      fail:
        msg: "the ... fail"
      failed_when: provision.rc != 0 and provision.rc != -20010

remember to ignore all errors for the fist task

thanks, Link

mooky31 commented 4 years ago

Hi,

The doc you sent me still references this method : https://docs.ansible.com/ansible/latest/user_guide/playbooks_error_handling.html#controlling-what-defines-failure

I can't test yours because my fortimanager test licence has expired, i'm trying to get a new one.

But let's not forget the root problem: the tasks fails when the device already exists, whereas it should be an OK. Error handling is just a workaround.

chillancezen commented 4 years ago

Hi,

The doc you sent me still references this method : https://docs.ansible.com/ansible/latest/user_guide/playbooks_error_handling.html#controlling-what-defines-failure

I can't test yours because my fortimanager test licence has expired, i'm trying to get a new one.

But let's not forget the root problem: the tasks fails when the device already exists, whereas it should be an OK. Error handling is just a workaround.

hi @mooky31 ,

I can understand your expectation, there are thousands of export APIs, /dvm/cmd/add/device is only one of them. the generic module is designed to fit them all, only handling the result of /dvm/cmd/add/device somehow violates the nature of being generic.

Besides, there are lots of APIs is designed to be long-term task which may last for minutes. A single API request is not enough to satisfy users' demand, for example, to add a real fortigate device is rather complex:

we have an example in this repo: https://github.com/fortinet/ansible-fortimanager-generic/blob/master/example/example_add_device.yml

thanks, Link

mooky31 commented 4 years ago

You're right, those generic modules can't handle things like that.

But the problem is I can't handle it myself since my failed_when doesn't work. It would be cleaner than ignore_errors then fail. I think this might be due to how your module is coded, because I get weird results : fatal: [1.0.0.253]: FAILED! => {"ansible_facts": {"ansible_params": {"method": "exec", "params": [{"data": {"adom": "root", "device": {"desc": "Provisioned by ansible", "device action": "add_model", "mgmt_mode": "fmg", "mr": 2, "name": "fortiVM01", "os_type": "fos", "os_ver": "6.0", "sn": "FGVMEVHBR3TXNV04"}, "flags": ["none"]}, "url": "/dvm/cmd/add/device"}]}, "connected_fmgr": null, "discovered_interpreter_python": "/usr/bin/python", "paramgram": {"method": "exec", "params": [{"data": {"adom": "root", "device": {"desc": "Provisioned by ansible", "device action": "add_model", "mgmt_mode": "fmg", "mr": 2, "name": "fortiVM01", "os_type": "fos", "os_ver": "6.0", "sn": "FGVMEVHBR3TXNV04"}, "flags": ["none"]}, "url": "/dvm/cmd/add/device"}]}, "response": [-20010, {"status": {"code": -20010, "message": "Serial number already in use"}, "url": "/dvm/cmd/add/device"}]}, "ansible_module_results": {"status": {"code": -20010, "message": "Serial number already in use"}, "url": "/dvm/cmd/add/device"}, "changed": false, "msg": "Operation Finished", "rc": -20010, "success": true, "unreachable": false}

Why is it fatal while "success": true ? This might explain why even failed_when: False doesn't work.

chillancezen commented 4 years ago

hi @mooky31 ,

this is absolutely not the root cause. I had a test: it returned all OK, everything is OK. but I want it fail by setting failed_when, it just didn't take effect no matter what the check content is.

I see you filed an issue in ansible upstream repo, but nobody updates. I also sought help from Ansible slack hoping Ansible redhat can give us an update.

thanks, Link

mooky31 commented 4 years ago

I found that the problem comes from the module_util fortimanager.py, function return_response :

                    module.exit_json(msg=msg, success=success, changed=changed, unreachable=unreachable,
                                     skipped=skipped, ansible_module_results=results[1], ansible_facts=ansible_facts, rc=results[0],
                                     invocation={"module_args": ansible_facts["ansible_params"]})

If I remove skipped=skipped, I can catch the failure using failed_when.

I tried to force it to True or False, but it doesn't change a thing : it prevents me from catching the error.