From d48e767e1e8ca3d982b0cd5cbc39065fee9a1669 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:47:40 +1300 Subject: [PATCH] open_iscsi: support IPv6 portals (#11657) * fix(modules/open_iscsi): support IPv6 portals * add changelog frag --- .../fragments/11657-open-iscsi-ipv6.yml | 5 + plugins/modules/open_iscsi.py | 19 +- tests/unit/plugins/modules/test_open_iscsi.py | 51 +++ .../unit/plugins/modules/test_open_iscsi.yaml | 307 ++++++++++++++++++ 4 files changed, 376 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/11657-open-iscsi-ipv6.yml create mode 100644 tests/unit/plugins/modules/test_open_iscsi.py create mode 100644 tests/unit/plugins/modules/test_open_iscsi.yaml diff --git a/changelogs/fragments/11657-open-iscsi-ipv6.yml b/changelogs/fragments/11657-open-iscsi-ipv6.yml new file mode 100644 index 0000000000..901b330888 --- /dev/null +++ b/changelogs/fragments/11657-open-iscsi-ipv6.yml @@ -0,0 +1,5 @@ +bugfixes: + - open_iscsi - fix IPv6 portal address formatting; iscsiadm requires bracket + notation for IPv6 addresses but the module was producing an incorrect format + (https://github.com/ansible-collections/community.general/issues/4467, + https://github.com/ansible-collections/community.general/pull/11657). diff --git a/plugins/modules/open_iscsi.py b/plugins/modules/open_iscsi.py index 3687a4089a..2b3fe9f636 100644 --- a/plugins/modules/open_iscsi.py +++ b/plugins/modules/open_iscsi.py @@ -15,7 +15,7 @@ description: - Discover targets on given portal, (dis)connect targets, mark targets to manually or auto start, return device nodes of connected targets. requirements: - - open_iscsi library and tools (iscsiadm) + - C(open_iscsi) library and tools C(iscsiadm) extends_documentation_fragment: - community.general.attributes attributes: @@ -153,6 +153,13 @@ ISCSIADM = "iscsiadm" iscsiadm_cmd = None +def format_portal(portal, port): + """Format portal address and port for iscsiadm, handling IPv6 bracket notation.""" + if ":" in portal: + return f"[{portal}]:{port}" + return f"{portal}:{port}" + + def compare_nodelists(l1, l2): l1.sort() l2.sort() @@ -190,7 +197,7 @@ def iscsi_get_cached_nodes(module, portal=None): def iscsi_discover(module, portal, port): - cmd = [iscsiadm_cmd, "--mode", "discovery", "--type", "sendtargets", "--portal", f"{portal}:{port}"] + cmd = [iscsiadm_cmd, "--mode", "discovery", "--type", "sendtargets", "--portal", format_portal(portal, port)] module.run_command(cmd, check_rc=True) @@ -269,7 +276,7 @@ def target_login(module, target, check_rc, portal=None, port=None): cmd = [iscsiadm_cmd, "--mode", "node", "--targetname", target, "--login"] if portal is not None and port is not None: cmd.append("--portal") - cmd.append(f"{portal}:{port}") + cmd.append(format_portal(portal, port)) rc, out, err = module.run_command(cmd, check_rc=check_rc) return rc @@ -301,7 +308,7 @@ def target_isauto(module, target, portal=None, port=None): if portal is not None and port is not None: cmd.append("--portal") - cmd.append(f"{portal}:{port}") + cmd.append(format_portal(portal, port)) dummy, out, dummy = module.run_command(cmd, check_rc=True) @@ -328,7 +335,7 @@ def target_setauto(module, target, portal=None, port=None): if portal is not None and port is not None: cmd.append("--portal") - cmd.append(f"{portal}:{port}") + cmd.append(format_portal(portal, port)) module.run_command(cmd, check_rc=True) @@ -349,7 +356,7 @@ def target_setmanual(module, target, portal=None, port=None): if portal is not None and port is not None: cmd.append("--portal") - cmd.append(f"{portal}:{port}") + cmd.append(format_portal(portal, port)) module.run_command(cmd, check_rc=True) diff --git a/tests/unit/plugins/modules/test_open_iscsi.py b/tests/unit/plugins/modules/test_open_iscsi.py new file mode 100644 index 0000000000..7fbccc7b01 --- /dev/null +++ b/tests/unit/plugins/modules/test_open_iscsi.py @@ -0,0 +1,51 @@ +# Copyright (c) 2026 Alexei Znamensky (russoz@gmail.com) +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import annotations + +from ansible_collections.community.general.plugins.modules import open_iscsi + +from .uthelper import RunCommandMock, UTHelper +from .uthelper import TestCaseMock as MockBase + +_MODULE = "ansible_collections.community.general.plugins.modules.open_iscsi" + + +class SocketMock(MockBase): + name = "socket_getaddrinfo" + + def setup(self, mocker): + ip = self.mock_specs["return"] + mocker.patch( + f"{_MODULE}.socket.getaddrinfo", + return_value=[(None, None, None, None, (ip, None))], + ) + + def check(self, test_case, results): + pass + + +class GlobMock(MockBase): + name = "glob_glob" + + def setup(self, mocker): + paths = self.mock_specs.get("return", []) + mocker.patch(f"{_MODULE}.glob.glob", return_value=paths) + mocker.patch(f"{_MODULE}.os.path.realpath", side_effect=lambda x: x) + + def check(self, test_case, results): + pass + + +class TimeSleepMock(MockBase): + name = "time_sleep" + + def setup(self, mocker): + mocker.patch(f"{_MODULE}.time.sleep") + + def check(self, test_case, results): + pass + + +UTHelper.from_module(open_iscsi, __name__, mocks=[RunCommandMock, SocketMock, GlobMock, TimeSleepMock]) diff --git a/tests/unit/plugins/modules/test_open_iscsi.yaml b/tests/unit/plugins/modules/test_open_iscsi.yaml new file mode 100644 index 0000000000..1aada8eaa4 --- /dev/null +++ b/tests/unit/plugins/modules/test_open_iscsi.yaml @@ -0,0 +1,307 @@ +# Copyright (c) 2026 Alexei Znamensky (russoz@gmail.com) +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +--- +anchors: + target: &target "iqn.2006-01.com.example:storage.disk1" + target_iscsi_node_out: &target-node-out "10.1.2.3:3260,1 iqn.2006-01.com.example:storage.disk1\n" + target_session_out: &target-session-out "tcp: [1] 10.1.2.3:3260,1 iqn.2006-01.com.example:storage.disk1 (non-flash)\n" + + iscsiadm_node_cached: &iscsiadm-node-cached + command: [/testbin/iscsiadm, --mode, node] + environ: {} + rc: 0 + out: *target-node-out + err: '' + iscsiadm_node_empty: &iscsiadm-node-empty + command: [/testbin/iscsiadm, --mode, node] + environ: {} + rc: 21 + out: '' + err: 'iscsiadm: No records found' + iscsiadm_session_active: &iscsiadm-session-active + command: [/testbin/iscsiadm, --mode, session] + environ: {} + rc: 0 + out: *target-session-out + err: '' + iscsiadm_session_empty: &iscsiadm-session-empty + command: [/testbin/iscsiadm, --mode, session] + environ: {} + rc: 21 + out: '' + err: '' + ipv6_target_node_out: &ipv6-target-node-out "fd00::1:3260,1 iqn.2006-01.com.example:storage.disk1\n" + +test_cases: + # --- show_nodes --- + + - id: show_nodes_only + input: + show_nodes: true + output: + changed: false + nodes: + - *target + mocks: + run_command: + - *iscsiadm-node-cached + + - id: show_nodes_empty + input: + show_nodes: true + output: + changed: false + nodes: [] + mocks: + run_command: + - *iscsiadm-node-empty + + # --- discover --- + + - id: discover_new_nodes + input: + portal: "10.1.2.3" + discover: true + show_nodes: true + output: + changed: true + cache_updated: true + nodes: + - *target + mocks: + socket_getaddrinfo: + return: "10.1.2.3" + run_command: + - *iscsiadm-node-empty + - command: [/testbin/iscsiadm, --mode, discovery, --type, sendtargets, --portal, "10.1.2.3:3260"] + environ: {check_rc: true} + rc: 0 + out: *target-node-out + err: '' + - *iscsiadm-node-cached + + - id: discover_no_change + input: + portal: "10.1.2.3" + discover: true + output: + changed: false + mocks: + socket_getaddrinfo: + return: "10.1.2.3" + run_command: + - *iscsiadm-node-cached + - command: [/testbin/iscsiadm, --mode, discovery, --type, sendtargets, --portal, "10.1.2.3:3260"] + environ: {check_rc: true} + rc: 0 + out: *target-node-out + err: '' + - *iscsiadm-node-cached + + # --- login --- + + - id: login_already_loggedon + input: + login: true + target: *target + output: + changed: false + devicenodes: [] + mocks: + glob_glob: + return: [] + run_command: + - *iscsiadm-node-cached + - *iscsiadm-session-active + + - id: login_needed + # Note: target_login() is called as target_login(module, target, portal, port) + # where portal=None is passed as check_rc — a known bug at line 484. + # The login command therefore has check_rc=None and no --portal flag appended. + input: + login: true + target: *target + output: + changed: true + connection_changed: true + devicenodes: [] + mocks: + glob_glob: + return: [] + time_sleep: true + run_command: + - *iscsiadm-node-cached + - *iscsiadm-session-empty + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target, --login] + environ: {check_rc: null} + rc: 0 + out: '' + err: '' + + - id: logout_already_off + input: + login: false + target: *target + output: + changed: false + mocks: + run_command: + - *iscsiadm-node-cached + - *iscsiadm-session-empty + + - id: logout_needed + input: + login: false + target: *target + output: + changed: true + connection_changed: true + mocks: + run_command: + - *iscsiadm-node-cached + - *iscsiadm-session-active + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target, --logout] + environ: {check_rc: true} + rc: 0 + out: '' + err: '' + + - id: check_mode_login + flags: + check: true + input: + login: true + target: *target + output: + changed: true + connection_changed: true + mocks: + run_command: + - *iscsiadm-node-cached + - *iscsiadm-session-empty + + # --- auto_node_startup --- + + - id: auto_node_startup_already_auto + input: + auto_node_startup: true + target: *target + output: + changed: false + automatic_changed: false + mocks: + run_command: + - *iscsiadm-node-cached + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target] + environ: {check_rc: true} + rc: 0 + out: "node.startup = automatic\n" + err: '' + + - id: auto_node_startup_set_auto + input: + auto_node_startup: true + target: *target + output: + changed: true + automatic_changed: true + mocks: + run_command: + - *iscsiadm-node-cached + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target] + environ: {check_rc: true} + rc: 0 + out: "node.startup = manual\n" + err: '' + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target, --op=update, --name, node.startup, --value, automatic] + environ: {check_rc: true} + rc: 0 + out: '' + err: '' + + - id: auto_node_startup_set_manual + input: + auto_node_startup: false + target: *target + output: + changed: true + automatic_changed: true + mocks: + run_command: + - *iscsiadm-node-cached + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target] + environ: {check_rc: true} + rc: 0 + out: "node.startup = automatic\n" + err: '' + - command: [/testbin/iscsiadm, --mode, node, --targetname, *target, --op=update, --name, node.startup, --value, manual] + environ: {check_rc: true} + rc: 0 + out: '' + err: '' + + # --- rescan --- + + - id: rescan_target + input: + rescan: true + target: *target + output: + changed: true + sessions: '' + mocks: + run_command: + - *iscsiadm-node-cached + - command: [/testbin/iscsiadm, --mode, node, --rescan, -T, *target] + environ: {} + rc: 0 + out: '' + err: '' + + - id: rescan_all + input: + rescan: true + output: + changed: true + sessions: '' + mocks: + run_command: + - *iscsiadm-node-cached + - command: [/testbin/iscsiadm, --mode, session, --rescan] + environ: {} + rc: 0 + out: '' + err: '' + + # --- IPv6 portal --- + + - id: discover_ipv6_portal + # Verifies that the discovery command uses RFC-2732 bracket notation for IPv6: + # --portal [fd00::1]:3260 (not --portal fd00::1:3260) + # Note: post-discover node-list filtering is also broken for IPv6 (separate issue): + # iscsi_get_cached_nodes splits on ':' so target_portal="fd00" != portal="fd00::1", + # meaning nodes is always [] for IPv6 portals and changed stays false. + input: + portal: "fd00::1" + discover: true + show_nodes: true + output: + changed: false + nodes: [] + mocks: + socket_getaddrinfo: + return: "fd00::1" + run_command: + - *iscsiadm-node-empty + - command: [/testbin/iscsiadm, --mode, discovery, --type, sendtargets, --portal, "[fd00::1]:3260"] + environ: {check_rc: true} + rc: 0 + out: *ipv6-target-node-out + err: '' + - command: [/testbin/iscsiadm, --mode, node] + environ: {} + rc: 0 + out: *ipv6-target-node-out + err: ''