web: Allow changing file system location (#1141)

By default, all the file systems are created as partitions at the disk selected for installation. This PR adds an optiton for changing the location of a file system, allowing to select a target disk or to follow the installation device. Moreover, it offers an option to create a dedicated LVM volume group. For more details see https://github.com/openSUSE/agama/blob/master/doc/storage_ui.md#file-systems. Bonus: This PR introduces quite some changes to enable type checking for several components. ## Screenshots ![localhost_8080_ (37)](https://github.com/openSUSE/agama/assets/1112304/33775787-836d-475c-a5ec-30ed599207c6)
agama-project · Apr 12, 2024 · 766a871 · 766a871
2 parents 67081db + 3b4ec55
commit 766a871
Show file tree

Hide file tree

Showing 24 changed files with 1,109 additions and 335 deletions.
diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes
@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Thu Apr 11 15:16:42 UTC 2024 - José Iván López González <[email protected]>
+
+- Allow changing the location of a file system
+  (gh#openSUSE/agama#1141).
+
 -------------------------------------------------------------------
 Mon Apr  8 14:17:45 UTC 2024 - José Iván López González <[email protected]>
 

diff --git a/web/src/client/mixins.js b/web/src/client/mixins.js
@@ -155,7 +155,7 @@ const WithStatus = (superclass, object_path) => class extends superclass {
    * Register a callback to run when the "CurrentInstallationPhase" changes
    *
    * @param {function} handler - callback function
-   * @return {function} function to disable the callback
+   * @return {() => void} function to disable the callback
    */
   onStatusChange(handler) {
     return this.client.onObjectChanged(object_path, STATUS_IFACE, (changes) => {

diff --git a/web/src/client/storage.js b/web/src/client/storage.js
@@ -61,7 +61,7 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
  * @property {string} [busId] - DASD Bus ID (only for "dasd" type)
  * @property {string} [transport]
  * @property {boolean} [sdCard]
- * @property {boolean} [dellBOOS]
+ * @property {boolean} [dellBOSS]
  * @property {string[]} [devices] - RAID devices (only for "raid" and "md" types)
  * @property {string[]} [wires] - Multipath wires (only for "multipath" type)
  * @property {string} [level] - MD RAID level (only for "md" type)
@@ -110,8 +110,10 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
  * @property {boolean} subvol
  * @property {boolean} delete
  *
+ * @todo Define an enum for space policies.
+ *
  * @typedef {object} ProposalSettings
- * @property {string} target
+ * @property {ProposalTarget} target
  * @property {string} [targetDevice]
  * @property {string[]} targetPVDevices
  * @property {boolean} configureBoot
@@ -124,13 +126,15 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
  * @property {Volume[]} volumes
  * @property {StorageDevice[]} installationDevices
  *
+ * @typedef {keyof ProposalTargets} ProposalTarget
+ *
  * @typedef {object} SpaceAction
  * @property {string} device
  * @property {string} action
  *
  * @typedef {object} Volume
  * @property {string} mountPath
- * @property {string} target
+ * @property {VolumeTarget} target
  * @property {StorageDevice} [targetDevice]
  * @property {string} fsType
  * @property {number} minSize
@@ -140,15 +144,44 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
  * @property {boolean} transactional
  * @property {VolumeOutline} outline
  *
+ * @typedef {keyof VolumeTargets} VolumeTarget
+ *
+ * @todo Define an enum for file system types.
+ *
  * @typedef {object} VolumeOutline
  * @property {boolean} required
  * @property {string[]} fsTypes
+ * @property {boolean} adjustByRam
  * @property {boolean} supportAutoSize
  * @property {boolean} snapshotsConfigurable
  * @property {boolean} snapshotsAffectSizes
  * @property {string[]} sizeRelevantVolumes
  */
 
+/**
+ * Enum for the possible proposal targets.
+ *
+ * @readonly
+ */
+const ProposalTargets = Object.freeze({
+  DISK: "disk",
+  NEW_LVM_VG: "newLvmVg",
+  REUSED_LVM_VG: "reusedLvmVg"
+});
+
+/**
+ * Enum for the possible volume targets.
+ *
+ * @readonly
+ */
+const VolumeTargets = Object.freeze({
+  DEFAULT: "default",
+  NEW_PARTITION: "new_partition",
+  NEW_VG: "new_vg",
+  DEVICE: "device",
+  FILESYSTEM: "filesystem"
+});
+
 /**
  * Enum for the encryption method values
  *
@@ -463,6 +496,23 @@ class ProposalManager {
         };
       };
 
+      /**
+       * Builds the proposal target from a D-Bus value.
+       *
+       * @param {string} dbusTarget
+       * @returns {ProposalTarget}
+       */
+      const buildTarget = (dbusTarget) => {
+        switch (dbusTarget) {
+          case "disk": return "DISK";
+          case "newLvmVg": return "NEW_LVM_VG";
+          case "reusedLvmVg": return "REUSED_LVM_VG";
+          default:
+            console.info(`Unknown proposal target "${dbusTarget}", using "disk".`);
+            return "DISK";
+        }
+      };
+
       const buildTargetPVDevices = dbusTargetPVDevices => {
         if (!dbusTargetPVDevices) return [];
 
@@ -474,7 +524,7 @@ class ProposalManager {
         const findDevice = (name) => {
           const device = devices.find(d => d.name === name);
 
-          if (device === undefined) console.log("D-Bus object not found: ", name);
+          if (device === undefined) console.error("D-Bus object not found: ", name);
 
           return device;
         };
@@ -489,14 +539,14 @@ class ProposalManager {
         const names = uniq(compact(values)).filter(d => d.length > 0);
 
         // #findDevice returns undefined if no device is found with the given name.
-        return compact(names.map(n => findDevice(n)));
+        return compact(names.map(findDevice));
       };
 
       const dbusSettings = proxy.Settings;
 
       return {
         settings: {
-          target: dbusSettings.Target.v,
+          target: buildTarget(dbusSettings.Target.v),
           targetDevice: dbusSettings.TargetDevice?.v,
           targetPVDevices: buildTargetPVDevices(dbusSettings.TargetPVDevices),
           configureBoot: dbusSettings.ConfigureBoot.v,
@@ -560,15 +610,15 @@ class ProposalManager {
         MinSize: { t: "t", v: volume.minSize },
         MaxSize: { t: "t", v: volume.maxSize },
         AutoSize: { t: "b", v: volume.autoSize },
-        Target: { t: "s", v: volume.target },
+        Target: { t: "s", v: VolumeTargets[volume.target] },
         TargetDevice: { t: "s", v: volume.targetDevice?.name },
         Snapshots: { t: "b", v: volume.snapshots },
         Transactional: { t: "b", v: volume.transactional },
       });
     };
 
     const dbusSettings = removeUndefinedCockpitProperties({
-      Target: { t: "s", v: target },
+      Target: { t: "s", v: ProposalTargets[target] },
       TargetDevice: { t: "s", v: targetDevice },
       TargetPVDevices: { t: "as", v: targetPVDevices },
       ConfigureBoot: { t: "b", v: configureBoot },
@@ -635,6 +685,25 @@ class ProposalManager {
    * @returns {Volume}
    */
   buildVolume(dbusVolume, devices) {
+    /**
+     * Builds a volume target from a D-Bus value.
+     *
+     * @param {string} dbusTarget
+     * @returns {VolumeTarget}
+     */
+    const buildTarget = (dbusTarget) => {
+      switch (dbusTarget) {
+        case "default": return "DEFAULT";
+        case "new_partition": return "NEW_PARTITION";
+        case "new_vg": return "NEW_VG";
+        case "device": return "DEVICE";
+        case "filesystem": return "FILESYSTEM";
+        default:
+          console.info(`Unknown volume target "${dbusTarget}", using "default".`);
+          return "DEFAULT";
+      }
+    };
+
     const buildOutline = (dbusOutline) => {
       if (dbusOutline === undefined) return null;
 
@@ -650,7 +719,7 @@ class ProposalManager {
     };
 
     return {
-      target: dbusVolume.Target.v,
+      target: buildTarget(dbusVolume.Target.v),
       targetDevice: devices.find(d => d.name === dbusVolume.TargetDevice?.v),
       mountPath: dbusVolume.MountPath.v,
       fsType: dbusVolume.FsType.v,

diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js
@@ -1467,7 +1467,7 @@ describe("#proposal", () => {
 
       expect(home).toStrictEqual({
         mountPath: "/home",
-        target: "default",
+        target: "DEFAULT",
         targetDevice: undefined,
         fsType: "XFS",
         minSize: 2048,
@@ -1490,7 +1490,7 @@ describe("#proposal", () => {
 
       expect(generic).toStrictEqual({
         mountPath: "",
-        target: "default",
+        target: "DEFAULT",
         targetDevice: undefined,
         fsType: "Ext4",
         minSize: 1024,
@@ -1535,7 +1535,7 @@ describe("#proposal", () => {
         const { settings, actions } = await client.proposal.getResult();
 
         expect(settings).toMatchObject({
-          target: "newLvmVg",
+          target: "NEW_LVM_VG",
           targetPVDevices: ["/dev/sda", "/dev/sdb"],
           configureBoot: true,
           bootDevice: "/dev/sda",
@@ -1549,7 +1549,7 @@ describe("#proposal", () => {
           volumes: [
             {
               mountPath: "/",
-              target: "default",
+              target: "DEFAULT",
               targetDevice: undefined,
               fsType: "Btrfs",
               minSize: 1024,
@@ -1568,7 +1568,7 @@ describe("#proposal", () => {
             },
             {
               mountPath: "/home",
-              target: "default",
+              target: "DEFAULT",
               targetDevice: undefined,
               fsType: "XFS",
               minSize: 2048,
@@ -1615,7 +1615,7 @@ describe("#proposal", () => {
 
     it("calculates a proposal with the given settings", async () => {
       await client.proposal.calculate({
-        target: "disk",
+        target: "DISK",
         targetDevice: "/dev/vdc",
         configureBoot: true,
         bootDevice: "/dev/vdb",

diff --git a/web/src/components/core/Description.jsx b/web/src/components/core/Description.jsx
@@ -19,15 +19,19 @@
  * find current contact information at www.suse.com.
  */
 
+// @ts-check
+
 import React from "react";
 import { Popover, Button } from "@patternfly/react-core";
 
 /**
  * Displays details popup after clicking the children elements
  * @component
  *
- * @param {(JSX.Element|null)} description content displayed in a popup
- * @param {JSX.Element} children the wrapped content
+ * @param {object} props
+ * @param {React.ReactElement} props.description - Content displayed in a popup.
+ * @param {React.ReactNode} props.children - The wrapped content.
+ * @param {import("@patternfly/react-core").PopoverProps} [props.otherProps]
  */
 export default function Description ({ description, children, ...otherProps }) {
   if (description) {

diff --git a/web/src/components/core/NumericTextInput.jsx b/web/src/components/core/NumericTextInput.jsx
@@ -19,6 +19,8 @@
  * find current contact information at www.suse.com.
  */
 
+// @ts-check
+
 import React from "react";
 import { TextInput } from "@patternfly/react-core";
 import { noop } from "~/utils";
@@ -42,9 +44,7 @@ import { noop } from "~/utils";
  * @param {object} props
  * @param {string|number} props.value - the input value
  * @param {onChangeFn} props.onChange - the callback to be called when the entered value match the input pattern
- * @param {object} props.textInputProps - @see {@link https://www.patternfly.org/components/forms/text-input#props}
- *
- * @returns {ReactComponent}
+ * @param {import("@patternfly/react-core").TextInputProps} props.textInputProps
  */
 export default function NumericTextInput({ value = "", onChange = noop, ...textInputProps }) {
   // NOTE: Using \d* instead of \d+ at the beginning to allow empty

diff --git a/web/src/components/core/RowActions.jsx b/web/src/components/core/RowActions.jsx
@@ -52,8 +52,6 @@ import { _ } from "~/i18n";
  * @param {object} [props.rest]
  *
  * @typedef {import("@patternfly/react-table").IAction} Action
- *
- * @return {React.ActionsColumn}
  */
 export default function RowActions({ id, actions, "aria-label": toggleAriaLabel, ...rest }) {
   const actionsToggle = (props) => (

diff --git a/web/src/components/core/Tip.jsx b/web/src/components/core/Tip.jsx
@@ -19,6 +19,8 @@
  * find current contact information at www.suse.com.
  */
 
+// @ts-check
+
 import React from "react";
 import { Label } from "@patternfly/react-core";
 
@@ -31,8 +33,9 @@ import { Icon } from "~/components/layout";
  * If the label is not defined or is empty it behaves like a plain label.
  * @component
  *
- * @param {JSX.Element} description details displayed after clicking the label
- * @param {JSX.Element} children the content of the label
+ * @param {object} props
+ * @param {React.ReactElement} props.description - Details displayed after clicking the label.
+ * @param {React.ReactNode} props.children - The content of the label.
  */
 export default function Tip ({ description, children }) {
   if (description) {