Skip to content

Commit

Permalink
Merge pull request #354 from netscaler/issue-328-cant-delete-serviceg…
Browse files Browse the repository at this point in the history 
…roup

issue 328 cant delete servicegroup and other minor changes
  • Loading branch information
@sumanth-lingappa
sumanth-lingappa authored Jan 25, 2024
2 parents 350281c + a409781 commit 6c80ccb
Show file tree
Hide file tree
Showing 11 changed files with 1,276 additions and 98 deletions.
5 changes: 5 additions & 0 deletions examples/netprofile.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
hosts: localhost
gather_facts: false
tasks:
- name: Add IPSet
delegate_to: localhost
netscaler.adc.ipset:
state: present
name: ipset-001
- name: Sample Task | netProfile
delegate_to: localhost
netscaler.adc.netprofile:
Expand Down
14 changes: 10 additions & 4 deletions examples/sslcertfile.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,23 @@
hosts: localhost
gather_facts: false
tasks:
- name: Sample Task | Copy certfile to netscaler
delegate_to: localhost
netscaler.adc.systemfile:
state: present
filecontent: "{{ lookup('file', '../tests/data/test-certfile.crt') | b64encode
}}"
filelocation: /var/tmp/
filename: test-certfile.crt
- name: Sample Task | import sslcertfile
delegate_to: localhost
netscaler.adc.sslcertfile:
state: imported
name: ansible-test.crt
src: http://10.06.10.10:8000/test-certfile.crt
# src: local:test-certfile.crt # `local:` means `/var/tmp/` in netscaler
src: local:test-certfile.crt # `local:` means `/var/tmp/` in netscaler
# src: http://10.06.10.10:8000/test-certfile.crt
- name: Sample Task | remove sslcertfile
delegate_to: localhost
netscaler.adc.sslcertfile:
state: absent
name: ansible-test.crt
src: http://10.10.10.10:8000/test-certfile.crt
# src: local:test-certfile.crt # `local:` means `/var/tmp/` in netscaler
159 changes: 79 additions & 80 deletions plugins/module_utils/common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,18 @@ def _check_create_resource_params(resource_name, resource_module_params, action=
]
except KeyError:
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
)
# resource_primary_key = get_primary_key(resource_name, resource_module_params)

if (
resource_primary_key
and resource_primary_key not in resource_module_params.keys()
):
err = "ERROR: Primary key `{}` is missing for the resource `{}`".format(
resource_primary_key, resource_name
)
log(err)
return False, err, {}
# if (
# resource_primary_key
# and resource_primary_key not in resource_module_params.keys()
# ):
# err = "ERROR: Primary key `{}` is missing for the resource `{}`".format(
# resource_primary_key, resource_name
# )
# log(err)
# return False, err, {}

# TODO: check for other mandatory keys for the resource
# This will be checked by ansible itself by reading the `required` field in the schema
Expand Down Expand Up @@ -193,16 +190,24 @@ def create_resource(client, resource_name, resource_module_params, action=None):


@trace
def change_primary_key(resource_name, resource_module_params, resource_primary_key):
def get_primary_key(resource_name, resource_module_params):
resource_primary_key = NITRO_RESOURCE_MAP[resource_name]["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

if resource_primary_key in resource_module_params.keys():
return resource_primary_key

return None

# if resource_name == "ntpserver":
# if "serverip" not in resource_module_params.keys():
# resource_primary_key = "servername"
# return resource_primary_key


@trace
Expand All @@ -218,20 +223,16 @@ def _check_update_resource_params(resource_name, resource_module_params):
else:
resource_update_keys = NITRO_RESOURCE_MAP[resource_name]["update_payload_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
)
# resource_primary_key = get_primary_key(resource_name, resource_module_params)

if resource_primary_key and (
resource_primary_key not in resource_module_params.keys()
):
err = "ERROR: Primary key `{}` is missing for the resource `{}`".format(
resource_primary_key, resource_name
)
log(err)
return False, err, {}
# if resource_primary_key and (
# resource_primary_key not in resource_module_params.keys()
# ):
# err = "ERROR: Primary key `{}` is missing for the resource `{}`".format(
# resource_primary_key, resource_name
# )
# log(err)
# return False, err, {}

# TODO: check for other mandatory keys for the resource

Expand Down Expand Up @@ -279,68 +280,62 @@ def update_resource(client, resource_name, resource_module_params):
)


@trace
def _check_delete_resource_params(resource_name, resource_module_params):
# check if resource_module_params contains any key other than allowed keys for the resource
# check also if resource_module_params contains all the required keys for the resource
# if not, return False, err

resource_primary_key = NITRO_RESOURCE_MAP[resource_name]["primary_key"]
# @trace
# def _check_delete_resource_params(resource_name, resource_module_params):
# # check if resource_module_params contains any key other than allowed keys for the resource
# # check also if resource_module_params contains all the required keys for the resource
# # if not, return False, err

resource_primary_key = change_primary_key(
resource_name, resource_module_params, resource_primary_key
)
# resource_primary_key = get_primary_key(resource_name, resource_module_params)

if (
resource_primary_key
and resource_primary_key not in resource_module_params.keys()
):
err = "ERROR: Primary key `{}` is missing for the resource `{}`".format(
resource_primary_key, resource_name
)
log(err)
return False, err
# if (
# resource_primary_key
# and resource_primary_key not in resource_module_params.keys()
# ):
# err = "ERROR: Primary key `{}` is missing for the resource `{}`".format(
# resource_primary_key, resource_name
# )
# log(err)
# return False, err

return True, None
# return True, None


@trace
def delete_resource(client, resource_name, resource_module_params):
ok, err = _check_delete_resource_params(resource_name, resource_module_params)
if not ok:
return False, err
# ok, err = _check_delete_resource_params(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_primary_key = get_primary_key(resource_name, resource_module_params)

resource_id = resource_module_params[resource_primary_key]
else:
resource_id = None
resource_id = (
resource_module_params[resource_primary_key] if resource_primary_key else None
)

args = {}
for arg_key in NITRO_RESOURCE_MAP[resource_name]["delete_arg_keys"]:
log("DEBUG: arg_key: {}".format(arg_key))
# FIXME: after discussion with Nitro team
if (
resource_name == "lbvserver_servicegroup_binding"
and arg_key == "servicename"
):
# 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`
if resource_primary_key == arg_key:
continue
# if (
# resource_name == "lbvserver_servicegroup_binding"
# and resource_primary_key == "servicename"
# and arg_key == "servicename"
# ):
# # 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:
Expand Down Expand Up @@ -400,7 +395,11 @@ def bind_resource(client, binding_name, binding_module_params):


@trace
def unbind_resource(client, binding_name, binding_module_params):
def unbind_resource(client, binding_name, bindprimary_key, binding_module_params):
# delete all the binding_module_params key:value pairs except the bindprimary_key
for key in binding_module_params.copy().keys():
if key != bindprimary_key:
del binding_module_params[key]
return delete_resource(
client=client,
resource_name=binding_name,
Expand Down
34 changes: 20 additions & 14 deletions plugins/module_utils/module_executor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def __init__(self, resource_name, supports_check_mode=True):
else:
self.resource_id = self.module.params[self.resource_primary_key]
self.resource_module_params = {}
self.desired_bindings = []
self.desired_bindings = {}
self.existing_resource = dict()

self.module_result = dict(
Expand Down Expand Up @@ -168,7 +168,7 @@ def _filter_desired_bindings(self):
in NITRO_RESOURCE_MAP[self.resource_name]["readwrite_arguments"].keys()
):
if v:
self.desired_bindings.append(k)
self.desired_bindings.update({k: v})
log(
"DEBUG: Desired `%s` module specific bindings are: %s"
% (self.resource_name, self.desired_bindings)
Expand Down Expand Up @@ -341,7 +341,7 @@ def create_or_update(self):
self.update_diff_list(
existing=self.existing_resource, desired=self.resource_module_params
)
if not self.existing_resource:
if not self.existing_resource and "add" in self.supported_operations:
self.module_result["changed"] = True
log(
"INFO: Resource %s:%s does not exist. Will be CREATED."
Expand Down Expand Up @@ -448,12 +448,12 @@ def delete_bindings(
self, binding_name, bindprimary_key, to_be_deleted_bindings, existing_bindings
):
for d in list(to_be_deleted_bindings):
deleting_binding = {}
existing_binding_to_delete = {}
for e in existing_bindings:
if d == e[bindprimary_key]:
deleting_binding = e
existing_binding_to_delete = e
break
if not deleting_binding:
if not existing_binding_to_delete:
msg = (
"Binding %s not found in the existing resources. Continuing..." % d
)
Expand All @@ -466,16 +466,19 @@ def delete_bindings(
if binding_name == "servicegroup_servicegroupmember_binding":
try:
if (
deleting_binding["ip"] == "0.0.0.0"
and deleting_binding["servername"]
existing_binding_to_delete["ip"] == "0.0.0.0"
and existing_binding_to_delete["servername"]
):
deleting_binding.pop("ip")
existing_binding_to_delete.pop("ip")
except KeyError:
pass

# Remove all the key:value from deleting_existing_binding which are not part of the playbook
ok, err = unbind_resource(
self.client,
binding_name=binding_name,
binding_module_params=deleting_binding,
bindprimary_key=bindprimary_key,
binding_module_params=existing_binding_to_delete,
)
if not ok:
return False, err
Expand Down Expand Up @@ -550,6 +553,7 @@ def update_bindings(
ok, err = unbind_resource(
self.client,
binding_name=binding_name,
bindprimary_key=bindprimary_key,
binding_module_params=existing_binding,
)
if not ok:
Expand Down Expand Up @@ -581,7 +585,7 @@ def update_bindings(

@trace
def sync_all_bindings(self):
for binding_name in self.desired_bindings:
for binding_name in self.desired_bindings.keys():
self.sync_single_binding(binding_name)

@trace
Expand Down Expand Up @@ -621,11 +625,10 @@ def sync_single_binding(self, binding_name):

if self.module.params["state"] == "absent":
# In `absent` state, we will delete all the existing bindings
to_be_deleted_bindings = existing_binding_members_bindprimary_keys
ok, err = self.delete_bindings(
binding_name=binding_name,
bindprimary_key=bindprimary_key,
to_be_deleted_bindings=to_be_deleted_bindings,
to_be_deleted_bindings=existing_binding_members_bindprimary_keys,
existing_bindings=existing_bindings,
)
if not ok:
Expand Down Expand Up @@ -923,7 +926,10 @@ def main(self):
elif self.module.params["state"] in {"absent"}:
if self.resource_primary_key:
# Bindings
if "bindings" in NITRO_RESOURCE_MAP[self.resource_name].keys():
if (
"bindings" in NITRO_RESOURCE_MAP[self.resource_name].keys()
and NITRO_RESOURCE_MAP[self.resource_name]["bindings"]
):
self.sync_all_bindings()
self.delete()
else:
Expand Down
Loading

0 comments on commit 6c80ccb

Please sign in to comment.