From 9d6dd93328022da3cfd6a0b31c02003e733f5995 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, 27 Sep 2024 13:40:52 +0100 Subject: [PATCH] WIP --- .../storage/config_conversions/from_json.rb | 5 +- .../lib/agama/storage/config_json_solver.rb | 143 +++++++-------- .../config_conversions/from_json_test.rb | 165 ++++++++++++------ service/test/y2storage/agama_proposal_test.rb | 2 +- 4 files changed, 173 insertions(+), 142 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/from_json.rb b/service/lib/agama/storage/config_conversions/from_json.rb index 7f7cf2bcb..26e210765 100644 --- a/service/lib/agama/storage/config_conversions/from_json.rb +++ b/service/lib/agama/storage/config_conversions/from_json.rb @@ -34,7 +34,7 @@ class FromJSON # @param config_json [Hash] # @param product_config [Agama::Config, nil] def initialize(config_json, product_config: nil) - # Copies the JSON object to avoid changes in the given parameter, see {ConfigJSONSolver}. + # Copies the JSON hash to avoid changes in the given parameter, see {ConfigJSONSolver}. @config_json = json_dup(config_json) @product_config = product_config || Agama::Config.new end @@ -46,8 +46,7 @@ def convert # TODO: Raise error if config_json does not match the JSON schema. # Implementation idea: ConfigJSONChecker class which reports issues if: # * The JSON does not match the schema. - # * The JSON contains both "default" and "mandatory" for partitions or logical volumes. - # * The JSON contains "default" or "mandatory" more than once. + # * The JSON contains more than one "generate" for partitions and logical volumes. # * The JSON contains invalid aliases (now checked by ConfigChecker). ConfigJSONSolver .new(product_config) diff --git a/service/lib/agama/storage/config_json_solver.rb b/service/lib/agama/storage/config_json_solver.rb index 2ecae54e3..96b8f9f7d 100644 --- a/service/lib/agama/storage/config_json_solver.rb +++ b/service/lib/agama/storage/config_json_solver.rb @@ -69,7 +69,7 @@ def initialize(product_config = nil) def solve(config_json) @config_json = config_json - solve_keywords + solve_generate end private @@ -80,52 +80,40 @@ def solve(config_json) # @return [Agama::Config] attr_reader :product_config - def solve_keywords - drives_with_keyword.each { |c| solve_partitions_keyword(c) } - volume_groups_with_keyword.each { |c| solve_logical_volumes_keyword(c) } - end - - # @param config [Hash] - def solve_partitions_keyword(config) - partitions = config[:partitions] - return unless partitions + def solve_generate + configs = configs_with_generate + return unless configs.any? - solve_keyword(partitions) + expand_generate(configs.first) + configs.each { |c| remove_generate(c) } end - # @param config [Hash] - def solve_logical_volumes_keyword(config) - logical_volumes = config[:logicalVolumes] - return unless logical_volumes + def expand_generate(config) + configs = volume_configs(config) + index = configs.index { |v| with_generate?(v) } - solve_keyword(logical_volumes) - end + return unless index - # @param configs [Array] - def solve_keyword(configs) - if with_default_keyword?(configs) - solve_default_keyword(configs) - elsif with_mandatory_keyword?(configs) - solve_mandatory_keyword(configs) - end + generate_config = configs[index] + configs[index] = volumes_from_generate(generate_config) + configs.flatten! end - # @param configs [Array] - def solve_default_keyword(configs) - configs.delete("default") - configs.delete("mandatory") - configs.concat(missing_default_configs) + def remove_generate(config) + volume_configs(config).delete_if { |c| with_generate?(c) } end - # @param configs [Array] - def solve_mandatory_keyword(configs) - configs.delete("mandatory") - configs.concat(missing_mandatory_configs) + def volumes_from_generate(config) + if with_generate_default?(config) + missing_default_volumes(config) + elsif with_generate_mandatory?(config) + missing_mandatory_volumes(config) + end end # @return [Array] - def missing_default_configs - missing_default_paths.map { |p| volume_config(p) } + def missing_default_volumes(config) + missing_default_paths.map { |p| volume_from_generate(config, p) } end # @return [Array] @@ -147,8 +135,8 @@ def current_paths end # @return [Array] - def missing_mandatory_configs - missing_mandatory_paths.map { |p| volume_config(p) } + def missing_mandatory_volumes(config) + missing_mandatory_paths.map { |p| volume_from_generate(config, p) } end # @return [Array] @@ -169,54 +157,21 @@ def mandatory_path?(path) # @param path [String] # @return [Hash] - def volume_config(path) - { filesystem: { path: path } } - end - - # @return [Array] - def drives_with_keyword - drive_configs.select { |c| with_partitions_keyword?(c) } - end - - # @return [Array] - def volume_groups_with_keyword - volume_group_configs.select { |c| with_logical_volumes_keyword?(c) } - end - - # @param config [Hash] - # @return [Boolean] - def with_partitions_keyword?(config) - partitions = config[:partitions] - return false unless partitions - - with_keyword?(partitions) - end - - # @param config [Hash] - # @return [Boolean] - def with_logical_volumes_keyword?(config) - logical_volumes = config[:logicalVolumes] - return false unless logical_volumes + def volume_from_generate(config, path) + volume = { filesystem: { path: path } } - with_keyword?(logical_volumes) - end + return volume unless config[:generate].is_a?(Hash) - # @param configs [Array] - # @return [Boolean] - def with_keyword?(configs) - with_default_keyword?(configs) || with_mandatory_keyword?(configs) - end + generate = config[:generate] + generate.delete(:partitions) + generate.delete(:logicalVolumes) - # @param configs [Array] - # @return [Boolean] - def with_default_keyword?(configs) - configs.include?("default") + volume.merge(generate) end - # @param configs [Array] - # @return [Boolean] - def with_mandatory_keyword?(configs) - configs.include?("mandatory") + def configs_with_generate + configs = drive_configs + volume_group_configs + configs.select { |c| with_volume_generate?(c) } end # @return [Array] @@ -254,6 +209,34 @@ def logical_volume_configs .compact end + def volume_configs(config) + config[:partitions] || config[:logicalVolumes] || [] + end + + def with_volume_generate?(config) + volume_configs(config).any? { |c| with_generate?(c) } + end + + def with_generate?(config) + !config[:generate].nil? + end + + def with_generate_default?(config) + with_generate_value?(config, "default") + end + + def with_generate_mandatory?(config) + with_generate_value?(config, "mandatory") + end + + def with_generate_value?(config, value) + generate = config[:generate] + + return generate == value unless generate.is_a?(Hash) + + generate[:partitions] == value || generate[:logicalVolumes] == value + end + # @return [VolumeTemplatesBuilder] def volume_builder @volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config) 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 dd0d9def1..a7f5b8631 100644 --- a/service/test/agama/storage/config_conversions/from_json_test.rb +++ b/service/test/agama/storage/config_conversions/from_json_test.rb @@ -1037,13 +1037,13 @@ include_examples "size limits", result end - context "using the 'default' keyword for partitions in a drive" do + context "using 'generate' with 'default' for partitions in a drive" do let(:config_json) do { drives: [ { partitions: [ - "default" + { generate: "default" } ] } ] @@ -1065,13 +1065,13 @@ expect(swap.size.default?).to eq(true) end - context "if the drive already defines any of the default paths" do + context "if the drive already defines some of the default paths" do let(:config_json) do { drives: [ { partitions: [ - "default", + { generate: "default" }, { filesystem: { path: "swap" }, size: "2 GiB" @@ -1100,15 +1100,14 @@ end end - context "if the drive contains the 'default' keyword several times" do + context "if there are more than one 'generate'" do let(:config_json) do { drives: [ { partitions: [ - "default", - "default", - "default" + { generate: "default" }, + { generate: "default" } ] } ] @@ -1129,14 +1128,14 @@ end end - context "if the drive also contains the 'mandatory' keyword" do + context "if there is a 'generate' with 'mandatory'" do let(:config_json) do { drives: [ { partitions: [ - "default", - "mandatory" + { generate: "default" }, + { generate: "mandatory" } ] } ] @@ -1157,13 +1156,13 @@ end end - context "if other drive already defines any of the default paths" do + context "if other drive already defines some of the default paths" do let(:config_json) do { drives: [ { partitions: [ - "default" + { generate: "default" } ] }, { @@ -1201,18 +1200,18 @@ end end - context "if other drive also contains the 'default' keyword" do + context "if other drive also contains a 'generate'" do let(:config_json) do { drives: [ { partitions: [ - "default" + { generate: "default" } ] }, { partitions: [ - "default" + { generate: "default" } ] } ] @@ -1235,53 +1234,55 @@ expect(partitions1.size).to eq(0) end end + end - context "if other device already defines any of the default paths" do - let(:config_json) do - { - drives: [ - { - partitions: [ - "default" - ] - } - ], - volumeGroups: [ - { - logicalVolumes: [ - { - filesystem: { path: "swap" }, - size: "2 GiB" + context "using 'generate' with more properties for partitions in a drive" do + let(:config_json) do + { + drives: [ + { + partitions: [ + { + generate: { + partitions: "default", + encryption: { + luks1: { password: "12345" } + } } - ] - } - ] - } - end + } + ] + } + ] + } + end - it "only includes the missing default partitions" do - config = subject.convert - partitions = config.drives.first.partitions - logical_volumes = config.volume_groups.first.logical_volumes + it "includes the default partitions defined by the product with the given properties" do + config = subject.convert + partitions = config.drives.first.partitions - expect(partitions.size).to eq(1) - expect(logical_volumes.size).to eq(1) + expect(partitions.size).to eq(2) - root = partitions.first - expect(root.filesystem.path).to eq("/") - expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) - expect(root.size.default?).to eq(true) - end + root = partitions.find { |p| p.filesystem.path == "/" } + expect(root.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::BTRFS) + expect(root.size.default?).to eq(true) + expect(root.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) + expect(root.encryption.password).to eq("12345") + + swap = partitions.find { |p| p.filesystem.path == "swap" } + expect(swap.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::SWAP) + expect(swap.size.default?).to eq(true) + expect(swap.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1) + expect(swap.encryption.password).to eq("12345") end end - context "using the 'mandatory' keyword for partitions in a drive" do + context "using 'generate' with 'mandatory' for partitions in a drive" do let(:config_json) do { drives: [ { partitions: [ - "mandatory" + { generate: "mandatory" } ] } ] @@ -1299,13 +1300,13 @@ expect(root.size.default?).to eq(true) end - context "if other device already defines any of the mandatory paths" do + context "if other device already defines some of the mandatory paths" do let(:config_json) do { drives: [ { partitions: [ - "mandatory" + { generate: "mandatory" } ] } ], @@ -1331,13 +1332,13 @@ end end - context "using the 'default' keyword for logical volumes" do + context "using 'generate' with 'default' for logical volumes" do let(:config_json) do { volumeGroups: [ { logicalVolumes: [ - "default" + { generate: "default" } ] } ] @@ -1374,7 +1375,7 @@ volumeGroups: [ { logicalVolumes: [ - "default" + { generate: "default" } ] } ] @@ -1393,13 +1394,13 @@ end end - context "using the 'mandatory' keyword for logical volumes" do + context "using 'generate' with 'mandatory' for logical volumes" do let(:config_json) do { volumeGroups: [ { logicalVolumes: [ - "mandatory" + { generate: "mandatory" } ] } ] @@ -1433,7 +1434,7 @@ volumeGroups: [ { logicalVolumes: [ - "mandatory" + { generate: "mandatory" } ] } ] @@ -1448,5 +1449,53 @@ end end end + + context "using both 'generate' with 'default' and with 'mandatory'" do + let(:config_json) do + { + drives: [ + { + partitions: [ + first_generate, + second_generate + ] + } + ] + } + end + + context "if 'default' appears first" do + let(:first_generate) { { generate: "default" } } + let(:second_generate) { { generate: "mandatory" } } + + it "includes the default partitions defined by the product" do + config = subject.convert + partitions = config.drives.first.partitions + + expect(partitions.size).to eq(2) + + root = partitions.find { |p| p.filesystem.path == "/" } + swap = partitions.find { |p| p.filesystem.path == "swap" } + + expect(root).to_not be_nil + expect(swap).to_not be_nil + end + end + + context "if 'mandatory' appears first" do + let(:first_generate) { { generate: "mandatory" } } + let(:second_generate) { { generate: "default" } } + + it "includes the mandatory partitions defined by the product" do + config = subject.convert + partitions = config.drives.first.partitions + + expect(partitions.size).to eq(1) + + root = partitions.find { |p| p.filesystem.path == "/" } + expect(root).to_not be_nil + end + end + end end end diff --git a/service/test/y2storage/agama_proposal_test.rb b/service/test/y2storage/agama_proposal_test.rb index dafde8f6c..e31e7902a 100644 --- a/service/test/y2storage/agama_proposal_test.rb +++ b/service/test/y2storage/agama_proposal_test.rb @@ -227,7 +227,7 @@ def partition_config(name: nil, filesystem: nil, size: nil) drives: [ { partitions: [ - "default" + { generate: "default" } ] } ]