netscaler / ansible-collection-netscaleradc

Custom Ansible modules for NetScaler ADC and NetScaler ADM. Part of NetScaler Automation Toolkit | https://github.com/netscaler/automation-toolkit
https://netscaler.github.io/ansible-collection-netscaleradc/
MIT License
114 stars 56 forks source link

netscaler_lb_vserver: nitro exception errorcode=1092 #4

Closed cwegener closed 7 years ago

cwegener commented 7 years ago

Netscaler version:

NetScaler NS11.1: Build 50.10.nc, Date: Nov  6 2016, 05:42:29

Ansible task from playbook:

    - name: LB vServer
      delegate_to: localhost
      connection: local
      netscaler_lb_vserver:
        nsip: "{{ ctx_ns_nsip }}"
        nitro_user: "{{ ctx_ns_nitro_user }}"
        nitro_pass: "{{ ctx_ns_nitro_pass }}"
        comment: Managed by Ansible
        ipv46: 10.30.3.176
        lbmethod: LRTM
        name: "lb_vs_http_ctx_sf_80"
        persistencetype: SOURCEIP
        port: 80
        servicetype: HTTP
        servicebindings:
          - servicename: svc_http_LAB3TST3_80

Tail of ansible output:

    "loglines": [
        "Applying actions for state present",
        "Checking if lb vserver exists",
        "Add lb vserver"
    ],
    "msg": "nitro exception errorcode=1092, message=Arguments cannot both be specified [RecursionAvailable, serviceType==HTTP]"

I'll try a firmware update to the latest 11.1 to see if it fixes the error.

cwegener commented 7 years ago

Upgraded to latest 11.1 build and the error remains exactly the same.

NetScaler NS11.1: Build 54.14.nc, Date: Jun  7 2017, 20:10:48
cwegener commented 7 years ago

This is the POST body being sent to /nitro/v1/config/lbvserver:

{
  "lbvserver": {
    "ipv46": "10.30.3.176",
    "comment": "Managed by Ansible",
    "persistencetype": "SOURCEIP",
    "cacheable": "NO",
    "name": "lb_vs_http_ctx_sf_80",
    "rtspnat": "OFF",
    "servicetype": "HTTP",
    "bypassaaaa": "NO",
    "authn401": "OFF",
    "lbmethod": "LRTM",
    "authentication": "OFF",
    "l2conn": "OFF",
    "pushmulticlients": "NO",
    "recursionavailable": "NO",
    "port": 80
  }
}

There clearly are too many attributes in the JSON object that I did not specify in the playbook task.

Are these attribute values added by the SDK?

cwegener commented 7 years ago

Ok. I can see that the additional attribute values somehow must get added during instantiation of the ConfigProxy

This seems like a netscaler-ansible-modules bug.

The relavant code is: https://github.com/citrix/netscaler-ansible-modules/blob/e888231e7631b3443152fb7d01eee2b5323bb235/ansible-modules/netscaler.py#L59

If an attribute is not specified in the playbook, it must not be added to the NITRO API call.

giorgos-nikolopoulos commented 7 years ago

This is a valid issue.

As it is the option value gets passed when it is not defined.

The options in the transforms dictionary are also defined as module arguments with type='bool' .

This guarantees that whatever value they have in the playbook will be forced to a value of True or False.

Unfortunately I did not take into account the fact that Ansible gives the value None to any module argument that is not defined in the playbook and does not have some other default value setup.

So in this case the expression evaluated to if None is True 'YES' else 'NO' which resulted in passing a 'NO' value to the nitro object.

I did make a fix that takes None values into account explicitly and does not copy the attribute to the nitro object, which fixes this issue without handling the False value explicitely.

I think though that your changes add clarity to this point of the code and make it explicit that the only accepted values are [True, False, None] so I will be merging it.

giorgos-nikolopoulos commented 7 years ago

71122ef4a0c1be736db5df2448c25855881a1cf3 Addresses the None value explicitely.

cwegener commented 7 years ago

Thanks for your feedback. I hadn't investigated this whole None/null topic in Ansible.

I looked up the definition of the null type in YAML and from looking at the definition it kinda makes sense that ansible defines all the dictionary keys with their appropriate undefined value. I think it's a bit of a weird behavior by ansible and the ansible devs should probably have some sort of a Gotchas document for module developers where things like that are mentioned.

http://yaml.org/type/null.html

Devoid of value.

A null value is used to indicate the lack of a value. This is typically converted into any native null-like value (e.g., undef in Perl, None in Python). Note that a null is different from an empty string and that a mapping entry with some key and a null value is valid and different from not having that key in the mapping.