This patch fixes a number of outstanding bugs and code convention problems. (#31618)

* documentation was not inline with other Ansible modules
* Python 3 specific imports were missing
* monitor_type is no longer required when creating a new pool; it is now the default.
* A new monitor_type choice of "single" was added for a more intuitive way to specify "a single monitor". It uses "and_list" underneath, but provides additional checks to ensure that you are specifying only a single monitor.
* host and port arguments have been deprecated for now. Please use bigip_pool_member instead.
* 'partition' field was missing from documentation.
* A note that "python 2.7 or greater is required" has been added for those who were not aware that this applies for ALL F5 modules.
* Unit tests were fixed to support the above module
This commit is contained in:
Tim Rupp 2017-10-13 09:47:49 -07:00 committed by GitHub
parent 89428a40b3
commit ecee475a3a
2 changed files with 387 additions and 197 deletions

View file

@ -4,26 +4,21 @@
# Copyright (c) 2017 F5 Networks Inc.
# 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 = '''
DOCUMENTATION = r'''
---
module: bigip_pool
short_description: Manages F5 BIG-IP LTM pools.
short_description: Manages F5 BIG-IP LTM pools
description:
- Manages F5 BIG-IP LTM pools via iControl REST API.
version_added: 1.2
author:
- Tim Rupp (@caphrim007)
- Wojciech Wypior (@wojtek0806)
notes:
- Requires BIG-IP software version >= 11.
- F5 developed module 'F5-SDK' required (https://github.com/F5Networks/f5-common-python).
- Best run as a local_action in your playbook.
requirements:
- f5-sdk
options:
description:
description:
@ -62,9 +57,15 @@ options:
- weighted-least-connections-nod
monitor_type:
description:
- Monitor rule type when C(monitors) > 1.
- Monitor rule type when C(monitors) is specified. When creating a new
pool, if this value is not specified, the default of 'and_list' will
be used.
- Both C(single) and C(and_list) are functionally identical since BIG-IP
considers all monitors as "a list". BIG=IP either has a list of many,
or it has a list of one. Where they differ is in the extra guards that
C(single) provides; namely that it only allows a single monitor.
version_added: "1.3"
choices: ['and_list', 'm_of_n']
choices: ['and_list', 'm_of_n', 'single']
quorum:
description:
- Monitor quorum value when C(monitor_type) is C(m_of_n).
@ -96,136 +97,212 @@ options:
host:
description:
- Pool member IP.
- Deprecated in 2.4. Use the C(bigip_pool_member) module instead.
aliases:
- address
port:
description:
- Pool member port.
- Deprecated in 2.4. Use the C(bigip_pool_member) module instead.
partition:
description:
- Device partition to manage resources on.
default: Common
version_added: 2.5
notes:
- Requires BIG-IP software version >= 11.
- F5 developed module 'F5-SDK' required (https://github.com/F5Networks/f5-common-python).
- Best run as a local_action in your playbook.
requirements:
- f5-sdk
- Python >= 2.7
extends_documentation_fragment: f5
author:
- Tim Rupp (@caphrim007)
- Wojciech Wypior (@wojtek0806)
'''
EXAMPLES = '''
EXAMPLES = r'''
- name: Create pool
bigip_pool:
server: "lb.mydomain.com"
user: "admin"
password: "secret"
state: "present"
name: "my-pool"
partition: "Common"
lb_method: "least_connection_member"
slow_ramp_time: 120
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
lb_method: least_connection_member
slow_ramp_time: 120
delegate_to: localhost
- name: Modify load balancer method
bigip_pool:
server: "lb.mydomain.com"
user: "admin"
password: "secret"
state: "present"
name: "my-pool"
partition: "Common"
lb_method: "round_robin"
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
lb_method: round_robin
delegate_to: localhost
- name: Add pool member
bigip_pool:
server: "lb.mydomain.com"
user: "admin"
password: "secret"
state: "present"
name: "my-pool"
partition: "Common"
host: "{{ ansible_default_ipv4['address'] }}"
port: 80
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
host: "{{ ansible_default_ipv4['address'] }}"
port: 80
delegate_to: localhost
- name: Set a single monitor (with enforcement)
bigip_pool:
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
monitor_type: single
monitors:
- http
delegate_to: localhost
- name: Set a single monitor (without enforcement)
bigip_pool:
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
monitors:
- http
delegate_to: localhost
- name: Set multiple monitors (all must succeed)
bigip_pool:
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
monitor_type: and_list
monitors:
- http
- tcp
delegate_to: localhost
- name: Set multiple monitors (at least 1 must succeed)
bigip_pool:
server: lb.mydomain.com
user: admin
password: secret
state: present
name: my-pool
partition: Common
monitor_type: m_of_n
quorum: 1
monitors:
- http
- tcp
delegate_to: localhost
- name: Remove pool member from pool
bigip_pool:
server: "lb.mydomain.com"
user: "admin"
password: "secret"
state: "absent"
name: "my-pool"
partition: "Common"
host: "{{ ansible_default_ipv4['address'] }}"
port: 80
server: lb.mydomain.com
user: admin
password: secret
state: absent
name: my-pool
partition: Common
host: "{{ ansible_default_ipv4['address'] }}"
port: 80
delegate_to: localhost
- name: Delete pool
bigip_pool:
server: "lb.mydomain.com"
user: "admin"
password: "secret"
state: "absent"
name: "my-pool"
partition: "Common"
server: lb.mydomain.com
user: admin
password: secret
state: absent
name: my-pool
partition: Common
delegate_to: localhost
'''
RETURN = '''
RETURN = r'''
monitor_type:
description: The contact that was set on the datacenter.
returned: changed
type: string
sample: "admin@root.local"
description: The contact that was set on the datacenter.
returned: changed
type: string
sample: admin@root.local
quorum:
description: The quorum that was set on the pool
returned: changed
type: int
sample: 2
description: The quorum that was set on the pool.
returned: changed
type: int
sample: 2
monitors:
description: Monitors set on the pool.
returned: changed
type: list
sample: ['/Common/http', '/Common/gateway_icmp']
description: Monitors set on the pool.
returned: changed
type: list
sample: ['/Common/http', '/Common/gateway_icmp']
service_down_action:
description: Service down action that is set on the pool.
returned: changed
type: string
sample: "reset"
description: Service down action that is set on the pool.
returned: changed
type: string
sample: reset
description:
description: Description set on the pool.
returned: changed
type: string
sample: "Pool of web servers"
description: Description set on the pool.
returned: changed
type: string
sample: Pool of web servers
lb_method:
description: The LB method set for the pool.
returned: changed
type: string
sample: "round-robin"
description: The LB method set for the pool.
returned: changed
type: string
sample: round-robin
host:
description: IP of pool member included in pool.
returned: changed
type: string
sample: "10.10.10.10"
description: IP of pool member included in pool.
returned: changed
type: string
sample: 10.10.10.10
port:
description: Port of pool member included in pool.
returned: changed
type: int
sample: 80
description: Port of pool member included in pool.
returned: changed
type: int
sample: 80
slow_ramp_time:
description: The new value that is set for the slow ramp-up time.
returned: changed
type: int
sample: 500
description: The new value that is set for the slow ramp-up time.
returned: changed
type: int
sample: 500
reselect_tries:
description: The new value that is set for the number of tries to contact member
returned: changed
type: int
sample: 10
description: The new value that is set for the number of tries to contact member.
returned: changed
type: int
sample: 10
'''
import re
import os
from ansible.module_utils.f5_utils import AnsibleF5Client
from ansible.module_utils.f5_utils import AnsibleF5Parameters
from ansible.module_utils.f5_utils import HAS_F5SDK
from ansible.module_utils.f5_utils import F5ModuleError
from ansible.module_utils.six import iteritems
from collections import defaultdict
from netaddr import IPAddress, AddrFormatError
from ansible.module_utils.f5_utils import (
AnsibleF5Client,
AnsibleF5Parameters,
HAS_F5SDK,
F5ModuleError,
iControlUnexpectedHTTPError
)
try:
from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
except ImportError:
HAS_F5SDK = False
class Parameters(AnsibleF5Parameters):
@ -233,13 +310,13 @@ class Parameters(AnsibleF5Parameters):
'loadBalancingMode': 'lb_method',
'slowRampTime': 'slow_ramp_time',
'reselectTries': 'reselect_tries',
'serviceDownAction': 'service_down_action'
'serviceDownAction': 'service_down_action',
'monitor': 'monitors'
}
updatables = [
'monitor_type', 'quorum', 'monitors', 'service_down_action',
'description', 'lb_method', 'slow_ramp_time', 'reselect_tries',
'host', 'port'
api_attributes = [
'description', 'name', 'loadBalancingMode', 'monitor', 'slowRampTime',
'reselectTries', 'serviceDownAction'
]
returnables = [
@ -248,15 +325,41 @@ class Parameters(AnsibleF5Parameters):
'reselect_tries', 'monitor', 'member_name', 'name', 'partition'
]
api_attributes = [
'description', 'name', 'loadBalancingMode', 'monitor', 'slowRampTime',
'reselectTries', 'serviceDownAction'
updatables = [
'monitor_type', 'quorum', 'monitors', 'service_down_action',
'description', 'lb_method', 'slow_ramp_time', 'reselect_tries',
'host', 'port'
]
def __init__(self, params=None):
super(Parameters, self).__init__(params)
self._values = defaultdict(lambda: None)
if params:
self.update(params=params)
self._values['__warnings'] = []
def update(self, params=None):
if params:
for k, v in iteritems(params):
if self.api_map is not None and k in self.api_map:
map_key = self.api_map[k]
else:
map_key = k
# Handle weird API parameters like `dns.proxy.__iter__` by
# using a map provided by the module developer
class_attr = getattr(type(self), map_key, None)
if isinstance(class_attr, property):
# There is a mapped value for the api_map key
if class_attr.fset is None:
# If the mapped value does not have an associated setter
self._values[map_key] = v
else:
# The mapped value has a setter
setattr(self, map_key, v)
else:
# If the mapped value is not a @property
self._values[map_key] = v
@property
def lb_method(self):
lb_map = {
@ -295,71 +398,74 @@ class Parameters(AnsibleF5Parameters):
raise F5ModuleError('Provided lb_method is unknown')
return lb_method
def _fqdn_name(self, value):
if value.startswith('/'):
name = os.path.basename(value)
result = '/{0}/{1}'.format(self.partition, name)
else:
result = '/{0}/{1}'.format(self.partition, value)
return result
@property
def monitors_list(self):
if self._values['monitors'] is None:
return []
try:
result = re.findall(r'/\w+/[^\s}]+', self._values['monitors'])
return result
except Exception:
return self._values['monitors']
@property
def monitors(self):
monitors = list()
monitor_list = self._values['monitors']
monitor_type = self._values['monitor_type']
error1 = "The 'monitor_type' parameter cannot be empty when " \
"'monitors' parameter is specified."
error2 = "The 'monitor' parameter cannot be empty when " \
"'monitor_type' parameter is specified"
if monitor_list is not None and monitor_type is None:
raise F5ModuleError(error1)
elif monitor_list is None and monitor_type is not None:
raise F5ModuleError(error2)
elif monitor_list is None:
if self._values['monitors'] is None:
return None
monitors = [self._fqdn_name(x) for x in self.monitors_list]
if self.monitor_type == 'm_of_n':
monitors = ' '.join(monitors)
result = 'min %s of { %s }' % (self.quorum, monitors)
else:
result = ' and '.join(monitors).strip()
for m in monitor_list:
if re.match(r'\/\w+\/\w+', m):
m = '/{0}/{1}'.format(self.partition, os.path.basename(m))
elif re.match(r'\w+', m):
m = '/{0}/{1}'.format(self.partition, m)
else:
raise F5ModuleError(
"Unknown monitor format '{0}'".format(m)
)
monitors.append(m)
return monitors
return result
@property
def quorum(self):
value = self._values['quorum']
error = "Quorum value must be specified with monitor_type 'm_of_n'."
if self._values['monitor_type'] == 'm_of_n' and value is None:
raise F5ModuleError(error)
return value
if self.kind == 'tm:ltm:pool:poolstate':
if self._values['monitors'] is None:
return None
pattern = r'min\s+(?P<quorum>\d+)\s+of'
matches = re.search(pattern, self._values['monitors'])
if matches:
quorum = matches.group('quorum')
else:
quorum = None
else:
quorum = self._values['quorum']
try:
if quorum is None:
return None
return int(quorum)
except ValueError:
raise F5ModuleError(
"The specified 'quorum' must be an integer."
)
@property
def monitor(self):
monitors = self.monitors
monitor_type = self._values['monitor_type']
quorum = self.quorum
if monitors is None:
return None
if monitor_type == 'and_list':
and_list = list()
for m in monitors:
if monitors.index(m) == 0:
and_list.append(m)
else:
and_list.append('and')
and_list.append(m)
result = ' '.join(and_list)
def monitor_type(self):
if self.kind == 'tm:ltm:pool:poolstate':
if self._values['monitors'] is None:
return None
pattern = r'min\s+\d+\s+of'
matches = re.search(pattern, self._values['monitors'])
if matches:
return 'm_of_n'
else:
return 'and_list'
else:
min_list = list()
prefix = 'min {0} of {{'.format(str(quorum))
min_list.append(prefix)
for m in monitors:
min_list.append(m)
min_list.append('}')
result = ' '.join(min_list)
return result
if self._values['monitor_type'] is None:
return None
return self._values['monitor_type']
@property
def host(self):
@ -418,12 +524,83 @@ class Parameters(AnsibleF5Parameters):
return result
class Changes(Parameters):
pass
class Difference(object):
def __init__(self, want, have=None):
self.want = want
self.have = have
def compare(self, param):
try:
result = getattr(self, param)
return result
except AttributeError:
return self.__default(param)
def __default(self, param):
attr1 = getattr(self.want, param)
try:
attr2 = getattr(self.have, param)
if attr1 != attr2:
return attr1
except AttributeError:
return attr1
@property
def monitor_type(self):
if self.want.monitor_type is None:
self.want.update(dict(monitor_type=self.have.monitor_type))
if self.want.quorum is None:
self.want.update(dict(quorum=self.have.quorum))
if self.want.monitor_type == 'm_of_n' and self.want.quorum is None:
raise F5ModuleError(
"Quorum value must be specified with monitor_type 'm_of_n'."
)
elif self.want.monitor_type == 'single':
if len(self.want.monitors_list) > 1:
raise F5ModuleError(
"When using a 'monitor_type' of 'single', only one monitor may be provided."
)
elif len(self.have.monitors_list) > 1 and len(self.want.monitors_list) == 0:
# Handle instances where there already exists many monitors, and the
# user runs the module again specifying that the monitor_type should be
# changed to 'single'
raise F5ModuleError(
"A single monitor must be specified if more than one monitor currently exists on your pool."
)
# Update to 'and_list' here because the above checks are all that need
# to be done before we change the value back to what is expected by
# BIG-IP.
#
# Remember that 'single' is nothing more than a fancy way of saying
# "and_list plus some extra checks"
self.want.update(dict(monitor_type='and_list'))
if self.want.monitor_type != self.have.monitor_type:
return self.want.monitor_type
@property
def monitors(self):
if self.want.monitor_type is None:
self.want.update(dict(monitor_type=self.have.monitor_type))
if not self.want.monitors_list:
self.want.monitors = self.have.monitors_list
if not self.want.monitors and self.want.monitor_type is not None:
raise F5ModuleError(
"The 'monitors' parameter cannot be empty when 'monitor_type' parameter is specified"
)
if self.want.monitors != self.have.monitors:
return self.want.monitors
class ModuleManager(object):
def __init__(self, client):
self.client = client
self.have = None
self.want = Parameters(self.client.module.params)
self.changes = Parameters()
self.changes = Changes()
def exec_module(self):
changed = False
@ -465,13 +642,15 @@ class ModuleManager(object):
self.changes = Parameters(changed)
def _update_changed_options(self):
changed = {}
for key in Parameters.updatables:
if getattr(self.want, key) is not None:
attr1 = getattr(self.want, key)
attr2 = getattr(self.have, key)
if attr1 != attr2:
changed[key] = attr1
diff = Difference(self.want, self.have)
updatables = Parameters.updatables
changed = dict()
for k in updatables:
change = diff.compare(k)
if change is None:
continue
else:
changed[k] = change
if changed:
self.changes = Parameters(changed)
return True
@ -528,6 +707,24 @@ class ModuleManager(object):
return True
def create(self):
if self.want.monitor_type is not None:
if not self.want.monitors_list:
raise F5ModuleError(
"The 'monitors' parameter cannot be empty when 'monitor_type' parameter is specified"
)
else:
if self.want.monitor_type is None:
self.want.update(dict(monitor_type='and_list'))
if self.want.monitor_type == 'm_of_n' and self.want.quorum is None:
raise F5ModuleError(
"Quorum value must be specified with monitor_type 'm_of_n'."
)
elif self.want.monitor_type == 'single' and len(self.want.monitors_list) > 1:
raise F5ModuleError(
"When using a 'monitor_type' of 'single', only one monitor may be provided"
)
self._set_changed_options()
if self.client.check_mode:
return True
@ -665,7 +862,7 @@ class ArgumentSpec(object):
),
monitor_type=dict(
choices=[
'and_list', 'm_of_n'
'and_list', 'm_of_n', 'single'
]
),
quorum=dict(

View file

@ -40,11 +40,13 @@ try:
from library.bigip_pool import Parameters
from library.bigip_pool import ModuleManager
from library.bigip_pool import ArgumentSpec
from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
except ImportError:
try:
from ansible.modules.network.f5.bigip_pool import Parameters
from ansible.modules.network.f5.bigip_pool import ModuleManager
from ansible.modules.network.f5.bigip_pool import ArgumentSpec
from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
except ImportError:
raise SkipTest("F5 Ansible modules require the f5-sdk Python library")
@ -82,10 +84,9 @@ class BigIpObj(object):
class TestParameters(unittest.TestCase):
def test_module_parameters(self):
m = ['/Common/Fake', '/Common/Fake2']
args = dict(
monitor_type='m_of_n',
monitors=m,
monitors=['/Common/Fake', '/Common/Fake2'],
quorum=1,
slow_ramp_time=200,
reselect_tries=5,
@ -97,8 +98,7 @@ class TestParameters(unittest.TestCase):
p = Parameters(args)
assert p.monitor_type == 'm_of_n'
assert p.quorum == 1
assert p.monitors == m
assert p.monitor == 'min 1 of { /Common/Fake /Common/Fake2 }'
assert p.monitors == 'min 1 of { /Common/Fake /Common/Fake2 }'
assert p.host == '192.168.1.1'
assert p.port == 8080
assert p.member_name == '192.168.1.1:8080'
@ -117,7 +117,7 @@ class TestParameters(unittest.TestCase):
)
p = Parameters(args)
assert p.monitor == '/Common/Fake and /Common/Fake2'
assert p.monitors == '/Common/Fake and /Common/Fake2'
assert p.slow_ramp_time == 200
assert p.reselect_tries == 5
assert p.service_down_action == 'drop'
@ -297,12 +297,12 @@ class TestManager(unittest.TestCase):
mm.create_on_device = Mock(return_value=True)
mm.exists = Mock(return_value=False)
msg = "The 'monitor_type' parameter cannot be empty when " \
"'monitors' parameter is specified."
with pytest.raises(F5ModuleError) as err:
mm.exec_module()
results = mm.exec_module()
assert str(err.value) == msg
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == '/Common/tcp and /Common/http'
assert results['monitor_type'] == 'and_list'
def test_create_pool_monitors_missing(self, *args):
set_module_args(dict(
@ -325,7 +325,7 @@ class TestManager(unittest.TestCase):
mm.create_on_device = Mock(return_value=True)
mm.exists = Mock(return_value=False)
msg = "The 'monitor' parameter cannot be empty when " \
msg = "The 'monitors' parameter cannot be empty when " \
"'monitor_type' parameter is specified"
with pytest.raises(F5ModuleError) as err:
mm.exec_module()
@ -385,9 +385,8 @@ class TestManager(unittest.TestCase):
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == ['/Common/tcp', '/Common/http']
assert results['monitors'] == '/Common/tcp and /Common/http'
assert results['monitor_type'] == 'and_list'
assert results['monitor'] == '/Common/tcp and /Common/http'
def test_create_pool_monitor_m_of_n(self, *args):
set_module_args(dict(
@ -415,9 +414,8 @@ class TestManager(unittest.TestCase):
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == ['/Common/tcp', '/Common/http']
assert results['monitors'] == 'min 1 of { /Common/tcp /Common/http }'
assert results['monitor_type'] == 'm_of_n'
assert results['monitor'] == 'min 1 of { /Common/tcp /Common/http }'
def test_update_monitors(self, *args):
set_module_args(dict(
@ -452,9 +450,8 @@ class TestManager(unittest.TestCase):
results = mm.exec_module()
assert results['changed'] is True
assert results['monitors'] == ['/Common/http', '/Common/tcp']
assert results['monitor_type'] == 'and_list'
assert results['monitor'] == '/Common/http and /Common/tcp'
assert results['monitors'] == '/Common/http and /Common/tcp'
def test_update_pool_new_member(self, *args):
set_module_args(dict(
@ -552,9 +549,8 @@ class TestManager(unittest.TestCase):
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == ['/Common/tcp', '/Common/http']
assert results['monitors'] == '/Common/tcp and /Common/http'
assert results['monitor_type'] == 'and_list'
assert results['monitor'] == '/Common/tcp and /Common/http'
def test_create_pool_monitor_m_of_n_no_partition(self, *args):
set_module_args(dict(
@ -581,9 +577,8 @@ class TestManager(unittest.TestCase):
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == ['/Common/tcp', '/Common/http']
assert results['monitors'] == 'min 1 of { /Common/tcp /Common/http }'
assert results['monitor_type'] == 'm_of_n'
assert results['monitor'] == 'min 1 of { /Common/tcp /Common/http }'
def test_create_pool_monitor_and_list_custom_partition(self, *args):
set_module_args(dict(
@ -610,9 +605,8 @@ class TestManager(unittest.TestCase):
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == ['/Testing/tcp', '/Testing/http']
assert results['monitors'] == '/Testing/tcp and /Testing/http'
assert results['monitor_type'] == 'and_list'
assert results['monitor'] == '/Testing/tcp and /Testing/http'
def test_create_pool_monitor_m_of_n_custom_partition(self, *args):
set_module_args(dict(
@ -640,6 +634,5 @@ class TestManager(unittest.TestCase):
assert results['changed'] is True
assert results['name'] == 'fake_pool'
assert results['monitors'] == ['/Testing/tcp', '/Testing/http']
assert results['monitors'] == 'min 1 of { /Testing/tcp /Testing/http }'
assert results['monitor_type'] == 'm_of_n'
assert results['monitor'] == 'min 1 of { /Testing/tcp /Testing/http }'