Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web] Changes in the volumes table #1125

Merged
merged 3 commits into from
Apr 4, 2024
Merged

[web] Changes in the volumes table #1125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
afe690c
[web] Changes in the volumes table
ancorgs Apr 3, 2024
ed23c10
[web] Fixes from code review
ancorgs Apr 4, 2024
a01f288
Changelog entry
ancorgs Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions web/package/cockpit-agama.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
-------------------------------------------------------------------
Thu Apr 4 15:59:37 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

- Changes in the table of file systems (gh#openSUSE/agama#1125)

-------------------------------------------------------------------
Wed Apr 3 15:16:07 UTC 2024 - José Iván López González <[email protected]>

Expand Down
15 changes: 10 additions & 5 deletions web/src/client/storage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ const ZFCP_DISK_IFACE = "org.opensuse.Agama.Storage1.ZFCP.Disk";
* @typedef {object} Volume
* @property {string} mountPath
* @property {string} target
* @property {string} [targetDevice]
* @property {StorageDevice} [targetDevice]
* @property {string} fsType
* @property {number} minSize
* @property {number} [maxSize]
Expand Down Expand Up @@ -430,7 +430,8 @@ class ProposalManager {
*/
async defaultVolume(mountPath) {
const proxy = await this.proxies.proposalCalculator;
return this.buildVolume(await proxy.DefaultVolume(mountPath));
const systemDevices = await this.system.getDevices();
return this.buildVolume(await proxy.DefaultVolume(mountPath), systemDevices);
}

/**
Expand Down Expand Up @@ -505,7 +506,7 @@ class ProposalManager {
spaceActions: dbusSettings.SpaceActions.v.map(a => buildSpaceAction(a.v)),
encryptionPassword: dbusSettings.EncryptionPassword.v,
encryptionMethod: dbusSettings.EncryptionMethod.v,
volumes: dbusSettings.Volumes.v.map(vol => this.buildVolume(vol.v)),
volumes: dbusSettings.Volumes.v.map(vol => this.buildVolume(vol.v, systemDevices)),
// NOTE: strictly speaking, installation devices does not belong to the settings. It
// should be a separate method instead of an attribute in the settings object.
// Nevertheless, it was added here for simplicity and to avoid passing more props in some
Expand Down Expand Up @@ -559,6 +560,8 @@ 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 },
TargetDevice: { t: "s", v: volume.targetDevice?.name },
Snapshots: { t: "b", v: volume.snapshots },
Transactional: { t: "b", v: volume.transactional },
});
Expand Down Expand Up @@ -597,6 +600,8 @@ class ProposalManager {
* @property {CockpitBoolean} AutoSize
* @property {CockpitBoolean} Snapshots
* @property {CockpitBoolean} Transactional
* @property {CockpitString} Target
* @property {CockpitString} [TargetDevice]
* @property {CockpitVolumeOutline} Outline
*
* @typedef {Object} DBusVolumeOutline
Expand Down Expand Up @@ -629,7 +634,7 @@ class ProposalManager {
*
* @returns {Volume}
*/
buildVolume(dbusVolume) {
buildVolume(dbusVolume, devices) {
const buildOutline = (dbusOutline) => {
if (dbusOutline === undefined) return null;

Expand All @@ -646,7 +651,7 @@ class ProposalManager {

return {
target: dbusVolume.Target.v,
targetDevice: dbusVolume.TargetDevice?.v,
targetDevice: devices.find(d => d.name === dbusVolume.TargetDevice?.v),
mountPath: dbusVolume.MountPath.v,
fsType: dbusVolume.FsType.v,
minSize: dbusVolume.MinSize.v,
Expand Down
8 changes: 4 additions & 4 deletions web/src/client/storage.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,7 +1468,7 @@ describe("#proposal", () => {
expect(home).toStrictEqual({
mountPath: "/home",
target: "default",
targetDevice: "",
targetDevice: undefined,
fsType: "XFS",
minSize: 2048,
maxSize: 4096,
Expand All @@ -1491,7 +1491,7 @@ describe("#proposal", () => {
expect(generic).toStrictEqual({
mountPath: "",
target: "default",
targetDevice: "",
targetDevice: undefined,
fsType: "Ext4",
minSize: 1024,
maxSize: 2048,
Expand Down Expand Up @@ -1550,7 +1550,7 @@ describe("#proposal", () => {
{
mountPath: "/",
target: "default",
targetDevice: "",
targetDevice: undefined,
fsType: "Btrfs",
minSize: 1024,
maxSize: 2048,
Expand All @@ -1569,7 +1569,7 @@ describe("#proposal", () => {
{
mountPath: "/home",
target: "default",
targetDevice: "",
targetDevice: undefined,
fsType: "XFS",
minSize: 2048,
maxSize: 4096,
Expand Down
58 changes: 36 additions & 22 deletions web/src/components/storage/ProposalVolumes.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ import { Table, Thead, Tr, Th, Tbody, Td } from '@patternfly/react-table';
import { sprintf } from "sprintf-js";

import { _ } from "~/i18n";
import { Em, If, Popup, RowActions, Tip } from '~/components/core';
import { Icon } from '~/components/layout';
import { If, Popup, RowActions, Tip } from '~/components/core';
import { VolumeForm } from '~/components/storage';
import { deviceSize, hasSnapshots, isTransactionalRoot } from '~/components/storage/utils';
import { noop } from "~/utils";
Expand Down Expand Up @@ -185,8 +184,12 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) =>
};

const SizeLimits = ({ volume }) => {
const minSize = deviceSize(volume.minSize);
const maxSize = volume.maxSize ? deviceSize(volume.maxSize) : undefined;
let targetSize;
if (volume.target === "filesystem" || volume.target === "device")
targetSize = volume.targetDevice.size;

const minSize = deviceSize(targetSize || volume.minSize);
const maxSize = targetSize ? deviceSize(targetSize) : volume.maxSize ? deviceSize(volume.maxSize) : undefined;
const isAuto = volume.autoSize;

let size = minSize;
Expand All @@ -203,27 +206,34 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) =>
);
};

const Details = ({ volume, options }) => {
const Details = ({ volume }) => {
const snapshots = hasSnapshots(volume);
const transactional = isTransactionalRoot(volume);

// TRANSLATORS: the filesystem uses a logical volume (LVM)
const text = `${volume.fsType} ${options.lvm ? _("logical volume") : _("partition")}`;
const lockIcon = <Icon name="lock" size="xxxs" />;
const snapshotsIcon = <Icon name="add_a_photo" size="xxxs" />;
const transactionalIcon = <Icon name="sync" size="xxxs" />;
if (volume.target === "filesystem")
// TRANSLATORS: %s will be replaced by a file-system type like "Btrfs" or "Ext4"
return sprintf(_("Reused %s"), volume.targetDevice?.filesystem?.type || "");
if (transactional)
return _("Transactional Btrfs");
if (snapshots)
return _("Btrfs with snapshots");

return (
<div className="split">
<span>{text}</span>
{/* TRANSLATORS: filesystem flag, it uses an encryption */}
<If condition={options.encryption} then={<Em icon={lockIcon}>{_("encrypted")}</Em>} />
{/* TRANSLATORS: filesystem flag, it allows creating snapshots */}
<If condition={snapshots && !transactional} then={<Em icon={snapshotsIcon}>{_("with snapshots")}</Em>} />
{/* TRANSLATORS: flag for transactional file system */}
<If condition={transactional} then={<Em icon={transactionalIcon}>{_("transactional")}</Em>} />
</div>
);
return volume.fsType;
};

const Location = ({ volume, options }) => {
if (volume.target === "new_partition")
// TRANSLATORS: %s will be replaced by a disk name (eg. "/dev/sda")
return sprintf(_("Partition at %s"), volume.targetDevice?.name || "");
if (volume.target === "new_vg")
// TRANSLATORS: %s will be replaced by a disk name (eg. "/dev/sda")
return sprintf(_("Separate LVM at %s"), volume.targetDevice?.name || "");
if (volume.target === "device" || volume.target === "filesystem")
return volume.targetDevice?.name || "";
if (options.lvm)
return _("Logical volume at system LVM");

return _("Partition at installation disk");
};

const VolumeActions = ({ volume, onEdit, onDelete }) => {
Expand Down Expand Up @@ -265,8 +275,9 @@ const VolumeRow = ({ columns, volume, options, isLoading, onEdit, onDelete }) =>
<>
<Tr>
<Td dataLabel={columns.mountPath}>{volume.mountPath}</Td>
<Td dataLabel={columns.details}><Details volume={volume} options={options} /></Td>
<Td dataLabel={columns.details}><Details volume={volume} /></Td>
<Td dataLabel={columns.size}><SizeLimits volume={volume} /></Td>
<Td dataLabel={columns.location}><Location volume={volume} options={options} /></Td>
<Td isActionCell>
<VolumeActions
volume={volume}
Expand Down Expand Up @@ -311,6 +322,8 @@ const VolumesTable = ({ volumes, options, isLoading, onVolumesChange }) => {
mountPath: _("Mount point"),
details: _("Details"),
size: _("Size"),
// TRANSLATORS: where (and how) the file-system is going to be created
location: _("Location"),
actions: _("Actions")
};

Expand Down Expand Up @@ -352,6 +365,7 @@ const VolumesTable = ({ volumes, options, isLoading, onVolumesChange }) => {
<Th>{columns.mountPath}</Th>
<Th>{columns.details}</Th>
<Th>{columns.size}</Th>
<Th>{columns.location}</Th>
<Th />
</Tr>
</Thead>
Expand Down
87 changes: 80 additions & 7 deletions web/src/components/storage/ProposalVolumes.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,41 @@ const volumes = {
}
};

const sda = {
sid: 59,
name: "/dev/sda",
isDrive: true,
type: "disk",
vendor: "Micron",
model: "Micron 1100 SATA",
transport: "usb",
size: 1024
};

const sda1 = {
sid: 69,
name: "/dev/sda1",
isDrive: false,
type: "partition",
size: 256,
filesystem: {
sid: 169,
type: "Swap"
}
};

const sda2 = {
sid: 79,
name: "/dev/sda2",
isDrive: false,
type: "partition",
size: 512,
filesystem: {
sid: 179,
type: "Ext4"
}
};

let props;

beforeEach(() => {
Expand Down Expand Up @@ -181,9 +216,9 @@ describe("if there are volumes", () => {
const [, body] = await screen.findAllByRole("rowgroup");

expect(within(body).queryAllByRole("row").length).toEqual(3);
within(body).getByRole("row", { name: "/ Btrfs partition 1 KiB - 2 KiB" });
within(body).getByRole("row", { name: "/home XFS partition At least 1 KiB" });
within(body).getByRole("row", { name: "swap Swap partition 1 KiB" });
within(body).getByRole("row", { name: "/ Btrfs 1 KiB - 2 KiB Partition at installation disk" });
within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" });
within(body).getByRole("row", { name: "swap Swap 1 KiB Partition at installation disk" });
});

it("allows deleting the volume", async () => {
Expand All @@ -192,7 +227,7 @@ describe("if there are volumes", () => {
const { user } = plainRender(<ProposalVolumes {...props} />);

const [, body] = await screen.findAllByRole("rowgroup");
const row = within(body).getByRole("row", { name: "/home XFS partition At least 1 KiB" });
const row = within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" });
const actions = within(row).getByRole("button", { name: "Actions" });
await user.click(actions);
const deleteAction = within(row).queryByRole("menuitem", { name: "Delete" });
Expand All @@ -207,7 +242,7 @@ describe("if there are volumes", () => {
const { user } = plainRender(<ProposalVolumes {...props} />);

const [, body] = await screen.findAllByRole("rowgroup");
const row = within(body).getByRole("row", { name: "/home XFS partition At least 1 KiB" });
const row = within(body).getByRole("row", { name: "/home XFS At least 1 KiB Partition at installation disk" });
const actions = within(row).getByRole("button", { name: "Actions" });
await user.click(actions);
const editAction = within(row).queryByRole("menuitem", { name: "Edit" });
Expand All @@ -227,7 +262,7 @@ describe("if there are volumes", () => {

const [, volumes] = await screen.findAllByRole("rowgroup");

within(volumes).getByRole("row", { name: "/ Btrfs partition transactional 1 KiB - 2 KiB" });
within(volumes).getByRole("row", { name: "/ Transactional Btrfs 1 KiB - 2 KiB Partition at installation disk" });
});
});

Expand All @@ -241,7 +276,45 @@ describe("if there are volumes", () => {

const [, volumes] = await screen.findAllByRole("rowgroup");

within(volumes).getByRole("row", { name: "/ Btrfs partition with snapshots 1 KiB - 2 KiB" });
within(volumes).getByRole("row", { name: "/ Btrfs with snapshots 1 KiB - 2 KiB Partition at installation disk" });
});
});

describe("and some volumes are allocated at separate disks", () => {
beforeEach(() => {
props.volumes = [
volumes.root,
{ ...volumes.swap, target: "new_partition", targetDevice: sda },
{ ...volumes.home, target: "new_vg", targetDevice: sda }
];
});

it("renders the locations", async () => {
plainRender(<ProposalVolumes {...props} />);

const [, volumes] = await screen.findAllByRole("rowgroup");

within(volumes).getByRole("row", { name: "swap Swap 1 KiB Partition at /dev/sda" });
within(volumes).getByRole("row", { name: "/home XFS At least 1 KiB Separate LVM at /dev/sda" });
});
});

describe("and some volumes are reusing existing block devices", () => {
beforeEach(() => {
props.volumes = [
volumes.root,
{ ...volumes.swap, target: "filesystem", targetDevice: sda1 },
{ ...volumes.home, target: "device", targetDevice: sda2 }
];
});

it("renders the locations", async () => {
plainRender(<ProposalVolumes {...props} />);

const [, volumes] = await screen.findAllByRole("rowgroup");

within(volumes).getByRole("row", { name: "swap Reused Swap 256 B /dev/sda1" });
within(volumes).getByRole("row", { name: "/home XFS 512 B /dev/sda2" });
});
});
});
Expand Down
Loading