diff --git a/service/lib/agama/config.rb b/service/lib/agama/config.rb index b36ec4c524..dfb1e395c1 100644 --- a/service/lib/agama/config.rb +++ b/service/lib/agama/config.rb @@ -19,174 +19,42 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "yaml" -require "yast2/arch_filter" require "agama/copyable" -require "agama/config_reader" -require "agama/product_reader" module Agama - # Class responsible for getting current configuration. - # It is smarter then just plain yaml reader as it also evaluates - # conditions in it, so it is result of all conditions in file. - # This also means that config needs to be re-evaluated if conditions - # data change, like if user pick different distro to install. + # Class representing the current product configuration. class Config include Copyable + include Yast2::Equatable # @return [Hash] configuration data - attr_accessor :pure_data - attr_accessor :logger - - class << self - attr_accessor :current, :base - - # Loads base and current config reading configuration from the system - def load(logger = Logger.new($stdout)) - @base = ConfigReader.new(logger: logger).config - @current = @base&.copy - end - - # It resets the configuration internal state - def reset - @base = nil - @current = nil - end - - # Load the configuration from a given file - # - # @param path [String|Pathname] File path - def from_file(path, logger = Logger.new($stdout)) - new(YAML.safe_load(File.read(path.to_s)), logger) - end - end + attr_reader :data # Constructor # # @param config_data [Hash] configuration data - def initialize(config_data = nil, logger = Logger.new($stdout)) - @pure_data = config_data - @logger = logger - end - - # parse loaded yaml file, so it properly applies conditions - # with default options it load file without conditions - def parse_file(_arch = nil, _distro = nil) - # TODO: move to internal only. public one should be something - # like evaluate or just setter for distro and arch - # logger.info "parse file with #{arch} and #{distro}" - # TODO: do real evaluation of conditions - data - end - - def data - return @data if @data - - @data = @pure_data.dup || {} - pick_product(products.keys.first) unless products.empty? - @data + def initialize(config_data = {}) + @data = config_data end - # Currently product merges its config to global config. - # Keys defined in constant are the ones specific to product that - # should not be merged to global config. - PRODUCT_SPECIFIC_KEYS = ["id", "name", "description"].freeze - def pick_product(product_id) - to_merge = products[product_id] - to_merge = to_merge.reject { |k, _v| PRODUCT_SPECIFIC_KEYS.include?(k) } - data.merge!(to_merge) - end - - # hash of available base products for current architecture - # @return [Hash{String => Hash}] product_id => product - def products - return @products if @products - - products = ProductReader.new(logger: @logger).load_products - - products.select! do |product| - product["archs"].nil? || - Yast2::ArchFilter.from_string(product["archs"]).match? - end - - @products = products.each_with_object({}) do |product, result| - result[product["id"]] = product - end - end - - # Whether there are more than one product + # Updates the internal values if needed # - # @return [Boolean] false if there is only one product; true otherwise - def multi_product? - products.size > 1 - end - - # Returns a copy of this Object + # This update mechanism exists because the current implementation of Agama relies on the + # previous behavior of this class, in which a single shared Config object was constructed only + # once (when the configuration files were read) and the values returned by that object were + # later adjusted by subsequent calls to #pick_product on that shared object. # - # @return [Config] - def copy - logger = self.logger - @logger = nil # cannot dump logger as it can contain IO - new_config = super - @logger = logger - new_config.logger = logger - - new_config - end - - # Returns a new {Config} with the merge of the given ones - # - # @param config [Config, Hash] - # @return [Config] new Configuration with the merge of the given ones - def merge(config) - Config.new(simple_merge(data, config.data)) - end - - # Elements that match the current arch. - # - # @example - # config.products #=> - # { - # "ALP-Dolomite" => { - # "software" => { - # "installation_repositories" => [ - # { - # "url" => "https://updates.suse.com/SUSE/Products/ALP-Dolomite/1.0/x86_64/product/", - # "archs" => "x86_64" - # }, - # { - # "url" => https://updates.suse.com/SUSE/Products/ALP-Dolomite/1.0/aarch64/product/", - # "archs" => "aarch64" - # }, - # "https://updates.suse.com/SUSE/Products/ALP-Dolomite/1.0/noarch/" - # ] - # } - # } - # } - # - # Yast::Arch.rpm_arch #=> "x86_64" - # config.arch_elements_from("ALP-Dolomite", "software", "installation_repositories", - # property: :url) #=> ["https://.../SUSE/Products/ALP-Dolomite/1.0/x86_64/product/", - # #=> "https://updates.suse.com/SUSE/Products/ALP-Dolomite/1.0/noarch/"] - # - # @param keys [Array] Config data keys of the collection. - # @param property [Symbol|String|nil] Property to retrieve of the elements. - # @param default [Object] The default value returned when the value is not - # found or is not an array + # To keep a similar behavior, this method provides a way to update the configuration values + # without creating a new Config object. # - # @return [Array] - def arch_elements_from(*keys, property: nil, default: []) - keys.map!(&:to_s) - elements = products.dig(*keys) - return default unless elements.is_a?(Array) + # @param new_config_data [Hash] new values for the configuration data + # @return [boolean] true if the internal values were really modified, false if there was no need + # to do so because the values are the same + def update(new_config_data) + return false if new_config_data == @data - elements.map do |element| - if !element.is_a?(Hash) - element - elsif arch_match?(element["archs"]) - property ? element[property.to_s] : element - end - end.compact + @data = new_config_data + true end # Default paths to be created for the product. @@ -234,32 +102,5 @@ def mandatory_path?(path) template.dig("outline", "required") || false end - - # Simple deep merge - # - # @param a_hash [Hash] Default values - # @param another_hash [Hash] Pillar data - # @return [Hash] - def simple_merge(a_hash, another_hash) - a_hash.reduce({}) do |all, (k, v)| - next all.merge(k => v) if another_hash[k].nil? - - if v.is_a?(Hash) - all.merge(k => simple_merge(a_hash[k], another_hash[k])) - else - all.merge(k => another_hash[k]) - end - end - end - - # Whether the current arch matches any of the given archs. - # - # @param archs [String] E.g., "x86_64,aarch64" - # @return [Boolean] - def arch_match?(archs) - return true if archs.nil? - - Yast2::ArchFilter.from_string(archs).match? - end end end diff --git a/service/lib/agama/config_reader.rb b/service/lib/agama/config_reader.rb deleted file mode 100644 index 87f42d0297..0000000000 --- a/service/lib/agama/config_reader.rb +++ /dev/null @@ -1,166 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] 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 "yast" -require "yaml" -require "logger" -require "agama/config" -require "agama/cmdline_args" -require "transfer/file_from_url" - -Yast.import "URL" -Yast.import "Directory" - -module Agama - # This class is responsible for reading Agama configuration from different locations - # including kernel cmdline options - class ConfigReader - include Yast::Transfer::FileFromUrl - include Yast::I18n - - # Default Agama configuration which should define all the possible values - SYSTEM_PATH = "/etc/agama.yaml" - GIT_PATH = File.expand_path("#{__dir__}/../../etc/agama.yaml") - GIT_DIR = File.expand_path("#{__dir__}/../../../.git") - REMOTE_BOOT_CONFIG = "agama_boot.yaml" - - PATHS = [ - "/usr/share/agama/conf.d", - "/etc/agama.d", - "/run/agama.d" - ].freeze - - attr_reader :logger - attr_reader :workdir - - # Constructor - # - # @param logger [Logger] - # @param workdir [String] Root directory to read the configuration from - def initialize(logger: nil, workdir: "/") - @logger = logger || ::Logger.new($stdout) - @workdir = workdir - end - - # loads correct yaml file - def config_from_file(path = nil) - raise "Missing config file at #{path}" unless File.exist?(path) - - logger.info "Reading configuration from #{path}" - Config.from_file(path, logger) - end - - # Return an array with the different {Config} objects read from the different locations - # - # TODO: handle precedence correctly - # - # @return [Array] an array with all the configurations read from the system - def configs - return @configs if @configs - - @configs = config_paths.map { |path| config_from_file(path) } - @configs << remote_config if remote_config - @configs << cmdline_config if cmdline_config - @configs - end - - # Return a {Config} object - # @return [Config] resultant Config after merging all the configurations - def config - config = configs.first || Config.new(nil, logger) - (configs[1..-1] || []).each { |c| config = config.merge(c) } - config - end - - private - - # Copy a file from a potentially remote location - # - # @param location [String] File location. It might be an URL-like string (e.g., - # "http://example.net/example.yml"). - # @param target [String] Path to copy the file to. - # @return [Boolean] Whether the file was successfully copied or not - def copy_file(location, target) - url = Yast::URL.Parse(location) - - res = get_file_from_url( - scheme: url["scheme"], - host: url["host"], - urlpath: url["path"], - localfile: target, - urltok: url, - destdir: "/" - ) - - # TODO: exception? - logger.error "script #{location} could not be retrieved" unless res - res - end - - # @return [CmdlineArgs] - def cmdline_args - @cmdline_args ||= CmdlineArgs.read_from(File.join(workdir, "/run/agama/cmdline.d/agama.conf")) - end - - # return [Config] - def cmdline_config - Config.new(cmdline_args.data, logger) - end - - # return [Config] - def remote_config - return unless cmdline_args.config_url - - file_path = File.join(Yast::Directory.tmpdir, REMOTE_BOOT_CONFIG) - logger.info "Copying boot config to #{file_path}" - - copy_file(cmdline_args.config_url, file_path) - config_from_file(file_path) - end - - def default_path - File.exist?(GIT_DIR) ? GIT_PATH : SYSTEM_PATH - end - - def config_paths - paths = PATHS.each_with_object([]) do |path, all| - all.concat(file_paths_in(File.join(workdir, path))) - end - - paths.uniq! { |f| File.basename(f) } - # Sort files lexicographic - paths.sort_by! { |f| File.basename(f) } - paths.prepend(default_path) if File.exist?(default_path) - - paths - end - - def file_paths_in(path) - if File.file?(path) - [path] - elsif File.directory?(path) - Dir.glob("#{path}/*.{yml,yaml}") - else - [] - end - end - end -end diff --git a/service/lib/agama/dbus/service_runner.rb b/service/lib/agama/dbus/service_runner.rb index 1794f5a8e8..dac90bc967 100644 --- a/service/lib/agama/dbus/service_runner.rb +++ b/service/lib/agama/dbus/service_runner.rb @@ -80,8 +80,7 @@ def stop def config # TODO: do not require "yast" until it is needed require "agama/config" - Config.load(logger) unless Config.current - Config.current + Config.new end # Set up a service diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 539c92547f..a99f91b731 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -65,14 +65,13 @@ def initialize(backend, logger: nil) dbus_interface "org.opensuse.Agama.Storage1" do dbus_method(:Activate) { activate } dbus_method(:Probe) { probe } - dbus_method(:SetProduct, "in id:s") { |id| configure_product(id) } dbus_method(:Install) { install } dbus_method(:Finish) { finish } dbus_method(:SetLocale, "in locale:s") { |locale| backend.configure_locale(locale) } # TODO: receive a product_config instead of an id. dbus_method(:GetSystem, "out system:s") { recover_system } dbus_method(:GetConfig, "out config:s") { recover_config } - dbus_method(:SetConfig, "in config:s") { |c| configure(c) } + dbus_method(:SetConfig, "in product:s, in config:s") { |p, c| configure(p, c) } dbus_method(:GetConfigModel, "out model:s") { recover_config_model } dbus_method(:SetConfigModel, "in model:s") { |m| configure_with_model(m) } dbus_method(:SolveConfigModel, "in model:s, out result:s") { |m| solve_config_model(m) } @@ -115,25 +114,6 @@ def probe finish_progress end - # Implementation for the API method #SetProduct. - def configure_product(id) - backend.product_config.pick_product(id) - - start_progress(3, ACTIVATING_STEP) - backend.activate unless backend.activated? - - next_progress_step(PROBING_STEP) - if !backend.probed? - backend.probe - emit_system_changed - end - - next_progress_step(CONFIGURING_STEP) - calculate_proposal - - finish_progress - end - # Implementation for the API method #Install. def install start_progress(4, _("Preparing bootloader proposal")) @@ -201,11 +181,36 @@ def recover_config_model # # @raise If the config is not valid. # + # @param serialized_product [String] Serialized product config. # @param serialized_config [String] Serialized storage config. - def configure(serialized_config) - start_progress(1, CONFIGURING_STEP) + def configure(serialized_product, serialized_config) + new_product_data = JSON.parse(serialized_product) + # Potential change in system - productMountPoints, encryptionMethods, volumeTemplates + system_changed = product_config.update(new_product_data) + + start_progress(3, ACTIVATING_STEP) + if !backend.activated? + backend.activate + # Potential change in system - issues + system_changed = true + end + next_progress_step(PROBING_STEP) + if !backend.probed? + backend.probe + # Potential change in system - devices, issues, candidateX, availableX + system_changed = true + end + + emit_system_changed if system_changed + + next_progress_step(CONFIGURING_STEP) config_json = JSON.parse(serialized_config, symbolize_names: true) + + # If config_json is nil, calculate_proposal re-calculates the proposal with the current + # config. We could skip that re-calculation if system_changed is false, but that's a + # pretty theoretical case (the method was called with an unchanged product configuration + # and with no storage configuration). calculate_proposal(config_json) finish_progress @@ -395,7 +400,7 @@ def configure_with_current config_json = proposal.storage_json return unless config_json - configure(config_json) + calculate_proposal(config_json) end # @see #configure diff --git a/service/lib/agama/storage/manager.rb b/service/lib/agama/storage/manager.rb index 92070590d9..4c6bdf6fa2 100644 --- a/service/lib/agama/storage/manager.rb +++ b/service/lib/agama/storage/manager.rb @@ -46,7 +46,7 @@ class Manager include WithProgressManager # @return [Agama::Config] - attr_reader :product_config + attr_accessor :product_config # @return [Bootloader] attr_reader :bootloader diff --git a/service/test/agama/config_reader_test.rb b/service/test/agama/config_reader_test.rb deleted file mode 100644 index ea4ea3f7e8..0000000000 --- a/service/test/agama/config_reader_test.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require_relative "../test_helper" -require "agama/config_reader" - -describe Agama::ConfigReader do - let(:workdir) { File.join(FIXTURES_PATH, "root_dir") } - let(:logger) { Logger.new($stdout, level: :warn) } - subject { described_class.new(logger: logger, workdir: workdir) } - before do - allow(Yast::Directory).to receive(:tmpdir).and_return(File.join(workdir, "tmpdir")) - allow(subject).to receive(:copy_file) - end - - describe "#config_from_file" do - it "returns a Config object with the configuration read from the given file" do - config = subject.config_from_file(File.join(workdir, "etc", "agama.yaml")) - expect(config).to be_a(Agama::Config) - expect(config.data["web"].keys).to include("ssl") - end - end - - describe "#config" do - it "returns the resultant config after merging all found configurations" do - config = subject.config - expect(config.data.dig("web", "ssl")).to eql(true) - end - end - - describe "#configs" do - it "returns an array with all the Configs present in the system" do - configs = subject.configs - # Default, RemoteBootConfig, CmdlineConfig - expect(configs.size).to eql(3) - expect(configs[0].data.dig("web", "ssl")).to eql(nil) - expect(configs[1].data.dig("web", "ssl")).to eql(false) - expect(configs[2].data.dig("web", "ssl")).to eql(true) - end - end -end diff --git a/service/test/agama/config_test.rb b/service/test/agama/config_test.rb index f6e6c4dc58..65d4a5b3be 100644 --- a/service/test/agama/config_test.rb +++ b/service/test/agama/config_test.rb @@ -29,240 +29,9 @@ describe Agama::Config do let(:config) { described_class.new("web" => { "ssl" => "SOMETHING" }) } - describe ".load" do - before do - described_class.reset - end - - it "reads the configuration from different locations" do - expect_any_instance_of(Agama::ConfigReader).to receive(:config) - described_class.load - end - - it "stores the read configuration and set it as the current one" do - allow_any_instance_of(Agama::ConfigReader).to receive(:config).and_return(config) - expect { described_class.load }.to change { described_class.base }.from(nil).to(config) - expect(described_class.base).to_not eql(described_class.current) - expect(described_class.base.data).to eql(described_class.current.data) - end - end - - describe ".from_file" do - it "builds a new instance from a given file" do - config = described_class.from_file( - File.join(FIXTURES_PATH, "root_dir", "etc", "agama.yaml") - ) - expect(config).to be_a(described_class) - end - end - - describe ".reset" do - it "resets base and current configuration" do - allow_any_instance_of(Agama::ConfigReader).to receive(:config).and_return(config) - described_class.load - expect { described_class.reset }.to change { described_class.base }.from(config).to(nil) - .and change { described_class.current }.to(nil) - end - end - describe "#data" do it "returns memoized configuration data" do expect(config.data).to eql("web" => { "ssl" => "SOMETHING" }) end end - - describe "#merge" do - let(:config_to_merge) { described_class.new("web" => { "ssl" => "MERGED" }) } - it "returns a new Config with the object data merged with the given Config data" do - merged_config = config.merge(config_to_merge) - expect(merged_config.object_id).to_not eql(config.object_id) - expect(merged_config.object_id).to_not eql(config_to_merge.object_id) - expect(merged_config.data).to eql(config_to_merge.data) - end - end - - describe "#copy" do - it "returns a copy of the object" do - copy = subject.copy - expect(copy.object_id).to_not eq(subject.object_id) - expect(copy.data).to eql(subject.data) - end - end - - describe "#products" do - it "returns products available for current hardware" do - allow(Agama::ProductReader).to receive(:new).and_return(double(load_products: - [ - { - "id" => "test", - "archs" => "x86_64" - }, - { - "id" => "test2", - "archs" => "s390x" - } - ])) - expect(Yast2::ArchFilter).to receive(:from_string).twice.and_return(double(match?: true), - double(match?: false)) - expect(subject.products.size).to eq 1 - end - end - - describe "#multi_product?" do - context "when more than one product is defined" do - before do - allow(Agama::ProductReader).to receive(:new).and_call_original - end - - it "returns true" do - expect(subject.multi_product?).to eq(true) - end - end - - context "when just one product is defined" do - before do - allow(Agama::ProductReader).to receive(:new).and_call_original - products = Agama::ProductReader.new.load_products - allow(Agama::ProductReader).to receive(:new) - .and_return(double(load_products: [products.first])) - end - - it "returns false" do - expect(subject.multi_product?).to eq(false) - end - end - end - - describe "#arch_elements_from" do - subject { described_class.new } - - before do - allow(Agama::ProductReader).to receive(:new).and_return(reader) - end - - let(:reader) { instance_double(Agama::ProductReader, load_products: products) } - - context "when the given set of keys does not match any data" do - let(:products) do - [ - { - "id" => "Product1", - "name" => "Test product 1" - } - ] - end - - it "returns an empty array" do - expect(subject.arch_elements_from("Product1", "some", "collection")).to be_empty - end - end - - context "when the given set of keys does not contain a collection" do - let(:products) do - [ - { - "id" => "Product1", - "name" => "Test product 1" - } - ] - end - - it "returns an empty array" do - expect(subject.arch_elements_from("Product1", "name")).to be_empty - end - end - - context "when the given set of keys contains a collection" do - let(:products) do - [ - { - "id" => "Product1", - "some" => { - "collection" => [ - "element1", - { - "element" => "element2" - }, - { - "element" => "element3", - "archs" => "x86_64" - }, - { - "element" => "element4", - "archs" => "x86_64,aarch64" - }, - { - "element" => "element5", - "archs" => "ppc64" - } - ] - } - } - ] - end - - before do - allow(Yast::Arch).to receive("x86_64").and_return(true) - allow(Yast::Arch).to receive("aarch64").and_return(false) - allow(Yast::Arch).to receive("ppc64").and_return(false) - end - - it "returns all the elements that match the current arch" do - elements = subject.arch_elements_from("Product1", "some", "collection") - - expect(elements).to contain_exactly( - "element1", - { "element" => "element2" }, - { "element" => "element3", "archs" => "x86_64" }, - { "element" => "element4", "archs" => "x86_64,aarch64" } - ) - end - - context "and there are no elements matching the current arch" do - let(:products) do - [ - { - "id" => "Product1", - "some" => { - "collection" => [ - { - "element" => "element1", - "archs" => "aarch64" - }, - { - "element" => "element2", - "archs" => "ppc64" - } - ] - } - } - ] - end - - it "returns an empty list" do - elements = subject.arch_elements_from("Product1", "some", "collection") - - expect(elements).to be_empty - end - end - - context "and some property is requested" do - it "returns the property from all elements that match the current arch" do - elements = subject.arch_elements_from( - "Product1", "some", "collection", property: :element - ) - - expect(elements).to contain_exactly("element1", "element2", "element3", "element4") - end - end - - context "and the requested property does not exit" do - it "only return elements that are direct values" do - elements = subject.arch_elements_from("Product1", "some", "collection", property: :foo) - - expect(elements).to contain_exactly("element1") - end - end - end - end end diff --git a/service/test/agama/dbus/service_runner_test.rb b/service/test/agama/dbus/service_runner_test.rb index 5be98a4492..ccedf34f86 100644 --- a/service/test/agama/dbus/service_runner_test.rb +++ b/service/test/agama/dbus/service_runner_test.rb @@ -29,16 +29,12 @@ describe "#run" do subject(:runner) { Agama::DBus::ServiceRunner.new(:storage) } - let(:config) { Agama::Config.new } - let(:logger) { Logger.new($stdout) } let(:storage) { instance_double(Agama::Storage::Manager) } let(:service) { instance_double(Agama::DBus::StorageService, start: nil) } before do - allow(Agama::Config).to receive(:current).and_return(config) - allow(Agama::Storage::Manager).to receive(:new).with(config).and_return(storage) - allow(Agama::DBus::StorageService).to receive(:new).with(config, Logger) - .and_return(service) + allow(Agama::Storage::Manager).to receive(:new).and_return(storage) + allow(Agama::DBus::StorageService).to receive(:new).and_return(service) end it "starts the given service" do diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 030dc900f8..7a5f301f9f 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -445,14 +445,183 @@ def parse(string) describe "#configure" do before do allow(subject).to receive(:ProposalChanged) + allow(subject).to receive(:SystemChanged) allow(subject).to receive(:ProgressChanged) allow(subject).to receive(:ProgressFinished) + + allow(backend).to receive(:activated?).and_return activated + allow(backend).to receive(:probed?).and_return probed + + allow(backend).to receive(:activate) + allow(backend).to receive(:probe) + end + + # Set some known initial product configuration for the backend + let(:config_data) do + { "storage" => { "volumes" => ["/"], "volume_templates" => [{ "mount_path" => "/" }] } } end + let(:activated) { true } + let(:probed) { true } + + let(:serialized_product) { config_data.to_json } let(:serialized_config) { config_json.to_json } - context "if the serialized config contains storage settings" do - let(:config_json) do + RSpec.shared_examples "emit SystemChanged" do + it "emits the signal SystemChanged" do + expect(subject).to receive(:SystemChanged) + subject.configure(serialized_product, serialized_config) + end + end + + RSpec.shared_examples "do not emit SystemChanged" do + it "does not emit the signal SystemChanged" do + expect(subject).to_not receive(:SystemChanged) + subject.configure(serialized_product, serialized_config) + end + end + + RSpec.shared_examples "update product configuration" do |mount_paths| + it "updates the backend product configuration" do + expect(backend.product_config.default_paths).to_not contain_exactly(*mount_paths) + subject.configure(serialized_product, serialized_config) + expect(backend.product_config.default_paths).to contain_exactly(*mount_paths) + end + + it "emits signals for SystemChanged, ProgressChanged and ProgressFinished" do + expect(subject).to receive(:SystemChanged) do |system_str| + system = parse(system_str) + expect(system[:productMountPoints]).to contain_exactly(*mount_paths) + end + expect(subject).to receive(:ProgressChanged).with(/storage configuration/i) + expect(subject).to receive(:ProgressFinished) + + subject.configure(serialized_product, serialized_config) + end + end + + RSpec.shared_examples "do not update product configuration" do + it "does not update the backend product configuration" do + data = backend.product_config.data.dup + subject.configure(serialized_product, serialized_config) + expect(backend.product_config.data).to eq data + end + + it "emits signals for ProgressChanged and ProgressFinished" do + expect(subject).to receive(:ProgressChanged).with(/storage configuration/i) + expect(subject).to receive(:ProgressFinished) + subject.configure(serialized_product, serialized_config) + end + end + + RSpec.shared_examples "activate and probe" do + it "activates the storage devices" do + expect(backend).to receive(:activate) + subject.configure(serialized_product, serialized_config) + end + + it "runs a storage probbing process" do + expect(backend).to receive(:probe) + subject.configure(serialized_product, serialized_config) + end + end + + RSpec.shared_examples "do not activate or probe" do + it "does not activate devices" do + expect(backend).to_not receive(:activate) + subject.configure(serialized_product, serialized_config) + end + + it "does not run a probbing process" do + expect(backend).to_not receive(:probe) + subject.configure(serialized_product, serialized_config) + end + end + + RSpec.shared_examples "re-calculate proposal" do + it "re-calculates the proposal with the previous configuration" do + expect(backend).to receive(:configure).with(nil) + subject.configure(serialized_product, serialized_config) + end + + it "emits the signal ProposalChanged" do + expect(subject).to receive(:ProposalChanged) + subject.configure(serialized_product, serialized_config) + end + end + + context "if no storage configuration is given" do + let(:serialized_config) { nil.to_json } + + before do + allow(backend).to receive(:configure) + end + + context "when storage devices are already activated and probed" do + before do + allow(backend).to receive(:activated?).and_return true + allow(backend).to receive(:probed?).and_return true + end + + context "if the product configuration has changed" do + let(:serialized_product) { new_config_data.to_json } + let(:new_config_data) do + { + "storage" => { + "volumes" => ["/", "swap"], + "volume_templates" => [{ "mount_path" => "/" }, { "mount_path" => "swap" }] + } + } + end + + include_examples "do not activate or probe" + include_examples "update product configuration", ["/", "swap"] + include_examples "re-calculate proposal" + include_examples "emit SystemChanged" + end + + context "if the product configuration is equal to the current one" do + include_examples "do not activate or probe" + include_examples "do not update product configuration" + include_examples "do not emit SystemChanged" + end + end + + context "when storage devices are not activated and probed" do + before do + allow(backend).to receive(:activated?).and_return(false, true) + allow(backend).to receive(:probed?).and_return(false, true) + end + + context "if the product configuration has changed" do + let(:serialized_product) { new_config_data.to_json } + let(:new_config_data) do + { + "storage" => { + "volumes" => ["/", "swap"], + "volume_templates" => [{ "mount_path" => "/" }, { "mount_path" => "swap" }] + } + } + end + + include_examples "activate and probe" + include_examples "update product configuration", ["/", "swap"] + include_examples "emit SystemChanged" + include_examples "re-calculate proposal" + end + + context "if the product configuration is equal to the current one" do + include_examples "activate and probe" + include_examples "do not update product configuration" + include_examples "emit SystemChanged" + include_examples "re-calculate proposal" + end + end + end + + context "if an Agama storage configuration is given" do + let(:serialized_config) { config_hash.to_json } + let(:config_hash) do { storage: { drives: [ @@ -470,59 +639,115 @@ def parse(string) } 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) + RSpec.shared_examples "calculate new proposal" do + 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) - 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 - partition = drive.partitions.first - expect(partition.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(partition.filesystem.path).to eq("/") + subject.configure(serialized_product, serialized_config) end - subject.configure(serialized_config) - end + it "emits the signal ProposalChanged" do + allow(proposal).to receive(:calculate_agama) + expect(subject).to receive(:ProposalChanged) + subject.configure(serialized_product, serialized_config) + end - it "emits signals for ProposalChanged, ProgressChanged and ProgressFinished" do - allow(proposal).to receive(:calculate_agama) + context "if the serialized config contains legacy AutoYaST settings" do + let(:config_hash) do + { + legacyAutoyastStorage: [ + { device: "/dev/vda" } + ] + } + end - expect(subject).to receive(:ProposalChanged) - expect(subject).to receive(:ProgressChanged).with(/storage configuration/i) - expect(subject).to receive(:ProgressFinished) + it "calculates an AutoYaST proposal with the given settings" do + expect(proposal).to receive(:calculate_autoyast) do |settings| + expect(settings).to eq(config_hash[:legacyAutoyastStorage]) + end - subject.configure(serialized_config) - end - end + subject.configure(serialized_product, serialized_config) + end - context "if the serialized config contains legacy AutoYaST settings" do - let(:config_json) do - { - legacyAutoyastStorage: [ - { device: "/dev/vda" } - ] - } + it "emits the signal ProposalChanged" do + allow(proposal).to receive(:calculate_agama) + expect(subject).to receive(:ProposalChanged) + subject.configure(serialized_product, serialized_config) + end + end end - it "calculates an AutoYaST proposal with the given settings" do - expect(proposal).to receive(:calculate_autoyast) do |settings| - expect(settings).to eq(config_json[:legacyAutoyastStorage]) + context "when storage devices are already activated and probed" do + before do + allow(backend).to receive(:activated?).and_return true + allow(backend).to receive(:probed?).and_return true + end + + context "if the product configuration has changed" do + let(:serialized_product) { new_config_data.to_json } + let(:new_config_data) do + { + "storage" => { + "volumes" => ["/", "swap"], + "volume_templates" => [{ "mount_path" => "/" }, { "mount_path" => "swap" }] + } + } + end + + include_examples "do not activate or probe" + include_examples "update product configuration", ["/", "swap"] + include_examples "emit SystemChanged" + include_examples "calculate new proposal" end - subject.configure(serialized_config) + context "if the product configuration is equal to the current one" do + include_examples "do not activate or probe" + include_examples "do not update product configuration" + include_examples "do not emit SystemChanged" + include_examples "calculate new proposal" + end end - it "emits signals for ProposalChanged, ProgressChanged and ProgressFinished" do - allow(proposal).to receive(:calculate_autoyast) + context "when storage devices are not activated and probed" do + before do + allow(backend).to receive(:activated?).and_return(false, true) + allow(backend).to receive(:probed?).and_return(false, true) + end - expect(subject).to receive(:ProposalChanged) - expect(subject).to receive(:ProgressChanged).with(/storage configuration/i) - expect(subject).to receive(:ProgressFinished) + context "if the product configuration has changed" do + let(:serialized_product) { new_config_data.to_json } + let(:new_config_data) do + { + "storage" => { + "volumes" => ["/", "swap"], + "volume_templates" => [{ "mount_path" => "/" }, { "mount_path" => "swap" }] + } + } + end + + include_examples "activate and probe" + include_examples "update product configuration", ["/", "swap"] + include_examples "emit SystemChanged" + include_examples "calculate new proposal" + end - subject.configure(serialized_config) + context "if the product configuration is equal to the current one" do + include_examples "activate and probe" + include_examples "do not update product configuration" + include_examples "emit SystemChanged" + include_examples "calculate new proposal" + end end end end @@ -843,7 +1068,7 @@ def parse(string) context "when a storage configuration was previously set" do before do - allow(proposal).to receive(:storage_json).and_return config_json.to_json + allow(proposal).to receive(:storage_json).and_return config_json allow(subject).to receive(:ProposalChanged) end diff --git a/service/test/agama/storage/autoyast_proposal_test.rb b/service/test/agama/storage/autoyast_proposal_test.rb index 24bdbb6851..1e0b21e1b1 100644 --- a/service/test/agama/storage/autoyast_proposal_test.rb +++ b/service/test/agama/storage/autoyast_proposal_test.rb @@ -25,6 +25,7 @@ require "agama/storage/proposal" require "agama/issue" require "y2storage" +require "yaml" describe Agama::Storage::Proposal do include Agama::RSpec::StorageHelpers @@ -37,6 +38,8 @@ before do mock_storage(devicegraph: scenario) allow(Y2Storage::Arch).to receive(:new).and_return(arch) + # This environment is enforced by Agama + allow(Y2Storage::StorageEnv.instance).to receive(:no_bls_bootloader).and_return true end let(:scenario) { "windows-linux-pc.yml" } @@ -48,7 +51,7 @@ let(:config_path) do File.join(FIXTURES_PATH, "storage.yaml") end - let(:config) { Agama::Config.from_file(config_path) } + let(:config) { Agama::Config.new(YAML.load_file(config_path)) } ROOT_PART = { "filesystem" => :ext4, "mount" => "/", "size" => "25%", "label" => "new_root" diff --git a/service/test/agama/storage/finisher_test.rb b/service/test/agama/storage/finisher_test.rb index 572abb52b7..895a1fe3ed 100644 --- a/service/test/agama/storage/finisher_test.rb +++ b/service/test/agama/storage/finisher_test.rb @@ -25,6 +25,7 @@ require "agama/helpers" require "agama/config" require "agama/storage/finisher" +require "yaml" describe Agama::Storage::Finisher do include Agama::RSpec::StorageHelpers @@ -37,7 +38,7 @@ end let(:destdir) { File.join(FIXTURES_PATH, "target_dir") } - let(:config) { Agama::Config.from_file(config_path) } + let(:config) { Agama::Config.new(YAML.load_file(config_path)) } let(:copy_files) { Agama::Storage::Finisher::CopyFilesStep.new(logger) } let(:progress) { instance_double(Agama::OldProgress, step: nil) } diff --git a/service/test/agama/storage/manager_test.rb b/service/test/agama/storage/manager_test.rb index 80a39ae596..69e18d6c0d 100644 --- a/service/test/agama/storage/manager_test.rb +++ b/service/test/agama/storage/manager_test.rb @@ -37,6 +37,7 @@ require "y2storage/issue" require "y2storage/luks" require "yast2/fs_snapshot" +require "yaml" Yast.import "Installation" @@ -49,7 +50,7 @@ let(:config_path) do File.join(FIXTURES_PATH, "root_dir", "etc", "agama.yaml") end - let(:config) { Agama::Config.from_file(config_path) } + let(:config) { Agama::Config.new(YAML.load_file(config_path)) } let(:tmp_dir) { Dir.mktmpdir } let(:http_client) { instance_double(Agama::HTTP::Clients::Main) }