Merge pull request #338 from netscaler/issue-322

.github/workflows/test.yml

@@ -172,7 +172,7 @@ jobs:
           echo "> ansible-test --version"
           ansible-test --version
           echo "> ansible-test integration -v --color yes"
-          ansible-test integration -v --color yes
+          ansible-test integration netscaler/cpx/ -v --color yes
       - name: Stop containers
         if: always()
         run: docker-compose -f "docker-compose.yml" down

Makefile

@@ -2,14 +2,17 @@ fmt:
 	autoflake plugins/modules/*.py
 	autoflake plugins/module_utils/*.py
 	autoflake tools/module_generator.py
+	autoflake --recursive tests/
 
 	black plugins/modules/*.py
 	black plugins/module_utils/*.py
 	black tools/module_generator.py
+	black tests/
 
 	isort plugins/modules/*.py
 	isort plugins/module_utils/*.py
 	isort tools/module_generator.py
+	isort tests/
 
 	yamlfmt .
 

meta/runtime.yml

@@ -2,7 +2,6 @@
 # Collections must specify a minimum required ansible version to upload
 # to galaxy
 requires_ansible: '>=2.14.0'
-
 # Content that Ansible needs to load from another location or that has
 # been deprecated/removed
 # plugin_routing:

plugins/module_utils/common.py

@@ -100,6 +100,10 @@ def _check_create_resource_params(resource_name, resource_module_params, action=
         resource_action_keys = []
     resource_primary_key = NITRO_RESOURCE_MAP[resource_name]["primary_key"]
 
+    resource_primary_key = change_primary_key(
+        resource_name, resource_module_params, resource_primary_key
+    )
+
     if (
         resource_primary_key
         and resource_primary_key not in resource_module_params.keys()
@@ -190,6 +194,19 @@ def create_resource(client, resource_name, resource_module_params, action=None):
     )
 
 
+@trace
+def change_primary_key(resource_name, resource_module_params, resource_primary_key):
+    # FIXME: This is a temporary fix for ntpserver resource
+    # `ntpserver` has two possible primary keys: `serverip` and `servername`
+    # But, the schema has only `serverip` as the primary key
+    # So, we are checking for `serverip` as the primary key and if it is not present, we are checking for `servername`
+    # This is a temporary fix until the schema is updated
+    if resource_name == "ntpserver":
+        if "serverip" not in resource_module_params.keys():
+            resource_primary_key = "servername"
+    return resource_primary_key
+
+
 @trace
 def _check_update_resource_params(resource_name, resource_module_params):
     # check if resource_module_params contains any key other than allowed keys for the resource
@@ -205,6 +222,10 @@ def _check_update_resource_params(resource_name, resource_module_params):
 
     resource_primary_key = NITRO_RESOURCE_MAP[resource_name]["primary_key"]
 
+    resource_primary_key = change_primary_key(
+        resource_name, resource_module_params, resource_primary_key
+    )
+
     if resource_primary_key and (
         resource_primary_key not in resource_module_params.keys()
     ):
@@ -268,6 +289,10 @@ def _check_delete_resource_params(resource_name, resource_module_params):
 
     resource_primary_key = NITRO_RESOURCE_MAP[resource_name]["primary_key"]
 
+    resource_primary_key = change_primary_key(
+        resource_name, resource_module_params, resource_primary_key
+    )
+
     if (
         resource_primary_key
         and resource_primary_key not in resource_module_params.keys()
@@ -287,6 +312,16 @@ def delete_resource(client, resource_name, resource_module_params):
     if not ok:
         return False, err
 
+    resource_primary_key = NITRO_RESOURCE_MAP[resource_name]["primary_key"]
+    if resource_primary_key:
+        resource_primary_key = change_primary_key(
+            resource_name, resource_module_params, resource_primary_key
+        )
+
+        resource_id = resource_module_params[resource_primary_key]
+    else:
+        resource_id = None
+
     args = {}
     for arg_key in NITRO_RESOURCE_MAP[resource_name]["delete_arg_keys"]:
         log("DEBUG: arg_key: {}".format(arg_key))
@@ -298,6 +333,16 @@ def delete_resource(client, resource_name, resource_module_params):
             # status_code: 400;
             # Reason:{'errorcode': 1092, 'message': 'Arguments cannot both be specified [serviceGroupName, serviceName]', 'severity': 'ERROR'}
             continue
+        elif (
+            resource_name == "ntpserver"
+            and resource_primary_key == "servername"
+            and arg_key == "servername"
+        ):
+            # for `ntpserver`, there are two possible primary keys: `serverip` and `servername`
+            # But, the schema has only `serverip` as the primary key
+            # if `servername` is the primary_key, it will be resource_id and not arg_key
+            # so, we are skipping arg_key `servername` for `ntpserver`
+            continue
         try:
             args[arg_key] = resource_module_params[arg_key]
         except KeyError:
@@ -309,13 +354,6 @@ def delete_resource(client, resource_name, resource_module_params):
             )
             continue
 
-    if NITRO_RESOURCE_MAP[resource_name]["primary_key"]:
-        resource_id = resource_module_params[
-            NITRO_RESOURCE_MAP[resource_name]["primary_key"]
-        ]
-    else:
-        resource_id = None
-
     if resource_name.endswith("_binding"):
         if not is_resource_exists(client, resource_name, resource_id, filter=args):
             return True, None

plugins/module_utils/module_executor.py

@@ -355,6 +355,37 @@ def create_or_update(self):
             )
             if not ok:
                 self.return_failure(err)
+
+            # There can be module_params in the playbook which are not part of `add_payload_keys`, but part of `update_payload_keys` in the NITRO_RESOURCE_MAP
+            # For example, `ntpserver` resource has `preferredntpserver` attribute which is not part of `add_payload_keys`, but part of `update_payload_keys`.
+            # If `preferredntpserver` is also part of the playbook-task, to make it true desired state, we will update the resource with the module_params
+            add_payload_keys = NITRO_RESOURCE_MAP[self.resource_name][
+                "add_payload_keys"
+            ]
+            update_payload_keys = NITRO_RESOURCE_MAP[self.resource_name][
+                "update_payload_keys"
+            ]
+
+            keys_in_upload_payload_and_not_in_add_payload = set(
+                update_payload_keys
+            ) - set(add_payload_keys)
+
+            is_module_params_contain_update_params = bool(
+                set(keys_in_upload_payload_and_not_in_add_payload).intersection(
+                    set(self.resource_module_params.keys())
+                )
+            )
+
+            if is_module_params_contain_update_params:
+                log(
+                    "INFO: module_params has keys %s which are not part of `add_payload_keys`. Hence updating the resource again"
+                    % keys_in_upload_payload_and_not_in_add_payload
+                )
+                ok, err = update_resource(
+                    self.client, self.resource_name, self.resource_module_params
+                )
+                if not ok:
+                    self.return_failure(err)
         else:
             # Update only if resource is not identical (idempotent)
             if self.is_resource_identical():

tests/integration/targets/ipset/aliases

@@ -1 +1,3 @@
-gather_facts/no
+gather_facts/no
+netscaler/cpx/
+netscaler/vpx/

tests/integration/targets/lbmonitor/aliases

@@ -1 +1,3 @@
-gather_facts/no
+gather_facts/no
+netscaler/cpx/
+netscaler/vpx/

tests/integration/targets/nsip/aliases

@@ -1 +1,3 @@
-gather_facts/no
+gather_facts/no
+netscaler/cpx/
+netscaler/vpx/

tests/integration/targets/ntpserver/aliases

@@ -0,0 +1,2 @@
+gather_facts/no
+netscaler/vpx/

tests/integration/targets/ntpserver/tasks/main.yaml

@@ -0,0 +1,125 @@
+---
+- name: NTPSERVER | ADD | --check
+  delegate_to: localhost
+  register: result
+  check_mode: true
+  tags: test
+  netscaler.adc.ntpserver:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: present
+    servername: pool.ntp.org
+    preferredntpserver: "YES"
+- name: Assert | NTPSERVER | ADD | --check
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: NTPSERVER | ADD
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.ntpserver:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: present
+    servername: pool.ntp.org
+    preferredntpserver: "YES"
+- name: Assert | NTPSERVER | ADD
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: NTPSERVER | ADD | idempotent
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.ntpserver:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: present
+    servername: pool.ntp.org
+    preferredntpserver: "YES"
+- name: Assert | NTPSERVER | ADD | idempotent
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==false"
+- name: NTPSERVER | DELETE | --check
+  delegate_to: localhost
+  register: result
+  check_mode: true
+  tags: test
+  netscaler.adc.ntpserver:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: absent
+    servername: pool.ntp.org
+    preferredntpserver: "YES"
+- name: Assert | NTPSERVER | DELETE | --check
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: NTPSERVER | DELETE
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.ntpserver:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: absent
+    servername: pool.ntp.org
+    preferredntpserver: "YES"
+- name: Assert | NTPSERVER | DELETE
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: NTPSERVER | DELETE | idempotent
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.ntpserver:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: absent
+    servername: pool.ntp.org
+    preferredntpserver: "YES"
+- name: Assert | NTPSERVER | DELETE | idempotent
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==false"

tests/integration/targets/sslprofile_sslcipher_binding/aliases

@@ -1 +1,3 @@
-gather_facts/no
+gather_facts/no
+netscaler/cpx/
+netscaler/vpx/