From d06c45db59d0e917dbab4c283f2f04c8f9206a6e Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Mon, 6 Mar 2023 10:51:42 +0100 Subject: [PATCH 1/5] Allow changing iSCSI initiator name after setting it Resolves: rhbz#2083139 --- blivet/iscsi.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/blivet/iscsi.py b/blivet/iscsi.py index 86451db3..0d063f2a 100644 --- a/blivet/iscsi.py +++ b/blivet/iscsi.py @@ -212,14 +212,23 @@ class iSCSI(object): @initiator.setter @storaged_iscsi_required(critical=True, eval_mode=util.EvalMode.onetime) def initiator(self, val): - if self.initiator_set and val != self._initiator: - raise ValueError(_("Unable to change iSCSI initiator name once set")) if len(val) == 0: raise ValueError(_("Must provide an iSCSI initiator name")) + active = self._get_active_sessions() + if active: + raise errors.ISCSIError(_("Cannot change initiator name with an active session")) + log.info("Setting up iSCSI initiator name %s", self.initiator) args = GLib.Variant("(sa{sv})", (val, None)) self._call_initiator_method("SetInitiatorName", args) + + if self.initiator_set and val != self._initiator: + log.info("Restarting iscsid after initiator name change") + rc = util.run_program(["systemctl", "restart", "iscsid"]) + if rc != 0: + raise errors.ISCSIError(_("Failed to restart iscsid after initiator name change")) + self._initiator = val def active_nodes(self, target=None): -- 2.40.1 From b71991d65c270c023364b03c499b4bf3e245fbd0 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Mon, 6 Mar 2023 15:10:28 +0100 Subject: [PATCH 2/5] Add a basic test case for the iscsi module Related: rhbz#2083139 --- tests/storage_tests/__init__.py | 2 + tests/storage_tests/iscsi_test.py | 157 +++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+) create mode 100644 tests/storage_tests/iscsi_test.py diff --git a/tests/storage_tests/__init__.py b/tests/storage_tests/__init__.py index 3b2a6cc4..e69fcc34 100644 --- a/tests/storage_tests/__init__.py +++ b/tests/storage_tests/__init__.py @@ -3,3 +3,5 @@ from .formats_test import * from .partitioning_test import * from .unsupported_disklabel_test import * + +from .iscsi_test import * diff --git a/tests/storage_tests/iscsi_test.py b/tests/storage_tests/iscsi_test.py new file mode 100644 index 00000000..00cc7c36 --- /dev/null +++ b/tests/storage_tests/iscsi_test.py @@ -0,0 +1,157 @@ +import glob +import os +import re +import shutil +import subprocess +import unittest + +from contextlib import contextmanager + +from .storagetestcase import create_sparse_tempfile + + +def read_file(filename, mode="r"): + with open(filename, mode) as f: + content = f.read() + return content + + +@contextmanager +def udev_settle(): + try: + yield + finally: + os.system("udevadm settle") + + +def _delete_backstore(name): + status = subprocess.call(["targetcli", "/backstores/fileio/ delete %s" % name], + stdout=subprocess.DEVNULL) + if status != 0: + raise RuntimeError("Failed to delete the '%s' fileio backstore" % name) + + +def delete_iscsi_target(iqn, backstore=None): + status = subprocess.call(["targetcli", "/iscsi delete %s" % iqn], + stdout=subprocess.DEVNULL) + if status != 0: + raise RuntimeError("Failed to delete the '%s' iscsi device" % iqn) + + if backstore is not None: + _delete_backstore(backstore) + + +def create_iscsi_target(fpath, initiator_name=None): + """ + Creates a new iSCSI target (using targetcli) on top of the + :param:`fpath` backing file. + + :param str fpath: path of the backing file + :returns: iSCSI IQN, backstore name + :rtype: tuple of str + + """ + + # "register" the backing file as a fileio backstore + store_name = os.path.basename(fpath) + status = subprocess.call(["targetcli", "/backstores/fileio/ create %s %s" % (store_name, fpath)], stdout=subprocess.DEVNULL) + if status != 0: + raise RuntimeError("Failed to register '%s' as a fileio backstore" % fpath) + + out = subprocess.check_output(["targetcli", "/backstores/fileio/%s info" % store_name]) + out = out.decode("utf-8") + store_wwn = None + for line in out.splitlines(): + if line.startswith("wwn: "): + store_wwn = line[5:] + if store_wwn is None: + raise RuntimeError("Failed to determine '%s' backstore's wwn" % store_name) + + # create a new iscsi device + out = subprocess.check_output(["targetcli", "/iscsi create"]) + out = out.decode("utf-8") + match = re.match(r'Created target (.*).', out) + if match: + iqn = match.groups()[0] + else: + _delete_backstore(store_name) + raise RuntimeError("Failed to create a new iscsi target") + + if initiator_name: + status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/acls create %s" % (iqn, initiator_name)], stdout=subprocess.DEVNULL) + if status != 0: + delete_iscsi_target(iqn, store_name) + raise RuntimeError("Failed to set ACLs for '%s'" % iqn) + + with udev_settle(): + status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/luns create /backstores/fileio/%s" % (iqn, store_name)], stdout=subprocess.DEVNULL) + if status != 0: + delete_iscsi_target(iqn, store_name) + raise RuntimeError("Failed to create a new LUN for '%s' using '%s'" % (iqn, store_name)) + + status = subprocess.call(["targetcli", "/iscsi/%s/tpg1 set attribute generate_node_acls=1" % iqn], stdout=subprocess.DEVNULL) + if status != 0: + raise RuntimeError("Failed to set ACLs for '%s'" % iqn) + + return iqn, store_name + + +@unittest.skipUnless(os.geteuid() == 0, "requires root privileges") +@unittest.skipUnless(os.environ.get("JENKINS_HOME"), "jenkins only test") +@unittest.skipUnless(shutil.which("iscsiadm"), "iscsiadm not available") +class ISCSITestCase(unittest.TestCase): + + _disk_size = 512 * 1024**2 + initiator = 'iqn.1994-05.com.redhat:iscsi-test' + + def setUp(self): + self.addCleanup(self._clean_up) + + self._dev_file = None + self.dev = None + + self._dev_file = create_sparse_tempfile("blivet_test", self._disk_size) + try: + self.dev, self.backstore = create_iscsi_target(self._dev_file, self.initiator) + except RuntimeError as e: + raise RuntimeError("Failed to setup targetcli device for testing: %s" % e) + + def _force_logout(self): + subprocess.call(["iscsiadm", "--mode", "node", "--logout", "--name", self.dev], stdout=subprocess.DEVNULL) + + def _clean_up(self): + self._force_logout() + delete_iscsi_target(self.dev, self.backstore) + os.unlink(self._dev_file) + + def test_discover_login(self): + from blivet.iscsi import iscsi, has_iscsi + + if not has_iscsi(): + self.skipTest("iSCSI not available, skipping") + + iscsi.initiator = self.initiator + nodes = iscsi.discover("127.0.0.1") + self.assertTrue(nodes) + + if len(nodes) > 1: + self.skipTest("Discovered more than one iSCSI target on localhost, skipping") + + self.assertEqual(nodes[0].address, "127.0.0.1") + self.assertEqual(nodes[0].port, 3260) + self.assertEqual(nodes[0].name, self.dev) + + # change the initiator name + iscsi.initiator = self.initiator + "_1" + self.assertEqual(iscsi.initiator, self.initiator + "_1") + + # try to login + ret, err = iscsi.log_into_node(nodes[0]) + self.assertTrue(ret, "Login failed: %s" % err) + + # check the session for initiator name + sessions = glob.glob("/sys/class/iscsi_session/*/") + self.assertTrue(sessions) + self.assertEqual(len(sessions), 1) + initiator = read_file(sessions[0] + "initiatorname").strip() + self.assertEqual(initiator, iscsi.initiator) -- 2.40.1 From 65e8150a7404e37dd2740841a88e7f2565836406 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Mon, 6 Mar 2023 15:14:40 +0100 Subject: [PATCH 3/5] tests: Use blivet-specific prefix for targetcli backing files The code is originally from libblockdev hence the "bd" prefix, we should use a different prefix for blivet to be able to identify which test suite failed to clean the files. Related: rhbz#2083139 --- tests/storage_tests/storagetestcase.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage_tests/storagetestcase.py b/tests/storage_tests/storagetestcase.py index 35d57ce9..9f859977 100644 --- a/tests/storage_tests/storagetestcase.py +++ b/tests/storage_tests/storagetestcase.py @@ -39,7 +39,7 @@ def create_sparse_tempfile(name, size): :param size: the file size (in bytes) :returns: the path to the newly created file """ - (fd, path) = tempfile.mkstemp(prefix="bd.", suffix="-%s" % name) + (fd, path) = tempfile.mkstemp(prefix="blivet.", suffix="-%s" % name) os.close(fd) create_sparse_file(path, size) return path -- 2.40.1 From 41278ef1b3f949303fd30fff2ccdde75f713c9f8 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 19 Jul 2023 13:57:39 +0200 Subject: [PATCH 4/5] iscsi: Save firmware initiator name to /etc/iscsi/initiatorname.iscsi Resolves: rhbz#2084043 --- blivet/iscsi.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/blivet/iscsi.py b/blivet/iscsi.py index 0d063f2a..8080a671 100644 --- a/blivet/iscsi.py +++ b/blivet/iscsi.py @@ -160,6 +160,11 @@ class iSCSI(object): self._initiator = initiatorname except Exception as e: # pylint: disable=broad-except log.info("failed to get initiator name from iscsi firmware: %s", str(e)) + else: + # write the firmware initiator to /etc/iscsi/initiatorname.iscsi + log.info("Setting up firmware iSCSI initiator name %s", self.initiator) + args = GLib.Variant("(sa{sv})", (initiatorname, None)) + self._call_initiator_method("SetInitiatorName", args) # So that users can write iscsi() to get the singleton instance def __call__(self): -- 2.40.1 From fce8b73965d968aab546bc7e0ecb65d1995da46f Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 19 Jul 2023 10:38:45 +0200 Subject: [PATCH 5/5] tests: Improve iscsi_test.ISCSITestCase Changed how we create the initiator name ACLs based on RTT test case for rhbz#2084043 and also improved the test case itself. Related: rhbz#2083139 --- tests/storage_tests/iscsi_test.py | 36 +++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/tests/storage_tests/iscsi_test.py b/tests/storage_tests/iscsi_test.py index 00cc7c36..6cc83a59 100644 --- a/tests/storage_tests/iscsi_test.py +++ b/tests/storage_tests/iscsi_test.py @@ -77,21 +77,17 @@ def create_iscsi_target(fpath, initiator_name=None): _delete_backstore(store_name) raise RuntimeError("Failed to create a new iscsi target") - if initiator_name: - status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/acls create %s" % (iqn, initiator_name)], stdout=subprocess.DEVNULL) - if status != 0: - delete_iscsi_target(iqn, store_name) - raise RuntimeError("Failed to set ACLs for '%s'" % iqn) - with udev_settle(): status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/luns create /backstores/fileio/%s" % (iqn, store_name)], stdout=subprocess.DEVNULL) if status != 0: delete_iscsi_target(iqn, store_name) raise RuntimeError("Failed to create a new LUN for '%s' using '%s'" % (iqn, store_name)) - status = subprocess.call(["targetcli", "/iscsi/%s/tpg1 set attribute generate_node_acls=1" % iqn], stdout=subprocess.DEVNULL) - if status != 0: - raise RuntimeError("Failed to set ACLs for '%s'" % iqn) + if initiator_name: + status = subprocess.call(["targetcli", "/iscsi/%s/tpg1/acls create %s" % (iqn, initiator_name)], stdout=subprocess.DEVNULL) + if status != 0: + delete_iscsi_target(iqn, store_name) + raise RuntimeError("Failed to set ACLs for '%s'" % iqn) return iqn, store_name @@ -130,6 +126,7 @@ class ISCSITestCase(unittest.TestCase): if not has_iscsi(): self.skipTest("iSCSI not available, skipping") + # initially set the initiator to the correct/allowed one iscsi.initiator = self.initiator nodes = iscsi.discover("127.0.0.1") self.assertTrue(nodes) @@ -141,11 +138,28 @@ class ISCSITestCase(unittest.TestCase): self.assertEqual(nodes[0].port, 3260) self.assertEqual(nodes[0].name, self.dev) - # change the initiator name + # change the initiator name to a wrong one iscsi.initiator = self.initiator + "_1" self.assertEqual(iscsi.initiator, self.initiator + "_1") - # try to login + # check the change made it to /etc/iscsi/initiatorname.iscsi + initiator_file = read_file("/etc/iscsi/initiatorname.iscsi").strip() + self.assertEqual(initiator_file, "InitiatorName=%s" % self.initiator + "_1") + + # try to login (should fail) + ret, err = iscsi.log_into_node(nodes[0]) + self.assertFalse(ret) + self.assertIn("authorization failure", err) + + # change the initiator name back to the correct one + iscsi.initiator = self.initiator + self.assertEqual(iscsi.initiator, self.initiator) + + # check the change made it to /etc/iscsi/initiatorname.iscsi + initiator_file = read_file("/etc/iscsi/initiatorname.iscsi").strip() + self.assertEqual(initiator_file, "InitiatorName=%s" % self.initiator) + + # try to login (should work now) ret, err = iscsi.log_into_node(nodes[0]) self.assertTrue(ret, "Login failed: %s" % err) -- 2.40.1