From 055998c81217ededacc95f0f96a4a8bf684fbfe0 Mon Sep 17 00:00:00 2001
From: Lucas Alvares Gomes <lucasagomes@gmail.com>
Date: Thu, 25 Feb 2016 16:32:47 +0000
Subject: [PATCH] Wait for udev to settle before listing the block devices

This patch is making the list_all_block_devices() method to wait for
udev to settle it's event queue prior to listing the devices.

Sometimes the ironic-python-agent service may start before all devices
were detected and end up erroring out because it couldn't find a
suitable disk for deployment.

Closes-Bug: #1551300
Change-Id: I1ae2062a711115a1ea14b79ae9ace7ddd2fff9d5
---
 ironic_python_agent/hardware.py               | 17 +++++++++++++
 .../tests/unit/test_hardware.py               | 25 +++++++++++++------
 .../notes/udev-settle-f75db34db990ad68.yaml   |  3 +++
 3 files changed, 38 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/udev-settle-f75db34db990ad68.yaml

diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 97d64162b..e1a728184 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -49,6 +49,21 @@ def _get_device_vendor(dev):
         LOG.warning("Can't find the device vendor for device %s", dev)
 
 
+def _udev_settle():
+    """Wait for the udev event queue to settle.
+
+    Wait for the udev event queue to settle to make sure all devices
+    are detected once the machine boots up.
+
+    """
+    try:
+        utils.execute('udevadm', 'settle')
+    except processutils.ProcessExecutionError as e:
+        LOG.warning('Something went wrong when waiting for udev '
+                    'to settle. Error: %s', e)
+        return
+
+
 def list_all_block_devices(block_type='disk'):
     """List all physical block devices
 
@@ -63,6 +78,8 @@ def list_all_block_devices(block_type='disk'):
     :return: A list of BlockDevices
     """
 
+    _udev_settle()
+
     columns = ['KNAME', 'MODEL', 'SIZE', 'ROTA', 'TYPE']
     report = utils.execute('lsblk', '-Pbdi', '-o{}'.format(','.join(columns)),
                            check_exit_code=[0])[0]
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index c02519e3c..9bf0b7bf4 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -246,6 +246,7 @@ class TestHardwareManagerLoading(test_base.BaseTestCase):
         ])
 
 
+@mock.patch.object(hardware, '_udev_settle', lambda *_: None)
 class TestGenericHardwareManager(test_base.BaseTestCase):
     def setUp(self):
         super(TestGenericHardwareManager, self).setUp()
@@ -889,13 +890,14 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
                          self.hardware.get_system_vendor_info().manufacturer)
 
 
+@mock.patch.object(utils, 'execute', autospec=True)
 class TestModuleFunctions(test_base.BaseTestCase):
 
     @mock.patch.object(hardware, '_get_device_vendor', lambda x: "FooTastic")
+    @mock.patch.object(hardware, '_udev_settle', autospec=True)
     @mock.patch.object(hardware.pyudev.Device, "from_device_file")
-    @mock.patch.object(utils, 'execute', autospec=True)
-    def test_list_all_block_devices_success(self, mocked_execute,
-                                            mocked_fromdevfile):
+    def test_list_all_block_devices_success(self, mocked_fromdevfile,
+                                            mocked_udev, mocked_execute):
         mocked_fromdevfile.return_value = {}
         mocked_execute.return_value = (BLK_DEVICE_TEMPLATE_SMALL, '')
         result = hardware.list_all_block_devices()
@@ -903,19 +905,23 @@ class TestModuleFunctions(test_base.BaseTestCase):
             'lsblk', '-Pbdi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
             check_exit_code=[0])
         self.assertEqual(BLK_DEVICE_TEMPLATE_SMALL_DEVICES, result)
+        mocked_udev.assert_called_once_with()
 
-    @mock.patch.object(utils, 'execute', autospec=True)
     @mock.patch.object(hardware, '_get_device_vendor', lambda x: "FooTastic")
-    def test_list_all_block_devices_wrong_block_type(self, mocked_execute):
+    @mock.patch.object(hardware, '_udev_settle', autospec=True)
+    def test_list_all_block_devices_wrong_block_type(self, mocked_udev,
+                                                     mocked_execute):
         mocked_execute.return_value = ('TYPE="foo" MODEL="model"', '')
         result = hardware.list_all_block_devices()
         mocked_execute.assert_called_once_with(
             'lsblk', '-Pbdi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
             check_exit_code=[0])
         self.assertEqual([], result)
+        mocked_udev.assert_called_once_with()
 
-    @mock.patch.object(utils, 'execute', autospec=True)
-    def test_list_all_block_devices_missing(self, mocked_execute):
+    @mock.patch.object(hardware, '_udev_settle', autospec=True)
+    def test_list_all_block_devices_missing(self, mocked_udev,
+                                            mocked_execute):
         """Test for missing values returned from lsblk"""
         mocked_execute.return_value = ('TYPE="disk" MODEL="model"', '')
         self.assertRaisesRegexp(
@@ -923,3 +929,8 @@ class TestModuleFunctions(test_base.BaseTestCase):
             r'^Block device caused unknown error: KNAME, ROTA, SIZE must be '
             r'returned by lsblk.$',
             hardware.list_all_block_devices)
+        mocked_udev.assert_called_once_with()
+
+    def test__udev_settle(self, mocked_execute):
+        hardware._udev_settle()
+        mocked_execute.assert_called_once_with('udevadm', 'settle')
diff --git a/releasenotes/notes/udev-settle-f75db34db990ad68.yaml b/releasenotes/notes/udev-settle-f75db34db990ad68.yaml
new file mode 100644
index 000000000..39e52334f
--- /dev/null
+++ b/releasenotes/notes/udev-settle-f75db34db990ad68.yaml
@@ -0,0 +1,3 @@
+---
+fixes:
+  - Ensure block devices are detected by the host OS before listing them.