From 78a8baeb99ba862ddbe180c02fc90e9fbff6742c Mon Sep 17 00:00:00 2001 From: Simon Dodsley Date: Fri, 17 Jul 2026 11:38:53 -0400 Subject: [PATCH 1/3] purefa_info: host login ports and preferred-arrays crash fix Two related fixes to the hosts subset of purefa_info: - Add logged_in_ports to each host, built from get_ports_initiators, mapping every host WWN/IQN/NQN to the array target ports (e.g. CT0.FC1) it is currently logged into. An empty list means the identifier is not logged in anywhere and can be safely removed. WWNs are matched case- and separator-insensitively. - Fix a TypeError ("list indices must be integers or slices, not Reference") raised whenever a host had preferred arrays set. preferred_arrays is a list of Reference objects, but the code indexed the list by the iterated Reference; iterate and use each element's .name instead. Tests: added logged_in_ports coverage and a preferred_arrays regression test (the existing test only used an empty preferred_arrays, which never exercised the crash). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fragments/1027_purefa_info_host_dict.yaml | 4 + plugins/modules/purefa_info.py | 33 ++++++- .../unit/plugins/modules/test_purefa_info.py | 89 +++++++++++++++++++ 3 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/1027_purefa_info_host_dict.yaml 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/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_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: From 23a1a936517f259e952b4d14d7dcae20f990a4cc Mon Sep 17 00:00:00 2001 From: Simon Dodsley Date: Mon, 20 Jul 2026 09:12:17 -0400 Subject: [PATCH 2/3] purefa_host - reconcile host initiators to the requested set Removing a single WWN, IQN or NQN from a host was not possible: the module only ever sent the full desired list in the patch_hosts wwns/ iqns/nqns fields, which *replace* the whole associated list, so it could not drop one initiator while keeping the rest. Treat the supplied list as the complete desired set and reconcile to it using the add_*/remove_* fields (--addwwnlist/--remwwnlist), not the replacing wwns/iqns/nqns fields (--wwnlist): add only the missing initiators and remove only the unwanted ones in a single patch. An unused WWN can now be removed simply by omitting it, and the initiators that stay are left untouched instead of being removed and re-added. Signed-off-by: Simon Dodsley --- .../1027_purefa_host_initiators.yaml | 2 + plugins/modules/purefa_host.py | 176 ++++++++---------- .../unit/plugins/modules/test_purefa_host.py | 125 +++++++++++++ 3 files changed, 204 insertions(+), 99 deletions(-) create mode 100644 changelogs/fragments/1027_purefa_host_initiators.yaml 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/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/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""" From d06de5ac4fc5c4f2b627339e205c0a9e7dcacf5a Mon Sep 17 00:00:00 2001 From: Simon Dodsley Date: Tue, 21 Jul 2026 09:58:57 -0400 Subject: [PATCH 3/3] Align collection with partner certification requirements - README: sync Requirements section with requirements.txt (py-pure-client >= 1.82.0, fix pytz typo, drop packaging) - galaxy.yml: add build_ignore to exclude non-content dirs from tarball - add .ansible-lint (production profile, no-log-password warn_list) Signed-off-by: Simon Dodsley Co-Authored-By: Claude Opus 4.8 (1M context) --- .ansible-lint | 17 +++++++++++++++++ README.md | 5 ++--- galaxy.yml | 15 +++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 .ansible-lint 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/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