Netscaler various fixes (#34800)

* Add default lb vserver option in netscaler_cs_vserver
Add documentation for ssl_certkey option in netscaler_cs_vserver

* Add options for gracefully disabling netscaler_server

* Add "state" suboption for netscaler_servicegroup servicemembers
Fix servicemember modification algorithm in netscaler_servicegroup
Fix monitorbindings modification algorithm in netscaler_servicegroup

6 files changed, 248 insertions, 36 deletions

diff --git a/lib/ansible/modules/network/netscaler/netscaler_cs_vserver.py b/lib/ansible/modules/network/netscaler/netscaler_cs_vserver.py
index a105ce87a9..611be345f0 100644
--- a/lib/ansible/modules/network/netscaler/netscaler_cs_vserver.py
+++ b/lib/ansible/modules/network/netscaler/netscaler_cs_vserver.py
@@ -500,6 +500,19 @@ options:
             - "."
             - "Minimum value = C(1)"
 
+    lbvserver:
+        description:
+            - The default Load Balancing virtual server.
+        version_added: "2.5"
+
+    ssl_certkey:
+        description:
+            - The name of the ssl certificate that is bound to this service.
+            - The ssl certificate must already exist.
+            - Creating the certificate can be done with the M(netscaler_ssl_certkey) module.
+            - This option is only applicable only when C(servicetype) is C(SSL).
+        version_added: "2.5"
+
     disabled:
         description:
             - When set to C(yes) the cs vserver will be disabled.
@@ -570,6 +583,7 @@ from ansible.module_utils.network.netscaler.netscaler import (
 )
 try:
     from nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver import csvserver
+    from nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver_lbvserver_binding import csvserver_lbvserver_binding
     from nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver_cspolicy_binding import csvserver_cspolicy_binding
     from nssrc.com.citrix.netscaler.nitro.resource.config.ssl.sslvserver_sslcertkey_binding import sslvserver_sslcertkey_binding
     from nssrc.com.citrix.netscaler.nitro.exception.nitro_exception import nitro_exception
@@ -624,6 +638,75 @@ def get_configured_policybindings(client, module):
     return bindings
 
 
+def get_default_lb_vserver(client, module):
+    try:
+        default_lb_vserver = csvserver_lbvserver_binding.get(client, module.params['name'])
+        return default_lb_vserver[0]
+    except nitro_exception as e:
+        if e.errorcode == 258:
+            return csvserver_lbvserver_binding()
+        else:
+            raise
+
+
+def default_lb_vserver_identical(client, module):
+    d = get_default_lb_vserver(client, module)
+    configured = ConfigProxy(
+        actual=csvserver_lbvserver_binding(),
+        client=client,
+        readwrite_attrs=[
+            'name',
+            'lbvserver',
+        ],
+        attribute_values_dict={
+            'name': module.params['name'],
+            'lbvserver': module.params['lbvserver'],
+        }
+    )
+    log('default lb vserver %s' % ((d.name, d.lbvserver),))
+    if d.name is None and module.params['lbvserver'] is None:
+        log('Default lb vserver identical missing')
+        return True
+    elif d.name is not None and module.params['lbvserver'] is None:
+        log('Default lb vserver needs removing')
+        return False
+    elif configured.has_equal_attributes(d):
+        log('Default lb vserver identical')
+        return True
+    else:
+        log('Default lb vserver not identical')
+        return False
+
+
+def sync_default_lb_vserver(client, module):
+    d = get_default_lb_vserver(client, module)
+
+    if module.params['lbvserver'] is not None:
+        configured = ConfigProxy(
+            actual=csvserver_lbvserver_binding(),
+            client=client,
+            readwrite_attrs=[
+                'name',
+                'lbvserver',
+            ],
+            attribute_values_dict={
+                'name': module.params['name'],
+                'lbvserver': module.params['lbvserver'],
+            }
+        )
+
+        if not configured.has_equal_attributes(d):
+            if d.name is not None:
+                log('Deleting default lb vserver %s' % d.lbvserver)
+                csvserver_lbvserver_binding.delete(client, d)
+            log('Adding default lb vserver %s' % configured.lbvserver)
+            configured.add()
+    else:
+        if d.name is not None:
+            log('Deleting default lb vserver %s' % d.lbvserver)
+            csvserver_lbvserver_binding.delete(client, d)
+
+
 def get_actual_policybindings(client, module):
     log('Getting actual policy bindigs')
     bindings = {}
@@ -949,6 +1032,7 @@ def main():
             type='bool',
             default=False
         ),
+        lbvserver=dict(type='str'),
     )
 
     argument_spec = dict()
@@ -1168,6 +1252,12 @@ def main():
 
                     module_result['changed'] = True
 
+            # Check default lb vserver
+            if not default_lb_vserver_identical(client, module):
+                if not module.check_mode:
+                    sync_default_lb_vserver(client, module)
+                module_result['changed'] = True
+
             if not module.check_mode:
                 res = do_state_change(client, module, csvserver_proxy)
                 if res.errorcode != 0:
diff --git a/lib/ansible/modules/network/netscaler/netscaler_server.py b/lib/ansible/modules/network/netscaler/netscaler_server.py
index 7e07c1661a..5b5a7bf1f7 100644
--- a/lib/ansible/modules/network/netscaler/netscaler_server.py
+++ b/lib/ansible/modules/network/netscaler/netscaler_server.py
@@ -87,6 +87,21 @@ options:
             - "Minimum value = C(0)"
             - "Maximum value = C(4094)"
 
+    graceful:
+        description:
+            - >-
+                Shut down gracefully, without accepting any new connections, and disabling each service when all of
+                its connections are closed.
+            - This option is meaningful only when setting the I(disabled) option to C(true)
+        type: bool
+        version_added: "2.5"
+
+    delay:
+        description:
+            - Time, in seconds, after which all the services configured on the server are disabled.
+            - This option is meaningful only when setting the I(disabled) option to C(true)
+        version_added: "2.5"
+
     disabled:
         description:
             - When set to C(true) the server state will be set to C(disabled).
@@ -160,8 +175,15 @@ def server_identical(client, module, server_proxy):
     log('Checking if configured server is identical')
     if server.count_filtered(client, 'name:%s' % module.params['name']) == 0:
         return False
-    server_list = server.get_filtered(client, 'name:%s' % module.params['name'])
-    if server_proxy.has_equal_attributes(server_list[0]):
+    diff = diff_list(client, module, server_proxy)
+
+    # Remove options that are not present in nitro server object
+    # These are special options relevant to the disabled action
+    for option in ['graceful', 'delay']:
+        if option in diff:
+            del diff[option]
+
+    if diff == {}:
         return True
     else:
         return False
@@ -197,6 +219,8 @@ def main():
         ),
         comment=dict(type='str'),
         td=dict(type='float'),
+        graceful=dict(type='bool'),
+        delay=dict(type='float')
     )
 
     hand_inserted_arguments = dict(
@@ -251,6 +275,8 @@ def main():
         'translationmask',
         'domainresolveretry',
         'ipv6address',
+        'graceful',
+        'delay',
         'comment',
         'td',
     ]
@@ -289,6 +315,7 @@ def main():
     ]
 
     transforms = {
+        'graceful': ['bool_yes_no'],
         'ipv6address': ['bool_yes_no'],
     }
 
diff --git a/lib/ansible/modules/network/netscaler/netscaler_servicegroup.py b/lib/ansible/modules/network/netscaler/netscaler_servicegroup.py
index f913d3bc21..0965937faa 100644
--- a/lib/ansible/modules/network/netscaler/netscaler_servicegroup.py
+++ b/lib/ansible/modules/network/netscaler/netscaler_servicegroup.py
@@ -271,6 +271,12 @@ options:
                     - Server port number.
                     - Range C(1) - C(65535)
                     - "* in CLI is represented as 65535 in NITRO API"
+            state:
+                choices:
+                    - 'enabled'
+                    - 'disabled'
+                description:
+                    - Initial state of the service after binding.
             hashid:
                 description:
                     - The hash identifier for the service.
@@ -427,6 +433,7 @@ def get_configured_service_members(client, module):
         'servicegroupname',
         'ip',
         'port',
+        'state',
         'hashid',
         'serverid',
         'servername',
@@ -460,8 +467,7 @@ def get_configured_service_members(client, module):
     return members
 
 
-def servicemembers_identical(client, module):
-    log('servicemembers_identical')
+def get_actual_service_members(client, module):
     try:
         # count() raises nitro exception instead of returning 0
         count = servicegroup_servicegroupmember_binding.count(client, module.params['servicegroupname'])
@@ -474,7 +480,13 @@ def servicemembers_identical(client, module):
             servicegroup_members = []
         else:
             raise
+    return servicegroup_members
+
+
+def servicemembers_identical(client, module):
+    log('servicemembers_identical')
 
+    servicegroup_members = get_actual_service_members(client, module)
     log('servicemembers %s' % servicegroup_members)
     module_servicegroups = get_configured_service_members(client, module)
     log('Number of service group members %s' % len(servicegroup_members))
@@ -497,33 +509,55 @@ def servicemembers_identical(client, module):
 
 def sync_service_members(client, module):
     log('sync_service_members')
-    delete_all_servicegroup_members(client, module)
-
-    for member in get_configured_service_members(client, module):
-        member.add()
-
-
-def delete_all_servicegroup_members(client, module):
-    log('delete_all_servicegroup_members')
-    if servicegroup_servicegroupmember_binding.count(client, module.params['servicegroupname']) == 0:
-        return
-    servicegroup_members = servicegroup_servicegroupmember_binding.get(client, module.params['servicegroupname'])
-    log('len %s' % len(servicegroup_members))
-    log('count %s' % servicegroup_servicegroupmember_binding.count(client, module.params['servicegroupname']))
-    for member in servicegroup_members:
-        log('%s' % dir(member))
-        log('ip %s' % member.ip)
-        log('servername %s' % member.servername)
+    configured_service_members = get_configured_service_members(client, module)
+    actual_service_members = get_actual_service_members(client, module)
+    skip_add = []
+    skip_delete = []
+
+    # Find positions of identical service members
+    for (configured_index, configured_service) in enumerate(configured_service_members):
+        for (actual_index, actual_service) in enumerate(actual_service_members):
+            if configured_service.has_equal_attributes(actual_service):
+                skip_add.append(configured_index)
+                skip_delete.append(actual_index)
+
+    # Delete actual that are not identical to any configured
+    for (actual_index, actual_service) in enumerate(actual_service_members):
+        # Skip identical
+        if actual_index in skip_delete:
+            log('Skipping actual delete at index %s' % actual_index)
+            continue
+
+        # Fallthrouth to deletion
         if all([
-            hasattr(member, 'ip'),
-            member.ip is not None,
-            hasattr(member, 'servername'),
-            member.servername is not None,
+            hasattr(actual_service, 'ip'),
+            actual_service.ip is not None,
+            hasattr(actual_service, 'servername'),
+            actual_service.servername is not None,
         ]):
-            member.ip = None
+            actual_service.ip = None
+
+        actual_service.servicegroupname = module.params['servicegroupname']
+        servicegroup_servicegroupmember_binding.delete(client, actual_service)
+
+    # Add configured that are not already present in actual
+    for (configured_index, configured_service) in enumerate(configured_service_members):
+
+        # Skip identical
+        if configured_index in skip_add:
+            log('Skipping configured add at index %s' % configured_index)
+            continue
 
-        member.servicegroupname = module.params['servicegroupname']
-        servicegroup_servicegroupmember_binding.delete(client, member)
+        # Fallthrough to addition
+        configured_service.add()
+
+
+def monitor_binding_equal(configured, actual):
+    if any([configured.monitorname != actual.monitor_name,
+            configured.servicegroupname != actual.servicegroupname,
+            configured.weight != float(actual.weight)]):
+        return False
+    return True
 
 
 def get_configured_monitor_bindings(client, module):
@@ -593,11 +627,13 @@ def monitor_bindings_identical(client, module):
     # Compare key to key
     for key in configured_key_set:
         configured_proxy = configured_bindings[key]
+
+        # Follow nscli convention for missing weight value
+        if not hasattr(configured_proxy, 'weight'):
+            configured_proxy.weight = 1
         log('configured_proxy %s' % [configured_proxy.monitorname, configured_proxy.servicegroupname, configured_proxy.weight])
         log('actual_bindings %s' % [actual_bindings[key].monitor_name, actual_bindings[key].servicegroupname, actual_bindings[key].weight])
-        if any([configured_proxy.monitorname != actual_bindings[key].monitor_name,
-                configured_proxy.servicegroupname != actual_bindings[key].servicegroupname,
-                configured_proxy.weight != float(actual_bindings[key].weight)]):
+        if not monitor_binding_equal(configured_proxy, actual_bindings[key]):
             return False
 
     # Fallthrought to success
@@ -606,8 +642,23 @@ def monitor_bindings_identical(client, module):
 
 def sync_monitor_bindings(client, module):
     log('Entering sync_monitor_bindings')
-    # Delete existing bindings
-    for binding in get_actual_monitor_bindings(client, module).values():
+
+    actual_bindings = get_actual_monitor_bindings(client, module)
+
+    # Exclude default monitors from deletion
+    for monitorname in ('tcp-default', 'ping-default'):
+        if monitorname in actual_bindings:
+            del actual_bindings[monitorname]
+
+    configured_bindings = get_configured_monitor_bindings(client, module)
+
+    to_remove = list(set(actual_bindings.keys()) - set(configured_bindings.keys()))
+    to_add = list(set(configured_bindings.keys()) - set(actual_bindings.keys()))
+    to_modify = list(set(configured_bindings.keys()) & set(actual_bindings.keys()))
+
+    # Delete existing and modifiable bindings
+    for key in to_remove + to_modify:
+        binding = actual_bindings[key]
         b = lbmonitor_servicegroup_binding()
         b.monitorname = binding.monitor_name
         b.servicegroupname = module.params['servicegroupname']
@@ -616,9 +667,9 @@ def sync_monitor_bindings(client, module):
             continue
         lbmonitor_servicegroup_binding.delete(client, b)
 
-    # Apply configured bindings
-
-    for binding in get_configured_monitor_bindings(client, module).values():
+    # Add new and modified bindings
+    for key in to_add + to_modify:
+        binding = configured_bindings[key]
         log('Adding %s' % binding.monitorname)
         binding.add()
 
diff --git a/test/units/modules/network/netscaler/test_netscaler_cs_vserver.py b/test/units/modules/network/netscaler/test_netscaler_cs_vserver.py
index 6c7927850c..d586cc45a2 100644
--- a/test/units/modules/network/netscaler/test_netscaler_cs_vserver.py
+++ b/test/units/modules/network/netscaler/test_netscaler_cs_vserver.py
@@ -45,6 +45,8 @@ class TestNetscalerCSVserverModule(TestModule):
             'nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver.csvserver': m,
             'nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver_cspolicy_binding': m,
             'nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver_cspolicy_binding.csvserver_cspolicy_binding': m,
+            'nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver_lbvserver_binding': m,
+            'nssrc.com.citrix.netscaler.nitro.resource.config.cs.csvserver_lbvserver_binding.csvserver_lbvserver_binding': m,
             'nssrc.com.citrix.netscaler.nitro.resource.config.ssl': m,
             'nssrc.com.citrix.netscaler.nitro.resource.config.ssl.sslvserver_sslcertkey_binding': m,
             'nssrc.com.citrix.netscaler.nitro.resource.config.ssl.sslvserver_sslcertkey_binding.sslvserver_sslcertkey_binding': m,
diff --git a/test/units/modules/network/netscaler/test_netscaler_server.py b/test/units/modules/network/netscaler/test_netscaler_server.py
index 32aafb4072..237a56b2ff 100644
--- a/test/units/modules/network/netscaler/test_netscaler_server.py
+++ b/test/units/modules/network/netscaler/test_netscaler_server.py
@@ -177,6 +177,7 @@ class TestNetscalerServerModule(TestModule):
             get_nitro_client=m,
             server_exists=Mock(side_effect=[False, True]),
             ConfigProxy=Mock(return_value=server_proxy_mock),
+            diff_list=Mock(return_value={}),
             do_state_change=Mock(return_value=Mock(errorcode=0))
         ):
             self.module = netscaler_server
@@ -203,6 +204,7 @@ class TestNetscalerServerModule(TestModule):
             get_nitro_client=m,
             server_exists=Mock(side_effect=[True, False]),
             ConfigProxy=Mock(return_value=server_proxy_mock),
+            diff_list=Mock(return_value={}),
             do_state_change=Mock(return_value=Mock(errorcode=0))
         ):
             self.module = netscaler_server
@@ -230,6 +232,7 @@ class TestNetscalerServerModule(TestModule):
             get_nitro_client=m,
             server_exists=Mock(side_effect=[False, True]),
             ConfigProxy=Mock(return_value=server_proxy_mock),
+            diff_list=Mock(return_value={}),
             do_state_change=Mock(return_value=Mock(errorcode=0))
         ):
             self.module = netscaler_server
@@ -284,12 +287,48 @@ class TestNetscalerServerModule(TestModule):
             get_nitro_client=m,
             server_exists=Mock(side_effect=[True, False]),
             ConfigProxy=Mock(return_value=server_proxy_mock),
+            diff_list=Mock(return_value={}),
             do_state_change=Mock(return_value=Mock(errorcode=1, message='Failed on purpose'))
         ):
             self.module = netscaler_server
             result = self.failed()
             self.assertEqual(result['msg'], 'Error when setting disabled state. errorcode: 1 message: Failed on purpose')
 
+    def test_disable_server_graceful(self):
+        set_module_args(dict(
+            nitro_user='user',
+            nitro_pass='pass',
+            nsip='1.1.1.1',
+            state='present',
+            disabled=True,
+            graceful=True
+        ))
+        from ansible.modules.network.netscaler import netscaler_server
+
+        client_mock = Mock()
+
+        m = Mock(return_value=client_mock)
+
+        server_proxy_mock = Mock()
+
+        d = {
+            'graceful': True,
+            'delay': 20,
+        }
+        with patch.multiple(
+            'ansible.modules.network.netscaler.netscaler_server',
+            nitro_exception=self.MockException,
+            get_nitro_client=m,
+            diff_list=Mock(return_value=d),
+            get_immutables_intersection=Mock(return_value=[]),
+            server_exists=Mock(side_effect=[True, True]),
+            ConfigProxy=Mock(return_value=server_proxy_mock),
+            do_state_change=Mock(return_value=Mock(errorcode=0))
+        ):
+            self.module = netscaler_server
+            result = self.exited()
+            self.assertEqual(d, {}, 'Graceful disable options were not discarded from the diff_list with the actual object')
+
     def test_new_server_execution_flow(self):
         set_module_args(dict(
             nitro_user='user',
diff --git a/test/units/modules/network/netscaler/test_netscaler_servicegroup.py b/test/units/modules/network/netscaler/test_netscaler_servicegroup.py
index 4f5c9f794a..8c2d8a526b 100644
--- a/test/units/modules/network/netscaler/test_netscaler_servicegroup.py
+++ b/test/units/modules/network/netscaler/test_netscaler_servicegroup.py
@@ -161,12 +161,15 @@ class TestNetscalerServicegroupModule(TestModule):
         m = MagicMock(return_value=servicegroup_proxy_mock)
         servicegroup_exists_mock = Mock(side_effect=[False, True])
 
+        servicegroup_servicegroupmember_binding_mock = Mock(count=Mock(return_value=0))
+
         with patch.multiple(
             'ansible.modules.network.netscaler.netscaler_servicegroup',
             ConfigProxy=m,
             servicegroup_exists=servicegroup_exists_mock,
             servicemembers_identical=Mock(side_effect=[False, True]),
             do_state_change=Mock(return_value=Mock(errorcode=0)),
+            servicegroup_servicegroupmember_binding=servicegroup_servicegroupmember_binding_mock,
             nitro_exception=self.MockException,
         ):
             self.module = netscaler_servicegroup