From 03cea3558caf8f6889dc57c483b13b9b2f61d608 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Tue, 9 Jan 2024 09:53:36 +0200 Subject: [PATCH] storage: Don't offer to mount while formatting in Anaconda mode It's the normal thing to do during installation. --- pkg/storaged/block/format-dialog.jsx | 23 ++++-- pkg/storaged/filesystem/utils.jsx | 40 ++++++---- pkg/storaged/stratis/pool.jsx | 15 +++- test/verify/check-storage-anaconda | 111 ++++++++++++++++++++------- 4 files changed, 140 insertions(+), 49 deletions(-) diff --git a/pkg/storaged/block/format-dialog.jsx b/pkg/storaged/block/format-dialog.jsx index e94f26f7f515..be0a29b6c8c5 100644 --- a/pkg/storaged/block/format-dialog.jsx +++ b/pkg/storaged/block/format-dialog.jsx @@ -271,8 +271,20 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended, else at_boot = "local"; - const action_title = create_partition ? _("Create and mount") : _("Format and mount"); - const action_variant = { tag: "nomount", Title: create_partition ? _("Create only") : _("Format only") }; + let action_variants = [ + { tag: null, Title: create_partition ? _("Create and mount") : _("Format and mount") }, + { tag: "nomount", Title: create_partition ? _("Create only") : _("Format only") } + ]; + + const action_variants_for_empty = [ + { tag: null, Title: create_partition ? _("Create") : _("Format") } + ]; + + if (client.in_anaconda_mode()) { + action_variants = [ + { tag: "nomount", Title: create_partition ? _("Create") : _("Format") } + ]; + } const dlg = dialog_open({ Title: title, @@ -402,16 +414,15 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended, dlg.set_options("at_boot", { explanation: mount_explanation[vals.at_boot] }); else if (trigger == "type") { if (dlg.get_value("type") == "empty") { - dlg.update_actions({ Variants: null, Title: _("Format") }); + dlg.update_actions({ Variants: action_variants_for_empty }); } else { - dlg.update_actions({ Variants: [action_variant], Title: action_title }); + dlg.update_actions({ Variants: action_variants }); } } }, Action: { - Title: action_title, + Variants: action_variants, Danger: (create_partition ? null : _("Formatting erases all data on a storage device.")), - Variants: [action_variant], wrapper: job_progress_wrapper(client, block.path, client.blocks_cleartext[block.path]?.path), disable_on_error: usage.Teardown, action: function (vals) { diff --git a/pkg/storaged/filesystem/utils.jsx b/pkg/storaged/filesystem/utils.jsx index 18ad8419057f..a2c0801f5c18 100644 --- a/pkg/storaged/filesystem/utils.jsx +++ b/pkg/storaged/filesystem/utils.jsx @@ -147,24 +147,35 @@ export const MountPoint = ({ fstab_config, forced_options, backing_block, conten } let extra_text = null; - if (!is_filesystem_mounted) { + if (client.in_anaconda_mode()) { if (!old_dir) - extra_text = _("The filesystem has no permanent mount point."); - else - extra_text = _("The filesystem is not mounted."); - } else if (backing_block != content_block) { - if (!opt_never_auto) - extra_text = _("The filesystem will be unlocked and mounted on the next boot. This might require inputting a passphrase."); + extra_text = _("The filesystem has no assigned mount point."); + } else { + if (!is_filesystem_mounted) { + if (!old_dir) + extra_text = _("The filesystem has no permanent mount point."); + else + extra_text = _("The filesystem is not mounted."); + } else if (backing_block != content_block) { + if (!opt_never_auto) + extra_text = _("The filesystem will be unlocked and mounted on the next boot. This might require inputting a passphrase."); + } + } + + if (!mount_point_text) { + mount_point_text = extra_text; + extra_text = null; } - if (extra_text && mount_point_text) + if (extra_text) extra_text = <>
{extra_text}; return ( <> - { mount_point_text && + { mount_point_text && { mount_point_text } + } mounting_dialog(client, content_block || backing_block, @@ -174,7 +185,6 @@ export const MountPoint = ({ fstab_config, forced_options, backing_block, conten - } { extra_text } ); }; @@ -185,9 +195,13 @@ export const mount_point_text = (mount_point, mounted) => { mp_text = client.strip_mount_point_prefix(mount_point); if (mp_text == false) return null; - if (!mounted) + if (!mounted && !client.in_anaconda_mode()) mp_text = mp_text + " " + _("(not mounted)"); - } else - mp_text = _("(not mounted)"); + } else { + if (client.in_anaconda_mode()) + mp_text = _("(no assigned mount point)"); + else + mp_text = _("(not mounted)"); + } return mp_text; }; diff --git a/pkg/storaged/stratis/pool.jsx b/pkg/storaged/stratis/pool.jsx index 13e63348e24c..0dfbd0235df9 100644 --- a/pkg/storaged/stratis/pool.jsx +++ b/pkg/storaged/stratis/pool.jsx @@ -74,6 +74,18 @@ function create_fs(pool) { const forced_options = ["x-systemd.requires=stratis-fstab-setup@" + pool.Uuid + ".service"]; const managed_fsys_sizes = client.features.stratis_managed_fsys_sizes && !pool.Overprovisioning; + let action_variants; + if (!client.in_anaconda_mode()) { + action_variants = [ + { tag: null, Title: _("Create and mount") }, + { tag: "nomount", Title: _("Create only") }, + ]; + } else { + action_variants = [ + { tag: "nomount", Title: _("Create") }, + ]; + } + dialog_open({ Title: _("Create filesystem"), Fields: [ @@ -137,8 +149,7 @@ function create_fs(pool) { dlg.set_options("at_boot", { explanation: mount_explanation[vals.at_boot] }); }, Action: { - Title: _("Create and mount"), - Variants: [{ tag: "nomount", Title: _("Create only") }], + Variants: action_variants, action: function (vals) { return client.stratis_create_filesystem(pool, vals.name, vals.size) .then(std_reply) diff --git a/test/verify/check-storage-anaconda b/test/verify/check-storage-anaconda index 03cc99748ee9..4052df4062ac 100755 --- a/test/verify/check-storage-anaconda +++ b/test/verify/check-storage-anaconda @@ -26,6 +26,12 @@ import testlib @testlib.nondestructive class TestStorageAnaconda(storagelib.StorageCase): + def enterAnacondaMode(self, config): + b = self.browser + b.call_js_func("window.localStorage.setItem", "cockpit_anaconda", json.dumps(config)) + b.reload() + b.enter_page("/storage") + def testBasic(self): m = self.machine b = self.browser @@ -38,9 +44,7 @@ class TestStorageAnaconda(storagelib.StorageCase): } self.login_and_go("/storage") - b.call_js_func("window.localStorage.setItem", "cockpit_anaconda", json.dumps(anaconda_config)) - b.reload() - b.enter_page("/storage") + self.enterAnacondaMode(anaconda_config) # There should be only one row, for our disk b.wait(lambda: b.call_js_func('ph_count', self.card("Storage") + " tbody tr") == 1) @@ -62,28 +66,23 @@ class TestStorageAnaconda(storagelib.StorageCase): self.dialog_wait_open() self.dialog_wait_val("at_boot", "local") self.dialog_set_val("type", "ext4") + self.dialog_set_val("mount_point", "/") self.dialog_set_val("crypto", self.default_crypto_type) self.dialog_set_val("passphrase", "vainu-reku-toma-rolle-kaja") self.dialog_set_val("passphrase2", "vainu-reku-toma-rolle-kaja") - # Empty mount point should be failure - self.dialog_set_val("mount_point", "") - self.dialog_apply() - self.dialog_wait_error("mount_point", "Mount point cannot be empty") - self.dialog_set_val("mount_point", "/") self.dialog_apply() self.dialog_wait_close() - self.assertNotIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + self.assertIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) self.assertNotIn("nofail", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) - b.wait_visible(self.card_row_col("Storage", 3, 5) + " .usage-bar[role=progressbar]") b.assert_pixels("body", "page") - # Unount/mount the filesystem, edit mount options + # Mount/Unmount the filesystem, edit mount options self.click_card_row("Storage", location="/") b.wait_visible(self.card("Encryption")) - b.wait_in_text(self.card_desc("ext4 filesystem", "Mount point"), "/ (stop boot on failure)") - b.click(self.card_desc("ext4 filesystem", "Mount point") + " button") + b.wait_in_text(self.card_desc("Filesystem", "Mount point"), "/ (stop boot on failure)") + b.click(self.card_desc("Filesystem", "Mount point") + " button") self.dialog_wait_open() self.dialog_wait_val("mount_point", "/") self.dialog_set_val("mount_options.ro", val=True) @@ -95,21 +94,28 @@ class TestStorageAnaconda(storagelib.StorageCase): self.dialog_apply() self.dialog_wait_close() self.assertIn("ro", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + + b.click(self.card_button("Filesystem", "Mount")) + self.dialog_wait_open() + self.dialog_wait_val("mount_point", "/") + self.dialog_set_val("passphrase", "vainu-reku-toma-rolle-kaja") + self.dialog_apply() + self.dialog_wait_close() + self.assertNotIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + b.click(self.card_button("ext4 filesystem", "Unmount")) self.dialog_wait_open() b.wait_text("#dialog .pf-v5-c-modal-box__title-text", "Unmount filesystem /") self.dialog_apply() self.dialog_wait_close() - self.wait_not_mounted("Filesystem") self.assertIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + + # Mount again, to check location in tear down information b.click(self.card_button("Filesystem", "Mount")) self.dialog_wait_open() - self.dialog_wait_val("mount_point", "/") self.dialog_set_val("passphrase", "vainu-reku-toma-rolle-kaja") self.dialog_apply() self.dialog_wait_close() - self.wait_mounted("ext4 filesystem") - self.assertNotIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) # Check and delete volume group b.click(self.card_parent_link()) @@ -144,9 +150,7 @@ class TestStorageAnaconda(storagelib.StorageCase): } self.login_and_go("/storage") - b.call_js_func("window.localStorage.setItem", "cockpit_anaconda", json.dumps(anaconda_config)) - b.reload() - b.enter_page("/storage") + self.enterAnacondaMode(anaconda_config) # Create a Stratis pool self.click_devices_dropdown("Create Stratis pool") @@ -163,12 +167,12 @@ class TestStorageAnaconda(storagelib.StorageCase): self.dialog_apply() self.dialog_wait_close() - self.assertNotIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + self.assertIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) self.assertNotIn("nofail", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) b.wait_visible(self.card_row_col("Storage", 3, 5) + " .usage-bar[role=progressbar]") - # Unount/mount the filesystem, edit mount options + # Mount/Unmount the filesystem, edit mount options self.click_card_row("Storage", location="/") b.wait_in_text(self.card_desc("Stratis filesystem", "Mount point"), "/ (stop boot on failure)") b.click(self.card_desc("Stratis filesystem", "Mount point") + " button") @@ -178,19 +182,24 @@ class TestStorageAnaconda(storagelib.StorageCase): self.dialog_apply() self.dialog_wait_close() self.assertIn("ro", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + + b.click(self.card_button("Stratis filesystem", "Mount")) + self.dialog_wait_open() + self.dialog_wait_val("mount_point", "/") + self.dialog_apply() + self.dialog_wait_close() + self.assertNotIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + b.click(self.card_button("Stratis filesystem", "Unmount")) self.dialog_wait_open() b.wait_text("#dialog .pf-v5-c-modal-box__title-text", "Unmount filesystem /") self.dialog_apply() self.dialog_wait_close() - self.wait_not_mounted("Stratis filesystem") self.assertIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) + + # Mount again, to check location in tear down information b.click(self.card_button("Stratis filesystem", "Mount")) - self.dialog_wait_open() - self.dialog_wait_val("mount_point", "/") - self.dialog_apply() - self.dialog_wait_close() - self.wait_mounted("Stratis filesystem") + self.confirm() self.assertNotIn("noauto", m.execute("findmnt --fstab -n -o OPTIONS /sysroot")) # Check and delete pool @@ -203,6 +212,52 @@ class TestStorageAnaconda(storagelib.StorageCase): self.dialog_wait_close() m.execute("! findmnt --fstab -n /sysroot") + def testNoMounting(self): + m = self.machine + b = self.browser + + disk = self.add_ram_disk() + + anaconda_config = { + "mount_point_prefix": "/sysroot", + "available_devices": [disk], + } + + self.login_and_go("/storage") + self.enterAnacondaMode(anaconda_config) + + # Create a partition, there should be no "Create and mount" button + b.wait_text(self.card_row_col("Storage", 1, 3), "Unformatted data") + self.click_dropdown(self.card_row("Storage", 1), "Create partition table") + self.confirm() + b.wait_text(self.card_row_col("Storage", 2, 2), "Free space") + + # Only one apply button in the Create Partition dialog + self.click_dropdown(self.card_row("Storage", 2), "Create partition") + self.dialog_wait_open() + self.dialog_set_val("type", "ext4") + b.wait(lambda: b.call_js_func('ph_count', "#dialog button.apply") == 1) + b.wait_text("#dialog button.apply", "Create") + self.dialog_apply() + self.dialog_wait_close() + + # Page talks about assigned mount points instead of "not mounted". + b.wait_text(self.card_row_col("Storage", 2, 4), "(no assigned mount point)") + + # Only one apply button in the Format dialog + self.click_dropdown(self.card_row("Storage", 2), "Format") + self.dialog_wait_open() + self.dialog_set_val("mount_point", "/boot") + self.dialog_set_val("type", "ext4") + b.wait(lambda: b.call_js_func('ph_count', "#dialog button.apply") == 1) + b.wait_text("#dialog button.apply", "Format") + self.dialog_apply() + self.dialog_wait_close() + + # Filesystem is not mounted but page doesn't mention "not mounted". + m.execute(f"! findmnt {disk}") + b.wait_text(self.card_row_col("Storage", 2, 4), "/boot") + if __name__ == '__main__': testlib.test_main()