From 2df11392ab6f28d828d48f2151f4903da888d216 Mon Sep 17 00:00:00 2001
From: sslypushenko <sslypushenko@mirantis.com>
Date: Wed, 11 May 2016 19:54:42 +0300
Subject: [PATCH] Add bootable disk node flag

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
---
 .../extensions/volume_manager/manager.py      | 106 ++++++++++++++-
 .../volume_manager/objects/adapters.py        |   6 +
 .../volume_manager/tests/test_node_disks.py   | 128 ++++++++++++++++++
 .../volume_manager/tests/test_pipelines.py    |   1 +
 .../volume_manager/validators/disks.py        |  11 ++
 .../validators/json_schema/disks.py           |   4 +
 nailgun/nailgun/fixtures/openstack.yaml       |   8 ++
 nailgun/nailgun/objects/cluster.py            |   2 +
 nailgun/nailgun/plugins/manager.py            |   5 +-
 .../test/integration/test_plugin_manager.py   |   3 +-
 nailgun/nailgun/test/unit/test_objects.py     |   3 +-
 11 files changed, 268 insertions(+), 9 deletions(-)

diff --git a/nailgun/nailgun/extensions/volume_manager/manager.py b/nailgun/nailgun/extensions/volume_manager/manager.py
index b234fb87c3..a1b73d010b 100644
--- a/nailgun/nailgun/extensions/volume_manager/manager.py
+++ b/nailgun/nailgun/extensions/volume_manager/manager.py
@@ -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])
diff --git a/nailgun/nailgun/extensions/volume_manager/objects/adapters.py b/nailgun/nailgun/extensions/volume_manager/objects/adapters.py
index 60e30323ce..6a555e65a4 100644
--- a/nailgun/nailgun/extensions/volume_manager/objects/adapters.py
+++ b/nailgun/nailgun/extensions/volume_manager/objects/adapters.py
@@ -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']
diff --git a/nailgun/nailgun/extensions/volume_manager/tests/test_node_disks.py b/nailgun/nailgun/extensions/volume_manager/tests/test_node_disks.py
index d2fbf3c3fc..10a4659166 100644
--- a/nailgun/nailgun/extensions/volume_manager/tests/test_node_disks.py
+++ b/nailgun/nailgun/extensions/volume_manager/tests/test_node_disks.py
@@ -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(
diff --git a/nailgun/nailgun/extensions/volume_manager/tests/test_pipelines.py b/nailgun/nailgun/extensions/volume_manager/tests/test_pipelines.py
index 08b4ed2730..715ae4240b 100644
--- a/nailgun/nailgun/extensions/volume_manager/tests/test_pipelines.py
+++ b/nailgun/nailgun/extensions/volume_manager/tests/test_pipelines.py
@@ -131,6 +131,7 @@ class TestDeploymentAttributesSerialization80(
                 ],
                 u'type': u'disk',
                 u'id': u'sda',
+                u'bootable': True,
                 u'size': 958
             },
             {
diff --git a/nailgun/nailgun/extensions/volume_manager/validators/disks.py b/nailgun/nailgun/extensions/volume_manager/validators/disks.py
index 4d8c611710..e46863c190 100644
--- a/nailgun/nailgun/extensions/volume_manager/validators/disks.py
+++ b/nailgun/nailgun/extensions/volume_manager/validators/disks.py
@@ -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)))
diff --git a/nailgun/nailgun/extensions/volume_manager/validators/json_schema/disks.py b/nailgun/nailgun/extensions/volume_manager/validators/json_schema/disks.py
index 9b19a808f0..56c3a12f5d 100644
--- a/nailgun/nailgun/extensions/volume_manager/validators/json_schema/disks.py
+++ b/nailgun/nailgun/extensions/volume_manager/validators/json_schema/disks.py
@@ -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',
diff --git a/nailgun/nailgun/fixtures/openstack.yaml b/nailgun/nailgun/fixtures/openstack.yaml
index 5f6c357f8f..cfdf9882a9 100644
--- a/nailgun/nailgun/fixtures/openstack.yaml
+++ b/nailgun/nailgun/fixtures/openstack.yaml
@@ -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:
diff --git a/nailgun/nailgun/objects/cluster.py b/nailgun/nailgun/objects/cluster.py
index e1effd425c..4a12ebfcfc 100644
--- a/nailgun/nailgun/objects/cluster.py
+++ b/nailgun/nailgun/objects/cluster.py
@@ -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
 
diff --git a/nailgun/nailgun/plugins/manager.py b/nailgun/nailgun/plugins/manager.py
index 73005c6ca2..da3b338e91 100644
--- a/nailgun/nailgun/plugins/manager.py
+++ b/nailgun/nailgun/plugins/manager.py
@@ -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
 
diff --git a/nailgun/nailgun/test/integration/test_plugin_manager.py b/nailgun/nailgun/test/integration/test_plugin_manager.py
index f21b7f98c9..a95626b2ca 100644
--- a/nailgun/nailgun/test/integration/test_plugin_manager.py
+++ b/nailgun/nailgun/test/integration/test_plugin_manager.py
@@ -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)
diff --git a/nailgun/nailgun/test/unit/test_objects.py b/nailgun/nailgun/test/unit/test_objects.py
index 6357217a0f..a9082b1941 100644
--- a/nailgun/nailgun/test/unit/test_objects.py
+++ b/nailgun/nailgun/test/unit/test_objects.py
@@ -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: