From 18c8e8a657163ebec236613bc3d92ed3126dbec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:15:45 +0000 Subject: [PATCH 01/29] service: Add settings for different target devices --- .../{lvm_settings.rb => boot_settings.rb} | 21 ++-- service/lib/agama/storage/device_settings.rb | 66 ++++++++++ .../lib/agama/storage/proposal_settings.rb | 77 +++++++++--- .../agama/storage/proposal_settings_test.rb | 119 ++++++++++++++++++ 4 files changed, 255 insertions(+), 28 deletions(-) rename service/lib/agama/storage/{lvm_settings.rb => boot_settings.rb} (68%) create mode 100644 service/lib/agama/storage/device_settings.rb create mode 100644 service/test/agama/storage/proposal_settings_test.rb diff --git a/service/lib/agama/storage/lvm_settings.rb b/service/lib/agama/storage/boot_settings.rb similarity index 68% rename from service/lib/agama/storage/lvm_settings.rb rename to service/lib/agama/storage/boot_settings.rb index 245ad27b45..4ef2e4dca8 100644 --- a/service/lib/agama/storage/lvm_settings.rb +++ b/service/lib/agama/storage/boot_settings.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2024] SUSE LLC # # All Rights Reserved. # @@ -21,22 +21,21 @@ module Agama module Storage - # Settings regarding LVM for the Agama storage proposal - class LvmSettings - # Whether to use LVM + # Class for configuring the boot settings of the Agama storage proposal. + class BootSettings + # Whether to configure partitions for booting. # # @return [Boolean] - attr_accessor :enabled - alias_method :enabled?, :enabled + attr_accessor :configure + alias_method :configure?, :configure - # Devices to use for the system LVM volume group + # Device to use for booting. # - # @return [Array] - attr_accessor :system_vg_devices + # @return [String, nil] + attr_accessor :device def initialize - @enabled = false - @system_vg_devices = [] + @configure = true end end end diff --git a/service/lib/agama/storage/device_settings.rb b/service/lib/agama/storage/device_settings.rb new file mode 100644 index 0000000000..ec44343cdc --- /dev/null +++ b/service/lib/agama/storage/device_settings.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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. + +module Agama + module Storage + # Module for the settings to configure the device of the Agama storage proposal. + module DeviceSettings + # Class to be used when the target device is a disk. + class Disk + # Name of the device to use. + # + # @return [String, nil] + attr_accessor :name + + # @param name [String, nil] + def initialize(name = nil) + @name = name + end + end + + # Class to be used when the target device is a reused LVM volume group. + class ReusedLvmVg + # Name of the LVM volume group to reuse. + # + # @return [String, nil] + attr_accessor :name + + # @param name [String, nil] + def initialize(name = nil) + @name = name + end + end + + # Class to be used when the target device is a new LVM volume group. + class NewLvmVg + # List of candidate devices to create physical volumes. + # + # @return [Array] + attr_accessor :candidate_pv_devices + + # @param candidates [Array] + def initialize(candidates = []) + @candidate_pv_devices = candidates + end + end + end + end +end diff --git a/service/lib/agama/storage/proposal_settings.rb b/service/lib/agama/storage/proposal_settings.rb index c0e237de50..9ad451a402 100644 --- a/service/lib/agama/storage/proposal_settings.rb +++ b/service/lib/agama/storage/proposal_settings.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022-2023] SUSE LLC +# Copyright (c) [2022-2024] SUSE LLC # # All Rights Reserved. # @@ -19,7 +19,8 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "agama/storage/lvm_settings" +require "agama/storage/boot_settings" +require "agama/storage/device_settings" require "agama/storage/encryption_settings" require "agama/storage/space_settings" @@ -27,38 +28,80 @@ module Agama module Storage # Settings used to calculate an storage proposal. class ProposalSettings - # Configuration of LVM + # Target device settings. # - # @return [LvmSettings] - attr_reader :lvm + # @return [DeviceSettings::Disk, DeviceSettings::NewLvmVg, DeviceSettings::ReusedLvmVg] + attr_accessor :device - # Encryption settings + # Boot settings. # - # @return [EncryptionSettings] - attr_reader :encryption + # @return [BootSettings] + attr_accessor :boot - # Settings to configure the behavior when making space to allocate the new partitions + # Encryption settings. # - # @return [SpaceSettings] - attr_reader :space + # @return [EncryptionSettings] + attr_accessor :encryption - # Device name of the disk that will be used for booting the system and also to allocate all - # the partitions, except those that have been explicitly assigned to other disk(s). + # Settings to configure the behavior when making space to allocate the new partitions. # - # @return [String, nil] nil if no device has been selected yet. - attr_accessor :boot_device + # @return [SpaceSettings] + attr_accessor :space - # Set of volumes to create + # Set of volumes to create. # # @return [Array] attr_accessor :volumes def initialize - @lvm = LvmSettings.new + @device = DeviceSettings::Disk.new + @boot = BootSettings.new @encryption = EncryptionSettings.new @space = SpaceSettings.new @volumes = [] end + + # All devices involved in the installation. + # + # @return [Array] + def installation_devices + [boot_device, target_devices, volume_devices] + .flatten + .compact + .uniq + end + + private + + # Device used for booting. + # + # @return [String, nil] + def boot_device + return nil unless boot.configure? + + boot.device + end + + # Target devices for the installation depending on the device settings. + # + # @return [Array] + def target_devices + case device + when DeviceSettings::Disk, DeviceSettings::ReusedLvmVg + [device.name].compact + when DeviceSettings::NewLvmVg + device.candidate_pv_devices + else + [] + end + end + + # Devices directly assigned to the volumes. + # + # @return [Array] + def volume_devices + volumes.map(&:location).reject(&:reuse_device?).map(&:device) + end end end end diff --git a/service/test/agama/storage/proposal_settings_test.rb b/service/test/agama/storage/proposal_settings_test.rb new file mode 100644 index 0000000000..75f3cb21a8 --- /dev/null +++ b/service/test/agama/storage/proposal_settings_test.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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. + +require_relative "../../test_helper" +require "agama/storage/device_settings" +require "agama/storage/proposal_settings" +require "agama/storage/volume" + +describe Agama::Storage::ProposalSettings do + describe "#installation_devices" do + shared_examples "boot device" do + context "when boot is set to be configured" do + before do + subject.boot.configure = true + subject.boot.device = "/dev/sdc" + end + + it "includes the boot device" do + expect(subject.installation_devices).to include("/dev/sdc") + end + end + + context "when boot is not set to be configured" do + before do + subject.boot.configure = false + subject.boot.device = "/dev/sdc" + end + + it "does not include the boot device" do + expect(subject.installation_devices).to_not include("/dev/sdc") + end + end + end + + shared_examples "volume devices" do + before do + subject.volumes = [volume1, volume2] + end + + let(:volume1) do + Agama::Storage::Volume.new("/").tap do |volume| + volume.location.target = :new_partition + volume.location.device = "/dev/sdd" + end + end + + let(:volume2) do + Agama::Storage::Volume.new("/").tap do |volume| + volume.location.target = :new_partition + volume.location.device = "/dev/sde" + end + end + + it "includes the devices assigned to the volumes" do + expect(subject.installation_devices).to include("/dev/sdd", "/dev/sde") + end + end + + context "when the device is configured to use a disk" do + before do + subject.device = Agama::Storage::DeviceSettings::Disk.new("/dev/sda") + end + + it "includes the target device" do + expect(subject.installation_devices).to include("/dev/sda") + end + + include_examples "boot device" + + include_examples "volume devices" + end + + context "when the device is configured to create a new LVM volume group" do + before do + subject.device = Agama::Storage::DeviceSettings::NewLvmVg.new(["/dev/sda", "/dev/sdb"]) + end + + it "includes the target devices for creating new LVM physical volumes" do + expect(subject.installation_devices).to include("/dev/sda", "/dev/sdb") + end + + include_examples "boot device" + + include_examples "volume devices" + end + + context "when the device is configured to reuse a LVM volume group" do + before do + subject.device = Agama::Storage::DeviceSettings::ReusedLvmVg.new("/dev/vg0") + end + + it "includes the target LVM volume group" do + expect(subject.installation_devices).to include("/dev/vg0") + end + + include_examples "boot device" + + include_examples "volume devices" + end + end +end From dab18673912a347dff835ca7622d66d9ee18f2af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:20:09 +0000 Subject: [PATCH 02/29] service: Adapt conversions to and from Y2Storage --- .../storage/proposal_settings_conversion.rb | 14 +- .../from_y2storage.rb | 110 +++------- .../to_y2storage.rb | 91 +++++--- .../lib/agama/storage/volume_conversion.rb | 14 +- .../volume_conversion/from_y2storage.rb | 80 ++----- .../storage/volume_conversion/to_y2storage.rb | 4 +- .../from_y2storage_test.rb | 188 ++++------------- .../to_y2storage_test.rb | 198 +++++++++++------- .../proposal_settings_conversion_test.rb | 12 +- .../volume_conversion/from_y2storage_test.rb | 145 +++++-------- .../volume_conversion/to_y2storage_test.rb | 64 +++--- .../agama/storage/volume_conversion_test.rb | 17 +- 12 files changed, 388 insertions(+), 549 deletions(-) diff --git a/service/lib/agama/storage/proposal_settings_conversion.rb b/service/lib/agama/storage/proposal_settings_conversion.rb index 56e860a396..f8439c7531 100644 --- a/service/lib/agama/storage/proposal_settings_conversion.rb +++ b/service/lib/agama/storage/proposal_settings_conversion.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -26,17 +26,17 @@ module Agama module Storage # Conversions for the proposal settings. module ProposalSettingsConversion - # Performs conversion from Y2Storage format. + # Performs conversion from Y2Storage. # - # @param settings [Y2Storage::ProposalSettings] - # @param config [Agama::Config] + # @param y2storage_settings [Y2Storage::ProposalSettings] + # @param settings [Agama::Storage::ProposalSettings] # # @return [Agama::Storage::ProposalSettings] - def self.from_y2storage(settings, config:) - FromY2Storage.new(settings, config: config).convert + def self.from_y2storage(y2storage_settings, settings) + FromY2Storage.new(y2storage_settings, settings).convert end - # Performs conversion to Y2Storage format. + # Performs conversion to Y2Storage. # # @param settings [Agama::Storage::ProposalSettings] # @param config [Agama::Config] diff --git a/service/lib/agama/storage/proposal_settings_conversion/from_y2storage.rb b/service/lib/agama/storage/proposal_settings_conversion/from_y2storage.rb index a60a0c59a3..475e53e25c 100644 --- a/service/lib/agama/storage/proposal_settings_conversion/from_y2storage.rb +++ b/service/lib/agama/storage/proposal_settings_conversion/from_y2storage.rb @@ -19,30 +19,32 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "agama/storage/proposal_settings" require "agama/storage/volume_conversion" module Agama module Storage module ProposalSettingsConversion - # Proposal settings conversion from Y2Storage format. + # Proposal settings conversion from Y2Storage. + # + # @note This class does not perform a real conversion from Y2Storage settings. Instead of + # that, it copies the given settings and recovers some values from Y2Storage. + # A real conversion is not needed because the original settings are always available. + # Moreover, Agama introduces some concepts that do not exist in the Y2Storage settings + # (e.g., target, boot device or space policy), which could be impossible to infer. class FromY2Storage - # @param settings [Y2Storage::ProposalSettings] - # @param config [Agama::Config] - def initialize(settings, config:) + # @param y2storage_settings [Y2Storage::ProposalSettings] + # @param settings [Agama::Storage::ProposalSettings] Settings to be copied and modified. + def initialize(y2storage_settings, settings) + @y2storage_settings = y2storage_settings @settings = settings - @config = config end - # Performs the conversion from Y2Storage format. + # Performs the conversion from Y2Storage. # # @return [Agama::Storage::ProposalSettings] def convert - ProposalSettings.new.tap do |target| - boot_device_conversion(target) - lvm_conversion(target) - encryption_conversion(target) - space_settings_conversion(target) + settings.dup.tap do |target| + space_actions_conversion(target) volumes_conversion(target) end end @@ -50,82 +52,32 @@ def convert private # @return [Y2Storage::ProposalSettings] - attr_reader :settings - - # @return [Agama::Config] - attr_reader :config - - # @param target [Agama::Storage::ProposalSettings] - def boot_device_conversion(target) - target.boot_device = settings.root_device - end - - # @param target [Agama::Storage::ProposalSettings] - def lvm_conversion(target) - target.lvm.enabled = settings.lvm - - # FIXME: The candidate devices list represents the system VG devices if it contains any - # device different to the root device. If the candidate devices only contains the root - # device, then there is no way to know whether the root device was explicitly assigned - # as system VG device. Note that candidate devices will also contain the root device - # when the system VG devices list was empty. - candidate_devices = settings.candidate_devices || [] - return unless candidate_devices.reject { |d| d == settings.root_device }.any? + attr_reader :y2storage_settings - target.lvm.system_vg_devices = settings.candidate_devices - end - - # @param target [Agama::Storage::ProposalSettings] - def encryption_conversion(target) - target.encryption.password = settings.encryption_password - target.encryption.method = settings.encryption_method - target.encryption.pbkd_function = settings.encryption_pbkdf - end + # @return [Agama::Storage::ProposalSettings] + attr_reader :settings - # @note The space policy cannot be inferred from Y2Storage settings. + # Recovers space actions. + # + # @note Space actions are generated in the conversion of the settings to Y2Storage format, + # see {ProposalSettingsConversion::ToY2Storage}. + # # @param target [Agama::Storage::ProposalSettings] - def space_settings_conversion(target) - target.space.actions = settings.space_settings.actions + def space_actions_conversion(target) + target.space.actions = y2storage_settings.space_settings.actions end + # Some values of the volumes have to be recovered from Y2Storage proposal. + # # @param target [Agama::Storage::ProposalSettings] def volumes_conversion(target) - target.volumes = volumes.select(&:proposed?).map do |spec| - VolumeConversion.from_y2storage(spec, config: config) - end - - fallbacks_conversion(target) - end - - # @param target [Agama::Storage::ProposalSettings] - def fallbacks_conversion(target) - target.volumes.each do |volume| - volume.min_size_fallback_for = volumes_with_min_size_fallback(volume.mount_path) - volume.max_size_fallback_for = volumes_with_max_size_fallback(volume.mount_path) - end - end - - # @param mount_path [String] - # @return [Array] - def volumes_with_min_size_fallback(mount_path) - specs = volumes.select { |s| s.fallback_for_min_size == mount_path } - specs.map(&:mount_point) + target.volumes = target.volumes.map { |v| volume_conversion(v) } end - # @param mount_path [String] - # @return [Array] - def volumes_with_max_size_fallback(mount_path) - specs = volumes.select { |s| s.fallback_for_max_size == mount_path } - specs.map(&:mount_point) - end - - # Volumes from settings. - # - # Note that volumes might be nil in Y2Storage settings. - # - # @return [Array] - def volumes - settings.volumes || [] + # @param volume [Agama::Storage::Volume] + # @return [Agama::Storage::Volume] + def volume_conversion(volume) + VolumeConversion.from_y2storage(volume) end end end diff --git a/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb b/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb index 0f695d0774..5ed2c94299 100644 --- a/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb +++ b/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -20,13 +20,14 @@ # find current contact information at www.suse.com. require "y2storage" +require "agama/storage/device_settings" require "agama/storage/volume_conversion" require "agama/storage/volume_templates_builder" module Agama module Storage module ProposalSettingsConversion - # Proposal settings conversion to Y2Storage format. + # Proposal settings conversion to Y2Storage. class ToY2Storage # @param settings [Agama::Storage::ProposalSettings] # @param config [Agama::Config] @@ -35,7 +36,7 @@ def initialize(settings, config:) @config = config end - # Performs the conversion to Y2Storage format. + # Performs the conversion to Y2Storage. # # @return [Y2Storage::ProposalSettings] def convert @@ -43,9 +44,8 @@ def convert # generic default values that are independent of the product (there is no YaST # ProductFeatures mechanism in place). Y2Storage::ProposalSettings.new_for_current_product.tap do |target| - boot_device_conversion(target) - candidate_devices_conversion(target) - lvm_conversion(target) + device_conversion(target) + boot_conversion(target) encryption_conversion(target) space_policy_conversion(target) volumes_conversion(target) @@ -61,32 +61,56 @@ def convert attr_reader :config # @param target [Y2Storage::ProposalSettings] - def boot_device_conversion(target) - target.root_device = settings.boot_device + def device_conversion(target) + device_settings = settings.device + + case device_settings + when DeviceSettings::Disk + disk_device_conversion(target) + when DeviceSettings::NewLvmVg + new_lvm_vg_device_conversion(target, device_settings) + when DeviceSettings::ReusedLvmVg + reused_lvm_vg_device_conversion(target, device_settings) + end end # @param target [Y2Storage::ProposalSettings] - def candidate_devices_conversion(target) - candidate_devices = [] - - if settings.lvm.enabled? && settings.lvm.system_vg_devices.any? - candidate_devices = settings.lvm.system_vg_devices - elsif settings.boot_device - candidate_devices = [settings.boot_device] - end + def disk_device_conversion(target) + target.lvm = false + target.candidate_devices = [settings.boot.device].compact + end - target.candidate_devices = candidate_devices + # @param target [Y2Storage::ProposalSettings] + # @param device_settings [DeviceSettings::NewLvmVg] + def new_lvm_vg_device_conversion(target, device_settings) + enable_lvm(target) + target.candidate_devices = device_settings.candidate_pv_devices end # @param target [Y2Storage::ProposalSettings] - def lvm_conversion(target) - lvm = settings.lvm.enabled? + # @param _device_settings [DeviceSettings::ReusedLvmVg] + def reused_lvm_vg_device_conversion(target, _device_settings) + enable_lvm(target) + # TODO: Sets the reused VG (not supported by yast2-storage-ng yet). + # TODO: Decide what to consider as candidate devices. + target.candidate_devices = [] + end - target.lvm = lvm - # Activate support for dedicated volume groups + # @param target [Y2Storage::ProposalSettings] + def enable_lvm(target) + target.lvm = true + # Activate support for dedicated volume groups. target.separate_vgs = true # Prevent VG reuse target.lvm_vg_reuse = false + # Create VG as big as needed to allocate the LVs. + target.lvm_vg_strategy = :use_needed + end + + # @param target [Y2Storage::ProposalSettings] + def boot_conversion(target) + target.root_device = settings.boot.device + target.boot = settings.boot.configure? end # @param target [Y2Storage::ProposalSettings] @@ -119,12 +143,14 @@ def volumes_conversion(target) target.swap_reuse = :none volumes = settings.volumes.map { |v| VolumeConversion.to_y2storage(v) } + disabled_volumes = missing_volumes.map do |volume| VolumeConversion.to_y2storage(volume).tap { |v| v.proposed = false } end target.volumes = volumes + disabled_volumes + volume_device_conversion(target) fallbacks_conversion(target) end @@ -137,6 +163,15 @@ def missing_volumes .reject { |t| t.mount_path.empty? } end + # Assigns the target device if needed. + # + # @param target [Y2Storage::ProposalSettings] + def volume_device_conversion(target) + return unless settings.device.is_a?(DeviceSettings::Disk) + + target.volumes.select(&:proposed?).each { |v| v.device ||= settings.device.name } + end + # @param target [Y2Storage::ProposalSettings] def fallbacks_conversion(target) target.volumes.each do |spec| @@ -163,17 +198,15 @@ def find_max_size_fallback(mount_path) # All block devices affected by the space policy. # - # This includes the partitions from the boot device, the candidate devices and from the - # devices directly assigned to a volume as target device. If a device is not partitioned, - # then the device itself is included. + # @see ProposalSettings#installation_devices + # + # If a device is partitioned, then its partitions are included instead of the device. # # @return [Array] def all_devices - devices = [settings.boot_device] + - settings.lvm.system_vg_devices + - settings.volumes.map(&:location).reject(&:reuse_device?).map(&:device) - - devices.compact.uniq.map { |d| device_or_partitions(d) }.flatten + settings.installation_devices + .map { |d| device_or_partitions(d) } + .flatten end # @param device [String] diff --git a/service/lib/agama/storage/volume_conversion.rb b/service/lib/agama/storage/volume_conversion.rb index 73ca398f91..432f39034f 100644 --- a/service/lib/agama/storage/volume_conversion.rb +++ b/service/lib/agama/storage/volume_conversion.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -26,17 +26,15 @@ module Agama module Storage # Conversions for a volume module VolumeConversion - # Performs conversion from Y2Storage format. - # - # @param spec [Y2Storage::VolumeSpecification] - # @param config [Agama::Config] + # Performs conversion from Y2Storage. # + # @param volume [Agama::Storage::Volume] # @return [Agama::Storage::Volume] - def self.from_y2storage(spec, config:) - FromY2Storage.new(spec, config: config).convert + def self.from_y2storage(volume) + FromY2Storage.new(volume).convert end - # Performs conversion to Y2Storage format. + # Performs conversion to Y2Storage. # # @param volume [Agama::Storage::Volume] # @return [Y2Storage::VolumeSpecification] diff --git a/service/lib/agama/storage/volume_conversion/from_y2storage.rb b/service/lib/agama/storage/volume_conversion/from_y2storage.rb index e881a0f26a..d7f1b93226 100644 --- a/service/lib/agama/storage/volume_conversion/from_y2storage.rb +++ b/service/lib/agama/storage/volume_conversion/from_y2storage.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -20,91 +20,51 @@ # find current contact information at www.suse.com. require "y2storage/storage_manager" -require "agama/storage/volume_templates_builder" module Agama module Storage module VolumeConversion - # Volume conversion from Y2Storage format. + # Volume conversion from Y2Storage. + # + # @note This class does not perform a real conversion from Y2Storage. Instead of that, it + # copies the given volume and recovers some values from Y2Storage. class FromY2Storage - # @param spec [Y2Storage::VolumeSpecification] - # @param config [Agama::Config] - def initialize(spec, config:) - @spec = spec - @config = config + # @param volume [Agama::Storage::ProposalSettings] + def initialize(volume) + @volume = volume end - # Performs the conversion from Y2Storage format. + # Performs the conversion from Y2Storage. # # @return [Agama::Storage::Volume] def convert - volume = VolumeTemplatesBuilder.new_from_config(config).for(spec.mount_point || "") - - volume.tap do |target| - target.mount_options = spec.mount_options - target.fs_type = spec.fs_type - + volume.dup.tap do |target| sizes_conversion(target) - btrfs_conversion(target) - location_conversion(target) end end private - # @return [Y2Storage::VolumeSpecification] - attr_reader :spec - - # @return [Agama::Config] - attr_reader :config + # @return [Agama::Storage::ProposalSettings] + attr_reader :volume # @param target [Agama::Storage::Volume] def sizes_conversion(target) - target.auto_size = auto_size? - - # The volume specification contains the min and max sizes for the volume. But the final - # range of sizes used by the Y2Storage proposal depends on the fallback sizes (if this - # volume is fallback for other volume) and the size for snapshots (if snapshots is - # active). The planned device contains the real range of sizes used by the proposal. + # The final range of sizes used by the Y2Storage proposal depends on the fallback sizes + # (if this volume is fallback for other volume) and the size for snapshots (if snapshots + # is active). The planned device contains the real range of sizes used by the proposal. # # From Agama point of view, this is the way of recovering the range of sizes used by # Y2Storage when a volume is set to have auto size. - planned = planned_device_for(spec.mount_point) - target.min_size = planned&.min || spec.min_size - target.max_size = planned&.max || spec.max_size - end + planned = planned_device_for(target.mount_path) + return unless planned - # @see #sizes_conversion - # - # @return [Boolean] - def auto_size? - # The three ignore_xxx attributes (ignore_snapshots_sizes, ignore_fallback_sizes and - # ignore_adjust_by_ram) are always in sync and always initialized to the inverse of - # #auto_size - !spec.ignore_fallback_sizes? - end - - # @param target [Agama::Storage::Volume] - def btrfs_conversion(target) - target.btrfs.snapshots = spec.snapshots? - target.btrfs.subvolumes = spec.subvolumes - target.btrfs.default_subvolume = spec.btrfs_default_subvolume - target.btrfs.read_only = spec.btrfs_read_only - end - - # @param target [Agama::Storage::Volume] - def location_conversion(target) - if spec.reuse? - target.location.target = spec.reformat? ? :device : :filesystem - target.location.device = spec.reuse_name - elsif !!spec.device - target.location.target = spec.separate_vg? ? :new_vg : :new_partition - target.location.device = spec.device - end + target.min_size = planned.min + target.max_size = planned.max end # Planned device for the given mount path. - + # # @param mount_path [String] # @return [Y2Storage::Planned::Device, nil] def planned_device_for(mount_path) diff --git a/service/lib/agama/storage/volume_conversion/to_y2storage.rb b/service/lib/agama/storage/volume_conversion/to_y2storage.rb index 9c00bcd1e0..dd258c3bce 100644 --- a/service/lib/agama/storage/volume_conversion/to_y2storage.rb +++ b/service/lib/agama/storage/volume_conversion/to_y2storage.rb @@ -25,14 +25,14 @@ module Agama module Storage module VolumeConversion - # Volume conversion to Y2Storage format. + # Volume conversion to Y2Storage. class ToY2Storage # @param volume [Agama::Storage::Volume] def initialize(volume) @volume = volume end - # Performs the conversion to Y2Storage format. + # Performs the conversion to Y2Storage. # # @return [Y2Storage::VolumeSpecification] def convert # rubocop:disable Metrics/AbcSize diff --git a/service/test/agama/storage/proposal_settings_conversion/from_y2storage_test.rb b/service/test/agama/storage/proposal_settings_conversion/from_y2storage_test.rb index c01e40a4e9..93f0f9e519 100644 --- a/service/test/agama/storage/proposal_settings_conversion/from_y2storage_test.rb +++ b/service/test/agama/storage/proposal_settings_conversion/from_y2storage_test.rb @@ -20,168 +20,60 @@ # find current contact information at www.suse.com. require_relative "../../../test_helper" -require "agama/storage/proposal_settings_conversion/from_y2storage" require "agama/config" +require "agama/storage/proposal_settings" +require "agama/storage/proposal_settings_conversion/from_y2storage" require "y2storage" describe Agama::Storage::ProposalSettingsConversion::FromY2Storage do - subject { described_class.new(y2storage_settings, config: config) } - - let(:config) { Agama::Config.new } + subject { described_class.new(y2storage_settings, original_settings) } + + let(:y2storage_settings) do + Y2Storage::ProposalSettings.new.tap do |settings| + settings.space_settings.actions = { + "/dev/sda" => :force_delete, + "/dev/sdb1" => :resize + } + end + end - describe "#convert" do - let(:y2storage_settings) do - Y2Storage::ProposalSettings.new.tap do |settings| - settings.root_device = "/dev/sda" - settings.lvm = false - settings.candidate_devices = ["/dev/sda"] - settings.encryption_password = "notsecret" - settings.encryption_method = Y2Storage::EncryptionMethod::LUKS2 - settings.encryption_pbkdf = Y2Storage::PbkdFunction::ARGON2ID - settings.space_settings.actions = { - "/dev/sda" => :force_delete, - "/dev/sdb1" => :resize - } - settings.volumes = [] - end + let(:original_settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device.name = "/dev/sda" + settings.boot.device = "/dev/sdb" + settings.encryption.password = "notsecret" + settings.encryption.method = Y2Storage::EncryptionMethod::LUKS2 + settings.encryption.pbkd_function = Y2Storage::PbkdFunction::ARGON2ID + settings.space.policy = :delete + settings.space.actions = [] + settings.volumes = [Agama::Storage::Volume.new("/test")] end + end - it "converts the Y2Storage settings to Agama settings" do + describe "#convert" do + it "generates settings with the same values as the given settings" do settings = subject.convert expect(settings).to be_a(Agama::Storage::ProposalSettings) - expect(settings).to have_attributes( - boot_device: "/dev/sda", - lvm: an_object_having_attributes( - enabled: false, - system_vg_devices: [] - ), - encryption: an_object_having_attributes( - password: "notsecret", - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::ARGON2ID - ), - space: an_object_having_attributes( - actions: { "/dev/sda" => :force_delete, "/dev/sdb1" => :resize } - ), - volumes: [] + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to eq("/dev/sda") + expect(settings.boot.device).to eq("/dev/sdb") + expect(settings.encryption.password).to eq("notsecret") + expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2ID) + expect(settings.space.policy).to eq(:delete) + expect(settings.volumes).to contain_exactly( + an_object_having_attributes(mount_path: "/test") ) end - context "LVM settings conversion" do - before do - y2storage_settings.root_device = "/dev/sda" - y2storage_settings.candidate_devices = candidate_devices - end - - context "when the candidate devices only includes the root device" do - let(:candidate_devices) { ["/dev/sda"] } - - it "does not set system VG devices" do - settings = subject.convert - - expect(settings).to have_attributes( - lvm: an_object_having_attributes( - system_vg_devices: [] - ) - ) - end - end - - context "when the candidate devices includes the root device and other devices" do - let(:candidate_devices) { ["/dev/sda", "/dev/sdb"] } - - it "sets the candidate devices as system VG devices" do - settings = subject.convert - - expect(settings).to have_attributes( - lvm: an_object_having_attributes( - system_vg_devices: contain_exactly("/dev/sda", "/dev/sdb") - ) - ) - end - end - - context "when the candidate devices only includes other devices" do - let(:candidate_devices) { ["/dev/sdb", "/dev/sdc"] } - - it "sets the candidate devices as system VG devices" do - settings = subject.convert - - expect(settings).to have_attributes( - lvm: an_object_having_attributes( - system_vg_devices: contain_exactly("/dev/sdb", "/dev/sdc") - ) - ) - end - - it "does not set the root device as system VG device" do - settings = subject.convert - - expect(settings.lvm.system_vg_devices).to_not include("/dev/sda") - end - end - end - - context "volumes conversion" do - before do - y2storage_settings.volumes = [spec1, spec2, spec3] - end - - let(:spec1) do - Y2Storage::VolumeSpecification.new({}).tap do |spec| - spec.mount_point = "/" - spec.proposed = true - end - end - - let(:spec2) do - Y2Storage::VolumeSpecification.new({}).tap do |spec| - spec.mount_point = "/home" - spec.proposed = true - spec.fallback_for_min_size = "/" - spec.fallback_for_max_size = "/" - end - end - - let(:spec3) do - Y2Storage::VolumeSpecification.new({}).tap do |spec| - spec.mount_point = "swap" - spec.proposed = false - spec.fallback_for_min_size = "/" - spec.fallback_for_max_size = "/home" - end - end - - it "only includes volumes for the proposed specs" do - settings = subject.convert - - expect(settings).to have_attributes( - volumes: contain_exactly( - an_object_having_attributes(mount_path: "/"), - an_object_having_attributes(mount_path: "/home") - ) - ) - end - - it "sets the fallbacks for min and max sizes" do - settings = subject.convert + it "restores the space actions from Y2Storage" do + settings = subject.convert - expect(settings).to have_attributes( - volumes: contain_exactly( - an_object_having_attributes( - mount_path: "/", - min_size_fallback_for: contain_exactly("/home", "swap"), - max_size_fallback_for: contain_exactly("/home") - ), - an_object_having_attributes( - mount_path: "/home", - min_size_fallback_for: be_empty, - max_size_fallback_for: contain_exactly("swap") - ) - ) - ) - end + expect(settings.space.actions).to eq( + "/dev/sda" => :force_delete, + "/dev/sdb1" => :resize + ) end end end diff --git a/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb b/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb index 8eae303204..e0ef192136 100644 --- a/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb +++ b/service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -21,9 +21,10 @@ require_relative "../../../test_helper" require_relative "../storage_helpers" -require "agama/storage/proposal_settings_conversion/to_y2storage" -require "agama/storage/proposal_settings" require "agama/config" +require "agama/storage/device_settings" +require "agama/storage/proposal_settings" +require "agama/storage/proposal_settings_conversion/to_y2storage" require "y2storage" describe Agama::Storage::ProposalSettingsConversion::ToY2Storage do @@ -36,9 +37,8 @@ describe "#convert" do let(:settings) do Agama::Storage::ProposalSettings.new.tap do |settings| - settings.boot_device = "/dev/sda" - settings.lvm.enabled = true - settings.lvm.system_vg_devices = ["/dev/sda", "/dev/sdb"] + settings.device.name = "/dev/sdc" + settings.boot.device = "/dev/sda" settings.encryption.password = "notsecret" settings.encryption.method = Y2Storage::EncryptionMethod::LUKS2 settings.encryption.pbkd_function = Y2Storage::PbkdFunction::ARGON2ID @@ -46,7 +46,7 @@ settings.space.actions = { "/dev/sda" => :force_delete } volume = Agama::Storage::Volume.new("/test").tap do |vol| vol.location.target = :new_partition - vol.location.device = "/dev/sdc" + vol.location.device = "/dev/sdb" end settings.volumes = [volume] end @@ -56,111 +56,157 @@ y2storage_settings = subject.convert expect(y2storage_settings).to be_a(Y2Storage::ProposalSettings) - expect(y2storage_settings).to have_attributes( - candidate_devices: contain_exactly("/dev/sda", "/dev/sdb"), - root_device: "/dev/sda", - swap_reuse: :none, - lvm: true, - separate_vgs: true, - lvm_vg_reuse: false, - encryption_password: "notsecret", - encryption_method: Y2Storage::EncryptionMethod::LUKS2, - encryption_pbkdf: Y2Storage::PbkdFunction::ARGON2ID, - space_settings: an_object_having_attributes( - strategy: :bigger_resize, - actions: { "/dev/sda" => :force_delete } - ), - volumes: include(an_object_having_attributes(mount_point: "/test")) + expect(y2storage_settings.root_device).to eq("/dev/sda") + expect(y2storage_settings.candidate_devices).to eq(["/dev/sda"]) + expect(y2storage_settings.swap_reuse).to eq(:none) + expect(y2storage_settings.lvm).to eq(false) + expect(y2storage_settings.encryption_password).to eq("notsecret") + expect(y2storage_settings.encryption_method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(y2storage_settings.encryption_pbkdf).to eq(Y2Storage::PbkdFunction::ARGON2ID) + expect(y2storage_settings.space_settings.strategy).to eq(:bigger_resize) + expect(y2storage_settings.space_settings.actions).to eq({ "/dev/sda" => :force_delete }) + expect(y2storage_settings.volumes).to include( + an_object_having_attributes( + mount_point: "/test", + device: "/dev/sdb" + ) ) end - context "candidate devices conversion" do - context "when LVM is not set" do - before do - settings.lvm.enabled = false - settings.lvm.system_vg_devices = ["/dev/sdb"] + context "device conversion" do + shared_examples "lvm conversion" do + it "configures LVM settings" do + y2storage_settings = subject.convert + + expect(y2storage_settings.lvm).to eq(true) + expect(y2storage_settings.separate_vgs).to eq(true) + expect(y2storage_settings.lvm_vg_reuse).to eq(false) + expect(y2storage_settings.lvm_vg_strategy).to eq(:use_needed) end + end - context "and there is a boot device" do - before do - settings.boot_device = "/dev/sda" + before do + settings.volumes = [ + Agama::Storage::Volume.new("/test1"), + Agama::Storage::Volume.new("/test2"), + Agama::Storage::Volume.new("/test3").tap do |volume| + volume.location.target = :new_partition + volume.location.device = "/dev/sdb" end + ] + end - it "uses the boot device as candidate device" do - y2storage_settings = subject.convert + context "when the device settings is set to use a disk" do + before do + settings.device = Agama::Storage::DeviceSettings::Disk.new("/dev/sda") + end - expect(y2storage_settings).to have_attributes( - candidate_devices: contain_exactly("/dev/sda") - ) - end + it "sets lvm to false" do + y2storage_settings = subject.convert + + expect(y2storage_settings.lvm).to eq(false) end - context "and there is no boot device" do - before do - settings.boot_device = nil - end + it "sets the boot device as candidate device" do + y2storage_settings = subject.convert - it "does not set candidate devices" do - y2storage_settings = subject.convert + expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda") + end - expect(y2storage_settings).to have_attributes( - candidate_devices: be_empty - ) - end + it "sets the target device as device for the volumes with missing device" do + y2storage_settings = subject.convert + + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/test1", device: "/dev/sda"), + an_object_having_attributes(mount_point: "/test2", device: "/dev/sda"), + an_object_having_attributes(mount_point: "/test3", device: "/dev/sdb") + ) end end - context "when LVM is set but no system VG devices are indicated" do + context "when the device settings is set to create a new LVM volume group" do before do - settings.lvm.enabled = true - settings.lvm.system_vg_devices = [] + settings.device = Agama::Storage::DeviceSettings::NewLvmVg.new(["/dev/sda", "/dev/sdb"]) end - context "and there is a boot device" do - before do - settings.boot_device = "/dev/sda" - end + include_examples "lvm conversion" - it "uses the boot device as candidate device" do - y2storage_settings = subject.convert + it "sets the candidate PV devices as candidate devices" do + y2storage_settings = subject.convert - expect(y2storage_settings).to have_attributes( - candidate_devices: contain_exactly("/dev/sda") - ) - end + expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda", "/dev/sdb") end - context "and there is no boot device" do - before do - settings.boot_device = nil - end - - it "does not set candidate device" do - y2storage_settings = subject.convert + it "does not set the device for the volumes with missing device" do + y2storage_settings = subject.convert - expect(y2storage_settings).to have_attributes( - candidate_devices: be_empty - ) - end + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/test1", device: nil), + an_object_having_attributes(mount_point: "/test2", device: nil), + an_object_having_attributes(mount_point: "/test3", device: "/dev/sdb") + ) end end - context "when LVM is set and some system VG devices are indicated" do + context "when the device settings is set to reuse a LVM volume group" do before do - settings.lvm.enabled = true - settings.lvm.system_vg_devices = ["/dev/sdb", "/dev/sdc"] + settings.device = Agama::Storage::DeviceSettings::ReusedLvmVg.new("/dev/vg0") + end + + include_examples "lvm conversion" + + xit "sets the reused LVM volume group as target device" do + # TODO: Feature not supported yet by yast2-storage-ng. end - it "uses the system VG devices as candidate devices" do + it "does not set the device for the volumes with missing device" do y2storage_settings = subject.convert - expect(y2storage_settings).to have_attributes( - candidate_devices: contain_exactly("/dev/sdb", "/dev/sdc") + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/test1", device: nil), + an_object_having_attributes(mount_point: "/test2", device: nil), + an_object_having_attributes(mount_point: "/test3", device: "/dev/sdb") ) end end end + context "boot conversion" do + before do + settings.boot.device = "/dev/sda" + end + + it "sets the boot device as root device" do + y2storage_settings = subject.convert + + expect(y2storage_settings.root_device).to eq("/dev/sda") + end + + context "if boot configuration is enabled" do + before do + settings.boot.configure = true + end + + it "sets boot to true" do + y2storage_settings = subject.convert + + expect(y2storage_settings.boot).to eq(true) + end + end + + context "if boot configuration is disabled" do + before do + settings.boot.configure = false + end + + it "sets boot to false" do + y2storage_settings = subject.convert + + expect(y2storage_settings.boot).to eq(false) + end + end + end + context "space policy conversion" do before do mock_storage(devicegraph: "staging-plain-partitions.yaml") diff --git a/service/test/agama/storage/proposal_settings_conversion_test.rb b/service/test/agama/storage/proposal_settings_conversion_test.rb index 63f9cde502..085e35f37b 100644 --- a/service/test/agama/storage/proposal_settings_conversion_test.rb +++ b/service/test/agama/storage/proposal_settings_conversion_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -20,19 +20,19 @@ # find current contact information at www.suse.com. require_relative "../../test_helper" -require "agama/storage/proposal_settings_conversion" -require "agama/storage/proposal_settings" require "agama/config" +require "agama/storage/proposal_settings" +require "agama/storage/proposal_settings_conversion" require "y2storage" describe Agama::Storage::ProposalSettingsConversion do describe "#from_y2storage" do - let(:config) { Agama::Config.new } - let(:y2storage_settings) { Y2Storage::ProposalSettings.new } + let(:settings) { Agama::Storage::ProposalSettings.new } + it "generates proposal settings from Y2Storage settings" do - result = described_class.from_y2storage(y2storage_settings, config: config) + result = described_class.from_y2storage(y2storage_settings, settings) expect(result).to be_a(Agama::Storage::ProposalSettings) end end diff --git a/service/test/agama/storage/volume_conversion/from_y2storage_test.rb b/service/test/agama/storage/volume_conversion/from_y2storage_test.rb index 22108805de..55da4f8edb 100644 --- a/service/test/agama/storage/volume_conversion/from_y2storage_test.rb +++ b/service/test/agama/storage/volume_conversion/from_y2storage_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -22,8 +22,8 @@ require_relative "../../../test_helper" require_relative "../storage_helpers" require_relative "../../rspec/matchers/storage" +require "agama/storage/volume" require "agama/storage/volume_conversion/from_y2storage" -require "agama/config" require "y2storage" describe Agama::Storage::VolumeConversion::FromY2Storage do @@ -31,86 +31,53 @@ before { mock_storage } - subject { described_class.new(spec, config: config) } - - let(:config) { Agama::Config.new } - - describe "#convert" do - let(:spec) do - Y2Storage::VolumeSpecification.new({}).tap do |spec| - spec.mount_point = "/" - spec.device = "/dev/sda" - spec.separate_vg_name = "/dev/vg0" - spec.mount_options = ["defaults", "ro"] - spec.fs_type = Y2Storage::Filesystems::Type::BTRFS - spec.min_size = Y2Storage::DiskSize.GiB(5) - spec.max_size = Y2Storage::DiskSize.GiB(20) - spec.snapshots = true - spec.subvolumes = ["@/home", "@/var"] - spec.btrfs_default_subvolume = "@" - spec.btrfs_read_only = true - end - end - - it "converts the Y2Storage volume spec to an Agama volume" do - volume = subject.convert - - expect(volume).to be_a(Agama::Storage::Volume) - expect(volume).to have_attributes( - mount_path: "/", - location: an_object_having_attributes( - device: "/dev/sda", - target: :new_vg - ), - mount_options: contain_exactly("defaults", "ro"), - fs_type: Y2Storage::Filesystems::Type::BTRFS, - min_size: Y2Storage::DiskSize.GiB(5), - max_size: Y2Storage::DiskSize.GiB(20), - btrfs: an_object_having_attributes( - snapshots?: true, - subvolumes: contain_exactly("@/home", "@/var"), - default_subvolume: "@", - read_only?: true - ) - ) - - outline = Agama::Storage::VolumeTemplatesBuilder.new_from_config(config).for("/").outline - expect(volume.outline).to eq_outline(outline) + subject { described_class.new(volume) } + + let(:btrfs) { Y2Storage::Filesystems::Type::BTRFS } + let(:ext4) { Y2Storage::Filesystems::Type::EXT4 } + let(:xfs) { Y2Storage::Filesystems::Type::XFS } + + let(:volume) do + Agama::Storage::Volume.new("/").tap do |volume| + volume.location.target = :new_vg + volume.location.device = "/dev/sda" + volume.mount_options = ["defaults"] + volume.fs_type = btrfs + volume.auto_size = false + volume.min_size = Y2Storage::DiskSize.GiB(5) + volume.max_size = Y2Storage::DiskSize.GiB(20) + volume.btrfs.snapshots = true + volume.btrfs.subvolumes = ["@/home", "@/var"] + volume.btrfs.default_subvolume = "@" + volume.btrfs.read_only = true + volume.outline.required = true + volume.outline.filesystems = [btrfs, ext4, xfs] + volume.outline.adjust_by_ram = false + volume.outline.snapshots_configurable = true + volume.outline.snapshots_size = Y2Storage::DiskSize.GiB(10) + volume.outline.snapshots_percentage = 20 end + end - context "auto size conversion" do - before do - spec.ignore_fallback_sizes = ignore_fallback_sizes - spec.ignore_snapshots_sizes = ignore_snapshots_sizes - end - - context "if :ignore_fallback_sizes and :ignore_snapshots_sizes are set" do - let(:ignore_fallback_sizes) { true } - - let(:ignore_snapshots_sizes) { true } - - it "does not set auto size" do - volume = subject.convert - - expect(volume).to have_attributes( - auto_size?: false - ) - end - end - - context "if :ignore_fallback_sizes and :ignore_snapshots_sizes are not set" do - let(:ignore_fallback_sizes) { false } - - let(:ignore_snapshots_sizes) { false } - - it "sets auto size" do - volume = subject.convert - - expect(volume).to have_attributes( - auto_size?: true - ) - end - end + describe "#convert" do + it "generates a volume with the same values as the given volume" do + result = subject.convert + + expect(result).to be_a(Agama::Storage::Volume) + expect(result).to_not equal(volume) + expect(result.location.target).to eq(:new_vg) + expect(result.location.device).to eq("/dev/sda") + expect(result.mount_path).to eq("/") + expect(result.mount_options).to contain_exactly("defaults") + expect(result.fs_type).to eq(btrfs) + expect(result.auto_size).to eq(false) + expect(result.min_size).to eq(Y2Storage::DiskSize.GiB(5)) + expect(result.max_size).to eq(Y2Storage::DiskSize.GiB(20)) + expect(result.btrfs.snapshots).to eq(true) + expect(result.btrfs.subvolumes).to contain_exactly("@/home", "@/var") + expect(result.btrfs.default_subvolume).to eq("@") + expect(result.btrfs.read_only).to eq(true) + expect(result.outline).to eq_outline(volume.outline) end context "sizes conversion" do @@ -133,12 +100,10 @@ end it "sets the min and max sizes according to the planned device" do - volume = subject.convert + result = subject.convert - expect(volume).to have_attributes( - min_size: Y2Storage::DiskSize.GiB(10), - max_size: Y2Storage::DiskSize.GiB(40) - ) + expect(result.min_size).to eq(Y2Storage::DiskSize.GiB(10)) + expect(result.max_size).to eq(Y2Storage::DiskSize.GiB(40)) end end @@ -152,13 +117,11 @@ end end - it "sets the min and max sizes according to the volume spec" do - volume = subject.convert + it "sets the sizes of the given volume" do + result = subject.convert - expect(volume).to have_attributes( - min_size: Y2Storage::DiskSize.GiB(5), - max_size: Y2Storage::DiskSize.GiB(20) - ) + expect(result.min_size).to eq(Y2Storage::DiskSize.GiB(5)) + expect(result.max_size).to eq(Y2Storage::DiskSize.GiB(20)) end end end diff --git a/service/test/agama/storage/volume_conversion/to_y2storage_test.rb b/service/test/agama/storage/volume_conversion/to_y2storage_test.rb index 792ee31eec..8b52a1372f 100644 --- a/service/test/agama/storage/volume_conversion/to_y2storage_test.rb +++ b/service/test/agama/storage/volume_conversion/to_y2storage_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -57,31 +57,29 @@ spec = subject.convert expect(spec).to be_a(Y2Storage::VolumeSpecification) - expect(spec).to have_attributes( - device: "/dev/sda", - separate_vg_name: "vg-root", - mount_point: "/", - mount_options: "defaults", - proposed?: true, - proposed_configurable?: false, - fs_types: contain_exactly(btrfs, ext4, xfs), - fs_type: btrfs, - weight: 100, - adjust_by_ram?: false, - ignore_fallback_sizes: true, - ignore_snapshots_sizes: true, - ignore_adjust_by_ram: true, - min_size: Y2Storage::DiskSize.GiB(5), - max_size: Y2Storage::DiskSize.GiB(20), - max_size_lvm: Y2Storage::DiskSize.GiB(20), - snapshots: true, - snapshots_configurable?: true, - snapshots_size: Y2Storage::DiskSize.GiB(10), - snapshots_percentage: 20, - subvolumes: ["@/home", "@/var"], - btrfs_default_subvolume: "@", - btrfs_read_only: true - ) + expect(spec.device).to eq("/dev/sda") + expect(spec.separate_vg_name).to eq("vg-root") + expect(spec.mount_point).to eq("/") + expect(spec.mount_options).to eq("defaults") + expect(spec.proposed).to eq(true) + expect(spec.proposed_configurable).to eq(false) + expect(spec.fs_types).to contain_exactly(btrfs, ext4, xfs) + expect(spec.fs_type).to eq(btrfs) + expect(spec.weight).to eq(100) + expect(spec.adjust_by_ram).to eq(false) + expect(spec.ignore_fallback_sizes).to eq(true) + expect(spec.ignore_snapshots_sizes).to eq(true) + expect(spec.ignore_adjust_by_ram).to eq(true) + expect(spec.min_size).to eq(Y2Storage::DiskSize.GiB(5)) + expect(spec.max_size).to eq(Y2Storage::DiskSize.GiB(20)) + expect(spec.max_size_lvm).to eq(Y2Storage::DiskSize.GiB(20)) + expect(spec.snapshots).to eq(true) + expect(spec.snapshots_configurable).to eq(true) + expect(spec.snapshots_size).to eq(Y2Storage::DiskSize.GiB(10)) + expect(spec.snapshots_percentage).to eq(20) + expect(spec.subvolumes).to contain_exactly("@/home", "@/var") + expect(spec.btrfs_default_subvolume).to eq("@") + expect(spec.btrfs_read_only).to eq(true) end context "when auto size is used" do @@ -94,14 +92,12 @@ it "sets the min and max spec sizes according to the volume outline" do spec = subject.convert - expect(spec).to have_attributes( - ignore_fallback_sizes: false, - ignore_snapshots_sizes: false, - ignore_adjust_by_ram: false, - min_size: Y2Storage::DiskSize.GiB(10), - max_size: Y2Storage::DiskSize.GiB(50), - max_size_lvm: Y2Storage::DiskSize.GiB(50) - ) + expect(spec.ignore_fallback_sizes).to eq(false) + expect(spec.ignore_snapshots_sizes).to eq(false) + expect(spec.ignore_adjust_by_ram).to eq(false) + expect(spec.min_size).to eq(Y2Storage::DiskSize.GiB(10)) + expect(spec.max_size).to eq(Y2Storage::DiskSize.GiB(50)) + expect(spec.max_size_lvm).to eq(Y2Storage::DiskSize.GiB(50)) end end diff --git a/service/test/agama/storage/volume_conversion_test.rb b/service/test/agama/storage/volume_conversion_test.rb index e9aec67a44..141831b385 100644 --- a/service/test/agama/storage/volume_conversion_test.rb +++ b/service/test/agama/storage/volume_conversion_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -20,19 +20,18 @@ # find current contact information at www.suse.com. require_relative "../../test_helper" -require "agama/storage/volume_conversion" -require "agama/storage/volume" require "agama/config" +require "agama/storage/proposal_settings" +require "agama/storage/volume" +require "agama/storage/volume_conversion" require "y2storage" describe Agama::Storage::VolumeConversion do describe "#from_y2storage" do - let(:config) { Agama::Config.new } - - let(:spec) { Y2Storage::VolumeSpecification.new({}) } + let(:volume) { Agama::Storage::Volume.new("/test") } - it "generates a volume from a Y2Storage volume spec" do - result = described_class.from_y2storage(spec, config: config) + it "generates a volume" do + result = described_class.from_y2storage(volume) expect(result).to be_a(Agama::Storage::Volume) end end @@ -40,7 +39,7 @@ describe "#to_y2storage" do let(:volume) { Agama::Storage::Volume.new("/test") } - it "generates a Y2Storage volume spec from a volume" do + it "generates a Y2Storage volume spec" do result = described_class.to_y2storage(volume) expect(result).to be_a(Y2Storage::VolumeSpecification) end From cc61008cf8d4d1b798949aee5e048d846e1d4b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:21:21 +0000 Subject: [PATCH 03/29] service: Adapt settings reader --- .../agama/storage/proposal_settings_reader.rb | 11 +- .../storage/proposal_settings_reader_test.rb | 216 ++++++++---------- 2 files changed, 108 insertions(+), 119 deletions(-) diff --git a/service/lib/agama/storage/proposal_settings_reader.rb b/service/lib/agama/storage/proposal_settings_reader.rb index fff9245e9e..e0c038fd6f 100644 --- a/service/lib/agama/storage/proposal_settings_reader.rb +++ b/service/lib/agama/storage/proposal_settings_reader.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -19,10 +19,11 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. +require "agama/storage/device_settings" +require "agama/storage/space_settings" +require "agama/storage/volume_templates_builder" require "y2storage/encryption_method" require "y2storage/pbkd_function" -require "agama/storage/volume_templates_builder" -require "agama/storage/space_settings" module Agama module Storage @@ -63,7 +64,9 @@ def read # @param settings [Agama::Storage::ProposalSettings] # @param value [Boolean] def lvm_reader(settings, value) - settings.lvm.enabled = value + return unless value + + settings.device = DeviceSettings::NewLvmVg.new end # @param settings [Agama::Storage::ProposalSettings] diff --git a/service/test/agama/storage/proposal_settings_reader_test.rb b/service/test/agama/storage/proposal_settings_reader_test.rb index b5bed59cf0..ed7b77b199 100644 --- a/service/test/agama/storage/proposal_settings_reader_test.rb +++ b/service/test/agama/storage/proposal_settings_reader_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2022-2023] SUSE LLC +# Copyright (c) [2022-2024] SUSE LLC # # All Rights Reserved. # @@ -20,9 +20,10 @@ # find current contact information at www.suse.com. require_relative "../../test_helper" -require "agama/storage/proposal_settings_reader" -require "agama/storage/proposal_settings" require "agama/config" +require "agama/storage/device_settings" +require "agama/storage/proposal_settings" +require "agama/storage/proposal_settings_reader" require "y2storage" describe Agama::Storage::ProposalSettingsReader do @@ -31,147 +32,132 @@ subject { described_class.new(config) } describe "#read" do - let(:config_data) do - { - "storage" => { - "lvm" => true, - "space_policy" => "delete", - "encryption" => { - "method" => "luks2", - "pbkd_function" => "argon2id" - }, - "volumes" => ["/", "swap"], - "volume_templates" => [ - { - "mount_path" => "/", - "outline" => { "required" => true } - }, - { - "mount_path" => "/home", - "outline" => { "required" => false } - }, - { - "mount_path" => "swap", - "outline" => { "required" => false } - } - ] - } - } - end - - it "generates proposal settings from the config" do - settings = subject.read - - expect(settings).to be_a(Agama::Storage::ProposalSettings) - expect(settings).to have_attributes( - boot_device: nil, - lvm: an_object_having_attributes( - enabled?: true, - system_vg_devices: be_empty - ), - encryption: an_object_having_attributes( - password: nil, - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::ARGON2ID - ), - space: an_object_having_attributes( - policy: :delete, - actions: {} - ), - volumes: contain_exactly( - an_object_having_attributes(mount_path: "/"), - an_object_having_attributes(mount_path: "swap") - ) - ) - end - context "when the config does not contain storage section" do let(:config_data) { {} } it "generates proposal settings with default values" do settings = subject.read - expect(settings).to have_attributes( - boot_device: nil, - lvm: an_object_having_attributes( - enabled?: false, - system_vg_devices: be_empty - ), - encryption: an_object_having_attributes( - password: nil, - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::PBKDF2 - ), - space: an_object_having_attributes( - policy: :keep, - actions: {} - ), - volumes: be_empty - ) + expect(settings).to be_a(Agama::Storage::ProposalSettings) + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to be_nil + expect(settings.boot.device).to be_nil + expect(settings.encryption.password).to be_nil + expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::PBKDF2) + expect(settings.space.policy).to eq(:keep) + expect(settings.space.actions).to eq({}) + expect(settings.volumes).to be_empty end end - context "when the config contains an unknown encryption method" do + context "when the config contains a storage section" do let(:config_data) do { "storage" => { - "encyption" => { - "method" => "fooenc" - } + "lvm" => false, + "space_policy" => "delete", + "encryption" => { + "method" => "luks2", + "pbkd_function" => "argon2id" + }, + "volumes" => ["/", "swap"], + "volume_templates" => [ + { + "mount_path" => "/", + "outline" => { "required" => true } + }, + { + "mount_path" => "/home", + "outline" => { "required" => false } + }, + { + "mount_path" => "swap", + "outline" => { "required" => false } + } + ] } } end - it "uses the default encryption method" do + it "generates proposal settings from the config" do settings = subject.read - expect(settings).to have_attributes( - encryption: an_object_having_attributes( - method: Y2Storage::EncryptionMethod::LUKS2 - ) + expect(settings).to be_a(Agama::Storage::ProposalSettings) + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to be_nil + expect(settings.boot.device).to be_nil + expect(settings.encryption.password).to be_nil + expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2ID) + expect(settings.space.policy).to eq(:delete) + expect(settings.space.actions).to eq({}) + expect(settings.volumes).to contain_exactly( + an_object_having_attributes(mount_path: "/"), + an_object_having_attributes(mount_path: "swap") ) end - end - context "when the config contains an unknown password derivation function" do - let(:config_data) do - { - "storage" => { - "encyption" => { - "pbkd_function" => "foo" - } - } - } + context "if lvm is disabled" do + before do + config_data["storage"]["lvm"] = false + end + + it "generates device settings to use a disk as target device" do + settings = subject.read + + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to be_nil + end end - it "sets the default derivation function" do - settings = subject.read + context "if lvm is enabled" do + before do + config_data["storage"]["lvm"] = true + end - expect(settings).to have_attributes( - encryption: an_object_having_attributes( - pbkd_function: Y2Storage::PbkdFunction::PBKDF2 - ) - ) + it "generates device settings to use a new LVM volume group as target device" do + settings = subject.read + + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::NewLvmVg) + expect(settings.device.candidate_pv_devices).to be_empty + end end - end - context "when the config contains an unknown space policy" do - let(:config_data) do - { - "storage" => { - "space_policy" => "foo" - } - } + context "if the encryption method is unknown" do + before do + config_data["storage"]["encryption"]["method"] = "fooenc" + end + + it "uses the default encryption method" do + settings = subject.read + + expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + end end - it "uses the default space policy" do - settings = subject.read + context "if the password derivation function is unknown" do + before do + config_data["storage"]["encryption"]["pbkd_function"] = "foo" + end - expect(settings).to have_attributes( - space: an_object_having_attributes( - policy: :keep - ) - ) + it "uses the default derivation function" do + settings = subject.read + + expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::PBKDF2) + end + end + + context "if the space policy is unknown" do + before do + config_data["storage"]["space_policy"] = "foo" + end + + it "uses the default space policy" do + settings = subject.read + + expect(settings.space.policy).to eq(:keep) + end end end end From 48c77aa4930ba69f8de2fd51af8a43aaa1425353 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:22:49 +0000 Subject: [PATCH 04/29] service: Adapt proposal to use the new settings --- service/lib/agama/storage/proposal.rb | 105 +++++++------ service/test/agama/storage/proposal_test.rb | 139 +++++++----------- .../agama/storage/proposal_volumes_test.rb | 32 +++- 3 files changed, 140 insertions(+), 136 deletions(-) diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index 0b3d546dc3..ee71c68637 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -19,18 +19,32 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "y2storage" require "agama/issue" require "agama/storage/actions" +require "agama/storage/device_settings" require "agama/storage/proposal_settings_conversion" +require "yast" +require "y2storage" module Agama module Storage # Backend class to calculate a storage proposal. class Proposal + include Yast::I18n + + # Settings used for calculating the proposal. + # + # @note Some values are recoverd from Y2Storage, see + # {ProposalSettingsConversion::FromY2Storage} + # + # @return [ProposalSettings, nil] nil if no proposal has been calculated yet. + attr_reader :settings + # @param config [Config] Agama config # @param logger [Logger] def initialize(config, logger: nil) + textdomain "agama" + @config = config @logger = logger || Logger.new($stdout) @on_calculate_callbacks = [] @@ -60,40 +74,16 @@ def available_devices # @param settings [Agamal::Storage::ProposalSettings] settings to calculate the proposal. # @return [Boolean] whether the proposal was correctly calculated. def calculate(settings) - # Use the first available device if no boot device is indicated. - settings.boot_device ||= available_devices.first&.name - - @original_settings = settings + select_target_device(settings) if missing_target_device?(settings) calculate_proposal(settings) + @settings = ProposalSettingsConversion.from_y2storage(proposal.settings, settings) @on_calculate_callbacks.each(&:call) success? end - # Settings used for calculating the proposal. - # - # Note that this settings might differ from the {#original_settings}. For example, the sizes - # of some volumes could be adjusted if auto size is set. - # - # @return [ProposalSettings, nil] nil if no proposal has been calculated yet. - def settings - return nil unless calculated? - - ProposalSettingsConversion.from_y2storage( - proposal.settings, - config: config - ).tap do |settings| - # The conversion from Y2Storage cannot infer the space policy. Copying space policy from - # the original settings. - settings.space.policy = original_settings.space.policy - # FIXME: The conversion from Y2Storage cannot reliably infer the system VG devices in all - # cases. Copying system VG devices from the original settings. - settings.lvm.system_vg_devices = original_settings.lvm.system_vg_devices - end - end - # Storage actions. # # @return [Array] @@ -110,7 +100,7 @@ def issues return [] if !calculated? || success? [ - boot_device_issue, + target_device_issue, missing_devices_issue, proposal_issue ].compact @@ -124,11 +114,6 @@ def issues # @return [Logger] attr_reader :logger - # Settings originally used for calculating the proposal (without conversion from Y2Storage). - # - # @return [Agama::Storage::ProposalSettings] - attr_reader :original_settings - # @return [Y2Storage::MinGuidedProposal, nil] def proposal storage_manager.proposal @@ -141,6 +126,36 @@ def calculated? !proposal.nil? end + # Selects the first available device as target device for installation. + # + # @param settings [ProposalSettings] + def select_target_device(settings) + device = available_devices.first&.name + return unless device + + case settings.device + when DeviceSettings::Disk + settings.device.name = device + when DeviceSettings::NewLvmVg + settings.device.candidate_pv_devices = [device] + when DeviceSettings::ReusedLvmVg + # TODO: select an existing VG? + end + end + + # Whether the given settings has no target device for the installation. + # + # @param settings [ProposalSettings] + # @return [Boolean] + def missing_target_device?(settings) + case settings.device + when DeviceSettings::Disk, DeviceSettings::ReusedLvmVg + settings.device.name.nil? + when DeviceSettings::NewLvmVg + settings.device.candidate_pv_devices.empty? + end + end + # Instantiates and executes a Y2Storage proposal with the given settings # # @param settings [Y2Storage::ProposalSettings] @@ -171,13 +186,13 @@ def storage_manager Y2Storage::StorageManager.instance end - # Returns an issue if there is no boot device. + # Returns an issue if there is no target device. # # @return [Issue, nil] - def boot_device_issue - return if settings.boot_device + def target_device_issue + return unless missing_target_device?(settings) - Issue.new("No device selected for installation", + Issue.new(_("No device selected for installation"), source: Issue::Source::CONFIG, severity: Issue::Severity::ERROR) end @@ -186,13 +201,17 @@ def boot_device_issue # # @return [Issue, nil] def missing_devices_issue - # At this moment, only the boot device is checked. - return unless settings.boot_device - return if available_devices.map(&:name).include?(settings.boot_device) + available = available_devices.map(&:name) + missing = settings.installation_devices.reject { |d| available.include?(d) } - Issue.new("Selected device is not found in the system", + return if missing.none? + + Issue.new( + format(_("The following selected devices are not found in the system: %{devices}"), + devices: missing.join(", ")), source: Issue::Source::CONFIG, - severity: Issue::Severity::ERROR) + severity: Issue::Severity::ERROR + ) end # Returns an issue if the proposal is not valid. @@ -201,7 +220,7 @@ def missing_devices_issue def proposal_issue return if success? - Issue.new("Cannot accommodate the required file systems for installation", + Issue.new(_("Cannot accommodate the required file systems for installation"), source: Issue::Source::CONFIG, severity: Issue::Severity::ERROR) end diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb index c56237b394..a0e15d78a7 100644 --- a/service/test/agama/storage/proposal_test.rb +++ b/service/test/agama/storage/proposal_test.rb @@ -21,9 +21,10 @@ require_relative "../../test_helper" require_relative "storage_helpers" +require "agama/config" +require "agama/storage/device_settings" require "agama/storage/proposal" require "agama/storage/proposal_settings" -require "agama/config" require "y2storage" describe Agama::Storage::Proposal do @@ -39,14 +40,15 @@ let(:achievable_settings) do Agama::Storage::ProposalSettings.new.tap do |settings| - settings.boot_device = "/dev/sdb" + settings.device.name = "/dev/sdb" + settings.boot.device = "/dev/sda" settings.volumes = [Agama::Storage::Volume.new("/")] end end let(:impossible_settings) do Agama::Storage::ProposalSettings.new.tap do |settings| - settings.boot_device = "/dev/sdb" + settings.device.name = "/dev/sdb" settings.volumes = [ # The boot disk size is 500 GiB, so it cannot accomodate a 1 TiB volume. Agama::Storage::Volume.new("/").tap { |v| v.min_size = Y2Storage::DiskSize.TiB(1) } @@ -89,11 +91,10 @@ subject.calculate(achievable_settings) expect(Y2Storage::StorageManager.instance.proposal).to_not be_nil - expect(Y2Storage::StorageManager.instance.proposal.settings).to have_attributes( - root_device: "/dev/sdb", - volumes: contain_exactly( - an_object_having_attributes(mount_point: "/") - ) + y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings + expect(y2storage_settings.root_device).to eq("/dev/sda") + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/", device: "/dev/sdb") ) end @@ -115,17 +116,43 @@ expect(subject.calculate(impossible_settings)).to eq(false) end - context "if the given settings does not indicate a boot device" do - let(:settings) do - achievable_settings.tap { |s| s.boot_device = nil } + context "if the given device settings sets a disk as target" do + before do + achievable_settings.device = Agama::Storage::DeviceSettings::Disk.new end - it "calculates a new proposal using the first disk as boot device" do - subject.calculate(settings) + context "and the target disk is not indicated" do + before do + achievable_settings.device.name = nil + end - expect(Y2Storage::StorageManager.instance.proposal.settings).to have_attributes( - root_device: "/dev/sda" - ) + it "sets the first available device as target device for volumes" do + subject.calculate(achievable_settings) + y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings + + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/", device: "/dev/sda") + ) + end + end + end + + context "if the given device settings sets a new LVM volume group as target" do + before do + achievable_settings.device = Agama::Storage::DeviceSettings::NewLvmVg.new + end + + context "and the target disks for physical volumes are not indicated" do + before do + achievable_settings.device.candidate_pv_devices = [] + end + + it "sets the first available device as candidate device" do + subject.calculate(achievable_settings) + y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings + + expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda") + end end end end @@ -137,79 +164,19 @@ context "if the proposal was already calculated" do before do - subject.calculate(settings) - end - - let(:settings) do - achievable_settings.tap do |settings| - settings.space.policy = :custom - settings.space.actions = { "/dev/sda" => :force_delete } - end + subject.calculate(achievable_settings) end it "returns the settings used for calculating the proposal" do expect(subject.settings).to be_a(Agama::Storage::ProposalSettings) expect(subject.settings).to have_attributes( - boot_device: "/dev/sdb", - volumes: contain_exactly( + device: an_object_having_attributes(name: "/dev/sdb"), + volumes: contain_exactly( an_object_having_attributes(mount_path: "/") - ), - # Checking space policy explicitly here because the settings converter cannot infer the - # space policy from the Y2Storage settings. The space policy is directly recovered from - # the original settings passed to #calculate. - space: an_object_having_attributes(policy: :custom) + ) ) end - - # Checking system VG devices explicitly here because the settings converter cannot infer the - # system VG devices from the Y2Storage settings in all cases. The system VG devices are - # directly recovered from the original settings passed to #calculate. - context "system VG devices" do - let(:settings) do - achievable_settings.tap do |settings| - settings.lvm.system_vg_devices = system_vg_devices - end - end - - context "if no devices were assigned as system VG devices" do - let(:system_vg_devices) { [] } - - it "returns settings containing no system VG devices " do - expect(subject.settings).to have_attributes( - lvm: an_object_having_attributes( - system_vg_devices: be_empty - ) - ) - end - end - - context "if only boot device was assigned as system VG device" do - let(:system_vg_devices) { ["/dev/sdb"] } - - # This case cannot be inferred by conversion from Y2Storage, so the test does not pass if - # system VG devices are not copied from the settings passed to #calculate. - it "returns settings containing only boot device as system VG device" do - expect(subject.settings).to have_attributes( - lvm: an_object_having_attributes( - system_vg_devices: contain_exactly("/dev/sdb") - ) - ) - end - end - - context "if several devices were assigned as system VG devices" do - let(:system_vg_devices) { ["/dev/sdb", "/dev/sdc"] } - - it "returns settings containing the system VG devices" do - expect(subject.settings).to have_attributes( - lvm: an_object_having_attributes( - system_vg_devices: contain_exactly("/dev/sdb", "/dev/sdc") - ) - ) - end - end - end end end @@ -263,13 +230,13 @@ ) end - context "and the settings does not indicate a boot device" do + context "and the settings does not indicate a target device" do before do # Avoid to automatically set the first device allow(subject).to receive(:available_devices).and_return([]) end - let(:settings) { impossible_settings.tap { |s| s.boot_device = nil } } + let(:settings) { impossible_settings.tap { |s| s.device.name = nil } } it "includes an error because a device is not selected" do subject.calculate(settings) @@ -279,19 +246,19 @@ ) expect(subject.issues).to_not include( - an_object_having_attributes(description: /device is not found/) + an_object_having_attributes(description: /are not found/) ) end end - context "and the boot device is missing in the system" do - let(:settings) { impossible_settings.tap { |s| s.boot_device = "/dev/vdz" } } + context "and some installation device is missing in the system" do + let(:settings) { impossible_settings.tap { |s| s.device.name = "/dev/vdz" } } it "includes an error because the device is not found" do subject.calculate(settings) expect(subject.issues).to include( - an_object_having_attributes(description: /device is not found/) + an_object_having_attributes(description: /are not found/) ) end end diff --git a/service/test/agama/storage/proposal_volumes_test.rb b/service/test/agama/storage/proposal_volumes_test.rb index e571321799..d6f3867f6a 100644 --- a/service/test/agama/storage/proposal_volumes_test.rb +++ b/service/test/agama/storage/proposal_volumes_test.rb @@ -83,11 +83,14 @@ end let(:y2storage_proposal) do - instance_double(Y2Storage::MinGuidedProposal, propose: true, failed?: false) + instance_double(Y2Storage::MinGuidedProposal, + propose: true, failed?: false, settings: y2storage_settings, planned_devices: []) end let(:vol_builder) { Agama::Storage::VolumeTemplatesBuilder.new_from_config(config) } + let(:y2storage_settings) { Y2Storage::ProposalSettings.new } + # Constructs a Agama volume with the given set of attributes # # @param attrs [Hash] set of attributes and their values (sizes can be provided as strings) @@ -113,7 +116,7 @@ def test_vol(path, attrs = {}) # 'y2storage_proposal' # # @param specs [Hash] arguments to check on each VolumeSpecification object - def expect_proposal_with_expects(*specs) + def expect_proposal_with_specs(*specs) expect(Y2Storage::MinGuidedProposal).to receive(:new) do |**args| expect(args[:settings]).to be_a(Y2Storage::ProposalSettings) expect(args[:settings].volumes).to all(be_a(Y2Storage::VolumeSpecification)) @@ -130,11 +133,14 @@ def expect_proposal_with_expects(*specs) describe "#calculate" do before do + allow(Y2Storage::StorageManager.instance) + .to receive(:proposal).and_return(y2storage_proposal) + allow(Y2Storage::StorageManager.instance).to receive(:proposal=) end it "runs the Y2Storage proposal with the correct set of VolumeSpecification" do - expect_proposal_with_expects( + expect_proposal_with_specs( { mount_point: "/", proposed: true, snapshots: false, ignore_fallback_sizes: false, ignore_snapshots_sizes: false, @@ -168,11 +174,14 @@ def expect_proposal_with_expects(*specs) describe "#calculate" do before do + allow(Y2Storage::StorageManager.instance) + .to receive(:proposal).and_return(y2storage_proposal) + allow(Y2Storage::StorageManager.instance).to receive(:proposal=) end it "runs the Y2Storage proposal with the correct set of VolumeSpecification" do - expect_proposal_with_expects( + expect_proposal_with_specs( { mount_point: "/", proposed: true, snapshots: true, ignore_fallback_sizes: false, ignore_snapshots_sizes: false, @@ -207,11 +216,14 @@ def expect_proposal_with_expects(*specs) describe "#calculate" do before do + allow(Y2Storage::StorageManager.instance) + .to receive(:proposal).and_return(y2storage_proposal) + allow(Y2Storage::StorageManager.instance).to receive(:proposal=) end it "runs the Y2Storage proposal with the correct set of VolumeSpecification" do - expect_proposal_with_expects( + expect_proposal_with_specs( { mount_point: "/", proposed: true, snapshots: true, ignore_fallback_sizes: false, ignore_snapshots_sizes: false, @@ -246,11 +258,14 @@ def expect_proposal_with_expects(*specs) describe "#calculate" do before do + allow(Y2Storage::StorageManager.instance) + .to receive(:proposal).and_return(y2storage_proposal) + allow(Y2Storage::StorageManager.instance).to receive(:proposal=) end it "runs the Y2Storage proposal with the correct set of VolumeSpecification" do - expect_proposal_with_expects( + expect_proposal_with_specs( { mount_point: "/", proposed: true }, { mount_point: "/two", proposed: false }, { @@ -288,11 +303,14 @@ def expect_proposal_with_expects(*specs) describe "#calculate" do before do + allow(Y2Storage::StorageManager.instance) + .to receive(:proposal).and_return(y2storage_proposal) + allow(Y2Storage::StorageManager.instance).to receive(:proposal=) end it "runs the Y2Storage proposal with the correct set of VolumeSpecification" do - expect_proposal_with_expects( + expect_proposal_with_specs( { mount_point: "/", proposed: true, snapshots: false, ignore_fallback_sizes: true, ignore_snapshots_sizes: true, From 9e5f396d0e87aaeb9c5f80710ca3944427da03f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:24:55 +0000 Subject: [PATCH 05/29] service: Add classes to validate D-Bus values --- service/lib/agama/dbus/hash_validator.rb | 154 ++++++++++++++ service/lib/agama/dbus/types.rb | 159 ++++++++++++++ .../test/agama/dbus/hash_validator_test.rb | 197 ++++++++++++++++++ service/test/agama/dbus/types_test.rb | 167 +++++++++++++++ 4 files changed, 677 insertions(+) create mode 100644 service/lib/agama/dbus/hash_validator.rb create mode 100644 service/lib/agama/dbus/types.rb create mode 100644 service/test/agama/dbus/hash_validator_test.rb create mode 100644 service/test/agama/dbus/types_test.rb diff --git a/service/lib/agama/dbus/hash_validator.rb b/service/lib/agama/dbus/hash_validator.rb new file mode 100644 index 0000000000..a7c7a4c3e9 --- /dev/null +++ b/service/lib/agama/dbus/hash_validator.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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. + +require "agama/dbus/types" + +module Agama + module DBus + # Validates a Hash (dictionary) from D-Bus according to an scheme. + # + # This validation is useful to check the expected types of a D-Bus call when some parameter is + # a dictionary with variant types. + # + # @example + # # Let's say there is a D-Bus method with the following signature: + # + # dbus_method :Calculate, "in settings:a{sv}" + # + # # The settings parameter will be transformed to a ruby Hash and this class allows to + # # validate the types of the hash values. + # + # scheme = { + # "ID" => Integer, + # "Name" => String, + # "Children" => Agama::DBus::Types.Array.new(Integer) + # } + # + # value1 = { "ID" => 10, "Name" => "Foo", "Color" => "red" } + # validator = HashValidator.new(value1, scheme: scheme) + # validator.valid? #=> true + # validator.missing_keys #=> ["Children"] + # validator.extra_keys #=> ["Color"] + # validator.issues #=> ["Unknown D-Bus property Color"] + # + # value2 = { "ID" => 10, "Name" => 33 } + # validator = HashValidator.new(value2, scheme: scheme) + # validator.valid? #=> false + # validator.missing_keys #=> ["Children"] + # validator.wrong_type_keys #=> ["Name"] + # validator.issues.size #=> 1 + class HashValidator + # @param value [Hash{String => Object}] Hash to validate. + # @param scheme [Hash{String => Class, Types::BOOL, Types::Array, Types::Hash}] Scheme + # for validating the hash. + def initialize(value, scheme:) + @value = value + @scheme = scheme + end + + # Whether the hash is valid. + # + # The hash is consider as valid if there is no key with wrong type. Missing and extra keys are + # not validated. + # + # @return [Boolean] + def valid? + wrong_type_keys.none? + end + + # Keys with correct type. + # + # Missing and extra keys are ignored. + # + # @return [Array] + def valid_keys + value.keys.select { |k| valid_key?(k) } + end + + # Keys with incorrect type. + # + # Missing and extra keys are ignored. + # + # @return [Array] + def wrong_type_keys + value.keys.select { |k| !extra_key?(k) && wrong_type_key?(k) } + end + + # Keys not included in the scheme. + # + # @return [Array] + def extra_keys + value.keys.select { |k| extra_key?(k) } + end + + # Keys included in the scheme but missing in the hash value. + # + # @return [Array] + def missing_keys + scheme.keys - value.keys + end + + # List of issues. + # + # There is an issue for each extra key and for each key with wrong type. + # + # @return [Array] + def issues + issues = [] + + extra_keys.map do |key| + issues << "Unknown D-Bus property #{key}" + end + + wrong_type_keys.map do |key| + type = scheme[key] + value = self.value[key] + + issues << "D-Bus property #{key} must be #{type}: #{value} (#{value.class})" + end + + issues + end + + private + + # @return [Hash{String => Object}] + attr_reader :value + + # @return [Hash{String => Class, Types::BOOL, Types::Array, Types::Hash}] + attr_reader :scheme + + def valid_key?(key) + !(extra_key?(key) || wrong_type_key?(key)) + end + + def extra_key?(key) + !scheme.keys.include?(key) + end + + def wrong_type_key?(key) + type = scheme[key] + checker = Types::Checker.new(type) + !checker.match?(value[key]) + end + end + end +end diff --git a/service/lib/agama/dbus/types.rb b/service/lib/agama/dbus/types.rb new file mode 100644 index 0000000000..0109dd5e40 --- /dev/null +++ b/service/lib/agama/dbus/types.rb @@ -0,0 +1,159 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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. + +module Agama + module DBus + # This module offers classes to help to validate the expected types from a D-Bus call when + # variant types are involved, typically using dictionaries. + # + # @example + # # Let's say there is a D-Bus method with the following signature: + # + # dbus_method :Calculate, "in settings:a{sv}" + # + # # The settings parameter will be transformed to a ruby Hash. This module provides a + # # {Checker} class that helps to validate the expected types. + # + # # Checks whether the value is a String + # checker = Types::Checker.new(String) + # checker.match?(settings["Foo"]) + # + # # Checks whether the value is bool + # checker = Types::Checker.new(Types::BOOL) + # checker.match?(settings["Foo"]) + # + # # Checks whether the value is an Array of String + # checker = Types::Checker.new(Types::Array.new(String)) + # checker.match?(settings["Foo"]) + # + # # Checks whether the value is a Hash of String keys and Integer values + # checker = Types::Checker.new(Types::Hash.new(k: String, v: Integer)) + # checker.match?(settings["Foo"]) + # + # See {HashValidator} for validating hashes coming from D-Bus according to a scheme. + module Types + # Type representing a boolean (true or false). + BOOL = :bool + + # Type representing an array of values. + class Array + # @return [Class, BOOL, Array, Hash, nil] + attr_reader :elements_type + + # @param elements_type [Class, BOOL, Array, Hash, nil] The type of the elements in the + # array. If nil, the type of the elements is not checked. + def initialize(elements_type = nil) + @elements_type = elements_type + end + end + + # Type representing a hash. + class Hash + # @return [Class, BOOL, Array, Hash, nil] + attr_reader :keys_type + + # @return [Class, BOOL, Array, Hash, nil] + attr_reader :values_type + + # @param key [Class, BOOL, Array, Hash, nil] The type of keys. If nil, the type of the keys + # is not checked. + # @param value [Class, BOOL, Array, Hash, nil] The type of values. If nil, the type of the + # values is not checked. + def initialize(key: nil, value: nil) + @keys_type = key + @values_type = value + end + end + + # Checks whether a value matches a type. + class Checker + # @param type [Class, BOOL, Array, Hash] The type to check. + def initialize(type) + @type = type + end + + # Whether the given value matches the type. + # + # @param value [Object] + # @return [Boolean] + def match?(value) + case type + when BOOL + match_bool?(value) + when Agama::DBus::Types::Array + match_array?(value) + when Agama::DBus::Types::Hash + match_hash?(value) + when Class, Module + value.is_a?(type) + else + false + end + end + + private + + # @return [Class, BOOL, Array, Hash] + attr_reader :type + + # Whether the value matches with {BOOL} type. + # + # @return [Boolean] + def match_bool?(value) + value.is_a?(TrueClass) || value.is_a?(FalseClass) + end + + # Whether the value matches with {Array} type. + # + # @return [Boolean] + def match_array?(value) + return false unless value.is_a?(::Array) + + if type.elements_type + checker = Checker.new(type.elements_type) + return value.all? { |v| checker.match?(v) } + end + + true + end + + # Whether the value matches with {Hash} type. + # + # @return [Boolean] + def match_hash?(value) + return false unless value.is_a?(::Hash) + + if type.keys_type + checker = Checker.new(Types::Array.new(type.keys_type)) + return false unless checker.match?(value.keys) + end + + if type.values_type + checker = Checker.new(Types::Array.new(type.values_type)) + return false unless checker.match?(value.values) + end + + true + end + end + end + end +end diff --git a/service/test/agama/dbus/hash_validator_test.rb b/service/test/agama/dbus/hash_validator_test.rb new file mode 100644 index 0000000000..69b995af31 --- /dev/null +++ b/service/test/agama/dbus/hash_validator_test.rb @@ -0,0 +1,197 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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. + +require_relative "../../test_helper" +require "agama/dbus/hash_validator" +require "agama/dbus/types" + +describe Agama::DBus::HashValidator do + subject { described_class.new(value, scheme: scheme) } + + let(:scheme) do + { + "Name" => String, + "Surname" => Agama::DBus::Types::Array.new(String), + "Age" => Integer, + "Height" => Integer, + "Children" => Array + } + end + + describe "#valid?" do + context "if there is any key with unexpected type" do + let(:value) do + { + "Gender" => "Male", + "Height" => true + } + end + + it "returns false" do + expect(subject.valid?).to eq(false) + end + end + + context "if there is no key with unexpected type" do + let(:value) do + { + "Name" => "John", + "Gender" => "Male", + "Height" => 175 + } + end + + it "returns true" do + expect(subject.valid?).to eq(true) + end + end + end + + describe "#valid_keys" do + let(:value) do + { + "Name" => "John", + "Gender" => "Male", + "Age" => 45, + "Height" => true, + "Children" => ["Mark", "Zara"] + } + end + + it "returns the hash keys defined in the scheme and with the type indicated in the scheme" do + expect(subject.valid_keys).to eq(["Name", "Age", "Children"]) + end + end + + describe "#wrong_type_keys" do + context "if the hash contains the same types as the scheme" do + let(:value) do + { + "Name" => "John", + "Age" => 45, + "Children" => ["Mark", "Zara"] + } + end + + it "returns an empty list" do + expect(subject.wrong_type_keys).to eq([]) + end + end + + context "if the hash contains types different to the scheme" do + let(:value) do + { + "Name" => true, + "Age" => 45, + "Children" => "none" + } + end + + it "returns the keys with wrong type" do + expect(subject.wrong_type_keys).to eq(["Name", "Children"]) + end + end + end + + describe "#extra_keys" do + context "if the hash does not contain keys that are not included in the scheme" do + let(:value) do + { + "Name" => "Jhon", + "Children" => [] + } + end + + it "returns an empty list" do + expect(subject.extra_keys).to eq([]) + end + end + + context "if the hash contains some keys that are not included in the scheme" do + let(:value) do + { + "Name" => "Jhon", + "Gender" => "Male", + "Birthday" => nil, + "Children" => [] + } + end + + it "returns a list with the extra keys" do + expect(subject.extra_keys).to eq(["Gender", "Birthday"]) + end + end + end + + describe "#missing_keys" do + context "if the hash contains all the keys defined in the scheme" do + let(:value) do + { + "Name" => "Jhon", + "Surname" => [], + "Age" => 45, + "Height" => 176, + "Children" => [] + } + end + + it "returns an empty list" do + expect(subject.missing_keys).to eq([]) + end + end + + context "if the hash does not contain any of the keys defined in the scheme" do + let(:value) do + { + "Surname" => [], + "Age" => 45, + "Height" => 176 + } + end + + it "returns a list with the missing keys" do + expect(subject.missing_keys).to eq(["Name", "Children"]) + end + end + end + + describe "#issues" do + let(:value) do + { + "Name" => "John", + "Age" => 45, + "Gender" => "Male", + "Birthday" => nil, + "Height" => "175", + "Children" => {} + } + end + + it "generates an issue for each extra key and for each wrong type" do + expect(subject.issues).to contain_exactly( + /Unknown .* Gender/, + /Unknown .* Birthday/, + /Height must be/, + /Children must be/ + ) + end + end +end diff --git a/service/test/agama/dbus/types_test.rb b/service/test/agama/dbus/types_test.rb new file mode 100644 index 0000000000..6661508be7 --- /dev/null +++ b/service/test/agama/dbus/types_test.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# 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. + +require_relative "../../test_helper" +require "agama/dbus/types" + +describe Agama::DBus::Types::Checker do + describe "#match?" do + describe "for Bool type" do + subject { described_class.new(Agama::DBus::Types::BOOL) } + + it "returns true if the given value is true or false" do + expect(subject.match?(true)).to eq(true) + expect(subject.match?(false)).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?(nil)).to eq(false) + expect(subject.match?("foo")).to eq(false) + expect(subject.match?(10)).to eq(false) + expect(subject.match?([])).to eq(false) + expect(subject.match?({})).to eq(false) + end + end + + describe "for Array type" do + subject { described_class.new(array_type) } + + let(:array_type) { Agama::DBus::Types::Array.new } + + it "returns true if the given value is an array" do + expect(subject.match?([])).to eq(true) + expect(subject.match?([1, 2])).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?(nil)).to eq(false) + expect(subject.match?("foo")).to eq(false) + expect(subject.match?(10)).to eq(false) + expect(subject.match?(true)).to eq(false) + expect(subject.match?({})).to eq(false) + end + + context "if the elements of the array have to be of a specific type" do + let(:array_type) { Agama::DBus::Types::Array.new(String) } + + it "returns true if all the elements match the given type" do + expect(subject.match?([])).to eq(true) + expect(subject.match?(["foo", "bar"])).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?([nil])).to eq(false) + expect(subject.match?([10])).to eq(false) + expect(subject.match?([true])).to eq(false) + expect(subject.match?([[]])).to eq(false) + expect(subject.match?([{}])).to eq(false) + end + end + end + + describe "for Hash type" do + subject { described_class.new(hash_type) } + + let(:hash_type) { Agama::DBus::Types::Hash.new } + + it "returns true if the given value is an hash" do + expect(subject.match?({})).to eq(true) + expect(subject.match?({ foo: "", bar: 1 })).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?(nil)).to eq(false) + expect(subject.match?("foo")).to eq(false) + expect(subject.match?(10)).to eq(false) + expect(subject.match?(true)).to eq(false) + expect(subject.match?([])).to eq(false) + end + + context "if the keys of the hash have to be of a specific type" do + let(:hash_type) { Agama::DBus::Types::Hash.new(key: String) } + + it "returns true if all the keys match the given type" do + expect(subject.match?({})).to eq(true) + expect(subject.match?({ "foo" => "", "bar" => 1 })).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?({ nil: 1 })).to eq(false) + expect(subject.match?({ a: 1 })).to eq(false) + expect(subject.match?({ 10 => 1 })).to eq(false) + expect(subject.match?({ true => 1 })).to eq(false) + expect(subject.match?({ [] => 1 })).to eq(false) + expect(subject.match?({ {} => 1 })).to eq(false) + end + end + + context "if the values of the hash have to be of a specific type" do + let(:hash_type) { Agama::DBus::Types::Hash.new(value: Integer) } + + it "returns true if all the values match the given type" do + expect(subject.match?({})).to eq(true) + expect(subject.match?({ "foo" => 1, bar: 2 })).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?({ foo: nil })).to eq(false) + expect(subject.match?({ foo: 1.0 })).to eq(false) + expect(subject.match?({ foo: "" })).to eq(false) + expect(subject.match?({ foo: [] })).to eq(false) + expect(subject.match?({ foo: {} })).to eq(false) + end + end + + context "if the keys and the values of the hash have to be of a specific type" do + let(:hash_type) do + Agama::DBus::Types::Hash.new(key: String, value: Agama::DBus::Types::BOOL) + end + + it "returns true if all the keys and values match the given types" do + expect(subject.match?({})).to eq(true) + expect(subject.match?({ "foo" => true, "bar" => false })).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?({ "foo" => nil })).to eq(false) + expect(subject.match?({ foo: true })).to eq(false) + end + end + end + + describe "for other types" do + subject { described_class.new(Array) } + + it "returns true if the given value is an instance of the given type" do + expect(subject.match?([])).to eq(true) + expect(subject.match?([1, 2])).to eq(true) + end + + it "returns false otherwise" do + expect(subject.match?(nil)).to eq(false) + expect(subject.match?("foo")).to eq(false) + expect(subject.match?(10)).to eq(false) + expect(subject.match?(true)).to eq(false) + expect(subject.match?({})).to eq(false) + end + end + end +end From 49b845beffb4b6f1ec1e7a2d5e7b97d612f1ad98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:29:25 +0000 Subject: [PATCH 06/29] service: Adapt conversions to and from D-Bus --- service/lib/agama/dbus/storage/manager.rb | 4 +- .../storage/proposal_settings_conversion.rb | 7 +- .../proposal_settings_conversion/from_dbus.rb | 178 ++++++++++++++---- .../proposal_settings_conversion/to_dbus.rb | 126 +++++++++++-- .../agama/dbus/storage/volume_conversion.rb | 7 +- .../storage/volume_conversion/from_dbus.rb | 118 +++++++++--- .../dbus/storage/volume_conversion/to_dbus.rb | 35 +++- .../test/agama/dbus/storage/manager_test.rb | 22 ++- .../from_dbus_test.rb | 153 ++++++++++++--- .../to_dbus_test.rb | 79 +++++++- .../proposal_settings_conversion_test.rb | 6 +- .../volume_conversion/from_dbus_test.rb | 4 +- .../storage/volume_conversion/to_dbus_test.rb | 4 +- .../dbus/storage/volume_conversion_test.rb | 6 +- 14 files changed, 614 insertions(+), 135 deletions(-) diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 3d9380569d..9686733630 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -152,7 +152,9 @@ def default_volume(mount_path) # @param dbus_settings [Hash] # @return [Integer] 0 success; 1 error def calculate_proposal(dbus_settings) - settings = ProposalSettingsConversion.from_dbus(dbus_settings, config: config) + settings = ProposalSettingsConversion.from_dbus(dbus_settings, + config: config, logger: logger) + logger.info( "Calculating storage proposal from D-Bus.\n " \ "D-Bus settings: #{dbus_settings}\n" \ diff --git a/service/lib/agama/dbus/storage/proposal_settings_conversion.rb b/service/lib/agama/dbus/storage/proposal_settings_conversion.rb index 47ac4f9536..dc8e94ffc7 100644 --- a/service/lib/agama/dbus/storage/proposal_settings_conversion.rb +++ b/service/lib/agama/dbus/storage/proposal_settings_conversion.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -31,10 +31,11 @@ module ProposalSettingsConversion # # @param dbus_settings [Hash] # @param config [Agama::Config] + # @param logger [Logger, nil] # # @return [Agama::Storage::ProposalSettings] - def self.from_dbus(dbus_settings, config:) - FromDBus.new(dbus_settings, config: config).convert + def self.from_dbus(dbus_settings, config:, logger: nil) + FromDBus.new(dbus_settings, config: config, logger: logger).convert end # Performs conversion to D-Bus format. diff --git a/service/lib/agama/dbus/storage/proposal_settings_conversion/from_dbus.rb b/service/lib/agama/dbus/storage/proposal_settings_conversion/from_dbus.rb index 412607d72c..c817076e53 100644 --- a/service/lib/agama/dbus/storage/proposal_settings_conversion/from_dbus.rb +++ b/service/lib/agama/dbus/storage/proposal_settings_conversion/from_dbus.rb @@ -19,12 +19,15 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "y2storage/encryption_method" -require "y2storage/pbkd_function" +require "agama/dbus/hash_validator" +require "agama/dbus/storage/volume_conversion" +require "agama/dbus/types" +require "agama/storage/device_settings" require "agama/storage/proposal_settings" require "agama/storage/proposal_settings_reader" require "agama/storage/space_settings" -require "agama/dbus/storage/volume_conversion" +require "y2storage/encryption_method" +require "y2storage/pbkd_function" module Agama module DBus @@ -34,25 +37,23 @@ module ProposalSettingsConversion class FromDBus # @param dbus_settings [Hash] # @param config [Agama::Config] - def initialize(dbus_settings, config:) + # @param logger [Logger, nil] + def initialize(dbus_settings, config:, logger: nil) @dbus_settings = dbus_settings @config = config + @logger = logger || Logger.new($stdout) end # Performs the conversion from D-Bus format. # # @return [Agama::Storage::ProposalSettings] def convert - settings = Agama::Storage::ProposalSettingsReader.new(config).read + logger.info("D-Bus settings: #{dbus_settings}") - settings.tap do |target| - dbus_settings.each do |dbus_property, dbus_value| - converter = CONVERSIONS[dbus_property] - # FIXME: likely ignoring the wrong attribute is not the best - next unless converter + dbus_settings_issues.each { |i| logger.warn(i) } - send(converter, target, dbus_value) - end + Agama::Storage::ProposalSettingsReader.new(config).read.tap do |target| + valid_dbus_properties.each { |p| conversion(target, p) } end end @@ -64,36 +65,145 @@ def convert # @return [Agama::Config] attr_reader :config - # D-Bus attributes and their converters. - CONVERSIONS = { - "BootDevice" => :boot_device_conversion, - "LVM" => :lvm_conversion, - "SystemVGDevices" => :system_vg_devices_conversion, - "EncryptionPassword" => :encryption_password_conversion, - "EncryptionMethod" => :encryption_method_conversion, - "EncryptionPBKDFunction" => :encryption_pbkd_function_conversion, - "SpacePolicy" => :space_policy_conversion, - "SpaceActions" => :space_actions_conversion, - "Volumes" => :volumes_conversion - }.freeze - private_constant :CONVERSIONS + # @return [Logger] + attr_reader :logger + + DBUS_PROPERTIES = [ + { + name: "Target", + type: String, + conversion: :device_conversion + }, + { + name: "TargetDevice", + type: String + }, + { + name: "TargetPVDevices", + type: Types::Array.new(String) + }, + { + name: "ConfigureBoot", + type: Types::BOOL, + conversion: :configure_boot_conversion + }, + { + name: "BootDevice", + type: String, + conversion: :boot_device_conversion + }, + { + name: "EncryptionPassword", + type: String, + conversion: :encryption_password_conversion + }, + { + name: "EncryptionMethod", + type: String, + conversion: :encryption_method_conversion + }, + { + name: "EncryptionPBKDFunction", + type: String, + conversion: :encryption_pbkd_function_conversion + }, + { + name: "SpacePolicy", + type: String, + conversion: :space_policy_conversion + }, + { + name: "SpaceActions", + type: Types::Array.new(Types::Hash.new(key: String, value: String)), + conversion: :space_actions_conversion + }, + { + name: "Volumes", + type: Types::Array.new(Types::Hash.new(key: String)), + conversion: :volumes_conversion + } + ].freeze + + private_constant :DBUS_PROPERTIES + + # Issues detected in the D-Bus settings, see {HashValidator#issues}. + # + # @return [Array] + def dbus_settings_issues + validator.issues + end + + # D-Bus properties with valid type, see {HashValidator#valid_keys}. + # + # @return [Array] + def valid_dbus_properties + validator.valid_keys + end + + # Validator for D-Bus settings. + # + # @return [HashValidator] + def validator + return @validator if @validator + + scheme = DBUS_PROPERTIES.map { |p| [p[:name], p[:type]] }.to_h + @validator = HashValidator.new(dbus_settings, scheme: scheme) + end + + # @param target [Agama::Storage::ProposalSettings] + # @param dbus_property_name [String] + def conversion(target, dbus_property_name) + dbus_property = DBUS_PROPERTIES.find { |d| d[:name] == dbus_property_name } + conversion_method = dbus_property[:conversion] + + return unless conversion_method + + send(conversion_method, target, dbus_settings[dbus_property_name]) + end # @param target [Agama::Storage::ProposalSettings] # @param value [String] - def boot_device_conversion(target, value) - target.boot_device = value.empty? ? nil : value + def device_conversion(target, value) + device_settings = case value + when "disk" + disk_device_conversion + when "newLvmVg" + new_lvm_vg_device_conversion + when "reusedLvmVg" + reused_lvm_vg_device_conversion + end + + target.device = device_settings + end + + # @return [Agama::Storage::DeviceSettings::Disk] + def disk_device_conversion + device = dbus_settings["TargetDevice"] + Agama::Storage::DeviceSettings::Disk.new(device) + end + + # @return [Agama::Storage::DeviceSettings::NewLvmVg] + def new_lvm_vg_device_conversion + candidates = dbus_settings["TargetPVDevices"] || [] + Agama::Storage::DeviceSettings::NewLvmVg.new(candidates) + end + + # @return [Agama::Storage::DeviceSettings::ReusedLvmVg] + def reused_lvm_vg_device_conversion + device = dbus_settings["TargetDevice"] + Agama::Storage::DeviceSettings::ReusedLvmVg.new(device) end # @param target [Agama::Storage::ProposalSettings] # @param value [Boolean] - def lvm_conversion(target, value) - target.lvm.enabled = value + def configure_boot_conversion(target, value) + target.boot.configure = value end # @param target [Agama::Storage::ProposalSettings] - # @param value [Array] - def system_vg_devices_conversion(target, value) - target.lvm.system_vg_devices = value + # @param value [String] + def boot_device_conversion(target, value) + target.boot.device = value.empty? ? nil : value end # @param target [Agama::Storage::ProposalSettings] @@ -144,7 +254,9 @@ def volumes_conversion(target, value) return if value.empty? required_volumes = target.volumes.select { |v| v.outline.required? } - volumes = value.map { |v| VolumeConversion.from_dbus(v, config: config) } + volumes = value.map do |dbus_volume| + VolumeConversion.from_dbus(dbus_volume, config: config, logger: logger) + end target.volumes = volumes + missing_volumes(required_volumes, volumes) end diff --git a/service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb b/service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb index d4820e847f..d29aa92941 100644 --- a/service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb +++ b/service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "agama/dbus/storage/volume_conversion" +require "agama/storage/device_settings" module Agama module DBus @@ -34,19 +35,26 @@ def initialize(settings) # Performs the conversion to D-Bus format. # - # @return [Hash] + # @return [Hash] + # * "Target" [String] + # * "Device" [String] Optional + # * "CandidatePVDevices" [Array] Optional + # * "ConfigureBoot" [Boolean] + # * "BootDevice" [String] + # * "EncryptionPassword" [String] + # * "EncryptionMethod" [String] + # * "EncryptionPBKDFunction" [String] + # * "SpacePolicy" [String] + # * "SpaceActions" [Array] see {#space_actions_conversion} + # * "Volumes" [Array] see {#volumes_conversion} def convert - { - "BootDevice" => settings.boot_device.to_s, - "LVM" => settings.lvm.enabled?, - "SystemVGDevices" => settings.lvm.system_vg_devices, - "EncryptionPassword" => settings.encryption.password.to_s, - "EncryptionMethod" => settings.encryption.method.id.to_s, - "EncryptionPBKDFunction" => settings.encryption.pbkd_function&.value || "", - "SpacePolicy" => settings.space.policy.to_s, - "SpaceActions" => space_actions_conversion, - "Volumes" => settings.volumes.map { |v| VolumeConversion.to_dbus(v) } - } + target = device_conversion + + DBUS_PROPERTIES.each do |dbus_property, conversion| + target[dbus_property] = send(conversion) + end + + target end private @@ -54,12 +62,104 @@ def convert # @return [Agama::Storage::ProposalSettings] attr_reader :settings - # @return [Array] + DBUS_PROPERTIES = { + "ConfigureBoot" => :configure_boot_conversion, + "BootDevice" => :boot_device_conversion, + "EncryptionPassword" => :encryption_password_conversion, + "EncryptionMethod" => :encryption_method_conversion, + "EncryptionPBKDFunction" => :encryption_pbkd_function_conversion, + "SpacePolicy" => :space_policy_conversion, + "SpaceActions" => :space_actions_conversion, + "Volumes" => :volumes_conversion + }.freeze + + private_constant :DBUS_PROPERTIES + + # @return [Hash] + def device_conversion + device_settings = settings.device + + case device_settings + when Agama::Storage::DeviceSettings::Disk + disk_device_conversion(device_settings) + when Agama::Storage::DeviceSettings::NewLvmVg + new_lvm_vg_device_conversion(device_settings) + when Agama::Storage::DeviceSettings::ReusedLvmVg + reused_lvm_vg_device_conversion(device_settings) + end + end + + # @param device_settings [Agama::Storage::DeviceSettings::Disk] + # @return [Hash] + def disk_device_conversion(device_settings) + { + "Target" => "disk", + "TargetDevice" => device_settings.name || "" + } + end + + # @param device_settings [Agama::Storage::DeviceSettings::NewLvmVg] + # @return [Hash] + def new_lvm_vg_device_conversion(device_settings) + { + "Target" => "newLvmVg", + "TargetPVDevices" => device_settings.candidate_pv_devices + } + end + + # @param device_settings [Agama::Storage::DeviceSettings::Disk] + # @return [Hash] + def reused_lvm_vg_device_conversion(device_settings) + { + "Target" => "reusedLvmVg", + "TargetDevice" => device_settings.name || "" + } + end + + # @return [Boolean] + def configure_boot_conversion + settings.boot.configure? + end + + # @return [String] + def boot_device_conversion + settings.boot.device || "" + end + + # @return [String] + def encryption_password_conversion + settings.encryption.password.to_s + end + + # @return [String] + def encryption_method_conversion + settings.encryption.method.id.to_s + end + + # @return [String] + def encryption_pbkd_function_conversion + settings.encryption.pbkd_function&.value || "" + end + + # @return [String] + def space_policy_conversion + settings.space.policy.to_s + end + + # @return [Array>] + # For each action: + # * "Device" [String] + # * "Action" [String] def space_actions_conversion settings.space.actions.each_with_object([]) do |(device, action), actions| actions << { "Device" => device, "Action" => action.to_s } end end + + # @return [Array] see {VolumeConversion::ToDBus}. + def volumes_conversion + settings.volumes.map { |v| VolumeConversion.to_dbus(v) } + end end end end diff --git a/service/lib/agama/dbus/storage/volume_conversion.rb b/service/lib/agama/dbus/storage/volume_conversion.rb index 75481e1a02..4c62d6b86f 100644 --- a/service/lib/agama/dbus/storage/volume_conversion.rb +++ b/service/lib/agama/dbus/storage/volume_conversion.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -31,10 +31,11 @@ module VolumeConversion # # @param dbus_volume [Hash] # @param config [Agama::Config] + # @param logger [Logger, nil] # # @return [Agama::Storage::Volume] - def self.from_dbus(dbus_volume, config:) - FromDBus.new(dbus_volume, config: config).convert + def self.from_dbus(dbus_volume, config:, logger: nil) + FromDBus.new(dbus_volume, config: config, logger: logger).convert end # Performs conversion to D-Bus format. diff --git a/service/lib/agama/dbus/storage/volume_conversion/from_dbus.rb b/service/lib/agama/dbus/storage/volume_conversion/from_dbus.rb index e66f361a3a..b2440dab4f 100644 --- a/service/lib/agama/dbus/storage/volume_conversion/from_dbus.rb +++ b/service/lib/agama/dbus/storage/volume_conversion/from_dbus.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -19,6 +19,8 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. +require "agama/dbus/hash_validator" +require "agama/dbus/types" require "agama/storage/volume" require "agama/storage/volume_location" require "agama/storage/volume_templates_builder" @@ -33,26 +35,24 @@ module VolumeConversion class FromDBus # @param dbus_volume [Hash] # @param config [Agama::Config] - def initialize(dbus_volume, config:) + # @param logger [Logger, nil] + def initialize(dbus_volume, config:, logger: nil) @dbus_volume = dbus_volume @config = config + @logger = logger || Logger.new($stdout) end # Performs the conversion from D-Bus format. # # @return [Agama::Storage::Volume] def convert - builder = Agama::Storage::VolumeTemplatesBuilder.new_from_config(config) - volume = builder.for(dbus_volume["MountPath"] || "") + logger.info("D-Bus volume: #{dbus_volume}") - volume.tap do |target| - dbus_volume.each do |dbus_property, dbus_value| - converter = CONVERSIONS[dbus_property] - # FIXME: likely ignoring the wrong attribute is not the best - next unless converter + dbus_volume_issues.each { |i| logger.warn(i) } - send(converter, target, dbus_value) - end + builder = Agama::Storage::VolumeTemplatesBuilder.new_from_config(config) + builder.for(dbus_volume["MountPath"] || "").tap do |target| + valid_dbus_properties.each { |p| conversion(target, p) } end end @@ -64,19 +64,89 @@ def convert # @return [Agama::Config] attr_reader :config - # D-Bus attributes and their converters. - CONVERSIONS = { - "MountPath" => :mount_path_conversion, - "MountOptions" => :mount_options_conversion, - "Target" => :target_conversion, - "TargetDevice" => :target_device_conversion, - "FsType" => :fs_type_conversion, - "MinSize" => :min_size_conversion, - "MaxSize" => :max_size_conversion, - "AutoSize" => :auto_size_conversion, - "Snapshots" => :snapshots_conversion - }.freeze - private_constant :CONVERSIONS + # @return [Logger] + attr_reader :logger + + DBUS_PROPERTIES = [ + { + name: "MountPath", + type: String, + conversion: :mount_path_conversion + }, + { + name: "MountOptions", + type: Types::Array.new(String), + conversion: :mount_options_conversion + }, + { + name: "Target", + type: String, + conversion: :target_conversion + }, + { + name: "TargetDevice", + type: String, + conversion: :target_device_conversion + }, + { + name: "FsType", + type: String, + conversion: :fs_type_conversion + }, + { + name: "MinSize", + type: Integer, + conversion: :min_size_conversion + }, + { + name: "MaxSize", + type: Integer, + conversion: :max_size_conversion + }, + { + name: "AutoSize", + type: Types::BOOL, + conversion: :auto_size_conversion + }, + { + name: "Snapshots", + type: Types::BOOL, + conversion: :snapshots_conversion + } + ].freeze + + private_constant :DBUS_PROPERTIES + + # Issues detected in the D-Bus volume, see {HashValidator#issues}. + # + # @return [Array] + def dbus_volume_issues + validator.issues + end + + # D-Bus properties with valid type, see {HashValidator#valid_keys}. + # + # @return [Array] + def valid_dbus_properties + validator.valid_keys + end + + # Validator for D-Bus volume. + # + # @return [HashValidator] + def validator + return @validator if @validator + + scheme = DBUS_PROPERTIES.map { |p| [p[:name], p[:type]] }.to_h + @validator = HashValidator.new(dbus_volume, scheme: scheme) + end + + # @param target [Agama::Storage::Volume] + # @param dbus_property_name [String] + def conversion(target, dbus_property_name) + dbus_property = DBUS_PROPERTIES.find { |d| d[:name] == dbus_property_name } + send(dbus_property[:conversion], target, dbus_volume[dbus_property_name]) + end # @param target [Agama::Storage::Volume] # @param value [String] diff --git a/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb b/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb index 9c4c51533b..5a79699000 100644 --- a/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb +++ b/service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -32,7 +32,18 @@ def initialize(volume) # Performs the conversion to D-Bus format. # - # @return [Hash] + # @return [Hash] + # * "MountPath" [String] + # * "MountOptions" [Array] + # * "TargetDevice" [String] + # * "TargetVG" [String] + # * "FsType" [String] + # * "MinSize" [Integer] + # * "MaxSize" [Integer] Optional + # * "AutoSize" [Boolean] + # * "Snapshots" [Booelan] + # * "Transactional" [Boolean] + # * "Outline" [Hash] see {#outline_conversion} def convert { "MountPath" => volume.mount_path.to_s, @@ -43,10 +54,11 @@ def convert "MinSize" => volume.min_size&.to_i, "AutoSize" => volume.auto_size?, "Snapshots" => volume.btrfs.snapshots?, - "Transactional" => volume.btrfs.read_only? + "Transactional" => volume.btrfs.read_only?, + "Outline" => outline_conversion }.tap do |target| + # Some volumes could not have "MaxSize". max_size_conversion(target) - outline_conversion(target) end end @@ -62,11 +74,20 @@ def max_size_conversion(target) target["MaxSize"] = volume.max_size.to_i end - # @param target [Hash] - def outline_conversion(target) + # Converts volume outline to D-Bus. + # + # @return [Hash] + # * "Required" [Boolean] + # * "FsTypes" [Array] + # * "SupportAutoSize" [Boolean] + # * "AdjustByRam" [Boolean] + # * "SnapshotsConfigurable" [Boolean] + # * "SnapshotsAffectSizes" [Boolean] + # * "SizeRelevantVolumes" [Array] + def outline_conversion outline = volume.outline - target["Outline"] = { + { "Required" => outline.required?, "FsTypes" => outline.filesystems.map(&:to_human_string), "SupportAutoSize" => outline.adaptive_sizes?, diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 958725b215..7e85660587 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -22,6 +22,7 @@ require_relative "../../../test_helper" require "agama/dbus/storage/manager" require "agama/dbus/storage/proposal" +require "agama/storage/device_settings" require "agama/storage/manager" require "agama/storage/proposal" require "agama/storage/proposal_settings" @@ -262,8 +263,9 @@ describe "#calculate_proposal" do let(:dbus_settings) do { - "BootDevice" => "/dev/vda", - "LVM" => true, + "Target" => "disk", + "TargetDevice" => "/dev/vda", + "BootDevice" => "/dev/vdb", "EncryptionPassword" => "n0ts3cr3t", "Volumes" => dbus_volumes } @@ -279,9 +281,9 @@ it "calculates a proposal with settings having values from D-Bus" do expect(proposal).to receive(:calculate) do |settings| expect(settings).to be_a(Agama::Storage::ProposalSettings) - expect(settings.boot_device).to eq "/dev/vda" - expect(settings.lvm).to be_a(Agama::Storage::LvmSettings) - expect(settings.lvm.enabled).to eq true + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to eq "/dev/vda" + expect(settings.boot.device).to eq "/dev/vdb" expect(settings.encryption).to be_a(Agama::Storage::EncryptionSettings) expect(settings.encryption.password).to eq("n0ts3cr3t") expect(settings.volumes).to contain_exactly( @@ -296,12 +298,12 @@ context "when the D-Bus settings does not include some values" do let(:dbus_settings) { {} } - it "calculates a proposal with default/empty values for the missing settings" do + it "calculates a proposal with default values for the missing settings" do expect(proposal).to receive(:calculate) do |settings| expect(settings).to be_a(Agama::Storage::ProposalSettings) - expect(settings.boot_device).to eq nil - expect(settings.lvm).to be_a(Agama::Storage::LvmSettings) - expect(settings.lvm.enabled).to eq false + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to be_nil + expect(settings.boot.device).to be_nil expect(settings.encryption).to be_a(Agama::Storage::EncryptionSettings) expect(settings.encryption.password).to be_nil expect(settings.volumes).to eq([]) diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb index 3114f1a9e6..b65221e7c7 100644 --- a/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb +++ b/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb @@ -20,14 +20,15 @@ # find current contact information at www.suse.com. require_relative "../../../../test_helper" -require "y2storage/encryption_method" -require "y2storage/pbkd_function" +require "agama/dbus/storage/proposal_settings_conversion/from_dbus" require "agama/config" +require "agama/storage/device_settings" require "agama/storage/proposal_settings" -require "agama/dbus/storage/proposal_settings_conversion/from_dbus" +require "y2storage/encryption_method" +require "y2storage/pbkd_function" describe Agama::DBus::Storage::ProposalSettingsConversion::FromDBus do - subject { described_class.new(dbus_settings, config: config) } + subject { described_class.new(dbus_settings, config: config, logger: logger) } let(:config) { Agama::Config.new(config_data) } @@ -59,12 +60,25 @@ } end + let(:logger) { Logger.new($stdout, level: :warn) } + + before do + allow(Agama::Storage::EncryptionSettings) + .to receive(:available_methods).and_return( + [ + Y2Storage::EncryptionMethod::LUKS1, + Y2Storage::EncryptionMethod::LUKS2 + ] + ) + end + describe "#convert" do let(:dbus_settings) do { - "BootDevice" => "/dev/sda", - "LVM" => true, - "SystemVGDevices" => ["/dev/sda", "/dev/sdb"], + "Target" => "disk", + "TargetDevice" => "/dev/sda", + "ConfigureBoot" => true, + "BootDevice" => "/dev/sdb", "EncryptionPassword" => "notsecret", "EncryptionMethod" => "luks1", "EncryptionPBKDFunction" => "pbkdf2", @@ -90,16 +104,20 @@ settings = subject.convert expect(settings).to be_a(Agama::Storage::ProposalSettings) - expect(settings.boot_device).to eq("/dev/sda") - expect(settings.lvm.enabled?).to eq(true) - expect(settings.lvm.system_vg_devices).to contain_exactly("/dev/sda", "/dev/sdb") + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to eq("/dev/sda") + expect(settings.boot.configure?).to eq(true) + expect(settings.boot.device).to eq("/dev/sdb") expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::PBKDF2) expect(settings.space.policy).to eq(:custom) expect(settings.space.actions).to eq({ "/dev/sda" => :force_delete, "/dev/sdb1" => :resize }) - expect(settings.volumes.map(&:mount_path)).to contain_exactly("/", "/test") + expect(settings.volumes).to contain_exactly( + an_object_having_attributes(mount_path: "/"), + an_object_having_attributes(mount_path: "/test") + ) end context "when some values are not provided from D-Bus" do @@ -109,22 +127,96 @@ settings = subject.convert expect(settings).to be_a(Agama::Storage::ProposalSettings) - expect(settings.boot_device).to be_nil - expect(settings.lvm.enabled?).to eq(false) + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to be_nil + expect(settings.boot.configure?).to eq(true) + expect(settings.boot.device).to be_nil expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2ID) expect(settings.space.policy).to eq(:delete) - expect(settings.volumes.map(&:mount_path)).to contain_exactly("/", "swap") + expect(settings.volumes).to contain_exactly( + an_object_having_attributes(mount_path: "/"), + an_object_having_attributes(mount_path: "swap") + ) + end + end + + context "when 'Target' is not provided from D-Bus" do + let(:dbus_settings) { {} } + + it "sets device settings to create partitions" do + settings = subject.convert + + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to be_nil + end + end + + context "when 'Target' has 'disk' value" do + let(:dbus_settings) do + { + "Target" => "disk", + "TargetDevice" => "/dev/vda" + } + end + + it "sets device settings to create partitions in the indicated device" do + settings = subject.convert + + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::Disk) + expect(settings.device.name).to eq("/dev/vda") end end - context "when an empty boot device is provided from D-Bus" do - let(:dbus_settings) { { "BootDevice" => "" } } + context "when 'Target' has 'newLvmVg' value" do + let(:dbus_settings) do + { + "Target" => "newLvmVg", + "TargetPVDevices" => ["/dev/vda", "/dev/vdb"] + } + end + + it "sets device settings to create a new LVM volume group in the indicated devices" do + settings = subject.convert + + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::NewLvmVg) + expect(settings.device.candidate_pv_devices).to contain_exactly("/dev/vda", "/dev/vdb") + end + end + + context "when 'Target' has 'reusedLvmVg' value" do + let(:dbus_settings) do + { + "Target" => "reusedLvmVg", + "TargetDevice" => "/dev/vg0" + } + end - it "sets the boot device to nil" do + it "sets device settings to reuse the indicated LVM volume group" do settings = subject.convert - expect(settings.boot_device).to be_nil + expect(settings.device).to be_a(Agama::Storage::DeviceSettings::ReusedLvmVg) + expect(settings.device.name).to eq("/dev/vg0") + end + end + + context "when some value provided from D-Bus has unexpected type" do + let(:dbus_settings) { { "BootDevice" => 1 } } + + it "ignores the value" do + settings = subject.convert + + expect(settings.boot.device).to be_nil + end + end + + context "when some unexpected setting is provided from D-Bus" do + let(:dbus_settings) { { "Foo" => 1 } } + + it "does not fail" do + settings = subject.convert + + expect(settings).to be_a(Agama::Storage::ProposalSettings) end end @@ -133,12 +225,17 @@ it "completes the volumes with the default volumes from config" do settings = subject.convert - expect(settings.volumes.map(&:mount_path)).to contain_exactly("/", "swap") + expect(settings.volumes).to contain_exactly( + an_object_having_attributes(mount_path: "/"), + an_object_having_attributes(mount_path: "swap") + ) end it "ignores templates of non-default volumes" do settings = subject.convert - expect(settings.volumes.map(&:mount_path)).to_not include("/home") + expect(settings.volumes).to_not include( + an_object_having_attributes(mount_path: "/home") + ) end end @@ -153,22 +250,30 @@ it "completes the volumes with the mandatory volumes" do settings = subject.convert - expect(settings.volumes.map(&:mount_path)).to include("/") + expect(settings.volumes).to include( + an_object_having_attributes(mount_path: "/") + ) end it "includes the volumes provided from D-Bus" do settings = subject.convert - expect(settings.volumes.map(&:mount_path)).to include("/test") + expect(settings.volumes).to include( + an_object_having_attributes(mount_path: "/test") + ) end it "ignores default volumes that are not mandatory" do settings = subject.convert - expect(settings.volumes.map(&:mount_path)).to_not include("swap") + expect(settings.volumes).to_not include( + an_object_having_attributes(mount_path: "swap") + ) end it "ignores templates for excluded volumes" do settings = subject.convert - expect(settings.volumes.map(&:mount_path)).to_not include("/home") + expect(settings.volumes).to_not include( + an_object_having_attributes(mount_path: "/home") + ) end end end diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb index c5961e1585..047ab626b2 100644 --- a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb +++ b/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb @@ -20,20 +20,20 @@ # find current contact information at www.suse.com. require_relative "../../../../test_helper" -require "y2storage/encryption_method" -require "y2storage/pbkd_function" require "agama/dbus/storage/proposal_settings_conversion/to_dbus" +require "agama/storage/device_settings" require "agama/storage/proposal_settings" require "agama/storage/volume" +require "y2storage/encryption_method" +require "y2storage/pbkd_function" describe Agama::DBus::Storage::ProposalSettingsConversion::ToDBus do let(:default_settings) { Agama::Storage::ProposalSettings.new } let(:custom_settings) do Agama::Storage::ProposalSettings.new.tap do |settings| - settings.boot_device = "/dev/sda" - settings.lvm.enabled = true - settings.lvm.system_vg_devices = ["/dev/sda", "/dev/sdb"] + settings.device.name = "/dev/sda" + settings.boot.device = "/dev/sdb" settings.encryption.password = "notsecret" settings.encryption.method = Y2Storage::EncryptionMethod::LUKS2 settings.encryption.pbkd_function = Y2Storage::PbkdFunction::ARGON2ID @@ -46,9 +46,10 @@ describe "#convert" do it "converts the settings to a D-Bus hash" do expect(described_class.new(default_settings).convert).to eq( + "Target" => "disk", + "TargetDevice" => "", + "ConfigureBoot" => true, "BootDevice" => "", - "LVM" => false, - "SystemVGDevices" => [], "EncryptionPassword" => "", "EncryptionMethod" => "luks2", "EncryptionPBKDFunction" => "pbkdf2", @@ -58,9 +59,10 @@ ) expect(described_class.new(custom_settings).convert).to eq( - "BootDevice" => "/dev/sda", - "LVM" => true, - "SystemVGDevices" => ["/dev/sda", "/dev/sdb"], + "Target" => "disk", + "TargetDevice" => "/dev/sda", + "ConfigureBoot" => true, + "BootDevice" => "/dev/sdb", "EncryptionPassword" => "notsecret", "EncryptionMethod" => "luks2", "EncryptionPBKDFunction" => "argon2id", @@ -99,5 +101,62 @@ ] ) end + + context "when the device is set to create partitions" do + let(:settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device = Agama::Storage::DeviceSettings::Disk.new("/dev/vda") + end + end + + it "generates settings to use a disk as target device" do + dbus_settings = described_class.new(settings).convert + + expect(dbus_settings).to include( + "Target" => "disk", + "TargetDevice" => "/dev/vda" + ) + + expect(dbus_settings).to_not include("TargetPVDevices") + end + end + + context "when the device is set to create a new LVM volume group" do + let(:settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device = Agama::Storage::DeviceSettings::NewLvmVg.new(["/dev/vda"]) + end + end + + it "generates settings to create a LVM volume group as target device" do + dbus_settings = described_class.new(settings).convert + + expect(dbus_settings).to include( + "Target" => "newLvmVg", + "TargetPVDevices" => ["/dev/vda"] + ) + + expect(dbus_settings).to_not include("TargetDevice") + end + end + + context "when the device is set to reuse a LVM volume group" do + let(:settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device = Agama::Storage::DeviceSettings::ReusedLvmVg.new("/dev/vg0") + end + end + + it "generates settings to reuse a LVM volume group as target device" do + dbus_settings = described_class.new(settings).convert + + expect(dbus_settings).to include( + "Target" => "reusedLvmVg", + "TargetDevice" => "/dev/vg0" + ) + + expect(dbus_settings).to_not include("TargetPVDevices") + end + end end end diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion_test.rb index f4a0c1502a..236e98db35 100644 --- a/service/test/agama/dbus/storage/proposal_settings_conversion_test.rb +++ b/service/test/agama/dbus/storage/proposal_settings_conversion_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -30,8 +30,10 @@ let(:dbus_settings) { {} } + let(:logger) { Logger.new($stdout, level: :warn) } + it "generates proposal settings from D-Bus settings" do - result = described_class.from_dbus(dbus_settings, config: config) + result = described_class.from_dbus(dbus_settings, config: config, logger: logger) expect(result).to be_a(Agama::Storage::ProposalSettings) end end diff --git a/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb b/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb index 5512783c33..e3d3e2bfa9 100644 --- a/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb +++ b/service/test/agama/dbus/storage/volume_conversion/from_dbus_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -61,6 +61,8 @@ } end + let(:logger) { Logger.new($stdout, level: :warn) } + describe "#convert" do let(:dbus_volume) do { diff --git a/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb b/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb index 2356c0c94e..9fc2db2749 100644 --- a/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb +++ b/service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb @@ -20,10 +20,10 @@ # find current contact information at www.suse.com. require_relative "../../../../test_helper" -require "y2storage/filesystems/type" -require "y2storage/disk_size" require "agama/dbus/storage/volume_conversion/to_dbus" require "agama/storage/volume" +require "y2storage/filesystems/type" +require "y2storage/disk_size" describe Agama::DBus::Storage::VolumeConversion::ToDBus do let(:default_volume) { Agama::Storage::Volume.new("/test") } diff --git a/service/test/agama/dbus/storage/volume_conversion_test.rb b/service/test/agama/dbus/storage/volume_conversion_test.rb index 5f158a1855..5dd15d7a98 100644 --- a/service/test/agama/dbus/storage/volume_conversion_test.rb +++ b/service/test/agama/dbus/storage/volume_conversion_test.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2023] SUSE LLC +# Copyright (c) [2023-2024] SUSE LLC # # All Rights Reserved. # @@ -30,8 +30,10 @@ let(:dbus_volume) { {} } + let(:logger) { Logger.new($stdout, level: :warn) } + it "generates a volume from D-Bus settings" do - result = described_class.from_dbus(dbus_volume, config: config) + result = described_class.from_dbus(dbus_volume, config: config, logger: logger) expect(result).to be_a(Agama::Storage::Volume) end end From d587eac29fc832b9cfa001170e5d8716408fa801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:30:12 +0000 Subject: [PATCH 07/29] service: Change proposal D-Bus API --- service/lib/agama/dbus/storage/proposal.rb | 103 ++------ .../test/agama/dbus/storage/proposal_test.rb | 219 +++--------------- 2 files changed, 53 insertions(+), 269 deletions(-) diff --git a/service/lib/agama/dbus/storage/proposal.rb b/service/lib/agama/dbus/storage/proposal.rb index 591d142451..f33e2e61ad 100644 --- a/service/lib/agama/dbus/storage/proposal.rb +++ b/service/lib/agama/dbus/storage/proposal.rb @@ -19,20 +19,18 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "dbus" require "agama/dbus/base_object" require "agama/dbus/storage/proposal_settings_conversion" +require "dbus" module Agama module DBus module Storage - # D-Bus object to manage the storage proposal + # D-Bus object to manage the storage proposal. class Proposal < BaseObject PATH = "/org/opensuse/Agama/Storage1/Proposal" private_constant :PATH - # Constructor - # # @param backend [Agama::Storage::Proposal] # @param logger [Logger] def initialize(backend, logger) @@ -44,88 +42,22 @@ def initialize(backend, logger) private_constant :STORAGE_PROPOSAL_INTERFACE dbus_interface STORAGE_PROPOSAL_INTERFACE do - dbus_reader :boot_device, "s" - - dbus_reader :lvm, "b", dbus_name: "LVM" - - dbus_reader :system_vg_devices, "as", dbus_name: "SystemVGDevices" - - dbus_reader :encryption_password, "s" - - dbus_reader :encryption_method, "s" - - dbus_reader :encryption_pbkd_function, "s", dbus_name: "EncryptionPBKDFunction" - - dbus_reader :space_policy, "s" - - dbus_reader :space_actions, "aa{sv}" - - dbus_reader :volumes, "aa{sv}" - + dbus_reader :settings, "a{sv}" dbus_reader :actions, "aa{sv}" end - # Device used as boot device by the storage proposal - # - # @return [String] Empty string if no device has been selected yet. - def boot_device - dbus_settings.fetch("BootDevice", "") - end - - # Whether the proposal creates logical volumes + # Proposal settings. # - # @return [Boolean] - def lvm - dbus_settings.fetch("LVM", false) - end - - def system_vg_devices - dbus_settings.fetch("SystemVGDevices", []) - end - - # Password for encrypting devices - # - # @return [String] - def encryption_password - dbus_settings.fetch("EncryptionPassword", "") - end - - # Encryption method + # @see ProposalSettingsConversion::ToDBus # - # @return [String] For the possible values, check Y2Storage::EncryptionMethod.all - def encryption_method - dbus_settings.fetch("EncryptionMethod", "") - end - - # PBKD function - # - # @return [String] For the possible values, check Y2Storage::PbkdFunction.all - def encryption_pbkd_function - dbus_settings.fetch("EncryptionPBKDFunction", "") - end - - # Space policy strategy - # - # @return [String] For the possible values, check Agama::Storage::SpaceSettings - def space_policy - dbus_settings.fetch("SpacePolicy", "") - end - - # Space actions - # - # @return [Array] - def space_actions - dbus_settings.fetch("SpaceActions", []) - end + # @return [Hash] + def settings + return {} unless backend.settings - # Volumes used to calculate the storage proposal - # - # @return [Array] - def volumes - dbus_settings.fetch("Volumes", []) + ProposalSettingsConversion.to_dbus(backend.settings) end - # List of sorted actions in D-Bus format + # List of sorted actions in D-Bus format. # # @see #to_dbus_action # @@ -142,17 +74,14 @@ def actions # @return [Logger] attr_reader :logger - # @return [Hash] - def dbus_settings - return {} unless backend.settings - - @dbus_settings ||= ProposalSettingsConversion.to_dbus(backend.settings) - end - - # Converts an action to D-Bus format + # Converts an action to D-Bus format. # # @param action [Y2Storage::CompoundAction] - # @return [Hash] + # @return [Hash] + # * "Device" [String] + # * "Text" [String] + # * "Subvol" [Boolean] + # * "Delete" [Boolean] def to_dbus_action(action) { "Device" => action.target_device.sid, diff --git a/service/test/agama/dbus/storage/proposal_test.rb b/service/test/agama/dbus/storage/proposal_test.rb index fc61302129..cfc18c7be4 100644 --- a/service/test/agama/dbus/storage/proposal_test.rb +++ b/service/test/agama/dbus/storage/proposal_test.rb @@ -21,6 +21,7 @@ require_relative "../../../test_helper" require "agama/dbus/storage/proposal" +require "agama/storage/device_settings" require "agama/storage/proposal" require "agama/storage/proposal_settings" require "agama/storage/volume" @@ -29,212 +30,66 @@ describe Agama::DBus::Storage::Proposal do subject { described_class.new(backend, logger) } - let(:logger) { Logger.new($stdout, level: :warn) } - let(:backend) do instance_double(Agama::Storage::Proposal, settings: settings) end - let(:settings) { nil } - - describe "#boot_device" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns an empty string" do - expect(subject.boot_device).to eq "" - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.boot_device = "/dev/vda" } - end - - it "returns the candidate devices used by the proposal" do - expect(subject.boot_device).to eq "/dev/vda" - end - end - end - - describe "#lvm" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns false" do - expect(subject.lvm).to eq(false) - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.lvm.enabled = true } - end - - it "return whether LVM was used" do - expect(subject.lvm).to eq(true) - end - end - end - - describe "#system_vg_devices" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns an empty list" do - expect(subject.system_vg_devices).to eq([]) - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.lvm.system_vg_devices = ["/dev/vda"] } - end - - it "returns the devices used for the system VG" do - expect(subject.system_vg_devices).to contain_exactly("/dev/vda") - end - end - end - - describe "#encryption_password" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns an empty string" do - expect(subject.encryption_password).to eq("") - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.encryption.password = "n0ts3cr3t" } - end - - it "return the encryption password used by the proposal" do - expect(subject.encryption_password).to eq("n0ts3cr3t") - end - end - end - - describe "#encryption_method" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns an empty string" do - expect(subject.encryption_method).to eq("") - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.encryption.method = luks2 } - end - - let(:luks2) { Y2Storage::EncryptionMethod::LUKS2 } - - it "return the encryption method used by the proposal" do - expect(subject.encryption_method).to eq(luks2.id.to_s) - end - end - end - - describe "#encryption_pbkd_function" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns an empty string" do - expect(subject.encryption_pbkd_function).to eq("") - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.encryption.pbkd_function = argon2id } - end - - let(:argon2id) { Y2Storage::PbkdFunction::ARGON2ID } - - it "return the encryption method used by the proposal" do - expect(subject.encryption_pbkd_function).to eq(argon2id.value) - end - end - end - - describe "#space_policy" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } - - it "returns an empty string" do - expect(subject.space_policy).to eq("") - end - end - - context "if a proposal has been calculated" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.space.policy = :delete } - end + let(:logger) { Logger.new($stdout, level: :warn) } - it "return the space policy used by the proposal" do - expect(subject.space_policy).to eq("delete") - end - end - end + let(:settings) { nil } - describe "#space_actions" do + describe "#settings" do context "if a proposal has not been calculated yet" do let(:settings) { nil } - it "returns an empty list" do - expect(subject.space_actions).to eq([]) + it "returns an empty hash" do + expect(subject.settings).to eq({}) end end context "if a proposal has been calculated" do let(:settings) do Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device = Agama::Storage::DeviceSettings::Disk.new("/dev/vda") + settings.boot.device = "/dev/vdb" + settings.encryption.password = "n0ts3cr3t" + settings.encryption.method = luks2 + settings.encryption.pbkd_function = argon2id + settings.space.policy = :custom settings.space.actions = { "/dev/vda1" => :force_delete, "/dev/vda2" => :resize } + settings.volumes = [ + Agama::Storage::Volume.new("/test1"), + Agama::Storage::Volume.new("/test2") + ] end end - it "return a list with a hash for each action" do - expect(subject.space_actions).to contain_exactly( - { "Device" => "/dev/vda1", "Action" => "force_delete" }, - { "Device" => "/dev/vda2", "Action" => "resize" } - ) - end - end - end - - describe "#volumes" do - let(:settings) do - Agama::Storage::ProposalSettings.new.tap { |s| s.volumes = calculated_volumes } - end - - context "if the calculated settings has no volumes" do - let(:calculated_volumes) { [] } - - it "returns an empty list" do - expect(subject.volumes).to eq([]) - end - end - - context "if the calculated settings has volumes" do - let(:calculated_volumes) { [calculated_volume1, calculated_volume2] } - let(:calculated_volume1) { Agama::Storage::Volume.new("/test1") } - let(:calculated_volume2) { Agama::Storage::Volume.new("/test2") } - - it "returns a list with a hash for each volume" do - expect(subject.volumes.size).to eq(2) - expect(subject.volumes).to all(be_a(Hash)) + let(:luks2) { Y2Storage::EncryptionMethod::LUKS2 } - volume1, volume2 = subject.volumes + let(:argon2id) { Y2Storage::PbkdFunction::ARGON2ID } - expect(volume1).to include("MountPath" => "/test1") - expect(volume2).to include("MountPath" => "/test2") + it "returns the proposal settings in D-Bus format" do + expect(subject.settings).to include( + "Target" => "disk", + "TargetDevice" => "/dev/vda", + "ConfigureBoot" => true, + "BootDevice" => "/dev/vdb", + "EncryptionPassword" => "n0ts3cr3t", + "EncryptionMethod" => luks2.id.to_s, + "EncryptionPBKDFunction" => argon2id.value, + "SpacePolicy" => "custom", + "SpaceActions" => [ + { "Device" => "/dev/vda1", "Action" => "force_delete" }, + { "Device" => "/dev/vda2", "Action" => "resize" } + ], + "Volumes" => [ + include("MountPath" => "/test1"), + include("MountPath" => "/test2") + ] + ) end end end From 3053d4f221fdbda9b530ef90827cb557c1e1a881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 25 Mar 2024 12:30:41 +0000 Subject: [PATCH 08/29] service: Update D-Bus documentation --- ...g.opensuse.Agama.Storage1.Proposal.bus.xml | 9 +--- ...Agama.Storage1.Proposal.Calculator.doc.xml | 54 +++++++++++++------ ...g.opensuse.Agama.Storage1.Proposal.doc.xml | 36 ++++++++----- 3 files changed, 62 insertions(+), 37 deletions(-) diff --git a/doc/dbus/bus/org.opensuse.Agama.Storage1.Proposal.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Storage1.Proposal.bus.xml index 3a5ee0600f..fc5aa6a047 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Storage1.Proposal.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Storage1.Proposal.bus.xml @@ -27,14 +27,7 @@ - - - - - - - - + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml index 254aa14124..ec60f49dcd 100644 --- a/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Storage1.Proposal.Calculator.doc.xml @@ -18,13 +18,14 @@ - + + diff --git a/doc/dbus/org.opensuse.Agama.Storage1.Proposal.doc.xml b/doc/dbus/org.opensuse.Agama.Storage1.Proposal.doc.xml index cf473ac671..9ee58dba88 100644 --- a/doc/dbus/org.opensuse.Agama.Storage1.Proposal.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Storage1.Proposal.doc.xml @@ -4,26 +4,35 @@ Interfaces with the properties of the calculated proposal. --> - - - - - - - - +