diff --git a/doc/dbus_api.md b/doc/dbus_api.md index 51d2e4d672..73b2ab857b 100644 --- a/doc/dbus_api.md +++ b/doc/dbus_api.md @@ -211,6 +211,12 @@ Install() Finish() ~~~ +##### Properties + +~~~ +DeprecatedSystem readable b +~~~ + #### `org.opensuse.DInstaller.Storage1.Proposal.Calculator` Interface Allows creating a storage proposal. diff --git a/service/lib/dinstaller/dbus/storage/manager.rb b/service/lib/dinstaller/dbus/storage/manager.rb index 5d5a9e5cf9..263d3f01a5 100644 --- a/service/lib/dinstaller/dbus/storage/manager.rb +++ b/service/lib/dinstaller/dbus/storage/manager.rb @@ -61,6 +61,7 @@ def initialize(backend, logger) register_progress_callbacks register_service_status_callbacks register_iscsi_callbacks + register_software_callbacks return unless Yast::Arch.s390 singleton_class.include DBus::Interfaces::Dasd @@ -71,7 +72,10 @@ def initialize(backend, logger) private_constant :STORAGE_INTERFACE def probe - busy_while { backend.probe } + busy_while do + backend.probe + storage_properties_changed + end end def install @@ -82,10 +86,18 @@ def finish busy_while { backend.finish } end + # Whether the system is in a deprecated status + # + # @return [Boolean] + def deprecated_system + backend.deprecated_system + end + dbus_interface STORAGE_INTERFACE do dbus_method(:Probe) { probe } dbus_method(:Install) { install } dbus_method(:Finish) { finish } + dbus_reader(:deprecated_system, "b") end PROPOSAL_CALCULATOR_INTERFACE = "org.opensuse.DInstaller.Storage1.Proposal.Calculator" @@ -246,6 +258,27 @@ def register_iscsi_callbacks backend.iscsi.on_probe do refresh_iscsi_nodes end + + backend.iscsi.on_sessions_change do + deprecate_system + end + end + + def register_software_callbacks + backend.software.on_product_selected do |_product| + backend.proposal.reset + end + end + + def deprecate_system + backend.deprecated_system = true + storage_properties_changed + update_validation + end + + def storage_properties_changed + properties = interfaces_and_properties[STORAGE_INTERFACE] + dbus_properties_changed(STORAGE_INTERFACE, properties, []) end def proposal_properties_changed diff --git a/service/lib/dinstaller/storage/iscsi/manager.rb b/service/lib/dinstaller/storage/iscsi/manager.rb index cfb34a1f86..d944ba52d5 100644 --- a/service/lib/dinstaller/storage/iscsi/manager.rb +++ b/service/lib/dinstaller/storage/iscsi/manager.rb @@ -51,6 +51,7 @@ def initialize(logger: nil) @on_activate_callbacks = [] @on_probe_callbacks = [] + @on_sessions_change_callbacks = [] end # Performs actions for activating iSCSI @@ -116,11 +117,14 @@ def login(node, authentication, startup: nil) ensure_activated - probe_after do + result = probe_after do Yast::IscsiClientLib.currentRecord = record_from(node) Yast::IscsiClientLib.login_into_current(authentication, silent: true) && Yast::IscsiClientLib.setStartupStatus(startup) end + + run_on_sessions_change_callbacks + result end # Closes an iSCSI session @@ -132,11 +136,14 @@ def login(node, authentication, startup: nil) def logout(node) ensure_activated - probe_after do + result = probe_after do Yast::IscsiClientLib.currentRecord = record_from(node) # Yes, this is the correct method name for logging out Yast::IscsiClientLib.deleteRecord end + + run_on_sessions_change_callbacks + result end # Deletes an iSCSI node from the database @@ -181,6 +188,13 @@ def on_probe(&block) @on_probe_callbacks << block end + # Registers a callback to be called when a session changes + # + # @param block [Proc] + def on_sessions_change(&block) + @on_sessions_change_callbacks << block + end + private # @return [Logger] @@ -248,6 +262,11 @@ def find_session_for(record) def probe_after(&block) block.call.tap { probe } end + + # Runs callbacks when a session changes + def run_on_sessions_change_callbacks + @on_sessions_change_callbacks.each(&:call) + end end end end diff --git a/service/lib/dinstaller/storage/manager.rb b/service/lib/dinstaller/storage/manager.rb index 3cc03fc6d7..d7ad71947d 100644 --- a/service/lib/dinstaller/storage/manager.rb +++ b/service/lib/dinstaller/storage/manager.rb @@ -29,6 +29,7 @@ require "dinstaller/storage/finisher" require "dinstaller/with_progress" require "dinstaller/security" +require "dinstaller/validation_error" require "dinstaller/dbus/clients/questions" require "dinstaller/dbus/clients/software" @@ -40,13 +41,23 @@ module Storage class Manager include WithProgress + # Whether the system is in a deprecated status + # + # The system is usually set as deprecated as effect of managing some kind of devices, for + # example, when iSCSI sessions are created. + # + # A deprecated system means that the probed system could not match with the current system. + attr_accessor :deprecated_system + def initialize(config, logger) @config = config @logger = logger + @deprecated_system = false end # Probes storage devices and performs an initial proposal def probe + @deprecated_system = false start_progress(4) config.pick_product(software.selected_product) progress.step("Activating storage devices") { activate_devices } @@ -97,7 +108,15 @@ def iscsi # # @return [Array] List of validation errors def validate - proposal.validate + errors = [deprecated_system_error] + proposal.validate + errors.compact + end + + # Returns the client to ask the software service + # + # @return [DInstaller::DBus::Clients::Software] + def software + @software ||= DBus::Clients::Software.new end private @@ -126,12 +145,18 @@ def probe_devices Y2Storage::StorageManager.instance.probe(Y2Storage::Callbacks::UserProbe.new) end - # Calculates the default proposal + # Calculates the proposal + # + # It reuses the settings from the previous proposal, if any. def calculate_proposal - settings = ProposalSettings.new - # FIXME: by now, the UI only allows to select one disk - device = proposal.available_devices.first&.name - settings.candidate_devices << device if device + settings = proposal.settings + + if !settings + settings = ProposalSettings.new + # FIXME: by now, the UI only allows to select one disk + device = proposal.available_devices.first&.name + settings.candidate_devices << device if device + end proposal.calculate(settings) end @@ -146,6 +171,15 @@ def add_packages Yast::PackagesProposal.SetResolvables(PROPOSAL_ID, :package, packages) end + # Returns an error if the system is deprecated + # + # @return [ValidationError, nil] + def deprecated_system_error + return unless deprecated_system + + ValidationError.new("The system devices have changed") + end + # Security manager # # @return [Security] @@ -159,13 +193,6 @@ def security def questions_client @questions_client ||= DInstaller::DBus::Clients::Questions.new(logger: logger) end - - # Returns the client to ask the software service - # - # @return [DInstaller::DBus::Clients::Software] - def software - @software ||= DBus::Clients::Software.new - end end end end diff --git a/service/lib/dinstaller/storage/proposal.rb b/service/lib/dinstaller/storage/proposal.rb index 9e50abd919..8287000d80 100644 --- a/service/lib/dinstaller/storage/proposal.rb +++ b/service/lib/dinstaller/storage/proposal.rb @@ -41,6 +41,11 @@ module Storage # proposal.calculate(settings) #=> true # proposal.calculated_volumes #=> [Volume, Volume] class Proposal + # Settings used for calculating the proposal + # + # @return [ProposalSettings, nil] + attr_reader :settings + # Constructor # # @param logger [Logger] @@ -51,6 +56,10 @@ def initialize(logger, config) @on_calculate_callbacks = [] end + def reset + @settings = nil + end + # Stores callbacks to be call after calculating a proposal def on_calculate(&block) @on_calculate_callbacks << block @@ -114,14 +123,14 @@ def calculated_settings # Calculates a new proposal # - # @param settings [ProposalSettings] settings to calculate the proposal + # @param settings [ProposalSettings, nil] settings to calculate the proposal # @return [Boolean] whether the proposal was correctly calculated def calculate(settings = nil) - settings ||= ProposalSettings.new - settings.freeze - proposal_settings = to_y2storage_settings(settings) + @settings = settings || ProposalSettings.new + @settings.freeze + y2storage_settings = to_y2storage_settings(@settings) - @proposal = new_proposal(proposal_settings) + @proposal = new_proposal(y2storage_settings) storage_manager.proposal = proposal @on_calculate_callbacks.each(&:call) @@ -145,9 +154,10 @@ def validate return [] if proposal.nil? [ - validate_proposal, - validate_available_devices, - validate_candidate_devices + empty_available_devices_error, + empty_candidate_devices_error, + missing_candidate_devices_error, + proposal_error ].compact end @@ -248,24 +258,32 @@ def storage_manager Y2Storage::StorageManager.instance end - def validate_proposal - return if candidate_devices.empty? || !proposal.failed? - - ValidationError.new("Cannot accommodate the required file systems for installation") - end - - def validate_available_devices + def empty_available_devices_error return if available_devices.any? ValidationError.new("There is no suitable device for installation") end - def validate_candidate_devices - return if available_devices.empty? || candidate_devices.any? + def empty_candidate_devices_error + return if candidate_devices.any? ValidationError.new("No devices are selected for installation") end + def missing_candidate_devices_error + available_names = available_devices.map(&:name) + missing = candidate_devices - available_names + return if missing.none? + + ValidationError.new("Some selected devices are not found in the system") + end + + def proposal_error + return unless proposal.failed? + + ValidationError.new("Cannot accommodate the required file systems for installation") + end + # Adjusts the encryption-related settings of the given Y2Storage::ProposalSettings object # # @param settings [Y2Storage::ProposalSettings] diff --git a/service/package/rubygem-d-installer.changes b/service/package/rubygem-d-installer.changes index 3936ffc754..55736e3154 100644 --- a/service/package/rubygem-d-installer.changes +++ b/service/package/rubygem-d-installer.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Mar 23 17:04:54 UTC 2023 - José Iván López González + +- Set system as deprecated after changing iSCSI sessions. +- Reuse settings from previous proposal. +- gh#yast/d-installer#484 + ------------------------------------------------------------------- Wed Mar 22 16:05:14 UTC 2023 - Knut Anderssen diff --git a/service/test/dinstaller/dbus/storage/manager_test.rb b/service/test/dinstaller/dbus/storage/manager_test.rb index b892d650a5..36ff7493fd 100644 --- a/service/test/dinstaller/dbus/storage/manager_test.rb +++ b/service/test/dinstaller/dbus/storage/manager_test.rb @@ -29,6 +29,7 @@ require "dinstaller/storage/iscsi/manager" require "dinstaller/storage/dasd/manager" require "dinstaller/dbus/storage/dasds_tree" +require "dinstaller/dbus/clients/software" require "y2storage" require "dbus" @@ -41,6 +42,7 @@ instance_double(DInstaller::Storage::Manager, proposal: proposal, iscsi: iscsi, + software: software, on_progress_change: nil, on_progress_finish: nil) end @@ -52,13 +54,40 @@ let(:settings) { nil } let(:iscsi) do - instance_double(DInstaller::Storage::ISCSI::Manager, on_activate: nil, on_probe: nil) + instance_double(DInstaller::Storage::ISCSI::Manager, + on_activate: nil, + on_probe: nil, + on_sessions_change: nil) end + let(:software) { instance_double(DInstaller::DBus::Clients::Software, on_product_selected: nil) } + before do allow(Yast::Arch).to receive(:s390).and_return false end + describe "#deprecated_system" do + before do + allow(backend).to receive(:deprecated_system).and_return(deprecated) + end + + context "if the system is set as deprecated" do + let(:deprecated) { true } + + it "returns true" do + expect(subject.deprecated_system).to eq(true) + end + end + + context "if the system is not set as deprecated" do + let(:deprecated) { false } + + it "returns false" do + expect(subject.deprecated_system).to eq(false) + end + end + end + describe "#available_devices" do before do allow(proposal).to receive(:available_devices).and_return(devices) diff --git a/service/test/dinstaller/storage/manager_test.rb b/service/test/dinstaller/storage/manager_test.rb index 7c2e4efaf7..f8846583a1 100644 --- a/service/test/dinstaller/storage/manager_test.rb +++ b/service/test/dinstaller/storage/manager_test.rb @@ -22,6 +22,7 @@ require_relative "../../test_helper" require_relative "storage_helpers" require "dinstaller/storage/manager" +require "dinstaller/storage/proposal_settings" require "dinstaller/storage/iscsi/manager" require "dinstaller/config" require "dinstaller/dbus/clients/questions" @@ -62,16 +63,28 @@ end let(:proposal) do - instance_double(DInstaller::Storage::Proposal, calculate: nil, available_devices: devices) + instance_double(DInstaller::Storage::Proposal, + settings: settings, + calculate: nil, + available_devices: devices) end let(:devices) { [disk1, disk2] } + let(:settings) { nil } let(:disk1) { instance_double(Y2Storage::Disk, name: "/dev/vda") } let(:disk2) { instance_double(Y2Storage::Disk, name: "/dev/vdb") } let(:iscsi) { DInstaller::Storage::ISCSI::Manager.new } + before do + allow(config).to receive(:pick_product) + allow(iscsi).to receive(:activate) + allow(y2storage_manager).to receive(:activate) + allow(iscsi).to receive(:probe) + allow(y2storage_manager).to receive(:probe) + end + it "probes the storage devices and calculates a proposal" do expect(config).to receive(:pick_product).with("ALP") expect(iscsi).to receive(:activate) @@ -83,6 +96,37 @@ expect(proposal).to receive(:calculate) storage.probe end + + it "sets the system as non deprecated" do + storage.deprecated_system = true + storage.probe + + expect(storage.deprecated_system).to eq(false) + end + + context "when there are settings from a previous proposal" do + let(:settings) { DInstaller::Storage::ProposalSettings.new } + + it "calculates a proposal using the previous settings" do + expect(proposal).to receive(:calculate).with(settings) + storage.probe + end + end + + context "when there are no settings from a previous proposal" do + let(:settings) { nil } + + let(:new_settings) { DInstaller::Storage::ProposalSettings.new } + + before do + allow(DInstaller::Storage::ProposalSettings).to receive(:new).and_return(new_settings) + end + + it "calculates a proposal using new settings" do + expect(proposal).to receive(:calculate).with(new_settings) + storage.probe + end + end end describe "#install" do @@ -198,5 +242,17 @@ it "returns the proposal errors" do expect(storage.validate).to eq(errors) end + + context "if the system is deprecated" do + before do + storage.deprecated_system = true + end + + it "includes an error" do + error = storage.validate.find { |e| e.message.match?(/devices have changed/) } + + expect(error).to_not be_nil + end + end end end diff --git a/service/test/dinstaller/storage/proposal_test.rb b/service/test/dinstaller/storage/proposal_test.rb index b3afad4552..9f36fb5e01 100644 --- a/service/test/dinstaller/storage/proposal_test.rb +++ b/service/test/dinstaller/storage/proposal_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022] SUSE LLC +# Copyright (c) [2022-2023] SUSE LLC # # All Rights Reserved. # @@ -22,6 +22,7 @@ require_relative "../../test_helper" require_relative "storage_helpers" require "dinstaller/storage/proposal" +require "dinstaller/storage/proposal_settings" require "dinstaller/config" describe DInstaller::Storage::Proposal do @@ -113,6 +114,15 @@ expect(var2).to eq 10 end + it "stores the given settings" do + expect(proposal.settings).to be_nil + + settings = DInstaller::Storage::ProposalSettings.new + proposal.calculate(settings) + + expect(proposal.settings).to eq(settings) + end + context "with undefined settings and no storage section in the config" do let(:config_data) { {} } @@ -339,7 +349,7 @@ end describe "#validate" do - let(:sda) { instance_double(Y2Storage::Device, display_name: "/dev/sda") } + let(:sda) { instance_double(Y2Storage::Disk, name: "/dev/sda") } let(:available_devices) { [sda] } let(:candidate_devices) { ["/dev/sda"] } @@ -361,37 +371,43 @@ let(:y2storage_proposal) { nil } it "returns an empty list" do - expect(subject.validate).to be_empty + expect(subject.validate).to eq([]) end end context "when there are not available storage devices" do let(:available_devices) { [] } - it "returns an error" do - errors = subject.validate - expect(errors.size).to eq(1) - expect(errors.first.message).to include("There is no suitable device") + it "returns a list of errors including the expected error" do + error = subject.validate.find { |e| e.message.match?(/no suitable device/) } + expect(error).to_not be_nil end end - context "when the proposal failed" do - let(:failed) { true } + context "when no candidate devices are selected" do + let(:candidate_devices) { [] } - it "returns an error" do - errors = subject.validate - expect(errors.size).to eq(1) - expect(errors.first.message).to include("Cannot accommodate") + it "returns a list of errors including the expected error" do + error = subject.validate.find { |e| e.message.match?(/No devices are selected/) } + expect(error).to_not be_nil end end - context "when no candidate devices are selected" do - let(:candidate_devices) { [] } + context "when any candidate device is missing" do + let(:candidate_devices) { ["/dev/vda"] } + + it "returns a list of errors including the expected error" do + error = subject.validate.find { |e| e.message.match?(/not found in the system/) } + expect(error).to_not be_nil + end + end + + context "when the proposal failed" do + let(:failed) { true } - it "returns an error" do - errors = subject.validate - expect(errors.size).to eq(1) - expect(errors.first.message).to include("No devices are selected for installation") + it "returns a list of errors including the expected error" do + error = subject.validate.find { |e| e.message.match?(/Cannot accommodate/) } + expect(error).to_not be_nil end end end diff --git a/web/package/cockpit-d-installer.changes b/web/package/cockpit-d-installer.changes index 4e29031961..db3ef4de0c 100644 --- a/web/package/cockpit-d-installer.changes +++ b/web/package/cockpit-d-installer.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Mar 23 17:07:29 UTC 2023 - José Iván López González + +- Reprobe storage if the system becomes deprecated + (gh#yast/d-installer#484). + ------------------------------------------------------------------- Tue Mar 21 16:41:06 UTC 2023 - Ladislav Slezák diff --git a/web/src/client/storage.js b/web/src/client/storage.js index bfac92a1d7..17c6b9d699 100644 --- a/web/src/client/storage.js +++ b/web/src/client/storage.js @@ -24,6 +24,7 @@ import DBusClient from "./dbus"; import { WithStatus, WithProgress, WithValidation } from "./mixins"; +const STORAGE_IFACE = "org.opensuse.DInstaller.Storage1"; const PROPOSAL_CALCULATOR_IFACE = "org.opensuse.DInstaller.Storage1.Proposal.Calculator"; const ISCSI_NODE_IFACE = "org.opensuse.DInstaller.Storage1.ISCSI.Node"; const ISCSI_NODES_NAMESPACE = "/org/opensuse/DInstaller/Storage1/iscsi_nodes"; @@ -59,7 +60,9 @@ class ProposalManager { */ constructor(client) { this.client = client; - this.proxies = {}; + this.proxies = { + proposalCalculator: this.client.proxy(PROPOSAL_CALCULATOR_IFACE, STORAGE_OBJECT) + }; } /** @@ -95,7 +98,7 @@ class ProposalManager { }; }; - const proxy = await this.proposalCalculatorProxy(); + const proxy = await this.proxies.proposalCalculator; return proxy.AvailableDevices.map(buildDevice); } @@ -214,23 +217,10 @@ class ProposalManager { Volumes: { t: "aa{sv}", v: volumes?.map(dbusVolume) } }); - const proxy = await this.proposalCalculatorProxy(); + const proxy = await this.proxies.proposalCalculator; return proxy.Calculate(settings); } - /** - * @private - * Proxy for org.opensuse.DInstaller.Storage1.Proposal.Calculator iface - * - * @returns {Promise} - */ - async proposalCalculatorProxy() { - if (!this.proxies.proposalCalculator) - this.proxies.proposalCalculator = await this.client.proxy(PROPOSAL_CALCULATOR_IFACE, STORAGE_OBJECT); - - return this.proxies.proposalCalculator; - } - /** * @private * Proxy for org.opensuse.DInstaller.Storage1.Proposal iface @@ -516,6 +506,40 @@ class StorageBaseClient { this.client = new DBusClient(StorageBaseClient.SERVICE, address); this.proposal = new ProposalManager(this.client); this.iscsi = new ISCSIManager(StorageBaseClient.SERVICE, address); + this.proxies = { + storage: this.client.proxy(STORAGE_IFACE) + }; + } + + /** + * Probes the system + */ + async probe() { + const proxy = await this.proxies.storage; + return proxy.Probe(); + } + + /** + * Whether the system is in a deprecated status + * + * @returns {Promise} + */ + async isDeprecated() { + const proxy = await this.proxies.storage; + return proxy.DeprecatedSystem; + } + + /** + * Runs a handler function when the system becomes deprecated + * + * @callback handlerFn + * + * @param {handlerFn} handler + */ + onDeprecate(handler) { + return this.client.onObjectChanged(STORAGE_OBJECT, STORAGE_IFACE, (changes) => { + if (changes.DeprecatedSystem?.v) return handler(); + }); } } diff --git a/web/src/client/storage.test.js b/web/src/client/storage.test.js index cf24eb2866..06475bea82 100644 --- a/web/src/client/storage.test.js +++ b/web/src/client/storage.test.js @@ -28,6 +28,8 @@ jest.mock("./dbus"); const cockpitProxies = {}; +const cockpitCallbacks = {}; + const contexts = { withoutProposal: () => { cockpitProxies.proposal = null; @@ -105,6 +107,7 @@ const contexts = { const mockProxy = (iface, path) => { switch (iface) { + case "org.opensuse.DInstaller.Storage1": return cockpitProxies.storage; case "org.opensuse.DInstaller.Storage1.Proposal": return cockpitProxies.proposal; case "org.opensuse.DInstaller.Storage1.Proposal.Calculator": return cockpitProxies.proposalCalculator; case "org.opensuse.DInstaller.Storage1.ISCSI.Initiator": return cockpitProxies.iscsiInitiator; @@ -118,18 +121,98 @@ const mockProxies = (iface) => { } }; -let client; +const mockOnObjectChanged = (path, iface, handler) => { + if (!cockpitCallbacks[path]) cockpitCallbacks[path] = {}; + cockpitCallbacks[path][iface] = handler; +}; + +const emitSignal = (path, iface, data) => { + if (!cockpitCallbacks[path]) return; + + const handler = cockpitCallbacks[path][iface]; + if (!handler) return; + + return handler(data); +}; beforeEach(() => { // @ts-ignore DBusClient.mockImplementation(() => { return { proxy: mockProxy, - proxies: mockProxies + proxies: mockProxies, + onObjectChanged: mockOnObjectChanged + }; + }); +}); + +let client; + +describe("#probe", () => { + beforeEach(() => { + cockpitProxies.storage = { + Probe: jest.fn() }; + + client = new StorageClient(); }); - client = new StorageClient(); + it("probes the system", async () => { + await client.probe(); + expect(cockpitProxies.storage.Probe).toHaveBeenCalled(); + }); +}); + +describe("#isDeprecated", () => { + describe("if the system is not deprecated", () => { + beforeEach(() => { + cockpitProxies.storage = { + DeprecatedSystem: false + }; + + client = new StorageClient(); + }); + + it("returns false", async () => { + const result = await client.isDeprecated(); + expect(result).toEqual(false); + }); + }); +}); + +describe("#onDeprecate", () => { + const handler = jest.fn(); + + beforeEach(() => { + client = new StorageClient(); + client.onDeprecate(handler); + }); + + describe("if the system was not deprecated", () => { + beforeEach(() => { + emitSignal( + "/org/opensuse/DInstaller/Storage1", + "org.opensuse.DInstaller.Storage1", + {}); + }); + + it("does not run the handler", async () => { + expect(handler).not.toHaveBeenCalled(); + }); + }); + + describe("if the system was deprecated", () => { + beforeEach(() => { + emitSignal( + "/org/opensuse/DInstaller/Storage1", + "org.opensuse.DInstaller.Storage1", + { DeprecatedSystem: true }); + }); + + it("runs the handler", async () => { + expect(handler).not.toHaveBeenCalled(); + }); + }); }); describe("#proposal", () => { @@ -170,6 +253,7 @@ describe("#proposal", () => { beforeEach(() => { contexts.withAvailableDevices(); contexts.withProposal(); + client = new StorageClient(); }); it("returns the available devices and the proposal result", async () => { @@ -182,6 +266,7 @@ describe("#proposal", () => { describe("#getAvailableDevices", () => { beforeEach(() => { contexts.withAvailableDevices(); + client = new StorageClient(); }); it("returns the list of available devices", async () => { @@ -194,6 +279,7 @@ describe("#proposal", () => { describe("if there is no proposal yet", () => { beforeEach(() => { contexts.withoutProposal(); + client = new StorageClient(); }); it("returns undefined", async () => { @@ -205,6 +291,7 @@ describe("#proposal", () => { describe("if there is a proposal", () => { beforeEach(() => { contexts.withProposal(); + client = new StorageClient(); }); it("returns the proposal settings and actions", async () => { @@ -219,6 +306,8 @@ describe("#proposal", () => { cockpitProxies.proposalCalculator = { Calculate: jest.fn() }; + + client = new StorageClient(); }); it("calculates a default proposal when no settings are given", async () => { @@ -276,6 +365,10 @@ describe("#proposal", () => { }); describe("#iscsi", () => { + beforeEach(() => { + client = new StorageClient(); + }); + describe("#getInitiatorName", () => { beforeEach(() => { cockpitProxies.iscsiInitiator = { diff --git a/web/src/components/core/ValidationErrors.jsx b/web/src/components/core/ValidationErrors.jsx index 81c9659c64..443ad954d5 100644 --- a/web/src/components/core/ValidationErrors.jsx +++ b/web/src/components/core/ValidationErrors.jsx @@ -1,6 +1,6 @@ /* - * Copyright (c) [2022] SUSE LLC + * Copyright (c) [2022-2023] SUSE LLC * * All Rights Reserved. * @@ -73,8 +73,13 @@ const ValidationErrors = ({ title = "Errors", errors }) => { return ( <>
- { warningIcon } - setPopoverVisible(true)}>{`${errors.length} errors found`} + { ]; const { user } = plainRender(); - const button = await screen.findByRole("link", { name: "2 errors found" }); + const button = await screen.findByRole("button", { name: "2 errors found" }); await user.click(button); await waitFor(() => { diff --git a/web/src/components/overview/StorageSection.jsx b/web/src/components/overview/StorageSection.jsx index d0587e68f6..879131fb45 100644 --- a/web/src/components/overview/StorageSection.jsx +++ b/web/src/components/overview/StorageSection.jsx @@ -75,13 +75,16 @@ export default function StorageSection({ showErrors }) { useEffect(() => { const updateProposal = async () => { + const isDeprecated = await cancellablePromise(client.isDeprecated()); + if (isDeprecated) await cancellablePromise(client.probe()); + const proposal = await cancellablePromise(client.proposal.getData()); const errors = await cancellablePromise(client.getValidationErrors()); dispatch({ type: "UPDATE_PROPOSAL", payload: { proposal, errors } }); }; - updateProposal(); + if (!state.busy) updateProposal(); }, [client, cancellablePromise, state.busy]); useEffect(() => { @@ -102,6 +105,10 @@ export default function StorageSection({ showErrors }) { }); }, [client, cancellablePromise]); + useEffect(() => { + return client.onDeprecate(() => client.probe()); + }, [client]); + const errors = showErrors ? state.errors : []; const busy = state.busy || !state.proposal; diff --git a/web/src/components/overview/StorageSection.test.jsx b/web/src/components/overview/StorageSection.test.jsx index 4b8ea81739..4deaec0f0f 100644 --- a/web/src/components/overview/StorageSection.test.jsx +++ b/web/src/components/overview/StorageSection.test.jsx @@ -55,7 +55,9 @@ beforeEach(() => { }), onProgressChange: noop, getValidationErrors: jest.fn().mockResolvedValue(errors), - onStatusChange: onStatusChangeFn + onStatusChange: onStatusChangeFn, + isDeprecated: jest.fn().mockResolvedValue(false), + onDeprecate: noop }, }; }); diff --git a/web/src/components/storage/ProposalPage.jsx b/web/src/components/storage/ProposalPage.jsx index bda752a747..04e36d811f 100644 --- a/web/src/components/storage/ProposalPage.jsx +++ b/web/src/components/storage/ProposalPage.jsx @@ -19,7 +19,7 @@ * find current contact information at www.suse.com. */ -import React, { useReducer, useEffect } from "react"; +import React, { useCallback, useReducer, useEffect } from "react"; import { Alert } from "@patternfly/react-core"; import { Link } from "react-router-dom"; @@ -34,24 +34,21 @@ import { } from "~/components/storage"; const initialState = { - busy: false, + loading: false, proposal: undefined, errors: [] }; const reducer = (state, action) => { switch (action.type) { - case "SET_BUSY" : { - return { ...state, busy: true }; + case "UPDATE_LOADING" : { + const { loading } = action.payload; + return { ...state, loading }; } - case "LOAD": { + case "UPDATE_PROPOSAL": { const { proposal, errors } = action.payload; - return { ...state, proposal, errors, busy: false }; - } - - case "CALCULATE": { - return initialState; + return { ...state, proposal, errors }; } default: { @@ -61,34 +58,46 @@ const reducer = (state, action) => { }; export default function ProposalPage() { - const client = useInstallerClient(); + const { storage: client } = useInstallerClient(); const { cancellablePromise } = useCancellablePromise(); const [state, dispatch] = useReducer(reducer, initialState); - useEffect(() => { - const loadProposal = async () => { - dispatch({ type: "SET_BUSY" }); + const loadProposal = useCallback(async (hooks = {}) => { + dispatch({ type: "UPDATE_LOADING", payload: { loading: true } }); + + if (hooks.before !== undefined) await cancellablePromise(hooks.before()); + const proposal = await cancellablePromise(client.proposal.getData()); + const errors = await cancellablePromise(client.getValidationErrors()); + + dispatch({ type: "UPDATE_PROPOSAL", payload: { proposal, errors } }); + dispatch({ type: "UPDATE_LOADING", payload: { loading: false } }); + }, [client, cancellablePromise]); - const proposal = await cancellablePromise(client.storage.proposal.getData()); - const errors = await cancellablePromise(client.storage.getValidationErrors()); + useEffect(() => { + const probeAndLoad = async () => { + await loadProposal({ before: () => client.probe() }); + }; - dispatch({ - type: "LOAD", - payload: { proposal, errors } - }); + const load = async () => { + const isDeprecated = await cancellablePromise(client.isDeprecated()); + isDeprecated ? probeAndLoad() : loadProposal(); }; - if (!state.proposal) loadProposal().catch(console.error); - }, [client.storage, cancellablePromise, state.proposal]); + load().catch(console.error); + + return client.onDeprecate(() => probeAndLoad()); + }, [client, cancellablePromise, loadProposal]); const calculateProposal = async (settings) => { - dispatch({ type: "SET_BUSY" }); - await client.storage.proposal.calculate({ ...state.proposal.result, ...settings }); - dispatch({ type: "CALCULATE" }); + const calculate = async () => { + await client.proposal.calculate({ ...state.proposal.result, ...settings }); + }; + + loadProposal({ before: calculate }).catch(console.error); }; const PageContent = () => { - if (state.busy || state.proposal?.result === undefined) return ; + if (state.loading || state.proposal?.result === undefined) return ; return ( <> diff --git a/web/src/components/storage/ProposalPage.test.jsx b/web/src/components/storage/ProposalPage.test.jsx index bfef1a791e..eaadc1969e 100644 --- a/web/src/components/storage/ProposalPage.test.jsx +++ b/web/src/components/storage/ProposalPage.test.jsx @@ -22,6 +22,7 @@ import React from "react"; import { screen, waitForElementToBeRemoved } from "@testing-library/react"; import { installerRender, mockComponent } from "~/test-utils"; +import { noop } from "~/utils"; import { createClient } from "~/client"; import { ProposalPage } from "~/components/storage"; @@ -50,7 +51,9 @@ beforeEach(() => { getData: jest.fn().mockResolvedValue(proposal), calculate: jest.fn().mockResolvedValue(0) }, - getValidationErrors: jest.fn().mockResolvedValue([]) + getValidationErrors: jest.fn().mockResolvedValue([]), + isDeprecated: jest.fn().mockResolvedValue(false), + onDeprecate: noop } }; }); diff --git a/web/src/components/storage/ProposalSummary.jsx b/web/src/components/storage/ProposalSummary.jsx index f4d83fb760..ddb16af79a 100644 --- a/web/src/components/storage/ProposalSummary.jsx +++ b/web/src/components/storage/ProposalSummary.jsx @@ -35,17 +35,11 @@ export default function ProposalSummary({ proposal }) { const [candidateDevice] = result.candidateDevices; const device = proposal.availableDevices.find(d => d.id === candidateDevice); - if (device === undefined) { - return ( - - Required device {candidateDevice} not found - - ); - } + const deviceLabel = device?.label || candidateDevice; return ( - Install using device {device.label} and deleting all its content + Install using device {deviceLabel} and deleting all its content ); } diff --git a/web/src/components/storage/ProposalSummary.test.jsx b/web/src/components/storage/ProposalSummary.test.jsx index 7a27e55f7f..a161e5eda3 100644 --- a/web/src/components/storage/ProposalSummary.test.jsx +++ b/web/src/components/storage/ProposalSummary.test.jsx @@ -47,22 +47,40 @@ describe("ProposalSummary", () => { }); describe("when the proposal is calculated", () => { - it("renders the candidate device label", () => { - const proposal = { + let proposal; + + beforeEach(() => { + proposal = { result: { candidateDevices: ["sdb"] }, availableDevices: [ - { id: "sda", label: "/dev/sda" }, - { id: "sdb", label: "/dev/sdb" }, + { id: "sda", label: "/dev/sda 300 MiB" }, + { id: "sdb", label: "/dev/sdb 5 GiB" }, ] }; + }); + it("renders the candidate device label", () => { installerRender( ); - screen.getByText("/dev/sdb"); + screen.getByText("/dev/sdb 5 GiB"); + }); + + describe("and the candidate device is missing", () => { + beforeEach(() => { + proposal.result.candidateDevices = ["sdc"]; + }); + + it("renders the candidate device name", () => { + installerRender( + + ); + + screen.getByText("sdc"); + }); }); }); }); diff --git a/web/src/components/storage/ProposalTargetForm.jsx b/web/src/components/storage/ProposalTargetForm.jsx index c1be608c17..a2bf1ba14f 100644 --- a/web/src/components/storage/ProposalTargetForm.jsx +++ b/web/src/components/storage/ProposalTargetForm.jsx @@ -19,7 +19,7 @@ * find current contact information at www.suse.com. */ -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { Form, @@ -30,6 +30,17 @@ import { DeviceSelector } from "~/components/storage"; export default function ProposalTargetForm({ id, proposal, onSubmit }) { const [candidateDevices, setCandidateDevices] = useState(proposal.result.candidateDevices); + useEffect(() => { + const existCandidates = () => { + const devices = proposal.result.candidateDevices; + const device = proposal.availableDevices.find(d => d.id === devices[0]); + return device !== undefined; + }; + + if (!existCandidates()) + setCandidateDevices([proposal.availableDevices[0].id]); + }, [proposal]); + const accept = (e) => { e.preventDefault(); onSubmit({ candidateDevices }); diff --git a/web/src/components/storage/ProposalTargetForm.test.jsx b/web/src/components/storage/ProposalTargetForm.test.jsx index 00627488f1..fea44093ff 100644 --- a/web/src/components/storage/ProposalTargetForm.test.jsx +++ b/web/src/components/storage/ProposalTargetForm.test.jsx @@ -22,7 +22,7 @@ import React from "react"; import { screen, within } from "@testing-library/react"; -import { installerRender } from "~/test-utils"; +import { plainRender } from "~/test-utils"; import { ProposalTargetForm } from "~/components/storage"; const proposal = { @@ -38,7 +38,7 @@ const onSubmitFn = jest.fn(); describe("ProposalTargetForm", () => { it("renders a selector for choosing candidate devices among available devices in given proposal", () => { - installerRender( + plainRender( ); @@ -49,7 +49,7 @@ describe("ProposalTargetForm", () => { describe("Selector for choosing candidate devices", () => { it("gets its initial value from given proposal", () => { - installerRender( + plainRender( ); @@ -58,7 +58,7 @@ describe("ProposalTargetForm", () => { }); it("changes its value when user changes the selection", async () => { - const { user } = installerRender( + const { user } = plainRender( ); @@ -88,7 +88,7 @@ describe("ProposalTargetForm", () => { ); }; - const { user } = installerRender(); + const { user } = plainRender(); const deviceSelector = screen.getByRole("combobox"); const sdbOption = within(deviceSelector).getByRole("option", { name: "/dev/sdb, 650 GiB" }); diff --git a/web/src/index.js b/web/src/index.js index b30a12b47d..64d847b54c 100644 --- a/web/src/index.js +++ b/web/src/index.js @@ -19,7 +19,7 @@ * find current contact information at www.suse.com. */ -import React, { StrictMode } from "react"; +import React from "react"; import { createRoot } from "react-dom/client"; import "core-js/stable"; import "regenerator-runtime/runtime"; @@ -52,28 +52,26 @@ const container = document.getElementById("root"); const root = createRoot(container); root.render( - - - - - - - }> - }> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - - } /> + + + + + + }> + }> + } /> + } /> + } /> + } /> + } /> + } /> + } /> - - - - - - + } /> + + + + + + );