Merge "Add secondary sorting by name when guessing root disk"

This commit is contained in:
Zuul 2019-02-13 07:41:41 +00:00 committed by Gerrit Code Review
commit e5da6d0007
4 changed files with 81 additions and 6 deletions

View File

@ -216,7 +216,7 @@ class TestCollectDefault(BaseDiscoverTest):
self.assertTrue(self.data['inventory'][key])
self.assertEqual('boot:if', self.data['boot_interface'])
self.assertEqual(self.inventory['disks'][0].name,
self.assertEqual(self.inventory['disks'][2].name,
self.data['root_disk'].name)
mock_dispatch.assert_called_once_with('list_hardware_info')

View File

@ -30,6 +30,7 @@ import six
import testtools
from ironic_python_agent import errors
from ironic_python_agent import hardware
from ironic_python_agent.tests.unit import base as ironic_agent_base
from ironic_python_agent import utils
@ -411,6 +412,67 @@ class TestUtils(testtools.TestCase):
'foo', binary=True, log_stdout=False)
self.assertEqual(contents, data.read())
@mock.patch.object(subprocess, 'check_call', autospec=True)
def test_guess_root_disk_primary_sort(self, mock_call):
block_devices = [
hardware.BlockDevice(name='/dev/sdc',
model='too small',
size=4294967295,
rotational=True),
hardware.BlockDevice(name='/dev/sda',
model='bigger than sdb',
size=21474836480,
rotational=True),
hardware.BlockDevice(name='/dev/sdb',
model='',
size=10737418240,
rotational=True),
hardware.BlockDevice(name='/dev/sdd',
model='bigger than sdb',
size=21474836480,
rotational=True),
]
device = utils.guess_root_disk(block_devices)
self.assertEqual(device.name, '/dev/sdb')
@mock.patch.object(subprocess, 'check_call', autospec=True)
def test_guess_root_disk_secondary_sort(self, mock_call):
block_devices = [
hardware.BlockDevice(name='/dev/sdc',
model='_',
size=10737418240,
rotational=True),
hardware.BlockDevice(name='/dev/sdb',
model='_',
size=10737418240,
rotational=True),
hardware.BlockDevice(name='/dev/sda',
model='_',
size=10737418240,
rotational=True),
hardware.BlockDevice(name='/dev/sdd',
model='_',
size=10737418240,
rotational=True),
]
device = utils.guess_root_disk(block_devices)
self.assertEqual(device.name, '/dev/sda')
@mock.patch.object(subprocess, 'check_call', autospec=True)
def test_guess_root_disk_disks_too_small(self, mock_call):
block_devices = [
hardware.BlockDevice(name='/dev/sda',
model='too small',
size=4294967295,
rotational=True),
hardware.BlockDevice(name='/dev/sdb',
model='way too small',
size=1,
rotational=True),
]
self.assertRaises(errors.DeviceNotFound,
utils.guess_root_disk, block_devices)
@mock.patch.object(subprocess, 'check_call', autospec=True)
def test_is_journalctl_present(self, mock_call):
self.assertTrue(utils.is_journalctl_present())

View File

@ -290,12 +290,15 @@ class AccumulatedFailures(object):
def guess_root_disk(block_devices, min_size_required=4 * units.Gi):
"""Find suitable disk provided that root device hints are not given.
If no hints are passed find the first device larger than min_size_required,
assume it is the OS disk
If no hints are passed, order the devices by size (primary key) and
name (secondary key), and return the first device larger than
min_size_required as the root disk.
"""
# TODO(russellhaering): This isn't a valid assumption in
# all cases, is there a more reasonable default behavior?
block_devices.sort(key=lambda device: device.size)
# NOTE(arne_wiebalck): Order devices by size and name. Secondary
# ordering by name is done to increase chances of successful
# booting for BIOSes which try only one (the "first") disk.
block_devices.sort(key=lambda device: (device.size, device.name))
if not block_devices or block_devices[-1].size < min_size_required:
raise errors.DeviceNotFound(
"No suitable device was found "

View File

@ -0,0 +1,10 @@
---
features:
- |
Adds secondary sorting by device name when guessing the root disk. This
makes the selection process more predicatble and increases the chances
that systems which try only one device for booting will actually
successfully boot after deployment. As the primary sorting is still done
by size, the root device hints still take priority, and the current
behaviour is basically not specifying the order beyond size, this change
does not break backwards compatibility.