From 6ba60026532f4abe00bfab0dfa2168c2b7a02ffe 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, 20 Sep 2024 13:37:44 +0100 Subject: [PATCH] WIP --- service/lib/agama/storage/config_builder.rb | 8 +- .../storage/config_conversions/from_json.rb | 7 +- .../config_conversions/from_json_test.rb | 392 ++++++++---------- service/test/y2storage/agama_proposal_test.rb | 309 +++++++++++--- 4 files changed, 438 insertions(+), 278 deletions(-) diff --git a/service/lib/agama/storage/config_builder.rb b/service/lib/agama/storage/config_builder.rb index 2b4ce7884b..adab684346 100644 --- a/service/lib/agama/storage/config_builder.rb +++ b/service/lib/agama/storage/config_builder.rb @@ -103,12 +103,12 @@ def unlimited_size def auto_size(outline, paths, snapshots) min_fallbacks = remove_paths(outline.min_size_fallback_for, paths) - min_size_fallbacks = min_fallbacks.map { |p| volume_builder.for(p).min_size }.sum - min = outline.base_min_size + min_size_fallbacks + min_size_fallbacks = min_fallbacks.map { |p| volume_builder.for(p).min_size } + min = min_size_fallbacks.reduce(outline.base_min_size, &:+) max_fallbacks = remove_paths(outline.max_size_fallback_for, paths) - max_size_fallbacks = max_fallbacks.map { |p| volume_builder.for(p).max_size }.sum - max = outline.base_max_size + max_size_fallbacks + max_size_fallbacks = max_fallbacks.map { |p| volume_builder.for(p).max_size } + max = max_size_fallbacks.reduce(outline.base_max_size, &:+) if outline.adjust_by_ram? min = size_with_ram(min) diff --git a/service/lib/agama/storage/config_conversions/from_json.rb b/service/lib/agama/storage/config_conversions/from_json.rb index ee87429063..6dbc12f933 100644 --- a/service/lib/agama/storage/config_conversions/from_json.rb +++ b/service/lib/agama/storage/config_conversions/from_json.rb @@ -19,6 +19,7 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # 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" @@ -28,11 +29,11 @@ module ConfigConversions # Config conversion from JSON hash according to schema. class FromJSON # @param config_json [Hash] - # @param product_config [Agama::Config] - def initialize(config_json, product_config:) + # @param product_config [Agama::Config, nil] + def initialize(config_json, product_config: nil) # TODO: Replace product_config param by a ProductDefinition. @config_json = config_json - @product_config = product_config + @product_config = product_config || Agama::Config.new end # Performs the conversion from Hash according to the JSON schema. 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 b75af5aa5b..29faa215ac 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -152,6 +152,146 @@ 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) + ) + ) + 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 + 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) + ) + ) + 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) + ) + ) + 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) + ) + ) + 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) + ) + ) + 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" } + } + ] + end + + include_examples "limit tests" + 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"] + } + ] + end + + include_examples "limit tests" + end + end + before do # Speed up tests by avoding real check of TPM presence. allow(Y2Storage::EncryptionMethod::TPM_FDE).to receive(:possible?).and_return(true) @@ -545,234 +685,20 @@ include_examples "fixed sizes", result end - # Note the min is mandatory context "specifying size limits for the partitions" do - RSpec.shared_examples "size limits" do - it "sets both min and max limits as requested if strings are used" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/home"), - size: have_attributes(default: false, min: 6.GiB, max: 9.GiB) - ) - ) - end - - it "makes a difference between SI units and binary units" do - config = subject.convert - partitions = config.drives.first.partitions - home_size = partitions.find { |p| p.filesystem.path == "/home" }.size - swap_size = partitions.find { |p| p.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 - end - - it "sets both min and max limits as requested if numbers are used" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).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) - ) - ) - end - - it "uses unlimited for the omitted max sizes" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: false, min: 3.GiB, - max: Y2Storage::DiskSize.unlimited) - ) - ) - end - - it "uses nil for min size as current" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/data1"), - size: have_attributes(default: false, min: be_nil, - max: Y2Storage::DiskSize.unlimited) - ) - ) - end - - it "uses nil for max size as current" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/data2"), - size: have_attributes(default: false, min: 10.GiB, max: be_nil) - ) - ) - end - end - - context "using a hash" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - 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" } - } - ] - } - ] - } - end - - include_examples "size limits" - end - - context "using an array" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - 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"] - } - ] - } - ] - } - end - - include_examples "size limits" - end - end - - # TODO: "default" is not currently accepted by the schema. - xcontext "using 'default' as size for some partitions and size limit for others" do let(:config_json) do { drives: [ { - partitions: [ - { - filesystem: { path: "/", size: "default" } - }, - { - filesystem: { path: "/opt" }, - size: { min: "6 GiB", max: "22 GiB" } - } - ] + partitions: example_configs } ] } end - it "uses the appropriate sizes for each partition" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: true, min: 40.GiB, - max: Y2Storage::DiskSize.unlimited) - ), - an_object_having_attributes( - filesystem: have_attributes(path: "/opt"), - size: have_attributes(default: false, min: 6.GiB, max: 22.GiB) - ) - ) - end - end - - # TODO: "default" is not currently accepted by the schema. - xcontext "using 'default' for a partition that is fallback for others" do - let(:config_json) { { drives: [{ partitions: partitions }] } } - let(:root) do - { filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, size: "default" } - end - let(:partitions) { [root] + other } - - context "if the other partitions are ommitted" do - let(:other) { [] } - - it "sums all the fallback sizes" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to contain_exactly( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: true, min: 10.GiB, - max: Y2Storage::DiskSize.unlimited) - ) - ) - end - end - - context "if the other partitions are included (even with non-exact name)" do - let(:other) { [{ filesystem: { path: "/home/" } }] } + result = proc { |config| config.drives.first.partitions } - it "ignores the fallback sizes" do - config = subject.convert - partitions = config.drives.first.partitions - expect(partitions).to include( - an_object_having_attributes( - filesystem: have_attributes(path: "/"), - size: have_attributes(default: true, min: 5.GiB, max: 10.GiB) - ) - ) - end - end + include_examples "size limits", result end context "configuring partial information for several mount points" do @@ -1078,5 +1004,37 @@ include_examples "omitting sizes", result end + + context "setting fixed sizes for the logical volumes" do + let(:config_json) do + { + volumeGroups: [ + { + logicalVolumes: example_configs + } + ] + } + end + + result = proc { |config| config.volume_groups.first.logical_volumes } + + include_examples "fixed sizes", result + end + + context "specifying size limits for the logical volumes" do + let(:config_json) do + { + volumeGroups: [ + { + logicalVolumes: example_configs + } + ] + } + end + + result = proc { |config| config.volume_groups.first.logical_volumes } + + include_examples "size limits", result + end end end diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index ad999a6fe6..9c4931521d 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -76,12 +76,68 @@ def partition_config(name: nil, filesystem: nil, size: nil) describe Y2Storage::AgamaProposal do include Agama::RSpec::StorageHelpers - let(:initial_config) do + subject(:proposal) do + described_class.new(config, product_config: product_config, issues_list: issues_list) + end + + let(:config) { config_json ? config_from_json : default_config } + + let(:config_from_json) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json, product_config: product_config) + .convert + end + + let(:default_config) do Agama::Storage::Config.new.tap do |settings| settings.drives = drives end end + let(:config_json) { nil } + + let(:product_config) { Agama::Config.new(product_data) } + + let(:product_data) do + { + "storage" => { + "lvm" => false, + "space_policy" => "delete", + "encryption" => { + "method" => "luks2" + }, + "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" } } + ] + } + } + end + let(:issues_list) { [] } let(:drives) { [drive0] } @@ -123,10 +179,8 @@ def partition_config(name: nil, filesystem: nil, size: nil) before do mock_storage(devicegraph: scenario) - end - - subject(:proposal) do - described_class.new(initial_config, issues_list: issues_list) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE).to receive(:possible?).and_return(true) end let(:scenario) { "empty-hd-50GiB.yaml" } @@ -149,7 +203,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) context "if no boot devices should be created" do before do - initial_config.boot = Agama::Storage::Configs::Boot.new.tap { |b| b.configure = false } + config.boot = Agama::Storage::Configs::Boot.new.tap { |b| b.configure = false } end it "proposes to create only the root device" do @@ -652,18 +706,37 @@ def partition_config(name: nil, filesystem: nil, size: nil) context "when reusing a partition" do let(:scenario) { "disks.yaml" } - let(:drives) { [drive] } - - let(:drive) do - drive_config.tap { |c| c.partitions = [partition] } + let(:config_json) do + { + drives: [ + { + partitions: [ + { + search: name, + filesystem: { + reuseIfPossible: reuse, + path: "/", + type: "ext3" + }, + size: size + }, + { + filesystem: { + path: "/home" + } + } + ] + } + ] + } end - let(:partition) { partition_config(name: name, filesystem: "ext3") } + let(:reuse) { nil } + + let(:size) { nil } context "if trying to reuse the file system" do - before do - partition.filesystem.reuse = true - end + let(:reuse) { true } context "and the partition is already formatted" do let(:name) { "/dev/vda2" } @@ -693,9 +766,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) end context "if not trying to reuse the file system" do - before do - partition.filesystem.reuse = false - end + let(:reuse) { false } context "and the partition is already formatted" do let(:name) { "/dev/vda2" } @@ -724,23 +795,54 @@ def partition_config(name: nil, filesystem: nil, size: nil) end end end + + context "if no size is indicated" do + let(:name) { "/dev/vda2" } + + let(:size) { nil } + + it "does not resize the partition" do + devicegraph = proposal.propose + + vda2 = devicegraph.find_by_name("/dev/vda2") + expect(vda2.size).to eq(20.GiB) + end + end end context "when creating a new partition" do let(:scenario) { "disks.yaml" } - let(:drives) { [drive] } - - let(:drive) do - drive_config.tap { |c| c.partitions = [partition] } + let(:config_json) do + { + drives: [ + { + partitions: [ + { + filesystem: { + reuseIfPossible: reuse, + path: "/", + type: "ext3" + }, + size: size + }, + { + filesystem: { + path: "/home" + } + } + ] + } + ] + } end - let(:partition) { partition_config(filesystem: "ext3", size: 1.GiB) } + let(:reuse) { nil } + + let(:size) { nil } context "if trying to reuse the file system" do - before do - partition.filesystem.reuse = true - end + let(:reuse) { true } it "creates the file system" do devicegraph = proposal.propose @@ -752,9 +854,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) end context "if not trying to reuse the file system" do - before do - partition.filesystem.reuse = false - end + let(:reuse) { false } it "creates the file system" do devicegraph = proposal.propose @@ -764,18 +864,55 @@ def partition_config(name: nil, filesystem: nil, size: nil) expect(filesystem.type).to eq(Y2Storage::Filesystems::Type::EXT3) end end - end - context "when the config has LVM volume groups" do - let(:scenario) { "empty-hd-50GiB.yaml" } + context "if no size is indicated" do + let(:size) { nil } + + it "creates the partition according to the size from the product definition" do + devicegraph = proposal.propose + + expect(devicegraph.partitions).to include( + an_object_having_attributes( + filesystem: an_object_having_attributes(mount_path: "/"), + size: 10.GiB - 1.MiB + ) + ) + end + end + + context "if a size is indicated" do + let(:size) { "5 GiB" } - let(:initial_config) do - Agama::Storage::ConfigConversions::FromJSON - .new(config_json, product_config: product_config) - .convert + it "creates the partition according to the given size" do + devicegraph = proposal.propose + + expect(devicegraph.partitions).to include( + an_object_having_attributes( + filesystem: an_object_having_attributes(mount_path: "/"), + size: 5.GiB + ) + ) + end end - let(:product_config) { Agama::Config.new } + context "if 'current' size is indicated" do + let(:size) { { min: "current" } } + + it "creates the partition according to the size from the product definition" do + devicegraph = proposal.propose + + expect(devicegraph.partitions).to include( + an_object_having_attributes( + filesystem: an_object_having_attributes(mount_path: "/"), + size: 10.GiB - 1.MiB + ) + ) + end + end + end + + context "when the config has LVM volume groups" do + let(:scenario) { "empty-hd-50GiB.yaml" } let(:config_json) do { @@ -835,7 +972,8 @@ def partition_config(name: nil, filesystem: nil, size: nil) filesystem: { path: "/home", type: "xfs" - } + }, + size: "2 GiB" } ] } @@ -860,6 +998,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) 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) ) @@ -908,7 +1047,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) an_object_having_attributes( lv_name: "home", lv_type: Y2Storage::LvType::NORMAL, - size: 5.GiB - 4.MiB, + size: 2.GiB, filesystem: an_object_having_attributes( type: Y2Storage::Filesystems::Type::XFS, mount_path: "/home" @@ -919,14 +1058,6 @@ def partition_config(name: nil, filesystem: nil, size: nil) end context "when a LVM physical volume is not found" do - let(:initial_config) do - Agama::Storage::ConfigConversions::FromJSON - .new(config_json, product_config: product_config) - .convert - end - - let(:product_config) { Agama::Config.new } - let(:config_json) do { drives: [ @@ -975,14 +1106,6 @@ def partition_config(name: nil, filesystem: nil, size: nil) end context "when a LVM thin pool volume is not found" do - let(:initial_config) do - Agama::Storage::ConfigConversions::FromJSON - .new(config_json, product_config: product_config) - .convert - end - - let(:product_config) { Agama::Config.new } - let(:config_json) do { drives: [ @@ -1033,5 +1156,83 @@ def partition_config(name: nil, filesystem: nil, size: nil) ) end end + + xcontext "using 'current' as size for some partitions and size limit for others" do + let(:scenario) { "empty-hd-50GiB.yaml" } + + let(:config_json) do + { + drives: [ + { + partitions: [ + { + filesystem: { path: "/", size: "default" } + }, + { + filesystem: { path: "/opt" }, + size: { min: "6 GiB", max: "22 GiB" } + } + ] + } + ] + } + end + + it "uses the appropriate sizes for each partition" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions).to contain_exactly( + an_object_having_attributes( + filesystem: have_attributes(path: "/"), + size: have_attributes(default: true, min: 40.GiB, + max: Y2Storage::DiskSize.unlimited) + ), + an_object_having_attributes( + filesystem: have_attributes(path: "/opt"), + size: have_attributes(default: false, min: 6.GiB, max: 22.GiB) + ) + ) + end + end + + # TODO: "default" is not currently accepted by the schema. + xcontext "using 'default' for a partition that is fallback for others" do + let(:config_json) { { drives: [{ partitions: partitions }] } } + let(:root) do + { filesystem: { path: "/", type: { btrfs: { snapshots: false } } }, size: "default" } + end + let(:partitions) { [root] + other } + + context "if the other partitions are ommitted" do + let(:other) { [] } + + it "sums all the fallback sizes" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions).to contain_exactly( + an_object_having_attributes( + filesystem: have_attributes(path: "/"), + size: have_attributes(default: true, min: 10.GiB, + max: Y2Storage::DiskSize.unlimited) + ) + ) + end + end + + context "if the other partitions are included (even with non-exact name)" do + let(:other) { [{ filesystem: { path: "/home/" } }] } + + it "ignores the fallback sizes" do + config = subject.convert + partitions = config.drives.first.partitions + expect(partitions).to include( + an_object_having_attributes( + filesystem: have_attributes(path: "/"), + size: have_attributes(default: true, min: 5.GiB, max: 10.GiB) + ) + ) + end + end + end end end