From b6a0bc9be45c8806e3b3c6ede78306217056a4a5 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Mon, 17 Jun 2024 16:55:08 +0300 Subject: [PATCH] storage: Expose Stratis virtual filesystem sizes Instead of the "Managed filesystem sizes" concept. This makes Cockpit easier to discover when you know the real Stratis concepts and exposes its full capabilities. --- pkg/storaged/stratis/create-dialog.jsx | 8 +- pkg/storaged/stratis/filesystem.jsx | 81 ++++++++--- pkg/storaged/stratis/pool.jsx | 79 +++++++--- test/common/storagelib.py | 2 + test/verify/check-storage-stratis | 194 ++++++++++++++++--------- 5 files changed, 247 insertions(+), 117 deletions(-) diff --git a/pkg/storaged/stratis/create-dialog.jsx b/pkg/storaged/stratis/create-dialog.jsx index 7257f389439..3aaae079f19 100644 --- a/pkg/storaged/stratis/create-dialog.jsx +++ b/pkg/storaged/stratis/create-dialog.jsx @@ -103,13 +103,13 @@ export function create_stratis_pool() { }), ] }), - CheckBoxes("managed", "", + CheckBoxes("overprov", "", { + value: { on: true }, fields: [ { tag: "on", - title: _("Manage filesystem sizes"), - tooltip: _("When this option is checked, the new pool will not allow overprovisioning. You need to specify a maximum size for each filesystem that is created in the pool. Filesystems can not be made larger after creation. Snapshots are fully allocated on creation. The sum of all maximum sizes can not exceed the size of the pool. The advantage of this is that filesystems in this pool can not run out of space in a surprising way. The disadvantage is that you need to know the maximum size for each filesystem in advance and creation of snapshots is limited.") + title: _("Overprovisioning"), } ] }) @@ -130,7 +130,7 @@ export function create_stratis_pool() { clevis_info ? [true, clevis_info] : [false, ["", ""]]) .then(std_reply) .then(result => { - if (vals.managed && vals.managed.on && result[0]) { + if (vals.overprov && !vals.overprov.on && result[0]) { const path = result[1][0]; return client.wait_for(() => client.stratis_pools[path]) .then(pool => { diff --git a/pkg/storaged/stratis/filesystem.jsx b/pkg/storaged/stratis/filesystem.jsx index ef23f098d6c..a1427e5a61e 100644 --- a/pkg/storaged/stratis/filesystem.jsx +++ b/pkg/storaged/stratis/filesystem.jsx @@ -25,7 +25,7 @@ import { CardBody } from "@patternfly/react-core/dist/esm/components/Card/index. import { DescriptionList } from "@patternfly/react-core/dist/esm/components/DescriptionList/index.js"; import { - dialog_open, TextInput, BlockingMessage, TeardownMessage, + dialog_open, TextInput, CheckBoxes, SizeSlider, BlockingMessage, TeardownMessage, init_teardown_usage, } from "../dialog.jsx"; import { StorageUsageBar, StorageLink } from "../storage-controls.jsx"; @@ -47,7 +47,7 @@ import { std_reply, validate_fs_name, set_mount_options, destroy_filesystem } fr const _ = cockpit.gettext; export function make_stratis_filesystem_page(parent, pool, fsys, - offset, forced_options, managed_fsys_sizes) { + offset, forced_options) { const filesystems = client.stratis_pool_filesystems[pool.path]; const stats = client.stratis_pool_stats[pool.path]; const block = client.slashdevs_block[fsys.Devnode]; @@ -70,15 +70,6 @@ export function make_stratis_filesystem_page(parent, pool, fsys, } function snapshot_fsys() { - if (managed_fsys_sizes && stats.pool_free < Number(fsys.Size)) { - dialog_open({ - Title: _("Not enough space"), - Body: cockpit.format(_("There is not enough space in the pool to make a snapshot of this filesystem. At least $0 are required but only $1 are available."), - fmt_size(Number(fsys.Size)), fmt_size(stats.pool_free)) - }); - return; - } - dialog_open({ Title: cockpit.format(_("Create a snapshot of filesystem $0"), fsys.Name), Fields: [ @@ -157,21 +148,24 @@ export function make_stratis_filesystem_page(parent, pool, fsys, next: null, page_location: ["pool", pool.Name, fsys.Name], page_name: fsys.Name, - page_size: (!managed_fsys_sizes - ? - : ), + page_size: , has_warning: !!mismount_warning, component: StratisFilesystemCard, - props: { pool, fsys, fstab_config, forced_options, managed_fsys_sizes, mismount_warning, offset }, + props: { pool, fsys, fstab_config, forced_options, mismount_warning, offset }, actions: [ client.in_anaconda_mode() && { title: _("Edit mount point"), action: () => edit_mount_point(block, forced_options) }, (fs_is_mounted ? { title: _("Unmount"), action: unmount } : { title: _("Mount"), action: mount }), - { title: _("Snapshot"), action: snapshot_fsys }, + { + title: _("Snapshot"), + action: snapshot_fsys, + excuse: ((!pool.Overprovisioning && stats.pool_free < Number(fsys.Size)) + ? _("Not enough free space") + : null), + }, { title: _("Delete"), action: delete_fsys, danger: true }, ] }); @@ -180,7 +174,7 @@ export function make_stratis_filesystem_page(parent, pool, fsys, } const StratisFilesystemCard = ({ - card, pool, fsys, fstab_config, forced_options, managed_fsys_sizes, mismount_warning, offset, + card, pool, fsys, fstab_config, forced_options, mismount_warning, offset, }) => { const filesystems = client.stratis_pool_filesystems[pool.path]; const stats = client.stratis_pool_stats[pool.path]; @@ -206,6 +200,42 @@ const StratisFilesystemCard = ({ }); } + function set_limit() { + dialog_open({ + Title: _("Set limit of virtual filesystem size"), + Fields: [ + CheckBoxes("size_options", _("Options"), + { + value: { + custom_limit: fsys.SizeLimit[0], + }, + fields: [ + { tag: "custom_limit", title: _("Limit virtual filesystem size") }, + ] + }), + SizeSlider("limit", _("Virtual size limit"), + { + visible: vals => vals.size_options.custom_limit, + value: fsys.SizeLimit[0] && Number(fsys.SizeLimit[1]), + min: Number(fsys.Size), + max: pool.Overprovisioning ? stats.pool_total : stats.pool_free + Number(fsys.Size), + allow_infinite: true, + round: 512 + }), + ], + Action: { + Title: _("Set"), + action: async function (vals) { + await client.stratis_set_property(fsys, + "SizeLimit", + "(bs)", (vals.size_options.custom_limit + ? [true, vals.limit.toString()] + : [false, ""])); + } + } + }); + } + return ( - {(!managed_fsys_sizes - ? - : ) - } + + + {_("edit")} + } /> diff --git a/pkg/storaged/stratis/pool.jsx b/pkg/storaged/stratis/pool.jsx index 097d01d0044..e9a77a86bac 100644 --- a/pkg/storaged/stratis/pool.jsx +++ b/pkg/storaged/stratis/pool.jsx @@ -30,7 +30,7 @@ import { Flex, FlexItem } from "@patternfly/react-core/dist/esm/layouts/Flex/ind import { VolumeIcon } from "../icons/gnome-icons.jsx"; import { fmt_to_fragments } from "utils.jsx"; -import { StorageButton, StorageUsageBar, StorageLink } from "../storage-controls.jsx"; +import { StorageButton, StorageUsageBar, StorageLink, StorageOnOff } from "../storage-controls.jsx"; import { StorageCard, StorageDescription, ChildrenTable, PageTable, new_page, new_card, PAGE_CATEGORY_VIRTUAL, @@ -43,7 +43,7 @@ import { } from "../utils.js"; import { - dialog_open, SelectSpaces, TextInput, PassInput, SelectOne, SizeSlider, + dialog_open, SelectSpaces, TextInput, PassInput, SelectOne, SizeSlider, CheckBoxes, BlockingMessage, TeardownMessage, init_teardown_usage } from "../dialog.jsx"; @@ -72,7 +72,6 @@ function create_fs(pool) { const filesystems = client.stratis_pool_filesystems[pool.path]; const stats = client.stratis_pool_stats[pool.path]; const forced_options = ["x-systemd.requires=stratis-fstab-setup@" + pool.Uuid + ".service"]; - const managed_fsys_sizes = !pool.Overprovisioning; let action_variants; if (!client.in_anaconda_mode()) { @@ -93,11 +92,31 @@ function create_fs(pool) { { validate: name => validate_fs_name(null, name, filesystems) }), - SizeSlider("size", _("Size"), + CheckBoxes("size_options", _("Manage virtual size"), { - visible: () => managed_fsys_sizes, + value: { + custom_size: !pool.Overprovisioning, + custom_limit: false, + }, + fields: [ + { tag: "custom_size", title: _("Specify initial virtual filesystem size") }, + { tag: "custom_limit", title: _("Limit virtual filesystem size") }, + ] + }), + SizeSlider("size", _("Initial virtual size"), + { + visible: vals => vals.size_options.custom_size, + min: fsys_min_size, + max: pool.Overprovisioning ? stats.pool_total : stats.pool_free, + allow_infinite: pool.Overprovisioning, + round: 512 + }), + SizeSlider("limit", _("Virtual size limit"), + { + visible: vals => vals.size_options.custom_limit, min: fsys_min_size, - max: stats.pool_free, + max: pool.Overprovisioning ? stats.pool_total : stats.pool_free, + allow_infinite: true, round: 512 }), TextInput("mount_point", _("Mount point"), @@ -116,12 +135,12 @@ function create_fs(pool) { Action: { Variants: action_variants, action: async function (vals) { - let size_spec = [false, ""]; - - if (managed_fsys_sizes) + let size_spec = [false, ""]; let limit_spec = [false, ""]; + if (vals.size_options.custom_size) size_spec = [true, vals.size.toString()]; - - const result = await pool.CreateFilesystems([[vals.name, size_spec, [false, ""]]]).then(std_reply); + if (vals.size_options.custom_limit) + limit_spec = [true, vals.limit.toString()]; + const result = await pool.CreateFilesystems([[vals.name, size_spec, limit_spec]]).then(std_reply); if (result[0]) await set_mount_options(result[1][0][0], vals, forced_options); } @@ -244,12 +263,10 @@ function make_stratis_filesystem_pages(parent, pool) { const filesystems = client.stratis_pool_filesystems[pool.path]; const stats = client.stratis_pool_stats[pool.path]; const forced_options = ["x-systemd.requires=stratis-fstab-setup@" + pool.Uuid + ".service"]; - const managed_fsys_sizes = !pool.Overprovisioning; filesystems.forEach((fs, i) => make_stratis_filesystem_page(parent, pool, fs, stats.fsys_offsets[i], - forced_options, - managed_fsys_sizes)); + forced_options)); } export function make_stratis_pool_page(parent, pool) { @@ -257,7 +274,6 @@ export function make_stratis_pool_page(parent, pool) { const blockdevs = client.stratis_pool_blockdevs[pool.path] || []; const can_grow = blockdevs.some(bd => (bd.NewPhysicalSize[0] && Number(bd.NewPhysicalSize[1]) > Number(bd.TotalPhysicalSize))); - const managed_fsys_sizes = !pool.Overprovisioning; const stats = client.stratis_pool_stats[pool.path]; const use = pool.TotalPhysicalUsed[0] && [Number(pool.TotalPhysicalUsed[1]), Number(pool.TotalPhysicalSize)]; @@ -272,11 +288,11 @@ export function make_stratis_pool_page(parent, pool) { page_name: pool.Name, page_icon: VolumeIcon, page_category: PAGE_CATEGORY_VIRTUAL, - page_size: ((!managed_fsys_sizes && use) + page_size: (use ? : Number(pool.TotalPhysicalSize)), component: StratisPoolCard, - props: { pool, degraded_ops, can_grow, managed_fsys_sizes, stats }, + props: { pool, degraded_ops, can_grow, stats }, actions: [ { title: _("Add block devices"), @@ -295,13 +311,13 @@ export function make_stratis_pool_page(parent, pool) { next: pool_card, has_warning: degraded_ops || can_grow, component: StratisFilesystemsCard, - props: { pool, degraded_ops, can_grow, managed_fsys_sizes, stats }, + props: { pool, degraded_ops, can_grow, stats }, actions: [ { title: _("Create new filesystem"), action: () => create_fs(pool), - excuse: (managed_fsys_sizes && stats.pool_free < fsys_min_size - ? _("Not enough space") + excuse: ((!pool.Overprovisioning && stats.pool_free < fsys_min_size) + ? _("Not enough free space") : null), }, ], @@ -312,7 +328,7 @@ export function make_stratis_pool_page(parent, pool) { make_stratis_filesystem_pages(p, pool); } -const StratisFilesystemsCard = ({ card, pool, degraded_ops, can_grow, managed_fsys_sizes, stats }) => { +const StratisFilesystemsCard = ({ card, pool, degraded_ops, can_grow, stats }) => { const blockdevs = client.stratis_pool_blockdevs[pool.path] || []; function grow_blockdevs() { @@ -359,7 +375,7 @@ const StratisFilesystemsCard = ({ card, pool, degraded_ops, can_grow, managed_fs ); }; -const StratisPoolCard = ({ card, pool, degraded_ops, can_grow, managed_fsys_sizes, stats }) => { +const StratisPoolCard = ({ card, pool, degraded_ops, can_grow, stats }) => { const key_desc = (pool.Encrypted && pool.KeyDescription[0] && pool.KeyDescription[1][1]); @@ -514,9 +530,24 @@ const StratisPoolCard = ({ card, pool, degraded_ops, can_grow, managed_fsys_size {_("edit")} } /> - { !managed_fsys_sizes && use && + { use && - + + + } + + client.stratis_set_property(pool, + "Overprovisioning", + "b", !pool.Overprovisioning)} + excuse={(pool.Overprovisioning && stats.fsys_total_size > stats.pool_total) + ? _("Virtual filesystem sizes are larger than the pool. Overprovisioning can not be disabled.") + : null} /> + + { !pool.Overprovisioning && + + } { pool.Encrypted && diff --git a/test/common/storagelib.py b/test/common/storagelib.py index 908566ac45c..b3900d8d56a 100644 --- a/test/common/storagelib.py +++ b/test/common/storagelib.py @@ -214,6 +214,8 @@ def dialog_wait_val(self, field, val, unit=None): self.browser.wait_val(sel + " .size-text input", str(val)) elif ftype == "select": self.browser.wait_attr(sel, "data-value", val) + elif ftype == "checkbox": + self.browser.wait_visible(sel + (":checked" if val else ":not(:checked)")) else: self.browser.wait_val(sel, val) diff --git a/test/verify/check-storage-stratis b/test/verify/check-storage-stratis index 722b7724df9..7bb5681bdfe 100755 --- a/test/verify/check-storage-stratis +++ b/test/verify/check-storage-stratis @@ -406,6 +406,135 @@ systemctl restart stratisd b.wait_visible(self.card("Storage")) b.wait_not_present(self.card_row("Storage", name="TEST1")) + def testOverprovisioning(self): + b = self.browser + + self.login_and_go("/storage") + + dev_1 = self.add_loopback_disk(PV_SIZE) + b.wait_visible(self.card_row("Storage", name=dev_1)) + + # Create a pool with overprov off + self.dialog_with_retry(trigger=lambda: self.click_devices_dropdown("Create Stratis pool"), + expect=lambda: (self.dialog_is_present('disks', dev_1) and + self.dialog_check({"name": "pool0"})), + values={ + "disks": {dev_1: True}, + "overprov.on": False + }) + self.click_card_row("Storage", name="pool0") + + overprov_toggle = "input[aria-label='Allow overprovisioning']" + b.wait_visible(f"{overprov_toggle}:not(:checked)") + + # Create filesystem with defaults. Initial virtual size + # should be appropriate. + + b.click(self.card_button("Stratis filesystems", "Create new filesystem")) + self.dialog_wait_open() + self.dialog_wait_val('size_options.custom_size', val=True) + self.dialog_set_val('name', 'fsys1') + self.dialog_apply_secondary() + self.dialog_wait_close() + + # Snapshot should be disabled now + + sel = self.card_row("Stratis filesystems", name="fsys1") + b.click(self.dropdown_toggle(sel)) + b.wait_visible(self.dropdown_action(sel, "Snapshot") + "[disabled]") + b.wait_text(self.dropdown_description(sel, "Snapshot"), + "Not enough free space") + b.click(self.dropdown_toggle(sel)) + + # Switch overprov on + + b.click(overprov_toggle) + b.wait_visible(f"{overprov_toggle}:checked") + + # Snapshot succeeds + + self.click_dropdown(self.card_row("Stratis filesystems", name="fsys1"), "Snapshot") + self.dialog_wait_open() + self.dialog_set_val('name', 'snap1') + self.dialog_apply_secondary() + self.dialog_wait_close() + + # Filesystem creation does not ask for file size by default + + b.click(self.card_button("Stratis filesystems", "Create new filesystem")) + self.dialog_wait_open() + self.dialog_wait_val('size_options.custom_size', val=False) + self.dialog_cancel() + self.dialog_wait_close() + + # Turning overprov off should be disabled now + + b.wait_visible(overprov_toggle + "[disabled]") + + # Delete snapshot + + self.click_dropdown(self.card_row("Stratis filesystems", name="snap1"), "Delete") + self.confirm() + + # Switch overprov off + + b.click(overprov_toggle) + b.wait_visible(f"{overprov_toggle}:not(:checked)") + + def testFilesystemSizes(self): + b = self.browser + + self.login_and_go("/storage") + + dev_1 = self.add_loopback_disk(PV_SIZE) + b.wait_visible(self.card_row("Storage", name=dev_1)) + + # Create a pool + self.dialog_with_retry(trigger=lambda: self.click_devices_dropdown("Create Stratis pool"), + expect=lambda: (self.dialog_is_present('disks', dev_1) and + self.dialog_check({"name": "pool0"})), + values={"disks": {dev_1: True}}) + self.click_card_row("Storage", name="pool0") + + # Create filesystem with initial virtual size and a limit + + b.click(self.card_button("Stratis filesystems", "Create new filesystem")) + self.dialog_wait_open() + self.dialog_set_val('name', 'fsys1') + self.dialog_set_val('size_options.custom_size', val=True) + self.dialog_set_val('size', 800) + self.dialog_set_val('size_options.custom_limit', val=True) + self.dialog_set_val('limit', 1600) + self.dialog_apply_secondary() + self.dialog_wait_close() + + self.click_card_row("Stratis filesystems", name="fsys1") + + b.wait_text(self.card_desc("Stratis filesystem", "Virtual size"), "800 MB") + b.wait_text(self.card_desc("Stratis filesystem", "Virtual size limit"), "1.60 GB") + + # Raise the limit + + b.click(self.card_desc_action("Stratis filesystem", "Virtual size limit")) + self.dialog_wait_open() + self.dialog_wait_val('size_options.custom_limit', val=True) + self.dialog_wait_val('limit', 1600) + self.dialog_set_val('limit', 3200) + self.dialog_apply() + self.dialog_wait_close() + + b.wait_text(self.card_desc("Stratis filesystem", "Virtual size limit"), "3.20 GB") + + # Remove limit + + b.click(self.card_desc_action("Stratis filesystem", "Virtual size limit")) + self.dialog_wait_open() + self.dialog_set_val('size_options.custom_limit', val=False) + self.dialog_apply() + self.dialog_wait_close() + + b.wait_text(self.card_desc("Stratis filesystem", "Virtual size limit"), "none") + @testlib.skipImage("No Stratis", "debian-*", "ubuntu-*") class TestStorageStratisReboot(storagelib.StorageCase): @@ -640,71 +769,6 @@ class TestStorageStratisReboot(storagelib.StorageCase): self.assertIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /foo")) destroy() - @testlib.skipImage("Stratis too old", "rhel-8-*") - def testManagedSizes(self): - m = self.machine - b = self.browser - - self.login_and_go("/storage") - - dev_1 = "/dev/sda" - m.add_disk("4G", serial="DISK1") - b.wait_visible(self.card_row("Storage", name=dev_1)) - - # Create a "managed" pool - self.dialog_with_retry(trigger=lambda: self.click_devices_dropdown("Create Stratis pool"), - expect=lambda: (self.dialog_is_present('disks', dev_1) and - self.dialog_check({"name": "pool0"})), - values={"managed.on": True, "disks": {dev_1: True}}) - self.click_card_row("Storage", name="pool0") - - # Create a small filesystem - b.click(self.card_button("Stratis filesystems", "Create new filesystem")) - self.dialog({'name': 'fsys1', - 'size': 900, - 'mount_point': '/run/fsys1'}) - b.wait_text(self.card_row_col("Stratis filesystems", 1, 1), "fsys1") - - # Make a snapshot of it - self.click_dropdown(self.card_row("Stratis filesystems", 1), "Snapshot") - self.dialog({'name': 'fsys1-copy', - 'mount_point': '/run/fsys1-copy'}) - b.wait_text(self.card_row_col("Stratis filesystems", 2, 1), "fsys1-copy") - - # And another filesystem - b.click(self.card_button("Stratis filesystems", "Create new filesystem")) - self.dialog({'name': 'fsys2', - 'size': 800, - 'mount_point': '/run/fsys2'}) - b.wait_text(self.card_row_col("Stratis filesystems", 3, 1), "fsys2") - - # And fill the rest by accepting the default size - b.click(self.card_button("Stratis filesystems", "Create new filesystem")) - self.dialog({'name': 'fsys3', - 'mount_point': '/run/fsys3'}) - b.wait_text(self.card_row_col("Stratis filesystems", 4, 1), "fsys3") - b.wait_visible(self.card_button("Stratis filesystems", "Create new filesystem") + ":disabled") - - # Snapshots are impossible now - self.click_dropdown(self.card_row("Stratis filesystems", 1), "Snapshot") - self.dialog_wait_open() - b.wait_in_text('#dialog', "Not enough space") - self.dialog_cancel() - self.dialog_wait_close() - - # Delete a filesystem, and make another snapshot - self.click_card_row("Stratis filesystems", 1) - self.click_card_dropdown("Stratis filesystem", "Delete") - self.confirm() - b.wait_visible(self.card_button("Stratis filesystems", "Create new filesystem") + ":not(:disabled)") - self.click_dropdown(self.card_row("Stratis filesystems", 2), "Snapshot") - self.dialog({'name': 'fsys2-copy', - 'mount_point': '/run/fsys2-copy'}) - b.wait_visible(self.card_row("Stratis filesystems", name="fsys2-copy")) - - # And the pool should be full again - b.wait_visible("button:contains(Create new filesystem):disabled") - @testlib.skipImage("Stratis too old", "rhel-8-*") def testPoolResize(self): m = self.machine