Merge pull request #5005 from KKoukiou/webui-fix-custom-part-recreate

webui: fix logic for when to re-create the partitioning

ui/webui/src/actions/storage-actions.js

@@ -85,12 +85,12 @@ export const getDiskSelectionAction = () => {
     };
 };
 
-export const getPartitioningDataAction = ({ requests, partitioning, updateOnly }) => {
+export const getPartitioningDataAction = ({ requests, partitioning }) => {
     return async (dispatch) => {
         const props = { path: partitioning };
         const convertRequests = reqs => reqs.map(request => Object.entries(request).reduce((acc, [key, value]) => ({ ...acc, [key]: value.v }), {}));
 
-        if (!updateOnly) {
+        if (!requests) {
             props.method = await getPartitioningMethod({ partitioning });
             if (props.method === "MANUAL") {
                 const reqs = await gatherRequests({ partitioning });

ui/webui/src/apis/storage.js

@@ -488,7 +488,7 @@ export const startEventMonitorStorage = ({ dispatch }) => {
                 if (args[0] === "org.fedoraproject.Anaconda.Modules.Storage.DiskSelection") {
                     dispatch(getDiskSelectionAction());
                 } else if (args[0] === "org.fedoraproject.Anaconda.Modules.Storage.Partitioning.Manual" && Object.hasOwn(args[1], "Requests")) {
-                    dispatch(getPartitioningDataAction({ requests: args[1].Requests.v, partitioning: path, updateOnly: true }));
+                    dispatch(getPartitioningDataAction({ requests: args[1].Requests.v, partitioning: path }));
                 } else if (args[0] === "org.fedoraproject.Anaconda.Modules.Storage" && Object.hasOwn(args[1], "CreatedPartitioning")) {
                     const last = args[1].CreatedPartitioning.v.length - 1;
                     dispatch(getPartitioningDataAction({ partitioning: args[1].CreatedPartitioning.v[last] }));

ui/webui/src/components/AnacondaWizard.jsx

@@ -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,
@@ -56,6 +56,20 @@ 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);
+
+    const availableDevices = useMemo(() => {
+        return Object.keys(storageData.devices);
+    }, [storageData.devices]);
+
+    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);
+    }, [availableDevices, storageData.diskSelection.selectedDisks]);
 
     const language = useMemo(() => {
         for (const l of Object.keys(localizationData.languages)) {
@@ -86,7 +100,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"

ui/webui/src/components/storage/EncryptedDevices.jsx

@@ -41,7 +41,6 @@ import { ModalError } from "cockpit-components-inline-notification.jsx";
 import { getDevicesAction } from "../../actions/storage-actions.js";
 
 import {
-    createPartitioning,
     unlockDevice,
 } from "../../apis/storage.js";
 
@@ -111,9 +110,6 @@ const UnlockDialog = ({ lockedLUKSDevices, onClose, dispatch }) => {
                     dispatch(getDevicesAction());
 
                     if (res.every(r => r.value[0])) {
-                        // Also refresh the partitioning data which will now show the children
-                        // of the unlocked device.
-                        createPartitioning({ method: "MANUAL" });
                         onClose();
                     } else {
                         dialogErrorSet(_("Incorrect passphrase"));

ui/webui/src/components/storage/MountPointMapping.jsx

@@ -55,6 +55,21 @@ const requiredMountPointOptions = [
     { value: "/", name: "root" },
 ];
 
+const getDeviceChildren = ({ deviceData, device }) => {
+    const children = [];
+    const deviceChildren = deviceData[device]?.children?.v || [];
+
+    if (deviceChildren.length === 0) {
+        children.push(device);
+    } else {
+        deviceChildren.forEach(child => {
+            children.push(...getDeviceChildren({ deviceData, device: child }));
+        });
+    }
+
+    return children;
+};
+
 const getInitialRequests = (partitioningData) => {
     const bootOriginalRequest = partitioningData.requests.find(r => r["mount-point"] === "/boot");
     const rootOriginalRequest = partitioningData.requests.find(r => r["mount-point"] === "/");
@@ -81,6 +96,26 @@ const isDuplicateRequestField = (requests, fieldName, fieldValue) => {
     return requests.filter((request) => request[fieldName] === fieldValue).length > 1;
 };
 
+const isReformatValid = (deviceData, request, requests) => {
+    const device = request["device-spec"];
+
+    if (!device || !request.reformat) {
+        return true;
+    }
+
+    const children = getDeviceChildren({ deviceData, device });
+
+    /* When parent device is re-formatted all children must:
+     * - either exist in the mount points mapper table and  be re-formatted
+     * - or not exist in the mountpoints mapper table
+     */
+    return children.every(child => {
+        const childRequest = requests.find(r => r["device-spec"] === child);
+
+        return !childRequest || childRequest.reformat === true;
+    });
+};
+
 const getLockedLUKSDevices = (requests, deviceData) => {
     const devs = requests?.map(r => r["device-spec"]) || [];
 
@@ -145,6 +180,7 @@ const DeviceColumnSelect = ({ deviceData, devices, idPrefix, lockedLUKSDevices,
 
         return (
             <SelectOption
+              data-value={device}
               isDisabled={isLockedLUKS}
               description={description}
               key={device}
@@ -200,9 +236,10 @@ const DeviceColumn = ({ deviceData, devices, idPrefix, handleRequestChange, lock
     );
 };
 
-const FormatColumn = ({ handleRequestChange, idPrefix, request }) => {
+const FormatColumn = ({ deviceData, handleRequestChange, idPrefix, request, requests }) => {
     const mountpoint = request["mount-point"];
     const isRootMountPoint = mountpoint === "/";
+    const reformatInvalid = !isReformatValid(deviceData, request, requests);
     const FormatSwitch = () => {
         return (
             <Switch
@@ -224,6 +261,12 @@ const FormatColumn = ({ handleRequestChange, idPrefix, request }) => {
                   content={_("The root partition is always re-formatted by the installer.")}>
                     <FormatSwitch />
                 </Tooltip>}
+            {reformatInvalid &&
+                <HelperText>
+                    <HelperTextItem variant="error" hasIcon>
+                        {_("Mismatch between parent device and child device reformat selection.")}
+                    </HelperTextItem>
+                </HelperText>}
         </Flex>
     );
 };
@@ -294,6 +337,7 @@ const RequestsTable = ({
                           handleRequestChange={handleRequestChange}
                           idPrefix={rowId + "-format"}
                           request={request}
+                          requests={requests}
                         />
                     ),
                     props: { className: columnClassName }
@@ -377,8 +421,10 @@ const MountPointMappingContent = ({ deviceData, partitioningData, dispatch, idPr
         if (requests) {
             const mountPoints = requests.map(r => r["mount-point"]);
             const devices = requests.map(r => r["device-spec"]);
+            const reformatInvalid = requests.some(request => !isReformatValid(deviceData, request, requests));
 
             const isFormValid = (
+                !reformatInvalid &&
                 new Set(mountPoints).size === mountPoints.length &&
                 new Set(devices).size === devices.length &&
                 mountPoints.every(m => m) &&
@@ -390,7 +436,7 @@ const MountPointMappingContent = ({ deviceData, partitioningData, dispatch, idPr
                 setUpdateRequestCnt(updateRequestCnt => updateRequestCnt + 1);
             }
         }
-    }, [requests, setIsFormValid]);
+    }, [deviceData, requests, setIsFormValid]);
 
     const handleRequestChange = (mountpoint, device, newRequestId, reformat) => {
         const data = deviceData[device];
@@ -457,18 +503,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(partitioningData?.path);
 
     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,
@@ -477,11 +516,14 @@ export const MountPointMapping = ({ deviceData, diskSelection, partitioningData,
              */
             setBootloaderDrive({ drive: "" })
                     .then(() => createPartitioning({ method: "MANUAL" }))
-                    .then(() => setCreatingPartitioning(false));
+                    .then(path => {
+                        setUsedPartitioning(path[0]);
+                        setReusePartitioning(true);
+                    });
         }
-    }, [canReusePartitioning]);
+    }, [reusePartitioning, setReusePartitioning, partitioningData?.method, partitioningData?.path]);
 
-    if (creatingPartitioning || !partitioningData?.path || (partitioningData?.requests?.length || 0) < 1) {
+    if (!reusePartitioning || usedPartitioning !== partitioningData.path) {
         return <EmptyStatePanel loading />;
     }
 

ui/webui/src/reducer.js

@@ -73,7 +73,7 @@ export const storageReducer = (state = storageInitialState, action) => {
     } else if (action.type === "GET_DISK_SELECTION") {
         return { ...state, diskSelection: action.payload.diskSelection };
     } else if (action.type === "GET_PARTITIONING_DATA") {
-        return { ...state, partitioning: action.payload.partitioningData };
+        return { ...state, partitioning: { ...state.partitioning, ...action.payload.partitioningData } };
     } else {
         return state;
     }