From 7e2042320541b91829965b8ee1d6f53627b8ac38 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, 30 Sep 2024 12:30:23 +0100 Subject: [PATCH 01/21] schema: 'generate' section for pvs --- .../share/examples/storage/generate_pvs.json | 48 ++++++++++++++++ rust/agama-lib/share/profile.schema.json | 55 +++++++++++++++++-- 2 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 rust/agama-lib/share/examples/storage/generate_pvs.json diff --git a/rust/agama-lib/share/examples/storage/generate_pvs.json b/rust/agama-lib/share/examples/storage/generate_pvs.json new file mode 100644 index 000000000..709406bb8 --- /dev/null +++ b/rust/agama-lib/share/examples/storage/generate_pvs.json @@ -0,0 +1,48 @@ +{ + "storage": { + "drives": [ + { + "alias": "first-disk" + }, + { + "partitions": [ + { + "alias": "pv1", + "id": "lvm", + "size": { "min": "10 GiB" } + } + ] + } + ], + "volumeGroups": [ + { + "name": "system", + "physicalVolumes": ["pv1"], + "logicalVolumes": [ + { + "filesystem": { "path": "/" } + } + ] + }, + { + "name": "logs", + "physicalVolumes": [ + { "generate": ["first-disk"] } + ] + }, + { + "name": "data", + "physicalVolumes": [ + { + "generate": { + "targetDevices": ["first-disk"], + "encryption": { + "luks2": { "password": "12345" } + } + } + } + ] + } + ] + } +} diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index dab5ccc21..8081fff53 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -529,11 +529,56 @@ "physicalVolumes": { "title": "Physical volumes", "description": "Devices to use as physical volumes.", - "type": "array", - "items": { - "title": "Device alias", - "type": "string" - } + "$comment": "In the future it would be possible to indicate both aliases and 'generate' items together.", + "anyOf": [ + { + "type": "array", + "items": { + "title": "Device alias", + "type": "string" + } + }, + { + "type": "array", + "items": { + "title": "Generate physical volumes", + "description": "Automatically creates the needed physical volumes in the indicated devices.", + "type": "object", + "additionalProperties": false, + "required": ["generate"], + "properties": { + "generate": { + "anyOf": [ + { + "type": "array", + "items": { + "title": "Device alias", + "type": "string" + } + }, + { + "type": "object", + "additionalProperties": false, + "required": ["targetDevices"], + "properties": { + "targetDevices": { + "type": "array", + "items": { + "title": "Device alias", + "type": "string" + } + }, + "encryption": { + "$ref": "#/$defs/encryption" + } + } + } + ] + } + } + } + } + ] }, "logicalVolumes": { "title": "Logical volumes", From 7e7bc4d10576de069c21cf465c06d37efce785a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 3 Oct 2024 12:35:14 +0100 Subject: [PATCH 02/21] storage: add solvers for filesystem and encryption --- .../agama/storage/config_encryption_solver.rb | 94 ++++++++++++++++ .../agama/storage/config_filesystem_solver.rb | 106 ++++++++++++++++++ .../lib/agama/storage/config_size_solver.rb | 4 + service/lib/agama/storage/config_solver.rb | 11 +- service/lib/agama/storage/configs/btrfs.rb | 19 +++- 5 files changed, 226 insertions(+), 8 deletions(-) create mode 100644 service/lib/agama/storage/config_encryption_solver.rb create mode 100644 service/lib/agama/storage/config_filesystem_solver.rb diff --git a/service/lib/agama/storage/config_encryption_solver.rb b/service/lib/agama/storage/config_encryption_solver.rb new file mode 100644 index 000000000..4a5996af8 --- /dev/null +++ b/service/lib/agama/storage/config_encryption_solver.rb @@ -0,0 +1,94 @@ +# 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/storage/config_builder" + +module Agama + module Storage + # Solver for the encryption configs. + # + # The encryption configs are solved by assigning the default encryption values defined by the + # productd, if needed. + class ConfigEncryptionSolver + # @param product_config [Agama::Config] + def initialize(product_config) + @product_config = product_config + end + + # Solves all the encryption configs within a given config. + # + # @note The config object is modified. + # + # @param config [Config] + def solve(config) + @config = config + + configs_with_encryption.each { |c| solve_encryption(c) } + end + + private + + # @return [Agama::Config] + attr_reader :product_config + + # @return [Config] + attr_reader :config + + # @param config [#encryption] + def solve_encryption(config) + return unless config.encryption + + encryption = config.encryption + encryption.method ||= default_encryption.method + + # Recovering values from the default encryption only makes sense if the encryption method is + # the same. + solve_encryption_values(encryption) if encryption.method == default_encryption.method + end + + # @param config [Configs::Encryption] + def solve_encryption_values(config) + config.password ||= default_encryption.password + config.pbkd_function ||= default_encryption.pbkd_function + config.label ||= default_encryption.label + config.cipher ||= default_encryption.cipher + config.key_size ||= default_encryption.key_size + end + + # @return [Array<#encryption>] + def configs_with_encryption + config.drives + config.partitions + config.logical_volumes + end + + # Default encryption defined by the product. + # + # @return [Configs::Encryption] + def default_encryption + @default_encryption ||= config_builder.default_encryption + end + + # @return [ConfigBuilder] + def config_builder + @config_builder ||= ConfigBuilder.new(product_config) + end + end + end +end diff --git a/service/lib/agama/storage/config_filesystem_solver.rb b/service/lib/agama/storage/config_filesystem_solver.rb new file mode 100644 index 000000000..abb9ba423 --- /dev/null +++ b/service/lib/agama/storage/config_filesystem_solver.rb @@ -0,0 +1,106 @@ +# 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/storage/config_builder" + +module Agama + module Storage + # Solver for the filesystem configs. + # + # The filesystem configs are solved by assigning the default filesystem values defined by the + # productd, if needed. + class ConfigFilesystemSolver + # @param product_config [Agama::Config] + def initialize(product_config) + @product_config = product_config + end + + # Solves all the filesystem configs within a given config. + # + # @note The config object is modified. + # + # @param config [Config] + def solve(config) + @config = config + + configs_with_filesystem.each { |c| solve_filesystem(c) } + end + + private + + # @return [Agama::Config] + attr_reader :product_config + + # @return [Config] + attr_reader :config + + # @param config [#filesystem] + def solve_filesystem(config) + return unless config.filesystem + + default_filesystem = default_filesystem(config.filesystem.path) + + config.filesystem.type ||= default_filesystem.type + config.filesystem.type.btrfs ||= default_filesystem.type.btrfs + solve_btrfs_values(config) + end + + # @param config [#filesystem] + def solve_btrfs_values(config) + btrfs = config.filesystem.type.btrfs + return unless btrfs + + default_btrfs = default_btrfs(config.filesystem.path) + + btrfs.snapshots = default_btrfs.snapshots? if btrfs.snapshots.nil? + btrfs.read_only = default_btrfs.read_only? if btrfs.read_only.nil? + btrfs.subvolumes ||= default_btrfs.subvolumes + btrfs.default_subvolume ||= (default_btrfs.default_subvolume || "") + end + + # @return [Array<#filesystem>] + def configs_with_filesystem + config.drives + config.partitions + config.logical_volumes + end + + # Default filesystem defined by the product. + # + # @param path [String, nil] + # @return [Configs::Filesystem] + def default_filesystem(path = nil) + config_builder.default_filesystem(path) + end + + # Default btrfs config defined by the product. + # + # @param path [String, nil] + # @return [Configs::Btrfs] + def default_btrfs(path = nil) + default_filesystem(path).type.btrfs + end + + # @return [ConfigBuilder] + def config_builder + @config_builder ||= ConfigBuilder.new(product_config) + end + end + end +end diff --git a/service/lib/agama/storage/config_size_solver.rb b/service/lib/agama/storage/config_size_solver.rb index 69449ae8c..bb8ffbc5b 100644 --- a/service/lib/agama/storage/config_size_solver.rb +++ b/service/lib/agama/storage/config_size_solver.rb @@ -175,6 +175,10 @@ def with_current_size?(config) !config.size.default? && (config.size.min.nil? || config.size.max.nil?) end + # Whether the config has to be considered. + # + # Note that a config could be ignored if a device is not found for its search. + # # @param config [Object] Any config from {Configs}. # @return [Boolean] def valid?(config) diff --git a/service/lib/agama/storage/config_solver.rb b/service/lib/agama/storage/config_solver.rb index 505dc06a8..36f8a9995 100644 --- a/service/lib/agama/storage/config_solver.rb +++ b/service/lib/agama/storage/config_solver.rb @@ -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/storage/config_encryption_solver" +require "agama/storage/config_filesystem_solver" require "agama/storage/config_search_solver" require "agama/storage/config_size_solver" @@ -26,7 +28,9 @@ module Agama module Storage # Class for solving a storage config. # - # It assigns proper devices and size values according to the product and the system. + # Solving a config means to assign proper values according to the product and the system. For + # example, the sizes of a partition config taking into account its fallbacks, assigning a + # specific device when a config has a search, etc. class ConfigSolver # @param devicegraph [Y2Storage::Devicegraph] # @param product_config [Agama::Config] @@ -35,13 +39,16 @@ def initialize(devicegraph, product_config) @product_config = product_config end - # Solves all the search and size configs within a given config. + # Solves the config according to the product and the system. # # @note The config object is modified. # # @param config [Config] def solve(config) + ConfigEncryptionSolver.new(product_config).solve(config) + ConfigFilesystemSolver.new(product_config).solve(config) ConfigSearchSolver.new(devicegraph).solve(config) + # Sizes must be solved once the searches are solved. ConfigSizeSolver.new(devicegraph, product_config).solve(config) end diff --git a/service/lib/agama/storage/configs/btrfs.rb b/service/lib/agama/storage/configs/btrfs.rb index 1dd3cf4de..21f7f65fe 100644 --- a/service/lib/agama/storage/configs/btrfs.rb +++ b/service/lib/agama/storage/configs/btrfs.rb @@ -26,26 +26,33 @@ module Configs class Btrfs # Whether there are snapshots. # - # @return [Boolean] + # @return [Boolean, nil] attr_accessor :snapshots - alias_method :snapshots?, :snapshots - # @return [Boolean] + # @return [Boolean, nil] attr_accessor :read_only - alias_method :read_only?, :read_only # @return [Array, nil] if nil, a historical fallback list # may be applied depending on the mount path of the volume attr_accessor :subvolumes - # @return [String] + # @return [String, nil] attr_accessor :default_subvolume # Constructor def initialize @snapshots = false @read_only = false - @default_subvolume = "" + end + + # @return [Boolean] + def snapshots? + !!snapshots + end + + # @return [Boolean] + def read_only? + !!read_only end end end From a37a516e2d903f57c9129470ebcbf6a7f31b1e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 3 Oct 2024 12:37:21 +0100 Subject: [PATCH 03/21] refactor(storage): remove solving values from JSON conversions - Solving encryption and filesystem configs is now done by the config solver. - JSON conversion only applies the values provided by the JSON data and expands 'generate' for partitions and logical volumes. --- service/lib/agama/config.rb | 23 + service/lib/agama/storage/config.rb | 10 - .../storage/config_conversions/from_json.rb | 40 +- .../from_json_conversions/base.rb | 25 +- .../from_json_conversions/boot.rb | 19 +- .../from_json_conversions/btrfs.rb | 19 +- .../from_json_conversions/config.rb | 33 +- .../from_json_conversions/drive.rb | 30 +- .../from_json_conversions/encryption.rb | 32 +- .../from_json_conversions/filesystem.rb | 36 +- .../from_json_conversions/filesystem_type.rb | 26 +- .../from_json_conversions/logical_volume.rb | 26 +- .../from_json_conversions/partition.rb | 27 +- .../from_json_conversions/search.rb | 19 +- .../from_json_conversions/size.rb | 19 +- .../from_json_conversions/volume_group.rb | 26 +- .../from_json_conversions/with_encryption.rb | 11 +- .../from_json_conversions/with_filesystem.rb | 14 +- .../from_json_conversions/with_partitions.rb | 9 +- .../from_json_conversions/with_ptable_type.rb | 6 +- .../from_json_conversions/with_search.rb | 10 +- .../from_json_conversions/with_size.rb | 9 +- .../lib/agama/storage/config_json_solver.rb | 44 +- .../config_conversions/from_json_test.rb | 2361 ++++++++--------- 24 files changed, 1327 insertions(+), 1547 deletions(-) diff --git a/service/lib/agama/config.rb b/service/lib/agama/config.rb index 3b4c2ba81..4712b5c55 100644 --- a/service/lib/agama/config.rb +++ b/service/lib/agama/config.rb @@ -186,8 +186,31 @@ def arch_elements_from(*keys, property: nil, default: []) end.compact end + # Default paths to be created for the product. + # + # @return [Array] + def default_paths + data.dig("storage", "volumes") || [] + end + + # Mandatory paths to be created for the product. + # + # @return [Array] + def mandatory_paths + default_paths.select { |p| mandatory_path?(p) } + end + private + def mandatory_path?(path) + templates = data.dig("storage", "volume_templates") || [] + template = templates.find { |t| t["mount_path"] == path } + + return false unless template + + template.dig("outline", "required") || false + end + # Simple deep merge # # @param a_hash [Hash] Default values diff --git a/service/lib/agama/storage/config.rb b/service/lib/agama/storage/config.rb index 05e532675..ab8166010 100644 --- a/service/lib/agama/storage/config.rb +++ b/service/lib/agama/storage/config.rb @@ -55,16 +55,6 @@ def initialize @nfs_mounts = [] end - # Creates a config from JSON hash according to schema. - # - # @param config_json [Hash] - # @param product_config [Agama::Config] - # - # @return [Storage::Config] - def self.new_from_json(config_json, product_config:) - ConfigConversions::FromJSON.new(config_json, product_config: product_config).convert - end - # Name of the device that will presumably be used to boot the target system # # @return [String, nil] nil if there is no enough information to infer a possible boot disk diff --git a/service/lib/agama/storage/config_conversions/from_json.rb b/service/lib/agama/storage/config_conversions/from_json.rb index 26e210765..86cddab38 100644 --- a/service/lib/agama/storage/config_conversions/from_json.rb +++ b/service/lib/agama/storage/config_conversions/from_json.rb @@ -20,7 +20,6 @@ # find current contact information at www.suse.com. require "agama/config" -require "agama/storage/config_builder" require "agama/storage/config_conversions/from_json_conversions/config" require "agama/storage/config_json_solver" @@ -29,14 +28,13 @@ module Storage module ConfigConversions # Config conversion from JSON hash according to schema. class FromJSON - # TODO: Replace product_config param by a ProductDefinition. - # # @param config_json [Hash] - # @param product_config [Agama::Config, nil] - def initialize(config_json, product_config: nil) - # Copies the JSON hash to avoid changes in the given parameter, see {ConfigJSONSolver}. - @config_json = json_dup(config_json) - @product_config = product_config || Agama::Config.new + # @param default_paths [Array] Default paths of the product. + # @param mandatory_paths [Array] Mandatory paths of the product. + def initialize(config_json, default_paths: [], mandatory_paths: []) + @config_json = config_json + @default_paths = default_paths + @mandatory_paths = mandatory_paths end # Performs the conversion from Hash according to the JSON schema. @@ -44,17 +42,15 @@ def initialize(config_json, product_config: nil) # @return [Storage::Config] def convert # TODO: Raise error if config_json does not match the JSON schema. - # Implementation idea: ConfigJSONChecker class which reports issues if: - # * The JSON does not match the schema. - # * The JSON contains more than one "generate" for partitions and logical volumes. - # * The JSON contains invalid aliases (now checked by ConfigChecker). + + # Copies the JSON hash to avoid changes in the given config, see {ConfigJSONSolver}. + config_json = json_dup(self.config_json) + ConfigJSONSolver - .new(product_config) + .new(default_paths: default_paths, mandatory_paths: mandatory_paths) .solve(config_json) - FromJSONConversions::Config - .new(config_json, config_builder: config_builder) - .convert + FromJSONConversions::Config.new(config_json).convert end private @@ -62,8 +58,11 @@ def convert # @return [Hash] attr_reader :config_json - # @return [Agama::Config] - attr_reader :product_config + # @return [Array] + attr_reader :default_paths + + # @return [Array] + attr_reader :mandatory_paths # Deep dup of the given JSON. # @@ -72,11 +71,6 @@ def convert def json_dup(json) Marshal.load(Marshal.dump(json)) end - - # @return [ConfigBuilder] - def config_builder - @config_builder ||= ConfigBuilder.new(product_config) - end end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb index c295b9242..e3e449f0f 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/base.rb @@ -25,35 +25,32 @@ module ConfigConversions module FromJSONConversions # Base class for conversions from JSON hash according to schema. class Base - # @param config_builder [ConfigBuilder, nil] - def initialize(config_builder = nil) - @config_builder = config_builder + def initialize(config_json) + @config_json = config_json end # Performs the conversion from Hash according to the JSON schema. # - # @param default [Object] A {Config} or any its configs from {Storage::Configs}. + # @param config [Object] A {Config} or any of its configs from {Storage::Configs}. # @return [Object] A {Config} or any its configs from {Storage::Configs}. - def convert(default) - default.dup.tap do |config| - conversions(config).each do |property, value| - next if value.nil? + def convert(config) + conversions.each do |property, value| + next if value.nil? - config.public_send("#{property}=", value) - end + config.public_send("#{property}=", value) end + + config end private - # @return [ConfigBuilder, nil] - attr_reader :config_builder + attr_reader :config_json # Values to apply to the config. # - # @param _default [Object] A {Config} or any its configs from {Storage::Configs}. # @return [Hash] e.g., { name: "/dev/vda" }. - def conversions(_default) + def conversions {} end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb index 259d12ec1..e2d2096ab 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/boot.rb @@ -28,30 +28,19 @@ module ConfigConversions module FromJSONConversions # Boot conversion from JSON hash according to schema. class Boot < Base - # @param boot_json [Hash] - def initialize(boot_json) - super() - @boot_json = boot_json - end - # @see Base#convert - # - # @param default [Configs::Boot, nil] # @return [Configs::Boot] - def convert(default = nil) - super(default || Configs::Boot.new) + def convert + super(Configs::Boot.new) end private - # @return [Hash] - attr_reader :boot_json + alias_method :boot_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Boot] # @return [Hash] - def conversions(_default) + def conversions { configure: boot_json[:configure], device: boot_json[:device] diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb index 0e95b0a24..f658287b7 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/btrfs.rb @@ -28,30 +28,19 @@ module ConfigConversions module FromJSONConversions # Btrfs conversion from JSON hash according to schema. class Btrfs < Base - # @param btrfs_json [Hash] - def initialize(btrfs_json) - super() - @btrfs_json = btrfs_json - end - # @see Base#convert - # - # @param default [Configs::Btrfs, nil] # @return [Configs::Btrfs] - def convert(default = nil) - super(default || Configs::Btrfs.new) + def convert + super(Configs::Btrfs.new) end private - # @return [String] - attr_reader :btrfs_json + alias_method :btrfs_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Btrfs] # @return [Hash] - def conversions(_default) + def conversions { snapshots: btrfs_json[:snapshots] } diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb index fbb4c6fd3..4e86fef67 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/config.rb @@ -31,45 +31,30 @@ module ConfigConversions module FromJSONConversions # Config conversion from JSON hash according to schema. class Config < Base - # @param config_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(config_json, config_builder: nil) - super(config_builder) - @config_json = config_json - end - # @see Base#convert - # - # @param default [Config, nil] # @return [Config] - def convert(default = nil) - super(default || Storage::Config.new) + def convert + super(Storage::Config.new) end private - # @return [Hash] - attr_reader :config_json - # @see Base#conversions - # - # @param default [Config] # @return [Hash] - def conversions(default) + def conversions { - boot: convert_boot(default.boot), + boot: convert_boot, drives: convert_drives, volume_groups: convert_volume_groups } end - # @param default [Configs::Boot, nil] # @return [Configs::Boot, nil] - def convert_boot(default = nil) + def convert_boot boot_json = config_json[:boot] return unless boot_json - FromJSONConversions::Boot.new(boot_json).convert(default) + FromJSONConversions::Boot.new(boot_json).convert end # @return [Array, nil] @@ -83,7 +68,7 @@ def convert_drives # @param drive_json [Hash] # @return [Configs::Drive] def convert_drive(drive_json) - FromJSONConversions::Drive.new(drive_json, config_builder: config_builder).convert + FromJSONConversions::Drive.new(drive_json).convert end # @return [Array, nil] @@ -97,9 +82,7 @@ def convert_volume_groups # @param volume_group_json [Hash] # @return [Configs::VolumeGroup] def convert_volume_group(volume_group_json) - FromJSONConversions::VolumeGroup - .new(volume_group_json, config_builder: config_builder) - .convert + FromJSONConversions::VolumeGroup.new(volume_group_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb index 8146da620..8fd661dd6 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/drive.rb @@ -39,38 +39,26 @@ class Drive < Base include WithPtableType include WithPartitions - # @param drive_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(drive_json, config_builder: nil) - super(config_builder) - @drive_json = drive_json - end - # @see Base#convert - # - # @param default [Configs::Drive, nil] # @return [Configs::Drive] - def convert(default = nil) - super(default || Configs::Drive.new) + def convert + super(Configs::Drive.new) end private - # @return [Hash] - attr_reader :drive_json + alias_method :drive_json, :config_json # @see Base#conversions - # - # @param default [Configs::Drive] # @return [Hash] - def conversions(default) + def conversions { - search: convert_search(drive_json, default: default.search), + search: convert_search, alias: drive_json[:alias], - encryption: convert_encryption(drive_json, default: default.encryption), - filesystem: convert_filesystem(drive_json, default: default.filesystem), - ptable_type: convert_ptable_type(drive_json), - partitions: convert_partitions(drive_json) + encryption: convert_encryption, + filesystem: convert_filesystem, + ptable_type: convert_ptable_type, + partitions: convert_partitions } end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb index c280f0ebf..0bafd6a99 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb @@ -30,34 +30,19 @@ module ConfigConversions module FromJSONConversions # Encryption conversion from JSON hash according to schema. class Encryption < Base - # @param encryption_json [Hash, String] - # @param config_builder [ConfigBuilder, nil] - def initialize(encryption_json, config_builder: nil) - super(config_builder) - @encryption_json = encryption_json - end - # @see Base#convert - # - # @param default [Configs::Encryption, nil] # @return [Configs::Encryption] - def convert(default = nil) - super(default || self.default) + def convert + super(Configs::Encryption.new) end private - # @return [Hash, String] - attr_reader :encryption_json - - # @return [Configs::Encryption] - attr_reader :default_config + alias_method :encryption_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Encryption] # @return [Hash] - def conversions(_default) + def conversions return luks1_conversions if luks1? return luks2_conversions if luks2? return pervasive_luks2_conversions if pervasive_luks2? @@ -159,15 +144,6 @@ def convert_label def convert_pbkd_function Y2Storage::PbkdFunction.find(encryption_json.dig(:luks2, :pbkdFunction)) end - - # Default encryption config. - # - # @return [Configs::Encryption] - def default - return Configs::Encryption.new unless config_builder - - config_builder.default_encryption - end end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb index 68b605c7f..b5aac1405 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem.rb @@ -30,31 +30,19 @@ module ConfigConversions module FromJSONConversions # Filesystem conversion from JSON hash according to schema. class Filesystem < Base - # @param filesystem_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(filesystem_json, config_builder: nil) - super(config_builder) - @filesystem_json = filesystem_json - end - # @see Base#convert - # - # @param default [Configs::Filesystem, nil] # @return [Configs::Filesystem] - def convert(default = nil) - super(default || self.default) + def convert + super(Configs::Filesystem.new) end private - # @return [Hash] - attr_reader :filesystem_json + alias_method :filesystem_json, :config_json # @see Base#conversions - # - # @param default [Configs::Filesystem] # @return [Hash] - def conversions(default) + def conversions { reuse: filesystem_json[:reuseIfPossible], label: filesystem_json[:label], @@ -62,7 +50,7 @@ def conversions(default) mount_options: filesystem_json[:mountOptions], mkfs_options: filesystem_json[:mkfsOptions], mount_by: convert_mount_by, - type: convert_type(default.type) + type: convert_type } end @@ -74,22 +62,12 @@ def convert_mount_by Y2Storage::Filesystems::MountByType.find(value.to_sym) end - # @param default [Configs::FilesystemType, nil] # @return [Configs::FilesystemType, nil] - def convert_type(default = nil) + def convert_type filesystem_type_json = filesystem_json[:type] return unless filesystem_type_json - FromJSONConversions::FilesystemType.new(filesystem_type_json).convert(default) - end - - # Default filesystem config. - # - # @return [Configs::Filesystem] - def default - return Configs::Filesystem.new unless config_builder - - config_builder.default_filesystem(filesystem_json[:path]) + FromJSONConversions::FilesystemType.new(filesystem_type_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb index 094e9976c..14e032d28 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/filesystem_type.rb @@ -31,33 +31,22 @@ module ConfigConversions module FromJSONConversions # Filesystem type conversion from JSON hash according to schema. class FilesystemType < Base - # @param filesystem_type_json [Hash, String] - def initialize(filesystem_type_json) - super() - @filesystem_type_json = filesystem_type_json - end - # @see Base#convert - # - # @param default [Configs::FilesystemType, nil] # @return [Configs::FilesystemType] - def convert(default = nil) - super(default || Configs::FilesystemType.new) + def convert + super(Configs::FilesystemType.new) end private - # @return [Hash, String] - attr_reader :filesystem_type_json + alias_method :filesystem_type_json, :config_json # @see Base#conversions - # - # @param default [Configs::FilesystemType] # @return [Hash] - def conversions(default) + def conversions { fs_type: convert_type, - btrfs: convert_btrfs(default.btrfs) + btrfs: convert_btrfs } end @@ -67,15 +56,14 @@ def convert_type Y2Storage::Filesystems::Type.find(value.to_sym) end - # @param default [Configs::Btrfs, nil] # @return [Configs::Btrfs, nil] - def convert_btrfs(default = nil) + def convert_btrfs return if filesystem_type_json.is_a?(String) btrfs_json = filesystem_type_json[:btrfs] return unless btrfs_json - FromJSONConversions::Btrfs.new(btrfs_json).convert(default) + FromJSONConversions::Btrfs.new(btrfs_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb index cd48f8de1..cbd98c3e3 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/logical_volume.rb @@ -36,36 +36,24 @@ class LogicalVolume < Base include WithFilesystem include WithSize - # @param logical_volume_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(logical_volume_json, config_builder: nil) - super(config_builder) - @logical_volume_json = logical_volume_json - end - # @see Base#convert - # - # @param default [Configs::LogicalVolume, nil] # @return [Configs::LogicalVolume] - def convert(default = nil) - super(default || Configs::LogicalVolume.new) + def convert + super(Configs::LogicalVolume.new) end private - # @return [Hash] - attr_reader :logical_volume_json + alias_method :logical_volume_json, :config_json # @see Base#conversions - # - # @param default [Configs::LogicalVolume] # @return [Hash] - def conversions(default) + def conversions { alias: logical_volume_json[:alias], - encryption: convert_encryption(logical_volume_json, default: default.encryption), - filesystem: convert_filesystem(logical_volume_json, default: default.filesystem), - size: convert_size(logical_volume_json, default: default.size), + encryption: convert_encryption, + filesystem: convert_filesystem, + size: convert_size, name: logical_volume_json[:name], stripes: logical_volume_json[:stripes], stripe_size: convert_stripe_size, diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb index 81a487057..00823a0c4 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/partition.rb @@ -38,36 +38,25 @@ class Partition < Base include WithFilesystem include WithSize - # @param partition_json [Hash] - def initialize(partition_json, config_builder: nil) - super(config_builder) - @partition_json = partition_json - end - # @see Base#convert - # - # @param default [Configs::Partition, nil] # @return [Configs::Partition] - def convert(default = nil) - super(default || Configs::Partition.new) + def convert + super(Configs::Partition.new) end private - # @return [Hash] - attr_reader :partition_json + alias_method :partition_json, :config_json # @see Base#conversions - # - # @param default [Configs::Partition] # @return [Hash] - def conversions(default) + def conversions { - search: convert_search(partition_json, default: default.search), + search: convert_search, alias: partition_json[:alias], - encryption: convert_encryption(partition_json, default: default.encryption), - filesystem: convert_filesystem(partition_json, default: default.filesystem), - size: convert_size(partition_json, default: default.size), + encryption: convert_encryption, + filesystem: convert_filesystem, + size: convert_size, id: convert_id, delete: partition_json[:delete], delete_if_needed: partition_json[:deleteIfNeeded] diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb index 327568a36..5df1bfb67 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/search.rb @@ -28,30 +28,19 @@ module ConfigConversions module FromJSONConversions # Search conversion from JSON hash according to schema. class Search < Base - # @param search_json [Hash, String] - def initialize(search_json) - super() - @search_json = search_json - end - # @see Base#convert - # - # @param default [Configs::Search, nil] # @return [Configs::Search] - def convert(default = nil) - super(default || Configs::Search.new) + def convert + super(Configs::Search.new) end private - # @return [Hash, String] - attr_reader :search_json + alias_method :search_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Partition] # @return [Hash] - def conversions(_default) + def conversions { name: convert_name, if_not_found: convert_not_found diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb index cb02433aa..c1cf93881 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/size.rb @@ -29,30 +29,19 @@ module ConfigConversions module FromJSONConversions # Size conversion from JSON hash according to schema. class Size < Base - # @param size_json [Hash] - def initialize(size_json) - super() - @size_json = size_json - end - # @see Base#convert - # - # @param default [Configs::Size, nil] # @return [Configs::Size] - def convert(default = nil) - super(default || Configs::Size.new) + def convert + super(Configs::Size.new) end private - # @return [Hash] - attr_reader :size_json + alias_method :size_json, :config_json # @see Base#conversions - # - # @param _default [Configs::Size] # @return [Hash] - def conversions(_default) + def conversions { default: false, min: convert_min_size, diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb index b88e26212..153331ced 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb @@ -21,7 +21,7 @@ require "agama/storage/config_conversions/from_json_conversions/base" require "agama/storage/config_conversions/from_json_conversions/logical_volume" -require "agama/storage/configs/drive" +require "agama/storage/configs/volume_group" module Agama module Storage @@ -29,31 +29,19 @@ module ConfigConversions module FromJSONConversions # Volume group conversion from JSON hash according to schema. class VolumeGroup < Base - # @param volume_group_json [Hash] - # @param config_builder [ConfigBuilder, nil] - def initialize(volume_group_json, config_builder: nil) - super(config_builder) - @volume_group_json = volume_group_json - end - # @see Base#convert - # - # @param default [Configs::VolumeGroup, nil] # @return [Configs::VolumeGroup] - def convert(default = nil) - super(default || Configs::VolumeGroup.new) + def convert + super(Configs::VolumeGroup.new) end private - # @return [Hash] - attr_reader :volume_group_json + alias_method :volume_group_json, :config_json # @see Base#conversions - # - # @param _default [Configs::VolumeGroup] # @return [Hash] - def conversions(_default) + def conversions { name: volume_group_json[:name], extent_size: convert_extent_size, @@ -81,9 +69,7 @@ def convert_logical_volumes # @param logical_volume_json [Hash] # @return [Configs::LogicalVolume] def convert_logical_volume(logical_volume_json) - FromJSONConversions::LogicalVolume - .new(logical_volume_json, config_builder: config_builder) - .convert + FromJSONConversions::LogicalVolume.new(logical_volume_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb index 2dce8c39d..3bfdd2dd6 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_encryption.rb @@ -27,17 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for encryption conversion. module WithEncryption - # @param json [Hash] - # @param default [Configs::Encryption, nil] - # # @return [Configs::Encryption, nil] - def convert_encryption(json, default: nil) - encryption_json = json[:encryption] + def convert_encryption + encryption_json = config_json[:encryption] return unless encryption_json - FromJSONConversions::Encryption - .new(encryption_json, config_builder: config_builder) - .convert(default) + FromJSONConversions::Encryption.new(encryption_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb index 9f5e84678..9c8b60be9 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_filesystem.rb @@ -27,20 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for filesystem conversion. module WithFilesystem - # @param json [Hash] - # @param default [Configs::Filesystem, nil] - # # @return [Configs::Filesystem, nil] - def convert_filesystem(json, default: nil) - filesystem_json = json[:filesystem] + def convert_filesystem + filesystem_json = config_json[:filesystem] return unless filesystem_json - # @todo Check whether the given filesystem can be used for the mount point. - # @todo Check whether snapshots can be configured and restore to default if needed. - - FromJSONConversions::Filesystem - .new(filesystem_json, config_builder: config_builder) - .convert(default) + FromJSONConversions::Filesystem.new(filesystem_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb index 432fb9ab6..9c5dad1e5 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_partitions.rb @@ -27,10 +27,9 @@ module ConfigConversions module FromJSONConversions # Mixin for partitions conversion. module WithPartitions - # @param json [Hash] # @return [Array, nil] - def convert_partitions(json) - partitions_json = json[:partitions] + def convert_partitions + partitions_json = config_json[:partitions] return unless partitions_json partitions_json.map { |p| convert_partition(p) } @@ -39,9 +38,7 @@ def convert_partitions(json) # @param partition_json [Hash] # @return [Configs::Partition] def convert_partition(partition_json) - FromJSONConversions::Partition - .new(partition_json, config_builder: config_builder) - .convert + FromJSONConversions::Partition.new(partition_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb index d6c8d6b6f..9c50f0cd0 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_ptable_type.rb @@ -27,11 +27,9 @@ module ConfigConversions module FromJSONConversions # Mixin for partition table type conversion. module WithPtableType - # @param json [Hash] - # # @return [Y2Storage::PartitionTables::Type, nil] - def convert_ptable_type(json) - value = json[:ptableType] + def convert_ptable_type + value = config_json[:ptableType] return unless value Y2Storage::PartitionTables::Type.find(value) diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb index 7a25dbc25..cf72d7642 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_search.rb @@ -27,16 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for search conversion. module WithSearch - # @param json [Hash] - # @param default [Configs::Search, nil] - # # @return [Configs::Search, nil] - def convert_search(json, default: nil) - search_json = json[:search] + def convert_search + search_json = config_json[:search] return unless search_json - converter = FromJSONConversions::Search.new(search_json) - converter.convert(default) + FromJSONConversions::Search.new(search_json).convert end end end diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb index 33e9baa81..bde583b87 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/with_size.rb @@ -27,15 +27,12 @@ module ConfigConversions module FromJSONConversions # Mixin for size conversion. module WithSize - # @param json [Hash] - # @param default [Configs::Size, nil] - # # @return [Configs::Size, nil] - def convert_size(json, default: nil) - size_json = json[:size] + def convert_size + size_json = config_json[:size] return unless size_json - FromJSONConversions::Size.new(size_json).convert(default) + FromJSONConversions::Size.new(size_json).convert end end end diff --git a/service/lib/agama/storage/config_json_solver.rb b/service/lib/agama/storage/config_json_solver.rb index cd210b662..3f05dda4a 100644 --- a/service/lib/agama/storage/config_json_solver.rb +++ b/service/lib/agama/storage/config_json_solver.rb @@ -27,9 +27,9 @@ module Storage # Class for solving a storage JSON config. # # A storage JSON config can contain a "generate" section for automatically generating partitions - # or logical volumes according to the product definition. That section is solved by replacing it - # with the corresponding configs. The solver takes into account other paths already present in - # the rest of the config. + # or logical volumes according to the indicated default and mandatory paths. The "generate" + # section is solved by replacing it with the corresponding configs. The solver takes into + # account other paths already present in the rest of the config. # # @example # config_json = { @@ -58,9 +58,11 @@ module Storage # ] # } class ConfigJSONSolver - # @param product_config [Agama::Config] - def initialize(product_config = nil) - @product_config = product_config || Agama::Config.new + # @param default_paths [Array] Default paths of the product. + # @param mandatory_paths [Array] Mandatory paths of the product. + def initialize(default_paths: [], mandatory_paths: []) + @default_paths = default_paths + @mandatory_paths = mandatory_paths end # Solves the generate section within a given JSON config. @@ -76,12 +78,15 @@ def solve(config_json) private + # @return [Array] + attr_reader :default_paths + + # @return [Array] + attr_reader :mandatory_paths + # @return [Hash] attr_reader :config_json - # @return [Agama::Config] - attr_reader :product_config - def solve_generate configs = configs_with_generate return unless configs.any? @@ -127,11 +132,6 @@ def missing_default_paths default_paths - current_paths end - # @return [Array] - def default_paths - product_config.data.dig("storage", "volumes") || [] - end - # @return [Array] def current_paths configs_with_filesystem @@ -151,17 +151,6 @@ def missing_mandatory_paths mandatory_paths - current_paths end - # @return [Array] - def mandatory_paths - default_paths.select { |p| mandatory_path?(p) } - end - - # @param path [String] - # @return [Volume] - def mandatory_path?(path) - volume_builder.for(path).outline.required? - end - # @param config [Hash] e.g., { generate: "default" } # @param path [String] # @@ -260,11 +249,6 @@ def with_generate_value?(config, value) generate[:partitions] == value || generate[:logicalVolumes] == value end - - # @return [VolumeTemplatesBuilder] - def volume_builder - @volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config) - end end end end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index a7f5b8631..e45beb63c 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -30,267 +30,626 @@ using Y2Storage::Refinements::SizeCasts -describe Agama::Storage::ConfigConversions::FromJSON do - subject { described_class.new(config_json, product_config: product_config) } +shared_examples "without search" do |config_proc| + it "does not set #search" do + config = config_proc.call(subject.convert) + expect(config.search).to be_nil + end +end + +shared_examples "without alias" do |config_proc| + it "does not set #alias" do + config = config_proc.call(subject.convert) + expect(config.alias).to be_nil + end +end + +shared_examples "without encryption" do |config_proc| + it "does not set #encryption" do + config = config_proc.call(subject.convert) + expect(config.encryption).to be_nil + end +end + +shared_examples "without filesystem" do |config_proc| + it "does not set #filesystem" do + config = config_proc.call(subject.convert) + expect(config.filesystem).to be_nil + end +end + +shared_examples "without ptableType" do |config_proc| + it "does not set #ptable_type" do + config = config_proc.call(subject.convert) + expect(config.ptable_type).to be_nil + end +end + +shared_examples "without partitions" do |config_proc| + it "sets #partitions to the expected value" do + config = config_proc.call(subject.convert) + expect(config.partitions).to eq([]) + end +end + +shared_examples "without size" do |config_proc| + it "sets #size to default size" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(true) + expect(config.size.min).to be_nil + expect(config.size.max).to be_nil + end +end + +shared_examples "without delete" do |config_proc| + it "sets #delete to false" do + config = config_proc.call(subject.convert) + expect(config.delete?).to eq(false) + end +end + +shared_examples "without deleteIfNeeded" do |config_proc| + it "sets #delete_if_needed to false" do + config = config_proc.call(subject.convert) + expect(config.delete_if_needed?).to eq(false) + end +end + +shared_examples "with search" do |config_proc| + context "with a device name" do + let(:search) { "/dev/vda1" } - let(:product_config) { Agama::Config.new(product_data) } + it "sets #search to the expected value" do + config = config_proc.call(subject.convert) + expect(config.search).to be_a(Agama::Storage::Configs::Search) + expect(config.search.name).to eq("/dev/vda1") + expect(config.search.if_not_found).to eq(:error) + end + end + + context "with a search section" do + let(:search) do + { + condition: { name: "/dev/vda1" }, + ifNotFound: "skip" + } + end + + it "sets #search to the expected value" do + config = config_proc.call(subject.convert) + expect(config.search).to be_a(Agama::Storage::Configs::Search) + expect(config.search.name).to eq("/dev/vda1") + expect(config.search.if_not_found).to eq(:skip) + end + end +end - let(:product_data) do +shared_examples "with alias" do |config_proc| + let(:device_alias) { "test" } + + it "sets #alias to the expected value" do + config = config_proc.call(subject.convert) + expect(config.alias).to eq("test") + end +end + +shared_examples "with encryption" do |config_proc| + let(:encryption) do { - "storage" => { - "lvm" => false, - "space_policy" => "delete", - "encryption" => { - "method" => "luks2", - "pbkd_function" => "argon2id" - }, - "volumes" => ["/", "swap"], - "volume_templates" => [ - { - "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, - "btrfs" => { - "snapshots" => true, "default_subvolume" => "@", - "subvolumes" => ["home", "opt", "root", "srv"] - }, - "outline" => { - "required" => true, "snapshots_configurable" => true, - "auto_size" => { - "base_min" => "5 GiB", "base_max" => "10 GiB", - "min_fallback_for" => ["/home"], "max_fallback_for" => ["/home"], - "snapshots_increment" => "300%" - } - } - }, - { - "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" }, - "filesystem" => "xfs", "outline" => { "required" => false } - }, - { - "mount_path" => "swap", "filesystem" => "swap", - "outline" => { "required" => false } - }, - { "mount_path" => "", "filesystem" => "ext4", - "size" => { "min" => "100 MiB" } } - ] + luks2: { + password: "12345", + keySize: 256, + pbkdFunction: "argon2i", + cipher: "twofish", + label: "test" } } end - shared_examples "omitting sizes" do |result| - let(:example_configs) do - [ - { filesystem: { path: "/", type: { btrfs: { snapshots: false } } } }, - { filesystem: { path: "/home" } }, - { filesystem: { path: "/opt" } }, - { filesystem: { path: "swap" } } - ] + it "sets #encryption to the expected value" do + config = config_proc.call(subject.convert) + encryption = config.encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.key_size).to eq(256) + expect(encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2I) + expect(encryption.cipher).to eq("twofish") + expect(encryption.label).to eq("test") + end + + context "if 'encryption' only specifies 'password'" do + let(:encryption) do + { + luks2: { + password: "12345" + } + } end - it "uses default sizes" do - config = subject.convert - devices = result.call(config) - expect(devices).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - size: have_attributes(default: true, min: be_nil, max: be_nil) - ) - ) + it "sets #encryption to the expected value" do + config = config_proc.call(subject.convert) + encryption = config.encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.key_size).to be_nil + expect(encryption.pbkd_function).to be_nil + expect(encryption.cipher).to be_nil + expect(encryption.label).to be_nil end end +end - shared_examples "fixed sizes" do |result| - let(:example_configs) do - [ - { filesystem: { path: "/" }, size: "10 GiB" }, - { filesystem: { path: "/home" }, size: "6Gb" }, - { filesystem: { path: "/opt" }, size: 3221225472 }, - { filesystem: { path: "swap" }, size: "6 Gib" } - ] +shared_examples "with filesystem" do |config_proc| + let(:filesystem) do + { + reuseIfPossible: true, + type: "xfs", + label: "test", + path: "/test", + mountBy: "device", + mkfsOptions: ["version=2"], + mountOptions: ["rw"] + } + end + + it "sets #filesystem to the expected value" do + config = config_proc.call(subject.convert) + filesystem = config.filesystem + expect(filesystem).to be_a(Agama::Storage::Configs::Filesystem) + expect(filesystem.reuse?).to eq(true) + expect(filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::XFS) + expect(filesystem.type.btrfs).to be_nil + expect(filesystem.label).to eq("test") + expect(filesystem.path).to eq("/test") + expect(filesystem.mount_by).to eq(Y2Storage::Filesystems::MountByType::DEVICE) + expect(filesystem.mkfs_options).to contain_exactly("version=2") + expect(filesystem.mount_options).to contain_exactly("rw") + end + + context "if 'filesystem' specifies a 'type' with a btrfs section" do + let(:filesystem) do + { + type: { + btrfs: { + snapshots: true + } + } + } end - it "sets both min and max to the same value if a string is used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: false, min: 10.GiB, max: 10.GiB) - ) - ) + it "sets #filesystem to the expected value" do + config = config_proc.call(subject.convert) + filesystem = config.filesystem + expect(filesystem).to be_a(Agama::Storage::Configs::Filesystem) + expect(filesystem.reuse?).to eq(false) + expect(filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) + expect(filesystem.type.btrfs.snapshots?).to eq(true) + expect(filesystem.label).to be_nil + expect(filesystem.path).to be_nil + expect(filesystem.mount_by).to be_nil + expect(filesystem.mkfs_options).to eq([]) + expect(filesystem.mount_options).to eq([]) end + end - it "sets both min and max to the same value if an integer is used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: false, min: 3.GiB, max: 3.GiB) - ) - ) + context "if 'filesystem' is an empty section" do + let(:filesystem) { {} } + + it "sets #filesystem to the expected value" do + config = config_proc.call(subject.convert) + filesystem = config.filesystem + expect(filesystem).to be_a(Agama::Storage::Configs::Filesystem) + expect(filesystem.reuse?).to eq(false) + expect(filesystem.type).to be_nil + expect(filesystem.label).to be_nil + expect(filesystem.path).to be_nil + expect(filesystem.mount_by).to be_nil + expect(filesystem.mkfs_options).to eq([]) + expect(filesystem.mount_options).to eq([]) end + end +end - it "makes a difference between SI units and binary units" do - config = subject.convert - devices = result.call(config) - home_size = devices.find { |d| d.filesystem.path == "/home" }.size - swap_size = devices.find { |d| d.filesystem.path == "swap" }.size - expect(swap_size.min.to_i).to eq 6 * 1024 * 1024 * 1024 - expect(home_size.max.to_i).to eq 6 * 1000 * 1000 * 1000 +shared_examples "with ptableType" do |config_proc| + let(:ptableType) { "gpt" } + + it "sets #ptable_type to the expected value" do + config = config_proc.call(subject.convert) + expect(config.ptable_type).to eq(Y2Storage::PartitionTables::Type::GPT) + end +end + +shared_examples "with size" do |config_proc| + context "if 'size' is a string" do + let(:size) { "10 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to eq(10.GiB) end end - shared_examples "size limits" do |result| - shared_examples "limit tests" do - it "sets both min and max limits as requested if strings are used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - size: have_attributes(default: false, min: 6.GiB, max: 9.GiB) - ) - ) + context "if 'size' is a number" do + let(:size) { 3221225472 } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(3.GiB) + end + end + + shared_examples "min size" do + context "and the value is a string" do + let(:min_size) { "10 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to eq(Y2Storage::DiskSize.unlimited) end + end - it "makes a difference between SI units and binary units" do - config = subject.convert - devices = result.call(config) - home_size = devices.find { |d| d.filesystem.path == "/home" }.size - swap_size = devices.find { |d| d.filesystem.path == "swap" }.size - expect(home_size.min.to_i).to eq 6 * 1024 * 1024 * 1024 - expect(swap_size.max.to_i).to eq 6 * 1000 * 1000 * 1000 + context "and the value is a number" do + let(:min_size) { 3221225472 } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(Y2Storage::DiskSize.unlimited) end + end - it "sets both min and max limits as requested if numbers are used" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - size: have_attributes(default: false, min: 1.GiB) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: false, min: 1.GiB, max: 3.GiB) - ) - ) + context "and the value is 'current'" do + let(:min_size) { "current" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to be_nil + expect(config.size.max).to eq(Y2Storage::DiskSize.unlimited) end + end + end - it "uses unlimited for the omitted max sizes" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: false, min: 3.GiB, - max: Y2Storage::DiskSize.unlimited) - ) - ) + shared_examples "min and max sizes" do + context "and the values are strings" do + let(:min_size) { "10 GiB" } + let(:max_size) { "20 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to eq(20.GiB) end + end - it "uses nil for min size as current" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/data1"), - size: have_attributes(default: false, min: be_nil, - max: Y2Storage::DiskSize.unlimited) - ) - ) + context "and the values are numbers" do + let(:min_size) { 3221225472 } + let(:max_size) { 10737418240 } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(10.GiB) end + end - it "uses nil for max size as current" do - config = subject.convert - devices = result.call(config) - expect(devices).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/data2"), - size: have_attributes(default: false, min: 10.GiB, max: be_nil) - ) - ) + context "and the values mixes string and number" do + let(:min_size) { 3221225472 } + let(:max_size) { "10 Gib" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(3.GiB) + expect(config.size.max).to eq(10.GiB) end end - context "using a hash" do - let(:example_configs) do - [ - { - filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, - size: { min: "3 GiB" } - }, - { - filesystem: { path: "/home" }, - size: { min: "6 GiB", max: "9 GiB" } - }, - { - filesystem: { path: "swap" }, - size: { min: 1073741824, max: "6 GB" } - }, - { - filesystem: { path: "/opt" }, - size: { min: "1073741824", max: 3221225472 } - }, - { - filesystem: { path: "/data1" }, - size: { min: "current" } - }, - { - filesystem: { path: "/data2" }, - size: { min: "10 GiB", max: "current" } - } - ] + context "and the min value is 'current'" do + let(:min_size) { "current" } + let(:max_size) { "10 GiB" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to be_nil + expect(config.size.max).to eq(10.GiB) end + end - include_examples "limit tests" + context "and the max value is 'current'" do + let(:min_size) { "10 GiB" } + let(:max_size) { "current" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to eq(10.GiB) + expect(config.size.max).to be_nil + end end - context "using an array" do - let(:example_configs) do - [ - { - filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, - size: ["3 GiB"] - }, - { - filesystem: { path: "/home" }, - size: ["6 GiB", "9 GiB"] - }, - { - filesystem: { path: "swap" }, - size: [1073741824, "6 GB"] - }, - { - filesystem: { path: "/opt" }, - size: ["1073741824", 3221225472] - }, - { - filesystem: { path: "/data1" }, - size: ["current"] - }, - { - filesystem: { path: "/data2" }, - size: ["10 GiB", "current"] + context "and both values are 'current'" do + let(:min_size) { "current" } + let(:max_size) { "current" } + + it "sets #size to the expected value" do + config = config_proc.call(subject.convert) + expect(config.size.default?).to eq(false) + expect(config.size.min).to be_nil + expect(config.size.max).to be_nil + end + end + end + + context "if 'size' is an array" do + context "and only contains one value" do + let(:size) { [min_size] } + include_examples "min size" + end + + context "and contains two values" do + let(:size) { [min_size, max_size] } + include_examples "min and max sizes" + end + end + + context "if 'size' is a hash" do + context "and only specifies 'min'" do + let(:size) { { min: min_size } } + include_examples "min size" + end + + context "and specifies 'min' and 'max'" do + let(:size) do + { + min: min_size, + max: max_size + } + end + + include_examples "min and max sizes" + end + end +end + +shared_examples "with delete" do |config_proc| + it "sets #delete to true" do + config = config_proc.call(subject.convert) + expect(config.delete?).to eq(true) + end +end + +shared_examples "with deleteIfNeeded" do |config_proc| + it "sets #delete_if_needed to true" do + config = config_proc.call(subject.convert) + expect(config.delete_if_needed?).to eq(true) + end +end + +shared_examples "with partitions" do |config_proc| + let(:partitions) do + [ + partition, + { + filesystem: { path: "/test" } + } + ] + end + + let(:partition) do + { + filesystem: { path: "/" } + } + end + + context "with an empty list" do + let(:partitions) { [] } + + it "sets #partitions to empty" do + config = config_proc.call(subject.convert) + expect(config.partitions).to eq([]) + end + end + + context "with a list of partitions" do + it "sets #partitions to the expected value" do + config = config_proc.call(subject.convert) + partitions = config.partitions + expect(partitions.size).to eq(2) + + partition1, partition2 = partitions + expect(partition1).to be_a(Agama::Storage::Configs::Partition) + expect(partition1.filesystem.path).to eq("/") + expect(partition2).to be_a(Agama::Storage::Configs::Partition) + expect(partition2.filesystem.path).to eq("/test") + end + end + + partition_proc = proc { |c| config_proc.call(c).partitions.first } + + context "if a partition does not spicify 'search'" do + let(:partition) { {} } + include_examples "without search", partition_proc + end + + context "if a partition does not spicify 'alias'" do + let(:partition) { {} } + include_examples "without alias", partition_proc + end + + context "if a partition does not spicify 'id'" do + let(:partition) { {} } + + it "does not set #id" do + partition = partition_proc.call(subject.convert) + expect(partition.id).to be_nil + end + end + + context "if a partition does not spicify 'size'" do + let(:partition) { {} } + include_examples "without size", partition_proc + end + + context "if a partition does not spicify 'encryption'" do + let(:partition) { {} } + include_examples "without encryption", partition_proc + end + + context "if a partition does not spicify 'filesystem'" do + let(:partition) { {} } + include_examples "without filesystem", partition_proc + end + + context "if a partition does not spicify 'delete'" do + let(:partition) { {} } + include_examples "without delete", partition_proc + end + + context "if a partition does not spicify 'deleteIfNeeded'" do + let(:partition) { {} } + include_examples "without deleteIfNeeded", partition_proc + end + + context "if a partition specifies 'search'" do + let(:partition) { { search: search } } + include_examples "with search", partition_proc + end + + context "if a partition specifies 'alias'" do + let(:partition) { { alias: device_alias } } + include_examples "with alias", partition_proc + end + + context "if a partition spicifies 'id'" do + let(:partition) { { id: "esp" } } + + it "sets #id to the expected value" do + partition = partition_proc.call(subject.convert) + expect(partition.id).to eq(Y2Storage::PartitionId::ESP) + end + end + + context "if a partition spicifies 'size'" do + let(:partition) { { size: size } } + include_examples "with size", partition_proc + end + + context "if a partition specifies 'encryption'" do + let(:partition) { { encryption: encryption } } + include_examples "with encryption", partition_proc + end + + context "if a partition specifies 'filesystem'" do + let(:partition) { { filesystem: filesystem } } + include_examples "with filesystem", partition_proc + end + + context "if a partition specifies 'delete'" do + let(:partition) { { delete: true } } + include_examples "with delete", partition_proc + end + + context "if a partition specifies 'deleteIfNeeded'" do + let(:partition) { { deleteIfNeeded: true } } + include_examples "with deleteIfNeeded", partition_proc + end + + context "if a partition specifies 'generate'" do + let(:partition) { { generate: generate } } + + partitions_proc = proc { |c| config_proc.call(c).partitions } + include_examples "with generate", partitions_proc + + context "with a generate section" do + let(:generate) do + { + partitions: "default", + encryption: { + luks2: { password: "12345" } } - ] + } end - include_examples "limit tests" + let(:default_paths) { ["/", "swap"] } + + it "adds the expected partitions" do + partitions = config_proc.call(subject.convert).partitions + expect(partitions.size).to eq(3) + + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + test_part = partitions.find { |p| p.filesystem.path == "/test" } + + expect(root_part).to_not be_nil + expect(root_part.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(root_part.encryption.password).to eq("12345") + + expect(swap_part).to_not be_nil + expect(swap_part.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(swap_part.encryption.password).to eq("12345") + + expect(test_part).to_not be_nil + expect(test_part.encryption).to be_nil + end end end +end + +shared_examples "with generate" do |configs_proc| + context "with 'default' value" do + let(:generate) { "default" } + + let(:default_paths) { ["/default1", "/default2"] } + + it "adds volumes for the default paths" do + configs = configs_proc.call(subject.convert) + + default1 = configs.find { |c| c.filesystem.path == "/default1" } + expect(default1).to_not be_nil + expect(default1.encryption).to be_nil + + default2 = configs.find { |c| c.filesystem.path == "/default2" } + expect(default2).to_not be_nil + expect(default2.encryption).to be_nil + end + end + + context "with 'mandatory' value" do + let(:generate) { "mandatory" } + + let(:mandatory_paths) { ["/mandatory1"] } + + it "adds volumes for the mandatory paths" do + configs = configs_proc.call(subject.convert) + + mandatory1 = configs.find { |c| c.filesystem.path == "/mandatory1" } + expect(mandatory1).to_not be_nil + expect(mandatory1.encryption).to be_nil + end + end +end + +describe Agama::Storage::ConfigConversions::FromJSON do + subject do + described_class.new(config_json, default_paths: default_paths, mandatory_paths: mandatory_paths) + end + + let(:default_paths) { [] } + + let(:mandatory_paths) { [] } before do # Speed up tests by avoding real check of TPM presence. @@ -298,1202 +657,828 @@ end describe "#convert" do - using Y2Storage::Refinements::SizeCasts + let(:config_json) { {} } - context "with an empty JSON configuration" do + it "returns a storage config" do + config = subject.convert + expect(config).to be_a(Agama::Storage::Config) + end + + context "with an empty JSON" do let(:config_json) { {} } - it "generates a storage configuration" do + it "sets #boot to the expected value" do config = subject.convert - expect(config).to be_a(Agama::Storage::Config) + expect(config.boot).to be_a(Agama::Storage::Configs::Boot) + expect(config.boot.configure).to eq(true) + expect(config.boot.device).to be_nil end - it "calculates default boot settings" do + it "sets #drives to the expected value" do config = subject.convert - expect(config.boot).to be_a(Agama::Storage::Configs::Boot) - expect(config.boot.configure).to eq true - expect(config.boot.device).to eq nil + expect(config.drives).to be_empty end - # @todo Generate default drive/LVM from product descripton. - it "does not include any device in the configuration" do + it "sets #volume_groups to the expected value" do config = subject.convert expect(config.drives).to be_empty + expect(config.volume_groups).to be_empty end end - context "with some drives and boot configuration at JSON" do + context "with a JSON specifying 'boot'" do let(:config_json) do { - boot: { configure: true, device: "/dev/sdb" }, - drives: [ - { - alias: "first-disk", - ptableType: "gpt", - partitions: [ - { - alias: "root", - filesystem: { path: "/" } - } - ] - } - ] + boot: { + configure: true, + device: "/dev/sdb" + } } end - it "generates a storage configuration" do - config = subject.convert - expect(config).to be_a(Agama::Storage::Config) - end - - it "calculates the corresponding boot settings" do + it "sets #boot to the expected value" do config = subject.convert expect(config.boot).to be_a(Agama::Storage::Configs::Boot) expect(config.boot.configure).to eq true expect(config.boot.device).to eq "/dev/sdb" end + end - it "includes the corresponding drives" do - config = subject.convert - expect(config.drives.size).to eq 1 - drive = config.drives.first - expect(drive).to be_a(Agama::Storage::Configs::Drive) - expect(drive.alias).to eq "first-disk" - expect(drive.ptable_type).to eq Y2Storage::PartitionTables::Type::GPT - expect(drive.partitions.size).to eq 1 - partition = drive.partitions.first - expect(partition.alias).to eq "root" - expect(partition.filesystem.path).to eq "/" + context "with a JSON specifying 'drives'" do + let(:config_json) do + { drives: drives } end - context "omitting search for a drive" do - let(:config_json) do - { - drives: [ - { - partitions: [] - } - ] - } + let(:drives) do + [ + drive, + { alias: "second-disk" } + ] + end + + let(:drive) do + { alias: "first-disk" } + end + + context "with an empty list" do + let(:drives) { [] } + + it "sets #drives to the expected value" do + config = subject.convert + expect(config.drives).to eq([]) end + end - it "sets the default search" do + context "with a list of drives" do + it "sets #drives to the expected value" do config = subject.convert - drive = config.drives.first + expect(config.drives.size).to eq(2) + expect(config.drives).to all(be_a(Agama::Storage::Configs::Drive)) + + drive1, drive2 = config.drives + expect(drive1.alias).to eq("first-disk") + expect(drive1.partitions).to eq([]) + expect(drive2.alias).to eq("second-disk") + expect(drive2.partitions).to eq([]) + end + end + + drive_proc = proc { |c| c.drives.first } + + context "if a drive does not specify 'search'" do + let(:drive) { {} } + + it "sets #search to the expected value" do + drive = drive_proc.call(subject.convert) expect(drive.search).to be_a(Agama::Storage::Configs::Search) expect(drive.search.name).to be_nil expect(drive.search.if_not_found).to eq(:error) end end - context "specifying search for a drive" do - let(:config_json) do - { - drives: [ - { - search: search, - partitions: [] - } - ] - } - end + context "if a drive does not spicify 'alias'" do + let(:drive) { {} } + include_examples "without alias", drive_proc + end - context "with a device name" do - let(:search) { "/dev/vda" } + context "if a drive does not spicify 'encryption'" do + let(:drive) { {} } + include_examples "without encryption", drive_proc + end - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - expect(drive.search).to be_a(Agama::Storage::Configs::Search) - expect(drive.search.name).to eq("/dev/vda") - expect(drive.search.if_not_found).to eq(:error) - end - end + context "if a drive does not spicify 'filesystem'" do + let(:drive) { {} } + include_examples "without filesystem", drive_proc + end - context "with a search section" do - let(:search) do - { - condition: { name: "/dev/vda" }, - ifNotFound: "skip" - } - end + context "if a drive does not spicify 'ptableType'" do + let(:drive) { {} } + include_examples "without ptableType", drive_proc + end - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - expect(drive.search).to be_a(Agama::Storage::Configs::Search) - expect(drive.search.name).to eq("/dev/vda") - expect(drive.search.if_not_found).to eq(:skip) - end - end + context "if a drive does not spicify 'partitions'" do + let(:drive) { {} } + include_examples "without partitions", drive_proc + end + + context "if a drive specifies 'search'" do + let(:drive) { { search: search } } + include_examples "with search", drive_proc + end + + context "if a drive specifies 'alias'" do + let(:drive) { { alias: device_alias } } + include_examples "with alias", drive_proc + end + + context "if a drive specifies 'encryption'" do + let(:drive) { { encryption: encryption } } + include_examples "with encryption", drive_proc + end + + context "if a drive specifies 'filesystem'" do + let(:drive) { { filesystem: filesystem } } + include_examples "with filesystem", drive_proc + end + + context "if a drive specifies 'ptableType'" do + let(:drive) { { ptableType: ptableType } } + include_examples "with ptableType", drive_proc + end + + context "if a drive specifies 'partitions'" do + let(:drive) { { partitions: partitions } } + include_examples "with partitions", drive_proc end end - context "specifying a filesystem for a drive" do + context "with a JSON specifying 'volumeGroups'" do let(:config_json) do - { - drives: [{ filesystem: filesystem }] - } + { volumeGroups: volume_groups } end - let(:filesystem) do - { - reuseIfPossible: true, - path: "/", - type: "xfs", - label: "root", - mkfsOptions: ["version=2"], - mountOptions: ["rw"], - mountBy: "label" - } + let(:volume_groups) do + [ + volume_group, + { name: "vg2" } + ] end - it "uses the specified attributes" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.reuse?).to eq true - expect(filesystem.path).to eq "/" - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::XFS - expect(filesystem.label).to eq "root" - expect(filesystem.mkfs_options).to eq ["version=2"] - expect(filesystem.mount_options).to eq ["rw"] - expect(filesystem.mount_by).to eq Y2Storage::Filesystems::MountByType::LABEL - end + let(:volume_group) { { name: "vg1" } } - context "if the filesystem specification only contains a path" do - let(:filesystem) { { path: "/" } } + context "with an empty list" do + let(:volume_groups) { [] } - it "uses the default type and btrfs attributes for that path" do + it "sets #volume_groups to the expected value" do config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.reuse?).to eq false - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - expect(filesystem.type.btrfs.snapshots).to eq true - expect(filesystem.type.btrfs.default_subvolume).to eq "@" - expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["home", "opt", "root", "srv"] + expect(config.volume_groups).to eq([]) end end - context "if the filesystem specification contains some btrfs settings" do - let(:filesystem) do - { path: "/", - type: { btrfs: { snapshots: false, default_subvolume: "", subvolumes: ["tmp"] } } } - end - - it "uses the specified btrfs attributes" do + context "with a list of volume groups" do + it "sets #volume_groups to the expected value" do config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - expect(filesystem.type.btrfs.snapshots).to eq false - # TODO: none of the following attributes are specified at the schema. Intentional? - # expect(filesystem.type.btrfs.default_subvolume).to eq "" - # expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["tmp"] + expect(config.volume_groups.size).to eq(2) + expect(config.volume_groups).to all(be_a(Agama::Storage::Configs::VolumeGroup)) + + volume_group1, volume_group2 = config.volume_groups + expect(volume_group1.name).to eq("vg1") + expect(volume_group1.logical_volumes).to eq([]) + expect(volume_group2.name).to eq("vg2") + expect(volume_group2.logical_volumes).to eq([]) end + end - context "and the default filesystem type is not btrfs" do - let(:filesystem) do - { path: "/home", type: { btrfs: { snapshots: false } } } - end + vg_proc = proc { |c| c.volume_groups.first } - it "uses btrfs filesystem" do - config = subject.convert - filesystem = config.drives.first.filesystem - expect(filesystem.type.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS - end + context "if a volume group does not spicify 'name'" do + let(:volume_group) { {} } + + it "does not set #name" do + vg = vg_proc.call(subject.convert) + expect(vg.name).to be_nil end end - end - context "omitting search for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { - path: "/" - } - } - ] - } - ] - } + context "if a volume group does not spicify 'extentSize'" do + let(:volume_group) { {} } + + it "does not set #extent_size" do + vg = vg_proc.call(subject.convert) + expect(vg.extent_size).to be_nil + end end - it "does not set a search" do - config = subject.convert - drive = config.drives.first - partition = drive.partitions.first - expect(partition.search).to be_nil + context "if a volume group does not spicify 'physicalVolumes'" do + let(:volume_group) { {} } + + it "sets #physical_volumes to the expected vale" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to eq([]) + end end - end - context "specifying search for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - search: search, - filesystem: { - path: "/" - } - } - ] - } - ] - } + context "if a volume group does not spicify 'logicalVolumes'" do + let(:volume_group) { {} } + + it "sets #logical_volumes to the expected vale" do + vg = vg_proc.call(subject.convert) + expect(vg.logical_volumes).to eq([]) + end end - context "with a device name" do - let(:search) { "/dev/vda1" } + context "if a volume group spicifies 'name'" do + let(:volume_group) { { name: "test" } } - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - partition = drive.partitions.first - expect(partition.search).to be_a(Agama::Storage::Configs::Search) - expect(partition.search.name).to eq("/dev/vda1") - expect(partition.search.if_not_found).to eq(:error) + it "sets #name to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.name).to eq("test") end end - context "with a search section" do - let(:search) do - { - condition: { name: "/dev/vda1" }, - ifNotFound: "skip" - } + context "if a volume group spicifies 'extentSize'" do + let(:volume_group) { { extentSize: size } } + + context "if 'extentSize' is a string" do + let(:size) { "4 KiB" } + + it "sets #extent_size to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.extent_size).to eq(4.KiB) + end end - it "sets the expected search" do - config = subject.convert - drive = config.drives.first - partition = drive.partitions.first - expect(partition.search).to be_a(Agama::Storage::Configs::Search) - expect(partition.search.name).to eq("/dev/vda1") - expect(partition.search.if_not_found).to eq(:skip) + context "if 'extentSize' is a number" do + let(:size) { 4096 } + + it "sets #extent_size to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.extent_size).to eq(4.KiB) + end end end - end - context "setting delete for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - search: "/dev/vda1", - delete: true - }, - { - filesystem: { path: "/" } - } - ] - } - ] - } - end + context "if a volume group spicifies 'physicalVolumes'" do + let(:volume_group) { { physicalVolumes: physical_volumes } } - it "sets #delete to true" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - search: have_attributes(name: "/dev/vda1"), - delete: true, - delete_if_needed: false - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - delete: false, - delete_if_needed: false - ) - ) - end - end + context "with an empty list" do + let(:physical_volumes) { [] } - context "setting delete if needed for a partition" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - search: "/dev/vda1", - deleteIfNeeded: true - }, - { - filesystem: { path: "/" } - } - ] - } - ] - } - end + it "sets #physical_volumes to empty" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to eq([]) + end + end - it "sets #delete_if_needed to true" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - search: have_attributes(name: "/dev/vda1"), - delete: false, - delete_if_needed: true - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - delete: false, - delete_if_needed: false - ) - ) + context "with a list of aliases" do + let(:physical_volumes) { ["pv1", "pv2"] } + + it "sets #physical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") + end + end end - end - context "omitting sizes for the partitions" do - let(:config_json) do - { - drives: [ - { - partitions: example_configs - } + context "if a volume group spicifies 'logicalVolumes'" do + let(:volume_group) { { logicalVolumes: logical_volumes } } + + let(:logical_volumes) do + [ + logical_volume, + { name: "test" } ] - } - end + end - result = proc { |config| config.drives.first.partitions } + let(:logical_volume) { { name: "root" } } - include_examples "omitting sizes", result - end + context "with an empty list" do + let(:logical_volumes) { [] } - context "setting fixed sizes for the partitions" do - let(:config_json) do - { - drives: [ - { - partitions: example_configs - } - ] - } - end + it "sets #logical_volumes to empty" do + vg = vg_proc.call(subject.convert) + expect(vg.logical_volumes).to eq([]) + end + end - result = proc { |config| config.drives.first.partitions } + context "with a list of logical volumes" do + it "sets #logical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + lvs = vg.logical_volumes + expect(lvs.size).to eq(2) + + lv1, lv2 = lvs + expect(lv1).to be_a(Agama::Storage::Configs::LogicalVolume) + expect(lv1.name).to eq("root") + expect(lv2).to be_a(Agama::Storage::Configs::LogicalVolume) + expect(lv2.name).to eq("test") + end + end - include_examples "fixed sizes", result - end + lv_proc = proc { |c| c.volume_groups.first.logical_volumes.first } - context "specifying size limits for the partitions" do - let(:config_json) do - { - drives: [ - { - partitions: example_configs - } - ] - } - end + context "if a logical volume does not specify 'name'" do + let(:logical_volume) { {} } - result = proc { |config| config.drives.first.partitions } + it "does not set #name" do + lv = lv_proc.call(subject.convert) + expect(lv.name).to be_nil + end + end - include_examples "size limits", result - end + context "if a logical volume does not specify 'stripes'" do + let(:logical_volume) { {} } - context "configuring partial information for several mount points" do - let(:config_json) { { drives: [{ partitions: partitions }] } } - let(:partitions) do - [ - { filesystem: { path: "/" } }, - { filesystem: { path: "swap" } }, - { filesystem: { path: "/opt" } } - ] - end + it "does not set #stripes" do + lv = lv_proc.call(subject.convert) + expect(lv.stripes).to be_nil + end + end - it "configures the filesystem types according to the product configuration" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes( - path: "/", type: have_attributes(fs_type: Y2Storage::Filesystems::Type::BTRFS) - ) - ), - an_object_having_attributes( - filesystem: have_attributes( - path: "swap", type: have_attributes(fs_type: Y2Storage::Filesystems::Type::SWAP) - ) - ), - an_object_having_attributes( - filesystem: have_attributes( - path: "/opt", type: have_attributes(fs_type: Y2Storage::Filesystems::Type::EXT4) - ) - ) - ) - end - end + context "if a logical volume does not specify 'stripeSize'" do + let(:logical_volume) { {} } - context "when some partition is configured to be encrypted" do - let(:config_json) do - { - drives: [{ partitions: partitions }] - } - end + it "does not set #stripe_size" do + lv = lv_proc.call(subject.convert) + expect(lv.stripe_size).to be_nil + end + end - let(:partitions) do - [ - { - id: "linux", size: { min: "10 GiB" }, - filesystem: { type: "xfs", path: "/home" }, - encryption: encryption_home - }, - { - size: { min: "2 GiB" }, - filesystem: { type: "swap", path: "swap" }, - encryption: encryption_swap - } - ] - end + context "if a logical volume does not specify 'pool'" do + let(:logical_volume) { {} } + + it "sets #pool? to false" do + lv = lv_proc.call(subject.convert) + expect(lv.pool?).to eq(false) + end + end + + context "if a logical volume does not specify 'usedPool'" do + let(:logical_volume) { {} } + + it "does not set #used_pool" do + lv = lv_proc.call(subject.convert) + expect(lv.used_pool).to be_nil + end + end + + context "if a logical volume does not specify 'alias'" do + let(:logical_volume) { {} } + include_examples "without alias", lv_proc + end + + context "if a logical volume does not specify 'size'" do + let(:logical_volume) { {} } + include_examples "without size", lv_proc + end + + context "if a logical volume does not specify 'encryption'" do + let(:logical_volume) { {} } + include_examples "without encryption", lv_proc + end + + context "if a logical volume does not specify 'filesystem'" do + let(:logical_volume) { {} } + include_examples "without filesystem", lv_proc + end + + context "if a logical volume specifies 'stripes'" do + let(:logical_volume) { { stripes: 10 } } + + it "sets #stripes to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.stripes).to eq(10) + end + end + + context "if a logical volume specifies 'stripeSize'" do + let(:logical_volume) { { stripeSize: size } } + + context "if 'stripeSize' is a string" do + let(:size) { "4 KiB" } + + it "sets #stripe_size to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.stripe_size).to eq(4.KiB) + end + end + + context "if 'stripeSize' is a number" do + let(:size) { 4096 } + + it "sets #stripe_size to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.stripe_size).to eq(4.KiB) + end + end + end + + context "if a logical volume specifies 'pool'" do + let(:logical_volume) { { pool: true } } + + it "sets #pool? to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.pool?).to eq(true) + end + end + + context "if a logical volume specifies 'usedPool'" do + let(:logical_volume) { { usedPool: "pool" } } + + it "sets #used_pool to the expected value" do + lv = lv_proc.call(subject.convert) + expect(lv.used_pool).to eq("pool") + end + end + + context "if a logical volume specifies 'alias'" do + let(:logical_volume) { { alias: device_alias } } + include_examples "with alias", lv_proc + end + + context "if a logical volume specifies 'size'" do + let(:logical_volume) { { size: size } } + include_examples "with size", lv_proc + end + + context "if a logical volume specifies 'encryption'" do + let(:logical_volume) { { encryption: encryption } } + include_examples "with encryption", lv_proc + end + + context "if a logical volume specifies 'filesystem'" do + let(:logical_volume) { { filesystem: filesystem } } + include_examples "with filesystem", lv_proc + end + + context "if a logical volume specifies 'generate'" do + let(:logical_volume) { { generate: generate } } + + logical_volumes_proc = proc { |c| c.volume_groups.first.logical_volumes } + include_examples "with generate", logical_volumes_proc - let(:encryption_home) do - { luks2: { password: "notsecret", keySize: 256 } } - end + context "with a generate section" do + let(:generate) do + { + logicalVolumes: "default", + encryption: { + luks2: { password: "12345" } + }, + stripes: 8, + stripeSize: "16 KiB" + } + end - let(:encryption_swap) { nil } + let(:default_paths) { ["/", "swap"] } - it "sets the encryption settings for the corresponding partition" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - encryption: have_attributes( - password: "notsecret", method: Y2Storage::EncryptionMethod::LUKS2, key_size: 256 - ) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - encryption: nil - ) - ) - end + it "adds the expected logical volumes" do + lvs = subject.convert.volume_groups.first.logical_volumes + expect(lvs.size).to eq(3) - context "if only the password is provided" do - let(:encryption_home) { { luks2: { password: "notsecret" } } } - let(:encryption_swap) { nil } + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + test_lv = lvs.find { |v| v.name == "test" } - it "uses the default derivation function" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - encryption: have_attributes( - password: "notsecret", - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::ARGON2ID - ) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - encryption: nil - ) - ) - end - end + expect(root_lv).to_not be_nil + expect(root_lv.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(root_lv.encryption.password).to eq("12345") - context "if random encryption is configured for swap" do - let(:encryption_home) { nil } - let(:encryption_swap) { "random_swap" } + expect(swap_lv).to_not be_nil + expect(swap_lv.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(swap_lv.encryption.password).to eq("12345") - it "sets the corresponding configuration" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - encryption: nil - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - encryption: have_attributes( - password: nil, - label: nil, - cipher: nil, - method: Y2Storage::EncryptionMethod::RANDOM_SWAP - ) - ) - ) + expect(test_lv).to_not be_nil + expect(test_lv.encryption).to be_nil + end + end end end end - context "when the id of some partition is specified" do + context "generating partitions" do let(:config_json) do { - drives: [{ partitions: partitions }] + drives: drives, + volumeGroups: volume_groups } end - let(:partitions) do - [ - { - id: "Esp", size: { min: "10 GiB" }, - filesystem: { type: "xfs", path: "/home" } - }, - { - size: { min: "2 GiB" }, - filesystem: { type: "swap", path: "swap" } - } - ] - end + let(:drives) { [] } - it "configures the corresponding id" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - id: Y2Storage::PartitionId::ESP - ), - an_object_having_attributes( - filesystem: have_attributes(path: "swap"), - id: nil - ) - ) - end - end + let(:volume_groups) { [] } - context "with some LVM volume groups" do - let(:config_json) do - { - volumeGroups: [ + let(:default_paths) { ["/", "swap", "/home"] } + + let(:mandatory_paths) { ["/", "swap"] } + + context "if the device already specifies any of the partitions" do + let(:drives) do + [ { - name: "vg0", - extentSize: "2 MiB", - physicalVolumes: ["alias1", "alias2"], - logicalVolumes: [ - { - name: "root", - filesystem: { path: "/" }, - encryption: { - luks2: { password: "12345" } - } - }, - { - alias: "thin-pool", - name: "pool", - pool: true, - size: "100 GiB", - stripes: 10, - stripeSize: "4 KiB" - }, - { - name: "data", - size: "50 GiB", - usedPool: "thin-pool", - filesystem: { type: "xfs" } - } + partitions: [ + { generate: "default" }, + { filesystem: { path: "/home" } } ] - }, - { - name: "vg1" } ] - } - end - - it "generates the corresponding volume groups and logical volumes" do - config = subject.convert + end - expect(config.volume_groups).to contain_exactly( - an_object_having_attributes( - name: "vg0", - extent_size: 2.MiB, - physical_volumes: ["alias1", "alias2"] - ), - an_object_having_attributes( - name: "vg1", - extent_size: be_nil, - physical_volumes: be_empty, - logical_volumes: be_empty - ) - ) - - logical_volumes = config.volume_groups - .find { |v| v.name == "vg0" } - .logical_volumes - - expect(logical_volumes).to include( - an_object_having_attributes( - alias: be_nil, - name: "root", - encryption: have_attributes( - password: "12345", - method: Y2Storage::EncryptionMethod::LUKS2, - pbkd_function: Y2Storage::PbkdFunction::ARGON2ID - ), - filesystem: have_attributes( - path: "/", - type: have_attributes( - fs_type: Y2Storage::Filesystems::Type::BTRFS - ) - ), - size: have_attributes( - default: true, - min: be_nil, - max: be_nil - ), - stripes: be_nil, - stripe_size: be_nil, - pool: false, - used_pool: be_nil - ), - an_object_having_attributes( - alias: "thin-pool", - name: "pool", - encryption: be_nil, - filesystem: be_nil, - size: have_attributes( - default: false, - min: 100.GiB, - max: 100.GiB - ), - stripes: 10, - stripe_size: 4.KiB, - pool: true, - used_pool: be_nil - ), - an_object_having_attributes( - alias: be_nil, - name: "data", - encryption: be_nil, - filesystem: have_attributes( - path: be_nil, - type: have_attributes( - fs_type: Y2Storage::Filesystems::Type::XFS - ) - ), - size: have_attributes( - default: false, - min: 50.GiB, - max: 50.GiB - ), - stripes: be_nil, - stripe_size: be_nil, - pool: false, - used_pool: "thin-pool" - ) - ) + it "only adds partitions for the the missing paths" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions.size).to eq(3) + + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + home_part = partitions.find { |p| p.filesystem.path == "/home" } + expect(root_part).to_not be_nil + expect(swap_part).to_not be_nil + expect(home_part).to_not be_nil + end end - end - context "omitting sizes for the logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if other device already specifies any of the partitions" do + let(:drives) do + [ { - logicalVolumes: example_configs + partitions: [ + { generate: "default" } + ] + }, + { + partitions: [ + { filesystem: { path: "/home" } } + ] } ] - } - end + end - result = proc { |config| config.volume_groups.first.logical_volumes } + it "only adds partitions for the the missing paths" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions.size).to eq(2) - include_examples "omitting sizes", result - end + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + expect(root_part).to_not be_nil + expect(swap_part).to_not be_nil + end + end - context "setting fixed sizes for the logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if a volume group already specifies any of the paths" do + let(:drives) do + [ { - logicalVolumes: example_configs + partitions: [ + { generate: "mandatory" } + ] } ] - } - end - - result = proc { |config| config.volume_groups.first.logical_volumes } - - include_examples "fixed sizes", result - end + end - context "specifying size limits for the logical volumes" do - let(:config_json) do - { - volumeGroups: [ + let(:volume_groups) do + [ { - logicalVolumes: example_configs + logicalVolumes: [ + { filesystem: { path: "swap" } } + ] } ] - } - end + end - result = proc { |config| config.volume_groups.first.logical_volumes } + it "only adds partitions for the the missing paths" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions.size).to eq(1) - include_examples "size limits", result - end + root_part = partitions.find { |p| p.filesystem.path == "/" } + expect(root_part).to_not be_nil + end + end - context "using 'generate' with 'default' for partitions in a drive" do - let(:config_json) do - { - drives: [ + context "if the device specifies several partitions with 'generate'" do + let(:drives) do + [ { partitions: [ + { generate: "mandatory" }, { generate: "default" } ] } ] - } - end - - it "includes the default partitions defined by the product" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(true) - end - - context "if the drive already defines some of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" }, - { - filesystem: { path: "swap" }, - size: "2 GiB" - } - ] - } - ] - } end - it "only includes the missing default partitions" do + it "only adds partitions for the first 'generate'" do config = subject.convert partitions = config.drives.first.partitions - expect(partitions.size).to eq(2) - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(false) - expect(swap.size.min).to eq(2.GiB) - expect(swap.size.max).to eq(2.GiB) + root_part = partitions.find { |p| p.filesystem.path == "/" } + swap_part = partitions.find { |p| p.filesystem.path == "swap" } + expect(root_part).to_not be_nil + expect(swap_part).to_not be_nil end end - context "if there are more than one 'generate'" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" }, - { generate: "default" } - ] - } - ] - } + context "if several devices specify partitions with 'generate'" do + let(:drives) do + [ + { + partitions: [ + { generate: "mandatory" } + ] + }, + { + partitions: [ + { generate: "default" } + ] + } + ] end - it "does not include the same partition twice" do + it "only adds partitions to the first device with a 'generate'" do config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap).to_not be_nil + drive1, drive2 = config.drives + expect(drive1.partitions.size).to eq(2) + expect(drive2.partitions.size).to eq(0) end end + end - context "if there is a 'generate' with 'mandatory'" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" }, - { generate: "mandatory" } - ] - } - ] - } - end + context "generating logical volumes" do + let(:config_json) do + { + drives: drives, + volumeGroups: volume_groups + } + end - it "does not include the same partition twice" do - config = subject.convert - partitions = config.drives.first.partitions + let(:drives) { [] } - expect(partitions.size).to eq(2) + let(:volume_groups) { [] } - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil + let(:default_paths) { ["/", "swap", "/home"] } - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap).to_not be_nil - end - end + let(:mandatory_paths) { ["/", "swap"] } - context "if other drive already defines some of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" } - ] - }, - { - partitions: [ - { - filesystem: { path: "swap" }, - size: "2 GiB" - } - ] - } - ] - } + context "if the volume group already specifies any of the logical volumes" do + let(:volume_groups) do + [ + { + logicalVolumes: [ + { generate: "default" }, + { filesystem: { path: "/home" } } + ] + } + ] end - it "only includes the missing default partitions" do + it "only adds logical volumes for the the missing paths" do config = subject.convert - partitions0 = config.drives[0].partitions - partitions1 = config.drives[1].partitions - - expect(partitions0.size).to eq(1) - - root = partitions0.first - expect(root.filesystem.path).to eq("/") - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - expect(partitions1.size).to eq(1) - - swap = partitions1.first - expect(swap.filesystem.path).to eq("swap") - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(false) - expect(swap.size.min).to eq(2.GiB) - expect(swap.size.max).to eq(2.GiB) + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(3) + + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + home_lv = lvs.find { |v| v.filesystem.path == "/home" } + expect(root_lv).to_not be_nil + expect(swap_lv).to_not be_nil + expect(home_lv).to_not be_nil end end - context "if other drive also contains a 'generate'" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "default" } - ] - }, - { - partitions: [ - { generate: "default" } - ] - } - ] - } + context "if other volume group already specifies any of the logical volumes" do + let(:volume_groups) do + [ + { + logicalVolumes: [ + { generate: "default" } + ] + }, + { + logicalVolumes: [ + { filesystem: { path: "/home" } } + ] + } + ] end - it "only includes the default partitions in the first drive" do + it "only adds logical volumes for the the missing paths" do config = subject.convert - partitions0 = config.drives[0].partitions - partitions1 = config.drives[1].partitions + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(2) - expect(partitions0.size).to eq(2) - - root = partitions0.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil - - swap = partitions0.find { |p| p.filesystem.path == "swap" } - expect(swap).to_not be_nil - - expect(partitions1.size).to eq(0) + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + expect(root_lv).to_not be_nil + expect(swap_lv).to_not be_nil end end - end - context "using 'generate' with more properties for partitions in a drive" do - let(:config_json) do - { - drives: [ + context "if a device already specifies a partition for any of the paths" do + let(:drives) do + [ { partitions: [ - { - generate: { - partitions: "default", - encryption: { - luks1: { password: "12345" } - } - } - } + { filesystem: { path: "swap" } } ] } ] - } - end - - it "includes the default partitions defined by the product with the given properties" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - expect(root.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) - expect(root.encryption.password).to eq("12345") - - swap = partitions.find { |p| p.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(true) - expect(swap.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) - expect(swap.encryption.password).to eq("12345") - end - end + end - context "using 'generate' with 'mandatory' for partitions in a drive" do - let(:config_json) do - { - drives: [ + let(:volume_groups) do + [ { - partitions: [ + logicalVolumes: [ { generate: "mandatory" } ] } ] - } - end - - it "includes the mandatory partitions defined by the product" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(1) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - end - - context "if other device already defines some of the mandatory paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { generate: "mandatory" } - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { - filesystem: { path: "/" } - } - ] - } - ] - } end - it "does not include the already defined mandatory paths" do + it "only adds logical volumes for the the missing paths" do config = subject.convert - partitions = config.drives.first.partitions - logical_volumes = config.volume_groups.first.logical_volumes + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(1) - expect(partitions.size).to eq(0) + root_lv = lvs.find { |v| v.filesystem.path == "/" } + expect(root_lv).to_not be_nil end end - end - context "using 'generate' with 'default' for logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if the volume group specifies several logical volumes with 'generate'" do + let(:volume_groups) do + [ { logicalVolumes: [ + { generate: "mandatory" }, { generate: "default" } ] } ] - } - end - - it "includes the default logical volumes defined by the product" do - config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(2) - - root = logical_volumes.find { |v| v.filesystem.path == "/" } - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - - swap = logical_volumes.find { |v| v.filesystem.path == "swap" } - expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) - expect(swap.size.default?).to eq(true) - end - - context "if other device already defines any of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/" } - } - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { generate: "default" } - ] - } - ] - } end - it "does not include the already defined default paths" do + it "only adds logical volumes for the first 'generate'" do config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(1) + lvs = config.volume_groups.first.logical_volumes + expect(lvs.size).to eq(2) - swap = logical_volumes.first - expect(swap.filesystem.path).to eq("swap") + root_lv = lvs.find { |v| v.filesystem.path == "/" } + swap_lv = lvs.find { |v| v.filesystem.path == "swap" } + expect(root_lv).to_not be_nil + expect(swap_lv).to_not be_nil end end - end - context "using 'generate' with 'mandatory' for logical volumes" do - let(:config_json) do - { - volumeGroups: [ + context "if several volume groups specify logical volumes with 'generate'" do + let(:volume_groups) do + [ { logicalVolumes: [ { generate: "mandatory" } ] + }, + { + logicalVolumes: [ + { generate: "default" } + ] } ] - } - end - - it "includes the mandatory logical volumes defined by the product" do - config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(1) - - root = logical_volumes.first - expect(root.filesystem.path).to eq("/") - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - end - - context "if other device already defines any of the mandatory paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/" } - } - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { generate: "mandatory" } - ] - } - ] - } end - it "does not include the already defined mandatory paths" do + it "only adds logical volumes to the first volume group with a 'generate'" do config = subject.convert - logical_volumes = config.volume_groups.first.logical_volumes - - expect(logical_volumes.size).to eq(0) + vg1, vg2 = config.volume_groups + expect(vg1.logical_volumes.size).to eq(2) + expect(vg2.logical_volumes.size).to eq(0) end end - end - context "using both 'generate' with 'default' and with 'mandatory'" do - let(:config_json) do - { - drives: [ + context "if a drive specifies a partition with 'generate'" do + let(:drives) do + [ { partitions: [ - first_generate, - second_generate + { generate: "mandatory" } ] } ] - } - end - - context "if 'default' appears first" do - let(:first_generate) { { generate: "default" } } - let(:second_generate) { { generate: "mandatory" } } - - it "includes the default partitions defined by the product" do - config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(2) - - root = partitions.find { |p| p.filesystem.path == "/" } - swap = partitions.find { |p| p.filesystem.path == "swap" } - - expect(root).to_not be_nil - expect(swap).to_not be_nil end - end - context "if 'mandatory' appears first" do - let(:first_generate) { { generate: "mandatory" } } - let(:second_generate) { { generate: "default" } } + let(:volume_groups) do + [ + { + logicalVolumes: [ + { generate: "mandatory" } + ] + } + ] + end - it "includes the mandatory partitions defined by the product" do + it "does not add logical volumes to the volume group" do config = subject.convert - partitions = config.drives.first.partitions - - expect(partitions.size).to eq(1) - - root = partitions.find { |p| p.filesystem.path == "/" } - expect(root).to_not be_nil + vg = config.volume_groups.first + expect(vg.logical_volumes.size).to eq(0) end end end From b751c4ceee109b2fb7cce58ea753d02ad91b9523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 3 Oct 2024 12:40:43 +0100 Subject: [PATCH 04/21] storage: adapt proposal to new JSON conversion API --- service/lib/agama/storage/proposal.rb | 9 ++++++--- service/test/agama/dbus/storage/manager_test.rb | 4 ++-- service/test/y2storage/agama_proposal_test.rb | 6 +++++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index f35a869c3..572458ce8 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -234,9 +234,12 @@ def calculate_guided_from_json(guided_json) # @param storage_json [Hash] e.g., { "drives": [] }. # @return [Boolean] Whether the proposal successes. def calculate_agama_from_json(storage_json) - storage_config = ConfigConversions::FromJSON - .new(storage_json, product_config: config) - .convert + storage_config = ConfigConversions::FromJSON.new( + storage_json, + default_paths: config.default_paths, + mandatory_paths: config.mandatory_paths + ).convert + calculate_agama(storage_config) end diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 89cafd221..f099957bd 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -592,10 +592,10 @@ def serialize(value) context "if an agama proposal has been calculated" do before do - proposal.calculate_agama(config) + proposal.calculate_agama(storage_config) end - let(:config) do + let(:storage_config) do fs_type = Agama::Storage::Configs::FilesystemType.new.tap do |t| t.fs_type = Y2Storage::Filesystems::Type::BTRFS end diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index e31e7902a..7d97b4d17 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -83,7 +83,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) let(:config_from_json) do Agama::Storage::ConfigConversions::FromJSON - .new(config_json, product_config: product_config) + .new(config_json, default_paths: default_paths, mandatory_paths: mandatory_paths) .convert end @@ -137,6 +137,10 @@ def partition_config(name: nil, filesystem: nil, size: nil) } end + let(:default_paths) { product_config.default_paths } + + let(:mandatory_paths) { product_config.mandatory_paths } + let(:issues_list) { [] } let(:drives) { [drive0] } From 1ff12350f88162ace9a31f0931aa87ed0ad72b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 4 Oct 2024 10:42:45 +0100 Subject: [PATCH 05/21] storage: add tests for config solver --- .../test/agama/storage/config_solver_test.rb | 728 ++++++++++++++++++ 1 file changed, 728 insertions(+) create mode 100644 service/test/agama/storage/config_solver_test.rb diff --git a/service/test/agama/storage/config_solver_test.rb b/service/test/agama/storage/config_solver_test.rb new file mode 100644 index 000000000..278e3a1a2 --- /dev/null +++ b/service/test/agama/storage/config_solver_test.rb @@ -0,0 +1,728 @@ +# 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 "./storage_helpers" +require "agama/config" +require "agama/storage/config_conversions/from_json" +require "agama/storage/config_solver" +require "y2storage" +require "y2storage/refinements" + +using Y2Storage::Refinements::SizeCasts + +describe Agama::Storage::ConfigSolver do + include Agama::RSpec::StorageHelpers + + let(:product_data) do + { + "storage" => { + "lvm" => false, + "space_policy" => "delete", + "encryption" => { + "method" => "luks2", + "pbkd_function" => "argon2i" + }, + "volumes" => ["/", "swap"], + "volume_templates" => [ + { + "mount_path" => "/", "filesystem" => "btrfs", + "size" => { "auto" => true, "min" => "5 GiB", "max" => "10 GiB" }, + "btrfs" => { + "snapshots" => true, "default_subvolume" => "@", + "subvolumes" => ["home", "opt", "root", "srv"] + }, + "outline" => { + "required" => true, "snapshots_configurable" => true, + "auto_size" => { + "base_min" => "5 GiB", "base_max" => "10 GiB", + "min_fallback_for" => min_fallbacks, "max_fallback_for" => max_fallbacks, + "snapshots_increment" => snapshots_increment + } + } + }, + { + "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" }, + "filesystem" => "xfs", "outline" => { "required" => false } + }, + { + "mount_path" => "swap", "filesystem" => "swap", "size" => { "auto" => true }, + "outline" => { + "auto_size" => { + "adjust_by_ram" => true, + "base_min" => "2 GiB", + "base_max" => "4 GiB" + } + } + }, + { "mount_path" => "", "filesystem" => "ext4", + "size" => { "min" => "100 MiB" } } + ] + } + } + end + + let(:min_fallbacks) { [] } + + let(:max_fallbacks) { [] } + + let(:snapshots_increment) { nil } + + let(:product_config) { Agama::Config.new(product_data) } + + let(:default_paths) { product_config.default_paths } + + let(:mandatory_paths) { product_config.mandatory_paths } + + let(:config_json) { nil } + + let(:config) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json) + .convert + end + + let(:devicegraph) { Y2Storage::StorageManager.instance.probed } + + before do + mock_storage(devicegraph: scenario) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE) + .to(receive(:possible?)) + .and_return(true) + end + + subject { described_class.new(devicegraph, product_config) } + + describe "#solve" do + let(:scenario) { "empty-hd-50GiB.yaml" } + + context "if a config does not specify all the encryption properties" do + let(:config_json) do + { + drives: [ + { + encryption: { + luks2: { password: "12345" } + } + } + ] + } + end + + it "completes the encryption config according to the product info" do + subject.solve(config) + + drive = config.drives.first + encryption = drive.encryption + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2I) + end + end + + context "if a config does not specify all the filesystem properties" do + let(:config_json) do + { + drives: [ + { + filesystem: { path: "/" } + } + ] + } + end + + it "completes the filesystem config according to the product info" do + subject.solve(config) + + drive = config.drives.first + filesystem = drive.filesystem + expect(filesystem.type).to be_a(Agama::Storage::Configs::FilesystemType) + expect(filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) + expect(filesystem.type.btrfs).to be_a(Agama::Storage::Configs::Btrfs) + expect(filesystem.type.btrfs.snapshots?).to eq(true) + expect(filesystem.type.btrfs.read_only?).to eq(false) + expect(filesystem.type.btrfs.default_subvolume).to eq("@") + expect(filesystem.type.btrfs.subvolumes).to all(be_a(Y2Storage::SubvolSpecification)) + end + end + + partition_proc = proc { |c| c.drives.first.partitions.first } + + context "if a config does not specify size" do + let(:config_json) do + { + drives: [ + { + partitions: partitions + } + ] + } + end + + let(:partitions) do + [ + { + filesystem: { path: "/" } + }, + { + filesystem: { path: "/home" } + }, + {} + ] + end + + let(:scenario) { "disks.yaml" } + + it "sets a size according to the product info" do + subject.solve(config) + + drive = config.drives.first + p1, p2, p3 = drive.partitions + + expect(p1.size.default?).to eq(true) + expect(p1.size.min).to eq(5.GiB) + expect(p1.size.max).to eq(10.GiB) + + expect(p2.size.default?).to eq(true) + expect(p2.size.min).to eq(5.GiB) + expect(p2.size.max).to eq(Y2Storage::DiskSize.unlimited) + + expect(p3.size.default?).to eq(true) + expect(p3.size.min).to eq(100.MiB) + expect(p3.size.max).to eq(Y2Storage::DiskSize.unlimited) + end + + context "and there is a device assigned" do + let(:partitions) do + [ + { + search: "/dev/vda2", + filesystem: { path: "/" } + } + ] + end + + # Enable fallbacks and snapshots to check they don't affect in this case. + let(:min_fallbacks) { ["/home"] } + let(:max_fallbacks) { ["/home"] } + let(:snapshots_increment) { "300%" } + + it "sets the device size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(20.GiB) + expect(partition.size.max).to eq(20.GiB) + end + end + + context "and the product defines size fallbacks" do + let(:min_fallbacks) { ["/home"] } + let(:max_fallbacks) { ["/home"] } + let(:snapshots_increment) { "300%" } + + context "and the config does not specify some of the paths" do + let(:partitions) do + [ + { + filesystem: { + type: "xfs", + path: "/" + } + } + ] + end + + it "sets a size adding the fallback sizes" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(Y2Storage::DiskSize.unlimited) + end + + context "and snapshots are enabled" do + let(:partitions) do + [ + { + filesystem: { + type: "btrfs", + path: "/" + } + } + ] + end + + it "sets a size adding the fallback and snapshots sizes" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(40.GiB) + expect(partition.size.max).to eq(Y2Storage::DiskSize.unlimited) + end + end + end + + context "and the config specifies the fallback paths" do + let(:partitions) do + [ + { + filesystem: { + type: filesystem, + path: "/" + } + }, + { + filesystem: { path: "/home" } + } + ] + end + + let(:filesystem) { "xfs" } + + it "sets a size ignoring the fallback sizes" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(5.GiB) + expect(partition.size.max).to eq(10.GiB) + end + + context "and snapshots are enabled" do + let(:filesystem) { "btrfs" } + + it "sets a size adding the snapshots size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(20.GiB) + expect(partition.size.max).to eq(40.GiB) + end + end + end + end + + context "and the volume has to be enlarged according to RAM size" do + before do + allow_any_instance_of(Y2Storage::Arch).to receive(:ram_size).and_return(8.GiB) + end + + let(:partitions) do + [ + { + filesystem: { path: "swap" } + } + ] + end + + it "sets the RAM size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(8.GiB) + expect(partition.size.max).to eq(8.GiB) + end + end + end + + context "if a config specifies a size" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { path: "/" }, + size: { + min: "10 GiB", + max: "15 GiB" + } + } + ] + } + ] + } + end + + let(:scenario) { "disks.yaml" } + + # Enable fallbacks and snapshots to check they don't affect in this case. + let(:min_fallbacks) { ["/home"] } + let(:max_fallbacks) { ["/home"] } + let(:snapshots_increment) { "300%" } + + context "and there is no device assigned" do + let(:search) { nil } + + it "sets the given size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(15.GiB) + end + end + + context "and there is a device assigned" do + let(:search) { "/dev/vda2" } + + it "sets the given size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(15.GiB) + end + end + end + + context "if a config specifies 'current' for min size" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { path: "/" }, + size: { + min: "current", + max: "40 GiB" + } + } + ] + } + ] + } + end + + context "and there is no device assigned" do + let(:search) { nil } + + it "sets a size according to the product info" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(5.GiB) + expect(partition.size.max).to eq(10.GiB) + end + end + + context "and there is a device assigned" do + let(:scenario) { "disks.yaml" } + + let(:search) { "/dev/vda2" } + + it "sets the device size as min size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(20.GiB) + expect(partition.size.max).to eq(40.GiB) + end + end + end + + context "if a config specifies 'current' for max size" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: search, + filesystem: { path: "/" }, + size: { + min: "10 GiB", + max: "current" + } + } + ] + } + ] + } + end + + context "and there is no device assigned" do + let(:search) { nil } + + it "sets a size according to the product info" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(true) + expect(partition.size.min).to eq(5.GiB) + expect(partition.size.max).to eq(10.GiB) + end + end + + context "and there is a device assigned" do + let(:scenario) { "disks.yaml" } + + let(:search) { "/dev/vda2" } + + it "sets the device size as max size" do + subject.solve(config) + partition = partition_proc.call(config) + expect(partition.size.default?).to eq(false) + expect(partition.size.min).to eq(10.GiB) + expect(partition.size.max).to eq(20.GiB) + end + end + end + end + + context "if a drive has a search without a device name" do + let(:config_json) { { drives: drives } } + + let(:drives) do + [ + {}, + {}, + {} + ] + end + + let(:scenario) { "disks.yaml" } + + it "sets the first unassigned device to the drive" do + subject.solve(config) + search1, search2, search3 = config.drives.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vda") + expect(search2.solved?).to eq(true) + expect(search2.device.name).to eq("/dev/vdb") + expect(search3.solved?).to eq(true) + expect(search3.device.name).to eq("/dev/vdc") + end + + context "and there is not unassigned device" do + let(:drives) do + [ + {}, + {}, + {}, + {} + ] + end + + it "does not set a device to the drive" do + subject.solve(config) + search = config.drives[3].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + end + + context "if a drive has a search with a device name" do + let(:config_json) { { drives: drives } } + + let(:drives) do + [ + { search: search } + ] + end + + let(:scenario) { "disks.yaml" } + + context "and the device is found" do + let(:search) { "/dev/vdb" } + + it "sets the device to the drive" do + subject.solve(config) + search = config.drives.first.search + expect(search.solved?).to eq(true) + expect(search.device.name).to eq("/dev/vdb") + end + end + + context "and the device is not found" do + let(:search) { "/dev/vdd" } + + it "does not set a device to the drive" do + subject.solve(config) + search = config.drives.first.search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and the device was already assigned" do + let(:drives) do + [ + {}, + { search: "/dev/vda" } + ] + end + + it "does not set a device to the drive" do + subject.solve(config) + search = config.drives[1].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and there is other drive with the same device" do + let(:drives) do + [ + { search: "/dev/vdb" }, + { search: "/dev/vdb" } + ] + end + + it "only sets the device to the first drive" do + subject.solve(config) + search1, search2 = config.drives.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vdb") + expect(search2.solved?).to eq(true) + expect(search2.device).to be_nil + end + end + end + + context "if a partition has a search without a device name" do + let(:config_json) do + { + drives: [ + { partitions: partitions } + ] + } + end + + let(:partitions) do + [ + { search: {} }, + { search: {} }, + { search: {} } + ] + end + + let(:scenario) { "disks.yaml" } + + it "sets the first unassigned partition to the config" do + subject.solve(config) + search1, search2, search3 = config.drives.first.partitions.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vda1") + expect(search2.solved?).to eq(true) + expect(search2.device.name).to eq("/dev/vda2") + expect(search3.solved?).to eq(true) + expect(search3.device.name).to eq("/dev/vda3") + end + + context "and there is not unassigned partition" do + let(:partitions) do + [ + { search: {} }, + { search: {} }, + { search: {} }, + { search: {} } + ] + end + + it "does not set a partition to the config" do + subject.solve(config) + search = config.drives.first.partitions[3].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + end + + context "if a partition has a search with a device name" do + let(:config_json) do + { + drives: [ + { partitions: partitions } + ] + } + end + + let(:partitions) do + [ + { search: search } + ] + end + + let(:scenario) { "disks.yaml" } + + search_proc = proc { |c| c.drives.first.partitions.first.search } + + context "and the partition is found" do + let(:search) { "/dev/vda2" } + + it "sets the partition to the config" do + subject.solve(config) + search = search_proc.call(config) + expect(search.solved?).to eq(true) + expect(search.device.name).to eq("/dev/vda2") + end + end + + context "and the device is not found" do + let(:search) { "/dev/vdb1" } + + it "does not set a partition to the config" do + subject.solve(config) + search = search_proc.call(config) + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and the device was already assigned" do + let(:partitions) do + [ + { search: {} }, + { search: "/dev/vda1" } + ] + end + + it "does not set a partition to the config" do + subject.solve(config) + search = config.drives.first.partitions[1].search + expect(search.solved?).to eq(true) + expect(search.device).to be_nil + end + end + + context "and there is other partition with the same device" do + let(:partitions) do + [ + { search: "/dev/vda2" }, + { search: "/dev/vda2" } + ] + end + + it "only sets the partition to the first config" do + subject.solve(config) + search1, search2 = config.drives.first.partitions.map(&:search) + expect(search1.solved?).to eq(true) + expect(search1.device.name).to eq("/dev/vda2") + expect(search2.solved?).to eq(true) + expect(search2.device).to be_nil + end + end + end +end From 09998fb95c12030212a85e1e513c50e3fe0935a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Fri, 4 Oct 2024 16:04:45 +0100 Subject: [PATCH 06/21] storage: generate physical volumes config --- .../from_json_conversions/volume_group.rb | 48 ++++++- .../agama/storage/config_encryption_solver.rb | 23 +++- .../lib/agama/storage/configs/volume_group.rb | 13 ++ .../config_conversions/from_json_test.rb | 118 +++++++++++++++++- .../test/agama/storage/config_solver_test.rb | 30 +++++ service/test/y2storage/agama_proposal_test.rb | 1 - 6 files changed, 226 insertions(+), 7 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb index 153331ced..f1516456b 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/volume_group.rb @@ -20,6 +20,7 @@ # find current contact information at www.suse.com. require "agama/storage/config_conversions/from_json_conversions/base" +require "agama/storage/config_conversions/from_json_conversions/encryption" require "agama/storage/config_conversions/from_json_conversions/logical_volume" require "agama/storage/configs/volume_group" @@ -43,10 +44,12 @@ def convert # @return [Hash] def conversions { - name: volume_group_json[:name], - extent_size: convert_extent_size, - physical_volumes: volume_group_json[:physicalVolumes], - logical_volumes: convert_logical_volumes + name: volume_group_json[:name], + extent_size: convert_extent_size, + physical_volumes_devices: convert_physical_volumes_devices, + physical_volumes_encryption: convert_physical_volumes_encryption, + physical_volumes: convert_physical_volumes, + logical_volumes: convert_logical_volumes } end @@ -58,6 +61,43 @@ def convert_extent_size Y2Storage::DiskSize.new(value) end + # @return [Array, nil] + def convert_physical_volumes_devices + generate_json = physical_volume_generate_json&.fetch(:generate) + return unless generate_json + + generate_json.is_a?(Array) ? generate_json : generate_json[:targetDevices] + end + + # @return [Configs::Encryption, nil] + def convert_physical_volumes_encryption + generate_json = physical_volume_generate_json&.fetch(:generate) + return unless generate_json.is_a?(Hash) + + encryption_json = generate_json[:encryption] + return unless encryption_json + + FromJSONConversions::Encryption.new(encryption_json).convert + end + + # JSON of the physical volume with a 'generate'. + # + # @return [Hash, nil] + def physical_volume_generate_json + physical_volumes_json = volume_group_json[:physicalVolumes] + return unless physical_volumes_json + + physical_volumes_json.find { |p| p.is_a?(Hash) } + end + + # @return [Array, nil] + def convert_physical_volumes + physical_volumes_json = volume_group_json[:physicalVolumes] + return unless physical_volumes_json + + physical_volumes_json.select { |c| c.is_a?(String) } + end + # @return [Array, nil] def convert_logical_volumes logical_volumes_json = volume_group_json[:logicalVolumes] diff --git a/service/lib/agama/storage/config_encryption_solver.rb b/service/lib/agama/storage/config_encryption_solver.rb index 4a5996af8..60a02e8a7 100644 --- a/service/lib/agama/storage/config_encryption_solver.rb +++ b/service/lib/agama/storage/config_encryption_solver.rb @@ -41,7 +41,8 @@ def initialize(product_config) def solve(config) @config = config - configs_with_encryption.each { |c| solve_encryption(c) } + solve_encryptions + solve_physical_volumes_encryptions end private @@ -52,6 +53,10 @@ def solve(config) # @return [Config] attr_reader :config + def solve_encryptions + configs_with_encryption.each { |c| solve_encryption(c) } + end + # @param config [#encryption] def solve_encryption(config) return unless config.encryption @@ -64,6 +69,22 @@ def solve_encryption(config) solve_encryption_values(encryption) if encryption.method == default_encryption.method end + def solve_physical_volumes_encryptions + config.volume_groups.each { |c| solve_physical_volumes_encryption(c) } + end + + # @param config [Configs::VolumeGroup] + def solve_physical_volumes_encryption(config) + return unless config.physical_volumes_encryption + + encryption = config.physical_volumes_encryption + encryption.method ||= default_encryption.method + + # Recovering values from the default encryption only makes sense if the encryption method is + # the same. + solve_encryption_values(encryption) if encryption.method == default_encryption.method + end + # @param config [Configs::Encryption] def solve_encryption_values(config) config.password ||= default_encryption.password diff --git a/service/lib/agama/storage/configs/volume_group.rb b/service/lib/agama/storage/configs/volume_group.rb index 0939c5710..c595b6e53 100644 --- a/service/lib/agama/storage/configs/volume_group.rb +++ b/service/lib/agama/storage/configs/volume_group.rb @@ -30,6 +30,18 @@ class VolumeGroup # @return [Y2Storage::DiskSize, nil] attr_accessor :extent_size + # Aliases of the devices used for automatically creating new physical volumes. + # + # @return [Array] + attr_accessor :physical_volumes_devices + + # Encryption for the new physical volumes created at the {physical_volumes_devices}. + # + # @return [Encryption, nil] + attr_accessor :physical_volumes_encryption + + # Aliases of the devices used as physical volumes. + # # @return [Array] attr_accessor :physical_volumes @@ -37,6 +49,7 @@ class VolumeGroup attr_accessor :logical_volumes def initialize + @physical_volumes_devices = [] @physical_volumes = [] @logical_volumes = [] end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index e45beb63c..119c8b8d7 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -924,10 +924,20 @@ context "with an empty list" do let(:physical_volumes) { [] } - it "sets #physical_volumes to empty" do + it "sets #physical_volumes to the expected value" do vg = vg_proc.call(subject.convert) expect(vg.physical_volumes).to eq([]) end + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to eq([]) + end + + it "sets #physical_volumes_encryption to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end end context "with a list of aliases" do @@ -937,6 +947,112 @@ vg = vg_proc.call(subject.convert) expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") end + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to eq([]) + end + + it "sets #physical_volumes_encryption to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end + end + + context "with a list including a physical volume with 'generate' array" do + let(:physical_volumes) do + [ + "pv1", + { generate: ["disk1", "disk2"] }, + "pv2" + ] + end + + it "sets #physical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") + end + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to contain_exactly("disk1", "disk2") + end + + it "does not set #physical_volumes_encryption" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end + end + + context "with a list including a physical volume with 'generate' section" do + let(:physical_volumes) do + [ + "pv1", + { + generate: { + targetDevices: target_devices, + encryption: encryption + } + }, + "pv2" + ] + end + + let(:target_devices) { nil } + + let(:encryption) { nil } + + it "sets #physical_volumes to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes).to contain_exactly("pv1", "pv2") + end + + context "if the physical volume does not specify 'targetDevices'" do + let(:target_devices) { nil } + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to eq([]) + end + end + + context "if the physical volume does not specify 'encryption'" do + let(:target_devices) { nil } + + it "does not set #physical_volumes_encryption" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_encryption).to be_nil + end + end + + context "if the physical volume specifies 'targetDevices'" do + let(:target_devices) { ["disk1"] } + + it "sets #physical_volumes_devices to the expected value" do + vg = vg_proc.call(subject.convert) + expect(vg.physical_volumes_devices).to contain_exactly("disk1") + end + end + + context "if the physical volume specifies 'encryption'" do + let(:encryption) do + { + luks1: { password: "12345" } + } + end + + it "sets #physical_volumes_encryption to the expected value" do + vg = vg_proc.call(subject.convert) + encryption = vg.physical_volumes_encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) + expect(encryption.password).to eq("12345") + expect(encryption.pbkd_function).to be_nil + expect(encryption.label).to be_nil + expect(encryption.cipher).to be_nil + expect(encryption.key_size).to be_nil + end + end end end diff --git a/service/test/agama/storage/config_solver_test.rb b/service/test/agama/storage/config_solver_test.rb index 278e3a1a2..4973011e5 100644 --- a/service/test/agama/storage/config_solver_test.rb +++ b/service/test/agama/storage/config_solver_test.rb @@ -138,6 +138,36 @@ end end + context "if a volume group does not specify all the pv encryption properties" do + let(:config_json) do + { + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + encryption: { + luks2: { password: "12345" } + } + } + } + ] + } + ] + } + end + + it "completes the encryption config according to the product info" do + subject.solve(config) + + volume_group = config.volume_groups.first + encryption = volume_group.physical_volumes_encryption + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::ARGON2I) + end + end + context "if a config does not specify all the filesystem properties" do let(:config_json) do { diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 7d97b4d17..2022e2b86 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -73,7 +73,6 @@ def partition_config(name: nil, filesystem: nil, size: nil) describe Y2Storage::AgamaProposal do include Agama::RSpec::StorageHelpers - using Y2Storage::Refinements::SizeCasts subject(:proposal) do described_class.new(config, product_config: product_config, issues_list: issues_list) From ebce78c28bba9fec0b93c730abed9670853b4fca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 8 Oct 2024 11:13:37 +0100 Subject: [PATCH 07/21] storage: add checks for physical volumes config --- service/lib/agama/storage/config_checker.rb | 281 ++++++--- .../from_json_conversions/encryption.rb | 4 +- .../test/agama/storage/config_checker_test.rb | 574 ++++++++++++++++++ .../config_conversions/from_json_test.rb | 22 + service/test/y2storage/agama_proposal_test.rb | 8 +- 5 files changed, 814 insertions(+), 75 deletions(-) create mode 100644 service/test/agama/storage/config_checker_test.rb diff --git a/service/lib/agama/storage/config_checker.rb b/service/lib/agama/storage/config_checker.rb index 281fb4447..453e33c1a 100644 --- a/service/lib/agama/storage/config_checker.rb +++ b/service/lib/agama/storage/config_checker.rb @@ -39,8 +39,7 @@ def initialize(config) # # @return [Array] def issues - config.drives.flat_map { |d| drive_issues(d) } + - config.volume_groups.flat_map { |v| volume_group_issues(v) } + drives_issues + volume_groups_issues end private @@ -48,6 +47,13 @@ def issues # @return [Storage::Config] attr_reader :config + # Issues from drives. + # + # @return [Array] + def drives_issues + config.drives.flat_map { |d| drive_issues(d) } + end + # Issues from a drive config. # # @param config [Configs::Drive] @@ -60,6 +66,94 @@ def drive_issues(config) ].flatten.compact end + # Issue for not found device. + # + # @param config [Configs::Drive, Configs::Partition] + # @return [Agama::Issue] + def search_issue(config) + return if !config.search || config.found_device + + if config.is_a?(Agama::Storage::Configs::Drive) + if config.search.skip_device? + warning(_("No device found for an optional drive")) + else + error(_("No device found for a mandatory drive")) + end + elsif config.search.skip_device? + warning(_("No device found for an optional partition")) + else + error(_("No device found for a mandatory partition")) + end + end + + # Issues related to encryption. + # + # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @return [Array] + def encryption_issues(config) + return [] unless config.encryption + + [ + missing_encryption_password_issue(config.encryption), + unavailable_encryption_method_issue(config.encryption), + wrong_encryption_method_issue(config) + ].compact + end + + # @see #encryption_issues + # + # @param config [Configs::Encryption] + # @return [Issue, nil] + def missing_encryption_password_issue(config) + return unless config.missing_password? + + error( + format( + # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device + # (e.g., 'luks1', 'random_swap'). + _("No passphrase provided (required for using the method '%{crypt_method}')."), + crypt_method: config.method.to_human_string + ) + ) + end + + # @see #encryption_issues + # + # @param config [Configs::Encryption] + # @return [Issue, nil] + def unavailable_encryption_method_issue(config) + method = config.method + return if !method || method.available? + + error( + format( + # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device + # (e.g., 'luks1', 'random_swap'). + _("Encryption method '%{crypt_method}' is not available in this system."), + crypt_method: method.to_human_string + ) + ) + end + + # @see #encryption_issues + # + # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @return [Issue, nil] + def wrong_encryption_method_issue(config) + method = config.encryption&.method + return unless method&.only_for_swap? + return if config.filesystem&.path == Y2Storage::MountPoint::SWAP_PATH.to_s + + error( + format( + # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device + # (e.g., 'luks1', 'random_swap'). + _("'%{crypt_method}' is not a suitable method to encrypt the device."), + crypt_method: method.to_human_string + ) + ) + end + # Issues from partitions. # # @param config [Configs::Drive] @@ -79,15 +173,70 @@ def partition_issues(config) ].flatten.compact end + # Issues from volume groups. + # + # @return [Array] + def volume_groups_issues + [ + overused_physical_volumes_devices_issues, + config.volume_groups.flat_map { |v| volume_group_issues(v) } + ].flatten + end + + # Issues for overused target devices for physical volumes. + # + # @note The Agama proposal is not able to calculate if the same target device is used by more + # than one volume group having several target devices. + # + # @return [Array] + def overused_physical_volumes_devices_issues + overused = overused_physical_volumes_devices + return [] if overused.none? + + overused.map do |device| + error( + format( + # TRANSLATORS: %s is the replaced by a device alias (e.g., "disk1"). + _("The device '%s' is used several times as target device for physical volumes"), + device + ) + ) + end + end + + # Aliases of overused target devices for physical volumes. + # + # @return [Array] + def overused_physical_volumes_devices + config.volume_groups + .map(&:physical_volumes_devices) + .map(&:uniq) + .select { |d| d.size > 1 } + .flatten + .tally + .select { |_, v| v > 1 } + .keys + end + # Issues from a volume group config. # # @param config [Configs::VolumeGroup] # @return [Array] def volume_group_issues(config) - lvs_issues = config.logical_volumes.flat_map { |v| logical_volume_issues(v, config) } - pvs_issues = config.physical_volumes.map { |v| missing_physical_volume_issue(v) }.compact + [ + logical_volumes_issues(config), + physical_volumes_issues(config), + physical_volumes_devices_issues(config), + physical_volumes_encryption_issues(config) + ].flatten + end - lvs_issues + pvs_issues + # Issues from a logical volumes. + # + # @param config [Configs::VolumeGroup] + # @return [Array] + def logical_volumes_issues(config) + config.logical_volumes.flat_map { |v| logical_volume_issues(v, config) } end # Issues from a logical volume config. @@ -103,26 +252,6 @@ def logical_volume_issues(lv_config, vg_config) ].compact.flatten end - # Issue for not found device. - # - # @param config [Configs::Drive, Configs::Partition] - # @return [Agama::Issue] - def search_issue(config) - return if !config.search || config.found_device - - if config.is_a?(Agama::Storage::Configs::Drive) - if config.search.skip_device? - warning(_("No device found for an optional drive")) - else - error(_("No device found for a mandatory drive")) - end - elsif config.search.skip_device? - warning(_("No device found for an optional partition")) - else - error(_("No device found for a mandatory partition")) - end - end - # @see #logical_volume_issues # # @param lv_config [Configs::LogicalVolume] @@ -141,13 +270,21 @@ def missing_thin_pool_issue(lv_config, vg_config) error( format( # TRANSLATORS: %s is the replaced by a device alias (e.g., "pv1"). - _("There is no LVM thin pool volume with alias %s"), + _("There is no LVM thin pool volume with alias '%s'"), lv_config.used_pool ) ) end - # @see #logical_volume_issues + # Issues from physical volumes. + # + # @param config [Configs::VolumeGroup] + # @return [Array] + def physical_volumes_issues(config) + config.physical_volumes.map { |v| missing_physical_volume_issue(v) }.compact + end + + # @see #physical_volumes_issues # # @param pv_alias [String] # @return [Issue, nil] @@ -158,80 +295,86 @@ def missing_physical_volume_issue(pv_alias) error( format( # TRANSLATORS: %s is the replaced by a device alias (e.g., "pv1"). - _("There is no LVM physical volume with alias %s"), + _("There is no LVM physical volume with alias '%s'"), pv_alias ) ) end - # Issues related to encryption. + # Issues from physical volumes devices (target devices). # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @param config [Configs::VolumeGroup] # @return [Array] - def encryption_issues(config) - return [] unless config.encryption - - [ - missing_encryption_password_issue(config), - available_encryption_method_issue(config), - wrong_encryption_method_issue(config) - ].compact + def physical_volumes_devices_issues(config) + config.physical_volumes_devices + .map { |d| missing_physical_volumes_device_issue(d) } + .compact end - # @see #encryption_issues + # @see #physical_volumes_devices_issue # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @param device_alias [String] # @return [Issue, nil] - def missing_encryption_password_issue(config) - return unless config.encryption&.missing_password? + def missing_physical_volumes_device_issue(device_alias) + return if config.drives.any? { |d| d.alias == device_alias } error( format( - # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device - # (e.g., 'luks1', 'random_swap'). - _("No passphrase provided (required for using the method '%{crypt_method}')."), - crypt_method: config.encryption.method.id.to_s + # TRANSLATORS: %s is the replaced by a device alias (e.g., "disk1"). + _("There is no target device for LVM physical volumes with alias '%s'"), + device_alias ) ) end - # @see #encryption_issues + # Issues from physical volumes encryption. # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] - # @return [Issue, nil] - def available_encryption_method_issue(config) - method = config.encryption&.method - return if !method || method.available? + # @param config [Configs::VolumeGroup] + # @return [Array] + def physical_volumes_encryption_issues(config) + encryption = config.physical_volumes_encryption + return [] unless encryption - error( - format( - # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device - # (e.g., 'luks1', 'random_swap'). - _("Encryption method '%{crypt_method}' is not available in this system."), - crypt_method: method.id.to_s - ) - ) + [ + missing_encryption_password_issue(encryption), + unavailable_encryption_method_issue(encryption), + wrong_physical_volumes_encryption_method_issue(encryption) + ].compact end - # @see #encryption_issues + # @see #physical_volumes_encryption_issues # - # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] + # @param config [Configs::Encryption] # @return [Issue, nil] - def wrong_encryption_method_issue(config) - method = config.encryption&.method - return unless method&.only_for_swap? - return if config.filesystem&.path == Y2Storage::MountPoint::SWAP_PATH.to_s + def wrong_physical_volumes_encryption_method_issue(config) + method = config.method + return if method.nil? || valid_physical_volumes_encryption_method?(method) error( format( # TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device - # (e.g., 'luks1', 'random_swap'). - _("'%{crypt_method}' is not a suitable method to encrypt the device."), - crypt_method: method.id.to_s + # (e.g., 'luks1'). + _("'%{crypt_method}' is not a suitable method to encrypt the physical volumes."), + crypt_method: method.to_human_string ) ) end + # Whether an encryption method can be used for encrypting physical volumes. + # + # @param method [Y2Storage::EncryptionMethod] + # @return [Boolean] + def valid_physical_volumes_encryption_method?(method) + valid_methods = [ + Y2Storage::EncryptionMethod::LUKS1, + Y2Storage::EncryptionMethod::LUKS2, + Y2Storage::EncryptionMethod::PERVASIVE_LUKS2, + Y2Storage::EncryptionMethod::TPM_FDE + ] + + valid_methods.include?(method) + end + # Creates a warning issue. # # @param message [String] diff --git a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb index 0bafd6a99..1956c458c 100644 --- a/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb +++ b/service/lib/agama/storage/config_conversions/from_json_conversions/encryption.rb @@ -65,7 +65,7 @@ def luks2? def pervasive_luks2? return false unless encryption_json.is_a?(Hash) - !encryption_json[:pervasive_luks2].nil? + !encryption_json[:pervasiveLuks2].nil? end # @return [Hash] @@ -96,7 +96,7 @@ def luks2_conversions # @return [Hash] def pervasive_luks2_conversions - pervasive_json = encryption_json[:pervasive_luks2] + pervasive_json = encryption_json[:pervasiveLuks2] { method: Y2Storage::EncryptionMethod::PERVASIVE_LUKS2, diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb new file mode 100644 index 000000000..7582d199f --- /dev/null +++ b/service/test/agama/storage/config_checker_test.rb @@ -0,0 +1,574 @@ +# 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 "./storage_helpers" +require "agama/config" +require "agama/storage/config_conversions/from_json" +require "agama/storage/config_checker" +require "agama/storage/config_solver" +require "y2storage" +require "y2storage/refinements" + +using Y2Storage::Refinements::SizeCasts + +shared_examples "encryption issues" do + let(:filesystem) { nil } + + context "without password" do + let(:encryption) do + { luks1: {} } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("No passphrase") + end + end + + context "with unavailable method" do + let(:encryption) do + { + pervasiveLuks2: { + password: "12345" + } + } + end + + before do + allow_any_instance_of(Y2Storage::EncryptionMethod::PervasiveLuks2) + .to(receive(:available?)) + .and_return(false) + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("'Pervasive Volume Encryption' is not available") + end + end + + context "with invalid method" do + let(:encryption) { "protected_swap" } + let(:filesystem) { { path: "/" } } + + before do + allow_any_instance_of(Y2Storage::EncryptionMethod::ProtectedSwap) + .to(receive(:available?)) + .and_return(true) + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description) + .to(match("'Encryption with Volatile Protected Key' is not a suitable")) + end + end + + context "with a valid encryption" do + let(:encryption) do + { + luks1: { + password: "12345" + } + } + end + + let(:filesystem) { { path: "/" } } + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end +end + +describe Agama::Storage::ConfigChecker do + include Agama::RSpec::StorageHelpers + + subject { described_class.new(config) } + + let(:config) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json) + .convert + end + + let(:config_json) { nil } + + before do + mock_storage(devicegraph: scenario) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE) + .to(receive(:possible?)) + .and_return(true) + end + + describe "#issues" do + before do + # Solves the config before checking. + devicegraph = Y2Storage::StorageManager.instance.probed + product_config = Agama::Config.new + + Agama::Storage::ConfigSolver + .new(devicegraph, product_config) + .solve(config) + end + + let(:scenario) { "disks.yaml" } + + context "if a drive has not found device" do + let(:config_json) do + { + drives: [ + { + search: { + condition: { name: "/dev/vdd" }, + ifNotFound: if_not_found + } + } + ] + } + end + + context "and the drive should be skipped" do + let(:if_not_found) { "skip" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(false) + expect(issue.description).to eq("No device found for an optional drive") + end + end + + context "and the drive should not be skipped" do + let(:if_not_found) { "error" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to eq("No device found for a mandatory drive") + end + end + end + + context "if a drive has a found device" do + let(:config_json) do + { + drives: [ + { search: "/dev/vda" } + ] + } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + + context "if a drive has encryption" do + let(:config_json) do + { + drives: [ + { + encryption: encryption, + filesystem: filesystem + } + ] + } + end + + include_examples "encryption issues" + end + + context "if a drive has partitions" do + let(:config_json) do + { + drives: [ + { + partitions: [partition] + } + ] + } + end + + context "and a partition has not found device" do + let(:partition) do + { + search: { + condition: { name: "/dev/vdb1" }, + ifNotFound: if_not_found + } + } + end + + context "and the partition should be skipped" do + let(:if_not_found) { "skip" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(false) + expect(issue.description).to eq("No device found for an optional partition") + end + end + + context "and the partition should not be skipped" do + let(:if_not_found) { "error" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to eq("No device found for a mandatory partition") + end + end + end + + context "and the partition has a found device" do + let(:partition) do + { search: "/dev/vda1" } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + + context "and a partition has encryption" do + let(:partition) do + { + encryption: encryption, + filesystem: filesystem + } + end + + include_examples "encryption issues" + end + end + + context "if a volume group has logical volumes" do + let(:config_json) do + { + volumeGroups: [ + { + logicalVolumes: [ + logical_volume, + { + alias: "pool", + pool: true + } + ] + } + ] + } + end + + context "and a logical volume has encryption" do + let(:logical_volume) do + { + encryption: encryption, + filesystem: filesystem + } + end + + include_examples "encryption issues" + end + + context "and a logical volume has an unknown pool" do + let(:logical_volume) do + { usedPool: "unknown" } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("no LVM thin pool") + end + end + + context "and a logical volume has a known pool" do + let(:logical_volume) do + { usedPool: "pool" } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end + + context "if a volume group has an unknown physical volume" do + let(:config_json) do + { + drives: [ + { + alias: "first-disk" + } + ], + volumeGroups: [ + { + physicalVolumes: ["first-disk", "pv1"] + } + ] + } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("no LVM physical volume with alias 'pv1'") + end + end + + context "if a volume group has an unknown target device for physical volumes" do + let(:config_json) do + { + drives: [ + { + alias: "first-disk" + } + ], + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + targetDevices: ["first-disk", "second-disk"] + } + } + ] + } + ] + } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description) + .to(match("no target device for LVM physical volumes with alias 'second-disk'")) + end + end + + context "if a volume group has encryption for physical volumes" do + let(:config_json) do + { + drives: [ + { + alias: "first-disk" + } + ], + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + targetDevices: ["first-disk"], + encryption: encryption + } + } + ] + } + ] + } + end + + context "without password" do + let(:encryption) do + { luks1: {} } + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("No passphrase") + end + end + + context "with unavailable method" do + let(:encryption) do + { + luks2: { + password: "12345" + } + } + end + + before do + allow_any_instance_of(Y2Storage::EncryptionMethod::Luks2) + .to(receive(:available?)) + .and_return(false) + end + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("'Regular LUKS2' is not available") + end + end + + context "with invalid method" do + let(:encryption) { "random_swap" } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description) + .to(match("'Encryption with Volatile Random Key' is not a suitable method")) + end + end + + context "with a valid encryption" do + let(:encryption) do + { + luks1: { + password: "12345" + } + } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end + + context "if there are overused physical volumes devices" do + let(:config_json) do + { + drives: [ + { alias: "disk1" }, + { alias: "disk2" }, + { alias: "disk3" } + ], + volumeGroups: [ + { + physicalVolumes: [ + { + generate: { + targetDevices: ["disk1", "disk2"] + } + } + ] + }, + { + physicalVolumes: [ + { + generate: { + targetDevices: ["disk2"] + } + } + ] + }, + { + physicalVolumes: [ + { + generate: { + targetDevices: ["disk1", "disk3", "disk3"] + } + } + ] + } + ] + } + end + + it "includes the expected issues" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to(match("The device 'disk1' is used several times")) + end + end + + context "if the config has several issues" do + let(:config_json) do + { + drives: [ + { + search: "/dev/vdd", + encryption: { luks2: {} } + } + ], + volumeGroups: [ + { + physicalVolumes: ["pv1"] + } + ] + } + end + + it "includes the expected issues" do + expect(subject.issues).to contain_exactly( + an_object_having_attributes( + description: match(/No device found for a mandatory drive/) + ), + an_object_having_attributes( + description: match(/No passphrase provided/) + ), + an_object_having_attributes( + description: match(/There is no LVM physical volume with alias 'pv1'/) + ) + ) + end + end + end +end diff --git a/service/test/agama/storage/config_conversions/from_json_test.rb b/service/test/agama/storage/config_conversions/from_json_test.rb index 119c8b8d7..87792bdb7 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -179,6 +179,28 @@ expect(encryption.label).to be_nil end end + + context "if 'encryption' is 'pervasiveLuks2'" do + let(:encryption) do + { + pervasiveLuks2: { + password: "12345" + } + } + end + + it "sets #encryption to the expected value" do + config = config_proc.call(subject.convert) + encryption = config.encryption + expect(encryption).to be_a(Agama::Storage::Configs::Encryption) + expect(encryption.method).to eq(Y2Storage::EncryptionMethod::PERVASIVE_LUKS2) + expect(encryption.password).to eq("12345") + expect(encryption.key_size).to be_nil + expect(encryption.pbkd_function).to be_nil + expect(encryption.cipher).to be_nil + expect(encryption.label).to be_nil + end + end end shared_examples "with filesystem" do |config_proc| diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 2022e2b86..f193bf4f0 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -396,7 +396,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /method 'luks2' is not available/, + description: /method 'Regular LUKS2' is not available/, severity: Agama::Issue::Severity::ERROR ) end @@ -413,7 +413,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /'random_swap' is not a suitable method/, + description: /'Encryption with Volatile Random Key' is not a suitable method/, severity: Agama::Issue::Severity::ERROR ) end @@ -1459,7 +1459,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /no LVM physical volume with alias pv2/, + description: /no LVM physical volume with alias 'pv2'/, severity: Agama::Issue::Severity::ERROR ) end @@ -1511,7 +1511,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) it "reports the corresponding error" do proposal.propose expect(proposal.issues_list).to include an_object_having_attributes( - description: /no LVM thin pool volume with alias pool/, + description: /no LVM thin pool volume with alias 'pool'/, severity: Agama::Issue::Severity::ERROR ) end From cf82e99884b824b4629184a54632d1c41b3abbbb 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, 7 Oct 2024 08:01:39 +0100 Subject: [PATCH 08/21] service: changelog --- service/package/rubygem-agama-yast.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 24704a441..cfd7f956c 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Mon Oct 7 06:58:48 UTC 2024 - José Iván López González + +- Storage: add support to the storage config for automatically + creating physical volumes (gh#agama-project/agama#1652). + ------------------------------------------------------------------- Fri Sep 27 14:15:16 UTC 2024 - José Iván López González From e4fe3dbc42a1cafb945bdc1aff41c205bb0a8abf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 8 Oct 2024 16:34:10 +0100 Subject: [PATCH 09/21] storage: add checks for filesystem config --- service/lib/agama/storage/config_checker.rb | 94 +++++++++++- service/lib/y2storage/agama_proposal.rb | 5 +- .../test/agama/storage/config_checker_test.rb | 139 +++++++++++++++++- service/test/y2storage/agama_proposal_test.rb | 55 +++++-- 4 files changed, 266 insertions(+), 27 deletions(-) diff --git a/service/lib/agama/storage/config_checker.rb b/service/lib/agama/storage/config_checker.rb index 453e33c1a..25c3c6a47 100644 --- a/service/lib/agama/storage/config_checker.rb +++ b/service/lib/agama/storage/config_checker.rb @@ -20,19 +20,24 @@ # find current contact information at www.suse.com. require "agama/issue" +require "agama/storage/volume_templates_builder" require "yast/i18n" require "y2storage/mount_point" module Agama module Storage # Class for checking a storage config. - class ConfigChecker + # + # TODO: Split in smaller checkers, for example: ConfigFilesystemChecker, etc. + class ConfigChecker # rubocop:disable Metrics/ClassLength include Yast::I18n # @param config [Storage::Config] - def initialize(config) + # @param product_config [Agama::Config] + def initialize(config, product_config) textdomain "agama" @config = config + @product_config = product_config || Agama::Config.new end # Issues detected in the config. @@ -47,6 +52,9 @@ def issues # @return [Storage::Config] attr_reader :config + # @return [Agama::Config] + attr_reader :product_config + # Issues from drives. # # @return [Array] @@ -61,6 +69,7 @@ def drives_issues def drive_issues(config) [ search_issue(config), + filesystem_issues(config), encryption_issues(config), partitions_issues(config) ].flatten.compact @@ -86,16 +95,76 @@ def search_issue(config) end end + # Issues related to the filesystem. + # + # @param config [#filesystem] + # @return [Array] + def filesystem_issues(config) + filesystem = config.filesystem + return [] unless filesystem + + [ + missing_filesystem_issue(filesystem), + invalid_filesystem_issue(filesystem) + ].compact + end + + # @see #filesystem_issues + # + # @param config [Configs::Filesystem] + # @return [Issue, nil] + def missing_filesystem_issue(config) + return if config.reuse? + return if config.type&.fs_type + + error( + format( + # TRANSLATORS: %s is the replaced by a mount path (e.g., "/home"). + _("Missing file system type for '%s'"), + config.path + ) + ) + end + + # @see #filesystem_issues + # + # @param config [Configs::Filesystem] + # @return [Issue, nil] + def invalid_filesystem_issue(config) + return if config.reuse? + + type = config.type&.fs_type + return unless type + + path = config.path + types = suitable_filesystem_types(path) + return if types.include?(type) + + # Let's consider a type as valid if the product does not define any suitable type. + return if types.empty? + + error( + format( + # TRANSLATORS: %{filesystem} is replaced by a file system type (e.g., "Btrfs") and + # %{path} is replaced by a mount path (e.g., "/home"). + _("The file system type '%{filesystem}' is not suitable for '%{path}'"), + filesystem: type.to_human_string, + path: path + ) + ) + end + # Issues related to encryption. # # @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume] # @return [Array] def encryption_issues(config) - return [] unless config.encryption + encryption = config.encryption + return [] unless encryption [ - missing_encryption_password_issue(config.encryption), - unavailable_encryption_method_issue(config.encryption), + missing_encryption_password_issue(encryption), + unavailable_encryption_method_issue(encryption), wrong_encryption_method_issue(config) ].compact end @@ -169,6 +238,7 @@ def partitions_issues(config) def partition_issues(config) [ search_issue(config), + filesystem_issues(config), encryption_issues(config) ].flatten.compact end @@ -247,6 +317,7 @@ def logical_volumes_issues(config) # @return [Array] def logical_volume_issues(lv_config, vg_config) [ + filesystem_issues(lv_config), encryption_issues(lv_config), missing_thin_pool_issue(lv_config, vg_config) ].compact.flatten @@ -375,6 +446,19 @@ def valid_physical_volumes_encryption_method?(method) valid_methods.include?(method) end + # Suitable file system types for the given path. + # + # @param path [String, nil] + # @return [Array] + def suitable_filesystem_types(path = nil) + volume_builder.for(path || "").outline.filesystems + end + + # @return [VolumeTemplatesBuilder] + def volume_builder + @volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config) + end + # Creates a warning issue. # # @param message [String] diff --git a/service/lib/y2storage/agama_proposal.rb b/service/lib/y2storage/agama_proposal.rb index a386da7e4..13675f089 100644 --- a/service/lib/y2storage/agama_proposal.rb +++ b/service/lib/y2storage/agama_proposal.rb @@ -96,7 +96,10 @@ def calculate_proposal .new(initial_devicegraph, product_config) .solve(config) - issues = Agama::Storage::ConfigChecker.new(config).issues + issues = Agama::Storage::ConfigChecker + .new(config, product_config) + .issues + issues_list.concat(issues) if fatal_error? diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb index 7582d199f..bf818958a 100644 --- a/service/test/agama/storage/config_checker_test.rb +++ b/service/test/agama/storage/config_checker_test.rb @@ -25,9 +25,6 @@ require "agama/storage/config_checker" require "agama/storage/config_solver" require "y2storage" -require "y2storage/refinements" - -using Y2Storage::Refinements::SizeCasts shared_examples "encryption issues" do let(:filesystem) { nil } @@ -110,10 +107,91 @@ end end +shared_examples "filesystem issues" do |filesystem_proc| + context "with invalid type" do + let(:filesystem) do + { + path: "/", + type: "vfat", + reuseIfPossible: reuse + } + end + + context "and without reusing the filesystem" do + let(:reuse) { false } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to match("type 'FAT' is not suitable for '/'") + end + end + + context "and reusing the filesystem" do + let(:reuse) { true } + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end + + context "with valid type" do + let(:filesystem) do + { + path: "/", + type: "btrfs" + } + end + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + + context "without a filesystem type" do + let(:filesystem) do + { + path: "/", + reuseIfPossible: reuse + } + end + + before do + # Explicitly remove the filesystem type. Otherwise the JSON conversion assigns a default type. + filesystem_proc.call(config).type = nil + end + + context "and without reusing the filesystem" do + let(:reuse) { false } + + it "includes the expected issue" do + issues = subject.issues + expect(issues.size).to eq(1) + + issue = issues.first + expect(issue.error?).to eq(true) + expect(issue.description).to eq("Missing file system type for '/'") + end + end + + context "and reusing the filesystem" do + let(:reuse) { true } + + it "does not include an issue" do + expect(subject.issues.size).to eq(0) + end + end + end +end + describe Agama::Storage::ConfigChecker do include Agama::RSpec::StorageHelpers - subject { described_class.new(config) } + subject { described_class.new(config, product_config) } let(:config) do Agama::Storage::ConfigConversions::FromJSON @@ -123,6 +201,22 @@ let(:config_json) { nil } + let(:product_config) { Agama::Config.new(product_data) } + + let(:product_data) do + { + "storage" => { + "volume_templates" => [ + { + "mount_path" => "/", + "filesystem" => "btrfs", + "outline" => { "filesystems" => ["btrfs", "xfs"] } + } + ] + } + } + end + before do mock_storage(devicegraph: scenario) # To speed-up the tests @@ -135,7 +229,6 @@ before do # Solves the config before checking. devicegraph = Y2Storage::StorageManager.instance.probed - product_config = Agama::Config.new Agama::Storage::ConfigSolver .new(devicegraph, product_config) @@ -214,6 +307,22 @@ include_examples "encryption issues" end + context "if a drive has filesystem" do + let(:config_json) do + { + drives: [ + { + filesystem: filesystem + } + ] + } + end + + filesystem_proc = proc { |c| c.drives.first.filesystem } + + include_examples "filesystem issues", filesystem_proc + end + context "if a drive has partitions" do let(:config_json) do { @@ -272,6 +381,16 @@ end end + context "if a partition has filesystem" do + let(:partition) do + { filesystem: filesystem } + end + + filesystem_proc = proc { |c| c.drives.first.partitions.first.filesystem } + + include_examples "filesystem issues", filesystem_proc + end + context "and a partition has encryption" do let(:partition) do { @@ -301,6 +420,16 @@ } end + context "if a logical volume has filesystem" do + let(:logical_volume) do + { filesystem: filesystem } + end + + filesystem_proc = proc { |c| c.volume_groups.first.logical_volumes.first.filesystem } + + include_examples "filesystem issues", filesystem_proc + end + context "and a logical volume has encryption" do let(:logical_volume) do { diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index f193bf4f0..33783ab5c 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -107,30 +107,52 @@ def partition_config(name: nil, filesystem: nil, size: nil) "volumes" => ["/", "swap"], "volume_templates" => [ { - "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true }, - "btrfs" => { - "snapshots" => true, "default_subvolume" => "@", - "subvolumes" => ["home", "opt", "root", "srv"] + "mount_path" => "/", + "filesystem" => "btrfs", + "size" => { "auto" => true }, + "btrfs" => { + "snapshots" => true, + "default_subvolume" => "@", + "subvolumes" => ["home", "opt", "root", "srv"] }, - "outline" => { - "required" => true, "snapshots_configurable" => true, - "auto_size" => { - "base_min" => "5 GiB", "base_max" => "10 GiB", - "min_fallback_for" => ["/home"], "max_fallback_for" => ["/home"], + "outline" => { + "required" => true, + "snapshots_configurable" => true, + "filesystems" => ["btrfs", "xfs", "ext3", "ext4"], + "auto_size" => { + "base_min" => "5 GiB", + "base_max" => "10 GiB", + "min_fallback_for" => ["/home"], + "max_fallback_for" => ["/home"], "snapshots_increment" => "300%" } } }, { - "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" }, - "filesystem" => "xfs", "outline" => { "required" => false } + "mount_path" => "/home", + "size" => { "auto" => false, "min" => "5 GiB" }, + "filesystem" => "xfs", + "outline" => { + "required" => false, + "filesystems" => ["xfs", "ext4"] + } }, { - "mount_path" => "swap", "filesystem" => "swap", - "outline" => { "required" => false } + "mount_path" => "swap", + "filesystem" => "swap", + "outline" => { + "required" => false, + "filesystems" => ["swap"] + } }, - { "mount_path" => "", "filesystem" => "ext4", - "size" => { "min" => "100 MiB" } } + { + "mount_path" => "", + "filesystem" => "ext4", + "size" => { "min" => "100 MiB" }, + "outline" => { + "filesystems" => ["xfs", "ext4"] + } + } ] } } @@ -1049,7 +1071,8 @@ def partition_config(name: nil, filesystem: nil, size: nil) "filesystem" => "swap", "size" => { "auto" => true }, "outline" => { - "auto_size" => { + "filesystems" => ["swap"], + "auto_size" => { "adjust_by_ram" => true, "base_min" => "2 GiB", "base_max" => "4 GiB" From 4bd5eacbbebf70665bbe2ca8d84a958766d583f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 9 Oct 2024 07:22:22 +0100 Subject: [PATCH 10/21] doc: document generate physical volumes --- doc/auto_storage.md | 75 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/doc/auto_storage.md b/doc/auto_storage.md index 3aea6a7ed..fa4aae674 100644 --- a/doc/auto_storage.md +++ b/doc/auto_storage.md @@ -944,6 +944,81 @@ The *mandatory* keyword can be used for only generating the mandatory partitions } ``` +### Generating Physical Volumes + +Volume groups can be configured to explicitly use a set of devices as physical volumes. The aliases +of the devices to use are added to the list of physical volumes: + +```json +"storage": { + "drives": [ + { + "search": "/dev/vda", + "partitions": [ + { "alias": "pv2" }, + { "alias": "pv1" } + ] + } + ], + "volumeGroups": [ + { + "physicalVolumes": ["pv1", "pv2"] + } + ] +} +``` + +The physical volumes can be automatically generated too, by simply indicating the target devices in +which to create the partitions. For that, a *generate* section is added to the list of physical +volumes: + +```json +"storage": { + "drives": [ + { + "search": "/dev/vda", + "alias": "pvs-disk" + } + ], + "volumeGroups": [ + { + "physicalVolumes": [ + { "generate": ["pvs-disk"] } + ] + } + ] +} +``` + +If the auto-generated physical volumes have to be encrypted, then the encryption config is added to +the *generate* section: + + +```json +"storage": { + "drives": [ + { + "search": "/dev/vda", + "alias": "pvs-disk" + } + ], + "volumeGroups": [ + { + "physicalVolumes": [ + { + "generate": { + "targetDevices": ["pvs-disk"], + "encryption": { + "luks2": { "password": "12345" } + } + } + } + ] + } + ] +} +``` + ### Using the Automatic Proposal On the first implementations, Agama can rely on the process known as Guided Proposal to calculate From cc01a014d1f6c3919cf83eb3bc2fac3b73416d95 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 3 Oct 2024 16:38:16 +0200 Subject: [PATCH 11/21] Improve detection of boot disk --- service/lib/agama/storage/config.rb | 57 +++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/service/lib/agama/storage/config.rb b/service/lib/agama/storage/config.rb index ab8166010..f8d9a1dd1 100644 --- a/service/lib/agama/storage/config.rb +++ b/service/lib/agama/storage/config.rb @@ -62,6 +62,18 @@ def boot_device explicit_boot_device || implicit_boot_device end + # return [Array] + def partitions + drives.flat_map(&:partitions) + end + + # return [Array] + def logical_volumes + volume_groups.flat_map(&:logical_volumes) + end + + private + # Device used for booting the target system # # @return [String, nil] nil if no disk is explicitly chosen @@ -73,9 +85,15 @@ def explicit_boot_device # Device that seems to be expected to be used for booting, according to the drive definitions # - # @return [String, nil] nil if the information cannot be inferred from the list of drives + # @return [String, nil] nil if the information cannot be inferred from the config def implicit_boot_device - # NOTE: preliminary implementation with very simplistic checks + implicit_drive_boot_device || implicit_lvm_boot_device + end + + # @see #implicit_boot_device + # + # @return [String, nil] nil if the information cannot be inferred from the list of drives + def implicit_drive_boot_device root_drive = drives.find do |drive| drive.partitions.any? { |p| p.filesystem&.root? } end @@ -83,14 +101,37 @@ def implicit_boot_device root_drive&.found_device&.name end - # return [Array] - def partitions - drives.flat_map(&:partitions) + # @see #implicit_boot_device + # + # @return [String, nil] nil if the information cannot be inferred from the list of LVM VGs + def implicit_lvm_boot_device + root_vg = root_volume_group + return nil unless root_vg + + root_drives = drives.select { |d| drive_for_vg?(d, root_vg) } + names = root_drives.map { |d| d.found_device&.name }.compact + # Return the first name in alphabetical order + names.min end - # return [Array] - def logical_volumes - volume_groups.flat_map(&:logical_volumes) + # @see #implicit_lvm_boot_device + # + # @return [Configs::VolumeGroup, nil] + def root_volume_group + volume_groups.find do |vg| + vg.logical_volumes.any? { |lv| lv.filesystem&.root? } + end + end + + # @see #implicit_lvm_boot_device + # + # @return [Boolean] + def drive_for_vg?(drive, volume_group) + return true if volume_group.physical_volumes_devices.any? { |d| drive.alias?(d) } + + volume_group.physical_volumes.any? do |pv| + drive.partitions.any? { |p| p.alias?(pv) } + end end end end From fa16efce406d5021c80674569429a0c804e7ef5e Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Thu, 3 Oct 2024 16:55:37 +0200 Subject: [PATCH 12/21] Support automatic LVM PVs - first step --- service/lib/y2storage/agama_proposal.rb | 25 ++-- .../proposal/agama_devices_creator.rb | 26 ++-- .../proposal/agama_devices_planner.rb | 2 +- .../y2storage/proposal/agama_lvm_helper.rb | 52 -------- .../y2storage/proposal/agama_space_maker.rb | 24 ++-- .../y2storage/proposal/agama_vg_planner.rb | 45 +++++-- service/test/fixtures/disks.yaml | 2 + service/test/y2storage/agama_proposal_test.rb | 111 ++++++++++++++++++ 8 files changed, 190 insertions(+), 97 deletions(-) delete mode 100644 service/lib/y2storage/proposal/agama_lvm_helper.rb diff --git a/service/lib/y2storage/agama_proposal.rb b/service/lib/y2storage/agama_proposal.rb index 13675f089..9d68c419d 100644 --- a/service/lib/y2storage/agama_proposal.rb +++ b/service/lib/y2storage/agama_proposal.rb @@ -145,7 +145,7 @@ def calculate_initial_planned(devicegraph) def clean_graph(devicegraph) remove_empty_partition_tables(devicegraph) # {Proposal::SpaceMaker#prepare_devicegraph} returns a copy of the given devicegraph. - space_maker.prepare_devicegraph(devicegraph, partitions_for_clean) + space_maker.prepare_devicegraph(devicegraph, disks_for_clean) end # Configures the disk devices on the given devicegraph to prefer the appropriate partition table @@ -206,14 +206,13 @@ def drives_with_empty_partition_table(devicegraph) devices.select { |d| d.partition_table && d.partitions.empty? } end - # Planned partitions that will hold the given planned devices + # Devices for which the mandatory actions must be executed # - # @return [Array] - def partitions_for_clean - # The current logic is quite trivial, but this is implemented as a separate method because - # some extra logic is expected in the future (eg. considering partitions on pre-existing - # RAIDs and more stuff). See the equivalent method at DevicegraphGenerator. - planned_devices.partitions + # @return [Array] names of partitionable devices + def disks_for_clean + return drives_names if config.boot_device.nil? || drives_names.include?(config.boot_device) + + drives_names + [config.boot_device] end # Creates the planned devices on a given devicegraph @@ -221,8 +220,14 @@ def partitions_for_clean # @param devicegraph [Devicegraph] the graph gets modified def create_devices(devicegraph) devices_creator = Proposal::AgamaDevicesCreator.new(devicegraph, issues_list) - names = config.drives.map(&:found_device).compact.map(&:name) - result = devices_creator.populated_devicegraph(planned_devices, names, space_maker) + result = devices_creator.populated_devicegraph(planned_devices, drives_names, space_maker) + end + + # Names of all the devices that correspond to a drive from the config + # + # @return [Array] + def drives_names + @drives_names ||= config.drives.map(&:found_device).compact.map(&:name) end # Equivalent device at the given devicegraph for the given configuration setting (eg. drive) diff --git a/service/lib/y2storage/proposal/agama_devices_creator.rb b/service/lib/y2storage/proposal/agama_devices_creator.rb index 8d5400b36..98d5b4163 100644 --- a/service/lib/y2storage/proposal/agama_devices_creator.rb +++ b/service/lib/y2storage/proposal/agama_devices_creator.rb @@ -20,7 +20,6 @@ # find current contact information at www.suse.com. require "y2storage/exceptions" -require "y2storage/proposal/agama_lvm_helper" require "y2storage/proposal/lvm_creator" require "y2storage/proposal/partition_creator" @@ -126,13 +125,11 @@ def process_devices def process_existing_partitionables partitions = partitions_for_existing(planned_devices) - # lvm_lvs = system_lvm_over_existing? ? system_lvs(planned_devices) : [] - lvm_lvs = [] - lvm_helper = AgamaLvmHelper.new(lvm_lvs) - # Check whether there is any chance of getting an unwanted order for the planned partitions # within a disk - space_result = provide_space(partitions, original_graph, lvm_helper) + space_result = space_maker.provide_space( + original_graph, partitions: partitions, volume_groups: automatic_vgs + ) self.devicegraph = space_result[:devicegraph] distribution = space_result[:partitions_distribution] @@ -146,6 +143,16 @@ def process_volume_groups planned_devices.vgs.map { |v| create_volume_group(v) } end + # Planned volume groups for which the proposal must automatically create the needed physical + # volumes + # + # @return [Array] + def automatic_vgs + planned_devices.select do |dev| + dev.is_a?(Planned::LvmVg) && dev.pvs_candidate_devices.any? + end + end + # Creates a volume group for the the given planned device. # # @param planned [Planned::LvmVg] @@ -174,13 +181,6 @@ def physical_volumes_for(vg_name) new_pvs + reused_pvs end - # @see #process_existing_partitionables - def provide_space(planned_partitions, devicegraph, lvm_helper) - result = space_maker.provide_space(devicegraph, planned_partitions, lvm_helper) - log.info "Found enough space" - result - end - # @see #process_existing_partitionables def grow_and_reuse_devices(distribution) planned_devices.select(&:reuse?).each do |planned| diff --git a/service/lib/y2storage/proposal/agama_devices_planner.rb b/service/lib/y2storage/proposal/agama_devices_planner.rb index a8788e040..79b81bb3f 100644 --- a/service/lib/y2storage/proposal/agama_devices_planner.rb +++ b/service/lib/y2storage/proposal/agama_devices_planner.rb @@ -75,7 +75,7 @@ def planned_drives(config) def planned_vgs(config) config.volume_groups.flat_map do |vg| planner = AgamaVgPlanner.new(devicegraph, issues_list) - planner.planned_devices(vg) + planner.planned_devices(vg, config) end end end diff --git a/service/lib/y2storage/proposal/agama_lvm_helper.rb b/service/lib/y2storage/proposal/agama_lvm_helper.rb deleted file mode 100644 index 80784a13b..000000000 --- a/service/lib/y2storage/proposal/agama_lvm_helper.rb +++ /dev/null @@ -1,52 +0,0 @@ -# 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 "y2storage/proposal/lvm_helper" -require "y2storage/proposal_settings" - -module Y2Storage - module Proposal - # LVM helper for Agama. - class AgamaLvmHelper < LvmHelper - # Constructor - def initialize(lvm_lvs) - super(lvm_lvs, guided_settings) - end - - private - - # Method used by the constructor to somehow simulate a typical Guided Proposal - def guided_settings - # Despite the "current_product" part in the name of the constructor, it only applies - # 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| - target.lvm_vg_strategy = :use_needed - target.lvm_vg_reuse = false - # TODO: Add encryption options. - target.encryption_password = nil - # target.encryption_pbkdf - # target.encryption_method - end - end - end - end -end diff --git a/service/lib/y2storage/proposal/agama_space_maker.rb b/service/lib/y2storage/proposal/agama_space_maker.rb index 69984ddf8..7fc75f822 100644 --- a/service/lib/y2storage/proposal/agama_space_maker.rb +++ b/service/lib/y2storage/proposal/agama_space_maker.rb @@ -25,30 +25,26 @@ module Y2Storage module Proposal # Space maker for Agama. + # + # FIXME: this class must dissappear. It does not implement any own logic compared to the + # original SpaceMaker. It simply encapsulates the conversion from Agama config to + # ProposalSpaceSettings. class AgamaSpaceMaker < SpaceMaker # @param disk_analyzer [DiskAnalyzer] # @param config [Agama::Storage::Config] def initialize(disk_analyzer, config) - super(disk_analyzer, guided_settings(config)) + super(disk_analyzer, space_settings(config)) end private - # Method used by the constructor to somehow simulate a typical Guided Proposal + # Method used by the constructor to convert the Agama config to ProposalSpaceSettings # # @param config [Agama::Storage::Config] - def guided_settings(config) - # Despite the "current_product" part in the name of the constructor, it only applies - # 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| - target.space_settings.strategy = :bigger_resize - target.space_settings.actions = space_actions(config) - - boot_device = config.boot_device - - target.root_device = boot_device - target.candidate_devices = [boot_device].compact + def space_settings(config) + Y2Storage::ProposalSpaceSettings.new.tap do |target| + target.strategy = :bigger_resize + target.actions = space_actions(config) end end diff --git a/service/lib/y2storage/proposal/agama_vg_planner.rb b/service/lib/y2storage/proposal/agama_vg_planner.rb index e510abad4..e56ffcdf1 100644 --- a/service/lib/y2storage/proposal/agama_vg_planner.rb +++ b/service/lib/y2storage/proposal/agama_vg_planner.rb @@ -28,26 +28,57 @@ module Proposal class AgamaVgPlanner < AgamaDevicePlanner # @param config [Agama::Storage::Configs::VolumeGroup] # @return [Array] - def planned_devices(config) - [planned_vg(config)] + def planned_devices(vg_config, config) + [planned_vg(vg_config, config)] end private - # @param config [Agama::Storage::Configs::VolumeGroup] + # @param vg_config [Agama::Storage::Configs::VolumeGroup] + # @param config [Agama::Storage::Config] # @return [Planned::LvmVg] - def planned_vg(config) + def planned_vg(vg_config, config) # TODO: A volume group name is expected. Otherwise, the planned physical volumes cannot # be associated to the planned volume group. Should the volume group name be # automatically generated if missing? # # @see AgamaDevicePlanner#configure_pv - Y2Storage::Planned::LvmVg.new(volume_group_name: config.name).tap do |planned| - planned.extent_size = config.extent_size - planned.lvs = planned_lvs(config) + Y2Storage::Planned::LvmVg.new(volume_group_name: vg_config.name).tap do |planned| + planned.extent_size = vg_config.extent_size + planned.lvs = planned_lvs(vg_config) + planned.size_strategy = :use_needed + planned.pvs_candidate_devices = devices_for_pvs(vg_config, config) + configure_pvs_encryption(planned, vg_config) end end + # Names of the devices that must be used to calculate automatic physical volumes + # for the given volume group + # + # @param vg_config [Agama::Storage::Configs::VolumeGroup] + # @param config [Agama::Storage::Config] + # @return [Array] + def devices_for_pvs(vg_config, config) + drives = vg_config.physical_volumes_devices.map do |dev_alias| + config.drives.find { |d| d.alias?(dev_alias) } + end.compact + + drives.map { |d| d.found_device.name } + end + + # Configures the encryption-related fields of the given planned volume group + # + # @param planned [Planned::LvmVg] + # @param config [Agama::Storage::Configs::VolumeGroup] + def configure_pvs_encryption(planned, config) + enc = config.physical_volumes_encryption + return unless enc + + planned.pvs_encryption_method = enc.method + planned.pvs_encryption_password = enc.password + planned.pvs_encryption_pbkdf = enc.pbkd_function + end + # @param config [Agama::Storage::Configs::VolumeGroup] # @return [Array] def planned_lvs(config) diff --git a/service/test/fixtures/disks.yaml b/service/test/fixtures/disks.yaml index 38dfaf5ff..cd4502450 100644 --- a/service/test/fixtures/disks.yaml +++ b/service/test/fixtures/disks.yaml @@ -12,11 +12,13 @@ size: 20 GiB name: "/dev/vda2" id: linux + label: "previous_root" file_system: btrfs - partition: size: 10 GiB name: "/dev/vda3" id: linux + label: "previous_home" file_system: xfs - disk: name: "/dev/vdb" diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 33783ab5c..29ce88213 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -1539,5 +1539,116 @@ def partition_config(name: nil, filesystem: nil, size: nil) ) end end + + context "when the config has LVM volume groups with generated physical volumes" do + let(:scenario) { "disks.yaml" } + + let(:config_json) do + { + drives: [ + { + alias: "vda", + partitions: [ + { + search: "/dev/vda2", + size: { min: "0", max: "current" } + }, + { + size: { min: "4 GiB" }, + filesystem: { path: "/foo" } + } + ] + }, + { + alias: "vdb" + }, + ], + volumeGroups: [ + { + name: "system", + physicalVolumes: [ + { generate: { targetDevices: ["vda"] } } + ], + logicalVolumes: [ + { + name: "root", + size: "10 GiB", + filesystem: { + path: "/", + type: "btrfs" + } + }, + { + name: "data", + size: "10 GiB", + filesystem: { type: "xfs" } + } + ] + }, + { + name: "vg1", + physicalVolumes: [ + { generate: { + targetDevices: ["vdb"], + "encryption": { "luks2": { "password": "s3cr3t" } } + } + } + ], + logicalVolumes: [ + { + name: "home", + filesystem: { + path: "/home", + type: "xfs" + }, + size: "20 GiB" + } + ] + } + ] + } + end + + before do + allow_any_instance_of(Y2Storage::Partition) + .to(receive(:detect_resize_info)) + .and_return(resize_info) + end + + let(:resize_info) do + instance_double( + Y2Storage::ResizeInfo, resize_ok?: true, + min_size: Y2Storage::DiskSize::GiB(3), max_size: Y2Storage::DiskSize::GiB(35) + ) + end + + it "proposes the expected devices" do + devicegraph = proposal.propose + + resized = devicegraph.find_by_name("/dev/vda2") + expect(resized.filesystem.label).to eq("previous_root") + expect(resized.size).to be > 15.GiB + expect(resized.size).to be < 16.GiB + + foo = devicegraph.find_by_name("/dev/vda4") + expect(foo.filesystem.mount_path).to eq("/foo") + expect(foo.size).to be > 4.GiB + expect(foo.size).to be < 5.GiB + + system = devicegraph.find_by_name("/dev/system") + expect(system.lvm_lvs.size).to eq 2 + expect(Y2Storage::DiskSize.sum(system.lvm_lvs.map(&:size))).to eq 20.GiB + expect(system.lvm_pvs.size).to eq 2 + + vg1 = devicegraph.find_by_name("/dev/vg1") + expect(vg1.lvm_lvs.size).to eq 1 + expect(vg1.lvm_lvs.first.size).to eq 20.GiB + expect(vg1.lvm_pvs.size).to eq 1 + + pv_vg1 = vg1.lvm_pvs.first + expect(pv_vg1.blk_device.is?(:encryption)).to eq true + expect(pv_vg1.blk_device.type.is?(:luks2)).to eq true + end + end end end From 120b0db854c6f097b3c12a1eeb1214992e80bcdc Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 7 Oct 2024 17:10:59 +0200 Subject: [PATCH 13/21] Separate LVM proposal tests --- .../test/y2storage/agama_proposal_lvm_test.rb | 413 ++++++++++++++++++ service/test/y2storage/agama_proposal_test.rb | 357 --------------- 2 files changed, 413 insertions(+), 357 deletions(-) create mode 100644 service/test/y2storage/agama_proposal_lvm_test.rb diff --git a/service/test/y2storage/agama_proposal_lvm_test.rb b/service/test/y2storage/agama_proposal_lvm_test.rb new file mode 100644 index 000000000..c0474bf2f --- /dev/null +++ b/service/test/y2storage/agama_proposal_lvm_test.rb @@ -0,0 +1,413 @@ +# 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 "../agama/storage/storage_helpers" +require "agama/config" +require "agama/storage/config" +require "agama/storage/config_conversions/from_json" +require "y2storage" +require "y2storage/agama_proposal" + +describe Y2Storage::AgamaProposal do + using Y2Storage::Refinements::SizeCasts + include Agama::RSpec::StorageHelpers + + subject(:proposal) do + described_class.new(config, issues_list: issues_list) + end + + let(:config) { config_from_json } + + let(:config_from_json) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json) + .convert + end + + let(:issues_list) { [] } + + before do + mock_storage(devicegraph: scenario) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE).to receive(:possible?).and_return(true) + end + + let(:scenario) { "empty-hd-50GiB.yaml" } + + describe "#propose" do + context "when the config has LVM volume groups" do + let(:scenario) { "empty-hd-50GiB.yaml" } + + let(:config_json) do + { + drives: [ + { + partitions: [ + { + alias: "system-pv", + size: "40 GiB" + }, + { + alias: "vg1-pv", + size: "5 GiB" + } + ] + } + ], + volumeGroups: [ + { + name: "system", + extentSize: "2 MiB", + physicalVolumes: ["system-pv"], + logicalVolumes: [ + { + name: "root", + size: "10 GiB", + filesystem: { + path: "/", + type: "btrfs" + }, + encryption: { + luks2: { password: "12345" } + } + }, + { + alias: "system-pool", + name: "pool", + pool: true, + size: "20 GiB", + stripes: 10, + stripeSize: "4 KiB" + }, + { + name: "data", + size: "50 GiB", + usedPool: "system-pool", + filesystem: { type: "xfs" } + } + ] + }, + { + name: "vg1", + physicalVolumes: ["vg1-pv"], + logicalVolumes: [ + { + name: "home", + filesystem: { + path: "/home", + type: "xfs" + }, + size: "2 GiB" + } + ] + } + ] + } + end + + it "proposes the expected devices" do + devicegraph = proposal.propose + + expect(devicegraph.lvm_vgs).to contain_exactly( + an_object_having_attributes( + vg_name: "system", + extent_size: 2.MiB + ), + an_object_having_attributes( + vg_name: "vg1", + extent_size: 4.MiB + ) + ) + + system_vg = devicegraph.find_by_name("/dev/system") + system_pvs = system_vg.lvm_pvs.map(&:plain_blk_device) + system_lvs = system_vg.lvm_lvs + + expect(system_pvs).to contain_exactly( + an_object_having_attributes(name: "/dev/sda2", size: 40.GiB) + ) + expect(system_lvs).to contain_exactly( + an_object_having_attributes( + lv_name: "root", + lv_type: Y2Storage::LvType::NORMAL, + size: 10.GiB, + filesystem: an_object_having_attributes( + type: Y2Storage::Filesystems::Type::BTRFS, + mount_path: "/" + ), + encryption: an_object_having_attributes( + type: Y2Storage::EncryptionType::LUKS2, + password: "12345" + ) + ), + an_object_having_attributes( + lv_name: "pool", + lv_type: Y2Storage::LvType::THIN_POOL, + size: 20.GiB, + filesystem: be_nil, + encryption: be_nil, + stripes: 10, + stripe_size: 4.KiB, + lvm_lvs: contain_exactly( + an_object_having_attributes( + lv_name: "data", + lv_type: Y2Storage::LvType::THIN, + size: 50.GiB, + filesystem: an_object_having_attributes( + type: Y2Storage::Filesystems::Type::XFS + ) + ) + ) + ) + ) + + vg1 = devicegraph.find_by_name("/dev/vg1") + vg1_pvs = vg1.lvm_pvs.map(&:plain_blk_device) + vg1_lvs = vg1.lvm_lvs + expect(vg1_pvs).to contain_exactly( + an_object_having_attributes(name: "/dev/sda3", size: 5.GiB) + ) + expect(vg1_lvs).to contain_exactly( + an_object_having_attributes( + lv_name: "home", + lv_type: Y2Storage::LvType::NORMAL, + size: 2.GiB, + filesystem: an_object_having_attributes( + type: Y2Storage::Filesystems::Type::XFS, + mount_path: "/home" + ) + ) + ) + end + end + + context "when a LVM physical volume is not found" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + size: "40 GiB" + }, + { + alias: "pv1", + size: "5 GiB" + } + ] + } + ], + volumeGroups: [ + { + name: "system", + extentSize: "2 MiB", + physicalVolumes: ["pv1", "pv2"], + logicalVolumes: [ + { + name: "root", + filesystem: { + path: "/" + } + } + ] + } + ] + } + end + + it "aborts the proposal process" do + proposal.propose + expect(proposal.failed?).to eq true + end + + it "reports the corresponding error" do + proposal.propose + expect(proposal.issues_list).to include an_object_having_attributes( + description: /no LVM physical volume with alias 'pv2'/, + severity: Agama::Issue::Severity::ERROR + ) + end + end + + context "when a LVM thin pool volume is not found" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + size: "40 GiB" + }, + { + alias: "pv1", + size: "5 GiB" + } + ] + } + ], + volumeGroups: [ + { + name: "system", + extentSize: "2 MiB", + physicalVolumes: ["pv1"], + logicalVolumes: [ + { + pool: true + }, + { + name: "root", + filesystem: { + path: "/" + }, + usedPool: "pool" + } + ] + } + ] + } + end + + it "aborts the proposal process" do + proposal.propose + expect(proposal.failed?).to eq true + end + + it "reports the corresponding error" do + proposal.propose + expect(proposal.issues_list).to include an_object_having_attributes( + description: /no LVM thin pool volume with alias 'pool'/, + severity: Agama::Issue::Severity::ERROR + ) + end + end + + context "when the config has LVM volume groups with generated physical volumes" do + let(:scenario) { "disks.yaml" } + + let(:config_json) do + { + drives: [ + { + alias: "vda", + partitions: [ + { + search: "/dev/vda2", + size: { min: "0", max: "current" } + }, + { + size: { min: "4 GiB" }, + filesystem: { path: "/foo" } + } + ] + }, + { + alias: "vdb" + }, + ], + volumeGroups: [ + { + name: "system", + physicalVolumes: [ + { generate: { targetDevices: ["vda"] } } + ], + logicalVolumes: [ + { + name: "root", + size: "10 GiB", + filesystem: { + path: "/", + type: "btrfs" + } + }, + { + name: "data", + size: "10 GiB", + filesystem: { type: "xfs" } + } + ] + }, + { + name: "vg1", + physicalVolumes: [ + { generate: { + targetDevices: ["vdb"], + "encryption": { "luks2": { "password": "s3cr3t" } } + } + } + ], + logicalVolumes: [ + { + name: "home", + filesystem: { + path: "/home", + type: "xfs" + }, + size: "20 GiB" + } + ] + } + ] + } + end + + before do + allow_any_instance_of(Y2Storage::Partition) + .to(receive(:detect_resize_info)) + .and_return(resize_info) + end + + let(:resize_info) do + instance_double( + Y2Storage::ResizeInfo, resize_ok?: true, + min_size: Y2Storage::DiskSize::GiB(3), max_size: Y2Storage::DiskSize::GiB(35) + ) + end + + it "proposes the expected devices" do + devicegraph = proposal.propose + + resized = devicegraph.find_by_name("/dev/vda2") + expect(resized.filesystem.label).to eq("previous_root") + expect(resized.size).to be > 15.GiB + expect(resized.size).to be < 16.GiB + + foo = devicegraph.find_by_name("/dev/vda4") + expect(foo.filesystem.mount_path).to eq("/foo") + expect(foo.size).to be > 4.GiB + expect(foo.size).to be < 5.GiB + + system = devicegraph.find_by_name("/dev/system") + expect(system.lvm_lvs.size).to eq 2 + expect(Y2Storage::DiskSize.sum(system.lvm_lvs.map(&:size))).to eq 20.GiB + expect(system.lvm_pvs.size).to eq 2 + + vg1 = devicegraph.find_by_name("/dev/vg1") + expect(vg1.lvm_lvs.size).to eq 1 + expect(vg1.lvm_lvs.first.size).to eq 20.GiB + expect(vg1.lvm_pvs.size).to eq 1 + + pv_vg1 = vg1.lvm_pvs.first + expect(pv_vg1.blk_device.is?(:encryption)).to eq true + expect(pv_vg1.blk_device.type.is?(:luks2)).to eq true + end + end + end +end diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index 29ce88213..fbc0924d0 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -1293,362 +1293,5 @@ def partition_config(name: nil, filesystem: nil, size: nil) end end end - - context "when the config has LVM volume groups" do - let(:scenario) { "empty-hd-50GiB.yaml" } - - let(:config_json) do - { - drives: [ - { - partitions: [ - { - alias: "system-pv", - size: "40 GiB" - }, - { - alias: "vg1-pv", - size: "5 GiB" - } - ] - } - ], - volumeGroups: [ - { - name: "system", - extentSize: "2 MiB", - physicalVolumes: ["system-pv"], - logicalVolumes: [ - { - name: "root", - size: "10 GiB", - filesystem: { - path: "/", - type: "btrfs" - }, - encryption: { - luks2: { password: "12345" } - } - }, - { - alias: "system-pool", - name: "pool", - pool: true, - size: "20 GiB", - stripes: 10, - stripeSize: "4 KiB" - }, - { - name: "data", - size: "50 GiB", - usedPool: "system-pool", - filesystem: { type: "xfs" } - } - ] - }, - { - name: "vg1", - physicalVolumes: ["vg1-pv"], - logicalVolumes: [ - { - name: "home", - filesystem: { - path: "/home", - type: "xfs" - }, - size: "2 GiB" - } - ] - } - ] - } - end - - it "proposes the expected devices" do - devicegraph = proposal.propose - - expect(devicegraph.lvm_vgs).to contain_exactly( - an_object_having_attributes( - vg_name: "system", - extent_size: 2.MiB - ), - an_object_having_attributes( - vg_name: "vg1", - extent_size: 4.MiB - ) - ) - - system_vg = devicegraph.find_by_name("/dev/system") - system_pvs = system_vg.lvm_pvs.map(&:plain_blk_device) - system_lvs = system_vg.lvm_lvs - - expect(system_pvs).to contain_exactly( - an_object_having_attributes(name: "/dev/sda2", size: 40.GiB) - ) - expect(system_lvs).to contain_exactly( - an_object_having_attributes( - lv_name: "root", - lv_type: Y2Storage::LvType::NORMAL, - size: 10.GiB, - filesystem: an_object_having_attributes( - type: Y2Storage::Filesystems::Type::BTRFS, - mount_path: "/" - ), - encryption: an_object_having_attributes( - type: Y2Storage::EncryptionType::LUKS2, - password: "12345" - ) - ), - an_object_having_attributes( - lv_name: "pool", - lv_type: Y2Storage::LvType::THIN_POOL, - size: 20.GiB, - filesystem: be_nil, - encryption: be_nil, - stripes: 10, - stripe_size: 4.KiB, - lvm_lvs: contain_exactly( - an_object_having_attributes( - lv_name: "data", - lv_type: Y2Storage::LvType::THIN, - size: 50.GiB, - filesystem: an_object_having_attributes( - type: Y2Storage::Filesystems::Type::XFS - ) - ) - ) - ) - ) - - vg1 = devicegraph.find_by_name("/dev/vg1") - vg1_pvs = vg1.lvm_pvs.map(&:plain_blk_device) - vg1_lvs = vg1.lvm_lvs - expect(vg1_pvs).to contain_exactly( - an_object_having_attributes(name: "/dev/sda3", size: 5.GiB) - ) - expect(vg1_lvs).to contain_exactly( - an_object_having_attributes( - lv_name: "home", - lv_type: Y2Storage::LvType::NORMAL, - size: 2.GiB, - filesystem: an_object_having_attributes( - type: Y2Storage::Filesystems::Type::XFS, - mount_path: "/home" - ) - ) - ) - end - end - - context "when a LVM physical volume is not found" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - size: "40 GiB" - }, - { - alias: "pv1", - size: "5 GiB" - } - ] - } - ], - volumeGroups: [ - { - name: "system", - extentSize: "2 MiB", - physicalVolumes: ["pv1", "pv2"], - logicalVolumes: [ - { - name: "root", - filesystem: { - path: "/" - } - } - ] - } - ] - } - end - - it "aborts the proposal process" do - proposal.propose - expect(proposal.failed?).to eq true - end - - it "reports the corresponding error" do - proposal.propose - expect(proposal.issues_list).to include an_object_having_attributes( - description: /no LVM physical volume with alias 'pv2'/, - severity: Agama::Issue::Severity::ERROR - ) - end - end - - context "when a LVM thin pool volume is not found" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - size: "40 GiB" - }, - { - alias: "pv1", - size: "5 GiB" - } - ] - } - ], - volumeGroups: [ - { - name: "system", - extentSize: "2 MiB", - physicalVolumes: ["pv1"], - logicalVolumes: [ - { - pool: true - }, - { - name: "root", - filesystem: { - path: "/" - }, - usedPool: "pool" - } - ] - } - ] - } - end - - it "aborts the proposal process" do - proposal.propose - expect(proposal.failed?).to eq true - end - - it "reports the corresponding error" do - proposal.propose - expect(proposal.issues_list).to include an_object_having_attributes( - description: /no LVM thin pool volume with alias 'pool'/, - severity: Agama::Issue::Severity::ERROR - ) - end - end - - context "when the config has LVM volume groups with generated physical volumes" do - let(:scenario) { "disks.yaml" } - - let(:config_json) do - { - drives: [ - { - alias: "vda", - partitions: [ - { - search: "/dev/vda2", - size: { min: "0", max: "current" } - }, - { - size: { min: "4 GiB" }, - filesystem: { path: "/foo" } - } - ] - }, - { - alias: "vdb" - }, - ], - volumeGroups: [ - { - name: "system", - physicalVolumes: [ - { generate: { targetDevices: ["vda"] } } - ], - logicalVolumes: [ - { - name: "root", - size: "10 GiB", - filesystem: { - path: "/", - type: "btrfs" - } - }, - { - name: "data", - size: "10 GiB", - filesystem: { type: "xfs" } - } - ] - }, - { - name: "vg1", - physicalVolumes: [ - { generate: { - targetDevices: ["vdb"], - "encryption": { "luks2": { "password": "s3cr3t" } } - } - } - ], - logicalVolumes: [ - { - name: "home", - filesystem: { - path: "/home", - type: "xfs" - }, - size: "20 GiB" - } - ] - } - ] - } - end - - before do - allow_any_instance_of(Y2Storage::Partition) - .to(receive(:detect_resize_info)) - .and_return(resize_info) - end - - let(:resize_info) do - instance_double( - Y2Storage::ResizeInfo, resize_ok?: true, - min_size: Y2Storage::DiskSize::GiB(3), max_size: Y2Storage::DiskSize::GiB(35) - ) - end - - it "proposes the expected devices" do - devicegraph = proposal.propose - - resized = devicegraph.find_by_name("/dev/vda2") - expect(resized.filesystem.label).to eq("previous_root") - expect(resized.size).to be > 15.GiB - expect(resized.size).to be < 16.GiB - - foo = devicegraph.find_by_name("/dev/vda4") - expect(foo.filesystem.mount_path).to eq("/foo") - expect(foo.size).to be > 4.GiB - expect(foo.size).to be < 5.GiB - - system = devicegraph.find_by_name("/dev/system") - expect(system.lvm_lvs.size).to eq 2 - expect(Y2Storage::DiskSize.sum(system.lvm_lvs.map(&:size))).to eq 20.GiB - expect(system.lvm_pvs.size).to eq 2 - - vg1 = devicegraph.find_by_name("/dev/vg1") - expect(vg1.lvm_lvs.size).to eq 1 - expect(vg1.lvm_lvs.first.size).to eq 20.GiB - expect(vg1.lvm_pvs.size).to eq 1 - - pv_vg1 = vg1.lvm_pvs.first - expect(pv_vg1.blk_device.is?(:encryption)).to eq true - expect(pv_vg1.blk_device.type.is?(:luks2)).to eq true - end - end end end From ab2c93493a0754a4b251d5d2fd9b3756284e4108 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 8 Oct 2024 13:46:54 +0200 Subject: [PATCH 14/21] Add more LVM test --- .../test/y2storage/agama_proposal_lvm_test.rb | 91 +++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/service/test/y2storage/agama_proposal_lvm_test.rb b/service/test/y2storage/agama_proposal_lvm_test.rb index c0474bf2f..0c7130eaf 100644 --- a/service/test/y2storage/agama_proposal_lvm_test.rb +++ b/service/test/y2storage/agama_proposal_lvm_test.rb @@ -313,14 +313,14 @@ size: { min: "0", max: "current" } }, { - size: { min: "4 GiB" }, + size: { min: "4 GiB" }, filesystem: { path: "/foo" } } ] }, { - alias: "vdb" - }, + alias: "vdb" + } ], volumeGroups: [ { @@ -347,10 +347,11 @@ { name: "vg1", physicalVolumes: [ - { generate: { + { + generate: { targetDevices: ["vdb"], - "encryption": { "luks2": { "password": "s3cr3t" } } - } + encryption: { luks2: { password: "s3cr3t" } } + } } ], logicalVolumes: [ @@ -409,5 +410,83 @@ expect(pv_vg1.blk_device.type.is?(:luks2)).to eq true end end + + context "when two volume groups with generated physical volumes share a disk" do + let(:scenario) { "disks.yaml" } + + let(:config_json) do + { + drives: [ + { alias: "vda" }, + { alias: "vdb" }, + { alias: "vdc" } + ], + volumeGroups: [ + { + name: "system", + physicalVolumes: [ + { generate: { targetDevices: ["vdb", "vdc"] } } + ], + logicalVolumes: [ + { + name: "root", + size: "10 GiB", + filesystem: { + path: "/", + type: "btrfs" + } + }, + { + name: "home", + size: { + min: "30 GiB" + }, + filesystem: { + path: "/home", + type: "xfs" + } + } + ] + }, + { + name: "vg1", + physicalVolumes: [ + { + generate: { + targetDevices: ["vdc"], + encryption: { luks1: { password: "s3cr3t" } } + } + } + ], + logicalVolumes: [ + { + name: "data", + filesystem: { type: "xfs" }, + size: { min: "30 GiB", max: "40 GiB" } + } + ] + } + ] + } + end + + it "proposes the expected devices" do + devicegraph = proposal.propose + + system = devicegraph.find_by_name("/dev/system") + expect(system.lvm_lvs.map { |lv| lv.mount_point.path }).to contain_exactly("/", "/home") + expect(system.lvm_pvs.map { |pv| pv.blk_device.partitionable.name }) + .to contain_exactly("/dev/vdb", "/dev/vdc") + + vg1 = devicegraph.find_by_name("/dev/vg1") + expect(vg1.lvm_lvs.map(&:lv_name)).to contain_exactly("data") + expect(vg1.lvm_pvs.map { |pv| pv.plain_blk_device.partitionable.name }) + .to contain_exactly("/dev/vdc") + + pv_vg1 = vg1.lvm_pvs.first + expect(pv_vg1.blk_device.is?(:encryption)).to eq true + expect(pv_vg1.blk_device.type.is?(:luks1)).to eq true + end + end end end From 774ba669de8e37d2cda00e3fbb2f57a933afb933 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Tue, 8 Oct 2024 14:24:35 +0200 Subject: [PATCH 15/21] Update dependency on yast2-storage-ng --- service/package/gem2rpm.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/package/gem2rpm.yml b/service/package/gem2rpm.yml index e3225ad52..bdd561fc9 100644 --- a/service/package/gem2rpm.yml +++ b/service/package/gem2rpm.yml @@ -39,7 +39,7 @@ Requires: yast2-iscsi-client >= 4.5.7 Requires: yast2-network Requires: yast2-proxy - Requires: yast2-storage-ng >= 5.0.18 + Requires: yast2-storage-ng >= 5.0.20 Requires: yast2-users %ifarch s390 s390x Requires: yast2-s390 >= 4.6.4 From 23af8f14e5445704331322b5ab4078c7fd0594ac Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 9 Oct 2024 10:22:42 +0200 Subject: [PATCH 16/21] Service: changelog --- service/package/rubygem-agama-yast.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index cfd7f956c..ad5609505 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Oct 8 12:25:08 UTC 2024 - Ancor Gonzalez Sosa + +- Storage: added support for automatic creation of physical volumes + (gh#agama-project/agama#1655). + ------------------------------------------------------------------- Mon Oct 7 06:58:48 UTC 2024 - José Iván López González From 483cfd513268e259dde89133e496558da44275cd Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Wed, 9 Oct 2024 11:58:52 +0200 Subject: [PATCH 17/21] Simplify code --- service/lib/y2storage/agama_proposal.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/service/lib/y2storage/agama_proposal.rb b/service/lib/y2storage/agama_proposal.rb index 9d68c419d..e9f42e78d 100644 --- a/service/lib/y2storage/agama_proposal.rb +++ b/service/lib/y2storage/agama_proposal.rb @@ -210,9 +210,7 @@ def drives_with_empty_partition_table(devicegraph) # # @return [Array] names of partitionable devices def disks_for_clean - return drives_names if config.boot_device.nil? || drives_names.include?(config.boot_device) - - drives_names + [config.boot_device] + (drives_names + [config.boot_device]).compact.uniq end # Creates the planned devices on a given devicegraph From c50672e473339f47a426a1360c5436fde3f2ccdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 9 Oct 2024 13:46:57 +0100 Subject: [PATCH 18/21] refactor(rust): introduce a new "transfer" module * This module replaces the old "download" function. --- rust/agama-cli/src/lib.rs | 8 ++++---- rust/agama-cli/src/profile.rs | 19 +++-------------- rust/agama-lib/src/error.rs | 7 +++++-- rust/agama-lib/src/lib.rs | 1 + rust/agama-lib/src/transfer.rs | 37 ++++++++++++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 22 deletions(-) create mode 100644 rust/agama-lib/src/transfer.rs diff --git a/rust/agama-cli/src/lib.rs b/rust/agama-cli/src/lib.rs index cc0b77f71..42d265e55 100644 --- a/rust/agama-cli/src/lib.rs +++ b/rust/agama-cli/src/lib.rs @@ -30,9 +30,9 @@ mod progress; mod questions; use crate::error::CliError; -use agama_lib::error::ServiceError; -use agama_lib::manager::ManagerClient; -use agama_lib::progress::ProgressMonitor; +use agama_lib::{ + error::ServiceError, manager::ManagerClient, progress::ProgressMonitor, transfer::Transfer, +}; use auth::run as run_auth_cmd; use commands::Commands; use config::run as run_config_cmd; @@ -158,7 +158,7 @@ pub async fn run_command(cli: Cli) -> Result<(), ServiceError> { Commands::Questions(subcommand) => run_questions_cmd(subcommand).await?, Commands::Logs(subcommand) => run_logs_cmd(subcommand).await?, Commands::Auth(subcommand) => run_auth_cmd(subcommand).await?, - Commands::Download { url } => crate::profile::download(&url, std::io::stdout())?, + Commands::Download { url } => Transfer::get(&url, std::io::stdout())?, }; Ok(()) diff --git a/rust/agama-cli/src/profile.rs b/rust/agama-cli/src/profile.rs index e5cde79d9..9585cb4c3 100644 --- a/rust/agama-cli/src/profile.rs +++ b/rust/agama-cli/src/profile.rs @@ -22,15 +22,15 @@ use agama_lib::{ auth::AuthToken, install_settings::InstallSettings, profile::{AutoyastProfile, ProfileEvaluator, ProfileValidator, ValidationResult}, + transfer::Transfer, Store as SettingsStore, }; use anyhow::Context; use clap::Subcommand; -use curl::easy::Easy; use std::os::unix::process::CommandExt; use std::{ fs::File, - io::{stdout, Write}, + io::stdout, path::{Path, PathBuf}, process::Command, }; @@ -78,19 +78,6 @@ pub enum ProfileCommands { }, } -pub fn download(url: &str, mut out_fd: impl Write) -> anyhow::Result<()> { - let mut handle = Easy::new(); - handle.url(url)?; - - let mut transfer = handle.transfer(); - transfer.write_function(|buf| - // unwrap here is ok, as we want to kill download if we failed to write content - Ok(out_fd.write(buf).unwrap()))?; - transfer.perform()?; - - Ok(()) -} - fn validate(path: &PathBuf) -> anyhow::Result<()> { let validator = ProfileValidator::default_schema()?; // let path = Path::new(&path); @@ -138,7 +125,7 @@ async fn import(url_string: String, dir: Option) -> anyhow::Result<()> AutoyastProfile::new(&url)?.read_into(output_fd)?; } else { // just download profile - download(&url_string, output_fd)?; + Transfer::get(&url_string, output_fd)?; } // exec shell scripts diff --git a/rust/agama-lib/src/error.rs b/rust/agama-lib/src/error.rs index ece6b685e..380aaafd8 100644 --- a/rust/agama-lib/src/error.rs +++ b/rust/agama-lib/src/error.rs @@ -18,12 +18,13 @@ // To contact SUSE LLC about this file by physical or electronic mail, you may // find current contact information at www.suse.com. -use curl; use serde_json; use std::io; use thiserror::Error; use zbus::{self, zvariant}; +use crate::transfer::TransferError; + #[derive(Error, Debug)] pub enum ServiceError { #[error("Cannot generate Agama logs: {0}")] @@ -66,12 +67,14 @@ pub enum ServiceError { // Specific error when something does not work as expected, but it is not user fault #[error("Internal error. Please report a bug and attach logs. Details: {0}")] InternalError(String), + #[error("Could not read the file: '{0}'")] + CouldNotTransferFile(#[from] TransferError), } #[derive(Error, Debug)] pub enum ProfileError { #[error("Could not read the profile")] - Unreachable(#[from] curl::Error), + Unreachable(#[from] TransferError), #[error("Jsonnet evaluation failed:\n{0}")] EvaluationError(String), #[error("I/O error")] diff --git a/rust/agama-lib/src/lib.rs b/rust/agama-lib/src/lib.rs index 524081496..6c86b035a 100644 --- a/rust/agama-lib/src/lib.rs +++ b/rust/agama-lib/src/lib.rs @@ -63,6 +63,7 @@ pub mod proxies; mod store; pub use store::Store; pub mod questions; +pub mod transfer; use crate::error::ServiceError; use reqwest::{header, Client}; diff --git a/rust/agama-lib/src/transfer.rs b/rust/agama-lib/src/transfer.rs new file mode 100644 index 000000000..2566c93c7 --- /dev/null +++ b/rust/agama-lib/src/transfer.rs @@ -0,0 +1,37 @@ +//! File transfer API for Agama. +//! +//! Implement a file transfer API which, in the future, will support Agama specific URLs. Check the +//! YaST document about [URL handling in the +//! installer](https://github.com/yast/yast-installation/blob/master/doc/url.md) for further +//! information. +//! +//! At this point, it only supports those schemes supported by CURL. + +use std::io::Write; + +use curl::easy::Easy; +use thiserror::Error; + +#[derive(Error, Debug)] +#[error(transparent)] +pub struct TransferError(#[from] curl::Error); +pub type TransferResult = Result; + +/// File transfer API +pub struct Transfer {} + +impl Transfer { + /// Retrieves and writes the data from an URL + /// + /// * `url`: URL to get the data from. + /// * `out_fd`: where to write the data. + pub fn get(url: &str, mut out_fd: impl Write) -> TransferResult<()> { + let mut handle = Easy::new(); + handle.url(url)?; + + let mut transfer = handle.transfer(); + transfer.write_function(|buf| Ok(out_fd.write(buf).unwrap()))?; + transfer.perform()?; + Ok(()) + } +} From f380791e77a552a061fdc397e0020f0c68cb7322 Mon Sep 17 00:00:00 2001 From: Victorhck Date: Thu, 10 Oct 2024 17:39:57 +0200 Subject: [PATCH 19/21] Update project URL in About.tsx --- web/src/components/core/About.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/core/About.tsx b/web/src/components/core/About.tsx index bad094d80..8d57309ce 100644 --- a/web/src/components/core/About.tsx +++ b/web/src/components/core/About.tsx @@ -80,7 +80,7 @@ data loss.", // TRANSLATORS: content of the "About" popup (2/2) // %s is replaced by the project URL _("For more information, please visit the project's repository at %s."), - "https://github.com/openSUSE/agama", + "https://github.com/agama-project/agama", )} From 00955a3e74b253e33a1b6fb05601b13c57203d4f Mon Sep 17 00:00:00 2001 From: Victorhck Date: Thu, 10 Oct 2024 17:51:33 +0200 Subject: [PATCH 20/21] change link to project website in About.tsx --- web/src/components/core/About.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/components/core/About.tsx b/web/src/components/core/About.tsx index 8d57309ce..0eacb2ce8 100644 --- a/web/src/components/core/About.tsx +++ b/web/src/components/core/About.tsx @@ -79,8 +79,8 @@ data loss.", {sprintf( // TRANSLATORS: content of the "About" popup (2/2) // %s is replaced by the project URL - _("For more information, please visit the project's repository at %s."), - "https://github.com/agama-project/agama", + _("For more information, please visit the project's page at %s."), + "https://agama-project.github.io/", )} From a234daeb9c2f009c2e4460bbdca0705de145110a Mon Sep 17 00:00:00 2001 From: Victorhck Date: Thu, 10 Oct 2024 17:58:02 +0200 Subject: [PATCH 21/21] Update agama-web-ui.changes --- web/package/agama-web-ui.changes | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index fab8f1b7c..d5b43ad4c 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Thu Oct 10 17:55:34 UTC 2024 - Victorhck + +- Update URL in About section (gh#agama-project/agama#1663). + ------------------------------------------------------------------- Thu Oct 3 10:20:34 UTC 2024 - Imobach Gonzalez Sosa