openstack/fuel-web
Code Issues Proposed changes

Add bootable disk node flag

Browse Source 
Adding this flag to specified disk allows user to choose bootable disk
from Fuel UI and CLi.

This change is very important for deployments with multipath connected
block devices. Mainly, it is because, order of disks in UI can change
easly in this case, so we need introduce to user a possibility to
choose bootable disk explicitly.

Change-Id: I22ffe9104d2ec5a6598d496691fffa0087111070
Partial-Bug: #1567450
This commit is contained in:
sslypushenko 2016-05-11 19:54:42 +03:00
parent 2b82c053cc
commit 2df11392ab
11 changed files with 268 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
nailgun/nailgun/extensions/volume_manager/manager.py
View File

@ -21,6 +21,7 @@ All sizes in megabytes.
from copy import deepcopy
from functools import partial
import re
from oslo_serialization import jsonutils
import six
@ -104,6 +105,13 @@ def modify_volumes_hook(role_mapping, node):
return role_mapping
def get_rule_to_pick_boot(node):
# FIXME(apopovych): ugly hack to avoid circular dependency
from nailgun import objects
metadata = objects.Cluster.get_volumes_metadata(node.cluster)
return metadata.get('rule_to_pick_boot_disk', [])
def get_node_spaces(node):
"""Helper for retrieving node volumes.
@ -188,6 +196,7 @@ class DisksFormatConvertor(object):
"type": "disk",
"id": "sda",
"size": 953869,
"bootable": True,
"volumes": [
{
"mount": "/boot",
@ -210,6 +219,7 @@ class DisksFormatConvertor(object):
{
"id": "sda",
"size": 953869,
"bootable": True,
"volumes": [
{
"name": "os",
@ -231,7 +241,7 @@ class DisksFormatConvertor(object):
volume['name'],
volume['size'])
volume_manager.set_volume_flags(disk['id'], volume)
volume_manager.update_bootable_flags(disks)
return volume_manager.volumes
@classmethod
@ -258,6 +268,7 @@ class DisksFormatConvertor(object):
'size': size,
'volumes': cls.serialize_volumes(disk['volumes']),
'extra': disk['extra'],
'bootable': disk['bootable']
}
disks_in_simple_format.append(disk_simple)
@ -342,7 +353,7 @@ class Disk(object):
def __init__(self, volumes, generator_method, disk_id, name,
size, boot_is_raid=True, possible_pvs_count=0,
disk_extra=None):
disk_extra=None, bootable=False):
"""Create disk.
:param volumes: volumes which need to allocate on disk
@ -354,6 +365,7 @@ class Disk(object):
equal to 'raid' else 'partition'
:param possible_pvs_count: used for lvm pool calculation
size of lvm pool = possible_pvs_count * lvm meta size
:param bootable: if True system will boot from this disk
"""
self.call_generator = generator_method
self.id = disk_id
@ -363,6 +375,7 @@ class Disk(object):
self.lvm_meta_size = self.call_generator('calc_lvm_meta_size')
self.max_lvm_meta_pool_size = self.lvm_meta_size * possible_pvs_count
self.free_space = self.size
self.bootable = bootable
self.set_volumes(volumes)
# For determination type of boot
@ -567,7 +580,8 @@ class Disk(object):
'type': 'disk',
'size': self.size,
'volumes': self.volumes,
'free_space': self.free_space
'free_space': self.free_space,
'bootable': self.bootable,
}
def __repr__(self):
@ -600,7 +614,7 @@ class VolumeManager(object):
# For swap calculation
self.ram = node.ram
self.allowed_volumes = node.get_node_spaces()
self.rule_to_pick_boot = node.get_rule_to_pick_boot()
self.disks = []
disks_count = len(node.disks)
for d in sorted(node.disks, key=lambda i: i['name']):
@ -610,7 +624,8 @@ class VolumeManager(object):
disk_id = existing_disk[0]['id'] if existing_disk else d["disk"]
disk_volumes = existing_disk[0].get(
'volumes', []) if existing_disk else []
bootable_flag = existing_disk[0].get(
'bootable', False) if existing_disk else False
disk = Disk(
disk_volumes,
self.call_generator,
@ -620,7 +635,8 @@ class VolumeManager(object):
boot_is_raid=boot_is_raid,
# Count of possible PVs equal to count of allowed VGs
possible_pvs_count=len(only_vg(self.allowed_volumes)),
disk_extra=d.get("extra", []))
disk_extra=d.get("extra", []),
bootable=bootable_flag)
self.disks.append(disk)
@ -780,6 +796,23 @@ class VolumeManager(object):
self.__logger('Updated volume flags %s' % self.volumes)
return self.volumes
def update_bootable_flags(self, disks):
"""Update value of bootable flag for specified disk"""
if not any(disk.get('bootable', False) for disk in disks):
self.pick_default_bootable_disk()
return
for disk in disks:
disk_to_update = next(d for d in self.disks if d.id == disk['id'])
if disk_to_update.bootable != disk.get('bootable', False):
self.__logger('Update bootable flag for disk=%s on value %s'
'' % (disk['id'], disk.get('bootable', False)))
disk_to_update.bootable = disk.get('bootable', False)
# Update self.volumes for consistency
for disk in self.disks:
for idx, volume in enumerate(self.volumes):
if volume.get('id') == disk.id:
self.volumes[idx]['bootable'] = disk.bootable
def get_space_type(self, volume_name):
"""Get type of space represente on disk as volume."""
for volume in self.allowed_volumes:
@ -1004,9 +1037,70 @@ class VolumeManager(object):
self.volumes = self.expand_generators(self.volumes)
self.pick_default_bootable_disk()
self.__logger('Generated volumes: %s' % self.volumes)
return self.volumes
def _get_root_disk(self, root_mount='/'):
"""Return the disk, which contains root partition"""
for disk in self.disks:
mounts = set((volume.get('mount') for volume in disk.volumes
if volume.get('mount')))
volume_groups = [volume.get('vg') for volume in disk.volumes
if volume['size'] > 0 and volume.get('vg')]
for vg_id in volume_groups:
vg = find_space_by_id(self.allowed_volumes, vg_id)
mounts.update(
[volume.get('mount') for volume in vg['volumes']])
if root_mount in mounts:
return disk
def pick_disk_as_bootable(self, disk=None):
"""Pick given disk as a bootable
If disk is None, then all disks will be set as bootable
"""
for d in self.disks:
d.bootable = False
if disk is not None:
disk.bootable = True
# Update self.volumes for consistency
for disk in self.disks:
for idx, volume in enumerate(self.volumes):
if volume.get('id') == disk.id:
self.volumes[idx]['bootable'] = disk.bootable
def pick_default_bootable_disk(self):
# If root disk name matchs with any regex from
# "pick_root_disk_if_disk_name_match" rule, it should be picked as
# bootable
for rule in self.rule_to_pick_boot:
if rule['type'] != 'pick_root_disk_if_disk_name_match':
continue
root_disk = self._get_root_disk(root_mount=rule.get('root_mount'))
regex = re.compile(rule['regex'])
if regex.match(root_disk.name):
self.pick_disk_as_bootable(root_disk)
return
# Exclude all disks which names match to regex in rules
# "exclude_disks_by_name"
bootable_disks = list(self.disks)
for rule in self.rule_to_pick_boot:
if rule['type'] != 'exclude_disks_by_name':
continue
regex = re.compile(rule['regex'])
bootable_disks = filter(lambda disk: not regex.match(disk.name),
bootable_disks)
if bootable_disks:
self.__logger('Disk %s is picked as bootable'
'' % bootable_disks[0].name)
self.pick_disk_as_bootable(bootable_disks[0])
# NOTE(sslypushenko) We should pick at least something
else:
self.__logger('It looks like, there are no disks suitable for '
'boot.')
self.pick_disk_as_bootable(self.disks[0])
@property
def _all_disks_free_space(self):
return sum([d.free_space for d in self.disks])

6
nailgun/nailgun/extensions/volume_manager/objects/adapters.py
View File

@ -41,6 +41,12 @@ class NailgunNodeAdapter(object):
return get_node_spaces(self.node)
return []
def get_rule_to_pick_boot(self):
from ..manager import get_rule_to_pick_boot
if self.node.cluster:
return get_rule_to_pick_boot(self.node)
return []
@property
def disks(self):
return self.node.meta['disks']

128
nailgun/nailgun/extensions/volume_manager/tests/test_node_disks.py
View File

@ -367,6 +367,106 @@ class TestNodeDisksHandlers(BaseIntegrationTest):
for partition_after in partitions_after_update:
self.assertEqual(partition_after['size'], new_volume_size)
def test_boot_flag(self):
disks = [{"name": "sda", "disk": "sda", "size": 2 ** 40},
{"name": "sdb", "disk": "sdb", "size": 2 ** 40}]
self.env.create(
nodes_kwargs=[{
"roles": ['controller'],
"pending_roles": [],
"meta": {"disks": disks}
}]
)
node_db = self.env.nodes[0]
# By default first disk (sda) should be picked as bootable
disks = self.get(node_db.id)
sda = next(disk for disk in disks if disk['id'] == 'sda')
sdb = next(disk for disk in disks if disk['id'] == 'sdb')
self.assertTrue(sda['bootable'])
self.assertFalse(sdb['bootable'])
# Pick sdb as a bootable disk
sda['bootable'] = False
sdb['bootable'] = True
self.put(node_db.id, disks)
disks = self.get(node_db.id)
sda = next(disk for disk in disks if disk['id'] == 'sda')
sdb = next(disk for disk in disks if disk['id'] == 'sdb')
self.assertFalse(sda['bootable'])
self.assertTrue(sdb['bootable'])
# Pick default bootable disk
sda['bootable'] = False
sdb['bootable'] = False
self.put(node_db.id, disks)
disks = self.get(node_db.id)
sda = next(disk for disk in disks if disk['id'] == 'sda')
sdb = next(disk for disk in disks if disk['id'] == 'sdb')
self.assertTrue(sda['bootable'])
self.assertFalse(sdb['bootable'])
# Two disks can not be picked as bootable at the same time
sda['bootable'] = True
sdb['bootable'] = True
response = self.put(node_db.id, disks, expect_errors=True)
self.assertEqual(response.status_code, 400)
self.assertEqual(
'Disks sda, sdb can not be selected as bootable at the same time. '
'Only one disk should be selected as bootable.',
response.json_body["message"])
def test_boot_flag_default_pick_nvme(self):
# NVMe disks should not be picked as bootable by default
disks = [{"name": "nvme0", "disk": "nvme0", "size": 2 ** 40},
{"name": "sda", "disk": "sda", "size": 2 ** 40}]
self.env.create(
nodes_kwargs=[{
"roles": ['controller'],
"pending_roles": [],
"meta": {"disks": disks}
}]
)
node_db = self.env.nodes[0]
# By default first disk (sda) should be picked as bootable
disks = self.get(node_db.id)
sda = next(disk for disk in disks if disk['id'] == 'sda')
nvme0 = next(disk for disk in disks if disk['id'] == 'nvme0')
self.assertTrue(sda['bootable'])
self.assertFalse(nvme0['bootable'])
def test_boot_flag_default_pick_md(self):
# If 'Base OS' installed on fake raid, it should picked as bootable
disks = [{"name": "md0", "disk": "md0", "size": 2 ** 40},
{"name": "hda", "disk": "hda", "size": 2 ** 40}]
self.env.create(
nodes_kwargs=[{
"roles": ['controller'],
"pending_roles": [],
"meta": {"disks": disks}
}]
)
node_db = self.env.nodes[0]
disks = self.get(node_db.id)
hda = next(disk for disk in disks if disk['id'] == 'hda')
md0 = next(disk for disk in disks if disk['id'] == 'md0')
self.assertTrue(hda['bootable'])
self.assertFalse(md0['bootable'])
# Move 'Base OS' to the fake raid
md0_os = next(v for v in md0['volumes'] if v['name'] == 'os')
if md0_os['size'] == 0:
md0_image = next(v for v in md0['volumes'] if v['name'] == 'image')
md0_image['size'] -= 50000
md0_os['size'] += 50000
hda_os = next(v for v in hda['volumes'] if v['name'] == 'os')
hda_os['size'] = 0
for disk in disks:
disk['bootable'] = False
self.put(node_db.id, disks)
# Now default pick should be md0
disks = self.get(node_db.id)
hda = next(disk for disk in disks if disk['id'] == 'hda')
md0 = next(disk for disk in disks if disk['id'] == 'md0')
self.assertFalse(hda['bootable'])
self.assertTrue(md0['bootable'])
def test_validator_at_least_one_disk_exists(self):
node = self.create_node()
response = self.put(node.id, [], True)
@ -414,6 +514,30 @@ class TestNodeDisksHandlers(BaseIntegrationTest):
"^All volumes with the same name should"
" have the same value for `keep_data` flag")
def test_validator_single_boot_disk(self):
disks = [{"name": "sda", "disk": "sda", "size": 2 ** 40},
{"name": "sdb", "disk": "sdb", "size": 2 ** 40}]
self.env.create(
nodes_kwargs=[{
"roles": ['controller'],
"pending_roles": [],
"meta": {"disks": disks}
}]
)
node_db = self.env.nodes[0]
disks = self.get(node_db.id)
sda = next(disk for disk in disks if disk['id'] == 'sda')
sdb = next(disk for disk in disks if disk['id'] == 'sdb')
# Two disks can not be picked as bootable at the same time
sda['bootable'] = True
sdb['bootable'] = True
response = self.put(node_db.id, disks, expect_errors=True)
self.assertEqual(response.status_code, 400)
self.assertEqual(
'Disks sda, sdb can not be selected as bootable at the same time. '
'Only one disk should be selected as bootable.',
response.json_body["message"])
class TestNodeDefaultsDisksHandler(BaseIntegrationTest):
@ -1038,6 +1162,10 @@ class TestVolumeManager(BaseIntegrationTest):
{'id': 'os', 'type': 'vg'},
{'id': 'image', 'type': 'vg'},
{'id': 'vm', 'type': 'vg'}
],
'rule_to_pick_boot_disk':
[
{'type': 'exclude_disks_by_name', 'regex': '^nvme'}
]
}
release = self.env.create_release(

1
nailgun/nailgun/extensions/volume_manager/tests/test_pipelines.py
View File

@ -131,6 +131,7 @@ class TestDeploymentAttributesSerialization80(
],
u'type': u'disk',
u'id': u'sda',
u'bootable': True,
u'size': 958
},
{

11
nailgun/nailgun/extensions/volume_manager/validators/disks.py
View File

@ -33,6 +33,7 @@ class NodeDisksValidator(BasicValidator):
# https://bugs.launchpad.net/fuel/+bug/1308592
if NailgunNodeAdapter(node).is_ubuntu:
cls.os_vg_single_disk(dict_data)
cls.check_single_disk_bootable(dict_data)
return dict_data
@classmethod
@ -80,3 +81,13 @@ class NodeDisksValidator(BasicValidator):
raise errors.InvalidData(u'All volumes with the same name should'
u' have the same value for `keep_data` '
u'flag, incorrect volumes: {0}'.format(s))
@classmethod
def check_single_disk_bootable(cls, data):
bootable_disks = [disk['id'] for disk in data
if disk.get('bootable', False)]
if len(bootable_disks) > 1:
raise errors.InvalidData(
u'Disks {} can not be selected as bootable at the same time. '
u'Only one disk should be selected as bootable.'
u''.format(', '.join(bootable_disks)))

4
nailgun/nailgun/extensions/volume_manager/validators/json_schema/disks.py
View File

@ -32,6 +32,10 @@ disks_simple_format_schema = {
'description': 'Disk size in megabytes',
'type': 'integer'
},
'bootable': {
'description': 'Flag to use this disk as bootable',
'type': 'boolean'
},
'volumes': {
'description': 'Volumes for disk',
'type': 'array',

8
nailgun/nailgun/fixtures/openstack.yaml
View File

@ -719,6 +719,14 @@
min_size: { generator: "calc_min_cinder_size" }
mount: "none"
volumes: []
rule_to_pick_boot_disk:
- type: "exclude_disks_by_name"
regex: "^nvme"
description: "NVMe drives should be skipped as accessing such drives during the boot typically requires using UEFI which is still not supported by fuel-agent (it always installs BIOS variant of grub). grub bug (http://savannah.gnu.org/bugs/?41883)"
- type: "pick_root_disk_if_disk_name_match"
regex: "^md"
root_mount: "/"
description: "If we have /root on fake raid, then /boot partition should land on to it too. We can't proceed with grub-install otherwise."
attributes_metadata:
editable:
access:

2
nailgun/nailgun/objects/cluster.py
View File

@ -1120,6 +1120,8 @@ class Cluster(NailgunObject):
plugin_volumes['volumes_roles_mapping'])
volumes_metadata['volumes'].extend(plugin_volumes['volumes'])
volumes_metadata['rule_to_pick_boot_disk'].extend(
plugin_volumes['rule_to_pick_boot_disk'])
return volumes_metadata

5
nailgun/nailgun/plugins/manager.py
View File

@ -286,7 +286,8 @@ class PluginManager(object):
"""
volumes_metadata = {
'volumes': [],
'volumes_roles_mapping': {}
'volumes_roles_mapping': {},
'rule_to_pick_boot_disk': [],
}
release_volumes = cluster.release.volumes_metadata.get('volumes', [])
release_volumes_ids = [v['id'] for v in release_volumes]
@ -317,6 +318,8 @@ class PluginManager(object):
metadata.get('volumes_roles_mapping', {}))
volumes_metadata.get('volumes', []).extend(
metadata.get('volumes', []))
volumes_metadata.get('rule_to_pick_boot_disk', []).extend(
metadata.get('rule_to_pick_boot_disk', []))
return volumes_metadata

3
nailgun/nailgun/test/integration/test_plugin_manager.py
View File

@ -85,7 +85,8 @@ class TestPluginManager(base.BaseIntegrationTest):
)
volumes_metadata = PluginManager.get_volumes_metadata(cluster)
expected_volumes_metadata = {
'volumes_roles_mapping': {}, 'volumes': []}
'volumes_roles_mapping': {}, 'volumes': [],
'rule_to_pick_boot_disk': []}
self.assertEqual(
volumes_metadata, expected_volumes_metadata)

3
nailgun/nailgun/test/unit/test_objects.py
View File

@ -1466,7 +1466,8 @@ class TestClusterObject(BaseTestCase):
'volumes': [
{'id': 'test_plugin_1', 'type': 'vg'},
{'id': 'test_plugin_2', 'type': 'vg'}
]
],
'rule_to_pick_boot_disk': []
}
with mock.patch.object(
PluginManager, 'get_volumes_metadata') as plugin_volumes: