Merge lp:~raharper/curtin/trunk.fix-raid-over-iscsi into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

install: ensure iscsi service is running to handle shutdown properly

In some cases it is problematic to shutdown the iscsi targets (like raid
over iscsi). Instead of introducing extra work to stop multi-layer
storage devices during the install of curtin, this patch addresses the
core issue around iscsi termination; ensuring the iscsi service is active
prior to shutting down. In Artful, the open-iscsi service does not
automatically start if iscsi configuration is not present during boot;
curtin will restart/start the service if the storage configuration
contains iscsi disks.

This approach successfully passes all of the iscsi configurations
(plain, lvm over iscsi, and raid over iscsi).

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:525
https://jenkins.ubuntu.com/server/job/curtin-ci/613/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/613
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/613
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/613
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/613

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/613/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

./tools/jenkins-runner -p 4 tests/vmtests/test_mdadm_bcache.py tests/vmtests/test_mdadm_iscsi.py tests/vmtests/test_raid5_bcache.py tests/vmtests/test_iscsi.py tests/vmtests/test_lvm_iscsi.py

...

Ran 489 tests in 2308.432s

OK (SKIP=2)

[Scott Moser (smoser)]

so somewhere during block-meta iscsi will get started if not already running?

[lp:~raharper/curtin/trunk.fix-raid-over-iscsi]

526. By Ryan Harper on 2017-09-08

merge from trunk

527. By Ryan Harper on 2017-09-08

uses_systemd: add caching, drop target_path use, ephemeral only

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:527
https://jenkins.ubuntu.com/server/job/curtin-ci/615/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/615
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/615
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/615
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/615

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/615/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

mainly just need to somehow document this in code for the point later when someone says "what is going on here... we start a service just to handle shutdown ! ?"

[lp:~raharper/curtin/trunk.fix-raid-over-iscsi]

528. By Ryan Harper on 2017-09-08

Add additional documentation in install.py on why we restart iscsi service

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:528
https://jenkins.ubuntu.com/server/job/curtin-ci/618/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/618
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/618
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/618
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/618

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/618/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

this is fine.
my only comment is that 'sdpath' is wierd as an argument and i'd never expect anythign to use it.

i realize it is just for testing. Woudl it be better to just use 'os.path.isdir()' rather than the lstat? and then you could easily mock the response to that.

[lp:~raharper/curtin/trunk.fix-raid-over-iscsi]

529. By Ryan Harper on 2017-09-12

Replace os.lstat with os.path.isdir; drop sdpath arg; adjust unittests

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:529
https://jenkins.ubuntu.com/server/job/curtin-ci/620/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/620
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/620
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/620
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/620

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/620/rebuild

review: Approve (continuous-integration)

[Scott Moser (smoser)]

i approve, just fix the vestigial 'sdpath' uses.

review: Approve

[lp:~raharper/curtin/trunk.fix-raid-over-iscsi]

530. By Ryan Harper on 2017-09-12

Drop useless tmp_dir() in uses_systemd unittest

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:530
https://jenkins.ubuntu.com/server/job/curtin-ci/621/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/621
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/621
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/621
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/621

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/621/rebuild

review: Approve (continuous-integration)

=== modified file 'curtin/block/iscsi.py'
--- curtin/block/iscsi.py	2017-08-31 19:22:08 +0000
+++ curtin/block/iscsi.py	2017-09-12 21:16:54 +0000
@@ -195,6 +195,15 @@
     return target_nodes_location
 
 
+def restart_iscsi_service():
+    LOG.info('restarting iscsi service')
+    if util.uses_systemd():
+        cmd = ['systemctl', 'reload-or-restart', 'open-iscsi']
+    else:
+        cmd = ['service', 'open-iscsi', 'restart']
+    util.subp(cmd, capture=True)
+
+
 def save_iscsi_config(iscsi_disk):
     state = util.load_command_environment()
     # A nodes directory will be created in the same directory as the

=== modified file 'curtin/commands/install.py'
--- curtin/commands/install.py	2017-08-31 18:41:35 +0000
+++ curtin/commands/install.py	2017-09-12 21:16:54 +0000
@@ -479,11 +479,23 @@
             copy_install_log(logfile, workingd.target, log_target_path)
         # unmount everything (including iscsi disks)
         util.do_umount(workingd.target, recursive=True)
-        # disconnect configured iscsi disks
-        LOG.info("Disconnecting iscsi targets (if present)")
-        for iscsi_disk in iscsi.get_iscsi_disks_from_config(cfg):
-            LOG.info("Attempting to disconnect %s", iscsi_disk)
-            iscsi_disk.disconnect()
+
+        # The open-iscsi service in the ephemeral environment handles
+        # disconnecting active sessions.  On Artful release the systemd
+        # unit file has conditionals that are not met at boot time and
+        # results in open-iscsi service not being started; This breaks
+        # shutdown on Artful releases.
+        # Additionally, in release < Artful, if the storage configuration
+        # is layered, like RAID over iscsi volumes, then disconnecting iscsi
+        # sessions before stopping the raid device hangs.
+        # As it turns out, letting the open-iscsi service take down the
+        # session last is the cleanest way to handle all releases regardless
+        # of what may be layered on top of the iscsi disks.
+        #
+        # Check if storage configuration has iscsi volumes and if so ensure
+        # iscsi service is active before exiting install
+        if iscsi.get_iscsi_disks_from_config(cfg):
+            iscsi.restart_iscsi_service()
 
         shutil.rmtree(workingd.top)
 

=== modified file 'curtin/util.py'
--- curtin/util.py	2017-06-26 16:48:30 +0000
+++ curtin/util.py	2017-09-12 21:16:54 +0000
@@ -58,6 +58,7 @@
 _INSTALLED_MAIN = '/usr/bin/curtin'
 
 _LSB_RELEASE = {}
+_USES_SYSTEMD = None
 _HAS_UNSHARE_PID = None
 
 _DNS_REDIRECT_IP = None
@@ -1400,4 +1401,17 @@
     return data
 
 
+def uses_systemd():
+    """ Check if current enviroment uses systemd by testing if
+        /run/systemd/system is a directory; only present if
+        systemd is available on running system.
+    """
+
+    global _USES_SYSTEMD
+    if _USES_SYSTEMD is None:
+        _USES_SYSTEMD = os.path.isdir('/run/systemd/system')
+
+    return _USES_SYSTEMD
+
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'tests/unittests/test_util.py'
--- tests/unittests/test_util.py	2017-08-03 19:31:49 +0000
+++ tests/unittests/test_util.py	2017-09-12 21:16:54 +0000
@@ -851,4 +851,55 @@
         }, observed)
 
 
+class TestUsesSystemd(CiTestCase):
+
+    def setUp(self):
+        super(TestUsesSystemd, self).setUp()
+        self._reset_cache()
+        self.add_patch('curtin.util.os.path.isdir', 'mock_isdir')
+
+    def _reset_cache(self):
+        util._USES_SYSTEMD = None
+
+    def test_uses_systemd_on_systemd(self):
+        """ Test that uses_systemd returns True if sdpath is a dir """
+        # systemd_enabled
+        self.mock_isdir.return_value = True
+        result = util.uses_systemd()
+        self.assertEqual(True, result)
+        self.assertEqual(1, len(self.mock_isdir.call_args_list))
+
+    def test_uses_systemd_cached(self):
+        """Test that we cache the uses_systemd result"""
+
+        # reset_cache should ensure it's unset
+        self.assertEqual(None, util._USES_SYSTEMD)
+
+        # systemd enabled
+        self.mock_isdir.return_value = True
+
+        # first time
+        first_result = util.uses_systemd()
+
+        # check the cache value
+        self.assertEqual(first_result, util._USES_SYSTEMD)
+
+        # second time
+        second_result = util.uses_systemd()
+
+        # results should match between tries
+        self.assertEqual(True, first_result)
+        self.assertEqual(True, second_result)
+
+        # isdir should only be called once
+        self.assertEqual(1, len(self.mock_isdir.call_args_list))
+
+    def test_uses_systemd_on_non_systemd(self):
+        """ Test that uses_systemd returns False if sdpath is not a dir """
+        # systemd not available
+        self.mock_isdir.return_value = False
+        result = util.uses_systemd()
+        self.assertEqual(False, result)
+
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'tests/vmtests/test_mdadm_iscsi.py'
--- tests/vmtests/test_mdadm_iscsi.py	2017-08-02 15:46:35 +0000
+++ tests/vmtests/test_mdadm_iscsi.py	2017-09-12 21:16:54 +0000
@@ -36,3 +36,7 @@
 
 class ZestyTestIscsiMdadm(relbase.zesty, TestMdadmIscsiAbs):
     __test__ = True
+
+
+class ArtfulTestIscsiMdadm(relbase.artful, TestMdadmIscsiAbs):
+    __test__ = True