From 9b6a92b80a1a261267480dd8fad788ffc53f2a61 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, 26 Aug 2024 12:20:37 +0100 Subject: [PATCH] WIP --- service/lib/agama/dbus/storage/manager.rb | 177 ++---- service/lib/agama/dbus/storage/proposal.rb | 4 +- service/lib/agama/storage/config.rb | 2 +- service/lib/agama/storage/proposal.rb | 137 ++++- .../lib/agama/storage/proposal_strategies.rb | 3 +- .../storage/proposal_strategies/agama.rb | 73 +++ .../agama/storage/proposal_strategies/base.rb | 7 - .../y2storage/proposal/agama_space_maker.rb | 2 +- .../test/agama/dbus/storage/manager_test.rb | 200 ++++--- .../test/agama/dbus/storage/proposal_test.rb | 12 +- service/test/agama/storage/proposal_test.rb | 556 +++++++++++++++--- .../agama/storage/proposal_volumes_test.rb | 10 +- 12 files changed, 850 insertions(+), 333 deletions(-) create mode 100644 service/lib/agama/storage/proposal_strategies/agama.rb diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index b92a1b9c9e..0514b159b5 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -45,7 +45,7 @@ module Agama module DBus module Storage # D-Bus object to manage storage installation - class Manager < BaseObject # rubocop:disable Metrics/ClassLength + class Manager < BaseObject include WithISCSIAuth include WithServiceStatus include ::DBus::ObjectManager @@ -91,30 +91,31 @@ def probe busy_while { backend.probe } end - # Calculates a proposal (guided or AutoYaST) from a given storage config. + # Applies the given serialized config according to the JSON schema. # - # @raise If config is not valid. + # The JSON schema supports two different variants: + # { "storage": ... } or { "legacyAutoyastStorage": ... }. # - # @param serialized_config [String] Serialized storage config. It can be storage or legacy - # AutoYaST settings: { "storage": ... } vs { "legacyAutoyastStorage": ... }. - def apply_storage_config(serialized_config) - config_json = JSON.parse(serialized_config, symbolize_names: true) + # @note The guided settings ({ "storage": { "guided": ... } }) are supported too, but it + # will be removed from the JSON schema. + # + # @raise If the config is not valid. + # + # @param serialized_config [String] Serialized storage config. + # @return [Integer] 0 success; 1 error + def apply_config(serialized_config) + logger.info("Setting storage config from D-Bus: #{serialized_config}") - if (settings_json = config_json.dig(:storage, :guided)) - calculate_guided_proposal(settings_json) - elsif (settings_json = config_json[:legacyAutoyastStorage]) - calculate_autoyast_proposal(settings_json) - else - raise "Invalid config: #{serialized_config}" - end + config_json = JSON.parse(serialized_config, symbolize_names: true) + proposal.calculate_from_json(config_json) + proposal.success? ? 0 : 1 end - # Serialized storage config. It can contain storage or legacy AutoYaST settings: - # { "storage": ... } vs { "legacyAutoyastStorage": ... } + # Gets and serializes the storage config used to calculate the current proposal. # - # @return [String] - def serialized_storage_config - JSON.pretty_generate(storage_config) + # @return [String] Serialized config according to the JSON schema. + def recover_config + JSON.pretty_generate(proposal.config_json) end def install @@ -135,9 +136,9 @@ def deprecated_system dbus_interface STORAGE_INTERFACE do dbus_method(:Probe) { probe } dbus_method(:SetConfig, "in serialized_config:s, out result:u") do |serialized_config| - busy_while { apply_storage_config(serialized_config) } + busy_while { apply_config(serialized_config) } end - dbus_method(:GetConfig, "out config:s") { serialized_storage_config } + dbus_method(:GetConfig, "out serialized_config:s") { recover_config } dbus_method(:Install) { install } dbus_method(:Finish) { finish } dbus_reader(:deprecated_system, "b") @@ -179,10 +180,23 @@ def update_actions dbus_reader_attr_accessor :actions, "aa{sv}" end - # @todo Rename as "org.opensuse.Agama.Storage1.Proposal". PROPOSAL_CALCULATOR_INTERFACE = "org.opensuse.Agama.Storage1.Proposal.Calculator" private_constant :PROPOSAL_CALCULATOR_INTERFACE + # Calculates a guided proposal. + # + # @param settings_dbus [Hash] + # @return [Integer] 0 success; 1 error + def calculate_guided_proposal(settings_dbus) + logger.info("Calculating guided storage proposal from D-Bus: #{settings_dbus}") + + settings = ProposalSettingsConversion.from_dbus(settings_dbus, + config: config, logger: logger) + + proposal.calculate_guided(settings) + proposal.success? ? 0 : 1 + end + # List of disks available for installation # # Each device is represented by an array containing the name of the device and the label to @@ -220,58 +234,6 @@ def default_volume(mount_path) VolumeConversion.to_dbus(volume) end - module ProposalStrategy - GUIDED = "guided" - AUTOYAST = "autoyast" - end - - # Calculates a guided proposal. - # @deprecated - # - # @param dbus_settings [Hash] - # @return [Integer] 0 success; 1 error - def calculate_proposal(dbus_settings) - settings = ProposalSettingsConversion.from_dbus(dbus_settings, - config: config, logger: logger) - - logger.info( - "Calculating guided storage proposal from D-Bus.\n " \ - "D-Bus settings: #{dbus_settings}\n" \ - "Agama settings: #{settings.inspect}" - ) - - success = proposal.calculate_guided(settings) - success ? 0 : 1 - end - - # Whether a proposal was calculated. - # - # @return [Boolean] - def proposal_calculated? - proposal.calculated? - end - - # Proposal result, including information about success, strategy and settings. - # - # @return [Hash] Empty if there is no proposal yet. - def proposal_result - return {} unless proposal.calculated? - - if proposal.strategy?(ProposalStrategy::GUIDED) - { - "success" => proposal.success?, - "strategy" => ProposalStrategy::GUIDED, - "settings" => proposal.settings.to_json_settings.to_json - } - else - { - "success" => proposal.success?, - "strategy" => ProposalStrategy::AUTOYAST, - "settings" => proposal.settings.to_json - } - end - end - dbus_interface PROPOSAL_CALCULATOR_INTERFACE do dbus_reader :available_devices, "ao" @@ -284,16 +246,12 @@ def proposal_result [default_volume(mount_path)] end - # @todo Rename as CalculateGuided + # @todo Receive guided json settings. # # result: 0 success; 1 error - dbus_method(:Calculate, "in settings:a{sv}, out result:u") do |settings| - busy_while { calculate_proposal(settings) } + dbus_method(:Calculate, "in settings_dbus:a{sv}, out result:u") do |settings_dbus| + busy_while { calculate_guided_proposal(settings_dbus) } end - - dbus_reader :proposal_calculated?, "b", dbus_name: "Calculated" - - dbus_reader :proposal_result, "a{sv}", dbus_name: "Result" end ISCSI_INITIATOR_INTERFACE = "org.opensuse.Agama.Storage1.ISCSI.Initiator" @@ -396,61 +354,6 @@ def proposal backend.proposal end - # Calculates a guided proposal. - # - # @param settings_json [Hash] JSON settings according to schema. - # @return [Integer] 0 success; 1 error - def calculate_guided_proposal(settings_json) - proposal_settings = Agama::Storage::ProposalSettings - .new_from_json(settings_json, config: config) - - logger.info( - "Calculating guided storage proposal from D-Bus.\n" \ - "Input settings: #{settings_json}\n" \ - "Agama settings: #{proposal_settings.inspect}" - ) - - success = proposal.calculate_guided(proposal_settings) - success ? 0 : 1 - end - - # Calculates an AutoYaST proposal. - # - # @param settings_json [Hash] AutoYaST settings. - # @return [Integer] 0 success; 1 error - def calculate_autoyast_proposal(settings_json) - # Ensures keys are strings. - autoyast_settings = JSON.parse(settings_json.to_json) - - logger.info( - "Calculating AutoYaST storage proposal from D-Bus.\n" \ - "Input settings: #{settings_json}\n" \ - "AutoYaST settings: #{autoyast_settings}" - ) - - success = proposal.calculate_autoyast(autoyast_settings) - success ? 0 : 1 - end - - # Storage config from the current proposal, if any. - # - # @return [Hash] Storage config according to JSON schema. - def storage_config - if proposal.strategy?(ProposalStrategy::GUIDED) - { - storage: { - guided: proposal.settings.to_json_settings - } - } - elsif proposal.strategy?(ProposalStrategy::AUTOYAST) - { - autoyastLegacyStorage: proposal.settings - } - else - {} - end - end - def register_storage_callbacks backend.on_issues_change { issues_properties_changed } backend.on_deprecated_system_change { storage_properties_changed } @@ -510,7 +413,7 @@ def export_proposal @dbus_proposal = nil end - return unless proposal.strategy?(ProposalStrategy::GUIDED) + return unless proposal.guided? @dbus_proposal = DBus::Storage::Proposal.new(proposal, logger) @service.export(@dbus_proposal) diff --git a/service/lib/agama/dbus/storage/proposal.rb b/service/lib/agama/dbus/storage/proposal.rb index fd9c45428f..9050c8a057 100644 --- a/service/lib/agama/dbus/storage/proposal.rb +++ b/service/lib/agama/dbus/storage/proposal.rb @@ -52,9 +52,9 @@ def initialize(backend, logger) # # @return [Hash] def settings - return {} unless backend.settings + return {} unless backend.guided? - ProposalSettingsConversion.to_dbus(backend.settings) + ProposalSettingsConversion.to_dbus(backend.guided_settings) end # List of sorted actions in D-Bus format. diff --git a/service/lib/agama/storage/config.rb b/service/lib/agama/storage/config.rb index 1deeba2a6e..27ad3a4f42 100644 --- a/service/lib/agama/storage/config.rb +++ b/service/lib/agama/storage/config.rb @@ -86,7 +86,7 @@ def explicit_boot_device def implicit_boot_device # NOTE: preliminary implementation with very simplistic checks root_drive = drives.find do |drive| - drive.partitions.any? { |p| p.filesystem.root? } + drive.partitions.any? { |p| p.filesystem&.root? } end root_drive&.found_device&.name diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index eb826128b4..f8d014e04f 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -21,17 +21,20 @@ require "agama/issue" require "agama/storage/actions_generator" +require "agama/storage/config_conversions/from_json" +require "agama/storage/proposal_settings" require "agama/storage/proposal_strategies" +require "json" require "yast" require "y2storage" module Agama module Storage - # Backend class to calculate a storage proposal. + # Class used for calculating a storage proposal. class Proposal include Yast::I18n - # @param config [Config] Agama config + # @param config [Agama::Config] Agama config # @param logger [Logger] def initialize(config, logger: nil) textdomain "agama" @@ -73,36 +76,92 @@ def available_devices disk_analyzer&.candidate_disks || [] end - # Settings used to calculate the current proposal. + # Storage config from the current proposal, if any. # - # The type depends on the kind of proposal, see {#calculate_guided} and {#calculate_autoyast}. - # - # @return [Agama::Storage::ProposalSettings, Array] - def settings - return unless calculated? - - strategy_object.settings + # @return [Hash] Storage config according to JSON schema. + def config_json + return {} unless calculated? + + case strategy_object + when ProposalStrategies::Guided + { + storage: { + guided: strategy_object.settings.to_json_settings + } + } + when ProposalStrategies::Agama + # @todo Convert config to JSON if there is no raw config. + raw_config || {} + when ProposalStrategies::Autoyast + raw_config + else + {} + end end - # Calculates a new guided proposal. + # Calculates a new proposal using the guided strategy. # - # @param settings [Agama::Storage::ProposalSettings] settings to calculate the proposal. - # @return [Boolean] whether the proposal was correctly calculated. + # @param settings [Agama::Storage::ProposalSettings] + # @return [Boolean] Whether the proposal successes. def calculate_guided(settings) + logger.info("Calculating proposal with guided strategy: #{settings.inspect}") + @raw_config = nil @strategy_object = ProposalStrategies::Guided.new(config, logger, settings) calculate end - # Calculates a new legacy AutoYaST proposal. + # Calculates a new proposal using the agama strategy. + # + # @param storage_config [Agama::Storage::Config] + # @return [Boolean] Whether the proposal successes. + def calculate_agama(storage_config) + logger.info("Calculating proposal with agama strategy: #{storage_config.inspect}") + @raw_config = nil + @strategy_object = ProposalStrategies::Agama.new(config, logger, storage_config) + calculate + end + + # Calculates a new proposal using the autoyast strategy. # # @param partitioning [Array] Hash-based representation of the section - # of the AutoYaST profile - # @return [Boolean] whether the proposal was correctly calculated. + # of the AutoYaST profile. + # @return [Boolean] Whether the proposal successes. def calculate_autoyast(partitioning) + logger.info("Calculating proposal with autoyast strategy: #{partitioning}") + @raw_config = nil + # Ensures keys are strings. + partitioning = JSON.parse(partitioning.to_json) @strategy_object = ProposalStrategies::Autoyast.new(config, logger, partitioning) calculate end + # Calculates a new proposal using the given JSON config. + # + # @raise If the config is not valid. + # + # @param config_json [Hash] Storage config according to the JSON schema. + # @return [Boolean] Whether the proposal successes. + def calculate_from_json(config_json) + # @todo Validate config_json with JSON schema. + + guided_json = config_json.dig(:storage, :guided) + storage_json = config_json[:storage] + autoyast_json = config_json[:legacyAutoyastStorage] + + if guided_json + calculate_guided_from_json(guided_json) + elsif storage_json + calculate_agama_from_json(storage_json) + elsif autoyast_json + calculate_autoyast(autoyast_json) + else + raise "Invalid storage config: #{config_json}" + end + + @raw_config = config_json + success? + end + # Storage actions. # # @return [Array] @@ -115,29 +174,61 @@ def actions ActionsGenerator.new(probed, target).generate end - # Whether the current proposal was calculated the given strategy (:autoyast or :guided). + # Whether the guided strategy was used for calculating the current proposal. # - # @param id [#downcase] # @return [Boolean] - def strategy?(id) + def guided? return false unless calculated? - id.downcase.to_sym == strategy_object.id + strategy_object.is_a?(ProposalStrategies::Guided) + end + + # Settings used for calculating the guided proposal, if any. + # + # @return [ProposalSettings, nil] + def guided_settings + return unless guided? + + strategy_object.settings end private - # @return [Config] + # @return [Agama::Config] attr_reader :config # @return [Logger] attr_reader :logger + # @return [ProposalStrategies::Base] attr_reader :strategy_object - # Calculates a new proposal. + # @return [Hash] JSON config without processing. + attr_reader :raw_config + + # Calculates a proposal from guided JSON settings. + # + # @param guided_json [Hash] e.g., { "target": { "disk": "/dev/vda" } }. + # @return [Boolean] Whether the proposal successes. + def calculate_guided_from_json(guided_json) + settings = ProposalSettings.new_from_json(guided_json, config: config) + calculate_guided(settings) + end + + # Calculates a proposal from storage JSON settings. + # + # @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 + calculate_agama(storage_config) + end + + # Calculates a new proposal with the assigned strategy. # - # @return [Boolean] whether the proposal was correctly calculated. + # @return [Boolean] Whether the proposal successes. def calculate return false unless storage_manager.probed? diff --git a/service/lib/agama/storage/proposal_strategies.rb b/service/lib/agama/storage/proposal_strategies.rb index c8efb53c08..8bfe8ade59 100644 --- a/service/lib/agama/storage/proposal_strategies.rb +++ b/service/lib/agama/storage/proposal_strategies.rb @@ -27,5 +27,6 @@ module ProposalStrategies end end -require "agama/storage/proposal_strategies/guided" +require "agama/storage/proposal_strategies/agama" require "agama/storage/proposal_strategies/autoyast" +require "agama/storage/proposal_strategies/guided" diff --git a/service/lib/agama/storage/proposal_strategies/agama.rb b/service/lib/agama/storage/proposal_strategies/agama.rb new file mode 100644 index 0000000000..a984f9eaac --- /dev/null +++ b/service/lib/agama/storage/proposal_strategies/agama.rb @@ -0,0 +1,73 @@ +# 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/proposal_strategies/base" +require "y2storage/agama_proposal" + +module Agama + module Storage + module ProposalStrategies + # Strategy for Agama proposal. + class Agama < Base + include Yast::I18n + + # @return [Agama::Storage::Config] + attr_reader :storage_config + alias_method :settings, :storage_config + + # @param config [Agama::Config] + # @param logger [Logger] + # @param storage_config [Agama::Storage::Config] + def initialize(config, logger, storage_config) + textdomain "agama" + + super(config, logger) + @storage_config = storage_config + end + + # @see Base#calculate + def calculate + proposal = agama_proposal + proposal.propose + ensure + storage_manager.proposal = proposal + end + + # @see Base#issues + def issues + storage_manager.proposal.issues_list + end + + private + + # Instance of the Y2Storage proposal to be used to run the calculation. + # + # @return [Y2Storage::AgamaProposal] + def agama_proposal + Y2Storage::AgamaProposal.new(storage_config, + issues_list: [], + devicegraph: probed_devicegraph, + disk_analyzer: disk_analyzer) + end + end + end + end +end diff --git a/service/lib/agama/storage/proposal_strategies/base.rb b/service/lib/agama/storage/proposal_strategies/base.rb index 51f15ba528..5663a8cf4b 100644 --- a/service/lib/agama/storage/proposal_strategies/base.rb +++ b/service/lib/agama/storage/proposal_strategies/base.rb @@ -48,13 +48,6 @@ def calculate raise NotImplementedError end - # Identifier for the strategy. - # - # @return [Symbol] - def id - self.class.name.split("::").last.downcase.to_sym - end - # List of issues. # # @return [Array] diff --git a/service/lib/y2storage/proposal/agama_space_maker.rb b/service/lib/y2storage/proposal/agama_space_maker.rb index bb14a399d3..4c60683ada 100644 --- a/service/lib/y2storage/proposal/agama_space_maker.rb +++ b/service/lib/y2storage/proposal/agama_space_maker.rb @@ -40,7 +40,7 @@ def guided_settings(settings) # ProductFeatures mechanism in place). Y2Storage::ProposalSettings.new_for_current_product.tap do |target| target.space_settings.strategy = :bigger_resize - target.space_settings.actions = [] + target.space_settings.actions = {} boot_device = settings.boot_device diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 03ac109478..09354d1ce0 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -23,6 +23,7 @@ require_relative "../../storage/storage_helpers" require "agama/dbus/storage/manager" require "agama/dbus/storage/proposal" +require "agama/storage/config" require "agama/storage/device_settings" require "agama/storage/manager" require "agama/storage/proposal" @@ -73,6 +74,8 @@ end before do + # Speed up tests by avoding real check of TPM presence. + allow(Y2Storage::EncryptionMethod::TPM_FDE).to receive(:possible?).and_return(true) allow(Yast::Arch).to receive(:s390).and_return false allow(proposal).to receive(:on_calculate) mock_storage(devicegraph: "empty-hd-50GiB.yaml") @@ -332,9 +335,11 @@ end end - describe "#apply_storage_config" do + describe "#apply_config" do + let(:serialized_config) { config_json.to_json } + context "if the serialized config contains guided proposal settings" do - let(:storage_config) do + let(:config_json) do { storage: { guided: { @@ -382,11 +387,11 @@ ) end - subject.apply_storage_config(storage_config.to_json) + subject.apply_config(serialized_config) end context "when the serialized config omits some settings" do - let(:storage_config) do + let(:config_json) do { storage: { guided: {} @@ -405,7 +410,7 @@ expect(settings.volumes).to eq([]) end - subject.apply_storage_config(storage_config.to_json) + subject.apply_config(serialized_config) end end @@ -455,7 +460,7 @@ expect(volume.btrfs.snapshots).to eq(true) end - subject.apply_storage_config(storage_config.to_json) + subject.apply_config(serialized_config) end context "and the volume settings omits some values" do @@ -490,14 +495,51 @@ expect(volume.btrfs.snapshots).to eq(false) end - subject.apply_storage_config(storage_config.to_json) + subject.apply_config(serialized_config) end end end end + context "if the serialized config contains storage settings" do + let(:config_json) do + { + storage: { + drives: [ + ptableType: "gpt", + partitions: [ + { + filesystem: { + type: "btrfs", + path: "/" + } + } + ] + ] + } + } + end + + it "calculates an agama proposal with the given config" do + expect(proposal).to receive(:calculate_agama) do |config| + expect(config).to be_a(Agama::Storage::Config) + expect(config.drives.size).to eq(1) + + drive = config.drives.first + expect(drive.ptable_type).to eq(Y2Storage::PartitionTables::Type::GPT) + expect(drive.partitions.size).to eq(1) + + partition = drive.partitions.first + expect(partition.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) + expect(partition.filesystem.path).to eq("/") + end + + subject.apply_config(serialized_config) + end + end + context "if the serialized config contains legacy AutoYaST settings" do - let(:storage_config) do + let(:config_json) do { legacyAutoyastStorage: [ { device: "/dev/vda" } @@ -507,26 +549,26 @@ it "calculates an AutoYaST proposal with the given settings" do expect(proposal).to receive(:calculate_autoyast) do |settings| - expect(settings).to eq([{ "device" => "/dev/vda" }]) + expect(settings).to eq(config_json[:legacyAutoyastStorage]) end - subject.apply_storage_config(storage_config.to_json) + subject.apply_config(serialized_config) end end end - describe "#serialized_storage_config" do - def pretty_json(value) + describe "#recover_config" do + def serialize(value) JSON.pretty_generate(value) end context "if a proposal has not been calculated" do it "returns serialized empty storage config" do - expect(subject.serialized_storage_config).to eq(pretty_json({})) + expect(subject.recover_config).to eq(serialize({})) end end - context "if a proposal has been calculated" do + context "if a guided proposal has been calculated" do before do proposal.calculate_guided(settings) end @@ -537,92 +579,102 @@ def pretty_json(value) end end - it "returns serialized storage config including guided proposal settings" do + it "returns serialized guided storage config" do expected_config = { storage: { guided: settings.to_json_settings } } - expect(subject.serialized_storage_config).to eq(pretty_json(expected_config)) + expect(subject.recover_config).to eq(serialize(expected_config)) end end - end - describe "#proposal_calculated?" do - before do - allow(proposal).to receive(:calculated?).and_return(calculated) - end + context "if an agama proposal has been calculated" do + before do + proposal.calculate_agama(config) + end - context "if the proposal is not calculated yet" do - let(:calculated) { false } + let(:config) do + fs_type = Agama::Storage::Configs::FilesystemType.new.tap do |t| + t.fs_type = Y2Storage::Filesystems::Type::BTRFS + end - it "returns false" do - expect(subject.proposal_calculated?).to eq(false) - end - end + filesystem = Agama::Storage::Configs::Filesystem.new.tap do |f| + f.type = fs_type + end - context "if the proposal is calculated" do - let(:calculated) { true } + drive = Agama::Storage::Configs::Drive.new.tap do |d| + d.filesystem = filesystem + end - it "returns true" do - expect(subject.proposal_calculated?).to eq(true) + Agama::Storage::Config.new.tap do |config| + config.drives = [drive] + end end - end - end - describe "#proposal_result" do - before do - allow(proposal).to receive(:calculated?).and_return(calculated) - end + it "returns serialized storage config" do + skip "Missing conversion from Agama::Storage::Config to JSON" - context "if the proposal is not calculated yet" do - let(:calculated) { false } + expected_config = { + storage: { + drives: [ + { + filesystem: { + type: "btrfs" + } + } + ] + } + } - it "returns an empty hash" do - expect(subject.proposal_result).to eq({}) + expect(subject.recover_config).to eq(serialize(expected_config)) end end - context "if the proposal is calculated" do - let(:calculated) { true } - let(:guided) { Agama::DBus::Storage::Manager::ProposalStrategy::GUIDED } - let(:autoyast) { Agama::DBus::Storage::Manager::ProposalStrategy::AUTOYAST } - - context "and it is a guided proposal" do - before do - allow(proposal).to receive(:strategy?).with(guided).and_return(true) - allow(proposal).to receive(:success?).and_return(true) - allow(proposal).to receive(:settings).and_return(Agama::Storage::ProposalSettings.new) - end + context "if a proposal was calculated from storage json" do + before do + proposal.calculate_from_json(config_json) + end - it "returns a Hash with success, strategy and settings" do - result = subject.proposal_result - serialized_settings = proposal.settings.to_json_settings.to_json + let(:config_json) do + { + storage: { + drives: [ + ptableType: "gpt", + partitions: [ + { + filesystem: { + type: "btrfs", + path: "/" + } + } + ] + ] + } + } + end - expect(result.keys).to contain_exactly("success", "strategy", "settings") - expect(result["success"]).to eq(true) - expect(result["strategy"]).to eq(guided) - expect(result["settings"]).to eq(serialized_settings) - end + it "returns the serialized storage config" do + expect(subject.recover_config).to eq(serialize(config_json)) end + end - context "and it is an autoyast proposal" do - before do - allow(proposal).to receive(:strategy?).with(guided).and_return(false) - allow(proposal).to receive(:success?).and_return(true) - allow(proposal).to receive(:settings).and_return({}) - end + context "if a proposal was calculated from AutoYaST json" do + before do + proposal.calculate_from_json(autoyast_json) + end - it "returns a Hash with success, strategy and settings" do - result = subject.proposal_result - serialized_settings = proposal.settings.to_json + let(:autoyast_json) do + { + legacyAutoyastStorage: [ + { device: "/dev/vda" } + ] + } + end - expect(result.keys).to contain_exactly("success", "strategy", "settings") - expect(result["success"]).to eq(true) - expect(result["strategy"]).to eq(autoyast) - expect(result["settings"]).to eq(serialized_settings) - end + it "returns the serialized AutoYaST config" do + expect(subject.recover_config).to eq(serialize(autoyast_json)) end end end diff --git a/service/test/agama/dbus/storage/proposal_test.rb b/service/test/agama/dbus/storage/proposal_test.rb index 59b5768c41..49237c240c 100644 --- a/service/test/agama/dbus/storage/proposal_test.rb +++ b/service/test/agama/dbus/storage/proposal_test.rb @@ -32,23 +32,27 @@ subject { described_class.new(backend, logger) } let(:backend) do - instance_double(Agama::Storage::Proposal, settings: settings) + instance_double(Agama::Storage::Proposal, guided_settings: settings, guided?: guided) end let(:logger) { Logger.new($stdout, level: :warn) } let(:settings) { nil } + let(:guided) { false } + describe "#settings" do - context "if a proposal has not been calculated yet" do - let(:settings) { nil } + context "if a guided proposal has not been calculated yet" do + let(:guided) { false } it "returns an empty hash" do expect(subject.settings).to eq({}) end end - context "if a proposal has been calculated" do + context "if a guided proposal has been calculated" do + let(:guided) { true } + let(:settings) do Agama::Storage::ProposalSettings.new.tap do |settings| settings.device = Agama::Storage::DeviceSettings::Disk.new("/dev/vda") diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb index ad66668f49..b29bf6f5d8 100644 --- a/service/test/agama/storage/proposal_test.rb +++ b/service/test/agama/storage/proposal_test.rb @@ -22,37 +22,65 @@ require_relative "../../test_helper" require_relative "storage_helpers" require "agama/config" +require "agama/storage/configs" require "agama/storage/device_settings" require "agama/storage/proposal" require "agama/storage/proposal_settings" require "y2storage" +def root_partition(size) + fs_type_config = Agama::Storage::Configs::FilesystemType.new.tap do |t| + t.fs_type = Y2Storage::Filesystems::Type::BTRFS + end + + filesystem_config = Agama::Storage::Configs::Filesystem.new.tap do |f| + f.type = fs_type_config + f.path = "/" + end + + size_config = Agama::Storage::Configs::Size.new.tap do |s| + s.min = size + s.max = size + end + + Agama::Storage::Configs::Partition.new.tap do |p| + p.filesystem = filesystem_config + p.size = size_config + end +end + +def drive(partitions) + Agama::Storage::Configs::Drive.new.tap do |d| + d.partitions = partitions + end +end + describe Agama::Storage::Proposal do include Agama::RSpec::StorageHelpers - before { mock_storage(devicegraph: "partitioned_md.yml") } - subject(:proposal) { described_class.new(config, logger: logger) } let(:logger) { Logger.new($stdout, level: :warn) } let(:config) { Agama::Config.new } - let(:achievable_settings) do - Agama::Storage::ProposalSettings.new.tap do |settings| - settings.device.name = "/dev/sdb" - settings.boot.device = "/dev/sda" - settings.volumes = [Agama::Storage::Volume.new("/")] + before do + mock_storage(devicegraph: "empty-hd-50GiB.yaml") + end + + let(:achivable_config) do + Agama::Storage::Config.new.tap do |config| + root = root_partition(Y2Storage::DiskSize.GiB(10)) + drive = drive([root]) + config.drives = [drive] end end - let(:impossible_settings) do - Agama::Storage::ProposalSettings.new.tap do |settings| - settings.device.name = "/dev/sdb" - settings.volumes = [ - # The boot disk size is 500 GiB, so it cannot accomodate a 1 TiB volume. - Agama::Storage::Volume.new("/").tap { |v| v.min_size = Y2Storage::DiskSize.TiB(1) } - ] + let(:impossible_config) do + Agama::Storage::Config.new.tap do |config| + root = root_partition(Y2Storage::DiskSize.GiB(100)) + drive = drive([root]) + config.drives = [drive] end end @@ -61,13 +89,13 @@ expect(subject.success?).to eq(false) end - context "if calculate_guided was already called" do + context "if a proposal was already calculated" do before do - subject.calculate_guided(settings) + subject.calculate_agama(config) end context "and the proposal was successful" do - let(:settings) { achievable_settings } + let(:config) { achivable_config } it "returns true" do expect(subject.success?).to eq(true) @@ -75,7 +103,7 @@ end context "and the proposal failed" do - let(:settings) { impossible_settings } + let(:config) { impossible_config } it "returns false" do expect(subject.success?).to eq(false) @@ -84,20 +112,141 @@ end end - describe "#calculate_guided" do - it "calculates a new proposal with the given settings" do - expect(Y2Storage::StorageManager.instance.proposal).to be_nil + describe "#config_json" do + context "if no proposal has been calculated yet" do + it "returns an empty hash" do + expect(subject.calculated?).to eq(false) + expect(proposal.config_json).to eq({}) + end + end - subject.calculate_guided(achievable_settings) + context "if a proposal was calculated with the guided strategy" do + before do + subject.calculate_guided(Agama::Storage::ProposalSettings.new) + end - expect(Y2Storage::StorageManager.instance.proposal).to_not be_nil - y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings - expect(y2storage_settings.root_device).to eq("/dev/sda") - expect(y2storage_settings.volumes).to contain_exactly( - an_object_having_attributes(mount_point: "/", device: "/dev/sdb") - ) + it "returns the guided JSON config" do + expected_json = { + storage: { + guided: { + boot: { + configure: true + }, + space: { + policy: "keep" + }, + target: { + disk: "/dev/sda" + }, + volumes: [] + } + } + } + + expect(subject.config_json).to eq(expected_json) + end + end + + context "if a proposal was calculated with the agama strategy" do + before do + subject.calculate_agama(achivable_config) + end + + it "returns the storage JSON config" do + skip "Missing conversion from Agama::Storage::Config to JSON" + end + end + + context "if a proposal was calculated from guided JSON config" do + before do + subject.calculate_from_json(config_json) + end + + let(:config_json) do + { + storage: { + guided: { + target: { + disk: "/dev/vda" + } + } + } + } + end + + it "returns the full guided JSON config" do + expected_json = { + storage: { + guided: { + boot: { + configure: true + }, + space: { + policy: "keep" + }, + target: { + disk: "/dev/vda" + }, + volumes: [] + } + } + } + + expect(subject.config_json).to eq(expected_json) + end + end + + context "if a proposal was calculated from storage JSON config" do + before do + subject.calculate_from_json(config_json) + end + + let(:config_json) do + { + storage: { + drives: [ + { + filesystem: { + type: "btrfs" + } + } + ] + } + } + end + + it "returns the given storage JSON config" do + expect(subject.config_json).to eq(config_json) + end end + context "if a proposal was calculated from autoyast JSON config" do + before do + subject.calculate_from_json(config_json) + end + + let(:config_json) do + { + legacyAutoyastStorage: [ + { + partitions: [ + { + mount: "/", + size: "10 GiB" + } + ] + } + ] + } + end + + it "returns the given autoyast JSON config" do + expect(subject.config_json).to eq(config_json) + end + end + end + + shared_examples "check proposal callbacks" do |action, settings| it "runs all the callbacks" do callback1 = proc {} callback2 = proc {} @@ -108,26 +257,106 @@ expect(callback1).to receive(:call) expect(callback2).to receive(:call) - subject.calculate_guided(achievable_settings) + subject.public_send(action, send(settings)) end + end + shared_examples "check proposal return" do |action, achivable_settings, impossible_settings| it "returns whether the proposal was successful" do - expect(subject.calculate_guided(achievable_settings)).to eq(true) - expect(subject.calculate_guided(impossible_settings)).to eq(false) + result = subject.public_send(action, send(achivable_settings)) + expect(result).to eq(true) + + result = subject.public_send(action, send(impossible_settings)) + expect(result).to eq(false) + end + end + + shared_examples "check early proposal" do |action, settings| + context "if the system has not been probed yet" do + before do + allow(Y2Storage::StorageManager.instance).to receive(:probed?).and_return(false) + end + + it "does not calculate a proposal" do + subject.public_send(action, send(settings)) + expect(Y2Storage::StorageManager.instance.proposal).to be_nil + end + + it "does not run the callbacks" do + callback1 = proc {} + callback2 = proc {} + + subject.on_calculate(&callback1) + subject.on_calculate(&callback2) + + expect(callback1).to_not receive(:call) + expect(callback2).to_not receive(:call) + + subject.public_send(action, send(settings)) + end + + it "returns false" do + result = subject.public_send(action, send(settings)) + expect(result).to eq(false) + end + end + end + + describe "#calculate_guided" do + before do + mock_storage(devicegraph: "partitioned_md.yml") + end + + let(:achivable_settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device.name = "/dev/sdb" + settings.boot.device = "/dev/sda" + settings.volumes = [Agama::Storage::Volume.new("/")] + end + end + + let(:impossible_settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device.name = "/dev/sdb" + settings.volumes = [ + # The boot disk size is 500 GiB, so it cannot accomodate a 1 TiB volume. + Agama::Storage::Volume.new("/").tap { |v| v.min_size = Y2Storage::DiskSize.TiB(1) } + ] + end + end + + it "calculates a proposal with the guided strategy and with the given settings" do + expect(Y2Storage::StorageManager.instance.proposal).to be_nil + + subject.calculate_guided(achivable_settings) + + expect(Y2Storage::StorageManager.instance.proposal).to_not be_nil + y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings + expect(y2storage_settings.root_device).to eq("/dev/sda") + expect(y2storage_settings.volumes).to contain_exactly( + an_object_having_attributes(mount_point: "/", device: "/dev/sdb") + ) end + include_examples "check proposal callbacks", :calculate_guided, :achivable_settings + + include_examples "check proposal return", + :calculate_guided, :achivable_settings, :impossible_settings + + include_examples "check early proposal", :calculate_guided, :achivable_settings + context "if the given device settings sets a disk as target" do before do - achievable_settings.device = Agama::Storage::DeviceSettings::Disk.new + achivable_settings.device = Agama::Storage::DeviceSettings::Disk.new end context "and the target disk is not indicated" do before do - achievable_settings.device.name = nil + achivable_settings.device.name = nil end it "sets the first available device as target device for volumes" do - subject.calculate_guided(achievable_settings) + subject.calculate_guided(achivable_settings) y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings expect(y2storage_settings.volumes).to contain_exactly( @@ -139,71 +368,166 @@ context "if the given device settings sets a new LVM volume group as target" do before do - achievable_settings.device = Agama::Storage::DeviceSettings::NewLvmVg.new + achivable_settings.device = Agama::Storage::DeviceSettings::NewLvmVg.new end context "and the target disks for physical volumes are not indicated" do before do - achievable_settings.device.candidate_pv_devices = [] + achivable_settings.device.candidate_pv_devices = [] end it "sets the first available device as candidate device" do - subject.calculate_guided(achievable_settings) + subject.calculate_guided(achivable_settings) y2storage_settings = Y2Storage::StorageManager.instance.proposal.settings expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda") end end end + end - context "if the system has not been probed yet" do - before do - allow(Y2Storage::StorageManager.instance).to receive(:probed?).and_return(false) - end + describe "#calculate_agama" do + it "calculates a proposal with the agama strategy and with the given config" do + expect(Y2Storage::StorageManager.instance.proposal).to be_nil - it "does not calculate a proposal" do - subject.calculate_guided(achievable_settings) - expect(Y2Storage::StorageManager.instance.proposal).to be_nil - end + subject.calculate_agama(achivable_config) - it "does not run the callbacks" do - callback1 = proc {} - callback2 = proc {} + expect(Y2Storage::StorageManager.instance.proposal).to be_a(Y2Storage::AgamaProposal) + end - subject.on_calculate(&callback1) - subject.on_calculate(&callback2) + include_examples "check proposal callbacks", :calculate_agama, :achivable_config - expect(callback1).to_not receive(:call) - expect(callback2).to_not receive(:call) + include_examples "check proposal return", + :calculate_agama, :achivable_config, :impossible_config - subject.calculate_guided(achievable_settings) + include_examples "check early proposal", :calculate_agama, :achivable_config + end + + describe "#calculate_autoyast" do + let(:achivable_settings) do + [ + { + partitions: [ + { + mount: "/", + size: "10 GiB" + } + ] + } + ] + end + + let(:impossible_settings) do + [ + { + device: "/dev/sdb", + partitions: [ + { + mount: "/", + size: "10 GiB" + } + ] + } + ] + end + + it "calculates a proposal with the autoyast strategy and with the given settings" do + expect(Y2Storage::StorageManager.instance.proposal).to be_nil + + subject.calculate_autoyast(achivable_settings) + expect(Y2Storage::StorageManager.instance.proposal).to be_a(Y2Storage::AutoinstProposal) + end + + include_examples "check proposal callbacks", :calculate_autoyast, :achivable_settings + + include_examples "check proposal return", + :calculate_autoyast, :achivable_settings, :impossible_settings + + include_examples "check early proposal", :calculate_autoyast, :achivable_settings + end + + describe "#calculate_from_json" do + context "if the JSON contains storage guided settings" do + let(:config_json) do + { + storage: { + guided: { + target: { + disk: "/dev/vda" + } + } + } + } end - it "returns false" do - expect(subject.calculate_guided(achievable_settings)).to eq(false) + it "calculates a proposal with the guided strategy and with the expected settings" do + expect(subject).to receive(:calculate_guided) do |settings| + expect(settings).to be_a(Agama::Storage::ProposalSettings) + expect(settings.device.name).to eq("/dev/vda") + end + + subject.calculate_from_json(config_json) end end - end - describe "#settings" do - it "returns nil if calculate has not been called yet" do - expect(proposal.settings).to be_nil + context "if the JSON contains storage settings" do + let(:config_json) do + { + storage: { + drives: [ + { + filesystem: { + type: "xfs" + } + } + ] + } + } + end + + it "calculates a proposal with the agama strategy and with the expected config" do + expect(subject).to receive(:calculate_agama) do |config| + expect(config).to be_a(Agama::Storage::Config) + expect(config.drives.size).to eq(1) + + drive = config.drives.first + expect(drive.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::XFS) + end + + subject.calculate_from_json(config_json) + end end - context "if the proposal was already calculated" do - before do - subject.calculate_guided(achievable_settings) + context "if the JSON contains autoyast settings" do + let(:config_json) do + { + legacyAutoyastStorage: [ + { + partitions: [ + { + mount: "/", + size: "10 GiB" + } + ] + } + ] + } end - it "returns the settings used for calculating the proposal" do - expect(subject.settings).to be_a(Agama::Storage::ProposalSettings) + it "calculates a proposal with the autoyast strategy and with the given settings" do + expect(subject).to receive(:calculate_autoyast) do |settings| + expect(settings).to eq(config_json[:legacyAutoyastStorage]) + end - expect(subject.settings).to have_attributes( - device: an_object_having_attributes(name: "/dev/sdb"), - volumes: contain_exactly( - an_object_having_attributes(mount_path: "/") - ) - ) + subject.calculate_from_json(config_json) + end + end + + context "if the JSON does not contain any of the storage settings" do + let(:config_json) { {} } + + it "raises an error" do + expect { subject.calculate_from_json(config_json) }.to raise_error(/Invalid storage/) end end end @@ -215,7 +539,7 @@ context "if the proposal failed" do before do - subject.calculate_guided(impossible_settings) + subject.calculate_agama(impossible_config) end it "returns an empty list" do @@ -225,12 +549,12 @@ context "if the proposal was successful" do before do - subject.calculate_guided(achievable_settings) + subject.calculate_agama(achivable_config) end it "returns the actions from the actiongraph" do expect(proposal.actions).to include( - an_object_having_attributes(text: /Create partition \/dev\/sdb1/) + an_object_having_attributes(text: /Create partition \/dev\/sda2/) ) end end @@ -242,21 +566,37 @@ end it "returns an empty list if the current proposal is successful" do - subject.calculate_guided(achievable_settings) + subject.calculate_agama(achivable_config) expect(subject.issues).to eq([]) end context "if the current proposal is failed" do - let(:settings) { impossible_settings } + let(:config) { impossible_config } - it "includes an error because the volumes cannot be accommodated" do - subject.calculate_guided(settings) + it "includes an error" do + subject.calculate_agama(config) expect(subject.issues).to include( - an_object_having_attributes(description: /Cannot accommodate/) + an_object_having_attributes(description: /A problem ocurred/) ) end + end + + context "if the proposal was calculated with the guided strategy" do + before do + mock_storage(devicegraph: "partitioned_md.yml") + end + + let(:impossible_settings) do + Agama::Storage::ProposalSettings.new.tap do |settings| + settings.device.name = "/dev/sdb" + settings.volumes = [ + # The boot disk size is 500 GiB, so it cannot accomodate a 1 TiB volume. + Agama::Storage::Volume.new("/").tap { |v| v.min_size = Y2Storage::DiskSize.TiB(1) } + ] + end + end context "and the settings does not indicate a target device" do before do @@ -297,4 +637,64 @@ end end end + + describe "#guided?" do + context "if no proposal has been calculated yet" do + it "returns false" do + expect(subject.calculated?).to eq(false) + expect(subject.guided?).to eq(false) + end + end + + context "if the proposal was calculated with the guided strategy" do + before do + settings = Agama::Storage::ProposalSettings.new + subject.calculate_guided(settings) + end + + it "returns true" do + expect(subject.guided?).to eq(true) + end + end + + context "if the proposal was calculated with any other strategy" do + before do + subject.calculate_agama(achivable_config) + end + + it "returns false" do + expect(subject.guided?).to eq(false) + end + end + end + + describe "#guided_settings" do + context "if no proposal has been calculated yet" do + it "returns nil" do + expect(subject.calculated?).to eq(false) + expect(subject.guided_settings).to be_nil + end + end + + context "if the proposal was calculated with the guided strategy" do + before do + settings = Agama::Storage::ProposalSettings.new + subject.calculate_guided(settings) + end + + it "returns the guided settings" do + expect(subject.guided_settings).to be_a(Agama::Storage::ProposalSettings) + end + end + + context "if the proposal was calculated with any other strategy" do + before do + subject.calculate_agama(achivable_config) + end + + it "returns nil" do + expect(subject.guided_settings).to be_nil + end + end + end end diff --git a/service/test/agama/storage/proposal_volumes_test.rb b/service/test/agama/storage/proposal_volumes_test.rb index fa055227b6..1638d997d1 100644 --- a/service/test/agama/storage/proposal_volumes_test.rb +++ b/service/test/agama/storage/proposal_volumes_test.rb @@ -157,7 +157,7 @@ def expect_proposal_with_specs(*specs) it "returns settings with a set of volumes with adjusted sizes" do proposal.calculate_guided(settings) - expect(proposal.settings.volumes).to contain_exactly( + expect(proposal.guided_settings.volumes).to contain_exactly( an_object_having_attributes( mount_path: "/", auto_size: true, @@ -198,7 +198,7 @@ def expect_proposal_with_specs(*specs) it "returns settings with a set of volumes with adjusted sizes" do proposal.calculate_guided(settings) - expect(proposal.settings.volumes).to contain_exactly( + expect(proposal.guided_settings.volumes).to contain_exactly( an_object_having_attributes( mount_path: "/", auto_size: true, @@ -240,7 +240,7 @@ def expect_proposal_with_specs(*specs) it "returns settings with a set of volumes with fixed limits and adjusted sizes" do proposal.calculate_guided(settings) - expect(proposal.settings.volumes).to contain_exactly( + expect(proposal.guided_settings.volumes).to contain_exactly( an_object_having_attributes( mount_path: "/", btrfs: an_object_having_attributes(snapshots?: true), @@ -281,7 +281,7 @@ def expect_proposal_with_specs(*specs) it "returns settings with a set of volumes with adjusted sizes" do proposal.calculate_guided(settings) - expect(proposal.settings.volumes).to contain_exactly( + expect(proposal.guided_settings.volumes).to contain_exactly( an_object_having_attributes(mount_path: "/", auto_size: true), an_object_having_attributes( mount_path: "swap", @@ -330,7 +330,7 @@ def expect_proposal_with_specs(*specs) it "returns settings with a set of volumes with fixed limits and adjusted sizes" do proposal.calculate_guided(settings) - expect(proposal.settings.volumes).to contain_exactly( + expect(proposal.guided_settings.volumes).to contain_exactly( an_object_having_attributes( mount_path: "/", btrfs: an_object_having_attributes(snapshots?: false),