diff --git a/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py b/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py index a31c94a855..afc22151a3 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.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) from __future__ import absolute_import, division, print_function @@ -248,10 +248,12 @@ class NetAppOntapIfGrp(object): exception=traceback.format_exc()) def modify_ports(self, current_ports): - for port in current_ports: - self.remove_port_to_if_grp(port) - for port in self.parameters['ports']: + add_ports = set(self.parameters['ports']) - set(current_ports) + remove_ports = set(current_ports) - set(self.parameters['ports']) + for port in add_ports: self.add_port_to_if_grp(port) + for port in remove_ports: + self.remove_port_to_if_grp(port) def remove_port_to_if_grp(self, port): """ @@ -275,9 +277,9 @@ class NetAppOntapIfGrp(object): def apply(self): self.autosupport_log() - current = self.get_if_grp() + current, modify = self.get_if_grp(), None cd_action = self.na_helper.get_cd_action(current, self.parameters) - if current and self.parameters['state'] == 'present': + if cd_action is None and self.parameters['state'] == 'present': current_ports = self.get_if_grp_ports() modify = self.na_helper.get_modified_attributes(current_ports, self.parameters) if self.na_helper.changed: diff --git a/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py b/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py index 0b678ba3fa..498dc46930 100644 --- a/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py +++ b/test/units/modules/storage/netapp/test_na_ontap_net_ifgrp.py @@ -17,7 +17,7 @@ from ansible.modules.storage.netapp.na_ontap_net_ifgrp \ import NetAppOntapIfGrp as ifgrp_module # module under test if not netapp_utils.has_netapp_lib(): - pytestmark = pytest.skip('skipping as missing required netapp_lib') + pytestmark = pytest.mark.skip('skipping as missing required netapp_lib') def set_module_args(args): @@ -249,13 +249,25 @@ class TestMyModule(unittest.TestCase): @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.remove_port_to_if_grp') @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.add_port_to_if_grp') - def test_modify_ports_calls(self, add_port, remove_port): + def test_modify_ports_calls_remove_existing_ports(self, add_port, remove_port): + ''' Test if already existing ports are not being added again ''' data = self.mock_args() - data['ports'] = ['1', '2', '3'] + data['ports'] = ['1', '2'] + set_module_args(data) + self.get_ifgrp_mock_object('ifgrp').modify_ports(current_ports=['1', '2', '3']) + assert remove_port.call_count == 1 + assert add_port.call_count == 0 + + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.remove_port_to_if_grp') + @patch('ansible.modules.storage.netapp.na_ontap_net_ifgrp.NetAppOntapIfGrp.add_port_to_if_grp') + def test_modify_ports_calls_add_new_ports(self, add_port, remove_port): + ''' Test new ports are added ''' + data = self.mock_args() + data['ports'] = ['1', '2', '3', '4'] set_module_args(data) self.get_ifgrp_mock_object('ifgrp').modify_ports(current_ports=['1', '2']) - assert remove_port.call_count == 2 - assert add_port.call_count == 3 + assert remove_port.call_count == 0 + assert add_port.call_count == 2 def test_get_ports_returns_none(self): set_module_args(self.mock_args())