Merge pull request #183 from KKoukiou/fix-btrfs

storage: do not overwrite request object structure - only extend it

src/components/storage/MountPointMapping.jsx

@@ -146,7 +146,9 @@ const requestsToDbus = (requests) => {
     return requests.map(row => {
         return {
             "device-spec": cockpit.variant("s", row["device-spec"] || ""),
+            "format-options": cockpit.variant("s", row["format-options"] || ""),
             "format-type": cockpit.variant("s", row["format-type"] || ""),
+            "mount-options": cockpit.variant("s", row["mount-options"] || ""),
             "mount-point": cockpit.variant("s", row["mount-point"] || ""),
             reformat: cockpit.variant("b", !!row.reformat),
         };
@@ -390,7 +392,10 @@ const getRequestRow = ({
     mountPointConstraints,
 }) => {
     const columnClassName = idPrefix + "__column";
-    const isRequiredMountPoint = mountPointConstraints.filter(val => val.required.v && val["mount-point"].v === request["mount-point"]).length > 0;
+    const isRequiredMountPoint = (
+        request["mount-point"] !== "" &&
+        mountPointConstraints.filter(val => val.required.v && val["mount-point"].v === request["mount-point"]).length > 0
+    );
     const isRecommendedMountPoint = mountPointConstraints.filter(val => val.recommended.v && val["mount-point"].v === request["mount-point"]).length > 0;
     const duplicatedMountPoint = isDuplicateRequestField(requests, "mount-point", request["mount-point"]);
     const rowId = idPrefix + "-row-" + (requestIndex + 1);
@@ -453,24 +458,21 @@ const getRequestRow = ({
 };
 
 const getNewRequestProps = ({ mountPoint, deviceSpec, reformat, requests }) => {
-    const formatType = requests.find(device => device["device-spec"] === deviceSpec)?.["format-type"];
-    const newProps = {};
+    const existingRequestForDev = requests.find(device => device["device-spec"] === deviceSpec);
+    const newProps = { ...existingRequestForDev };
 
     if (mountPoint !== undefined) {
         newProps["mount-point"] = mountPoint;
     }
     if (deviceSpec !== undefined) {
         newProps["device-spec"] = deviceSpec;
-        if (formatType === "swap") {
+        if (newProps["format-type"] === "swap") {
             newProps["mount-point"] = "swap";
         }
     }
     if (reformat !== undefined) {
         newProps.reformat = !!reformat;
     }
-    if (formatType !== undefined) {
-        newProps["format-type"] = formatType;
-    }
 
     return newProps;
 };
@@ -511,10 +513,15 @@ const RequestsTable = ({
             // Remove a request from the specified index
             newRequests.splice(requestIndex, 1);
         } else {
-            const newRequest = {
-                ...(newRequests[requestIndex] || {}),
-                ...getNewRequestProps({ mountPoint, deviceSpec, reformat, requests })
-            };
+            const newRequestProps = newRequests[requestIndex] || {};
+            const newRequest = (
+                getNewRequestProps({
+                    mountPoint: mountPoint || newRequestProps.mountPoint,
+                    deviceSpec,
+                    reformat: reformat !== undefined ? reformat : newRequestProps.reformat,
+                    requests
+                })
+            );
 
             if (requestIndex === unappliedRequests.length) {
                 // Add new request in the end of the array

test/check-storage

@@ -1292,6 +1292,97 @@ class TestStorageCockpitIntegration(anacondalib.VirtInstallMachineCase, StorageC
         r.check_disk_row(dev, "root, 15.0 GB: mount, /")
         r.check_disk_row(dev, "home, 15.0 GB: mount, /home")
 
+    @nondestructive
+    def testBtrfsAndMountPointAssignment(self):
+        b = self.browser
+        m = self.machine
+        i = Installer(b, m, scenario="use-configured-storage")
+        s = Storage(b, m)
+        r = Review(b)
+
+        tmp_mount = "/btrfs-mount-test"
+
+        i.open()
+        i.reach(i.steps.INSTALLATION_METHOD)
+        s.wait_scenario_visible("use-configured-storage", False)
+        s.check_single_disk_destination("vda")
+        s.modify_storage()
+        b.wait_visible(".cockpit-storage-integration-sidebar")
+
+        frame = "iframe[name='cockpit-storage']"
+        b._wait_present(frame)
+        b.switch_to_frame("cockpit-storage")
+        b._wait_present("#storage.ct-page-fill")
+
+        self.click_dropdown(self.card_row("Storage", 1), "Create partition table")
+        self.confirm()
+
+        self.click_dropdown(self.card_row("Storage", 2), "Create partition")
+        self.dialog({"size": 1, "type": "biosboot"})
+
+        self.click_dropdown(self.card_row("Storage", 3), "Create partition")
+        self.dialog({"size": 1070, "type": "ext4", "mount_point": "/boot"})
+
+        self.click_dropdown(self.card_row("Storage", 4), "Create partition")
+        self.dialog({"type": "btrfs"})
+
+        self.click_card_row("Storage", name="/")
+
+        b.click(self.card_button("btrfs subvolume", "Mount"))
+        self.dialog({"mount_point": tmp_mount})
+
+        b.click(self.card_button("btrfs subvolume", "Create subvolume"))
+        self.dialog({"name": "root", "mount_point": "/"})
+
+        b.click(self.card_button("btrfs subvolume", "Create subvolume"))
+        self.dialog({"name": "unused"})
+
+        b.click(self.card_button("btrfs subvolume", "Create subvolume"))
+        self.dialog({"name": "home", "mount_point": "/home"})
+
+        # Exit the cockpit-storage iframe
+        b.switch_to_top()
+
+        s.return_to_installation()
+        s.return_to_installation_confirm()
+
+        s.set_partitioning("mount-point-mapping")
+
+        i.next(next_page=i.steps.CUSTOM_MOUNT_POINT)
+
+        s.select_mountpoint_row_device(1, "root")
+        s.select_mountpoint_row_device(2, "vda2")
+        s.select_mountpoint_row_reformat(2)
+        s.add_mountpoint_row()
+
+        # Selecting first device, then mount point and then reformat
+        s.select_mountpoint_row_device(3, "home")
+        s.select_mountpoint_row_mountpoint(3, "/home")
+        s.select_mountpoint_row_reformat(3, False)
+
+        # Root should be preselected to reformat
+        s.check_mountpoint_row(1, "/", "root", True, "btrfs")
+        s.check_mountpoint_row(2, "/boot", "vda2", True, "ext4")
+        s.check_mountpoint_row(3, "/home", "home", False, "btrfs")
+
+        self.addCleanup(lambda: dbus_reset_users(m))
+        i.reach(i.steps.REVIEW)
+
+        # verify review screen
+        dev = "vda"
+        r.check_disk(dev, "16.1 GB vda (0x1af4)")
+
+        r.check_disk_row(dev, "vda2, 1.07 GB: format as ext4, /boot")
+        r.check_disk_row(dev, "root, 15.0 GB: format as btrfs, /")
+        r.check_disk_row(dev, "home, 15.0 GB: mount, /home")
+
+        # Check fstab
+        fstab = m.execute("cat /etc/fstab")
+        self.assertTrue("/mnt/sysroot/boot auto noauto 0 0" in fstab)
+        self.assertTrue("/mnt/sysroot/btrfs-mount-test auto subvol=/ 0 0" in fstab)
+        self.assertTrue("/mnt/sysroot btrfs noauto,subvol=root 0 0" in fstab)
+        self.assertTrue("/mnt/sysroot/home btrfs noauto,subvol=home 0 0" in fstab)
+
 
 class TestStorageMountPointsEFI(anacondalib.VirtInstallMachineCase):
     efi = True

test/helpers/installer.py

@@ -59,10 +59,10 @@ def __init__(self, browser, machine, hidden_steps=None, scenario=None):
             self.steps._steps_jump[self.steps.INSTALLATION_METHOD] = [self.steps.DISK_ENCRYPTION, self.steps.CUSTOM_MOUNT_POINT]
 
     @log_step(snapshot_before=True)
-    def begin_installation(self, should_fail=False, confirm_erase=True):
+    def begin_installation(self, should_fail=False, confirm_erase=True, button_text='Erase data and install'):
         current_page = self.get_current_page()
 
-        self.browser.click("button:contains('Erase data and install')")
+        self.browser.click(f"button:contains('{button_text}')")
 
         if confirm_erase:
             self.browser.click(f"#{self.steps.REVIEW}-disk-erase-confirm")