diff --git a/lib/ansible/modules/storage/netapp/na_ontap_net_routes.py b/lib/ansible/modules/storage/netapp/na_ontap_net_routes.py index f31f42aa08..afeeb89b9d 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_net_routes.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_net_routes.py @@ -1,6 +1,6 @@ #!/usr/bin/python -# (c) 2018, NetApp, Inc +# (c) 2018-2019, NetApp, Inc # GNU General Public License v3.0+ # (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) @@ -44,28 +44,32 @@ options: description: - Specify the route metric. - If this field is not provided the default will be set to 20. - new_destination: + from_destination: description: - - Specify the new route destination. - new_gateway: + - Specify the route destination that should be changed. + - new_destination was removed to fix idempotency issues. To rename destination the original goes to from_destination and the new goes to destination. + version_added: '2.8' + from_gateway: description: - - Specify the new route gateway. - new_metric: + - Specify the route gateway that should be changed. + version_added: '2.8' + from_metric: description: - - Specify the new route metric. + - Specify the route metric that should be changed. + version_added: '2.8' ''' EXAMPLES = """ - name: create route na_ontap_net_routes: - state=present - vserver={{ Vserver name }} - username={{ netapp_username }} - password={{ netapp_password }} - hostname={{ netapp_hostname }} - destination=10.7.125.5/20 - gateway=10.7.125.1 - metric=30 + state: present + vserver: "{{ Vserver name }}" + username: "{{ netapp_username }}" + password: "{{ netapp_password }}" + hostname: "{{ netapp_hostname }}" + destination: 10.7.125.5/20 + gateway: 10.7.125.1 + metric: 30 """ RETURN = """ @@ -99,9 +103,9 @@ class NetAppOntapNetRoutes(object): destination=dict(required=True, type='str'), gateway=dict(required=True, type='str'), metric=dict(required=False, type='str'), - new_destination=dict(required=False, type='str', default=None), - new_gateway=dict(required=False, type='str', default=None), - new_metric=dict(required=False, type='str', default=None), + from_destination=dict(required=False, type='str', default=None), + from_gateway=dict(required=False, type='str', default=None), + from_metric=dict(required=False, type='str', default=None), )) self.module = AnsibleModule( @@ -137,13 +141,15 @@ class NetAppOntapNetRoutes(object): % (to_native(error)), exception=traceback.format_exc()) - def delete_net_route(self): + def delete_net_route(self, params=None): """ Deletes a given Route """ route_obj = netapp_utils.zapi.NaElement('net-routes-destroy') - route_obj.add_new_child("destination", self.parameters['destination']) - route_obj.add_new_child("gateway", self.parameters['gateway']) + if params is None: + params = self.parameters + route_obj.add_new_child("destination", params['destination']) + route_obj.add_new_child("gateway", params['gateway']) try: self.server.invoke_successfully(route_obj, True) except netapp_utils.zapi.NaApiError as error: @@ -157,11 +163,13 @@ class NetAppOntapNetRoutes(object): """ # return if there is nothing to change for key, val in desired.items(): - if val == current[key]: - self.na_helper.changed = False - return + if val != current[key]: + self.na_helper.changed = True + break + if not self.na_helper.changed: + return # delete and re-create with new params - self.delete_net_route() + self.delete_net_route(current) route_obj = netapp_utils.zapi.NaElement('net-routes-create') for attribute in ['metric', 'destination', 'gateway']: if desired.get(attribute) is not None: @@ -191,7 +199,7 @@ class NetAppOntapNetRoutes(object): # we need at least on of the new_destination or new_gateway to fetch desired route if params.get('destination') is None and params.get('gateway') is None: return None - current = dict() + current = None route_obj = netapp_utils.zapi.NaElement('net-routes-get') for attr in ['destination', 'gateway']: if params and params.get(attr) is not None: @@ -201,10 +209,14 @@ class NetAppOntapNetRoutes(object): route_obj.add_new_child(attr, value) try: result = self.server.invoke_successfully(route_obj, True) - route_info = result.get_child_by_name('attributes').get_child_by_name('net-vs-routes-info') - current['destination'] = route_info.get_child_content('destination') - current['gateway'] = route_info.get_child_content('gateway') - current['metric'] = route_info.get_child_content('metric') + if result.get_child_by_name('attributes') is not None: + route_info = result.get_child_by_name('attributes').get_child_by_name('net-vs-routes-info') + current = { + 'destination': route_info.get_child_content('destination'), + 'gateway': route_info.get_child_content('gateway'), + 'metric': route_info.get_child_content('metric') + } + except netapp_utils.zapi.NaApiError as error: # Error 13040 denotes a route doesn't exist. if to_native(error.code) == "15661": @@ -260,21 +272,26 @@ class NetAppOntapNetRoutes(object): """ Run Module based on play book """ - # changed = False netapp_utils.ems_log_event("na_ontap_net_routes", self.server) - # route_exists = False current = self.get_net_route() modify, cd_action = None, None - modify_params = {'destination': self.parameters.get('new_destination'), - 'gateway': self.parameters.get('new_gateway'), - 'metric': self.parameters.get('new_metric')} - # if any new_* param is present in playbook, check for modify action + modify_params = {'destination': self.parameters.get('from_destination'), + 'gateway': self.parameters.get('from_gateway'), + 'metric': self.parameters.get('from_metric')} + # if any from_* param is present in playbook, check for modify action if any(modify_params.values()): - # get parameters that are eligible for modify - d = self.get_net_route(modify_params) - modify = self.na_helper.is_rename_action(current, d) + # destination and gateway combination is unique, and is considered like a id. so modify destination + # or gateway is considered a rename action. metric is considered an attribute of the route so it is + # considered as modify. + if modify_params.get('metric') is not None: + modify = True + old_params = current + else: + # get parameters that are eligible for modify + old_params = self.get_net_route(modify_params) + modify = self.na_helper.is_rename_action(old_params, current) if modify is None: - self.module.fail_json(msg="Error modifying: route %s does not exist" % self.parameters['destination']) + self.module.fail_json(msg="Error modifying: route %s does not exist" % self.parameters['from_destination']) else: cd_action = self.na_helper.get_cd_action(current, self.parameters) @@ -283,8 +300,13 @@ class NetAppOntapNetRoutes(object): elif cd_action == 'delete': self.delete_net_route() elif modify: - self.modify_net_route(current, modify_params) - + desired = {} + for key, value in old_params.items(): + desired[key] = value + for key, value in modify_params.items(): + if value is not None: + desired[key] = self.parameters.get(key) + self.modify_net_route(old_params, desired) self.module.exit_json(changed=self.na_helper.changed) diff --git a/test/units/modules/storage/netapp/test_na_ontap_net_routes.py b/test/units/modules/storage/netapp/test_na_ontap_net_routes.py new file mode 100644 index 0000000000..6e383098f9 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_net_routes.py @@ -0,0 +1,309 @@ +# (c) 2018, NetApp, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +''' unit test template for ONTAP Ansible module ''' + +from __future__ import print_function +import json +import pytest + +from units.compat import unittest +from units.compat.mock import patch, Mock +from ansible.module_utils import basic +from ansible.module_utils._text import to_bytes +import ansible.module_utils.netapp as netapp_utils + +from ansible.modules.storage.netapp.na_ontap_net_routes \ + import NetAppOntapNetRoutes as net_route_module # module under test + +if not netapp_utils.has_netapp_lib(): + pytestmark = pytest.mark.skip('skipping as missing required netapp_lib') + + +def set_module_args(args): + """prepare arguments so that they will be picked up during module creation""" + args = json.dumps({'ANSIBLE_MODULE_ARGS': args}) + basic._ANSIBLE_ARGS = to_bytes(args) # pylint: disable=protected-access + + +class AnsibleExitJson(Exception): + """Exception class to be raised by module.exit_json and caught by the test case""" + pass + + +class AnsibleFailJson(Exception): + """Exception class to be raised by module.fail_json and caught by the test case""" + pass + + +def exit_json(*args, **kwargs): # pylint: disable=unused-argument + """function to patch over exit_json; package return data into an exception""" + if 'changed' not in kwargs: + kwargs['changed'] = False + raise AnsibleExitJson(kwargs) + + +def fail_json(*args, **kwargs): # pylint: disable=unused-argument + """function to patch over fail_json; package return data into an exception""" + kwargs['failed'] = True + raise AnsibleFailJson(kwargs) + + +class MockONTAPConnection(object): + ''' mock server connection to ONTAP host ''' + + def __init__(self, kind=None, data=None): + ''' save arguments ''' + self.kind = kind + self.params = data + self.xml_in = None + self.xml_out = None + + def invoke_successfully(self, xml, enable_tunneling): # pylint: disable=unused-argument + ''' mock invoke_successfully returning xml data ''' + self.xml_in = xml + if self.kind == 'net_route': + xml = self.build_net_route_info(self.params) + self.xml_out = xml + return xml + + @staticmethod + def build_net_route_info(net_route_details): + ''' build xml data for net_route-attributes ''' + xml = netapp_utils.zapi.NaElement('xml') + attributes = { + 'attributes': { + 'net-vs-routes-info': { + 'address-family': 'ipv4', + 'destination': net_route_details['destination'], + 'gateway': net_route_details['gateway'], + 'metric': net_route_details['metric'], + 'vserver': net_route_details['vserver'] + } + } + } + xml.translate_struct(attributes) + return xml + + +class TestMyModule(unittest.TestCase): + ''' a group of related Unit Tests ''' + + def setUp(self): + self.mock_module_helper = patch.multiple(basic.AnsibleModule, + exit_json=exit_json, + fail_json=fail_json) + self.mock_module_helper.start() + self.addCleanup(self.mock_module_helper.stop) + self.server = MockONTAPConnection() + self.mock_net_route = { + 'destination': '176.0.0.0/24', + 'gateway': '10.193.72.1', + 'vserver': 'test_vserver', + 'metric': '70' + } + + def mock_args(self): + return { + 'vserver': self.mock_net_route['vserver'], + 'destination': self.mock_net_route['destination'], + 'gateway': self.mock_net_route['gateway'], + 'metric': self.mock_net_route['metric'], + 'hostname': 'test', + 'username': 'test_user', + 'password': 'test_pass!' + } + + def get_net_route_mock_object(self, kind=None, data=None): + """ + Helper method to return an na_ontap_net_route object + :param kind: passes this param to MockONTAPConnection() + :param data: passes this data to MockONTAPConnection() + :return: na_ontap_net_route object + """ + net_route_obj = net_route_module() + net_route_obj.ems_log_event = Mock(return_value=None) + net_route_obj.cluster = Mock() + net_route_obj.cluster.invoke_successfully = Mock() + if kind is None: + net_route_obj.server = MockONTAPConnection() + else: + if data is None: + net_route_obj.server = MockONTAPConnection(kind='net_route', data=self.mock_net_route) + else: + net_route_obj.server = MockONTAPConnection(kind='net_route', data=data) + return net_route_obj + + def test_module_fail_when_required_args_missing(self): + ''' required arguments are reported as errors ''' + with pytest.raises(AnsibleFailJson) as exc: + set_module_args({}) + net_route_module() + print('Info: %s' % exc.value.args[0]['msg']) + + def test_get_nonexistent_net_route(self): + ''' Test if get_net_route returns None for non-existent net_route ''' + set_module_args(self.mock_args()) + result = self.get_net_route_mock_object().get_net_route() + assert result is None + + def test_get_existing_job(self): + ''' Test if get_net_route returns details for existing net_route ''' + set_module_args(self.mock_args()) + result = self.get_net_route_mock_object('net_route').get_net_route() + assert result['destination'] == self.mock_net_route['destination'] + assert result['gateway'] == self.mock_net_route['gateway'] + + def test_create_error_missing_param(self): + ''' Test if create throws an error if destination is not specified''' + data = self.mock_args() + del data['destination'] + set_module_args(data) + with pytest.raises(AnsibleFailJson) as exc: + self.get_net_route_mock_object('net_route').create_net_route() + msg = 'missing required arguments: destination' + assert exc.value.args[0]['msg'] == msg + + @patch('ansible.modules.storage.netapp.na_ontap_net_routes.NetAppOntapNetRoutes.create_net_route') + def test_successful_create(self, create_net_route): + ''' Test successful create ''' + data = self.mock_args() + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object().apply() + assert exc.value.args[0]['changed'] + create_net_route.assert_called_with() + + def test_create_idempotency(self): + ''' Test create idempotency ''' + set_module_args(self.mock_args()) + obj = self.get_net_route_mock_object('net_route') + with pytest.raises(AnsibleExitJson) as exc: + obj.apply() + assert not exc.value.args[0]['changed'] + + def test_successful_delete(self): + ''' Test successful delete ''' + data = self.mock_args() + data['state'] = 'absent' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object('net_route').apply() + assert exc.value.args[0]['changed'] + + def test_delete_idempotency(self): + ''' Test delete idempotency ''' + data = self.mock_args() + data['state'] = 'absent' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object().apply() + assert not exc.value.args[0]['changed'] + + def test_successful_modify_metric(self): + ''' Test successful modify metric ''' + data = self.mock_args() + del data['metric'] + data['from_metric'] = '70' + data['metric'] = '40' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object('net_route').apply() + assert exc.value.args[0]['changed'] + + def test_modify_metric_idempotency(self): + ''' Test modify metric idempotency''' + data = self.mock_args() + data['metric'] = '70' + set_module_args(data) + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object('net_route').apply() + assert not exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_net_routes.NetAppOntapNetRoutes.get_net_route') + def test_successful_modify_gateway(self, get_net_route): + ''' Test successful modify gateway ''' + data = self.mock_args() + del data['gateway'] + data['from_gateway'] = '10.193.72.1' + data['gateway'] = '10.193.0.1' + set_module_args(data) + current = { + 'destination': '176.0.0.0/24', + 'gateway': '10.193.72.1', + 'metric': '70', + 'vserver': 'test_server' + } + get_net_route.side_effect = [ + None, + current + ] + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object().apply() + assert exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_net_routes.NetAppOntapNetRoutes.get_net_route') + def test__modify_gateway_idempotency(self, get_net_route): + ''' Test modify gateway idempotency ''' + data = self.mock_args() + del data['gateway'] + data['from_gateway'] = '10.193.72.1' + data['gateway'] = '10.193.0.1' + set_module_args(data) + current = { + 'destination': '178.0.0.1/24', + 'gateway': '10.193.72.1', + 'metric': '70', + 'vserver': 'test_server' + } + get_net_route.side_effect = [ + current, + None + ] + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object('net_route', current).apply() + assert not exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_net_routes.NetAppOntapNetRoutes.get_net_route') + def test_successful_modify_destination(self, get_net_route): + ''' Test successful modify destination ''' + data = self.mock_args() + del data['destination'] + data['from_destination'] = '176.0.0.0/24' + data['destination'] = '178.0.0.1/24' + set_module_args(data) + current = { + 'destination': '176.0.0.0/24', + 'gateway': '10.193.72.1', + 'metric': '70', + 'vserver': 'test_server' + } + get_net_route.side_effect = [ + None, + current + ] + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object().apply() + assert exc.value.args[0]['changed'] + + @patch('ansible.modules.storage.netapp.na_ontap_net_routes.NetAppOntapNetRoutes.get_net_route') + def test__modify_destination_idempotency(self, get_net_route): + ''' Test modify destination idempotency''' + data = self.mock_args() + del data['destination'] + data['from_destination'] = '176.0.0.0/24' + data['destination'] = '178.0.0.1/24' + set_module_args(data) + current = { + 'destination': '178.0.0.1/24', + 'gateway': '10.193.72.1', + 'metric': '70', + 'vserver': 'test_server' + } + get_net_route.side_effect = [ + current, + None + ] + with pytest.raises(AnsibleExitJson) as exc: + self.get_net_route_mock_object('net_route', current).apply() + assert not exc.value.args[0]['changed']