Skip to content

Commit

Permalink
webui: fix logic for when to re-create the partitioning
Browse files Browse the repository at this point in the history
Now partitioning is marked for re-creation when device selection
changes and when the user re-scans the device tree.

The latter invalidates the partitioning indirectly, by re-setting the
selected devices.
  • Loading branch information
KKoukiou committed Aug 4, 2023
1 parent f076f51 commit 97604a7
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 24 deletions.
14 changes: 12 additions & 2 deletions ui/webui/src/components/AnacondaWizard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* along with This program; If not, see <http://www.gnu.org/licenses/>.
*/
import cockpit from "cockpit";
import React, { useState, useMemo } from "react";
import React, { useEffect, useState, useMemo } from "react";

import {
ActionList,
Expand Down Expand Up @@ -56,6 +56,16 @@ export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, lo
const [isInProgress, setIsInProgress] = useState(false);
const [storageEncryption, setStorageEncryption] = useState(getStorageEncryptionState());
const [storageScenarioId, setStorageScenarioId] = useState(window.sessionStorage.getItem("storage-scenario-id") || getDefaultScenario().id);
const [reusePartitioning, setReusePartitioning] = useState(false);

useEffect(() => {
/*
* When disk selection changes or the user re-scans the devices we need to re-create the partitioning.
* For Automatic partitioning we do it each time we go to review page,
* but for custom mount assignment we try to reuse the partitioning when possible.
*/
setReusePartitioning(false);
}, [storageData.diskSelection.selectedDisks]);

const language = useMemo(() => {
for (const l of Object.keys(localizationData.languages)) {
Expand Down Expand Up @@ -86,7 +96,7 @@ export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, lo
label: _("Disk configuration"),
steps: [{
component: MountPointMapping,
data: { deviceData: storageData.devices, diskSelection: storageData.diskSelection, partitioningData: storageData.partitioning, dispatch },
data: { deviceData: storageData.devices, diskSelection: storageData.diskSelection, partitioningData: storageData.partitioning, dispatch, reusePartitioning, setReusePartitioning },
id: "mount-point-mapping",
label: _("Manual disk configuration"),
isHidden: storageScenarioId !== "mount-point-mapping"
Expand Down
24 changes: 11 additions & 13 deletions ui/webui/src/components/storage/MountPointMapping.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -453,18 +453,11 @@ const MountPointMappingContent = ({ deviceData, partitioningData, dispatch, idPr
}
};

export const MountPointMapping = ({ deviceData, diskSelection, partitioningData, dispatch, idPrefix, setIsFormValid, onAddErrorNotification, stepNotification }) => {
const [creatingPartitioning, setCreatingPartitioning] = useState(true);

// If device selection changed since the last partitioning request redo the partitioning
const selectedDevices = diskSelection.selectedDisks;
const partitioningDevices = partitioningData?.requests?.map(r => r["device-spec"]) || [];
const canReusePartitioning = selectedDevices.length === partitioningDevices.length && selectedDevices.every(d => partitioningDevices.includes(d));
export const MountPointMapping = ({ deviceData, diskSelection, partitioningData, dispatch, idPrefix, setIsFormValid, onAddErrorNotification, reusePartitioning, setReusePartitioning, stepNotification }) => {
const [usedPartitioning, setUsedPartitioning] = useState();

useEffect(() => {
if (canReusePartitioning) {
setCreatingPartitioning(false);
} else {
if (!reusePartitioning || partitioningData?.method !== "MANUAL") {
/* Reset the bootloader drive before we schedule partitions
* The bootloader drive is automatically set during the partitioning, so
* make sure we always reset the previous value before we run another one,
Expand All @@ -473,11 +466,16 @@ export const MountPointMapping = ({ deviceData, diskSelection, partitioningData,
*/
setBootloaderDrive({ drive: "" })
.then(() => createPartitioning({ method: "MANUAL" }))
.then(() => setCreatingPartitioning(false));
.then((path) => {
setUsedPartitioning(path);
setReusePartitioning(true);
});
} else {
setUsedPartitioning(partitioningData.path);
}
}, [canReusePartitioning]);
}, [reusePartitioning, setReusePartitioning, partitioningData?.method, partitioningData?.path]);

if (creatingPartitioning || !partitioningData?.path || (partitioningData?.requests?.length || 0) < 1) {
if (!usedPartitioning || usedPartitioning !== partitioningData.path) {
return <EmptyStatePanel loading />;
}

Expand Down
29 changes: 23 additions & 6 deletions ui/webui/test/check-storage
Original file line number Diff line number Diff line change
Expand Up @@ -485,13 +485,30 @@ class TestStorageMountPoints(anacondalib.VirtInstallMachineCase):
i.next()

# verify review screen
disk = "vda"
r.check_disk(disk, "16.1 GB vda (0x1af4)")
r.check_disk(dev, "16.1 GB vda (0x1af4)")

r.check_disk_row(disk, 1, "vda1, 1.05 MB: biosboot")
r.check_disk_row(disk, 2, "vda2, 1.07 GB: format as ext4, /boot")
r.check_disk_row(disk, 3, "btrfstest, 10.7 GB: format as btrfs, /")
r.check_disk_row(disk, 4, "vda4, 4.29 GB: mount, /home")
r.check_disk_row(dev, 1, "vda1, 1.05 MB: biosboot")
r.check_disk_row(dev, 2, "vda2, 1.07 GB: format as ext4, /boot")
r.check_disk_row(dev, 3, "btrfstest, 10.7 GB: format as btrfs, /")
r.check_disk_row(dev, 4, "vda4, 4.29 GB: mount, /home")

applied_partitioning = s.dbus_get_applied_partitioning()

# When adding a new partition a new partitioning should be created
i.back(previous_page=i.steps.CUSTOM_MOUNT_POINT)
i.back()

m.execute(f"sgdisk --new=0:0:0 {disk}")
s.rescan_disks()
self.select_mountpoint(i, s, [(dev, True)])
self.check_row_device(1, "Select a device")
self.check_row_device(2, "Select a device")
self.select_row_device(1, f"{dev}2")
self.select_row_device(2, f"btrfs")

i.next()
new_applied_partitioning = s.dbus_get_applied_partitioning()
self.assertNotEqual(new_applied_partitioning, applied_partitioning)

@nondestructive
def testNoRootMountPoint(self):
Expand Down
8 changes: 5 additions & 3 deletions ui/webui/test/helpers/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,18 @@ def check_next_disabled(self, disabled=True):
self.browser.wait_visible(f"#installation-next-btn:not([aria-disabled={value}]")

@log_step(snapshot_before=True)
def back(self, should_fail=False):
def back(self, should_fail=False, previous_page=""):
current_page = self.get_current_page()

self.browser.click("button:contains(Back)")

if should_fail:
self.wait_current_page(current_page)
else:
prev = [k for k, v in self.steps._steps_jump.items() if current_page in v][0]
self.wait_current_page(prev)
if not previous_page:
previous_page = [k for k, v in self.steps._steps_jump.items() if current_page in v][0]

self.wait_current_page(previous_page)

@log_step()
def open(self, step="installation-language"):
Expand Down

0 comments on commit 97604a7

Please sign in to comment.