From 7e5ff8ea1af704d58553d53f17188bc694b2cc0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 11 Feb 2025 11:54:13 +0000 Subject: [PATCH 1/4] storage: API for resetting the config --- .../bus/org.opensuse.Agama.Storage1.bus.xml | 3 +++ doc/dbus/org.opensuse.Agama.Storage1.doc.xml | 11 ++++++++++ rust/agama-lib/src/storage/client.rs | 5 +++++ .../agama-lib/src/storage/proxies/storage1.rs | 3 +++ rust/agama-server/src/storage/web.rs | 19 ++++++++++++++++++ service/lib/agama/dbus/storage/manager.rb | 18 ++++++++++++++++- service/lib/agama/storage/manager.rb | 20 +++++++++---------- .../test/agama/dbus/storage/manager_test.rb | 1 + 8 files changed, 69 insertions(+), 11 deletions(-) diff --git a/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml index 6a6b2cb38b..05e32dceac 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml @@ -51,6 +51,9 @@ + + + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.doc.xml index b199b5925f..3f06d349a1 100644 --- a/doc/dbus/org.opensuse.Agama.Storage1.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Storage1.doc.xml @@ -39,6 +39,17 @@ --> + + + + + diff --git a/rust/agama-lib/src/storage/client.rs b/rust/agama-lib/src/storage/client.rs index 381c747209..be11915821 100644 --- a/rust/agama-lib/src/storage/client.rs +++ b/rust/agama-lib/src/storage/client.rs @@ -152,6 +152,11 @@ impl<'a> StorageClient<'a> { .await?) } + /// Reset the storage config to the default value + pub async fn reset_config(&self) -> Result { + Ok(self.storage_proxy.reset_config().await?) + } + /// Get the storage config according to the JSON schema pub async fn get_config(&self) -> Result { let serialized_settings = self.storage_proxy.get_config().await?; diff --git a/rust/agama-lib/src/storage/proxies/storage1.rs b/rust/agama-lib/src/storage/proxies/storage1.rs index 2dc37e05bb..9d5b70faa0 100644 --- a/rust/agama-lib/src/storage/proxies/storage1.rs +++ b/rust/agama-lib/src/storage/proxies/storage1.rs @@ -62,6 +62,9 @@ pub trait Storage1 { /// Set the storage config according to the JSON schema fn set_config(&self, settings: &str) -> zbus::Result; + /// Reset the storage config to the default value + fn reset_config(&self) -> zbus::Result; + /// Get the current storage config according to the JSON schema fn get_config(&self) -> zbus::Result; diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 60e69beb98..b1bb497224 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -114,6 +114,7 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result>) -> Result, Error> { + let _status: u32 = state.client.reset_config().await.map_err(Error::Service)?; + Ok(Json(())) +} + /// Sets the storage config model. /// /// * `state`: service state. diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 938e8fb244..d17f951d42 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -45,7 +45,7 @@ module Agama module DBus module Storage # D-Bus object to manage storage installation - class Manager < BaseObject + class Manager < BaseObject # rubocop:disable Metrics/ClassLength include WithISCSIAuth include WithServiceStatus include ::DBus::ObjectManager @@ -119,6 +119,15 @@ def apply_config(serialized_config) proposal.success? ? 0 : 1 end + # Calculates the initial proposal. + # + # @return [Integer] 0 success; 1 error + def reset_config + logger.info("Reset storage config from D-Bus") + backend.calculate_proposal + backend.proposal.success? ? 0 : 1 + end + # Gets and serializes the storage config used for calculating the current proposal. # # @return [String] @@ -174,12 +183,19 @@ def deprecated_system backend.deprecated_system? end + # FIXME: Revisit return values. + # * Methods like #SetConfig or #ResetConfig return whether the proposal successes, but + # they should return whether the config was actually applied. + # * Methods like #Probe or #Install return nothing. dbus_interface STORAGE_INTERFACE do dbus_method(:Probe) { probe } dbus_method(:Reprobe) { probe(keep_config: true) } dbus_method(:SetConfig, "in serialized_config:s, out result:u") do |serialized_config| busy_while { apply_config(serialized_config) } end + dbus_method(:ResetConfig, "out result:u") do + busy_while { reset_config } + end dbus_method(:GetConfig, "out serialized_config:s") { recover_config } dbus_method(:SetConfigModel, "in serialized_model:s, out result:u") do |serialized_model| busy_while { apply_config_model(serialized_model) } diff --git a/service/lib/agama/storage/manager.rb b/service/lib/agama/storage/manager.rb index 48a28c427b..cee30a8bef 100644 --- a/service/lib/agama/storage/manager.rb +++ b/service/lib/agama/storage/manager.rb @@ -156,6 +156,16 @@ def finish Finisher.new(logger, product_config, security).run end + # Calculates the proposal. + # + # @param keep_config [Boolean] Whether to use the current storage config for calculating the + # proposal. If false, then the default config from the product is used. + def calculate_proposal(keep_config: false) + config_json = proposal.storage_json if keep_config + config_json ||= ConfigJSONReader.new(product_config).read + proposal.calculate_from_json(config_json) + end + # Storage proposal manager # # @return [Storage::Proposal] @@ -225,16 +235,6 @@ def probe_devices Y2Storage::StorageManager.instance.probe(callbacks) end - # Calculates the proposal. - # - # @param keep_config [Boolean] Whether to use the current storage config for calculating the - # proposal. If false, then the default config from the product is used. - def calculate_proposal(keep_config: false) - config_json = proposal.storage_json if keep_config - config_json ||= ConfigJSONReader.new(product_config).read - proposal.calculate_from_json(config_json) - end - # Adds the required packages to the list of resolvables to install def add_packages devicegraph = Y2Storage::StorageManager.instance.staging diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 23496d79e3..d9593d5db2 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -54,6 +54,7 @@ def serialize(value) iscsi: iscsi, software: software, product_config: product_config, + calculate_proposal: nil, on_probe: nil, on_progress_change: nil, on_progress_finish: nil, From 0a09305659eaaa0e595419e2485e24e527764364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 11 Feb 2025 11:59:58 +0000 Subject: [PATCH 2/4] web: add hook for setting the default storage config --- web/src/api/storage.ts | 3 +++ web/src/queries/storage.ts | 22 ++++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/web/src/api/storage.ts b/web/src/api/storage.ts index b2355c2ff2..22d2556576 100644 --- a/web/src/api/storage.ts +++ b/web/src/api/storage.ts @@ -41,6 +41,8 @@ const fetchConfigModel = (): Promise => const setConfig = (config: config.Config) => put("/api/storage/config", { storage: config }); +const resetConfig = () => put("/api/storage/config/reset", {}); + const setConfigModel = (model: configModel.Config) => put("/api/storage/config_model", model); const solveConfigModel = (model: configModel.Config): Promise => { @@ -79,6 +81,7 @@ export { fetchConfig, fetchConfigModel, setConfig, + resetConfig, setConfigModel, solveConfigModel, fetchUsableDevices, diff --git a/web/src/queries/storage.ts b/web/src/queries/storage.ts index 29e0277e16..32570b0525 100644 --- a/web/src/queries/storage.ts +++ b/web/src/queries/storage.ts @@ -25,6 +25,7 @@ import React from "react"; import { fetchConfig, setConfig, + resetConfig, fetchActions, fetchVolumes, fetchProductParams, @@ -66,20 +67,17 @@ const useConfigMutation = () => { return useMutation(query); }; -/** @todo Call to an API method to reset the default config instead of setting a config. */ +/** + * Hook for setting the default config. + */ const useResetConfigMutation = () => { - const { mutate } = useConfigMutation(); - - return { - mutate: () => - mutate({ - drives: [ - { - partitions: [{ search: "*", delete: true }, { generate: "default" }], - }, - ], - }), + const queryClient = useQueryClient(); + const query = { + mutationFn: async () => await resetConfig(), + onSuccess: () => queryClient.invalidateQueries({ queryKey: ["storage"] }), }; + + return useMutation(query); }; const devicesQuery = (scope: "result" | "system") => ({ From 941f57a975f7141b6034113986fc0e0c9a52f407 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 11 Feb 2025 12:00:50 +0000 Subject: [PATCH 3/4] web: add menu option for resetting to defaults --- .../storage/ConfigEditorMenu.test.tsx | 136 ++++++++++++++++++ .../components/storage/ConfigEditorMenu.tsx | 33 +++-- 2 files changed, 160 insertions(+), 9 deletions(-) create mode 100644 web/src/components/storage/ConfigEditorMenu.test.tsx diff --git a/web/src/components/storage/ConfigEditorMenu.test.tsx b/web/src/components/storage/ConfigEditorMenu.test.tsx new file mode 100644 index 0000000000..61e10fbe83 --- /dev/null +++ b/web/src/components/storage/ConfigEditorMenu.test.tsx @@ -0,0 +1,136 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender, mockNavigateFn } from "~/test-utils"; +import ConfigEditorMenu from "./ConfigEditorMenu"; +import { STORAGE as PATHS } from "~/routes/paths"; + +const mockUseZFCPSupported = jest.fn(); +jest.mock("~/queries/storage/zfcp", () => ({ + ...jest.requireActual("~/queries/storage/zfcp"), + useZFCPSupported: () => mockUseZFCPSupported(), +})); + +const mockUseDASDSupported = jest.fn(); +jest.mock("~/queries/storage/dasd", () => ({ + ...jest.requireActual("~/queries/storage/dasd"), + useDASDSupported: () => mockUseDASDSupported(), +})); + +const mockUseResetConfigMutation = jest.fn(); +jest.mock("~/queries/storage", () => ({ + ...jest.requireActual("~/queries/storage"), + useResetConfigMutation: () => mockUseResetConfigMutation(), +})); + +const mockReset = jest.fn(); +beforeEach(() => { + mockUseZFCPSupported.mockReturnValue(false); + mockUseDASDSupported.mockReturnValue(false); + mockUseResetConfigMutation.mockReturnValue({ mutate: mockReset }); +}); + +async function openMenu() { + const { user } = plainRender(); + const button = screen.getByRole("button", { name: "More options toggle" }); + await user.click(button); + const menu = screen.getByRole("menu"); + return { user, menu }; +} + +it("renders the menu", () => { + plainRender(); + expect(screen.queryByText("More options")).toBeInTheDocument(); +}); + +it("allows users to change the boot options", async () => { + const { user, menu } = await openMenu(); + const bootItem = within(menu).getByRole("menuitem", { name: /boot options/ }); + await user.click(bootItem); + expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.bootDevice); +}); + +it("allows users to reset the config", async () => { + const { user, menu } = await openMenu(); + const resetItem = within(menu).getByRole("menuitem", { name: /Reset to/ }); + await user.click(resetItem); + expect(mockReset).toHaveBeenCalled(); +}); + +it("allows users to configure iSCSI", async () => { + const { user, menu } = await openMenu(); + const iscsiItem = within(menu).getByRole("menuitem", { name: /iSCSI targets/ }); + await user.click(iscsiItem); + expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.iscsi); +}); + +describe("if zFCP is not supported", () => { + beforeEach(() => { + mockUseZFCPSupported.mockReturnValue(false); + }); + + it("does not allow users to configure zFCP", async () => { + const { menu } = await openMenu(); + const zfcpItem = within(menu).queryByRole("menuitem", { name: /Activate zFCP/ }); + expect(zfcpItem).not.toBeInTheDocument(); + }); +}); + +describe("if zFCP is supported", () => { + beforeEach(() => { + mockUseZFCPSupported.mockReturnValue(true); + }); + + it("allows users to configure zFCP", async () => { + const { user, menu } = await openMenu(); + const zfcpItem = within(menu).getByRole("menuitem", { name: /Activate zFCP/ }); + await user.click(zfcpItem); + expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.zfcp.root); + }); +}); + +describe("if DASD is not supported", () => { + beforeEach(() => { + mockUseDASDSupported.mockReturnValue(false); + }); + + it("does not allow users to configure DASD", async () => { + const { menu } = await openMenu(); + const dasdItem = within(menu).queryByRole("menuitem", { name: /Manage DASD/ }); + expect(dasdItem).not.toBeInTheDocument(); + }); +}); + +describe("if DASD is supported", () => { + beforeEach(() => { + mockUseDASDSupported.mockReturnValue(true); + }); + + it("allows users to configure DASD", async () => { + const { user, menu } = await openMenu(); + const dasdItem = within(menu).getByRole("menuitem", { name: /Manage DASD/ }); + await user.click(dasdItem); + expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.dasd); + }); +}); diff --git a/web/src/components/storage/ConfigEditorMenu.tsx b/web/src/components/storage/ConfigEditorMenu.tsx index 1dc674f393..e1547f6726 100644 --- a/web/src/components/storage/ConfigEditorMenu.tsx +++ b/web/src/components/storage/ConfigEditorMenu.tsx @@ -1,5 +1,5 @@ /* - * Copyright (c) [2024] SUSE LLC + * Copyright (c) [2024-2025] SUSE LLC * * All Rights Reserved. * @@ -21,7 +21,7 @@ */ import React, { useState } from "react"; -import { useNavigate, useHref } from "react-router-dom"; +import { useNavigate } from "react-router-dom"; import { _ } from "~/i18n"; import { Dropdown, @@ -31,10 +31,16 @@ import { DropdownItem, Divider, } from "@patternfly/react-core"; +import { useResetConfigMutation } from "~/queries/storage"; import { STORAGE as PATHS } from "~/routes/paths"; +import { useZFCPSupported } from "~/queries/storage/zfcp"; +import { useDASDSupported } from "~/queries/storage/dasd"; export default function ConfigEditorMenu() { const navigate = useNavigate(); + const isZFCPSupported = useZFCPSupported(); + const isDASDSupported = useDASDSupported(); + const { mutate: reset } = useResetConfigMutation(); const [isOpen, setIsOpen] = useState(false); const toggle = () => setIsOpen(!isOpen); @@ -56,17 +62,26 @@ export default function ConfigEditorMenu() { )} > - {_("Add LVM volume group")} - {_("Add MD RAID")} - - navigate(PATHS.bootDevice)}> + navigate(PATHS.bootDevice)}> {_("Change boot options")} - {_("Reinstall an existing system")} + reset()}> + {_("Reset to default configuration")} + - - {_("Configure iSCSI")} + navigate(PATHS.iscsi)}> + {_("Connect to iSCSI targets")} + {isZFCPSupported && ( + navigate(PATHS.zfcp.root)}> + {_("Activate zFCP disks")} + + )} + {isDASDSupported && ( + navigate(PATHS.dasd)}> + {_("Manage DASD devices")} + + )} ); From 9e63bc3920ffddca26269c66bd08597e0546014b Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 17 Feb 2025 11:46:03 +0000 Subject: [PATCH 4/4] web: Reorganize and rephrase ConfigEditor menus --- .../storage/ConfigEditorMenu.test.tsx | 10 ++--- .../components/storage/ConfigEditorMenu.tsx | 38 ++++++++++++++----- web/src/components/storage/ProposalPage.tsx | 5 +-- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/web/src/components/storage/ConfigEditorMenu.test.tsx b/web/src/components/storage/ConfigEditorMenu.test.tsx index 61e10fbe83..8fd8124006 100644 --- a/web/src/components/storage/ConfigEditorMenu.test.tsx +++ b/web/src/components/storage/ConfigEditorMenu.test.tsx @@ -80,7 +80,7 @@ it("allows users to reset the config", async () => { it("allows users to configure iSCSI", async () => { const { user, menu } = await openMenu(); - const iscsiItem = within(menu).getByRole("menuitem", { name: /iSCSI targets/ }); + const iscsiItem = within(menu).getByRole("menuitem", { name: /Configure iSCSI/ }); await user.click(iscsiItem); expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.iscsi); }); @@ -92,7 +92,7 @@ describe("if zFCP is not supported", () => { it("does not allow users to configure zFCP", async () => { const { menu } = await openMenu(); - const zfcpItem = within(menu).queryByRole("menuitem", { name: /Activate zFCP/ }); + const zfcpItem = within(menu).queryByRole("menuitem", { name: /Configure zFCP/ }); expect(zfcpItem).not.toBeInTheDocument(); }); }); @@ -104,7 +104,7 @@ describe("if zFCP is supported", () => { it("allows users to configure zFCP", async () => { const { user, menu } = await openMenu(); - const zfcpItem = within(menu).getByRole("menuitem", { name: /Activate zFCP/ }); + const zfcpItem = within(menu).getByRole("menuitem", { name: /Configure zFCP/ }); await user.click(zfcpItem); expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.zfcp.root); }); @@ -117,7 +117,7 @@ describe("if DASD is not supported", () => { it("does not allow users to configure DASD", async () => { const { menu } = await openMenu(); - const dasdItem = within(menu).queryByRole("menuitem", { name: /Manage DASD/ }); + const dasdItem = within(menu).queryByRole("menuitem", { name: /Configure DASD/ }); expect(dasdItem).not.toBeInTheDocument(); }); }); @@ -129,7 +129,7 @@ describe("if DASD is supported", () => { it("allows users to configure DASD", async () => { const { user, menu } = await openMenu(); - const dasdItem = within(menu).getByRole("menuitem", { name: /Manage DASD/ }); + const dasdItem = within(menu).getByRole("menuitem", { name: /Configure DASD/ }); await user.click(dasdItem); expect(mockNavigateFn).toHaveBeenCalledWith(PATHS.dasd); }); diff --git a/web/src/components/storage/ConfigEditorMenu.tsx b/web/src/components/storage/ConfigEditorMenu.tsx index e1547f6726..03e1449b0e 100644 --- a/web/src/components/storage/ConfigEditorMenu.tsx +++ b/web/src/components/storage/ConfigEditorMenu.tsx @@ -62,24 +62,44 @@ export default function ConfigEditorMenu() { )} > - navigate(PATHS.bootDevice)}> + navigate(PATHS.bootDevice)} + description={_("Select the disk to configure partitions for booting")} + > {_("Change boot options")} - reset()}> - {_("Reset to default configuration")} + reset()} + description={_("Start from scratch with the default configuration")} + > + {_("Reset to defaults")} - navigate(PATHS.iscsi)}> - {_("Connect to iSCSI targets")} + navigate(PATHS.iscsi)} + description={_("Discover and connect to iSCSI targets")} + > + {_("Configure iSCSI")} {isZFCPSupported && ( - navigate(PATHS.zfcp.root)}> - {_("Activate zFCP disks")} + navigate(PATHS.zfcp.root)} + description={_("Activate zFCP disks")} + > + {_("Configure zFCP")} )} {isDASDSupported && ( - navigate(PATHS.dasd)}> - {_("Manage DASD devices")} + navigate(PATHS.dasd)} + description={_("Activate and format DASD devices")} + > + {_("Configure DASD")} )} diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 525fcc8392..ae189925e9 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -193,12 +193,11 @@ function ProposalSections(): React.ReactNode { )} actions={ <> - - + - + }