diff --git a/autoinstallation/README.md b/autoinstallation/README.md index 02ef2225c7..35f85bb276 100644 --- a/autoinstallation/README.md +++ b/autoinstallation/README.md @@ -39,11 +39,7 @@ differences: "product": "ALP-Bedrock" }, "storage": { - "devices": [ - { - "name": "/dev/sda" - } - ] + "bootDevice": "/dev/sda" }, "user": { "fullName": "Jane Doe", @@ -62,8 +58,9 @@ and `root`. - **`localization`** *(object)*: Localization settings. - **`language`** *(string)*: System language ID (e.g., "en_US"). - **`storage`** *(object)*: Storage settings. - - **`devices`** *(array of strings)*: Storage devices to install the system to (e.g, - ["/dev/sda"]). + - **`bootDevice`** *(string)*: Storage device used for booting (e.g., "/dev/sda"). By default, all file system are created in the boot device. + - **`lvm`** *(boolean)*: Whether LVM is used. + - **`encryptionPassword`** *(string)*: If set, the devices are encrypted using the given password. - **`user`** *(object)*: First user settings. - **`fullName`** *(string)*: Full name (e.g., "Jane Doe"). - **`userName`** *(string)*: User login name (e.g., "jane.doe"). @@ -100,11 +97,7 @@ local findBiggestDisk(disks) = language: 'en_US', }, storage: { - devices: [ - { - name: findBiggestDisk(agama.disks), - }, - ], + bootDevice: findBiggestDisk(agama.disks), }, } ``` diff --git a/rust/agama-lib/share/examples/profile.json b/rust/agama-lib/share/examples/profile.json index bc379d9153..484108b9d4 100644 --- a/rust/agama-lib/share/examples/profile.json +++ b/rust/agama-lib/share/examples/profile.json @@ -7,11 +7,7 @@ "product": "ALP" }, "storage": { - "devices": [ - { - "name": "/dev/dm-1" - } - ] + "bootDevice": "/dev/dm-1" }, "user": { "fullName": "Jane Doe", diff --git a/rust/agama-lib/share/examples/profile.jsonnet b/rust/agama-lib/share/examples/profile.jsonnet index b893150f27..1d04c5a0ec 100644 --- a/rust/agama-lib/share/examples/profile.jsonnet +++ b/rust/agama-lib/share/examples/profile.jsonnet @@ -38,11 +38,7 @@ local findBiggestDisk(disks) = keyboard: 'en_US', }, storage: { - devices: [ - { - name: findBiggestDisk(agama.disks), - }, - ], + bootDevice: findBiggestDisk(agama.disks), }, network: { connections: [ diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index 842dd0f13b..3244483a1d 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -193,19 +193,17 @@ "description": "Storage settings", "type": "object", "properties": { - "devices": { - "description": "Storage devices to install the system to", - "type": "array", - "items": { - "type": "object", - "additionalProperties": false, - "properties": { - "name": { - "description": "Storage device name (e.g., '/dev/sda')", - "type": "string" - } - } - } + "bootDevice": { + "description": "Device used for booting (e.g., '/dev/sda'). By default, all file systems are created in the boot device.", + "type": "string" + }, + "lvm": { + "description": "Whether LVM is used.", + "type": "boolean" + }, + "encryptionPassword": { + "description": "If set, the devices are encrypted using the given password.", + "type": "string" } } } diff --git a/rust/agama-lib/src/software/store.rs b/rust/agama-lib/src/software/store.rs index ad1a76d562..45706ed531 100644 --- a/rust/agama-lib/src/software/store.rs +++ b/rust/agama-lib/src/software/store.rs @@ -2,17 +2,20 @@ use super::{SoftwareClient, SoftwareSettings}; use crate::error::ServiceError; +use crate::manager::ManagerClient; use zbus::Connection; /// Loads and stores the software settings from/to the D-Bus service. pub struct SoftwareStore<'a> { software_client: SoftwareClient<'a>, + manager_client: ManagerClient<'a>, } impl<'a> SoftwareStore<'a> { pub async fn new(connection: Connection) -> Result, ServiceError> { Ok(Self { - software_client: SoftwareClient::new(connection).await?, + software_client: SoftwareClient::new(connection.clone()).await?, + manager_client: ManagerClient::new(connection).await?, }) } @@ -30,6 +33,7 @@ impl<'a> SoftwareStore<'a> { let ids: Vec = products.into_iter().map(|p| p.id).collect(); if ids.contains(product) { self.software_client.select_product(product).await?; + self.manager_client.probe().await?; } else { return Err(ServiceError::UnknownProduct(product.clone(), ids)); } diff --git a/rust/agama-lib/src/storage/client.rs b/rust/agama-lib/src/storage/client.rs index 4748cb7d63..97d6829a60 100644 --- a/rust/agama-lib/src/storage/client.rs +++ b/rust/agama-lib/src/storage/client.rs @@ -1,9 +1,12 @@ //! Implements a client to access Agama's storage service. -use super::proxies::{CalculatorProxy, Storage1Proxy, StorageProposalProxy}; +use super::proxies::{BlockDeviceProxy, ProposalCalculatorProxy, ProposalProxy, Storage1Proxy}; +use super::StorageSettings; use crate::error::ServiceError; +use futures::future::join_all; use serde::Serialize; use std::collections::HashMap; +use zbus::zvariant::OwnedObjectPath; use zbus::Connection; /// Represents a storage device @@ -16,14 +19,14 @@ pub struct StorageDevice { /// D-Bus client for the storage service pub struct StorageClient<'a> { pub connection: Connection, - calculator_proxy: CalculatorProxy<'a>, + calculator_proxy: ProposalCalculatorProxy<'a>, storage_proxy: Storage1Proxy<'a>, } impl<'a> StorageClient<'a> { pub async fn new(connection: Connection) -> Result, ServiceError> { Ok(Self { - calculator_proxy: CalculatorProxy::new(&connection).await?, + calculator_proxy: ProposalCalculatorProxy::new(&connection).await?, storage_proxy: Storage1Proxy::new(&connection).await?, connection, }) @@ -33,8 +36,8 @@ impl<'a> StorageClient<'a> { /// /// The proposal might not exist. // NOTE: should we implement some kind of memoization? - async fn proposal_proxy(&self) -> Result, ServiceError> { - Ok(StorageProposalProxy::new(&self.connection).await?) + async fn proposal_proxy(&self) -> Result, ServiceError> { + Ok(ProposalProxy::new(&self.connection).await?) } /// Returns the available devices @@ -46,20 +49,67 @@ impl<'a> StorageClient<'a> { .available_devices() .await? .into_iter() - .map(|(name, description, _)| StorageDevice { name, description }) + .map(|path| self.storage_device(path)) .collect(); - Ok(devices) + + return join_all(devices).await.into_iter().collect(); + } + + /// Returns the storage device for the given D-Bus path + async fn storage_device( + &self, + dbus_path: OwnedObjectPath, + ) -> Result { + let proxy = BlockDeviceProxy::builder(&self.connection) + .path(dbus_path)? + .build() + .await?; + + let name = proxy.name().await?; + // TODO: The description is not used yet. Decide what info to show, for example the device + // size, see https://crates.io/crates/size. + let description = name.clone(); + + Ok(StorageDevice { name, description }) + } + + /// Returns the boot device proposal setting + pub async fn boot_device(&self) -> Result, ServiceError> { + let proxy = self.proposal_proxy().await?; + let value = self.proposal_value(proxy.boot_device().await)?; + + match value { + Some(v) if v.is_empty() => Ok(None), + Some(v) => Ok(Some(v)), + None => Ok(None), + } + } + + /// Returns the lvm proposal setting + pub async fn lvm(&self) -> Result, ServiceError> { + let proxy = self.proposal_proxy().await?; + self.proposal_value(proxy.lvm().await) } - /// Returns the candidate devices for the proposal - pub async fn candidate_devices(&self) -> Result, ServiceError> { + /// Returns the encryption password proposal setting + pub async fn encryption_password(&self) -> Result, ServiceError> { let proxy = self.proposal_proxy().await?; - match proxy.candidate_devices().await { - Ok(devices) => Ok(devices), + let value = self.proposal_value(proxy.encryption_password().await)?; + + match value { + Some(v) if v.is_empty() => Ok(None), + Some(v) => Ok(Some(v)), + None => Ok(None), + } + } + + fn proposal_value(&self, value: Result) -> Result, ServiceError> { + match value { + Ok(v) => Ok(Some(v)), Err(zbus::Error::MethodError(name, _, _)) if name.as_str() == "org.freedesktop.DBus.Error.UnknownObject" => { - Ok(vec![]) + Ok(None) } Err(e) => Err(e.into()), } @@ -70,22 +120,24 @@ impl<'a> StorageClient<'a> { Ok(self.storage_proxy.probe().await?) } - pub async fn calculate( - &self, - candidate_devices: Vec, - encryption_password: String, - lvm: bool, - ) -> Result { - let mut settings: HashMap<&str, zbus::zvariant::Value<'_>> = HashMap::new(); - settings.insert( - "CandidateDevices", - zbus::zvariant::Value::new(candidate_devices), - ); - settings.insert( - "EncryptionPassword", - zbus::zvariant::Value::new(encryption_password), - ); - settings.insert("LVM", zbus::zvariant::Value::new(lvm)); - Ok(self.calculator_proxy.calculate(settings).await?) + pub async fn calculate(&self, settings: &StorageSettings) -> Result { + let mut dbus_settings: HashMap<&str, zbus::zvariant::Value<'_>> = HashMap::new(); + + if let Some(boot_device) = settings.boot_device.clone() { + dbus_settings.insert("BootDevice", zbus::zvariant::Value::new(boot_device)); + } + + if let Some(encryption_password) = settings.encryption_password.clone() { + dbus_settings.insert( + "EncryptionPassword", + zbus::zvariant::Value::new(encryption_password), + ); + } + + if let Some(lvm) = settings.lvm { + dbus_settings.insert("LVM", zbus::zvariant::Value::new(lvm)); + } + + Ok(self.calculator_proxy.calculate(dbus_settings).await?) } } diff --git a/rust/agama-lib/src/storage/proxies.rs b/rust/agama-lib/src/storage/proxies.rs index 41a123c175..29f08c79cd 100644 --- a/rust/agama-lib/src/storage/proxies.rs +++ b/rust/agama-lib/src/storage/proxies.rs @@ -1,23 +1,8 @@ -//! D-Bus interface proxies for: `org.opensuse.Agama.Storage1.*` +//! D-Bus interface proxies for interfaces implemented by objects in the storage service. //! -//! This code was generated by `zbus-xmlgen` `3.1.0` from DBus introspection data.`. +//! This code was generated by `zbus-xmlgen` `3.1.1` from DBus introspection data. use zbus::dbus_proxy; -#[dbus_proxy( - interface = "org.opensuse.Agama1.Validation", - default_service = "org.opensuse.Agama.Storage1", - default_path = "/org/opensuse/Agama/Storage1" -)] -trait Validation { - /// Errors property - #[dbus_proxy(property)] - fn errors(&self) -> zbus::Result>; - - /// Valid property - #[dbus_proxy(property)] - fn valid(&self) -> zbus::Result; -} - #[dbus_proxy( interface = "org.opensuse.Agama.Storage1", default_service = "org.opensuse.Agama.Storage1", @@ -43,34 +28,30 @@ trait Storage1 { default_service = "org.opensuse.Agama.Storage1", default_path = "/org/opensuse/Agama/Storage1" )] -trait Calculator { +trait ProposalCalculator { /// Calculate method fn calculate( &self, settings: std::collections::HashMap<&str, zbus::zvariant::Value<'_>>, ) -> zbus::Result; + /// DefaultVolume method + fn default_volume( + &self, + mount_path: &str, + ) -> zbus::Result>; + /// AvailableDevices property #[dbus_proxy(property)] - fn available_devices( - &self, - ) -> zbus::Result< - Vec<( - String, - String, - std::collections::HashMap, - )>, - >; + fn available_devices(&self) -> zbus::Result>; - /// Result property + /// ProductMountPoints property #[dbus_proxy(property)] - fn result(&self) -> zbus::Result; + fn product_mount_points(&self) -> zbus::Result>; - /// VolumeTemplates property + /// Result property #[dbus_proxy(property)] - fn volume_templates( - &self, - ) -> zbus::Result>>; + fn result(&self) -> zbus::Result; } #[dbus_proxy( @@ -78,16 +59,24 @@ trait Calculator { default_service = "org.opensuse.Agama.Storage1", default_path = "/org/opensuse/Agama/Storage1/Proposal" )] -trait StorageProposal { +trait Proposal { /// Actions property #[dbus_proxy(property)] fn actions( &self, ) -> zbus::Result>>; - /// CandidateDevices property + /// BootDevice property + #[dbus_proxy(property)] + fn boot_device(&self) -> zbus::Result; + + /// EncryptionMethod property #[dbus_proxy(property)] - fn candidate_devices(&self) -> zbus::Result>; + fn encryption_method(&self) -> zbus::Result; + + /// EncryptionPBKDFunction property + #[dbus_proxy(property, name = "EncryptionPBKDFunction")] + fn encryption_pbkdfunction(&self) -> zbus::Result; /// EncryptionPassword property #[dbus_proxy(property)] @@ -97,9 +86,47 @@ trait StorageProposal { #[dbus_proxy(property, name = "LVM")] fn lvm(&self) -> zbus::Result; + /// SpacePolicy property + #[dbus_proxy(property)] + fn space_policy(&self) -> zbus::Result; + + /// SystemVGDevices property + #[dbus_proxy(property, name = "SystemVGDevices")] + fn system_vg_devices(&self) -> zbus::Result>>; + /// Volumes property #[dbus_proxy(property)] fn volumes( &self, ) -> zbus::Result>>; } + +#[dbus_proxy( + interface = "org.opensuse.Agama.Storage1.Block", + default_service = "org.opensuse.Agama.Storage1" +)] +trait BlockDevice { + /// Active property + #[dbus_proxy(property)] + fn active(&self) -> zbus::Result; + + /// Name property + #[dbus_proxy(property)] + fn name(&self) -> zbus::Result; + + /// Size property + #[dbus_proxy(property)] + fn size(&self) -> zbus::Result; + + /// Systems property + #[dbus_proxy(property)] + fn systems(&self) -> zbus::Result>; + + /// UdevIds property + #[dbus_proxy(property)] + fn udev_ids(&self) -> zbus::Result>; + + /// UdevPaths property + #[dbus_proxy(property)] + fn udev_paths(&self) -> zbus::Result>; +} diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index c6873edfa0..cb786d651f 100644 --- a/rust/agama-lib/src/storage/settings.rs +++ b/rust/agama-lib/src/storage/settings.rs @@ -1,6 +1,6 @@ //! Representation of the storage settings -use agama_settings::{error::ConversionError, SettingObject, Settings}; +use agama_settings::Settings; use serde::{Deserialize, Serialize}; /// Storage settings for installation @@ -11,34 +11,6 @@ pub struct StorageSettings { pub lvm: Option, /// Encryption password for the storage devices (in clear text) pub encryption_password: Option, - /// Devices to use in the installation - #[settings(collection)] - pub devices: Vec, -} - -/// Device to use in the installation -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct Device { - /// Device name (e.g., "/dev/sda") - pub name: String, -} - -impl From for Device { - fn from(value: String) -> Self { - Self { name: value } - } -} - -impl TryFrom for Device { - type Error = ConversionError; - - fn try_from(value: SettingObject) -> Result { - match value.get("name") { - Some(name) => Ok(Device { - name: name.clone().try_into()?, - }), - _ => Err(ConversionError::MissingKey("name".to_string())), - } - } + /// Boot device to use in the installation + pub boot_device: Option, } diff --git a/rust/agama-lib/src/storage/store.rs b/rust/agama-lib/src/storage/store.rs index 8b4118794d..7c6ea81b51 100644 --- a/rust/agama-lib/src/storage/store.rs +++ b/rust/agama-lib/src/storage/store.rs @@ -18,22 +18,20 @@ impl<'a> StorageStore<'a> { } pub async fn load(&self) -> Result { - let names = self.storage_client.candidate_devices().await?; - let devices = names.into_iter().map(|n| n.into()).collect(); + let boot_device = self.storage_client.boot_device().await?; + let lvm = self.storage_client.lvm().await?; + let encryption_password = self.storage_client.encryption_password().await?; + Ok(StorageSettings { - devices, + boot_device, + lvm, + encryption_password, ..Default::default() }) } pub async fn store(&self, settings: &StorageSettings) -> Result<(), ServiceError> { - self.storage_client - .calculate( - settings.devices.iter().map(|d| d.name.clone()).collect(), - settings.encryption_password.clone().unwrap_or_default(), - settings.lvm.unwrap_or_default(), - ) - .await?; + self.storage_client.calculate(settings).await?; Ok(()) } } diff --git a/rust/package/agama-cli.changes b/rust/package/agama-cli.changes index 5b423fe9de..b686dd2782 100644 --- a/rust/package/agama-cli.changes +++ b/rust/package/agama-cli.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Sep 19 11:16:16 UTC 2023 - José Iván López González + +- Adapt to new storage D-Bus API and explicitly call to probe after + selecting a new product (gh#openSUSE/agama#748). + ------------------------------------------------------------------- Thu Sep 14 19:44:57 UTC 2023 - Josef Reidinger diff --git a/service/lib/agama/dbus/manager.rb b/service/lib/agama/dbus/manager.rb index 55b66ffb82..1a759ce672 100644 --- a/service/lib/agama/dbus/manager.rb +++ b/service/lib/agama/dbus/manager.rb @@ -170,15 +170,6 @@ def register_callbacks backend.on_services_status_change do dbus_properties_changed(MANAGER_INTERFACE, { "BusyServices" => busy_services }, []) end - - backend.software.on_product_selected do |_product| - if service_status.busy? - logger.warn "Could not process the product change because the service is busy" - next - end - - busy_while { backend.config_phase } - end end end end diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index cb7f1ea4cc..5746b92318 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Sep 19 11:14:42 UTC 2023 - José Iván López González + +- Do not automatically probe after selecting a new product + (gh#openSUSE/agama#748). + ------------------------------------------------------------------- Thu Sep 14 09:04:29 UTC 2023 - Imobach Gonzalez Sosa diff --git a/service/test/agama/dbus/manager_test.rb b/service/test/agama/dbus/manager_test.rb index 627bb57962..d0b62ae234 100644 --- a/service/test/agama/dbus/manager_test.rb +++ b/service/test/agama/dbus/manager_test.rb @@ -81,11 +81,6 @@ subject end - it "configures callbacks to be called when a product is selected" do - expect(software_client).to receive(:on_product_selected) - subject - end - it "configures callbacks from Progress interface" do expect_any_instance_of(described_class).to receive(:register_progress_callbacks) subject diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 187fa506a9..e13348923d 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Sep 19 11:18:05 UTC 2023 - José Iván López González + +- Explicitly call to probe after selecting a new product + (gh#openSUSE/agama#748). + ------------------------------------------------------------------- Thu Sep 14 10:09:07 UTC 2023 - Imobach Gonzalez Sosa diff --git a/web/src/components/software/ProductSelectionPage.jsx b/web/src/components/software/ProductSelectionPage.jsx index d836bff8ef..c6f9bfa93a 100644 --- a/web/src/components/software/ProductSelectionPage.jsx +++ b/web/src/components/software/ProductSelectionPage.jsx @@ -52,7 +52,7 @@ function ProductSelectionPage() { const isSelected = p => p.id === selected; - const accept = (e) => { + const accept = async (e) => { e.preventDefault(); if (selected === previous) { navigate("/"); @@ -60,8 +60,9 @@ function ProductSelectionPage() { } // TODO: handle errors - client.software.selectProduct(selected) - .then(() => navigate("/")); + await client.software.selectProduct(selected); + client.manager.startProbing(); + navigate("/"); }; if (!products) return ( diff --git a/web/src/components/software/ProductSelectionPage.test.jsx b/web/src/components/software/ProductSelectionPage.test.jsx index b726a37b45..8fbc94477d 100644 --- a/web/src/components/software/ProductSelectionPage.test.jsx +++ b/web/src/components/software/ProductSelectionPage.test.jsx @@ -51,6 +51,10 @@ jest.mock("~/context/software", () => ({ jest.mock("~/components/layout/Layout", () => mockLayout()); +const managerMock = { + startProbing: jest.fn() +}; + const softwareMock = { getProducts: () => Promise.resolve(products), getSelectedProduct: jest.fn(() => Promise.resolve(products[0])), @@ -60,7 +64,10 @@ const softwareMock = { beforeEach(() => { createClient.mockImplementation(() => { - return { software: softwareMock }; + return { + manager: managerMock, + software: softwareMock + }; }); }); @@ -72,6 +79,7 @@ describe("when the user chooses a product", () => { const button = await screen.findByRole("button", { name: "Select" }); await user.click(button); expect(softwareMock.selectProduct).toHaveBeenCalledWith("MicroOS"); + expect(managerMock.startProbing).toHaveBeenCalled(); expect(mockNavigateFn).toHaveBeenCalledWith("/"); }); }); @@ -83,6 +91,7 @@ describe("when the user chooses does not change the product", () => { const button = await screen.findByRole("button", { name: "Select" }); await user.click(button); expect(softwareMock.selectProduct).not.toHaveBeenCalled(); + expect(managerMock.startProbing).not.toHaveBeenCalled(); expect(mockNavigateFn).toHaveBeenCalledWith("/"); }); });