python-blivet/0016-Backport-iSCSI-initiator-name-related-fixes.patch

From d06c45db59d0e917dbab4c283f2f04c8f9206a6e Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
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 <vtrefny@redhat.com>
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 <vtrefny@redhat.com>
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 <vtrefny@redhat.com>
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 <vtrefny@redhat.com>
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