From a01534441d73876804ca18e09488b84a183b0815 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Mon, 16 Jul 2018 06:17:12 +0100
Subject: [PATCH 01/25] added functions to retrieve all ips or primary ip of a
 product

---
 lib/ansible/module_utils/memset.py | 49 ++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/lib/ansible/module_utils/memset.py b/lib/ansible/module_utils/memset.py
index 51dce0c690758a..123cf28977bb41 100644
--- a/lib/ansible/module_utils/memset.py
+++ b/lib/ansible/module_utils/memset.py
@@ -149,3 +149,52 @@ def get_zone_id(zone_name, current_zones):
         msg = 'Zone ID could not be returned as duplicate zone names were detected'
 
     return(zone_exists, msg, counter, zone_id)
+
+
+def get_product_ips(api_key, product):
+    '''
+    Returns a list of IPs allocated to the product. Function
+    execution is considered successful unless msg var is populated.
+    '''
+    ips = []
+    msg = None
+    payload = { 'name': product }
+
+    api_method = 'server.info'
+    has_failed, _msg, response = memset_api_call(api_key=api_key, api_method=api_method, payload=payload)
+
+    if has_failed:
+        # return the API failure to the user
+        msg = _msg
+    elif len(response.json()['ips']) == 0:
+        # this should never happen
+        msg = "{0} has no IPs assigned." . format(product)
+    else:
+        for ip in response.json()['ips']:
+            ips.append(ip['address'])
+
+    return(ips, msg)
+
+
+def get_primary_ip(api_key, product):
+    '''
+    Returns the primary IP of any product which should have one
+    (servers, loadbalancers). Function execution is considered
+    successful unless msg var is populated.
+    '''
+    primary_ip, msg = None, None
+    payload = { 'name': product }
+
+    api_method = 'server.info'
+    has_failed, _msg, response = memset_api_call(api_key=api_key, api_method=api_method, payload=payload)
+
+    if has_failed:
+        # return the API failure to the user
+        msg = _msg
+    elif not response.json()['primary_ip']:
+        # this should never happen
+        msg = "{0} has no primary IP." . format(product)
+    else:
+        primary_ip = response.json()['primary_ip']
+
+    return(primary_ip, msg)

From b59eec8506579179f04b2263296114c7e62ce828 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Fri, 20 Jul 2018 18:17:35 +0100
Subject: [PATCH 02/25] improved docstrings and tidied some minor PEP8 issues

---
 lib/ansible/module_utils/memset.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ansible/module_utils/memset.py b/lib/ansible/module_utils/memset.py
index 123cf28977bb41..0cb13541986177 100644
--- a/lib/ansible/module_utils/memset.py
+++ b/lib/ansible/module_utils/memset.py
@@ -154,11 +154,11 @@ def get_zone_id(zone_name, current_zones):
 def get_product_ips(api_key, product):
     '''
     Returns a list of IPs allocated to the product. Function
-    execution is considered successful unless msg var is populated.
+    execution is considered successful unless msg var is not None.
     '''
     ips = []
     msg = None
-    payload = { 'name': product }
+    payload = {'name': product}
 
     api_method = 'server.info'
     has_failed, _msg, response = memset_api_call(api_key=api_key, api_method=api_method, payload=payload)
@@ -180,10 +180,10 @@ def get_primary_ip(api_key, product):
     '''
     Returns the primary IP of any product which should have one
     (servers, loadbalancers). Function execution is considered
-    successful unless msg var is populated.
+    successful unless msg var is not None.
     '''
     primary_ip, msg = None, None
-    payload = { 'name': product }
+    payload = {'name': product}
 
     api_method = 'server.info'
     has_failed, _msg, response = memset_api_call(api_key=api_key, api_method=api_method, payload=payload)

From 19e5d2c4d1d64ae0c46e81d7e9df60e1a6bea88d Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Fri, 20 Jul 2018 18:20:52 +0100
Subject: [PATCH 03/25] Initial commit of module to manage loadbalancer
 service. Sanity checks all pass.

---
 .../modules/cloud/memset/memset_lb_service.py | 326 ++++++++++++++++++
 1 file changed, 326 insertions(+)
 create mode 100644 lib/ansible/modules/cloud/memset/memset_lb_service.py

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
new file mode 100644
index 00000000000000..e6ffab5119585b
--- /dev/null
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -0,0 +1,326 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+#
+# Copyright (c) 2018, Simon Weald <ansible@simonweald.com>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+ANSIBLE_METADATA = {
+    'metadata_version': '1.1',
+    'status': ['preview'],
+    'supported_by': 'community'
+}
+
+DOCUMENTATION = '''
+---
+module: memset_lb_service
+author: "Simon Weald (@analbeard)"
+version_added: "2.7"
+short_description: Manage Memset loadbalancer services.
+notes:
+  - A loadbalancer service is logically the Internet-facing 'frontend'. This must
+    be backed by one or more servers using the C(memset_lb_server) module. An API key
+    generated via the Memset customer control panel is needed with the following
+    minimum scope - I(loadbalancer.service.add), I(loadbalancer.service.info),
+    I(loadbalancer.service.list), I(loadbalancer.service.remove),
+    I(loadbalancer.service.update).
+description:
+    - Manage Memset loadbalancer services.
+options:
+    state:
+        required: false
+        default: present
+        description:
+            - Indicates desired state of resource. Defaults to present.
+            - When deleting a service there must be no servers currently attached to it as this will raise an error.
+        choices: [ absent, present ]
+    api_key:
+        required: true
+        description:
+            - The API key obtained from the Memset control panel.
+    enabled:
+        required: false
+        default: true
+        type: bool
+        description:
+            - Whether the service is enabled or not. Defaults to True.
+    load_balancer:
+        required: true
+        description:
+            - The name of the load balancer - this is the product name e.g. C(lbtestyaa1).
+    port:
+        required: true
+        description:
+            - The port to be exposed to the Internet.
+            - Must be in the range 1 > 65535 (inclusive).
+    protocol:
+        required: true
+        description:
+            - The protocol to be used by the load balacer.
+        choices: [ tcp, http, https ]
+    service_name:
+        required: true
+        description:
+            - Unique name to identify the service by (must be unique).
+            - It can only consist of letters, numbers, underscores and hyphens and must be a maximum of 64 characters.
+        aliases: [ 'name' ]
+    virtual_ip:
+        required: false
+        description:
+            - The IP address to expose the service on (must be assigned to the loadbalancer product).
+            - If not provided, it will default to the primary IP of the loadbalancer.
+'''
+
+EXAMPLES = '''
+- name: create a loadbalanced service
+  memset_lb_service:
+    state: present
+    api_key: 5eb86c9196ab03919abcf03857163741
+    load_balancer: lbtestyaa1
+    port: 443
+    protocol: https
+    service_name: my_https_service
+    virtual_ip: 1.2.3.4
+  delegate_to: localhost
+'''
+
+RETURN = '''
+---
+memset_api:
+  description: Info from the Memset API
+  returned: when changed or state == present
+  type: complex
+  contains:
+    protocol:
+      description: The protocol to be loadbalanced.
+      returned: always
+      type: string
+      sample: https
+    name:
+      description: The name of the service.
+      returned: always
+      type: string
+      sample: my_https_service
+    enabled:
+      description: Whether the service is enabled.
+      returned: always
+      type: boolean
+      sample: true
+    servers:
+      description: List of dictionaries of the servers attached to the service.
+      returned: always
+      type: list
+      sample: [
+        {
+          "name": "testyaa1",
+          "ip_address": "10.0.0.10",
+          "port": "443",
+          "enabled": "true",
+          "fallback": "false",
+          "weight": "10"
+        }
+      ]
+    virtual_ip:
+      description: The IP address the service is exposed on.
+      returned: always
+      type: string
+      sample: 1.2.3.4
+    port:
+      description: The port exposed to the Internet.
+      returned: always
+      type: integer
+      sample: 443
+'''
+
+import re
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.memset import (memset_api_call, get_product_ips, get_primary_ip)
+
+
+def api_validation(args=None):
+    '''
+    Perform some validation which will be enforced by Memset's API (see:
+    https://www.memset.com/apidocs/methods_loadbalancer.service.html)
+    '''
+    re_match = r'^[a-z0-9-\_]{1,64}$'
+    errors = dict()
+
+    ips, msg = get_product_ips(api_key=args['api_key'], product=args['load_balancer'])
+
+    if not re.match(re_match, args['service_name'].lower()):
+        errors['service_name'] = "Service name can only be contain alphanumeric chars, hyphens and underscores, and must be 64 chars or less."
+    if not 1 <= args['port'] <= 65535:
+        errors['port'] = "Port must be in the range 1 > 65535 (inclusive)."
+    if len(ips) == 0:
+        errors['virtual_ip'] = msg
+    if args['virtual_ip'] and args['virtual_ip'] not in ips:
+            errors['virtual_ip'] = "{0} is not assigned to {1}" . format(args['virtual_ip'], args['load_balancer'])
+
+    if len(errors) > 0:
+        module.fail_json(failed=True, msg=errors)
+
+
+def create_lb_service(args=None, service=None):
+    '''
+    Creates or updates a service. Unique key is the service name
+    so if this isn't matched a new service will be created.
+    '''
+    has_changed, has_failed = False, False
+    memset_api, msg = None, None
+    payload, retvals = dict(), dict()
+
+    # if the user hasn't provided an IP, we use the primary IP of the loadbalancer
+    if not args['virtual_ip']:
+        primary_ip, _msg = get_primary_ip(api_key=args['api_key'], product=args['load_balancer'])
+        if not _msg:
+            args['virtual_ip'] = primary_ip
+        else:
+            retvals['failed'] = True
+            retvals['msg'] = _msg
+            return(retvals)
+
+    for arg in ['enabled', 'load_balancer', 'port', 'protocol', 'service_name', 'virtual_ip']:
+        payload[arg] = args[arg]
+
+    if service is None:
+        # create the service
+        if args['check_mode']:
+            has_changed = True
+            # return what would have been created to the user
+            memset_api = payload
+        else:
+            api_method = 'loadbalancer.service.add'
+            has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+            if not has_failed:
+                has_changed = True
+                memset_api = payload
+                # empty msg as we don't want to return a boatlad of json to the user.
+                msg = None
+    else:
+        # check if the service is the same, then update or do nothing
+        if service == payload:
+            memset_api == payload
+        else:
+            # update service
+            if args['check_mode']:
+                has_changed = True
+                memset_api == payload
+            else:
+                api_method = 'loadbalancer.service.update'
+                has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+                if not has_failed:
+                    has_changed = True
+                    memset_api = payload
+                    # empty msg as we don't want to return a boatload of json to the user.
+                    msg = None
+
+    return(has_changed, has_failed, memset_api, msg)
+
+
+def delete_lb_service(args=None, service=None):
+    '''
+    Deletes a service if it exists. If there are still servers
+    attached to the service then it will fail as these must be
+    detached first.
+    '''
+    has_changed, has_failed = False, False
+    memset_api, msg = None, None
+    payload = dict()
+
+    if service is not None:
+        for arg in ['load_balancer', 'service_name']:
+            payload[arg] = args[arg]
+        if args['check_mode']:
+            has_changed = True
+            memset_api = payload
+        else:
+            api_method = 'loadbalancer.service.remove'
+            has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+            if not has_failed:
+                has_changed = True
+                memset_api = payload
+                # empty msg as we don't want to return a boatload of json to the user.
+                msg = None
+
+    return(has_changed, has_failed, memset_api, msg)
+
+
+def create_or_delete(args=None):
+    '''
+    Performs initial auth validation and gets a list of
+    existing services to provide to create/delete functions.
+    '''
+    has_failed, has_changed = False, False
+    msg, memset_api, current_service = None, None, None
+    retvals, payload = dict(), dict()
+
+    # get the current services and check if the relevant service exists.
+    payload['load_balancer'] = args['load_balancer']
+    api_method = 'loadbalancer.service.list'
+    _has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+
+    if _has_failed:
+        # this is the first time the API is called; incorrect credentials will
+        # manifest themselves at this point so we need to ensure the user is
+        # informed of the reason.
+        retvals['failed'] = _has_failed
+        retvals['msg'] = msg
+        retvals['stderr'] = "API returned an error: {0}" . format(response.status_code)
+        return(retvals)
+
+    for service in response.json():
+        if service['name'] == args['service_name']:
+            current_service = service
+
+    if args['state'] == 'present':
+        has_changed, has_failed, memset_api, msg = create_lb_service(args=args, service=current_service)
+
+    if args['state'] == 'absent':
+        has_changed, has_failed, memset_api, msg = delete_lb_service(args=args, service=current_service)
+
+    retvals['changed'] = has_changed
+    retvals['failed'] = has_failed
+    for val in ['msg', 'memset_api']:
+        if val is not None:
+            retvals[val] = eval(val)
+
+    return(retvals)
+
+
+def main():
+    global module
+    module = AnsibleModule(
+        argument_spec=dict(
+            state=dict(default='present', choices=['present', 'absent'], type='str'),
+            api_key=dict(required=True, type='str', no_log=True),
+            enabled=dict(default="True", type='bool'),
+            load_balancer=dict(required=True, type='str'),
+            port=dict(required=True, type=int),
+            protocol=dict(required=True, choices=['tcp', 'http', 'https'], type='str'),
+            service_name=dict(required=True, type='str', aliases=['name']),
+            virtual_ip=dict(required=False, type='str')
+        ),
+        supports_check_mode=True
+    )
+
+    # populate the dict with the user-provided vars.
+    args = dict()
+    for key, arg in module.params.items():
+        args[key] = arg
+    args['check_mode'] = module.check_mode
+
+    # validate some API-specific limitations.
+    api_validation(args=args)
+
+    retvals = create_or_delete(args)
+
+    if retvals['failed']:
+        module.fail_json(**retvals)
+    else:
+        module.exit_json(**retvals)
+
+
+if __name__ == '__main__':
+    main()

From 7a3de2bcc873d916a46f3b5237ba839779ac92b7 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Sat, 21 Jul 2018 18:17:43 +0100
Subject: [PATCH 04/25] evaluate variables so we only return those which have
 been set

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index e6ffab5119585b..a0213a033d0c45 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -283,7 +283,7 @@ def create_or_delete(args=None):
     retvals['changed'] = has_changed
     retvals['failed'] = has_failed
     for val in ['msg', 'memset_api']:
-        if val is not None:
+        if eval(val) is not None:
             retvals[val] = eval(val)
 
     return(retvals)

From 6bf1b095ebf874337bc145a654d7eba5b7ff4930 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Sat, 21 Jul 2018 19:27:43 +0100
Subject: [PATCH 05/25] correct and tidy documentation, ensure seperate
 dictionary keys for any errors

---
 .../modules/cloud/memset/memset_lb_service.py | 41 ++++++++++---------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index a0213a033d0c45..adb3b7527bb375 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -93,24 +93,27 @@
   returned: when changed or state == present
   type: complex
   contains:
-    protocol:
-      description: The protocol to be loadbalanced.
-      returned: always
-      type: string
-      sample: https
-    name:
-      description: The name of the service.
-      returned: always
-      type: string
-      sample: my_https_service
     enabled:
       description: Whether the service is enabled.
-      returned: always
+      returned: when state=present
       type: boolean
       sample: true
+    load_balancer:
+      description: The name of the loadbalancer product.
+      returned: always
+      type: string
+      sample: lbtestyaa1
+    port:
+      description: The port the service is exposed on.
+      returned: when state=present
+    protocol:
+      description: The protocol to be loadbalanced.
+      returned: when state=present
+      type: string
+      sample: https
     servers:
       description: List of dictionaries of the servers attached to the service.
-      returned: always
+      returned: when state=present
       type: list
       sample: [
         {
@@ -122,16 +125,16 @@
           "weight": "10"
         }
       ]
+    service_name:
+      description: The name of the service.
+      returned: always
+      type: string
+      sample: my_https_service
     virtual_ip:
       description: The IP address the service is exposed on.
-      returned: always
+      returned: when state=present
       type: string
       sample: 1.2.3.4
-    port:
-      description: The port exposed to the Internet.
-      returned: always
-      type: integer
-      sample: 443
 '''
 
 import re
@@ -154,7 +157,7 @@ def api_validation(args=None):
     if not 1 <= args['port'] <= 65535:
         errors['port'] = "Port must be in the range 1 > 65535 (inclusive)."
     if len(ips) == 0:
-        errors['virtual_ip'] = msg
+        errors['misc'] = msg
     if args['virtual_ip'] and args['virtual_ip'] not in ips:
             errors['virtual_ip'] = "{0} is not assigned to {1}" . format(args['virtual_ip'], args['load_balancer'])
 

From e9883a4e77d0793cc78d14f654871d4b4c619ea4 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Fri, 2 Nov 2018 22:37:34 +0000
Subject: [PATCH 06/25] manipulate the returned service info in order to
 massage it into something we can use for comparisons

---
 .../modules/cloud/memset/memset_lb_service.py | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index adb3b7527bb375..f89e605b412a8e 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -184,10 +184,12 @@ def create_lb_service(args=None, service=None):
             retvals['msg'] = _msg
             return(retvals)
 
-    for arg in ['enabled', 'load_balancer', 'port', 'protocol', 'service_name', 'virtual_ip']:
+    for arg in ['enabled', 'port', 'protocol', 'service_name', 'virtual_ip']:
         payload[arg] = args[arg]
 
     if service is None:
+        # add load_balancer to the payload late
+        payload['load_balancer'] = args['load_balancer']
         # create the service
         if args['check_mode']:
             has_changed = True
@@ -202,10 +204,21 @@ def create_lb_service(args=None, service=None):
                 # empty msg as we don't want to return a boatlad of json to the user.
                 msg = None
     else:
-        # check if the service is the same, then update or do nothing
-        if service == payload:
+        # perform various horrible contortions in order to compare the existing service
+        # to the payload we intend to POST.
+        _service = service.copy()
+        _service['service_name'] = service['name']
+        del _service['name']
+        try:
+            del _service['servers']
+        except Exception:
+            pass
+
+        if _service == payload:
             memset_api == payload
         else:
+            # add load_balancer to the payload late so we can compare dicts beforehand
+            payload['load_balancer'] = args['load_balancer']
             # update service
             if args['check_mode']:
                 has_changed = True

From c95811bcf5f433d7bf44bb2d813a0874ee6503b7 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Mon, 17 Dec 2018 21:03:23 +0000
Subject: [PATCH 07/25] improve module notes

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index f89e605b412a8e..c7381d07df9c20 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -17,13 +17,13 @@
 ---
 module: memset_lb_service
 author: "Simon Weald (@analbeard)"
-version_added: "2.7"
+version_added: "2.8"
 short_description: Manage Memset loadbalancer services.
 notes:
   - A loadbalancer service is logically the Internet-facing 'frontend'. This must
-    be backed by one or more servers using the C(memset_lb_server) module. An API key
-    generated via the Memset customer control panel is needed with the following
-    minimum scope - I(loadbalancer.service.add), I(loadbalancer.service.info),
+    be backed by one or more servers using the C(memset_lb_server) module.
+  - An API key generated via the Memset customer control panel is needed with the
+    following minimum scope - I(loadbalancer.service.add), I(loadbalancer.service.info),
     I(loadbalancer.service.list), I(loadbalancer.service.remove),
     I(loadbalancer.service.update).
 description:

From af68f97e8c76a2f675155abd50193837ba8885ef Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Mon, 17 Dec 2018 21:22:03 +0000
Subject: [PATCH 08/25] rework module to avoid unecessarily passing around lots
 of  variables

---
 .../modules/cloud/memset/memset_lb_service.py | 74 ++++++++-----------
 1 file changed, 32 insertions(+), 42 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index c7381d07df9c20..a736deb5b7c167 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -170,9 +170,8 @@ def create_lb_service(args=None, service=None):
     Creates or updates a service. Unique key is the service name
     so if this isn't matched a new service will be created.
     '''
-    has_changed, has_failed = False, False
-    memset_api, msg = None, None
-    payload, retvals = dict(), dict()
+    retvals, payload = dict(), dict()
+    retvals['changed'], retvals['failed'] = False, False
 
     # if the user hasn't provided an IP, we use the primary IP of the loadbalancer
     if not args['virtual_ip']:
@@ -192,15 +191,15 @@ def create_lb_service(args=None, service=None):
         payload['load_balancer'] = args['load_balancer']
         # create the service
         if args['check_mode']:
-            has_changed = True
+            retvals['changed'] = True
             # return what would have been created to the user
-            memset_api = payload
+            retvals['memset_api'] = payload
         else:
             api_method = 'loadbalancer.service.add'
-            has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
-            if not has_failed:
-                has_changed = True
-                memset_api = payload
+            retvals['failed'], msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+            if not retvals['failed']:
+                retvals['changed'] = True
+                retvals['memset_api'] = payload
                 # empty msg as we don't want to return a boatlad of json to the user.
                 msg = None
     else:
@@ -215,24 +214,22 @@ def create_lb_service(args=None, service=None):
             pass
 
         if _service == payload:
-            memset_api == payload
+            memset_api = payload
         else:
             # add load_balancer to the payload late so we can compare dicts beforehand
             payload['load_balancer'] = args['load_balancer']
             # update service
             if args['check_mode']:
-                has_changed = True
-                memset_api == payload
+                retvals['changed'] = True
+                retvals['memset_api'] = payload
             else:
                 api_method = 'loadbalancer.service.update'
-                has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
-                if not has_failed:
-                    has_changed = True
-                    memset_api = payload
-                    # empty msg as we don't want to return a boatload of json to the user.
-                    msg = None
+                retvals['failed'], msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+                if not retvals['failed']:
+                    retvals['changed'] = True
+                    retvals['memset_api'] = payload
 
-    return(has_changed, has_failed, memset_api, msg)
+    return(retvals)
 
 
 def delete_lb_service(args=None, service=None):
@@ -241,26 +238,23 @@ def delete_lb_service(args=None, service=None):
     attached to the service then it will fail as these must be
     detached first.
     '''
-    has_changed, has_failed = False, False
-    memset_api, msg = None, None
-    payload = dict()
+    retvals, payload = dict(), dict()
+    retvals['changed'], retvals['failed'] = False, False
 
     if service is not None:
         for arg in ['load_balancer', 'service_name']:
             payload[arg] = args[arg]
         if args['check_mode']:
-            has_changed = True
-            memset_api = payload
+            retvals['changed'] = True
+            retvals['memset_api'] = payload
         else:
             api_method = 'loadbalancer.service.remove'
-            has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
-            if not has_failed:
-                has_changed = True
-                memset_api = payload
-                # empty msg as we don't want to return a boatload of json to the user.
-                msg = None
+            retvals['failed'], msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+            if not retvals['_failed']:
+                retvals['changed'] = True
+                retvals['memset_api'] = payload
 
-    return(has_changed, has_failed, memset_api, msg)
+    return(retvals)
 
 
 def create_or_delete(args=None):
@@ -268,9 +262,11 @@ def create_or_delete(args=None):
     Performs initial auth validation and gets a list of
     existing services to provide to create/delete functions.
     '''
-    has_failed, has_changed = False, False
-    msg, memset_api, current_service = None, None, None
+    # has_failed, has_changed = False, False
+    # msg, memset_api, current_service = None, None, None
+    # retvals, payload = dict(), dict()
     retvals, payload = dict(), dict()
+    retvals['changed'], retvals['failed'] = False, False
 
     # get the current services and check if the relevant service exists.
     payload['load_balancer'] = args['load_balancer']
@@ -281,7 +277,7 @@ def create_or_delete(args=None):
         # this is the first time the API is called; incorrect credentials will
         # manifest themselves at this point so we need to ensure the user is
         # informed of the reason.
-        retvals['failed'] = _has_failed
+        retvals['failed'] = True
         retvals['msg'] = msg
         retvals['stderr'] = "API returned an error: {0}" . format(response.status_code)
         return(retvals)
@@ -291,16 +287,10 @@ def create_or_delete(args=None):
             current_service = service
 
     if args['state'] == 'present':
-        has_changed, has_failed, memset_api, msg = create_lb_service(args=args, service=current_service)
+        retvals = create_lb_service(args=args, service=current_service)
 
     if args['state'] == 'absent':
-        has_changed, has_failed, memset_api, msg = delete_lb_service(args=args, service=current_service)
-
-    retvals['changed'] = has_changed
-    retvals['failed'] = has_failed
-    for val in ['msg', 'memset_api']:
-        if eval(val) is not None:
-            retvals[val] = eval(val)
+        retvals = delete_lb_service(args=args, service=current_service)
 
     return(retvals)
 

From 118479b0e2bceaffc1b90607d2bdf153064b9709 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Fri, 21 Dec 2018 09:03:55 +0000
Subject: [PATCH 09/25] a bunch more tidying up

---
 .../modules/cloud/memset/memset_lb_service.py      | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index a736deb5b7c167..c9698b62b3dde4 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -16,7 +16,7 @@
 DOCUMENTATION = '''
 ---
 module: memset_lb_service
-author: "Simon Weald (@analbeard)"
+author: "Simon Weald (@glitchcrab)"
 version_added: "2.8"
 short_description: Manage Memset loadbalancer services.
 notes:
@@ -203,7 +203,7 @@ def create_lb_service(args=None, service=None):
                 # empty msg as we don't want to return a boatlad of json to the user.
                 msg = None
     else:
-        # perform various horrible contortions in order to compare the existing service
+        # perform various contortions in order to compare the existing service
         # to the payload we intend to POST.
         _service = service.copy()
         _service['service_name'] = service['name']
@@ -214,13 +214,15 @@ def create_lb_service(args=None, service=None):
             pass
 
         if _service == payload:
-            memset_api = payload
+            # the payload and the service are the same, so we just exit unchanged
+            retvals['memset_api'] = payload
         else:
-            # add load_balancer to the payload late so we can compare dicts beforehand
+            # add load_balancer to the payload after we've compared the dicts
             payload['load_balancer'] = args['load_balancer']
             # update service
             if args['check_mode']:
                 retvals['changed'] = True
+                # TODO: return a diff instead of the the whole payload
                 retvals['memset_api'] = payload
             else:
                 api_method = 'loadbalancer.service.update'
@@ -246,7 +248,6 @@ def delete_lb_service(args=None, service=None):
             payload[arg] = args[arg]
         if args['check_mode']:
             retvals['changed'] = True
-            retvals['memset_api'] = payload
         else:
             api_method = 'loadbalancer.service.remove'
             retvals['failed'], msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
@@ -262,9 +263,6 @@ def create_or_delete(args=None):
     Performs initial auth validation and gets a list of
     existing services to provide to create/delete functions.
     '''
-    # has_failed, has_changed = False, False
-    # msg, memset_api, current_service = None, None, None
-    # retvals, payload = dict(), dict()
     retvals, payload = dict(), dict()
     retvals['changed'], retvals['failed'] = False, False
 

From b3e2f85c49efe844b514a6ace4227490707f822c Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Fri, 21 Dec 2018 20:12:52 +0000
Subject: [PATCH 10/25] do some tidying up and return a diff when updating

---
 .../modules/cloud/memset/memset_lb_service.py        | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index c9698b62b3dde4..c550d23f45c3d8 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -217,13 +217,13 @@ def create_lb_service(args=None, service=None):
             # the payload and the service are the same, so we just exit unchanged
             retvals['memset_api'] = payload
         else:
+            _diff = dict(payload.items() ^ _service.items())
             # add load_balancer to the payload after we've compared the dicts
             payload['load_balancer'] = args['load_balancer']
             # update service
             if args['check_mode']:
                 retvals['changed'] = True
-                # TODO: return a diff instead of the the whole payload
-                retvals['memset_api'] = payload
+                retvals['diff'] = _diff
             else:
                 api_method = 'loadbalancer.service.update'
                 retvals['failed'], msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
@@ -251,9 +251,8 @@ def delete_lb_service(args=None, service=None):
         else:
             api_method = 'loadbalancer.service.remove'
             retvals['failed'], msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
-            if not retvals['_failed']:
+            if not retvals['failed']:
                 retvals['changed'] = True
-                retvals['memset_api'] = payload
 
     return(retvals)
 
@@ -269,9 +268,9 @@ def create_or_delete(args=None):
     # get the current services and check if the relevant service exists.
     payload['load_balancer'] = args['load_balancer']
     api_method = 'loadbalancer.service.list'
-    _has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+    has_failed, msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
 
-    if _has_failed:
+    if has_failed:
         # this is the first time the API is called; incorrect credentials will
         # manifest themselves at this point so we need to ensure the user is
         # informed of the reason.
@@ -280,6 +279,7 @@ def create_or_delete(args=None):
         retvals['stderr'] = "API returned an error: {0}" . format(response.status_code)
         return(retvals)
 
+    current_service = None
     for service in response.json():
         if service['name'] == args['service_name']:
             current_service = service

From 559abf899b2b9513737ed078bec7969c6eabb26b Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Mon, 17 Dec 2018 10:49:06 +0000
Subject: [PATCH 11/25] cherry-picked e13d83edcf

---
 lib/ansible/module_utils/memset.py | 84 ++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/lib/ansible/module_utils/memset.py b/lib/ansible/module_utils/memset.py
index 0cb13541986177..2b0bd48f783b45 100644
--- a/lib/ansible/module_utils/memset.py
+++ b/lib/ansible/module_utils/memset.py
@@ -198,3 +198,87 @@ def get_primary_ip(api_key, product):
         primary_ip = response.json()['primary_ip']
 
     return(primary_ip, msg)
+
+
+class MemsetServer(object):
+
+    def __init__(self, data):
+        '''
+        Create an object which can represent either a server
+        or a loadbalancer product.
+        '''
+        self.data = data
+        # support reboots for miniservers only
+        if self.data['type'] in ['miniserver', 'fullserver']:
+            self.has_reboot = True
+        else:
+            self.has_reboot = False
+
+    def primary_vlan(self):
+        '''
+        Look up the product's primary VLAN (untagged). It is only
+        possible to have one untagged VLAN and so safe to return the
+        first list item.
+        '''
+        if self.data['vlans']['untagged']:
+            return self.data['vlans']['untagged'][0]
+        else:
+            return None
+
+    def tagged_vlans(self):
+        '''
+        Return a list of the product's secondary VLANs (tagged).
+        '''
+        tagged_vlans = []
+
+        if len(self.data['vlans']['tagged']) > 0:
+            for vlan in self.data['vlans']['tagged']:
+                tagged_vlans.append(vlan)
+
+        return tagged_vlans
+
+    def primary_ip(self):
+        '''
+        Return the primary IP.
+        '''
+        ip = None
+
+        if self.data['primary_ip']:
+            ip = self.data['primary_ip']
+
+        return ip
+
+    def all_ips(self):
+        '''
+        Return a list of all IPs attached to the product. Note that
+        this list is generated from Memset's product info and may not
+        match those configured on the server itself.
+        '''
+        ips = []
+
+        for ip in self.data['ips']:
+            for key, val in ip.items():
+                if key == 'address':
+                    ips.append(val)
+
+        return ips
+
+    def network_zone(self):
+        '''
+        Return the product's network zone. This is exposed as a list,
+        however the product can only ever have one network zone.
+        '''
+        for zone in self.data['network_zones']:
+            return zone
+
+    def reboot_server(self, api_key=None):
+        '''
+        Reboots the server, provided it can be rebooted through
+        the API.
+        '''
+        payload = dict()
+
+        if self.has_reboot:
+            payload['name'] = self.data['name']
+            api_method = 'server.reboot'
+            has_failed, msg, response = memset_api_call(api_key=api_key, api_method=api_method, payload=payload)

From 8efe84b694808037d043902f6f4ec7ba8a9f4878 Mon Sep 17 00:00:00 2001
From: Simon Weald <simon@simonweald.com>
Date: Fri, 21 Dec 2018 21:58:45 +0000
Subject: [PATCH 12/25] use MemsetServer class to represent the loadbalancer -
 saves on API calls

---
 .../modules/cloud/memset/memset_lb_service.py | 52 +++++++++++--------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index c550d23f45c3d8..c6015668ae8185 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -139,10 +139,10 @@
 
 import re
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.memset import (memset_api_call, get_product_ips, get_primary_ip)
+from ansible.module_utils.memset import (memset_api_call, get_product_ips, get_primary_ip, MemsetServer)
 
 
-def api_validation(args=None):
+def api_validation(args=None, loadbalancer=None):
     '''
     Perform some validation which will be enforced by Memset's API (see:
     https://www.memset.com/apidocs/methods_loadbalancer.service.html)
@@ -150,22 +150,20 @@ def api_validation(args=None):
     re_match = r'^[a-z0-9-\_]{1,64}$'
     errors = dict()
 
-    ips, msg = get_product_ips(api_key=args['api_key'], product=args['load_balancer'])
-
     if not re.match(re_match, args['service_name'].lower()):
         errors['service_name'] = "Service name can only be contain alphanumeric chars, hyphens and underscores, and must be 64 chars or less."
     if not 1 <= args['port'] <= 65535:
         errors['port'] = "Port must be in the range 1 > 65535 (inclusive)."
-    if len(ips) == 0:
-        errors['misc'] = msg
-    if args['virtual_ip'] and args['virtual_ip'] not in ips:
+    if len(loadbalancer.all_ips()) == 0:
+        errors['misc'] = 'No IPs attached to loadbalancer'
+    if args['virtual_ip'] and args['virtual_ip'] not in loadbalancer.all_ips():
             errors['virtual_ip'] = "{0} is not assigned to {1}" . format(args['virtual_ip'], args['load_balancer'])
 
     if len(errors) > 0:
         module.fail_json(failed=True, msg=errors)
 
 
-def create_lb_service(args=None, service=None):
+def create_lb_service(args=None, service=None, loadbalancer=None):
     '''
     Creates or updates a service. Unique key is the service name
     so if this isn't matched a new service will be created.
@@ -175,13 +173,7 @@ def create_lb_service(args=None, service=None):
 
     # if the user hasn't provided an IP, we use the primary IP of the loadbalancer
     if not args['virtual_ip']:
-        primary_ip, _msg = get_primary_ip(api_key=args['api_key'], product=args['load_balancer'])
-        if not _msg:
-            args['virtual_ip'] = primary_ip
-        else:
-            retvals['failed'] = True
-            retvals['msg'] = _msg
-            return(retvals)
+        args['virtual_ip'] = loadbalancer.primary_ip()
 
     for arg in ['enabled', 'port', 'protocol', 'service_name', 'virtual_ip']:
         payload[arg] = args[arg]
@@ -200,8 +192,6 @@ def create_lb_service(args=None, service=None):
             if not retvals['failed']:
                 retvals['changed'] = True
                 retvals['memset_api'] = payload
-                # empty msg as we don't want to return a boatlad of json to the user.
-                msg = None
     else:
         # perform various contortions in order to compare the existing service
         # to the payload we intend to POST.
@@ -209,6 +199,7 @@ def create_lb_service(args=None, service=None):
         _service['service_name'] = service['name']
         del _service['name']
         try:
+            # there may be servers attached to the service - remove those too.
             del _service['servers']
         except Exception:
             pass
@@ -257,7 +248,7 @@ def delete_lb_service(args=None, service=None):
     return(retvals)
 
 
-def create_or_delete(args=None):
+def create_or_delete(args=None, loadbalancer=None):
     '''
     Performs initial auth validation and gets a list of
     existing services to provide to create/delete functions.
@@ -285,7 +276,7 @@ def create_or_delete(args=None):
             current_service = service
 
     if args['state'] == 'present':
-        retvals = create_lb_service(args=args, service=current_service)
+        retvals = create_lb_service(args=args, service=current_service, loadbalancer=loadbalancer)
 
     if args['state'] == 'absent':
         retvals = delete_lb_service(args=args, service=current_service)
@@ -315,10 +306,29 @@ def main():
         args[key] = arg
     args['check_mode'] = module.check_mode
 
+    # make an initial API call to get the loadbalancer's info.
+    payload = dict()
+    payload['name'] = args['load_balancer']
+    api_method = 'server.info'
+    has_failed, _msg, response = memset_api_call(api_key=args['api_key'], api_method=api_method, payload=payload)
+
+    retvals = dict()
+    if has_failed:
+        # this is the first time the API is called; incorrect credentials will
+        # manifest themselves at this point so we need to ensure the user is
+        # informed of the reason.
+        retvals['failed'] = has_failed
+        retvals['msg'] = _msg
+        retvals['stderr'] = "API returned an error: {0}" . format(response.status_code)
+        module.fail_json(**retvals)
+
+    # create an object to represent this loadbalancer.
+    loadbalancer = MemsetServer(response.json())
+
     # validate some API-specific limitations.
-    api_validation(args=args)
+    api_validation(args, loadbalancer)
 
-    retvals = create_or_delete(args)
+    retvals = create_or_delete(args, loadbalancer)
 
     if retvals['failed']:
         module.fail_json(**retvals)

From e963fabb7f83aa7907b3e346b03aef280900d2ee Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Fri, 10 May 2019 20:54:32 +0100
Subject: [PATCH 13/25] now targetting 2.9

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index c6015668ae8185..a6b00e5fd99fc2 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -17,7 +17,7 @@
 ---
 module: memset_lb_service
 author: "Simon Weald (@glitchcrab)"
-version_added: "2.8"
+version_added: "2.9"
 short_description: Manage Memset loadbalancer services.
 notes:
   - A loadbalancer service is logically the Internet-facing 'frontend'. This must

From 63c37ed247a90e2b8e5e4533ba678fd970d7dfca Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Mon, 13 May 2019 10:24:33 +0100
Subject: [PATCH 14/25] improve docs to indicate service name pitfall

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index a6b00e5fd99fc2..d420e00fd3283c 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -63,7 +63,7 @@
     service_name:
         required: true
         description:
-            - Unique name to identify the service by (must be unique).
+            - Unique name to identify the service by (must be unique). Changing this will cause a new service to be created.
             - It can only consist of letters, numbers, underscores and hyphens and must be a maximum of 64 characters.
         aliases: [ 'name' ]
     virtual_ip:

From 2523b1598485b3924c98c623324c3c76ee30ff11 Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Mon, 13 May 2019 11:35:11 +0100
Subject: [PATCH 15/25] ensure a message is returned when a service cannot be
 added

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index d420e00fd3283c..4a03f823343cc3 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -192,6 +192,8 @@ def create_lb_service(args=None, service=None, loadbalancer=None):
             if not retvals['failed']:
                 retvals['changed'] = True
                 retvals['memset_api'] = payload
+            else:
+                retvals['msg'] = msg
     else:
         # perform various contortions in order to compare the existing service
         # to the payload we intend to POST.

From 13e39298b50fdba3fa9940423cf5fb87c8e30875 Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Mon, 13 May 2019 12:06:06 +0100
Subject: [PATCH 16/25] correct excessive indentation

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index 4a03f823343cc3..c1ec39982e64e9 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -157,7 +157,7 @@ def api_validation(args=None, loadbalancer=None):
     if len(loadbalancer.all_ips()) == 0:
         errors['misc'] = 'No IPs attached to loadbalancer'
     if args['virtual_ip'] and args['virtual_ip'] not in loadbalancer.all_ips():
-            errors['virtual_ip'] = "{0} is not assigned to {1}" . format(args['virtual_ip'], args['load_balancer'])
+        errors['virtual_ip'] = "{0} is not assigned to {1}" . format(args['virtual_ip'], args['load_balancer'])
 
     if len(errors) > 0:
         module.fail_json(failed=True, msg=errors)

From 8e35c320ebb0be6bd96ef50b762fa01296501adf Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Mon, 13 May 2019 13:09:18 +0100
Subject: [PATCH 17/25] use sets when comparing dicts to ensure compatibility
 with python2

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index c1ec39982e64e9..3995dda56f206d 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -210,7 +210,7 @@ def create_lb_service(args=None, service=None, loadbalancer=None):
             # the payload and the service are the same, so we just exit unchanged
             retvals['memset_api'] = payload
         else:
-            _diff = dict(payload.items() ^ _service.items())
+            _diff = dict(set(payload.items()) ^ set(_service.items()))
             # add load_balancer to the payload after we've compared the dicts
             payload['load_balancer'] = args['load_balancer']
             # update service

From 788ee6d64f1d3821b3e87d8ad8c2e904b462513d Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Mon, 13 May 2019 15:16:56 +0100
Subject: [PATCH 18/25] update copyright date, remove the period from the short
 description

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index 3995dda56f206d..b783770fb00ac6 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -1,7 +1,7 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
-# Copyright (c) 2018, Simon Weald <ansible@simonweald.com>
+# Copyright (c) 2019, Simon Weald <ansible@simonweald.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import (absolute_import, division, print_function)
@@ -27,7 +27,7 @@
     I(loadbalancer.service.list), I(loadbalancer.service.remove),
     I(loadbalancer.service.update).
 description:
-    - Manage Memset loadbalancer services.
+    - Manage Memset loadbalancer services
 options:
     state:
         required: false

From a3030a377b991e36135d55d36445341f0fe657c1 Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Mon, 13 May 2019 16:01:46 +0100
Subject: [PATCH 19/25] remove another erroneous period

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index b783770fb00ac6..e97f01eee9937a 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -18,7 +18,7 @@
 module: memset_lb_service
 author: "Simon Weald (@glitchcrab)"
 version_added: "2.9"
-short_description: Manage Memset loadbalancer services.
+short_description: Manage Memset loadbalancer services
 notes:
   - A loadbalancer service is logically the Internet-facing 'frontend'. This must
     be backed by one or more servers using the C(memset_lb_server) module.

From 9861a94da551d3c5dec58edcc2273fc12f46c0c8 Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Tue, 14 May 2019 09:03:34 +0100
Subject: [PATCH 20/25] add missing permission to API key requirements

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index e97f01eee9937a..d3f3d8f9e7bc1f 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -25,7 +25,7 @@
   - An API key generated via the Memset customer control panel is needed with the
     following minimum scope - I(loadbalancer.service.add), I(loadbalancer.service.info),
     I(loadbalancer.service.list), I(loadbalancer.service.remove),
-    I(loadbalancer.service.update).
+    I(loadbalancer.service.update), I(server.info).
 description:
     - Manage Memset loadbalancer services
 options:

From bb3b4fb4fd6e9e7d7e8f3b4ce5781588ed3affaf Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Fri, 17 May 2019 14:52:56 +0100
Subject: [PATCH 21/25] add type hinting to all options  in the documentation

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index d3f3d8f9e7bc1f..0e2e718b7c8ee8 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -30,14 +30,15 @@
     - Manage Memset loadbalancer services
 options:
     state:
-        required: false
         default: present
+        type: str
         description:
             - Indicates desired state of resource. Defaults to present.
             - When deleting a service there must be no servers currently attached to it as this will raise an error.
         choices: [ absent, present ]
     api_key:
         required: true
+        type: str
         description:
             - The API key obtained from the Memset control panel.
     enabled:
@@ -48,26 +49,31 @@
             - Whether the service is enabled or not. Defaults to True.
     load_balancer:
         required: true
+        type: str
         description:
             - The name of the load balancer - this is the product name e.g. C(lbtestyaa1).
     port:
         required: true
+        type: int
         description:
             - The port to be exposed to the Internet.
             - Must be in the range 1 > 65535 (inclusive).
     protocol:
         required: true
+        type: str
         description:
             - The protocol to be used by the load balacer.
         choices: [ tcp, http, https ]
     service_name:
         required: true
+        type: str
         description:
             - Unique name to identify the service by (must be unique). Changing this will cause a new service to be created.
             - It can only consist of letters, numbers, underscores and hyphens and must be a maximum of 64 characters.
         aliases: [ 'name' ]
     virtual_ip:
         required: false
+        type: str
         description:
             - The IP address to expose the service on (must be assigned to the loadbalancer product).
             - If not provided, it will default to the primary IP of the loadbalancer.

From e25567b57110426848b314a5722620ced98ab1fd Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Fri, 17 May 2019 14:54:09 +0100
Subject: [PATCH 22/25] add an example of deleting a loadbalancer service

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index 0e2e718b7c8ee8..720a27ef8a188a 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -90,6 +90,14 @@
     service_name: my_https_service
     virtual_ip: 1.2.3.4
   delegate_to: localhost
+
+- name: delete a loadbalanced service
+  memset_lb_service:
+    state: absent
+    api_key: 5eb86c9196ab03919abcf03857163741
+    load_balancer: lbtestyaa1
+    service_name: my_https_service
+  delegate_to: localhost
 '''
 
 RETURN = '''

From aa5397f918a2e89dbc572948d87314f135bda81e Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Fri, 17 May 2019 15:08:48 +0100
Subject: [PATCH 23/25] make port and protocol only required when state=present
 and update documentation accordingly

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index 720a27ef8a188a..48f7cf8a736667 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -53,13 +53,11 @@
         description:
             - The name of the load balancer - this is the product name e.g. C(lbtestyaa1).
     port:
-        required: true
         type: int
         description:
             - The port to be exposed to the Internet.
             - Must be in the range 1 > 65535 (inclusive).
     protocol:
-        required: true
         type: str
         description:
             - The protocol to be used by the load balacer.
@@ -308,12 +306,15 @@ def main():
             api_key=dict(required=True, type='str', no_log=True),
             enabled=dict(default="True", type='bool'),
             load_balancer=dict(required=True, type='str'),
-            port=dict(required=True, type=int),
-            protocol=dict(required=True, choices=['tcp', 'http', 'https'], type='str'),
+            port=dict(required=False, type=int),
+            protocol=dict(required=False, choices=['tcp', 'http', 'https'], type='str'),
             service_name=dict(required=True, type='str', aliases=['name']),
             virtual_ip=dict(required=False, type='str')
         ),
-        supports_check_mode=True
+        supports_check_mode=True,
+        required_if=[
+            ["state", "present", ["protocol", "port"]]
+        ]
     )
 
     # populate the dict with the user-provided vars.

From 79f3dba28871ad0bc6dbf66d0d88502945b0b77d Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Fri, 17 May 2019 15:16:22 +0100
Subject: [PATCH 24/25] return deleted resource when state=absent

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index 48f7cf8a736667..c3eca6f2022978 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -102,7 +102,7 @@
 ---
 memset_api:
   description: Info from the Memset API
-  returned: when changed or state == present
+  returned: when changed
   type: complex
   contains:
     enabled:
@@ -251,6 +251,7 @@ def delete_lb_service(args=None, service=None):
     if service is not None:
         for arg in ['load_balancer', 'service_name']:
             payload[arg] = args[arg]
+        retvals['memset_api'] = payload
         if args['check_mode']:
             retvals['changed'] = True
         else:

From 8feeb096a536e716241e932aba7b1b0cbca3309d Mon Sep 17 00:00:00 2001
From: Simon Weald <glitchcrab-github@simonweald.com>
Date: Fri, 17 May 2019 15:19:59 +0100
Subject: [PATCH 25/25] ensure types are quoted in the argument spec

---
 lib/ansible/modules/cloud/memset/memset_lb_service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ansible/modules/cloud/memset/memset_lb_service.py b/lib/ansible/modules/cloud/memset/memset_lb_service.py
index c3eca6f2022978..2fc5e9f74a8fc4 100644
--- a/lib/ansible/modules/cloud/memset/memset_lb_service.py
+++ b/lib/ansible/modules/cloud/memset/memset_lb_service.py
@@ -307,7 +307,7 @@ def main():
             api_key=dict(required=True, type='str', no_log=True),
             enabled=dict(default="True", type='bool'),
             load_balancer=dict(required=True, type='str'),
-            port=dict(required=False, type=int),
+            port=dict(required=False, type='int'),
             protocol=dict(required=False, choices=['tcp', 'http', 'https'], type='str'),
             service_name=dict(required=True, type='str', aliases=['name']),
             virtual_ip=dict(required=False, type='str')