Merge pull request #502 from rvykydal/home-reuse-existing-mountpoints…

…-check

home reuse: add check for unexpected existing mount points

src/apis/storage_partitioning.js

@@ -95,20 +95,16 @@ export const partitioningSetEncrypt = ({ encrypt, partitioning }) => {
             });
 };
 
-/**
- * @param {string} partitioning     DBus path to a partitioning
- * @param {string} scheme           autopartitioning scheme
- */
-export const partitioningSetHomeReuse = async ({ partitioning, scheme }) => {
-    const request = await getPartitioningRequest({ partitioning });
-
+export const getAutopartReuseDBusRequest = (scheme) => {
     const configurationSchemeToDBus = {
         BTRFS: cockpit.variant("i", 1),
         LVM: cockpit.variant("i", 2),
         LVM_THINP: cockpit.variant("i", 3),
         PLAIN: cockpit.variant("i", 0),
     };
-    request["partitioning-scheme"] = configurationSchemeToDBus?.[scheme];
+    const request = {
+        "partitioning-scheme": configurationSchemeToDBus?.[scheme],
+    };
 
     request["reused-mount-points"] = cockpit.variant("as", ["/home"]);
     if (scheme === "PLAIN") {
@@ -120,6 +116,17 @@ export const partitioningSetHomeReuse = async ({ partitioning, scheme }) => {
         request["removed-mount-points"] = cockpit.variant("as", ["/boot", "bootloader"]);
         request["reformatted-mount-points"] = cockpit.variant("as", ["/"]);
     }
+    return request;
+};
+
+/**
+ * @param {string} partitioning     DBus path to a partitioning
+ * @param {string} scheme           autopartitioning scheme
+ */
+export const partitioningSetHomeReuse = async ({ partitioning, scheme }) => {
+    const autopartRequest = await getPartitioningRequest({ partitioning });
+    const reuseRequest = getAutopartReuseDBusRequest(scheme);
+    const request = { ...autopartRequest, ...reuseRequest };
 
     await setPartitioningRequest({ partitioning, request });
 };

src/components/storage/InstallationScenario.jsx

@@ -26,10 +26,13 @@ import {
     Title,
 } from "@patternfly/react-core";
 
+import { getAutopartReuseDBusRequest } from "../../apis/storage_partitioning.js";
+
 import { setStorageScenarioAction } from "../../actions/storage-actions.js";
 
 import { debug } from "../../helpers/log.js";
 import {
+    bootloaderTypes,
     getDeviceAncestors,
     getLockedLUKSDevices,
 } from "../../helpers/storage.js";
@@ -154,6 +157,19 @@ const checkHomeReuse = ({ autopartScheme, devices, originalExistingSystems, sele
         return missingDisks.length === 0;
     };
 
+    const getUnknownMountPoints = (scheme, existingOS) => {
+        const reuseRequest = getAutopartReuseDBusRequest(scheme);
+        const isBootloader = (device) => bootloaderTypes.includes(devices[device].formatData.type.v);
+        const existingMountPoints = Object.entries(existingOS["mount-points"].v)
+                .map(([mountPoint, device]) => isBootloader(device) ? "bootloader" : mountPoint);
+
+        const managedMountPoints = reuseRequest["reformatted-mount-points"].v
+                .concat(reuseRequest["reused-mount-points"].v, reuseRequest["removed-mount-points"].v);
+
+        const unknownMountPoints = existingMountPoints.filter(i => !managedMountPoints.includes(i));
+        return unknownMountPoints;
+    };
+
     // Check that exactly one Linux OS is present and it is Fedora Linux
     // (Stronger check for mountpoints uniqueness is in the backend
     const linuxSystems = originalExistingSystems.filter(osdata => osdata["os-name"].v.includes("Linux"))
@@ -175,6 +191,7 @@ const checkHomeReuse = ({ autopartScheme, devices, originalExistingSystems, sele
         }
     }
 
+    debug(`home reuse: Default scheme is ${autopartScheme}.`);
     if (reusedOS) {
         // Check that required autopartitioning scheme matches reused OS.
         // Check just "/home". To be more generic we could check all reused devices (as the backend).
@@ -193,6 +210,17 @@ const checkHomeReuse = ({ autopartScheme, devices, originalExistingSystems, sele
         }
     }
 
+    if (reusedOS) {
+        // Check that existing system does not have mountpoints unexpected
+        // by the required autopartitioning scheme
+        const unknownMountPoints = getUnknownMountPoints(autopartScheme, reusedOS);
+        if (unknownMountPoints.length > 0) {
+            availability.available = false;
+            availability.hidden = true;
+            console.info(`home reuse: Unknown existing mountpoints found ${unknownMountPoints}`);
+        }
+    }
+
     // TODO checks:
     // - luks - partitions are unlocked - enforce? allow opt-out?
     // - size ?

src/helpers/storage.js

@@ -170,3 +170,5 @@ export const unitMultiplier = {
     GB: 1000000000,
     TB: 1000000000000,
 };
+
+export const bootloaderTypes = ["efi", "biosboot", "appleboot", "prepboot"];

test/check-storage-home-reuse

@@ -34,6 +34,22 @@ BOTS_DIR = f'{ROOT_DIR}/bots'
 class TestStorageHomeReuseFedora(VirtInstallMachineCase, StorageCase):
     disk_image = "fedora-rawhide"
 
+    def setUp(self):
+        super().setUp()
+
+        if self._testMethodName == "testHidden":
+            return
+
+        # Remove the /var subvolume from the default btrfs layout
+        # as /var/ is not default mount point in Fedora which results in
+        # the 'Reinstall Fedora' option to get hidden
+        self.machine.execute("""
+            mount /dev/vda4 /mnt;
+            btrfs subvolume delete /mnt/var/lib/machines;
+            btrfs subvolume delete /mnt/var;
+            umount /mnt
+        """)
+
     @run_boot("bios", "efi")
     def testBasic(self):
         b = self.browser
@@ -68,6 +84,21 @@ class TestStorageHomeReuseFedora(VirtInstallMachineCase, StorageCase):
         r.check_disk_row(dev, "/home", f"{dev}4", "12.8 GB", False, "btrfs", is_encrypted=False,
                          action="mount")
 
+    def testHidden(self):
+        b = self.browser
+        m = self.machine
+        i = Installer(b, m, scenario="home-reuse")
+        s = Storage(b, m)
+
+        pretend_default_scheme(self, "BTRFS")
+
+        i.open()
+        i.reach(i.steps.INSTALLATION_METHOD)
+        s.rescan_disks()
+
+        # The `Re-install Fedora` scenario should not be available
+        # when unexpected mount points are present
+        s.wait_scenario_visible("home-reuse", False)
 
     @skipImage("btrfs support missing on fedora-eln image", "fedora-eln-boot")
     def testMultipleRoots(self):

test/check-storage-home-reuse-e2e

@@ -33,6 +33,19 @@ BOTS_DIR = f'{ROOT_DIR}/bots'
 class TestStorageHomeReuse_E2E(VirtInstallMachineCase, StorageCase):
     disk_image = "fedora-rawhide"
 
+    def setUp(self):
+        super().setUp()
+
+        # Remove the /var subvolume from the default btrfs layout
+        # as /var/ is not default mount point in Fedora which results in
+        # the 'Reinstall Fedora' option to get hidden
+        self.machine.execute("""
+            mount /dev/vda4 /mnt;
+            btrfs subvolume delete /mnt/var/lib/machines;
+            btrfs subvolume delete /mnt/var;
+            umount /mnt
+        """)
+
     def install(self, needs_confirmation):
         b = self.browser
         m = self.machine