fix(web): storage reprobe (#1911)

## Problem

If the system is set as deprecated (e.g., after activating iSCSI
devices), then the web client automatically reprobes the storage devices
and recalculates the proposal. That is done by 3 different calls (fetch
config, probe, and set config). This makes the process very unreliable
if more than one reprobing is done at the same time. As side effect, the
current storage config could be lost, getting the initial storage
proposal instead.

## Solution

Add a new D-Bus method for reprobing in an atomic way. The system is set
as "no deprecated" at the end of the action. Now clients can
simultaneously call to reprobe without any risk.

doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml

@@ -45,6 +45,8 @@
   <interface name="org.opensuse.Agama.Storage1">
     <method name="Probe">
     </method>
+    <method name="Reprobe">
+    </method>
     <method name="SetConfig">
       <arg name="serialized_config" direction="in" type="s"/>
       <arg name="result" direction="out" type="u"/>

doc/dbus/org.opensuse.Agama.Storage1.doc.xml

@@ -7,6 +7,11 @@
   <interface name="org.opensuse.Agama.Storage1">
     <method name="Probe">
     </method>
+    <!--
+      Probes the system and recalculates the proposal using the current config.
+    -->
+    <method name="Reprobe">
+    </method>
     <!--
       Sets the storage config.
     -->

rust/agama-lib/src/storage/client.rs

@@ -1,4 +1,4 @@
-// Copyright (c) [2024] SUSE LLC
+// Copyright (c) [2024-2025] SUSE LLC
 //
 // All Rights Reserved.
 //
@@ -139,6 +139,11 @@ impl<'a> StorageClient<'a> {
         Ok(self.storage_proxy.probe().await?)
     }
 
+    /// Runs the reprobing process
+    pub async fn reprobe(&self) -> Result<(), ServiceError> {
+        Ok(self.storage_proxy.reprobe().await?)
+    }
+
     /// Set the storage config according to the JSON schema
     pub async fn set_config(&self, settings: StorageSettings) -> Result<u32, ServiceError> {
         Ok(self

rust/agama-lib/src/storage/proxies/storage1.rs

@@ -1,4 +1,4 @@
-// Copyright (c) [2024] SUSE LLC
+// Copyright (c) [2024-2025] SUSE LLC
 //
 // All Rights Reserved.
 //
@@ -56,6 +56,9 @@ pub trait Storage1 {
     /// Probe method
     fn probe(&self) -> zbus::Result<()>;
 
+    /// Reprobe method
+    fn reprobe(&self) -> zbus::Result<()>;
+
     /// Set the storage config according to the JSON schema
     fn set_config(&self, settings: &str) -> zbus::Result<u32>;
 

rust/agama-server/src/storage/web.rs

@@ -1,4 +1,4 @@
-// Copyright (c) [2024] SUSE LLC
+// Copyright (c) [2024-2025] SUSE LLC
 //
 // All Rights Reserved.
 //
@@ -116,6 +116,7 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
         .route("/config", put(set_config).get(get_config))
         .route("/config_model", put(set_config_model).get(get_config_model))
         .route("/probe", post(probe))
+        .route("/reprobe", post(reprobe))
         .route("/devices/dirty", get(devices_dirty))
         .route("/devices/system", get(system_devices))
         .route("/devices/result", get(staging_devices))
@@ -240,7 +241,7 @@ async fn set_config_model(
     path = "/probe",
     context_path = "/api/storage",
     responses(
-        (status = 200, description = "Devices were probed and an initial proposal were performed"),
+        (status = 200, description = "Devices were probed and an initial proposal was performed"),
         (status = 400, description = "The D-Bus service could not perform the action")
     ),
     operation_id = "storage_probe"
@@ -249,6 +250,21 @@ async fn probe(State(state): State<StorageState<'_>>) -> Result<Json<()>, Error>
     Ok(Json(state.client.probe().await?))
 }
 
+/// Reprobes the storage devices.
+#[utoipa::path(
+    post,
+    path = "/reprobe",
+    context_path = "/api/storage",
+    responses(
+        (status = 200, description = "Devices were probed and the proposal was recalculated"),
+        (status = 400, description = "The D-Bus service could not perform the action")
+    ),
+    operation_id = "storage_reprobe"
+)]
+async fn reprobe(State(state): State<StorageState<'_>>) -> Result<Json<()>, Error> {
+    Ok(Json(state.client.reprobe().await?))
+}
+
 /// Gets 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,

service/lib/agama/dbus/storage/manager.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2024] SUSE LLC
+# Copyright (c) [2022-2025] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -87,13 +87,15 @@ def issues
         STORAGE_INTERFACE = "org.opensuse.Agama.Storage1"
         private_constant :STORAGE_INTERFACE
 
-        def probe
+        # @param keep_config [Boolean] Whether to use the current storage config for calculating
+        #   the proposal.
+        def probe(keep_config: false)
           busy_while do
             # Clean trees in advance to avoid having old objects exported in D-Bus.
             system_devices_tree.clean
             staging_devices_tree.clean
 
-            backend.probe
+            backend.probe(keep_config: keep_config)
           end
         end
 
@@ -162,6 +164,7 @@ def deprecated_system
 
         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

service/lib/agama/storage/manager.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2024] SUSE LLC
+# Copyright (c) [2022-2025] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -113,14 +113,21 @@ def on_probe(&block)
       end
 
       # Probes storage devices and performs an initial proposal
-      def probe
+      #
+      # @param keep_config [Boolean] Whether to use the current storage config for calculating the
+      #   proposal.
+      def probe(keep_config: false)
         start_progress_with_size(4)
         product_config.pick_product(software.selected_product)
         check_multipath
         progress.step(_("Activating storage devices")) { activate_devices }
         progress.step(_("Probing storage devices")) { probe_devices }
-        progress.step(_("Calculating the storage proposal")) { calculate_proposal }
+        progress.step(_("Calculating the storage proposal")) do
+          calculate_proposal(keep_config: keep_config)
+        end
         progress.step(_("Selecting Linux Security Modules")) { security.probe }
+        # The system is not deprecated anymore
+        self.deprecated_system = false
         update_issues
         @on_probe_callbacks&.each(&:call)
       end
@@ -216,14 +223,15 @@ def probe_devices
 
         iscsi.probe
         Y2Storage::StorageManager.instance.probe(callbacks)
-
-        # The system is not deprecated anymore
-        self.deprecated_system = false
       end
 
-      # Calculates the proposal using the storage config from the product.
-      def calculate_proposal
-        config_json = ConfigJSONReader.new(product_config).read
+      # 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
 

service/test/agama/storage/manager_test.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2024] SUSE LLC
+# Copyright (c) [2022-2025] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -167,6 +167,10 @@
       allow(proposal).to receive(:issues).and_return(proposal_issues)
       allow(proposal).to receive(:available_devices).and_return(devices)
       allow(proposal).to receive(:calculate_from_json)
+      allow(proposal).to receive(:storage_json).and_return(current_config)
+
+      allow_any_instance_of(Agama::Storage::ConfigJSONReader)
+        .to receive(:read).and_return(default_config)
 
       allow(config).to receive(:pick_product)
       allow(iscsi).to receive(:activate)
@@ -181,6 +185,26 @@
 
     let(:proposal) { Agama::Storage::Proposal.new(config, logger: logger) }
 
+    let(:default_config) do
+      {
+        storage: {
+          drives: [
+            search: "/dev/vda1"
+          ]
+        }
+      }
+    end
+
+    let(:current_config) do
+      {
+        storage: {
+          drives: [
+            search: "/dev/vda2"
+          ]
+        }
+      }
+    end
+
     let(:iscsi) { Agama::Storage::ISCSI::Manager.new }
 
     let(:devices) { [disk1, disk2] }
@@ -194,7 +218,7 @@
 
     let(:callback) { proc {} }
 
-    it "probes the storage devices and calculates a proposal with the default settings" do
+    it "probes the storage devices and calculates a proposal" do
       expect(config).to receive(:pick_product).with("ALP")
       expect(iscsi).to receive(:activate)
       expect(y2storage_manager).to receive(:activate) do |callbacks|
@@ -237,6 +261,24 @@
       storage.probe
     end
 
+    context "if :keep_config is false" do
+      let(:keep_config) { false }
+
+      it "calculates a proposal using the default product config" do
+        expect(proposal).to receive(:calculate_from_json).with(default_config)
+        storage.probe(keep_config: keep_config)
+      end
+    end
+
+    context "if :keep_config is true" do
+      let(:keep_config) { true }
+
+      it "calculates a proposal using the current config" do
+        expect(proposal).to receive(:calculate_from_json).with(current_config)
+        storage.probe(keep_config: keep_config)
+      end
+    end
+
     context "if there are available devices" do
       let(:devices) { [disk1] }
 

web/src/api/storage.ts

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2024] SUSE LLC
+ * Copyright (c) [2024-2025] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -22,16 +22,17 @@
 
 import { get, post, put } from "~/api/http";
 import { Job } from "~/types/job";
-import { calculate, fetchSettings } from "~/api/storage/proposal";
-import { config, configModel } from "~/api/storage/types";
+import { Action, config, configModel, ProductParams, Volume } from "~/api/storage/types";
 
 /**
  * Starts the storage probing process.
  */
-
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 const probe = (): Promise<any> => post("/api/storage/probe");
 
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+const reprobe = (): Promise<any> => post("/api/storage/reprobe");
+
 const fetchConfig = (): Promise<config.Config | undefined> =>
   get("/api/storage/config").then((config) => config.storage);
 
@@ -42,6 +43,17 @@ const setConfig = (config: config.Config) => put("/api/storage/config", { storag
 
 const setConfigModel = (model: configModel.Config) => put("/api/storage/config_model", model);
 
+const fetchUsableDevices = (): Promise<number[]> => get(`/api/storage/proposal/usable_devices`);
+
+const fetchProductParams = (): Promise<ProductParams> => get("/api/storage/product/params");
+
+const fetchDefaultVolume = (mountPath: string): Promise<Volume | undefined> => {
+  const path = encodeURIComponent(mountPath);
+  return get(`/api/storage/product/volume_for?mount_path=${path}`);
+};
+
+const fetchActions = (): Promise<Action[]> => get("/api/storage/devices/actions");
+
 /**
  * Returns the list of jobs
  */
@@ -53,26 +65,17 @@ const fetchStorageJobs = (): Promise<Job[]> => get("/api/storage/jobs");
 const findStorageJob = (id: string): Promise<Job | undefined> =>
   fetchStorageJobs().then((jobs: Job[]) => jobs.find((value) => value.id === id));
 
-/**
- * Refreshes the storage layer.
- *
- * It does the probing again and recalculates the proposal with the same
- * settings. Internally, it is composed of three different API calls
- * (retrieve the settings, probe the system, and calculate the proposal).
- */
-const refresh = async (): Promise<void> => {
-  const settings = await fetchSettings();
-  await probe().catch(console.log);
-  await calculate(settings).catch(console.log);
-};
-
 export {
   probe,
+  reprobe,
   fetchConfig,
   fetchConfigModel,
   setConfig,
   setConfigModel,
+  fetchUsableDevices,
+  fetchProductParams,
+  fetchDefaultVolume,
+  fetchActions,
   fetchStorageJobs,
   findStorageJob,
-  refresh,
 };

web/src/api/storage/proposal.ts

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) [2024] SUSE LLC
+ * Copyright (c) [2024-2025] SUSE LLC
  *
  * All Rights Reserved.
  *
@@ -20,38 +20,10 @@
  * find current contact information at www.suse.com.
  */
 
-import { get, put } from "../http";
-import {
-  Action,
-  ProductParams,
-  ProposalSettings,
-  ProposalSettingsPatch,
-  Volume,
-} from "~/api/storage/types";
-
-const fetchUsableDevices = (): Promise<number[]> => get(`/api/storage/proposal/usable_devices`);
-
-const fetchProductParams = (): Promise<ProductParams> => get("/api/storage/product/params");
-
-const fetchDefaultVolume = (mountPath: string): Promise<Volume | undefined> => {
-  const path = encodeURIComponent(mountPath);
-  return get(`/api/storage/product/volume_for?mount_path=${path}`);
-};
-
-// NOTE: the settings might not exist.
-const fetchSettings = (): Promise<ProposalSettings> =>
-  get("/api/storage/proposal/settings").catch(() => null);
-
-const fetchActions = (): Promise<Action[]> => get("/api/storage/devices/actions");
+import { put } from "../http";
+import { ProposalSettingsPatch } from "~/api/storage/types";
 
 const calculate = (settings: ProposalSettingsPatch) =>
   put("/api/storage/proposal/settings", settings);
 
-export {
-  fetchUsableDevices,
-  fetchProductParams,
-  fetchDefaultVolume,
-  fetchSettings,
-  fetchActions,
-  calculate,
-};
+export { calculate };