Skip to content

Commit

Permalink
Simplify logic for possibly reusing partitioning
Browse files Browse the repository at this point in the history
If the type matches, (MANUAL vs AUTOMATIC) and the devices also match
then we can reuse the partitioning.
  • Loading branch information
KKoukiou committed Apr 5, 2024
1 parent 0c75664 commit 59a0eee
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 107 deletions.
74 changes: 3 additions & 71 deletions src/components/AnacondaWizard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
import cockpit from "cockpit";

import React, { useContext, useEffect, useMemo, useState } from "react";
import React, { useEffect, useState } from "react";
import {
PageSection,
PageSectionTypes,
Expand All @@ -25,40 +25,19 @@ import {
WizardStep,
} from "@patternfly/react-core";

import { resetPartitioning } from "../apis/storage_partitioning.js";

import { AnacondaPage } from "./AnacondaPage.jsx";
import { AnacondaWizardFooter } from "./AnacondaWizardFooter.jsx";
import { FooterContext, StorageContext } from "./Common.jsx";
import { FooterContext } from "./Common.jsx";
import { InstallationProgress } from "./installation/InstallationProgress.jsx";
import { getSteps } from "./steps.js";
import { CockpitStorageIntegration } from "./storage/CockpitStorageIntegration.jsx";
const N_ = cockpit.noop;

export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStorage }) => {
const [isFormDisabled, setIsFormDisabled] = useState(false);
const [isFormValid, setIsFormValid] = useState(false);
const [reusePartitioning, setReusePartitioning] = useState(false);
const [stepNotification, setStepNotification] = useState();
const [showWizard, setShowWizard] = useState(true);
const [currentStepId, setCurrentStepId] = useState();
const storageData = useContext(StorageContext);
const storageScenarioId = storageData.storageScenarioId;
const selectedDisks = storageData.diskSelection.selectedDisks;
const [scenarioPartitioningMapping, setScenarioPartitioningMapping] = useState({});

useEffect(() => {
if (storageScenarioId && storageData.partitioning.path) {
setScenarioPartitioningMapping(_scenarioPartitioningMapping => ({
..._scenarioPartitioningMapping,
[storageScenarioId]: storageData.partitioning.path
}));
}
}, [storageData.partitioning.path, storageScenarioId]);

const availableDevices = useMemo(() => {
return Object.keys(storageData.devices);
}, [storageData.devices]);

useEffect(() => {
if (!currentStepId) {
Expand All @@ -67,51 +46,18 @@ export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStora
cockpit.location.go([currentStepId]);
}, [currentStepId]);

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, selectedDisks]);

const componentProps = {
dispatch,
isFormDisabled,
onCritFail,
reusePartitioning,
scenarioPartitioningMapping,
setIsFormDisabled,
setIsFormValid,
setReusePartitioning,
setShowStorage,
};

const getFlattenedStepsIds = (steps) => {
const stepIds = [];
for (const step of steps) {
stepIds.push(step.id);
if (step.steps) {
for (const childStep of step.steps) {
if (childStep?.isHidden !== true) {
stepIds.push(childStep.id);
}
}
}
}
return stepIds;
};
const stepsOrder = getSteps();
const flattenedStepsIds = getFlattenedStepsIds(stepsOrder);
const firstStepId = stepsOrder[0].id;

const isStepFollowedBy = (earlierStepId, laterStepId) => {
const earlierStepIdx = flattenedStepsIds.findIndex(s => s === earlierStepId);
const laterStepIdx = flattenedStepsIds.findIndex(s => s === laterStepId);
return earlierStepIdx < laterStepIdx;
};

const createSteps = (stepsOrder, componentProps) => {
return stepsOrder.map(s => {
const isVisited = firstStepId === s.id || currentStepId === s.id;
Expand Down Expand Up @@ -156,20 +102,7 @@ export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStora
setIsFormValid(false);
}

// Reset the applied partitioning when going back from a step after creating partitioning to a step
// before creating partitioning.
if ((prevStep.id === "accounts" || isStepFollowedBy("accounts", prevStep.id)) &&
isStepFollowedBy(newStep.id, "accounts")) {
setIsFormDisabled(true);
resetPartitioning()
.then(
() => setCurrentStepId(newStep.id),
() => onCritFail({ context: cockpit.format(N_("Error was hit when going back from $0."), prevStep.prevName) })
)
.always(() => setIsFormDisabled(false));
} else {
setCurrentStepId(newStep.id);
}
setCurrentStepId(newStep.id);
};

if (!showWizard) {
Expand Down Expand Up @@ -200,7 +133,6 @@ export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStora
<CockpitStorageIntegration
dispatch={dispatch}
onCritFail={onCritFail}
scenarioPartitioningMapping={scenarioPartitioningMapping}
setShowStorage={setShowStorage}
/>
);
Expand Down
17 changes: 6 additions & 11 deletions src/components/storage/CockpitStorageIntegration.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export const CockpitStorageIntegration = ({
dispatch,
onCritFail,
scenarioAvailability,
scenarioPartitioningMapping,
setShowStorage,
}) => {
const [showDialog, setShowDialog] = useState(false);
Expand Down Expand Up @@ -139,7 +138,6 @@ export const CockpitStorageIntegration = ({
dispatch={dispatch}
onCritFail={onCritFail}
scenarioAvailability={scenarioAvailability}
scenarioPartitioningMapping={scenarioPartitioningMapping}
setShowDialog={setShowDialog}
setShowStorage={setShowStorage}
/>}
Expand Down Expand Up @@ -206,7 +204,6 @@ export const preparePartitioning = async ({ devices, newMountPoints }) => {
const CheckStorageDialog = ({
dispatch,
onCritFail,
scenarioPartitioningMapping,
setShowDialog,
setShowStorage,
}) => {
Expand All @@ -228,26 +225,23 @@ const CheckStorageDialog = ({
devices,
mountPointConstraints,
newMountPoints,
scenarioPartitioningMapping,
});

return availability.available;
}, [devices, mountPointConstraints, newMountPoints, scenarioPartitioningMapping]);
}, [devices, mountPointConstraints, newMountPoints]);

const useConfiguredStorageReview = useMemo(() => {
const availability = checkConfiguredStorage({
devices,
mountPointConstraints,
newMountPoints,
scenarioPartitioningMapping,
});

return availability.review;
}, [
devices,
mountPointConstraints,
newMountPoints,
scenarioPartitioningMapping,
]);

const useFreeSpace = useMemo(() => {
Expand All @@ -260,10 +254,14 @@ const CheckStorageDialog = ({
const storageRequirementsNotMet = !loading && (error || (!useConfiguredStorage && !useFreeSpace));

useEffect(() => {
const mode = useConfiguredStorage ? "use-configured-storage" : "use-free-space";

dispatch(setStorageScenarioAction(mode));

if (!useConfiguredStorage && checkStep === "prepare-partitioning") {
setCheckStep();
}
}, [useConfiguredStorage, checkStep]);
}, [useConfiguredStorage, checkStep, dispatch]);

useEffect(() => {
if (checkStep !== "luks") {
Expand Down Expand Up @@ -360,9 +358,6 @@ const CheckStorageDialog = ({
}, [useConfiguredStorage, checkStep, dispatch, setError]);

const goBackToInstallation = () => {
const mode = useConfiguredStorage ? "use-configured-storage" : "use-free-space";

dispatch(setStorageScenarioAction(mode));
setShowStorage(false);
};

Expand Down
17 changes: 16 additions & 1 deletion src/components/storage/InstallationDestination.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ export const InstallationDestination = ({
const [equalDisksNotify, setEqualDisksNotify] = useState(false);
const refUsableDisks = useRef();
const isBootIso = useContext(SystemTypeContext) === "BOOT_ISO";
const { devices, diskSelection } = useContext(StorageContext);
const { devices, diskSelection, partitioning } = useContext(StorageContext);

debug("DiskSelector: devices: ", JSON.stringify(Object.keys(devices)), ", diskSelection: ", JSON.stringify(diskSelection));

Expand All @@ -337,6 +337,21 @@ export const InstallationDestination = ({
}
}, [isRescanningDisks, diskSelection.usableDisks]);

useEffect(() => {
// Always reset the partitioning when entering the installation destination page
const resetPartitioningAsync = async () => {
setIsFormDisabled(true);
await resetPartitioning();
setIsFormDisabled(false);
};

// If the last partitioning applied was from the cockpit storage integration
// we should not reset it, as this option does apply the partitioning onNext
if (partitioning.storageScenarioId !== "use-configured-storage") {
resetPartitioningAsync();
}
}, [setIsFormDisabled, partitioning.storageScenarioId]);

useEffect(() => {
// Select default disks for the partitioning on component mount
if (refUsableDisks.current !== undefined) {
Expand Down
2 changes: 0 additions & 2 deletions src/components/storage/InstallationMethod.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const InstallationMethod = ({
isEfi,
isFormDisabled,
onCritFail,
scenarioPartitioningMapping,
setIsFormDisabled,
setIsFormValid,
setShowStorage,
Expand All @@ -61,7 +60,6 @@ const InstallationMethod = ({
idPrefix={idPrefix}
isFormDisabled={isFormDisabled}
onCritFail={onCritFail}
scenarioPartitioningMapping={scenarioPartitioningMapping}
setIsFormValid={setIsFormValid}
/>
</Form>
Expand Down
11 changes: 4 additions & 7 deletions src/components/storage/InstallationScenario.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ export const checkConfiguredStorage = ({
mountPointConstraints,
newMountPoints,
partitioning,
scenarioPartitioningMapping,
storageScenarioId,
}) => {
const availability = new AvailabilityState();

const currentPartitioningMatches = partitioning !== undefined && scenarioPartitioningMapping["use-configured-storage"] === partitioning;
const currentPartitioningMatches = storageScenarioId === "use-configured-storage";
availability.hidden = partitioning === undefined || !currentPartitioningMatches;

availability.available = (
Expand Down Expand Up @@ -269,7 +269,6 @@ const InstallationScenarioSelector = ({
dispatch,
idPrefix,
isFormDisabled,
scenarioPartitioningMapping,
setIsFormValid,
}) => {
const { deviceNames, devices, diskSelection, partitioning } = useContext(StorageContext);
Expand Down Expand Up @@ -302,7 +301,7 @@ const InstallationScenarioSelector = ({
mountPointConstraints,
partitioning: partitioning.path,
requiredSize,
scenarioPartitioningMapping,
storageScenarioId: partitioning.storageScenarioId,
usablePartitions,
});
newAvailability[scenario.id] = availability;
Expand All @@ -316,8 +315,8 @@ const InstallationScenarioSelector = ({
duplicateDeviceNames,
mountPointConstraints,
partitioning.path,
partitioning.storageScenarioId,
requiredSize,
scenarioPartitioningMapping,
usablePartitions,
]);

Expand Down Expand Up @@ -394,7 +393,6 @@ export const InstallationScenario = ({
dispatch,
idPrefix,
isFormDisabled,
scenarioPartitioningMapping,
setIsFormValid,
}) => {
const isBootIso = useContext(SystemTypeContext) === "BOOT_ISO";
Expand All @@ -408,7 +406,6 @@ export const InstallationScenario = ({
dispatch={dispatch}
idPrefix={idPrefix}
isFormDisabled={isFormDisabled}
scenarioPartitioningMapping={scenarioPartitioningMapping}
setIsFormValid={setIsFormValid}
/>
</FormGroup>
Expand Down
41 changes: 35 additions & 6 deletions src/components/storage/MountPointMapping.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
} from "../../apis/storage_partitioning.js";

import {
getDeviceAncestors,
getDeviceChildren,
getLockedLUKSDevices,
hasDuplicateFields,
Expand Down Expand Up @@ -611,13 +612,40 @@ const isUsableDevice = (devSpec, deviceData) => {
const MountPointMapping = ({
dispatch,
idPrefix,
reusePartitioning,
setIsFormValid,
setReusePartitioning,
setStepNotification,
}) => {
const { devices, partitioning } = useContext(StorageContext);
const [usedPartitioning, setUsedPartitioning] = useState(partitioning?.path);
const { devices, diskSelection, partitioning } = useContext(StorageContext);
const [usedPartitioning, setUsedPartitioning] = useState();
const reusePartitioning = useMemo(() => {
// Calculate usable devices for partitioning by replicating the logic in the backend
// FIXME: Create a backend API for that
// https://github.com/rhinstaller/anaconda/blob/f79f019e22c87dc388dbcc637a7a5612a3c223a7/pyanaconda/modules/storage/partitioning/manual/manual_module.py#L127
const usableDevices = Object.keys(devices).filter(device => {
const children = devices[device].children.v;
const ancestors = getDeviceAncestors(devices, device);

if (children.length > 0 && devices[device].type.v !== "btrfs subvolume") {
return false;
}

// We don't want to allow to use snapshots in mount point assignment.
if (devices[device].type.v === "btrfs snapshot") {
return false;
}

// All device's disks have to be in selected disks.
return diskSelection.selectedDisks.some(disk => ancestors.includes(disk));
});

const usedDevices = partitioning?.requests?.map(r => r["device-spec"]) || [];

if (usedDevices.every(d => usableDevices.includes(d)) && usableDevices.every(d => usedDevices.includes(d))) {
return true;
}
return false;
}, [devices, diskSelection.selectedDisks, partitioning.requests]);

const mountPointConstraints = useMountPointConstraints();
const [skipUnlock, setSkipUnlock] = useState(false);
const lockedLUKSDevices = useMemo(
Expand Down Expand Up @@ -647,10 +675,11 @@ const MountPointMapping = ({
.then(() => createPartitioning({ method: "MANUAL" }))
.then(path => {
setUsedPartitioning(path);
setReusePartitioning(true);
});
} else {
setUsedPartitioning(partitioning.path);
}
}, [reusePartitioning, setReusePartitioning, partitioning?.method, partitioning?.path]);
}, [reusePartitioning, partitioning?.method, partitioning?.path]);

const isLoadingNewPartitioning = !reusePartitioning || usedPartitioning !== partitioning.path;
const showLuksUnlock = lockedLUKSDevices?.length > 0 && !skipUnlock;
Expand Down
2 changes: 1 addition & 1 deletion src/helpers/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
* @param {string} device - The name of the device
* @returns {Array}
*/
const getDeviceAncestors = (deviceData, device) => {
export const getDeviceAncestors = (deviceData, device) => {
// device ancestors including the device itself
const ancestors = [];
const deviceParents = deviceData[device]?.parents?.v || [];
Expand Down
Loading

0 comments on commit 59a0eee

Please sign in to comment.