diff --git a/.ansible-lint b/.ansible-lint new file mode 100644 index 00000000..2262773e --- /dev/null +++ b/.ansible-lint @@ -0,0 +1,17 @@ +--- +# Red Hat Ansible Collection Certification .ansible-lint file +# Based on: +# https://github.com/ansible-collections/partner-certification-requirements +profile: production + +exclude_paths: + - tests/integration + - tests/unit + - extensions/molecule + - changelogs + - docs + - .ansible + - .github + +warn_list: + - no-log-password diff --git a/README.md b/README.md index 9a18e0b0..c3abc4cb 100644 --- a/README.md +++ b/README.md @@ -18,13 +18,12 @@ The Everpure FlashArray collection consists of the latest versions of the FlashA - some modules require higher versions of Purity - Some modules require specific Purity versions - distro -- py-pure-client >= 1.75.0 +- py-pure-client >= 1.82.0 - python >= 3.9 - netaddr >= 1.2.0 - requests - pycountry -- packaging -- pyz +- pytz - urllib3 ## Installation diff --git a/changelogs/fragments/1027_purefa_host_initiators.yaml b/changelogs/fragments/1027_purefa_host_initiators.yaml new file mode 100644 index 00000000..8685961a --- /dev/null +++ b/changelogs/fragments/1027_purefa_host_initiators.yaml @@ -0,0 +1,2 @@ +bugfixes: + - purefa_host - Reconcile a host's WWNs, IQNs and NQNs by adding only missing initiators and removing only unwanted ones, so one can be dropped by omitting it and those that stay are untouched diff --git a/changelogs/fragments/1027_purefa_info_host_dict.yaml b/changelogs/fragments/1027_purefa_info_host_dict.yaml new file mode 100644 index 00000000..c7ce2208 --- /dev/null +++ b/changelogs/fragments/1027_purefa_info_host_dict.yaml @@ -0,0 +1,4 @@ +minor_changes: + - purefa_info - Add ``logged_in_ports`` to each host in the ``hosts`` subset, mapping every host WWN/IQN/NQN to the array target ports it is currently logged into +bugfixes: + - purefa_info - Fix TypeError gathering the ``hosts`` subset for hosts that have preferred arrays set, as ``preferred_arrays`` is a list of references not a dict diff --git a/galaxy.yml b/galaxy.yml index f32332c1..06f665c6 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -18,3 +18,18 @@ dependencies: {} repository: https://github.com/Everpure-Ansible/FlashArray-Collection documentation: https://github.com/Everpure-Ansible/FlashArray-Collection issues: https://github.com/Everpure-Ansible/FlashArray-Collection/issues +build_ignore: + - tests/integration + - tests/unit + - extensions/molecule + - changelogs + - docs + - collections + - .github + - .ansible + - .tox + - .venv + - .vscode + - .pytest_cache + - .pre-commit-config.yaml + - settings.json diff --git a/plugins/modules/purefa_host.py b/plugins/modules/purefa_host.py index 46511a86..9c87ae59 100644 --- a/plugins/modules/purefa_host.py +++ b/plugins/modules/purefa_host.py @@ -62,16 +62,23 @@ elements: str description: - List of wwns of the host. + - This is the complete desired set of WWNs. The host is reconciled to match + it, adding any that are missing and removing any that are not listed, so a + single WWN can be removed by simply omitting it from the list. iqn: type: list elements: str description: - List of IQNs of the host. + - This is the complete desired set of IQNs; the host is reconciled to match + it, adding and removing IQNs as needed. nqn: type: list elements: str description: - List of NQNs of the host. Note that NMVe hosts can only possess NQNs. + - This is the complete desired set of NQNs; the host is reconciled to match + it, adding and removing NQNs as needed. Multi-protocol is not allowed for these hosts. volume: type: str @@ -445,113 +452,84 @@ def _set_host_initiators(module, array): ) +def _sync_initiators(module, array, add_field, remove_field, desired, current, error): + """Reconcile a single initiator type to the desired set. + + The FlashArray ``patch_hosts`` API exposes three fields per initiator type: + ``wwns``/``iqns``/``nqns`` *replace* the whole associated list, while + ``add_*`` associates extra initiators and ``remove_*`` disassociates + specific ones. We use the ``add_*``/``remove_*`` pair (never the replacing + field) so that only the genuinely changed initiators are touched: an + unwanted WWN/IQN/NQN is dropped by omitting it from the list, and the + initiators that stay are left untouched rather than being removed and + re-added (which would needlessly disrupt their existing sessions). + + Returns True if a change was (or would be) made. + """ + to_add = [item for item in desired if item not in current] + to_remove = [item for item in current if item not in desired] + if not to_add and not to_remove: + return False + if not module.check_mode: + patch = {} + if to_add: + patch[add_field] = to_add + if to_remove: + patch[remove_field] = to_remove + res = get_with_context( + array, + "patch_hosts", + CONTEXT_API_VERSION, + module, + names=[module.params["name"]], + host=HostPatch(**patch), + ) + check_response(res, module, error) + return True + + def _update_host_initiators(module, array, answer=False): - """Change host initiator if iscsi or nvme or add new FC WWNs""" + """Reconcile host initiators (iSCSI, NVMe and FC) to the requested set""" current_connectors = get_host(module, array) if module.params["nqn"]: - if module.params["nqn"] != [""]: - if sorted(current_connectors.nqns) != sorted(module.params["nqn"]): - answer = True - if not module.check_mode: - res = get_with_context( - array, - "patch_hosts", - CONTEXT_API_VERSION, - module, - names=[module.params["name"]], - host=HostPatch(nqns=module.params["nqn"]), - ) - check_response( - res, - module, - f"Change of NVMe NQNs failed on host {module.params['name']}", - ) - elif current_connectors.nqns: + desired = [] if module.params["nqn"] == [""] else module.params["nqn"] + if _sync_initiators( + module, + array, + "add_nqns", + "remove_nqns", + desired, + current_connectors.nqns or [], + f"Change of NVMe NQNs failed on host {module.params['name']}", + ): answer = True - if not module.check_mode: - res = get_with_context( - array, - "patch_hosts", - CONTEXT_API_VERSION, - module, - names=[module.params["name"]], - host=HostPatch(remove_nqns=current_connectors.nqns), - ) - check_response( - res, - module, - f"Removal of NVMe NQNs failed on host {module.params['name']}", - ) if module.params["iqn"]: - if module.params["iqn"] != [""]: - if sorted(current_connectors.iqns) != sorted(module.params["iqn"]): - answer = True - if not module.check_mode: - res = get_with_context( - array, - "patch_hosts", - CONTEXT_API_VERSION, - module, - names=[module.params["name"]], - host=HostPatch(iqns=module.params["iqn"]), - ) - check_response( - res, - module, - f"Change of iSCSI IQNs failed on host {module.params['name']}", - ) - elif current_connectors.iqns: + desired = [] if module.params["iqn"] == [""] else module.params["iqn"] + if _sync_initiators( + module, + array, + "add_iqns", + "remove_iqns", + desired, + current_connectors.iqns or [], + f"Change of iSCSI IQNs failed on host {module.params['name']}", + ): answer = True - if not module.check_mode: - res = get_with_context( - array, - "patch_hosts", - CONTEXT_API_VERSION, - module, - names=[module.params["name"]], - host=HostPatch(remove_iqns=current_connectors.iqns), - ) - check_response( - res, - module, - f"Removal of iSCSI IQNs failed on host {module.params['name']}", - ) if module.params["wwns"]: - module.params["wwns"] = [wwn.replace(":", "") for wwn in module.params["wwns"]] - module.params["wwns"] = [wwn.upper() for wwn in module.params["wwns"]] - if module.params["wwns"] != [""]: - if sorted(current_connectors.wwns) != sorted(module.params["wwns"]): - answer = True - if not module.check_mode: - res = get_with_context( - array, - "patch_hosts", - CONTEXT_API_VERSION, - module, - names=module.params["name"], - host=HostPatch(wwns=module.params["wwns"]), - ) - check_response( - res, - module, - f"Change of FC WWNs failed on host {module.params['name']}", - ) - elif current_connectors.wwns: + module.params["wwns"] = [ + wwn.replace(":", "").upper() for wwn in module.params["wwns"] + ] + desired = [] if module.params["wwns"] == [""] else module.params["wwns"] + if _sync_initiators( + module, + array, + "add_wwns", + "remove_wwns", + desired, + current_connectors.wwns or [], + f"Change of FC WWNs failed on host {module.params['name']}", + ): answer = True - if not module.check_mode: - res = get_with_context( - array, - "patch_hosts", - CONTEXT_API_VERSION, - module, - names=[module.params["name"]], - host=HostPatch(remove_wwns=current_connectors.wwns), - ) - check_response( - res, - module, - f"Removal of FC WWNs failed on host {module.params['name']}", - ) return answer diff --git a/plugins/modules/purefa_info.py b/plugins/modules/purefa_info.py index 25119073..d4039592 100644 --- a/plugins/modules/purefa_info.py +++ b/plugins/modules/purefa_info.py @@ -1890,6 +1890,22 @@ def generate_host_dict(array, performance): hosts_balance = list(array.get_hosts_performance_balance().items) if performance: hosts_performance = list(array.get_hosts_performance().items) + # Build a live login map (initiator identifier -> set of array target port + # names) so each host WWN/IQN/NQN can report exactly where it is logged in. + login_map = {} + for login in list(array.get_ports_initiators().items): + target_name = getattr(login.target, "name", None) + if not target_name: + continue + initiator = login.initiator + init_wwn = getattr(initiator, "wwn", None) + for key in ( + init_wwn.replace(":", "").upper() if init_wwn else None, + getattr(initiator, "iqn", None), + getattr(initiator, "nqn", None), + ): + if key: + login_map.setdefault(key, set()).add(target_name) for host in hosts: hostname = host.name host_info[hostname] = { @@ -1901,6 +1917,7 @@ def generate_host_dict(array, performance): "host_user": getattr(host.chap, "host_user", None), "target_user": getattr(host.chap, "target_user", None), "target_port": [], + "logged_in_ports": {}, "volumes": [], "tags": [], "performance": [], @@ -1910,6 +1927,17 @@ def generate_host_dict(array, performance): "time_remaining": getattr(host, "time_remaining", None), "vlan": getattr(host, "vlan", None), } + # Report which array target ports each host initiator is logged into. + # An empty list means the identifier is not logged into any port, so + # the WWN/IQN/NQN can be safely removed. + logged_in = {} + for wwn in getattr(host, "wwns", None) or []: + logged_in[wwn] = sorted(login_map.get(wwn.replace(":", "").upper(), [])) + for iqn in getattr(host, "iqns", None) or []: + logged_in[iqn] = sorted(login_map.get(iqn, [])) + for nqn in getattr(host, "nqns", None) or []: + logged_in[nqn] = sorted(login_map.get(nqn, [])) + host_info[hostname]["logged_in_ports"] = logged_in host_connections = list(array.get_connections(host_names=[hostname]).items) for connection in host_connections: connection_dict = { @@ -1920,9 +1948,8 @@ def generate_host_dict(array, performance): } host_info[hostname]["volumes"].append(connection_dict) for pref_array in host.preferred_arrays: - host_info[hostname]["preferred_array"].append( - host.preferred_arrays[pref_array].name - ) + # preferred_arrays is a list of Reference objects, not a dict + host_info[hostname]["preferred_array"].append(pref_array.name) if host.is_local: host_info[host.name]["port_connectivity"] = host.port_connectivity.details diff --git a/tests/unit/plugins/modules/test_purefa_host.py b/tests/unit/plugins/modules/test_purefa_host.py index dd44ca3e..c7c7afca 100644 --- a/tests/unit/plugins/modules/test_purefa_host.py +++ b/tests/unit/plugins/modules/test_purefa_host.py @@ -1923,6 +1923,131 @@ def test_update_wwns_remove(self, mock_host_patch, mock_get): assert result is True + @patch("plugins.modules.purefa_host.check_response") + @patch("plugins.modules.purefa_host.get_with_context") + @patch("plugins.modules.purefa_host.HostPatch") + def test_update_wwns_remove_specific(self, mock_host_patch, mock_get, mock_check): + """Dropping one WWN keeps the rest and only disassociates the dropped one""" + mock_module = Mock() + mock_module.check_mode = False + mock_module.params = { + "name": "test-host", + "nqn": None, + "iqn": None, + "wwns": ["5000000000000001"], + } + mock_array = Mock() + + mock_host = Mock() + mock_host.nqns = [] + mock_host.iqns = [] + mock_host.wwns = ["5000000000000001", "5000000000000002"] + mock_get.side_effect = [ + Mock(status_code=200, items=[mock_host]), + Mock(status_code=200), + ] + + result = _update_host_initiators(mock_module, mock_array) + + assert result is True + assert mock_get.call_count == 2 + assert mock_host_patch.call_args.kwargs == {"remove_wwns": ["5000000000000002"]} + + @patch("plugins.modules.purefa_host.check_response") + @patch("plugins.modules.purefa_host.get_with_context") + @patch("plugins.modules.purefa_host.HostPatch") + def test_update_wwns_add_keeps_existing( + self, mock_host_patch, mock_get, mock_check + ): + """Adding a WWN only associates the missing one, leaving current ones""" + mock_module = Mock() + mock_module.check_mode = False + mock_module.params = { + "name": "test-host", + "nqn": None, + "iqn": None, + "wwns": ["5000000000000001", "5000000000000002"], + } + mock_array = Mock() + + mock_host = Mock() + mock_host.nqns = [] + mock_host.iqns = [] + mock_host.wwns = ["5000000000000001"] + mock_get.side_effect = [ + Mock(status_code=200, items=[mock_host]), + Mock(status_code=200), + ] + + result = _update_host_initiators(mock_module, mock_array) + + assert result is True + assert mock_host_patch.call_args.kwargs == {"add_wwns": ["5000000000000002"]} + + @patch("plugins.modules.purefa_host.check_response") + @patch("plugins.modules.purefa_host.get_with_context") + @patch("plugins.modules.purefa_host.HostPatch") + def test_update_wwns_add_and_remove(self, mock_host_patch, mock_get, mock_check): + """Adding and removing in one call uses add_wwns/remove_wwns, not wwns. + + Host has wwn1+wwn2; requesting wwn2+wwn3 must add wwn3 and remove wwn1 + while leaving wwn2 untouched (the reported audit-trail regression). + """ + mock_module = Mock() + mock_module.check_mode = False + mock_module.params = { + "name": "test-host", + "nqn": None, + "iqn": None, + "wwns": ["1000000000000002", "1000000000000003"], + } + mock_array = Mock() + + mock_host = Mock() + mock_host.nqns = [] + mock_host.iqns = [] + mock_host.wwns = ["1000000000000001", "1000000000000002"] + mock_get.side_effect = [ + Mock(status_code=200, items=[mock_host]), + Mock(status_code=200), + ] + + result = _update_host_initiators(mock_module, mock_array) + + assert result is True + assert mock_host_patch.call_args.kwargs == { + "add_wwns": ["1000000000000003"], + "remove_wwns": ["1000000000000001"], + } + assert "wwns" not in mock_host_patch.call_args.kwargs + + @patch("plugins.modules.purefa_host.check_response") + @patch("plugins.modules.purefa_host.get_with_context") + @patch("plugins.modules.purefa_host.HostPatch") + def test_update_wwns_no_change(self, mock_host_patch, mock_get, mock_check): + """No patch is issued when the WWN set already matches (colon/case aware)""" + mock_module = Mock() + mock_module.check_mode = False + mock_module.params = { + "name": "test-host", + "nqn": None, + "iqn": None, + "wwns": ["50:00:00:00:00:00:00:01"], + } + mock_array = Mock() + + mock_host = Mock() + mock_host.nqns = [] + mock_host.iqns = [] + mock_host.wwns = ["5000000000000001"] + mock_get.side_effect = [Mock(status_code=200, items=[mock_host])] + + result = _update_host_initiators(mock_module, mock_array) + + assert result is False + assert mock_get.call_count == 1 + mock_host_patch.assert_not_called() + class TestUpdateHostPersonalityExtended: """Extended test cases for _update_host_personality function""" diff --git a/tests/unit/plugins/modules/test_purefa_info.py b/tests/unit/plugins/modules/test_purefa_info.py index b8c47901..74bd0025 100644 --- a/tests/unit/plugins/modules/test_purefa_info.py +++ b/tests/unit/plugins/modules/test_purefa_info.py @@ -839,11 +839,100 @@ def test_generate_host_dict_success(self): mock_array.get_hosts_performance_balance.return_value = Mock(items=[]) mock_array.get_connections.return_value = Mock(items=[]) mock_array.get_hosts_tags.return_value = Mock(items=[]) + mock_array.get_ports_initiators.return_value = Mock(items=[]) result = generate_host_dict(mock_array, performance=False) assert "test_host" in result assert result["test_host"]["personality"] == "linux" + assert result["test_host"]["logged_in_ports"] == { + "iqn.2021-01.com.example:host1": [] + } + + def test_generate_host_dict_preferred_arrays(self): + """preferred_arrays is a list of Reference objects, not a dict""" + mock_array = Mock() + mock_array.get_rest_version.return_value = "2.38" + + pref1 = Mock() + pref1.name = "arrayA" + pref2 = Mock() + pref2.name = "arrayB" + + mock_host = Mock() + mock_host.name = "sync_host" + mock_host.host_group = None + mock_host.nqns = None + mock_host.iqns = None + mock_host.wwns = None + mock_host.personality = None + mock_host.chap = Mock(host_user=None, target_user=None) + mock_host.preferred_arrays = [pref1, pref2] + mock_host.is_local = True + mock_host.port_connectivity = Mock(details={}) + mock_host.destroyed = False + mock_host.time_remaining = None + mock_host.vlan = None + + mock_array.get_hosts.return_value = Mock(items=[mock_host]) + mock_array.get_hosts_performance_balance.return_value = Mock(items=[]) + mock_array.get_connections.return_value = Mock(items=[]) + mock_array.get_hosts_tags.return_value = Mock(items=[]) + mock_array.get_ports_initiators.return_value = Mock(items=[]) + + result = generate_host_dict(mock_array, performance=False) + + assert result["sync_host"]["preferred_array"] == ["arrayA", "arrayB"] + + def test_generate_host_dict_logged_in_ports(self): + """Test logged_in_ports maps each WWN to the target ports it uses""" + + def _login(init_wwn, target_name): + login = Mock() + login.target = Mock() + login.target.name = target_name + login.initiator = Mock() + login.initiator.wwn = init_wwn + login.initiator.iqn = None + login.initiator.nqn = None + return login + + mock_array = Mock() + mock_array.get_rest_version.return_value = "2.38" + + mock_host = Mock() + mock_host.name = "fc_host" + mock_host.host_group = None + mock_host.nqns = None + mock_host.iqns = None + mock_host.wwns = ["52:4A:93:00:00:00:00:01", "52:4A:93:00:00:00:00:02"] + mock_host.personality = None + mock_host.chap = Mock(host_user=None, target_user=None) + mock_host.preferred_arrays = {} + mock_host.is_local = True + mock_host.port_connectivity = Mock(details={}) + mock_host.destroyed = False + mock_host.time_remaining = None + mock_host.vlan = None + + mock_array.get_hosts.return_value = Mock(items=[mock_host]) + mock_array.get_hosts_performance_balance.return_value = Mock(items=[]) + mock_array.get_connections.return_value = Mock(items=[]) + mock_array.get_hosts_tags.return_value = Mock(items=[]) + # First WWN logged into two ports (lower-case/colon variant to prove + # normalisation); second WWN not logged in anywhere. + mock_array.get_ports_initiators.return_value = Mock( + items=[ + _login("52:4a:93:00:00:00:00:01", "CT1.FC1"), + _login("52:4a:93:00:00:00:00:01", "CT0.FC1"), + ] + ) + + result = generate_host_dict(mock_array, performance=False) + + logged = result["fc_host"]["logged_in_ports"] + assert logged["52:4A:93:00:00:00:00:01"] == ["CT0.FC1", "CT1.FC1"] + assert logged["52:4A:93:00:00:00:00:02"] == [] class TestGeneratePgroupsDict: