switch to tracking install device by setting grub_device

subiquity/controllers/filesystem.py

@@ -169,17 +169,7 @@ class FilesystemController(SubiquityController):
         elif 'config' in self.ai_data:
             with self.context.child("applying_autoinstall"):
                 self.model.apply_autoinstall_config(self.ai_data['config'])
-            grub = self.ai_data.get('grub', {})
-            install_devices = grub.get('install_devices')
-            if install_devices:
-                device = install_devices[0]
-                for action in self.model._actions:
-                    if action.id == device:
-                        self.model.grub_install_device = action
-                        break
-                else:
-                    raise Exception(
-                        "failed to find install_device {!r}".format(device))
+            self.model.grub = self.ai_data.get('grub', {})
             self.model.swap = self.ai_data.get('swap')
 
     def start(self):
@@ -433,8 +423,10 @@ class FilesystemController(SubiquityController):
         self.model.remove_filesystem(fs)
     delete_format = delete_filesystem
 
-    def create_partition(self, device, spec, flag="", wipe=None):
-        part = self.model.add_partition(device, spec["size"], flag, wipe)
+    def create_partition(self, device, spec, flag="", wipe=None,
+                         grub_device=None):
+        part = self.model.add_partition(
+            device, spec["size"], flag, wipe, grub_device)
         self.create_filesystem(part, spec)
         return part
 
@@ -452,7 +444,7 @@ class FilesystemController(SubiquityController):
             part = self.create_partition(
                 disk,
                 dict(size=part_size, fstype='fat32', mount='/boot/efi'),
-                flag="boot")
+                flag="boot", grub_device=True)
         elif bootloader == Bootloader.PREP:
             log.debug('_create_boot_partition - adding PReP partition')
             part = self.create_partition(
@@ -460,15 +452,14 @@ class FilesystemController(SubiquityController):
                 dict(size=PREP_GRUB_SIZE_BYTES, fstype=None, mount=None),
                 # must be wiped or grub-install will fail
                 wipe='zero',
-                flag='prep')
-            self.model.grub_install_device = part
+                flag='prep', grub_device=True)
         elif bootloader == Bootloader.BIOS:
             log.debug('_create_boot_partition - adding bios_grub partition')
             part = self.create_partition(
                 disk,
                 dict(size=BIOS_GRUB_SIZE_BYTES, fstype=None, mount=None),
                 flag='bios_grub')
-            self.model.grub_install_device = disk
+            disk.grub_device = True
         return part
 
     def create_raid(self, spec):
@@ -539,8 +530,7 @@ class FilesystemController(SubiquityController):
             self.delete(subobj)
 
     def reformat(self, disk):
-        if disk is self.model.grub_install_device:
-            self.model.grub_install_device = None
+        disk.grub_device = False
         for p in list(disk.partitions()):
             self.delete_partition(p)
         self.clear(disk)
@@ -641,21 +631,25 @@ class FilesystemController(SubiquityController):
     def make_boot_disk(self, new_boot_disk):
         boot_partition = None
         if self.model.bootloader == Bootloader.BIOS:
-            install_dev = self.model.grub_install_device
+            install_dev = self.model._one(type="disk", grub_device=True)
             if install_dev:
+                install_dev.grub_device = False
                 boot_partition = install_dev._potential_boot_partition()
         elif self.model.bootloader == Bootloader.UEFI:
             mount = self.model._mount_for_path("/boot/efi")
             if mount is not None:
                 boot_partition = mount.device.volume
         elif self.model.bootloader == Bootloader.PREP:
-            boot_partition = self.model.grub_install_device
+            boot_partition = self.model._one(
+                type="partition", flag="prep", grub_device=True)
         if boot_partition is not None:
             if boot_partition.preserve:
                 if self.model.bootloader == Bootloader.PREP:
                     boot_partition.wipe = None
+                    boot_partition.grub_device = False
                 elif self.model.bootloader == Bootloader.UEFI:
                     self.delete_mount(boot_partition.fs().mount())
+                    boot_partition.grub_device = False
             else:
                 boot_disk = boot_partition.device
                 full = boot_disk.free_for_partitions == 0
@@ -672,16 +666,17 @@ class FilesystemController(SubiquityController):
                         new_boot_disk.free_for_partitions)
         if new_boot_disk._has_preexisting_partition():
             if self.model.bootloader == Bootloader.BIOS:
-                self.model.grub_install_device = new_boot_disk
+                new_boot_disk.grub_device = True
             elif self.model.bootloader == Bootloader.UEFI:
                 part = new_boot_disk._potential_boot_partition()
+                part.grub_device = True
                 if part.fs() is None:
                     self.model.add_filesystem(part, 'fat32')
                 self.model.add_mount(part.fs(), '/boot/efi')
             elif self.model.bootloader == Bootloader.PREP:
                 part = new_boot_disk._potential_boot_partition()
                 part.wipe = 'zero'
-                self.model.grub_install_device = part
+                part.grub_device = True
         else:
             new_boot_disk.preserve = False
             self._create_boot_partition(new_boot_disk)
@@ -733,8 +728,4 @@ class FilesystemController(SubiquityController):
             }
         if 'swap' in rendered:
             r['swap'] = rendered['swap']
-        if self.model.grub_install_device:
-            r['grub'] = {
-                'install_devices': [self.model.grub_install_device.id],
-                }
         return r

subiquity/controllers/tests/test_filesystem.py

@@ -20,7 +20,6 @@ from subiquity.controllers.filesystem import (
     FilesystemController,
     )
 from subiquity.models.tests.test_filesystem import (
-    fake_up_blockdata,
     make_disk,
     make_model,
     )
@@ -100,7 +99,7 @@ class TestFilesystemController(unittest.TestCase):
         controller.make_boot_disk(disk1)
         self.assertEqual(len(disk1.partitions()), 1)
         self.assertEqual(disk1.partitions()[0].flag, "bios_grub")
-        self.assertEqual(controller.model.grub_install_device, disk1)
+        self.assertTrue(disk1.grub_device)
 
         size_before = disk2p1.size
         controller.make_boot_disk(disk2)
@@ -110,7 +109,8 @@ class TestFilesystemController(unittest.TestCase):
         self.assertEqual(
             disk2.partitions()[0].size + disk2p1.size, size_before)
         self.assertEqual(disk2.partitions()[0].flag, "bios_grub")
-        self.assertEqual(controller.model.grub_install_device, disk2)
+        self.assertFalse(disk1.grub_device)
+        self.assertTrue(disk2.grub_device)
 
     def test_make_boot_disk_BIOS_existing(self):
         controller = make_controller(Bootloader.BIOS)
@@ -121,14 +121,15 @@ class TestFilesystemController(unittest.TestCase):
         disk2 = make_disk(controller.model, preserve=False)
 
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertFalse(disk1.grub_device)
         controller.make_boot_disk(disk1)
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, disk1)
+        self.assertTrue(disk1.grub_device)
 
         controller.make_boot_disk(disk2)
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, disk2)
+        self.assertFalse(disk1.grub_device)
+        self.assertTrue(disk2.grub_device)
 
     def test_make_boot_disk_UEFI(self):
         controller = make_controller(Bootloader.UEFI)
@@ -140,7 +141,7 @@ class TestFilesystemController(unittest.TestCase):
         controller.make_boot_disk(disk1)
         self.assertEqual(len(disk1.partitions()), 1)
         self.assertEqual(disk1.partitions()[0].flag, "boot")
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertTrue(disk1.partitions()[0].grub_device)
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt.device.volume, disk1.partitions()[0])
         self.assertEqual(disk1.partitions()[0].fs().fstype, "fat32")
@@ -153,7 +154,7 @@ class TestFilesystemController(unittest.TestCase):
         self.assertEqual(
             disk2.partitions()[0].size + disk2p1.size, size_before)
         self.assertEqual(disk2.partitions()[0].flag, "boot")
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertTrue(disk2.partitions()[0].grub_device)
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt.device.volume, disk2.partitions()[0])
 
@@ -166,19 +167,20 @@ class TestFilesystemController(unittest.TestCase):
         disk2 = make_disk(controller.model, preserve=True)
 
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertFalse(disk1p1.grub_device)
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt, None)
         controller.make_boot_disk(disk1)
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertTrue(disk1p1.grub_device)
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt.device.volume, disk1p1)
         self.assertEqual(disk1p1.fs().fstype, "fat32")
 
         controller.make_boot_disk(disk2)
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertFalse(disk1p1.grub_device)
+        self.assertTrue(disk2.partitions()[0].grub_device)
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt.device.volume, disk2.partitions()[0])
 
@@ -193,9 +195,7 @@ class TestFilesystemController(unittest.TestCase):
         self.assertEqual(len(disk1.partitions()), 1)
         self.assertEqual(disk1.partitions()[0].flag, "prep")
         self.assertEqual(disk1.partitions()[0].wipe, "zero")
-        self.assertEqual(
-            controller.model.grub_install_device,
-            disk1.partitions()[0])
+        self.assertTrue(disk1.partitions()[0].grub_device)
 
         size_before = disk2p1.size
         controller.make_boot_disk(disk2)
@@ -206,9 +206,7 @@ class TestFilesystemController(unittest.TestCase):
             disk2.partitions()[0].size + disk2p1.size, size_before)
         self.assertEqual(disk2.partitions()[0].flag, "prep")
         self.assertEqual(disk2.partitions()[0].wipe, "zero")
-        self.assertEqual(
-            controller.model.grub_install_device,
-            disk2.partitions()[0])
+        self.assertTrue(disk2.partitions()[0].grub_device)
 
     def test_make_boot_disk_PREP_existing(self):
         controller = make_controller(Bootloader.PREP)
@@ -219,21 +217,18 @@ class TestFilesystemController(unittest.TestCase):
         disk2 = make_disk(controller.model, preserve=False)
 
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, None)
+        self.assertFalse(disk1p1.grub_device)
         controller.make_boot_disk(disk1)
         self.assertEqual(disk1.partitions(), [disk1p1])
-        self.assertEqual(controller.model.grub_install_device, disk1p1)
+        self.assertTrue(disk1p1.grub_device)
         self.assertEqual(disk1p1.wipe, 'zero')
 
         controller.make_boot_disk(disk2)
         self.assertEqual(disk1.partitions(), [disk1p1])
         self.assertEqual(disk1p1.wipe, None)
-        self.assertEqual(
-            controller.model.grub_install_device, disk2.partitions()[0])
         self.assertEqual(disk2.partitions()[0].flag, "prep")
-        self.assertEqual(
-            controller.model.grub_install_device,
-            disk2.partitions()[0])
+        self.assertFalse(disk1p1.grub_device)
+        self.assertTrue(disk2.partitions()[0].grub_device)
 
     def test_mounting_partition_makes_boot_disk(self):
         controller = make_controller(Bootloader.UEFI)
@@ -248,25 +243,3 @@ class TestFilesystemController(unittest.TestCase):
             disk1, disk1p2, {'fstype': 'ext4', 'mount': '/'})
         efi_mnt = controller.model._mount_for_path("/boot/efi")
         self.assertEqual(efi_mnt.device.volume, disk1p1)
-
-    def test_autoinstall_grub_devices(self):
-        controller = make_controller(Bootloader.BIOS)
-        make_disk(controller.model)
-        fake_up_blockdata(controller.model)
-        controller.ai_data = {
-            'config': [{'type': 'disk', 'id': 'disk0'}],
-            'grub': {
-                'install_devices': ['disk0'],
-                },
-            }
-        controller.convert_autoinstall_config()
-        new_disk = controller.model._one(type="disk", id="disk0")
-        self.assertEqual(controller.model.grub_install_device, new_disk)
-
-    def test_make_autoinstall(self):
-        controller = make_controller(Bootloader.BIOS)
-        disk = make_disk(controller.model)
-        fake_up_blockdata(controller.model)
-        controller.guided_direct(disk)
-        ai_data = controller.make_autoinstall()
-        self.assertEqual(ai_data['grub']['install_devices'], [disk.id])

subiquity/models/filesystem.py

@@ -25,7 +25,6 @@ import os
 import pathlib
 import platform
 
-from curtin.block import partition_kname
 from curtin import storage_config
 from curtin.util import human2bytes
 
@@ -847,16 +846,16 @@ class Disk(_Device):
     def _can_TOGGLE_BOOT(self):
         bl = self._m.bootloader
         if bl == Bootloader.BIOS:
-            if self._m.grub_install_device is self:
+            if self.grub_device:
                 return False
         elif bl == Bootloader.UEFI:
             m = self._m._mount_for_path('/boot/efi')
             if m and m.device.volume.device is self:
                 return False
         elif bl == Bootloader.PREP:
-            install_dev = self._m.grub_install_device
-            if install_dev is not None and install_dev.device is self:
-                return False
+            for p in self._partitions:
+                if p.flag == "prep" and p.grub_device:
+                    return False
         if self._has_preexisting_partition():
             return self._can_be_boot_disk()
         else:
@@ -886,6 +885,7 @@ class Partition(_Formattable):
     flag = attr.ib(default=None)
     number = attr.ib(default=None)
     preserve = attr.ib(default=False)
+    grub_device = attr.ib(default=False)
 
     @property
     def annotations(self):
@@ -919,7 +919,7 @@ class Partition(_Formattable):
         return _("partition {}").format(self._number)
 
     def available(self):
-        if self.flag in ['bios_grub', 'prep']:
+        if self.flag in ['bios_grub', 'prep'] or self.grub_device:
             return False
         if self._constructed_device is not None:
             return False
@@ -1278,8 +1278,8 @@ class FilesystemModel(object):
         else:
             self._orig_config = []
             self._actions = []
-        self.grub_install_device = None
         self.swap = None
+        self.grub = None
 
     def _make_matchers(self, match):
         matchers = []
@@ -1545,16 +1545,8 @@ class FilesystemModel(object):
             }
         if self.swap is not None:
             config['swap'] = self.swap
-        if self.grub_install_device:
-            dev = self.grub_install_device
-            if dev.type == "partition":
-                disk_kname = dev.device.path[5:]  # chop off "/dev/"
-                devpath = "/dev/" + partition_kname(disk_kname, dev._number)
-            else:
-                devpath = dev.path
-            config['grub'] = {
-                'install_devices': [devpath],
-                }
+        if self.grub is not None:
+            config['grub'] = self.grub
         return config
 
     def load_probe_data(self, probe_data):
@@ -1608,12 +1600,11 @@ class FilesystemModel(object):
         return self._all(type='lvm_volgroup')
 
     def _remove(self, obj):
-        if obj is self.grub_install_device:
-            self.grub_install_device = None
         _remove_backlinks(obj)
         self._actions.remove(obj)
 
-    def add_partition(self, device, size, flag="", wipe=None):
+    def add_partition(self, device, size, flag="", wipe=None,
+                      grub_device=None):
         if size > device.free_for_partitions:
             raise Exception("%s > %s", size, device.free_for_partitions)
         real_size = align_up(size)
@@ -1621,7 +1612,8 @@ class FilesystemModel(object):
         if device._fs is not None:
             raise Exception("%s is already formatted" % (device.label,))
         p = Partition(
-            m=self, device=device, size=real_size, flag=flag, wipe=wipe)
+            m=self, device=device, size=real_size, flag=flag, wipe=wipe,
+            grub_device=grub_device)
         if flag in ("boot", "bios_grub", "prep"):
             device._partitions.insert(0, device._partitions.pop())
         device.ptable = device.ptable_for_new_partition()
@@ -1725,10 +1717,16 @@ class FilesystemModel(object):
         # s390x has no such thing
         if self.bootloader == Bootloader.NONE:
             return False
-        elif self.bootloader in [Bootloader.BIOS, Bootloader.PREP]:
-            return self.grub_install_device is None
+        elif self.bootloader == Bootloader.BIOS:
+            return self._one(type='disk', grub_device=True) is None
         elif self.bootloader == Bootloader.UEFI:
-            return self._mount_for_path('/boot/efi') is None
+            for esp in self._all(type='partition', grub_device=True):
+                if esp.fs() and esp.fs().mount():
+                    if esp.fs().mount().path == '/boot/efi':
+                        return False
+            return True
+        elif self.bootloader == Bootloader.PREP:
+            return self._one(type='partition', grub_device=True) is None
         else:
             raise AssertionError(
                 "unknown bootloader type {}".format(self.bootloader))

subiquity/ui/views/filesystem/tests/test_filesystem.py

@@ -24,7 +24,6 @@ class FilesystemViewTests(unittest.TestCase):
         controller.ui = mock.Mock()
         model.bootloader = Bootloader.NONE
         model.all_devices.return_value = devices
-        model.grub_install_device = None
         return FilesystemView(model, controller)
 
     def test_simple(self):

subiquity/ui/views/filesystem/tests/test_partition.py

@@ -215,6 +215,7 @@ class PartitionViewTests(unittest.TestCase):
         partition = model.add_partition(disk, 512*(2**20), "boot")
         fs = model.add_filesystem(partition, "fat32")
         model._orig_config = model._render_actions()
+        partition.grub_device = True
         disk.preserve = partition.preserve = fs.preserve = True
         model.add_mount(fs, '/boot/efi')
         view, stretchy = make_partition_view(model, disk, partition)