initial global btrfs option

web/package/cockpit-agama.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Mon Feb 26 20:46:45 UTC 2024 - Josef Reidinger <[email protected]>
+
+- Remove fs type option "Btrfs with snapshots" and create instead
+  global option that affects only root volume
+  (gh#openSUSE/agama#1039)
+
 -------------------------------------------------------------------
 Thu Feb 22 14:05:56 UTC 2024 - David Diaz <[email protected]>
 

web/src/components/storage/ProposalSettingsSection.jsx

@@ -26,6 +26,7 @@ import { _ } from "~/i18n";
 import { If, PasswordAndConfirmationInput, Section, Popup } from "~/components/core";
 import { Icon } from "~/components/layout";
 import { noop } from "~/utils";
+import { hasFS } from "~/components/storage/utils";
 
 /**
  * @typedef {import ("~/client/storage").ProposalManager.ProposalSettings} ProposalSettings
@@ -112,6 +113,65 @@ const EncryptionSettingsForm = ({
   );
 };
 
+/**
+ * Allows to define snapshots enablement
+ * @component
+ *
+ * @param {object} props
+ * @param {ProposalSettings} props.settings - Settings used for calculating a proposal.
+ * @param {onChangeFn} [props.onChange=noop] - On change callback
+ *
+ * @callback onChangeFn
+ * @param {object} settings
+ */
+const SnapshotsField = ({
+  settings,
+  onChange = noop
+}) => {
+  const rootVolume = (settings.volumes || []).find((i) => i.mountPath === "/");
+
+  const initialChecked = rootVolume !== undefined && hasFS(rootVolume, "Btrfs") && rootVolume.snapshots;
+  const [isChecked, setIsChecked] = useState(initialChecked);
+
+  // no root volume is probably some error or still loading
+  if (rootVolume === undefined) {
+    return <Skeleton width="25%" />;
+  }
+
+  const switchState = (_, checked) => {
+    setIsChecked(checked);
+    onChange({ active: checked, settings });
+  };
+
+  const configurableSnapshots = rootVolume.outline.snapshotsConfigurable;
+  const forcedSnapshots = !configurableSnapshots && hasFS(rootVolume, "Btrfs") && rootVolume.snapshots;
+
+  const SnapshotsToggle = () => {
+    const explanation = _("Uses Btrfs for the root file system allowing to boot to a previous version of the system after configuration changes or software upgrades.");
+    return (
+      <>
+        <Switch
+          id="snapshots"
+          label={_("Use Btrfs Snapshots")}
+          isReversed
+          isChecked={isChecked}
+          onChange={switchState}
+        />
+        <div>
+          {explanation}
+        </div>
+      </>
+    );
+  };
+
+  return (
+    <div>
+      <If condition={forcedSnapshots} then={_("Btrfs snapshots required by product.")} />
+      <If condition={configurableSnapshots} then={<SnapshotsToggle />} />
+    </div>
+  );
+};
+
 /**
  * Allows to define encryption
  * @component
@@ -237,11 +297,28 @@ export default function ProposalSettingsSection({
     onChange({ encryptionPassword: password, encryptionMethod: method });
   };
 
+  const changeBtrfsSnapshots = ({ active, settings }) => {
+    const rootVolume = settings.volumes.find((i) => i.mountPath === "/");
+
+    if (active) {
+      rootVolume.fsType = "Btrfs";
+      rootVolume.snapshots = true;
+    } else {
+      rootVolume.snapshots = false;
+    }
+
+    onChange({ volumes: settings.volumes });
+  };
+
   const encryption = settings.encryptionPassword !== undefined && settings.encryptionPassword.length > 0;
 
   return (
     <>
       <Section title={_("Settings")}>
+        <SnapshotsField
+          settings={settings}
+          onChange={changeBtrfsSnapshots}
+        />
         <EncryptionField
           password={settings.encryptionPassword || ""}
           method={settings.encryptionMethod}

web/src/components/storage/VolumeForm.jsx

@@ -123,11 +123,6 @@ const MountPointFormSelect = ({ value, volumes, onChange, ...selectProps }) => {
   );
 };
 
-/**
- * Btrfs file system type can be offered with two flavors, with and without snapshots.
- */
-const BTRFS_WITH_SNAPSHOTS = "BtrfsWithSnapshots";
-
 /**
  * Possible file system type options for a volume.
  * @function
@@ -136,40 +131,7 @@ const BTRFS_WITH_SNAPSHOTS = "BtrfsWithSnapshots";
  * @returns {string[]}
  */
 const fsOptions = (volume) => {
-  const options = (volume, fsType) => {
-    if (fsType !== "Btrfs") return fsType;
-
-    const { snapshotsConfigurable } = volume.outline;
-    if (snapshotsConfigurable) return [BTRFS_WITH_SNAPSHOTS, fsType];
-
-    return (volume.snapshots ? BTRFS_WITH_SNAPSHOTS : fsType);
-  };
-
-  return volume.outline.fsTypes.flatMap(fsType => options(volume, fsType));
-};
-
-/**
- * File system option according to the given type and the value of snapshots.
- * @function
- *
- * @param {FsValue} value
- * @returns {string}
- */
-const fsOption = ({ fsType, snapshots }) => {
-  if (fsType !== "Btrfs") return fsType;
-
-  return (snapshots ? BTRFS_WITH_SNAPSHOTS : fsType);
-};
-
-/**
- * Label for a file system type option.
- * @function
- *
- * @param {string} fsOption
- * @returns {string}
- */
-const fsOptionLabel = (fsOption) => {
-  return (fsOption === BTRFS_WITH_SNAPSHOTS ? sprintf("Btrfs %s", _("with snapshots")) : fsOption);
+  return volume.outline.fsTypes;
 };
 
 /**
@@ -180,10 +142,7 @@ const fsOptionLabel = (fsOption) => {
  * @returns {FsValue}
  */
 const fsValue = (fsOption) => {
-  if (fsOption === BTRFS_WITH_SNAPSHOTS)
-    return { fsType: "Btrfs", snapshots: true };
-  else
-    return { fsType: fsOption, snapshots: false };
+  return { fsType: fsOption, snapshots: false };
 };
 
 /**
@@ -196,27 +155,9 @@ const fsValue = (fsOption) => {
  * @returns {ReactComponent}
  */
 const FsSelectOption = ({ fsOption }) => {
-  const label = () => {
-    if (fsOption === BTRFS_WITH_SNAPSHOTS) return "Btrfs";
-    return fsOption;
-  };
-
-  const details = () => {
-    if (fsOption === BTRFS_WITH_SNAPSHOTS) return _("with snapshots");
-    return undefined;
-  };
-
-  const description = () => {
-    if (fsOption === BTRFS_WITH_SNAPSHOTS)
-      // TRANSLATORS: description of Btrfs snapshots feature.
-      return _("Allows rolling back any change done to the system and restoring its previous state");
-
-    return undefined;
-  };
-
   return (
-    <SelectOption value={fsOption} description={description()}>
-      <strong>{label()}</strong> <If condition={details()} then={details()} />
+    <SelectOption value={fsOption}>
+      <strong>{fsOption}</strong>
     </SelectOption>
   );
 };
@@ -237,7 +178,7 @@ const FsSelect = ({ id, value, volume, onChange }) => {
   const [isOpen, setIsOpen] = useState(false);
 
   const options = fsOptions(volume);
-  const selected = fsOption(value);
+  const selected = value;
 
   const onToggleClick = () => {
     setIsOpen(!isOpen);
@@ -257,7 +198,7 @@ const FsSelect = ({ id, value, volume, onChange }) => {
         isExpanded={isOpen}
         className="full-width"
       >
-        {fsOptionLabel(selected)}
+        {selected}
       </MenuToggle>
     );
   };
@@ -300,8 +241,13 @@ const FsSelect = ({ id, value, volume, onChange }) => {
  */
 const FsField = ({ value, volume, onChange }) => {
   const isSingleFs = () => {
-    const { fsTypes, snapshotsConfigurable } = volume.outline;
-    return fsTypes.length === 1 && (fsTypes[0] !== "Btrfs" || !snapshotsConfigurable);
+    // check for btrfs with snapshots
+    if (volume.fsType === "Btrfs" && volume.snapshots) {
+      return true;
+    }
+
+    const { fsTypes } = volume.outline;
+    return fsTypes.length === 1;
   };
 
   const Info = () => {
@@ -330,7 +276,7 @@ const FsField = ({ value, volume, onChange }) => {
       condition={isSingleFs()}
       then={
         <FormGroup label={label}>
-          <p>{fsOptionLabel(fsOption(value))}</p>
+          <p>{value}</p>
         </FormGroup>
       }
       else={
@@ -760,7 +706,7 @@ export default function VolumeForm({ id, volume: currentVolume, templates = [],
     if (!Object.keys(errors).length) onSubmit(volume);
   };
 
-  const { fsType, snapshots } = state.formData;
+  const { fsType } = state.formData;
 
   const ShowMountPointSelector = () => (
     <MountPointFormSelect
@@ -788,7 +734,7 @@ export default function VolumeForm({ id, volume: currentVolume, templates = [],
         }
       />
       <FsField
-        value={{ fsType, snapshots }}
+        value={fsType}
         volume={state.volume}
         onChange={updateData}
       />

web/src/components/storage/VolumeForm.test.jsx

@@ -95,12 +95,12 @@ beforeEach(() => {
 });
 
 it("renders a control for displaying/selecting the file system type", async () => {
-  const { user } = plainRender(<VolumeForm {...props} />);
+  // use home which is not using snapshots
+  const { user } = plainRender(<VolumeForm volume={{ ...volumes.home }} />);
 
   const fsTypeButton = screen.getByRole("button", { name: "File system type" });
   await user.click(fsTypeButton);
-  screen.getByRole("option", { name: /Btrfs with snapshots/ });
-  screen.getByRole("option", { name: "Btrfs" });
+  screen.getByRole("option", { name: "XFS" });
   screen.getByRole("option", { name: "Ext4" });
 });
 
@@ -117,6 +117,24 @@ it("does not render the file system control if there is only one option", async
   );
 });
 
+it("renders the file system control for root mount point without snapshots", async () => {
+  const { user } = plainRender(<VolumeForm volume={{ ...volumes.root, snapshots: false }} />);
+
+  const fsTypeButton = screen.getByRole("button", { name: "File system type" });
+  await user.click(fsTypeButton);
+  screen.getByRole("option", { name: "Btrfs" });
+  screen.getByRole("option", { name: "Ext4" });
+});
+
+it("does not render the file system control for root mount point with btrfs with snapshots", async () => {
+  plainRender(<VolumeForm {...props} />);
+
+  await screen.findByText("Btrfs");
+  await waitFor(() => (
+    expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument())
+  );
+});
+
 it("renders controls for setting the desired size", () => {
   plainRender(<VolumeForm {...props} />);
 
@@ -179,17 +197,16 @@ it("calls the onSubmit callback with resulting volume when the form is submitted
   await user.type(maxSizeInput, "25");
   await user.selectOptions(maxSizeUnitSelector, maxSizeGiBUnit);
 
-  const fsTypeButton = screen.getByRole("button", { name: "File system type" });
-  await user.click(fsTypeButton);
-  const ext4Button = screen.getByRole("option", { name: "Ext4" });
-  await user.click(ext4Button);
+  // root is with btrfs and snapshots, so it is not possible to select it
+  await screen.findByText("Btrfs");
+  await waitFor(() => (
+    expect(screen.queryByRole("button", { name: "File system type" })).not.toBeInTheDocument())
+  );
 
   await user.click(submitForm);
 
   expect(onSubmitFn).toHaveBeenCalledWith({
     ...volumes.root,
-    fsType: "Ext4",
-    snapshots: false,
     minSize: parseToBytes("10 GiB"),
     maxSize: parseToBytes("25 GiB")
   });

web/src/components/storage/utils.js

@@ -135,6 +135,20 @@ const deviceLabel = (device) => {
   return size ? `${name}, ${deviceSize(size)}` : name;
 };
 
+/**
+ * Checks if volume uses given fs. This method works same as in backend
+ * case insensitive.
+ *
+ * @param {import(~/clients/storage).Volume} volume
+ * @param {string} fs - Filesystem name to check.
+ * @returns {boolean} true when volume uses given fs
+ */
+const hasFS = (volume, fs) => {
+  const volFS = volume.fsType;
+
+  return volFS.toLowerCase() === fs.toLocaleLowerCase();
+};
+
 export {
   DEFAULT_SIZE_UNIT,
   SIZE_METHODS,
@@ -143,4 +157,5 @@ export {
   deviceSize,
   parseToBytes,
   splitSize,
+  hasFS
 };

web/src/components/storage/utils.test.js

@@ -19,7 +19,7 @@
  * find current contact information at www.suse.com.
  */
 
-import { deviceSize, deviceLabel, parseToBytes, splitSize } from "./utils";
+import { deviceSize, deviceLabel, parseToBytes, splitSize, hasFS } from "./utils";
 
 describe("deviceSize", () => {
   it("returns the size with units", () => {
@@ -86,3 +86,17 @@ describe("splitSize", () => {
     expect(splitSize("")).toEqual({ size: undefined, unit: undefined });
   });
 });
+
+describe("hasFS", () => {
+  it("returns true if volume has given filesystem", () => {
+    expect(hasFS({ fsType: "Btrfs" }, "Btrfs")).toBe(true);
+  });
+
+  it("returns true if volume has given filesystem regarding different case", () => {
+    expect(hasFS({ fsType: "btrfs" }, "Btrfs")).toBe(true);
+  });
+
+  it("returns false if volume has different filesystem", () => {
+    expect(hasFS({ fsType: "Btrfs" }, "EXT4")).toBe(false);
+  });
+});