From ccdd71800575a30797415771d72d74976f6bbba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 27 Feb 2025 13:12:31 +0000 Subject: [PATCH 01/22] storage: add vgs and lvs to model schema --- .../share/examples/storage/model.json | 28 ++++++++++++++++ .../agama-lib/share/storage.model.schema.json | 33 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/rust/agama-lib/share/examples/storage/model.json b/rust/agama-lib/share/examples/storage/model.json index 7f2155246e..b7a47507fa 100644 --- a/rust/agama-lib/share/examples/storage/model.json +++ b/rust/agama-lib/share/examples/storage/model.json @@ -75,6 +75,34 @@ } } ] + }, + { + "name": "/dev/vdc", + "alias": "lvm", + "spacePolicy": "delete" + } + ], + "volumeGroups": [ + { + "name": "vg0", + "extentSize": 1024, + "targetDevices": ["lvm"], + "logicalVolumes": [ + { + "name": "lv0", + "mountPath": "/data", + "filesystem": { + "default": false, + "type": "ext4" + }, + "size": { + "default": true, + "min": 1111 + }, + "stripes": 8, + "stripeSize": 1204 + } + ] } ] } diff --git a/rust/agama-lib/share/storage.model.schema.json b/rust/agama-lib/share/storage.model.schema.json index e1fcf25fda..8eb6034084 100644 --- a/rust/agama-lib/share/storage.model.schema.json +++ b/rust/agama-lib/share/storage.model.schema.json @@ -9,6 +9,10 @@ "drives": { "type": "array", "items": { "$ref": "#/$defs/drive" } + }, + "volumeGroups": { + "type": "array", + "items": { "$ref": "#/$defs/volumeGroup" } } }, "$defs": { @@ -79,6 +83,35 @@ "resizeIfNeeded": { "type": "boolean" } } }, + "volumeGroup": { + "type": "object", + "additionalProperties": false, + "required": ["name"], + "properties": { + "name": { "type": "string" }, + "extentSize": { "type": "integer" }, + "targetDevices": { + "type": "array", + "items": { "$ref": "#/$defs/alias" } + }, + "logicalVolumes": { + "type": "array", + "items": { "$ref": "#/$defs/logicalVolume" } + } + } + }, + "logicalVolume": { + "type": "object", + "additionalProperties": false, + "properties": { + "name": { "type": "string" }, + "mountPath": { "type": "string" }, + "filesystem": { "$ref": "#/$defs/filesystem" }, + "size": { "$ref": "#/$defs/size" }, + "stripes": { "type": "integer" }, + "stripeSize": { "type": "integer" } + } + }, "alias": { "description": "Alias used to reference a device.", "type": "string" From 8b545a12bbb8c8e25a72164671cbd6fdb4551813 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, 28 Feb 2025 15:29:08 +0000 Subject: [PATCH 02/22] storage: add conversion to model for LVM --- .../to_model_conversions.rb | 2 + .../to_model_conversions/config.rb | 13 +- .../to_model_conversions/logical_volume.rb | 59 +++++ .../to_model_conversions/volume_group.rb | 60 +++++ service/lib/agama/storage/proposal.rb | 14 +- .../config_conversions/to_model_test.rb | 220 +++++++++++++++++- 6 files changed, 357 insertions(+), 11 deletions(-) create mode 100644 service/lib/agama/storage/config_conversions/to_model_conversions/logical_volume.rb create mode 100644 service/lib/agama/storage/config_conversions/to_model_conversions/volume_group.rb diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions.rb b/service/lib/agama/storage/config_conversions/to_model_conversions.rb index 1d2c252603..8c4760dfdd 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions.rb @@ -26,9 +26,11 @@ require "agama/storage/config_conversions/to_model_conversions/drive" require "agama/storage/config_conversions/to_model_conversions/encryption" require "agama/storage/config_conversions/to_model_conversions/filesystem" +require "agama/storage/config_conversions/to_model_conversions/logical_volume" require "agama/storage/config_conversions/to_model_conversions/partition" require "agama/storage/config_conversions/to_model_conversions/size" require "agama/storage/config_conversions/to_model_conversions/space_policy" +require "agama/storage/config_conversions/to_model_conversions/volume_group" module Agama module Storage diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb index 2634fbe1b4..adc3220280 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb @@ -23,6 +23,7 @@ require "agama/storage/config_conversions/to_model_conversions/boot" require "agama/storage/config_conversions/to_model_conversions/encryption" require "agama/storage/config_conversions/to_model_conversions/drive" +require "agama/storage/config_conversions/to_model_conversions/volume_group" module Agama module Storage @@ -41,9 +42,10 @@ def initialize(config) # @see Base#conversions def conversions { - boot: convert_boot, - encryption: convert_encryption, - drives: convert_drives + boot: convert_boot, + encryption: convert_encryption, + drives: convert_drives, + volumeGroups: convert_volume_groups } end @@ -65,6 +67,11 @@ def convert_drives valid_drives.map { |d| ToModelConversions::Drive.new(d).convert } end + # @return [Array] + def convert_volume_groups + config.volume_groups.map { |v| ToModelConversions::VolumeGroup.new(v).convert } + end + # @return [Array] def valid_drives config.drives.select(&:found_device) diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/logical_volume.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/logical_volume.rb new file mode 100644 index 0000000000..2c674a4b52 --- /dev/null +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/logical_volume.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +# Copyright (c) [2025] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "agama/storage/config_conversions/to_model_conversions/base" +require "agama/storage/config_conversions/to_model_conversions/with_filesystem" +require "agama/storage/config_conversions/to_model_conversions/with_size" + +module Agama + module Storage + module ConfigConversions + module ToModelConversions + # LVM logical volume conversion to model according to the JSON schema. + class LogicalVolume < Base + include WithFilesystem + include WithSize + + # @param config [Configs::LogicalVolume] + def initialize(config) + super() + @config = config + end + + private + + # @see Base#conversions + def conversions + { + name: config.name, + alias: config.alias, + mountPath: config.filesystem&.path, + filesystem: convert_filesystem, + size: convert_size, + stripes: config.stripes, + stripeSize: config.stripe_size&.to_i + } + end + end + end + end + end +end diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/volume_group.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/volume_group.rb new file mode 100644 index 0000000000..ea66c7cb1f --- /dev/null +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/volume_group.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +# Copyright (c) [2025] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "agama/storage/config_conversions/to_model_conversions/base" +require "agama/storage/config_conversions/to_model_conversions/logical_volume" + +module Agama + module Storage + module ConfigConversions + module ToModelConversions + # LVM volume group conversion to model according to the JSON schema. + class VolumeGroup < Base + include WithFilesystem + + # @param config [Configs::VolumeGroup] + def initialize(config) + super() + @config = config + end + + private + + # @see Base#conversions + def conversions + { + name: config.name, + extentSize: config.extent_size&.to_i, + targetDevices: config.physical_volumes_devices, + logicalVolumes: convert_logical_volumes + } + end + + def convert_logical_volumes + config.logical_volumes.map do |logical_volume| + ToModelConversions::LogicalVolume.new(logical_volume).convert + end + end + end + end + end + end +end diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index 05f5f7593d..abcd501486 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -285,23 +285,25 @@ def config(solved: false) solved ? strategy.config : source_config end + # TODO: extract to a separate class (e.g., ModelSupportChecker) and add more checks, for + # example: the presence of thin pools, partitions without mount path, lvs without mount + # path, etc. + # # Whether the config model supports all features of the given config. # # @param config [Storage::Config] # @return [Boolean] def model_supported?(config) - unsupported_configs = [ - config.volume_groups, + unsupported_devices = [ config.md_raids, config.btrfs_raids, config.nfs_mounts ].flatten - encryptable_configs = [ - config.drives - ].flatten + # Only volume groups with automatically generated pvs are supported + volume_groups_with_pvs = config.volume_groups.select { |v| v.physical_volumes.any? } - unsupported_configs.empty? && encryptable_configs.none?(&:encryption) + unsupported_devices.empty? && volume_groups_with_pvs.empty? end # Calculates a proposal from guided JSON settings. diff --git a/service/test/agama/storage/config_conversions/to_model_test.rb b/service/test/agama/storage/config_conversions/to_model_test.rb index 927f60dbc0..9abcf75dda 100644 --- a/service/test/agama/storage/config_conversions/to_model_test.rb +++ b/service/test/agama/storage/config_conversions/to_model_test.rb @@ -627,11 +627,12 @@ it "generates the expected JSON" do expect(subject.convert).to eq( { - boot: { + boot: { configure: true, device: { default: true } }, - drives: [] + drives: [], + volumeGroups: [] } ) end @@ -909,5 +910,220 @@ include_examples "#spacePolicy property", drive_result_scope end end + + context "if #volume_groups is configured" do + let(:config_json) do + { volumeGroups: volume_groups } + end + + let(:volume_groups) do + [ + volume_group, + {} + ] + end + + let(:volume_group) { {} } + + it "generates the expected JSON for 'VolumeGroups'" do + volume_groups_model = subject.convert[:volumeGroups] + + expect(volume_groups_model).to eq( + [ + { targetDevices: [], logicalVolumes: [] }, + { targetDevices: [], logicalVolumes: [] } + ] + ) + end + + vg_result_scope = proc { |c| c[:volumeGroups].first } + + context "if #name is not configured for a volume group" do + let(:volume_group) { {} } + include_examples "without name", vg_result_scope + end + + context "if #extent_size is not configured for a volume group" do + let(:volume_group) { {} } + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json.keys).to_not include(:extentSize) + end + end + + context "if #physical_volumes_devices is not configured for a volume group" do + let(:volume_group) { {} } + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json[:targetDevices]).to eq([]) + end + end + + context "if #logical_volumes is not configured for a volume group" do + let(:volume_group) { {} } + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json[:logicalVolumes]).to eq([]) + end + end + + context "if #name is configured for a volume group" do + let(:volume_group) { { name: "test" } } + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json[:name]).to eq("test") + end + end + + context "if #extent_size is configured for a volume group" do + let(:volume_group) { { extentSize: "1 KiB" } } + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json[:extentSize]).to eq(1.KiB.to_i) + end + end + + context "if #physical_volumes_devices is configured for a volume group" do + let(:volume_group) do + { + physicalVolumes: [{ generate: ["disk1", "disk2"] }] + } + end + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json[:targetDevices]).to eq(["disk1", "disk2"]) + end + end + + context "if #logical_volumes is configured for a volume group" do + let(:volume_group) do + { + logicalVolumes: [logical_volume, {}] + } + end + + let(:logical_volume) { {} } + + it "generates the expected JSON" do + model_json = vg_result_scope.call(subject.convert) + expect(model_json[:logicalVolumes].size).to eq(2) + end + + lv_result_scope = proc { |c| vg_result_scope.call(c)[:logicalVolumes].first } + # partition_scope = proc { |c| device_scope.call(c).partitions.first } + + context "if #name is not configured for a logical volume" do + let(:logical_volume) { {} } + include_examples "without name", lv_result_scope + end + + context "if #alias is not configured for a logical volume" do + let(:logical_volume) { {} } + include_examples "without alias", lv_result_scope + end + + context "if #filesystem is not configured for a logical volume" do + let(:logical_volume) { {} } + include_examples "without filesystem", lv_result_scope + end + + context "if #size is not configured for a logical volume" do + let(:logical_volume) { {} } + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json[:size]).to eq( + { + default: true, + min: 100.MiB.to_i + } + ) + end + end + + context "if #stripes is not configured for a logical volume" do + let(:logical_volume) { {} } + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json.keys).to_not include(:stripes) + end + end + + context "if #stripe_size is not configured for a logical volume" do + let(:logical_volume) { {} } + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json.keys).to_not include(:stripeSize) + end + end + + context "if #name is configured for a logical volume" do + let(:logical_volume) { { name: "test" } } + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json[:name]).to eq("test") + end + end + + context "if #alias is configured for a logical volume" do + let(:logical_volume) { { alias: device_alias } } + include_examples "with alias", lv_result_scope + end + + context "if #filesystem is configured for a logical volume" do + let(:logical_volume) { { filesystem: filesystem } } + include_examples "with filesystem", lv_result_scope + end + + context "if #size is not configured for a logical volume" do + let(:logical_volume) do + { + size: { + min: "1 GiB", + max: "5 GiB" + } + } + end + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json[:size]).to eq( + { + default: false, + min: 1.GiB.to_i, + max: 5.GiB.to_i + } + ) + end + end + + context "if #stripes is configured for a logical volume" do + let(:logical_volume) { { stripes: 8 } } + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json[:stripes]).to eq(8) + end + end + + context "if #stripe_size is configured for a logical volume" do + let(:logical_volume) { { stripeSize: "4 KiB" } } + + it "generates the expected JSON" do + model_json = lv_result_scope.call(subject.convert) + expect(model_json[:stripeSize]).to eq(4.KiB.to_i) + end + end + end + end end end From 851d33df06335028023e263ef2d806b064257a75 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, 3 Mar 2025 06:17:51 +0000 Subject: [PATCH 03/22] feat(storage): make vg name mandatory --- rust/agama-lib/share/storage.schema.json | 1 + .../storage/config_checkers/volume_group.rb | 16 +++++++++++--- .../test/agama/storage/config_checker_test.rb | 22 +++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/rust/agama-lib/share/storage.schema.json b/rust/agama-lib/share/storage.schema.json index 27ec46f8e3..808433fd67 100644 --- a/rust/agama-lib/share/storage.schema.json +++ b/rust/agama-lib/share/storage.schema.json @@ -159,6 +159,7 @@ "description": "LVM volume group.", "type": "object", "additionalProperties": false, + "required": ["name"], "properties": { "name": { "description": "Volume group name.", diff --git a/service/lib/agama/storage/config_checkers/volume_group.rb b/service/lib/agama/storage/config_checkers/volume_group.rb index cbcae6025c..9d9f8757c2 100644 --- a/service/lib/agama/storage/config_checkers/volume_group.rb +++ b/service/lib/agama/storage/config_checkers/volume_group.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -41,16 +41,17 @@ def initialize(config, storage_config, product_config) @config = config end - # Volume group config issues. + # Issues from a volume group config. # # @return [Array] def issues [ + name_issue, logical_volumes_issues, physical_volumes_issues, physical_volumes_devices_issues, physical_volumes_encryption_issues - ].flatten + ].compact.flatten end private @@ -58,6 +59,15 @@ def issues # @return [Configs::VolumeGroup] attr_reader :config + # Issue if the volume group name is missing. + # + # @return [Issue, nil] + def name_issue + return if config.name && !config.name.empty? + + error(_("There is a volume group without name")) + end + # Issues from logical volumes. # # @return [Array] diff --git a/service/test/agama/storage/config_checker_test.rb b/service/test/agama/storage/config_checker_test.rb index b675e68b28..542423431a 100644 --- a/service/test/agama/storage/config_checker_test.rb +++ b/service/test/agama/storage/config_checker_test.rb @@ -515,12 +515,27 @@ end end + context "if a volume group has no name" do + let(:config_json) do + { + volumeGroups: [{ name: "test" }, { name: "" }, {}] + } + end + + it "includes the expected issue" do + issues = subject.issues.select { |i| i.description.match?(/without name/) } + expect(issues.size).to eq(2) + expect(issues.map(&:error?)).to all(eq(true)) + end + end + context "if a volume group has logical volumes" do let(:config_json) do { boot: { configure: false }, volumeGroups: [ { + name: "test", logicalVolumes: [ logical_volume, { @@ -591,6 +606,7 @@ ], volumeGroups: [ { + name: "test", physicalVolumes: ["first-disk", "pv1"] } ] @@ -618,6 +634,7 @@ ], volumeGroups: [ { + name: "test", physicalVolumes: [ { generate: { @@ -652,6 +669,7 @@ ], volumeGroups: [ { + name: "test", physicalVolumes: [ { generate: { @@ -745,6 +763,7 @@ ], volumeGroups: [ { + name: "test1", physicalVolumes: [ { generate: { @@ -754,6 +773,7 @@ ] }, { + name: "test2", physicalVolumes: [ { generate: { @@ -763,6 +783,7 @@ ] }, { + name: "test3", physicalVolumes: [ { generate: { @@ -797,6 +818,7 @@ ], volumeGroups: [ { + name: "test", physicalVolumes: ["pv1"] } ] From f2bdb8cce8db05a3b5c997e50fea8a61d42b633f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Mar 2025 12:20:11 +0000 Subject: [PATCH 04/22] fix(storage): drive model includes the searching name - Includes the name used for searching the device when the device is not found. --- .../config_conversions/to_model_conversions/drive.rb | 9 +++++++-- .../agama/storage/config_conversions/to_model_test.rb | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/drive.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/drive.rb index 0542d7f4fe..7589812a49 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions/drive.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/drive.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -45,7 +45,7 @@ def initialize(config) # @see Base#conversions def conversions { - name: config.found_device&.name, + name: convert_name, alias: config.alias, mountPath: config.filesystem&.path, filesystem: convert_filesystem, @@ -54,6 +54,11 @@ def conversions partitions: convert_partitions } end + + # @return [String, nil] + def convert_name + config.found_device&.name || config.search&.name + end end end end diff --git a/service/test/agama/storage/config_conversions/to_model_test.rb b/service/test/agama/storage/config_conversions/to_model_test.rb index 9abcf75dda..da9c07bf5f 100644 --- a/service/test/agama/storage/config_conversions/to_model_test.rb +++ b/service/test/agama/storage/config_conversions/to_model_test.rb @@ -841,6 +841,7 @@ expect(drives_model).to eq( [ + { name: "/dev/vdd", spacePolicy: "keep", partitions: [] }, { name: "/dev/vda", spacePolicy: "keep", partitions: [] } ] ) From e99fb7361d15d8cb3f4847f085c965b3b7bfa4ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Mar 2025 12:26:18 +0000 Subject: [PATCH 05/22] fix(storage): only exclude skipped devices from model - The model includes all the devices, including devices with errors. --- .../to_model_conversions/config.rb | 2 +- .../to_model_conversions/with_partitions.rb | 25 ++----------- .../config_conversions/to_model_test.rb | 35 +++++++++++++++++++ 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb index adc3220280..76f7f70db3 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb @@ -74,7 +74,7 @@ def convert_volume_groups # @return [Array] def valid_drives - config.drives.select(&:found_device) + config.drives.reject { |d| d.search&.skip_device? } end # TODO: proper support for a base encryption. diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/with_partitions.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/with_partitions.rb index d13152bb1e..f7d09c6eb0 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions/with_partitions.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/with_partitions.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -36,28 +36,7 @@ def convert_partitions # @return [Array] def valid_partitions - config.partitions.select { |p| valid_partition?(p) } - end - - # @param partition_config [Configs::Partition] - # @return [Boolean] - def valid_partition?(partition_config) - valid_new_partition(partition_config) || valid_existing_partition(partition_config) - end - - # @param partition_config [Configs::Partition] - # @return [Boolean] - def valid_new_partition(partition_config) - delete = partition_config.delete? || partition_config.delete_if_needed? - return false if delete - - partition_config.search.nil? || partition_config.search.create_device? - end - - # @param partition_config [Configs::Partition] - # @return [Boolean] - def valid_existing_partition(partition_config) - !partition_config.found_device.nil? + config.partitions.reject { |p| p.search&.skip_device? } end end end diff --git a/service/test/agama/storage/config_conversions/to_model_test.rb b/service/test/agama/storage/config_conversions/to_model_test.rb index da9c07bf5f..71b149c22d 100644 --- a/service/test/agama/storage/config_conversions/to_model_test.rb +++ b/service/test/agama/storage/config_conversions/to_model_test.rb @@ -173,6 +173,13 @@ model_json = result_scope.call(subject.convert) expect(model_json[:partitions]).to eq( [ + { + delete: true, + deleteIfNeeded: false, + resize: false, + resizeIfNeeded: false, + size: { default: true, min: 100.MiB.to_i } + }, default_partition_json ] ) @@ -186,6 +193,13 @@ model_json = result_scope.call(subject.convert) expect(model_json[:partitions]).to eq( [ + { + delete: false, + deleteIfNeeded: true, + resize: false, + resizeIfNeeded: false, + size: { default: true, min: 100.MiB.to_i } + }, default_partition_json ] ) @@ -846,6 +860,27 @@ ] ) end + + context "and the drive is set to be skipped" do + let(:drive) do + { + search: { + condition: { name: "/dev/vdd" }, + ifNotFound: "skip" + } + } + end + + it "generates the expected JSON for 'drives'" do + drives_model = subject.convert[:drives] + + expect(drives_model).to eq( + [ + { name: "/dev/vda", spacePolicy: "keep", partitions: [] } + ] + ) + end + end end context "if a device is found for a drive" do From 6970c142086b2ea2ba80843c037cc8b2a148ec81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Mar 2025 12:29:04 +0000 Subject: [PATCH 06/22] fix(storage): model always generates a min size - Min size is even generated when a device is not found. --- .../config_conversions/to_model_conversions/size.rb | 9 +++++++-- .../agama/storage/config_conversions/to_model_test.rb | 7 +++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/size.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/size.rb index 13ab91151a..b8000f920e 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions/size.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/size.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -39,11 +39,16 @@ def initialize(config) def conversions { default: config.default?, - min: config.min&.to_i, + min: convert_min_size, max: convert_max_size } end + # @return [Integer] + def convert_min_size + config.min&.to_i || 0 + end + # @return [Integer, nil] def convert_max_size max = config.max diff --git a/service/test/agama/storage/config_conversions/to_model_test.rb b/service/test/agama/storage/config_conversions/to_model_test.rb index 71b149c22d..db7317faf9 100644 --- a/service/test/agama/storage/config_conversions/to_model_test.rb +++ b/service/test/agama/storage/config_conversions/to_model_test.rb @@ -213,6 +213,13 @@ model_json = result_scope.call(subject.convert) expect(model_json[:partitions]).to eq( [ + { + delete: false, + deleteIfNeeded: false, + resize: false, + resizeIfNeeded: false, + size: { default: true, min: 0 } + }, default_partition_json ] ) From 4be9c6aca3b137f37b0fb661ae966c557426fa26 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, 3 Mar 2025 15:49:30 +0000 Subject: [PATCH 07/22] storage: add checker for model support --- .../agama/storage/model_support_checker.rb | 203 +++++++ .../model_support_checker_test.rb | 499 ++++++++++++++++++ 2 files changed, 702 insertions(+) create mode 100644 service/lib/agama/storage/model_support_checker.rb create mode 100644 service/test/agama/storage/config_conversions/model_support_checker_test.rb diff --git a/service/lib/agama/storage/model_support_checker.rb b/service/lib/agama/storage/model_support_checker.rb new file mode 100644 index 0000000000..4eefe3cc31 --- /dev/null +++ b/service/lib/agama/storage/model_support_checker.rb @@ -0,0 +1,203 @@ +# frozen_string_literal: true + +# Copyright (c) [2025] 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. + +module Agama + module Storage + # Class for checking whether a config is supported by the config model. + # + # Features will be added to the config model little by little. Ideally, this class will + # dissapear once the model supports all the features provided by the config. + class ModelSupportChecker + # @note A solved config is expected. Otherwise some checks cannot be done reliably. + # + # @param config [Storage::Config] + def initialize(config) + @config = config + end + + # Whether the config is completely supported by the config model. + # + # @return [Booelan] + def supported? + return @supported unless @supported.nil? + + @supported = !unsupported_config? + end + + private + + # @return [Storage::Config] + attr_reader :config + + # Whether the config is not supported by the config model. + # + # @return [Boolean] + def unsupported_config? # rubocop:disable Metrics/CyclomaticComplexity + any_unsupported_device? || + any_drive_without_name? || + any_drive_with_encryption? || + any_volume_group_without_name? || + any_volume_group_with_pvs? || + any_partition_without_mount_path? || + any_logical_volume_without_mount_path? || + any_logical_volume_with_encryption? + end + + # Whether there is any device that is not supported by the model. + # + # @return [Boolean] + def any_unsupported_device? + thin_pools = config.logical_volumes.select(&:pool?) + thin_volumes = config.logical_volumes.select(&:thin_volume?) + + [ + config.md_raids, + config.btrfs_raids, + config.nfs_mounts, + thin_pools, + thin_volumes + ].flatten.any? + end + + # Whether there is any mandatory drive without a name. + # + # @return [Boolean] + def any_drive_without_name? + config.drives.any? do |drive| + !drive.found_device && + !drive.search&.skip_device? && + !drive.search&.name + end + end + + # Whether there is any mandatory drive with encryption. + # + # @return [Boolean] + def any_drive_with_encryption? + config.drives.any? { |d| !d.search&.skip_device? && !d.encryption.nil? } + end + + # Whether there is any volume group without a name. + # + # @return [Boolean] + def any_volume_group_without_name? + !config.volume_groups.all?(&:name) + end + + # Only volume groups with automatically generated physical volumes are supported. + # @todo Revisit this check once individual physical volumes are supported by the model. + # + # @return [Boolean] + def any_volume_group_with_pvs? + config.volume_groups.any? { |v| v.physical_volumes.any? } + end + + # Whether there is any logical volume with missing mount path. + # @todo Revisit this check once volume groups can be reused. + # + # @return [Boolean] + def any_logical_volume_without_mount_path? + config.logical_volumes.any? { |p| !p.filesystem&.path } + end + + # Whether there is any logical volume with encryption. + # + # @return [Boolean] + def any_logical_volume_with_encryption? + config.logical_volumes.any?(&:encryption) + end + + # Whether there is any partition with missing mount path. + # @see #need_mount_path? + # + # @return [Boolean] + def any_partition_without_mount_path? + config.partitions.any? { |p| need_mount_path?(p) && !p.filesystem&.path } + end + + # Whether the config represents a partition that requires a mount path. + # + # A mount path is required for all the partitions that are going to be created. For a config + # reusing an existing partition, the mount path is required only if the partition does not + # represent a space policy action (delete or resize). + # + # @todo Revisit this check once individual physical volumes are supported by the model. The + # partitions representing the new physical volumes would not need a mount path. + # + # @param partition_config [Configs::Partition] + # @return [Boolean] + def need_mount_path?(partition_config) + return true if new_partition?(partition_config) + + reused_partition?(partition_config) && + !delete_action_partition?(partition_config) && + !resize_action_partition?(partition_config) + end + + # Whether the config represents a new partition to be created. + # + # @note The config has to be solved. Otherwise, in some cases it would be impossible to + # determine whether the partition is going to be created or reused. For example, if the + # config has a search and #if_not_found is set to :create. + # + # @param partition_config [Configs::Partition] + # @return [Boolean] + def new_partition?(partition_config) + partition_config.search.nil? || partition_config.search.create_device? + end + + # Whether the config is reusing an existing partition. + # + # @note The config has to be solved. Otherwise, in some cases it would be impossible to + # determine whether the partition is going to be reused or skipped. + # + # @param partition_config [Configs::Partition] + # @return [Boolean] + def reused_partition?(partition_config) + !new_partition?(partition_config) && !partition_config.search.skip_device? + end + + # Whether the partition is configured to be deleted or deleted if needed. + # + # @param partition_config [Configs::Partition] + # @return [Boolean] + def delete_action_partition?(partition_config) + return false unless reused_partition?(partition_config) + + partition_config.delete? || partition_config.delete_if_needed? + end + + # Whether the partition is configured to be resized if needed. + # + # @param partition_config [Configs::Partition] + # @return [Boolean] + def resize_action_partition?(partition_config) + return false unless reused_partition?(partition_config) + + partition_config.filesystem.nil? && + partition_config.encryption.nil? && + partition_config.size && + !partition_config.size.default? && + partition_config.size.min == Y2Storage::DiskSize.zero + end + end + end +end diff --git a/service/test/agama/storage/config_conversions/model_support_checker_test.rb b/service/test/agama/storage/config_conversions/model_support_checker_test.rb new file mode 100644 index 0000000000..75668465e7 --- /dev/null +++ b/service/test/agama/storage/config_conversions/model_support_checker_test.rb @@ -0,0 +1,499 @@ +# frozen_string_literal: true + +# Copyright (c) [2025] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../storage_helpers" +require_relative "../../../test_helper" +require "agama/storage/config_conversions/from_json" +require "agama/storage/config_solver" +require "agama/storage/model_support_checker" +require "y2storage/refinements" + +using Y2Storage::Refinements::SizeCasts + +describe Agama::Storage::ModelSupportChecker do + include Agama::RSpec::StorageHelpers + + let(:product_data) do + { + "storage" => { + "volumes" => ["/", "swap"], + "volume_templates" => [ + { + "mount_path" => "/", + "filesystem" => "btrfs", + "size" => { + "auto" => true, + "min" => "5 GiB", + "max" => "10 GiB" + }, + "btrfs" => { + "snapshots" => true + }, + "outline" => { + "required" => true, + "snapshots_configurable" => true, + "auto_size" => { + "base_min" => "5 GiB", + "base_max" => "10 GiB" + } + } + }, + { + "mount_path" => "/home", + "filesystem" => "xfs", + "size" => { + "auto" => false, + "min" => "5 GiB" + }, + "outline" => { + "required" => false + } + }, + { + "mount_path" => "swap", + "filesystem" => "swap", + "size" => { + "auto" => true + }, + "outline" => { + "auto_size" => { + "base_min" => "2 GiB", + "base_max" => "4 GiB" + } + } + }, + { + "mount_path" => "", + "filesystem" => "ext4", + "size" => { + "min" => "100 MiB" + } + } + ] + } + } + end + + let(:product_config) { Agama::Config.new(product_data) } + + let(:devicegraph) { Y2Storage::StorageManager.instance.probed } + + let(:config) do + Agama::Storage::ConfigConversions::FromJSON + .new(config_json) + .convert + .tap { |c| Agama::Storage::ConfigSolver.new(product_config, devicegraph).solve(c) } + end + + before do + mock_storage(devicegraph: scenario) + # To speed-up the tests + allow(Y2Storage::EncryptionMethod::TPM_FDE) + .to(receive(:possible?)) + .and_return(true) + end + + subject { described_class.new(config) } + + describe "#supported?" do + let(:scenario) { "disks.yaml" } + + # The drive is not found and it is not searched by name. + context "if there is a drive with unknown name" do + let(:scenario) { "empty-hd-50GiB.yaml" } + + let(:config_json) do + { + drives: [ + {}, + { search: { ifNotFound: if_not_found } } + ] + } + end + + context "and the drive is going to be skipped" do + let(:if_not_found) { "skip" } + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + + context "and the drive is not going to be skipped" do + let(:if_not_found) { "error" } + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + end + + context "if there is a drive with encryption" do + let(:config_json) do + { + drives: [ + { + search: { + condition: condition, + ifNotFound: "skip" + }, + encryption: { + luks1: { password: "12345" } + } + } + ] + } + end + + context "and the drive is going to be skipped" do + let(:condition) { { name: "/not/found" } } + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + + context "and the drive is not going to be skipped" do + let(:condition) { nil } + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + end + + context "if there is a LVM thin pool" do + let(:config_json) do + { + volumeGroups: [ + { + name: "system", + logicalVolumes: [ + { pool: true } + ] + } + ] + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "if there is a LVM thin volume" do + let(:config_json) do + { + volumeGroups: [ + { + name: "system", + logicalVolumes: [ + { usedPool: "pool" } + ] + } + ] + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "if there is a LVM volume group without name" do + let(:config_json) do + { + volumeGroups: [{}] + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "if there is a LVM volume group with specific physical volumes" do + let(:config_json) do + { + volumeGroups: [ + { + name: "system", + physicalVolumes: ["pv1"] + } + ] + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "if there is a partition without mount path" do + let(:scenario) { "disks.yaml" } + + let(:config_json) do + { + drives: [ + { + search: "/dev/vda", + partitions: [ + { + search: search, + delete: delete, + deleteIfNeeded: deleteIfNeeded, + filesystem: filesystem, + encryption: encryption, + size: size + } + ] + } + ] + } + end + + let(:search) { nil } + let(:delete) { nil } + let(:deleteIfNeeded) { nil } + let(:filesystem) { nil } + let(:encryption) { nil } + let(:size) { nil } + + context "and the partition has not a search (new partition)" do + let(:search) { nil } + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "and the partition has a search" do + let(:search) do + { + condition: condition, + ifNotFound: if_not_found + } + end + + let(:if_not_found) { nil } + + shared_examples "reused partition" do + context "and the partition is set to be deleted" do + let(:delete) { true } + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + + context "and the partition is set to be deleted if needed" do + let(:deleteIfNeeded) { true } + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + + context "and the partition is not set to be deleted" do + let(:delete) { false } + let(:deleteIfNeeded) { false } + + context "and the partition has encryption" do + let(:encryption) do + { luks1: { password: "12345" } } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "and the partition has filesystem" do + let(:filesystem) { { type: "xfs" } } + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "and the partition has a size" do + let(:size) do + { + default: false, + min: 1.GiB, + max: 10.GiB + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "and the partition is only set to be resized if needed" do + let(:encryption) { nil } + let(:filesystem) { nil } + let(:size) do + { + default: false, + min: Y2Storage::DiskSize.zero + } + end + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + end + end + + context "and the partition is found" do + let(:condition) { { name: "/dev/vda1" } } + + include_examples "reused partition" + end + + context "and the partition is not found" do + let(:condition) { { name: "/no/found" } } + + context "and the partition can be skipped" do + let(:if_not_found) { "skip" } + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + + context "and the partition cannot be skipped" do + let(:if_not_found) { "error" } + + include_examples "reused partition" + end + + context "and the partition can be created" do + let(:if_not_found) { "create" } + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + end + end + end + + context "if there is a LVM logical volume without mount path" do + let(:config_json) do + { + volumeGroups: [ + { + name: "system", + logicalVolumes: [{}] + } + ] + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "if there is a LVM logical volume with encryption" do + let(:config_json) do + { + volumeGroups: [ + { + name: "system", + logicalVolumes: [ + { + encryption: { + luks1: { password: "12345" } + } + } + ] + } + ] + } + end + + it "returns false" do + expect(subject.supported?).to eq(false) + end + end + + context "if the config is totally supported" do + let(:scenario) { "disks.yaml" } + + let(:config_json) do + { + drives: [ + { + search: "/dev/vda", + partitions: [ + { search: "/dev/vda1", delete: true }, + { + search: "/dev/vda2", + size: { default: false, min: 0 } + }, + { + encryption: { + luks1: { password: "12345" } + }, + filesystem: { path: "/", type: "btrfs" } + } + ] + }, + { + alias: "pv", + partitions: [ + { search: "*", delete: true } + ] + }, + { + partitions: [ + { search: "*", delete: true }, + { + filesystem: { path: "/home", type: "xfs" } + } + ] + } + ], + volumeGroups: [ + { + name: "data", + physicalVolumes: [{ generate: ["pv"] }], + logicalVolumes: [ + { + filesystem: { path: "/data" }, + size: { min: "10 GiB" } + } + ] + } + ] + } + end + + it "returns true" do + expect(subject.supported?).to eq(true) + end + end + end +end From e70204af8f1e38fd7fc399e23fa211f8adaa8782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Mar 2025 14:44:46 +0000 Subject: [PATCH 08/22] storage: use model support checker --- service/lib/agama/storage/proposal.rb | 16 +----- .../test/agama/dbus/storage/manager_test.rb | 14 ++--- service/test/agama/storage/proposal_test.rb | 51 ++++++++----------- 3 files changed, 30 insertions(+), 51 deletions(-) diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index abcd501486..2e622dee0e 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -24,6 +24,7 @@ require "agama/storage/config_checker" require "agama/storage/config_conversions" require "agama/storage/config_solver" +require "agama/storage/model_support_checker" require "agama/storage/proposal_settings" require "agama/storage/proposal_strategies" require "json" @@ -285,25 +286,12 @@ def config(solved: false) solved ? strategy.config : source_config end - # TODO: extract to a separate class (e.g., ModelSupportChecker) and add more checks, for - # example: the presence of thin pools, partitions without mount path, lvs without mount - # path, etc. - # # Whether the config model supports all features of the given config. # # @param config [Storage::Config] # @return [Boolean] def model_supported?(config) - unsupported_devices = [ - config.md_raids, - config.btrfs_raids, - config.nfs_mounts - ].flatten - - # Only volume groups with automatically generated pvs are supported - volume_groups_with_pvs = config.volume_groups.select { |v| v.physical_volumes.any? } - - unsupported_devices.empty? && volume_groups_with_pvs.empty? + ModelSupportChecker.new(config).supported? end # Calculates a proposal from guided JSON settings. diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index f582c396ea..bd6b664880 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -734,14 +734,14 @@ def serialize(value) it "returns the serialized config model" do expect(subject.recover_model).to eq( serialize({ - boot: { + boot: { configure: true, device: { default: true, name: "/dev/sda" } }, - drives: [ + drives: [ { name: "/dev/sda", alias: "root", @@ -765,7 +765,8 @@ def serialize(value) } ] } - ] + ], + volumeGroups: [] }) ) end @@ -810,14 +811,14 @@ def serialize(value) expect(result).to eq( serialize({ - boot: { + boot: { configure: true, device: { default: true, name: "/dev/sda" } }, - drives: [ + drives: [ { name: "/dev/sda", alias: "sda", @@ -841,7 +842,8 @@ def serialize(value) } ] } - ] + ], + volumeGroups: [] }) ) end diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb index dfe6246804..db828f00d7 100644 --- a/service/test/agama/storage/proposal_test.rb +++ b/service/test/agama/storage/proposal_test.rb @@ -358,7 +358,7 @@ def drive(partitions) subject.calculate_from_json(config_json) end - context "and the config contains an encrypted drive" do + context "and the model does not support the config" do let(:config_json) do { storage: { @@ -378,32 +378,12 @@ def drive(partitions) end end - context "and the config contains volume groups" do - let(:config_json) do - { - storage: { - volumeGroups: [ - { - name: "vg0" - } - ] - } - } - end - - it "returns nil" do - expect(subject.model_json).to be_nil - end - end - context "and the config has errors" do let(:config_json) do { storage: { drives: [ - { - search: "unknown" - } + { search: "unknown" } ] } } @@ -412,13 +392,20 @@ def drive(partitions) it "returns the config model" do expect(subject.model_json).to eq( { - boot: { + boot: { configure: true, device: { default: true } }, - drives: [] + drives: [ + { + name: "unknown", + spacePolicy: "keep", + partitions: [] + } + ], + volumeGroups: [] } ) end @@ -448,18 +435,18 @@ def drive(partitions) it "returns the config model" do expect(subject.model_json).to eq( { - boot: { + boot: { configure: true, device: { default: true, name: "/dev/sda" } }, - encryption: { + encryption: { method: "luks1", password: "12345" }, - drives: [ + drives: [ { name: "/dev/sda", alias: "root", @@ -483,7 +470,8 @@ def drive(partitions) } ] } - ] + ], + volumeGroups: [] } ) end @@ -510,14 +498,14 @@ def drive(partitions) result = subject.solve_model(model) expect(result).to eq({ - boot: { + boot: { configure: true, device: { default: true, name: "/dev/sda" } }, - drives: [ + drives: [ { name: "/dev/sda", alias: "sda", @@ -541,7 +529,8 @@ def drive(partitions) } ] } - ] + ], + volumeGroups: [] }) end From 5983925dc38579698fc0c272a0f550d589afc084 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Sat, 1 Mar 2025 23:02:25 +0000 Subject: [PATCH 09/22] web: Extract repeated code to a new DeviceMenu component --- web/src/components/storage/DriveEditor.tsx | 249 +++++------------- .../components/storage/utils/configEditor.tsx | 89 +++++++ 2 files changed, 159 insertions(+), 179 deletions(-) create mode 100644 web/src/components/storage/utils/configEditor.tsx diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx index 629c06716f..ab34d3ea9c 100644 --- a/web/src/components/storage/DriveEditor.tsx +++ b/web/src/components/storage/DriveEditor.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useId, useRef, useState } from "react"; +import React from "react"; import { useNavigate, generatePath } from "react-router-dom"; import { _, formatList } from "~/i18n"; import { sprintf } from "sprintf-js"; @@ -33,6 +33,7 @@ import { useDrive } from "~/queries/storage/config-model"; import * as driveUtils from "~/components/storage/utils/drive"; import * as partitionUtils from "~/components/storage/utils/partition"; import { contentDescription } from "~/components/storage/utils/device"; +import { DeviceMenu } from "~/components/storage/utils/configEditor"; import { Icon } from "../layout"; import { MenuHeader } from "~/components/core"; import MenuDeviceDescription from "./MenuDeviceDescription"; @@ -45,15 +46,9 @@ import { Flex, Label, Split, - Menu, - MenuContainer, - MenuContent, MenuItem, MenuItemAction, MenuList, - MenuToggle, - MenuToggleProps, - MenuToggleElement, MenuGroup, } from "@patternfly/react-core"; @@ -61,18 +56,6 @@ import spacingStyles from "@patternfly/react-styles/css/utilities/Spacing/spacin export type DriveEditorProps = { drive: configModel.Drive; driveDevice: StorageDevice }; -export const InlineMenuToggle = React.forwardRef( - (props: MenuToggleProps, ref: React.Ref) => ( - } - innerRef={ref} - variant="plain" - className="agm-inline-menu-toggle" - {...props} - /> - ), -); - // FIXME: Presentation is quite poor const SpacePolicySelectorIntro = ({ device }) => { const main = _("Choose what to with current content"); @@ -97,18 +80,13 @@ const SpacePolicySelectorIntro = ({ device }) => { }; const SpacePolicySelector = ({ drive, driveDevice }: DriveEditorProps) => { - const menuRef = useRef(); - const toggleMenuRef = useRef(); - const [isOpen, setIsOpen] = useState(false); const navigate = useNavigate(); const { setSpacePolicy } = useDrive(drive.name); - const onToggle = () => setIsOpen(!isOpen); const onSpacePolicyChange = (spacePolicy: configModel.SpacePolicy) => { if (spacePolicy === "custom") { return navigate(generatePath(PATHS.findSpace, { id: baseName(drive.name) })); } else { setSpacePolicy(spacePolicy); - setIsOpen(false); } }; @@ -132,31 +110,19 @@ const SpacePolicySelector = ({ drive, driveDevice }: DriveEditorProps) => { }; return ( - - {driveUtils.contentActionsDescription(drive)} - - } - menuRef={menuRef} - menu={ - - - }> - - - {SPACE_POLICIES.map((policy) => ( - - ))} - - - - - } - /> + {driveUtils.contentActionsDescription(drive)}} + activeItemId={currentPolicy.id} + > + }> + + + {SPACE_POLICIES.map((policy) => ( + + ))} + + + ); }; @@ -404,47 +370,22 @@ const RemoveDriveOption = ({ drive }) => { }; const DriveSelector = ({ drive, selected, toggleAriaLabel }) => { - const menuId = useId(); - const menuRef = useRef(); - const toggleMenuRef = useRef(); - const [isOpen, setIsOpen] = useState(false); const driveHandler = useDrive(drive.name); const onDriveChange = (newDriveName: string) => { driveHandler.switch(newDriveName); - setIsOpen(false); }; - const onToggle = () => setIsOpen(!isOpen); return ( - - {deviceLabel(selected)} - - } - menuRef={menuRef} - menu={ - - - - - - - - - } - // @ts-expect-error - popperProps={{ appendTo: document.body }} - /> + {deviceLabel(selected)}} + ariaLabel={toggleAriaLabel} + activeItemId={selected.sid} + > + + + + + ); }; @@ -519,52 +460,27 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { }; const PartitionsNoContentSelector = ({ drive, toggleAriaLabel }) => { - const menuId = useId(); - const menuRef = useRef(); - const toggleMenuRef = useRef(); - const [isOpen, setIsOpen] = useState(false); const navigate = useNavigate(); - const onToggle = () => setIsOpen(!isOpen); return ( - {_("No additional partitions will be created")}} + ariaLabel={toggleAriaLabel} + > + + navigate(generatePath(PATHS.addPartition, { id: baseName(drive.name) }))} > - {_("No additional partitions will be created")} - - } - menuRef={menuRef} - menu={ - - - - - navigate(generatePath(PATHS.addPartition, { id: baseName(drive.name) })) - } - > - - {_("Add or use partition")} - - - - - - } - /> + + {_("Add or use partition")} + + + + ); }; @@ -615,63 +531,38 @@ const PartitionMenuItem = ({ driveName, mountPath }) => { }; const PartitionsWithContentSelector = ({ drive, toggleAriaLabel }) => { - const menuId = useId(); - const menuRef = useRef(); - const toggleMenuRef = useRef(); - const [isOpen, setIsOpen] = useState(false); const navigate = useNavigate(); - const onToggle = () => setIsOpen(!isOpen); return ( - {driveUtils.contentDescription(drive)}} + ariaLabel={toggleAriaLabel} + > + + {drive.partitions + .filter((p) => p.mountPath) + .map((partition) => { + return ( + + ); + })} + + navigate(generatePath(PATHS.addPartition, { id: baseName(drive.name) }))} > - {driveUtils.contentDescription(drive)} - - } - menuRef={menuRef} - menu={ - - - - {drive.partitions - .filter((p) => p.mountPath) - .map((partition) => { - return ( - - ); - })} - - - navigate(generatePath(PATHS.addPartition, { id: baseName(drive.name) })) - } - > - - {_("Add or use partition")} - - - - - - } - /> + + {_("Add or use partition")} + + + + ); }; diff --git a/web/src/components/storage/utils/configEditor.tsx b/web/src/components/storage/utils/configEditor.tsx new file mode 100644 index 0000000000..24e4aed0fe --- /dev/null +++ b/web/src/components/storage/utils/configEditor.tsx @@ -0,0 +1,89 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +// @ts-check + +import React, { useId, useRef, useState } from "react"; +import { Icon } from "../../layout"; +import { + Menu, + MenuContainer, + MenuContent, + MenuToggle, + MenuToggleProps, + MenuToggleElement, +} from "@patternfly/react-core"; + +export const InlineMenuToggle = React.forwardRef( + (props: MenuToggleProps, ref: React.Ref) => ( + } + innerRef={ref} + variant="plain" + className="agm-inline-menu-toggle" + {...props} + /> + ), +); + +const DeviceMenu = ({ title, ariaLabel = undefined, activeItemId = undefined, children }) => { + const menuId = useId(); + const menuRef = useRef(); + const toggleMenuRef = useRef(); + const [isOpen, setIsOpen] = useState(false); + const onToggle = () => setIsOpen(!isOpen); + + return ( + + {title} + + } + menuRef={menuRef} + menu={ + setIsOpen(false)} + > + {children} + + } + // @ts-expect-error + popperProps={{ appendTo: document.body }} + /> + ); +}; + +export { DeviceMenu }; From 401648539a6fac218f35f523bc618783b6d1aa0b Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 3 Mar 2025 11:27:19 +0000 Subject: [PATCH 10/22] web: First real implementation of hasPV --- web/src/components/storage/DriveEditor.tsx | 22 +++++++++++----------- web/src/components/storage/utils/drive.tsx | 7 ------- web/src/queries/storage/config-model.ts | 8 ++++++++ 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx index ab34d3ea9c..2861a87059 100644 --- a/web/src/components/storage/DriveEditor.tsx +++ b/web/src/components/storage/DriveEditor.tsx @@ -130,7 +130,7 @@ const SearchSelectorIntro = ({ drive }: { drive: configModel.Drive }) => { const driveModel = useDrive(drive.name); if (!driveModel) return; - const { isBoot, isExplicitBoot } = driveModel; + const { isBoot, isExplicitBoot, hasPv } = driveModel; // TODO: Get volume groups associated to the drive. const volumeGroups = []; @@ -142,7 +142,7 @@ const SearchSelectorIntro = ({ drive }: { drive: configModel.Drive }) => { if (!driveUtils.hasFilesystem(drive)) { // The current device will be the only option to choose from - if (driveUtils.hasPv(drive)) { + if (hasPv) { if (volumeGroups.length > 1) { if (isExplicitBoot) { return _( @@ -195,7 +195,7 @@ const SearchSelectorIntro = ({ drive }: { drive: configModel.Drive }) => { const name = baseName(drive.name); - if (driveUtils.hasPv(drive)) { + if (hasPv) { if (volumeGroups.length > 1) { if (isExplicitBoot) { return sprintf( @@ -318,13 +318,13 @@ const SearchSelectorOptions = ({ drive, selected, onChange }) => { const driveModel = useDrive(drive.name); if (!driveModel) return; - const { isExplicitBoot } = driveModel; + const { isExplicitBoot, hasPv } = driveModel; // const boot = isExplicitBoot(drive.name); if (driveUtils.hasReuse(drive)) return ; if (!driveUtils.hasFilesystem(drive)) { - if (driveUtils.hasPv(drive) || isExplicitBoot) { + if (hasPv || isExplicitBoot) { return ; } @@ -349,10 +349,10 @@ const RemoveDriveOption = ({ drive }) => { if (!driveModel) return; - const { isExplicitBoot, delete: deleteDrive } = driveModel; + const { isExplicitBoot, hasPv, delete: deleteDrive } = driveModel; if (isExplicitBoot) return; - if (driveUtils.hasPv(drive)) return; + if (hasPv) return; if (driveUtils.hasRoot(drive)) return; return ( @@ -390,11 +390,11 @@ const DriveSelector = ({ drive, selected, toggleAriaLabel }) => { }; const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { - const { isBoot } = useDrive(drive.name); + const { isBoot, hasPv } = useDrive(drive.name); const text = (drive: configModel.Drive): string => { if (driveUtils.hasRoot(drive)) { - if (driveUtils.hasPv(drive)) { + if (hasPv) { if (isBoot) { // TRANSLATORS: %s will be replaced by the device name and its size - "/dev/sda, 20 GiB" return _("Use %s to install, host LVM and boot"); @@ -412,7 +412,7 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { } if (driveUtils.hasFilesystem(drive)) { - if (driveUtils.hasPv(drive)) { + if (hasPv) { if (isBoot) { // TRANSLATORS: %s will be replaced by the device name and its size - "/dev/sda, 20 GiB" return _("Use %s for LVM, additional partitions and booting"); @@ -429,7 +429,7 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { return _("Use %s for additional partitions"); } - if (driveUtils.hasPv(drive)) { + if (hasPv) { if (isBoot) { // TRANSLATORS: %s will be replaced by the device name and its size - "/dev/sda, 20 GiB" return _("Use %s to host LVM and boot"); diff --git a/web/src/components/storage/utils/drive.tsx b/web/src/components/storage/utils/drive.tsx index a4c3342444..41d39d0e0e 100644 --- a/web/src/components/storage/utils/drive.tsx +++ b/web/src/components/storage/utils/drive.tsx @@ -157,14 +157,7 @@ const hasReuse = (drive: configModel.Drive): boolean => { return drive.partitions && drive.partitions.some((p) => p.mountPath && p.name); }; -// TODO: maybe it should be moved to Drive hook from config-model. -// eslint-disable-next-line @typescript-eslint/no-unused-vars -const hasPv = (drive: configModel.Drive): boolean => { - return false; -}; - export { - hasPv, hasReuse, hasFilesystem, hasRoot, diff --git a/web/src/queries/storage/config-model.ts b/web/src/queries/storage/config-model.ts index 22dc392a2b..42fa0801f6 100644 --- a/web/src/queries/storage/config-model.ts +++ b/web/src/queries/storage/config-model.ts @@ -85,6 +85,12 @@ function isExplicitBoot(model: configModel.Config, driveName: string): boolean { return !model.boot?.device?.default && driveName === model.boot?.device?.name; } +function driveHasPv(model: configModel.Config, driveAlias: string): boolean { + if (!driveAlias) return false; + + return model.volumeGroups.flatMap((g) => g.targetDevices).includes(driveAlias); +} + function allMountPaths(drive: configModel.Drive): string[] { if (drive.mountPath) return [drive.mountPath]; @@ -411,6 +417,7 @@ export function useEncryption(): EncryptionHook { export type DriveHook = { isBoot: boolean; isExplicitBoot: boolean; + hasPv: boolean; allMountPaths: string[]; configuredExistingPartitions: configModel.Partition[]; switch: (newName: string) => void; @@ -432,6 +439,7 @@ export function useDrive(name: string): DriveHook | null { return { isBoot: isBoot(model, name), isExplicitBoot: isExplicitBoot(model, name), + hasPv: driveHasPv(model, drive.alias), allMountPaths: allMountPaths(drive), configuredExistingPartitions: configuredExistingPartitions(drive), switch: (newName) => mutate(switchDrive(model, name, newName)), From 451637c0b5cdf7371737d84deb4a5a0b47b5fc4d Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 3 Mar 2025 12:00:15 +0000 Subject: [PATCH 11/22] web: Refresh types/config-model.ts --- web/src/api/storage/types/config-model.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/web/src/api/storage/types/config-model.ts b/web/src/api/storage/types/config-model.ts index 8a109fb952..7cf4871e7d 100644 --- a/web/src/api/storage/types/config-model.ts +++ b/web/src/api/storage/types/config-model.ts @@ -37,6 +37,7 @@ export interface Config { boot?: Boot; encryption?: Encryption; drives?: Drive[]; + volumeGroups?: VolumeGroup[]; } export interface Boot { configure: boolean; @@ -83,3 +84,17 @@ export interface Size { min: number; max?: number; } +export interface VolumeGroup { + name: string; + extentSize?: number; + targetDevices?: Alias[]; + logicalVolumes?: LogicalVolume[]; +} +export interface LogicalVolume { + name?: string; + mountPath?: string; + filesystem?: Filesystem; + size?: Size; + stripes?: number; + stripeSize?: number; +} From eb08eb0d4225f27d70ca77ceff9fdc0f92fd6d02 Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 3 Mar 2025 12:35:02 +0000 Subject: [PATCH 12/22] web: First version of VolumeGroupEditor --- web/src/components/storage/ConfigEditor.tsx | 10 +- web/src/components/storage/DriveEditor.tsx | 10 +- .../components/storage/VolumeGroupEditor.tsx | 95 +++++++++++++++++++ .../components/storage/utils/configEditor.tsx | 14 ++- web/src/queries/storage/config-model.ts | 32 +++++++ 5 files changed, 152 insertions(+), 9 deletions(-) create mode 100644 web/src/components/storage/VolumeGroupEditor.tsx diff --git a/web/src/components/storage/ConfigEditor.tsx b/web/src/components/storage/ConfigEditor.tsx index 1e2238f1a9..548577a2a1 100644 --- a/web/src/components/storage/ConfigEditor.tsx +++ b/web/src/components/storage/ConfigEditor.tsx @@ -24,6 +24,7 @@ import React from "react"; import { useDevices } from "~/queries/storage"; import { useConfigModel } from "~/queries/storage/config-model"; import DriveEditor from "~/components/storage/DriveEditor"; +import VolumeGroupEditor from "~/components/storage/VolumeGroupEditor"; import { List, ListItem } from "@patternfly/react-core"; export default function ConfigEditor() { @@ -32,6 +33,13 @@ export default function ConfigEditor() { return ( + {model.volumeGroups.map((vg, i) => { + return ( + + + + ); + })} {model.drives.map((drive, i) => { const device = devices.find((d) => d.name === drive.name); @@ -42,7 +50,7 @@ export default function ConfigEditor() { if (device === undefined) return null; return ( - + ); diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx index 2861a87059..32c313eba5 100644 --- a/web/src/components/storage/DriveEditor.tsx +++ b/web/src/components/storage/DriveEditor.tsx @@ -33,7 +33,7 @@ import { useDrive } from "~/queries/storage/config-model"; import * as driveUtils from "~/components/storage/utils/drive"; import * as partitionUtils from "~/components/storage/utils/partition"; import { contentDescription } from "~/components/storage/utils/device"; -import { DeviceMenu } from "~/components/storage/utils/configEditor"; +import { DeviceHeader, DeviceMenu } from "~/components/storage/utils/configEditor"; import { Icon } from "../layout"; import { MenuHeader } from "~/components/core"; import MenuDeviceDescription from "./MenuDeviceDescription"; @@ -445,17 +445,13 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { // TRANSLATORS: %s will be replaced by the device name and its size - "/dev/sda, 20 GiB" return _("Use %s"); }; - - const [txt1, txt2] = text(drive).split("%s"); // TRANSLATORS: a disk drive const toggleAriaLabel = _("Drive"); return ( -

- {txt1} + - {txt2} -

+ ); }; diff --git a/web/src/components/storage/VolumeGroupEditor.tsx b/web/src/components/storage/VolumeGroupEditor.tsx new file mode 100644 index 0000000000..ea8dd7bfe8 --- /dev/null +++ b/web/src/components/storage/VolumeGroupEditor.tsx @@ -0,0 +1,95 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +import React from "react"; +import { _ } from "~/i18n"; +import { sprintf } from "sprintf-js"; +import { configModel } from "~/api/storage/types"; +import { useVolumeGroup } from "~/queries/storage/config-model"; +import { DeviceHeader, DeviceMenu } from "~/components/storage/utils/configEditor"; +import { + Card, + CardBody, + CardHeader, + CardTitle, + Flex, + MenuItem, + MenuList, +} from "@patternfly/react-core"; + +import spacingStyles from "@patternfly/react-styles/css/utilities/Spacing/spacing"; + +export type VolumeGroupEditorProps = { vg: configModel.VolumeGroup }; + +const RemoveVgOption = ({ vg }) => { + const volumeGroup = useVolumeGroup(vg.name); + const drive = volumeGroup.targetDrives[0]; + const desc = sprintf(_("The logical volumes will become partitions at %s"), drive.name); + + return ( + + {_("Do not create")} + + ); +}; + +const EditVgOption = () => { + return ( + + {_("Edit volume group")} + + ); +}; + +const VgMenu = ({ vg }) => { + return ( + {vg.name}}> + + + + + + ); +}; + +const VgHeader = ({ vg }: VolumeGroupEditorProps) => { + return ( + + + + ); +}; + +export default function VolumeGroupEditor({ vg }: VolumeGroupEditorProps) { + return ( + + + + + + + + + + + ); +} diff --git a/web/src/components/storage/utils/configEditor.tsx b/web/src/components/storage/utils/configEditor.tsx index 24e4aed0fe..bb60fccbbd 100644 --- a/web/src/components/storage/utils/configEditor.tsx +++ b/web/src/components/storage/utils/configEditor.tsx @@ -86,4 +86,16 @@ const DeviceMenu = ({ title, ariaLabel = undefined, activeItemId = undefined, ch ); }; -export { DeviceMenu }; +const DeviceHeader = ({ title, children }) => { + const [txt1, txt2] = title.split("%s"); + + return ( +

+ {txt1} + {children} + {txt2} +

+ ); +}; + +export { DeviceHeader, DeviceMenu }; diff --git a/web/src/queries/storage/config-model.ts b/web/src/queries/storage/config-model.ts index 42fa0801f6..41fddcbf6d 100644 --- a/web/src/queries/storage/config-model.ts +++ b/web/src/queries/storage/config-model.ts @@ -53,6 +53,14 @@ function findDrive(model: configModel.Config, driveName: string): configModel.Dr return drives.find((d) => d.name === driveName); } +function findVolumeGroup( + model: configModel.Config, + vgName: string, +): configModel.VolumeGroup | undefined { + const vgs = model?.volumeGroups || []; + return vgs.find((g) => g.name === vgName); +} + function removeDrive(model: configModel.Config, driveName: string): configModel.Config { model.drives = model.drives.filter((d) => d.name !== driveName); return model; @@ -91,6 +99,15 @@ function driveHasPv(model: configModel.Config, driveAlias: string): boolean { return model.volumeGroups.flatMap((g) => g.targetDevices).includes(driveAlias); } +function volumeGroupTargetDrives( + model: configModel.Config, + vg: configModel.VolumeGroup, +): configModel.Drive[] { + const aliases = vg.targetDevices; + + return aliases.map((a) => model.drives.find((d) => d.alias === a)).filter((d) => d); +} + function allMountPaths(drive: configModel.Drive): string[] { if (drive.mountPath) return [drive.mountPath]; @@ -455,6 +472,21 @@ export function useDrive(name: string): DriveHook | null { }; } +export type VolumeGroupHook = { + targetDrives: configModel.Drive[]; +}; + +export function useVolumeGroup(name: string): VolumeGroupHook | null { + const model = useConfigModel(); + const vg = findVolumeGroup(model, name); + + if (vg === undefined) return null; + + return { + targetDrives: volumeGroupTargetDrives(model, vg), + }; +} + export type ModelHook = { model: configModel.Config; usedMountPaths: string[]; From c0764ff4846f79e913e7e65085fdfd5d219376fb Mon Sep 17 00:00:00 2001 From: Ancor Gonzalez Sosa Date: Mon, 3 Mar 2025 16:50:38 +0000 Subject: [PATCH 13/22] web: Add list of logical volumes --- .../components/storage/DriveEditor.test.tsx | 3 +- web/src/components/storage/DriveEditor.tsx | 54 ++++--------------- .../components/storage/VolumeGroupEditor.tsx | 32 +++++++++-- .../components/storage/utils/configEditor.tsx | 45 +++++++++++++++- .../components/storage/utils/partition.tsx | 6 +-- .../components/storage/utils/volumeGroup.tsx | 46 ++++++++++++++++ 6 files changed, 134 insertions(+), 52 deletions(-) create mode 100644 web/src/components/storage/utils/volumeGroup.tsx diff --git a/web/src/components/storage/DriveEditor.test.tsx b/web/src/components/storage/DriveEditor.test.tsx index db24a8c89d..06c2e6b276 100644 --- a/web/src/components/storage/DriveEditor.test.tsx +++ b/web/src/components/storage/DriveEditor.test.tsx @@ -161,7 +161,6 @@ const drive2: ConfigModel.Drive = { }; const mockDeleteDrive = jest.fn(); -const mockGetPartition = jest.fn(); const mockDeletePartition = jest.fn(); jest.mock("~/queries/storage", () => ({ @@ -176,7 +175,7 @@ jest.mock("~/queries/storage/config-model", () => ({ useConfigModel: () => ({ drives: [drive1, drive2] }), useDrive: () => ({ delete: mockDeleteDrive, - getPartition: mockGetPartition, + getPartition: (path) => drive1.partitions.find((p) => p.mountPath === path), deletePartition: mockDeletePartition, }), })); diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx index 32c313eba5..3d592a2103 100644 --- a/web/src/components/storage/DriveEditor.tsx +++ b/web/src/components/storage/DriveEditor.tsx @@ -25,16 +25,18 @@ import { useNavigate, generatePath } from "react-router-dom"; import { _, formatList } from "~/i18n"; import { sprintf } from "sprintf-js"; import { baseName, deviceLabel, formattedPath, SPACE_POLICIES } from "~/components/storage/utils"; -import { useAvailableDevices, useVolume } from "~/queries/storage"; +import { useAvailableDevices } from "~/queries/storage"; import { configModel } from "~/api/storage/types"; import { StorageDevice } from "~/types/storage"; import { STORAGE as PATHS } from "~/routes/paths"; import { useDrive } from "~/queries/storage/config-model"; import * as driveUtils from "~/components/storage/utils/drive"; -import * as partitionUtils from "~/components/storage/utils/partition"; import { contentDescription } from "~/components/storage/utils/device"; -import { DeviceHeader, DeviceMenu } from "~/components/storage/utils/configEditor"; -import { Icon } from "../layout"; +import { + DeviceHeader, + DeviceMenu, + MountPathMenuItem, +} from "~/components/storage/utils/configEditor"; import { MenuHeader } from "~/components/core"; import MenuDeviceDescription from "./MenuDeviceDescription"; import { @@ -47,7 +49,6 @@ import { Label, Split, MenuItem, - MenuItemAction, MenuList, MenuGroup, } from "@patternfly/react-core"; @@ -481,48 +482,15 @@ const PartitionsNoContentSelector = ({ drive, toggleAriaLabel }) => { }; const PartitionMenuItem = ({ driveName, mountPath }) => { - const navigate = useNavigate(); const drive = useDrive(driveName); const partition = drive.getPartition(mountPath); - const volume = useVolume(mountPath); - const isRequired = volume.outline?.required || false; - const description = partition ? partitionUtils.typeWithSize(partition) : null; + const editPath = generatePath(PATHS.editPartition, { + id: baseName(driveName), + partitionId: encodeURIComponent(mountPath), + }); return ( - - } - actionId={`edit-${mountPath}`} - aria-label={`Edit ${mountPath}`} - onClick={() => - navigate( - generatePath(PATHS.editPartition, { - id: baseName(driveName), - partitionId: encodeURIComponent(mountPath), - }), - ) - } - /> - {!isRequired && ( - } - actionId={`delete-${mountPath}`} - aria-label={`Delete ${mountPath}`} - onClick={() => drive.deletePartition(mountPath)} - /> - )} - - } - > - {mountPath} - + ); }; diff --git a/web/src/components/storage/VolumeGroupEditor.tsx b/web/src/components/storage/VolumeGroupEditor.tsx index ea8dd7bfe8..139485ab47 100644 --- a/web/src/components/storage/VolumeGroupEditor.tsx +++ b/web/src/components/storage/VolumeGroupEditor.tsx @@ -24,8 +24,13 @@ import React from "react"; import { _ } from "~/i18n"; import { sprintf } from "sprintf-js"; import { configModel } from "~/api/storage/types"; +import { contentDescription } from "~/components/storage/utils/volumeGroup"; import { useVolumeGroup } from "~/queries/storage/config-model"; -import { DeviceHeader, DeviceMenu } from "~/components/storage/utils/configEditor"; +import { + DeviceHeader, + DeviceMenu, + MountPathMenuItem, +} from "~/components/storage/utils/configEditor"; import { Card, CardBody, @@ -72,13 +77,32 @@ const VgMenu = ({ vg }) => { }; const VgHeader = ({ vg }: VolumeGroupEditorProps) => { + const title = vg.logicalVolumes.length + ? _("Create LVM volume group %s") + : _("Empty LVM volume group %s"); + return ( - + ); }; +const LogicalVolumes = ({ vg }) => { + return ( + {contentDescription(vg)}} + ariaLabel={_("Logical volumes")} + > + + {vg.logicalVolumes.map((lv) => { + return ; + })} + + + ); +}; + export default function VolumeGroupEditor({ vg }: VolumeGroupEditorProps) { return ( @@ -88,7 +112,9 @@ export default function VolumeGroupEditor({ vg }: VolumeGroupEditorProps) { - + + + ); diff --git a/web/src/components/storage/utils/configEditor.tsx b/web/src/components/storage/utils/configEditor.tsx index bb60fccbbd..0fdafdf573 100644 --- a/web/src/components/storage/utils/configEditor.tsx +++ b/web/src/components/storage/utils/configEditor.tsx @@ -23,11 +23,16 @@ // @ts-check import React, { useId, useRef, useState } from "react"; +import { useNavigate } from "react-router-dom"; +import { useVolume } from "~/queries/storage"; +import * as partitionUtils from "~/components/storage/utils/partition"; import { Icon } from "../../layout"; import { Menu, MenuContainer, MenuContent, + MenuItem, + MenuItemAction, MenuToggle, MenuToggleProps, MenuToggleElement, @@ -98,4 +103,42 @@ const DeviceHeader = ({ title, children }) => { ); }; -export { DeviceHeader, DeviceMenu }; +const MountPathMenuItem = ({ device, editPath = undefined, deleteFn = undefined }) => { + const navigate = useNavigate(); + const mountPath = device.mountPath; + const volume = useVolume(mountPath); + const isRequired = volume.outline?.required || false; + const description = device ? partitionUtils.typeWithSize(device) : null; + + return ( + + } + actionId={`edit-${mountPath}`} + aria-label={`Edit ${mountPath}`} + onClick={() => editPath && navigate(editPath)} + /> + {!isRequired && ( + } + actionId={`delete-${mountPath}`} + aria-label={`Delete ${mountPath}`} + onClick={() => deleteFn && deleteFn(mountPath)} + /> + )} + + } + > + {mountPath} + + ); +}; + +export { DeviceHeader, DeviceMenu, MountPathMenuItem }; diff --git a/web/src/components/storage/utils/partition.tsx b/web/src/components/storage/utils/partition.tsx index 8dff68878b..98545476a8 100644 --- a/web/src/components/storage/utils/partition.tsx +++ b/web/src/components/storage/utils/partition.tsx @@ -30,7 +30,7 @@ import { configModel } from "~/api/storage/types"; /** * String to identify the drive. */ -const pathWithSize = (partition: configModel.Partition): string => { +const pathWithSize = (partition: configModel.Partition | configModel.LogicalVolume): string => { return sprintf( // TRANSLATORS: %1$s is an already formatted mount path (eg. "/"), // %2$s is a size description (eg. at least 10 GiB) @@ -43,7 +43,7 @@ const pathWithSize = (partition: configModel.Partition): string => { /** * String to identify the type of partition to be created (or used). */ -const typeDescription = (partition: configModel.Partition): string => { +const typeDescription = (partition: configModel.Partition | configModel.LogicalVolume): string => { const fs = filesystemType(partition.filesystem); if (partition.name) { @@ -64,7 +64,7 @@ const typeDescription = (partition: configModel.Partition): string => { /** * Combination of {@link typeDescription} and the size of the target partition. */ -const typeWithSize = (partition: configModel.Partition): string => { +const typeWithSize = (partition: configModel.Partition | configModel.LogicalVolume): string => { return sprintf( // TRANSLATORS: %1$s is a filesystem type description (eg. "Btrfs with snapshots"), // %2$s is a description of the size or the size limits (eg. "at least 10 GiB") diff --git a/web/src/components/storage/utils/volumeGroup.tsx b/web/src/components/storage/utils/volumeGroup.tsx new file mode 100644 index 0000000000..7a4bbd363d --- /dev/null +++ b/web/src/components/storage/utils/volumeGroup.tsx @@ -0,0 +1,46 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +// @ts-check + +import { _, n_, formatList } from "~/i18n"; +import { configModel } from "~/api/storage/types"; +import { formattedPath } from "~/components/storage/utils"; +import { sprintf } from "sprintf-js"; + +const contentDescription = (vg: configModel.VolumeGroup): string => { + if (vg.logicalVolumes.length === 0) return _("No logical volumes are defined yet"); + + const mountPaths = vg.logicalVolumes.map((v) => formattedPath(v.mountPath)); + return sprintf( + // TRANSLATORS: %s is a list of formatted mount points like '"/", "/var" and "swap"' (or a + // single mount point in the singular case). + n_( + "A new logical volume will be created for %s", + "New logical volumes will be created for %s", + mountPaths.length, + ), + formatList(mountPaths), + ); +}; + +export { contentDescription }; From 552352dce9fc5e353c1aeda108fab4f2cbf1bf6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Mar 2025 12:59:47 +0000 Subject: [PATCH 14/22] web: add initial storage model hook --- web/src/hooks/storage/model.ts | 88 +++++++++++++++++++++++++ web/src/queries/storage/config-model.ts | 2 + web/src/types/storage/model.ts | 38 +++++++++++ 3 files changed, 128 insertions(+) create mode 100644 web/src/hooks/storage/model.ts create mode 100644 web/src/types/storage/model.ts diff --git a/web/src/hooks/storage/model.ts b/web/src/hooks/storage/model.ts new file mode 100644 index 0000000000..be686feb2c --- /dev/null +++ b/web/src/hooks/storage/model.ts @@ -0,0 +1,88 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +import { useQuery } from "@tanstack/react-query"; +import { configModelQuery } from "~/queries/storage/config-model"; +import * as apiModel from "~/api/storage/types/config-model"; +import * as model from "~/types/storage/model"; + +function findDrive(modelData: apiModel.Config, alias: string): apiModel.Drive | undefined { + return modelData.drives.find((d) => d.alias === alias); +} + +function buildDrive(driveData: apiModel.Drive): model.Drive { + return { ...driveData }; +} + +function buildLogicalVolume(logicalVolumeData: apiModel.LogicalVolume): model.LogicalVolume { + return { ...logicalVolumeData }; +} + +function buildVolumeGroup( + volumeGroupData: apiModel.VolumeGroup, + modelData: apiModel.Config, +): model.VolumeGroup { + const buildTargetDevices = (): model.Drive[] => { + const aliases = volumeGroupData.targetDevices || []; + return aliases + .map((a) => findDrive(modelData, a)) + .filter((d) => d) + .map(buildDrive); + }; + + const buildLogicalVolumes = (): model.LogicalVolume[] => { + const logicalVolumesData = volumeGroupData.logicalVolumes || []; + return logicalVolumesData.map(buildLogicalVolume); + }; + + return { + ...volumeGroupData, + targetDevices: buildTargetDevices(), + logicalVolumes: buildLogicalVolumes(), + }; +} + +function buildModel(modelData: apiModel.Config): model.Model { + const buildVolumeGroups = (): model.VolumeGroup[] => { + const volumeGroupsData = modelData.volumeGroups || []; + return volumeGroupsData.map((v) => buildVolumeGroup(v, modelData)); + }; + + return { + volumeGroups: buildVolumeGroups(), + }; +} + +function useModel(): model.Model | null { + const { data } = useQuery(configModelQuery); + return data ? buildModel(data) : null; +} + +function useVolumeGroup(name: string): model.VolumeGroup | null { + const model = useModel(); + const volumeGroup = model?.volumeGroups?.find((v) => v.name === name); + return volumeGroup || null; +} + +export default useModel; + +export { useVolumeGroup }; diff --git a/web/src/queries/storage/config-model.ts b/web/src/queries/storage/config-model.ts index 41fddcbf6d..da8bdbb957 100644 --- a/web/src/queries/storage/config-model.ts +++ b/web/src/queries/storage/config-model.ts @@ -509,3 +509,5 @@ export function useModel(): ModelHook { unusedMountPaths: model ? unusedMountPaths(model, volumes) : [], }; } + +export { configModelQuery }; diff --git a/web/src/types/storage/model.ts b/web/src/types/storage/model.ts new file mode 100644 index 0000000000..4d3e4efdc5 --- /dev/null +++ b/web/src/types/storage/model.ts @@ -0,0 +1,38 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +import * as apiModel from "~/api/storage/types/config-model"; + +type Drive = apiModel.Drive; + +type LogicalVolume = apiModel.LogicalVolume; + +interface VolumeGroup extends Omit { + targetDevices: Drive[]; + logicalVolumes: LogicalVolume[]; +} + +type Model = { + volumeGroups: VolumeGroup[]; +}; + +export type { Model, Drive, VolumeGroup, LogicalVolume }; From 8555ca7a56be0e23dc8013b34bce730f7c091b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Mar 2025 14:13:25 +0000 Subject: [PATCH 15/22] web: use model hook --- .../components/storage/VolumeGroupEditor.tsx | 28 ++++++++-------- .../components/storage/utils/volumeGroup.tsx | 4 +-- web/src/queries/storage/config-model.ts | 32 ------------------- 3 files changed, 17 insertions(+), 47 deletions(-) diff --git a/web/src/components/storage/VolumeGroupEditor.tsx b/web/src/components/storage/VolumeGroupEditor.tsx index 139485ab47..1cdd49522e 100644 --- a/web/src/components/storage/VolumeGroupEditor.tsx +++ b/web/src/components/storage/VolumeGroupEditor.tsx @@ -23,9 +23,10 @@ import React from "react"; import { _ } from "~/i18n"; import { sprintf } from "sprintf-js"; -import { configModel } from "~/api/storage/types"; +import * as apiModel from "~/api/storage/types/config-model"; +import * as model from "~/types/storage/model"; import { contentDescription } from "~/components/storage/utils/volumeGroup"; -import { useVolumeGroup } from "~/queries/storage/config-model"; +import { useVolumeGroup } from "~/hooks/storage/model"; import { DeviceHeader, DeviceMenu, @@ -43,12 +44,9 @@ import { import spacingStyles from "@patternfly/react-styles/css/utilities/Spacing/spacing"; -export type VolumeGroupEditorProps = { vg: configModel.VolumeGroup }; - -const RemoveVgOption = ({ vg }) => { - const volumeGroup = useVolumeGroup(vg.name); - const drive = volumeGroup.targetDrives[0]; - const desc = sprintf(_("The logical volumes will become partitions at %s"), drive.name); +const RemoveVgOption = ({ vg }: { vg: model.VolumeGroup }) => { + const device = vg.targetDevices[0]; + const desc = sprintf(_("The logical volumes will become partitions at %s"), device.name); return ( @@ -65,7 +63,7 @@ const EditVgOption = () => { ); }; -const VgMenu = ({ vg }) => { +const VgMenu = ({ vg }: { vg: model.VolumeGroup }) => { return ( {vg.name}}> @@ -76,7 +74,7 @@ const VgMenu = ({ vg }) => { ); }; -const VgHeader = ({ vg }: VolumeGroupEditorProps) => { +const VgHeader = ({ vg }: { vg: model.VolumeGroup }) => { const title = vg.logicalVolumes.length ? _("Create LVM volume group %s") : _("Empty LVM volume group %s"); @@ -88,7 +86,7 @@ const VgHeader = ({ vg }: VolumeGroupEditorProps) => { ); }; -const LogicalVolumes = ({ vg }) => { +const LogicalVolumes = ({ vg }: { vg: model.VolumeGroup }) => { return ( {contentDescription(vg)}} @@ -103,17 +101,21 @@ const LogicalVolumes = ({ vg }) => { ); }; +export type VolumeGroupEditorProps = { vg: apiModel.VolumeGroup }; + export default function VolumeGroupEditor({ vg }: VolumeGroupEditorProps) { + const volumeGroup = useVolumeGroup(vg.name); + return ( - + - + diff --git a/web/src/components/storage/utils/volumeGroup.tsx b/web/src/components/storage/utils/volumeGroup.tsx index 7a4bbd363d..bbb70adb0c 100644 --- a/web/src/components/storage/utils/volumeGroup.tsx +++ b/web/src/components/storage/utils/volumeGroup.tsx @@ -23,11 +23,11 @@ // @ts-check import { _, n_, formatList } from "~/i18n"; -import { configModel } from "~/api/storage/types"; +import * as model from "~/types/storage/model"; import { formattedPath } from "~/components/storage/utils"; import { sprintf } from "sprintf-js"; -const contentDescription = (vg: configModel.VolumeGroup): string => { +const contentDescription = (vg: model.VolumeGroup): string => { if (vg.logicalVolumes.length === 0) return _("No logical volumes are defined yet"); const mountPaths = vg.logicalVolumes.map((v) => formattedPath(v.mountPath)); diff --git a/web/src/queries/storage/config-model.ts b/web/src/queries/storage/config-model.ts index da8bdbb957..f951e5325b 100644 --- a/web/src/queries/storage/config-model.ts +++ b/web/src/queries/storage/config-model.ts @@ -53,14 +53,6 @@ function findDrive(model: configModel.Config, driveName: string): configModel.Dr return drives.find((d) => d.name === driveName); } -function findVolumeGroup( - model: configModel.Config, - vgName: string, -): configModel.VolumeGroup | undefined { - const vgs = model?.volumeGroups || []; - return vgs.find((g) => g.name === vgName); -} - function removeDrive(model: configModel.Config, driveName: string): configModel.Config { model.drives = model.drives.filter((d) => d.name !== driveName); return model; @@ -99,15 +91,6 @@ function driveHasPv(model: configModel.Config, driveAlias: string): boolean { return model.volumeGroups.flatMap((g) => g.targetDevices).includes(driveAlias); } -function volumeGroupTargetDrives( - model: configModel.Config, - vg: configModel.VolumeGroup, -): configModel.Drive[] { - const aliases = vg.targetDevices; - - return aliases.map((a) => model.drives.find((d) => d.alias === a)).filter((d) => d); -} - function allMountPaths(drive: configModel.Drive): string[] { if (drive.mountPath) return [drive.mountPath]; @@ -472,21 +455,6 @@ export function useDrive(name: string): DriveHook | null { }; } -export type VolumeGroupHook = { - targetDrives: configModel.Drive[]; -}; - -export function useVolumeGroup(name: string): VolumeGroupHook | null { - const model = useConfigModel(); - const vg = findVolumeGroup(model, name); - - if (vg === undefined) return null; - - return { - targetDrives: volumeGroupTargetDrives(model, vg), - }; -} - export type ModelHook = { model: configModel.Config; usedMountPaths: string[]; From 4c46f937e7d46958759b84a628b27473546f9169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Mar 2025 15:03:18 +0000 Subject: [PATCH 16/22] web: add basic test for config editor --- .../components/storage/ConfigEditor.test.tsx | 112 ++++++++++++++++++ web/src/components/storage/ConfigEditor.tsx | 4 +- 2 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 web/src/components/storage/ConfigEditor.test.tsx diff --git a/web/src/components/storage/ConfigEditor.test.tsx b/web/src/components/storage/ConfigEditor.test.tsx new file mode 100644 index 0000000000..8c2e3e90fd --- /dev/null +++ b/web/src/components/storage/ConfigEditor.test.tsx @@ -0,0 +1,112 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +import React from "react"; +import { screen } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import ConfigEditor from "~/components/storage/ConfigEditor"; +import { StorageDevice } from "~/types/storage"; +import * as apiModel from "~/api/storage/types/config-model"; + +const disk: StorageDevice = { + sid: 60, + type: "disk", + isDrive: true, + description: "", + vendor: "Seagate", + model: "Unknown", + driver: ["ahci", "mmcblk"], + bus: "IDE", + name: "/dev/vda", + size: 1e6, +}; + +const mockUseDevices = jest.fn(); +jest.mock("~/queries/storage", () => ({ + ...jest.requireActual("~/queries/storage"), + useDevices: () => mockUseDevices(), +})); + +const mockUseConfigModel = jest.fn(); +jest.mock("~/queries/storage/config-model", () => ({ + ...jest.requireActual("~/queries/storage/config-model"), + useConfigModel: () => mockUseConfigModel(), +})); + +jest.mock("./DriveEditor", () => () =>
drive editor
); +jest.mock("./VolumeGroupEditor", () => () =>
volume group editor
); + +beforeEach(() => { + mockUseDevices.mockReturnValue([disk]); +}); + +describe("if no drive is used for installation", () => { + beforeEach(() => { + const modelData: apiModel.Config = {}; + mockUseConfigModel.mockReturnValue(modelData); + }); + + it("does not render the drive editor", () => { + plainRender(); + expect(screen.queryByText("drive editor")).not.toBeInTheDocument(); + }); +}); + +describe("if a drive is used for installation", () => { + beforeEach(() => { + const modelData: apiModel.Config = { + drives: [{ name: "/dev/vda" }], + }; + mockUseConfigModel.mockReturnValue(modelData); + }); + + it("renders the drive editor", () => { + plainRender(); + expect(screen.queryByText("drive editor")).toBeInTheDocument(); + }); +}); + +describe("if no volume group is used for installation", () => { + beforeEach(() => { + const modelData: apiModel.Config = {}; + mockUseConfigModel.mockReturnValue(modelData); + }); + + it("does not render the volume group editor", () => { + plainRender(); + expect(screen.queryByText("volume group editor")).not.toBeInTheDocument(); + }); +}); + +describe("if a volume group is used for installation", () => { + beforeEach(() => { + const modelData: apiModel.Config = { + volumeGroups: [{ name: "/dev/system" }], + }; + mockUseConfigModel.mockReturnValue(modelData); + }); + + it("renders the drive editor", () => { + plainRender(); + expect(screen.queryByText("volume group editor")).toBeInTheDocument(); + }); +}); diff --git a/web/src/components/storage/ConfigEditor.tsx b/web/src/components/storage/ConfigEditor.tsx index 548577a2a1..cd0335a13c 100644 --- a/web/src/components/storage/ConfigEditor.tsx +++ b/web/src/components/storage/ConfigEditor.tsx @@ -33,14 +33,14 @@ export default function ConfigEditor() { return ( - {model.volumeGroups.map((vg, i) => { + {model.volumeGroups?.map((vg, i) => { return ( ); })} - {model.drives.map((drive, i) => { + {model.drives?.map((drive, i) => { const device = devices.find((d) => d.name === drive.name); /** From e6cf582deed14d8c07d8e233a8df3c8ebb7ed170 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, 7 Mar 2025 11:32:14 +0000 Subject: [PATCH 17/22] storage: make global encryption to work with LVM --- .../to_model_conversions/config.rb | 32 ++- .../lib/agama/storage/config_solvers/boot.rb | 14 +- service/lib/agama/storage/configs/drive.rb | 7 +- .../agama/storage/configs/logical_volume.rb | 7 +- .../lib/agama/storage/configs/partition.rb | 9 +- .../agama/storage/configs/with_filesystem.rb | 39 +++ .../config_conversions/to_model_test.rb | 239 +++++++++++++----- 7 files changed, 255 insertions(+), 92 deletions(-) create mode 100644 service/lib/agama/storage/configs/with_filesystem.rb diff --git a/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb b/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb index 76f7f70db3..0b04adc599 100644 --- a/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb +++ b/service/lib/agama/storage/config_conversions/to_model_conversions/config.rb @@ -88,19 +88,37 @@ def base_encryption root_encryption || first_encryption end - # Encryption from root partition. + # Encryption for root. # - # @return [Configs::Encryption, nil] nil if there is no encryption for root partition. + # @note If root is a logical volume, then the encryption of the automatically generated + # physical volumes is considered. Encryption from logical volumes is ignored. + # + # @return [Configs::Encryption, nil] nil if there is no encryption for root. def root_encryption - root_partition = config.partitions.find { |p| p.filesystem&.root? } - root_partition&.encryption + root_partition&.encryption || root_volume_group&.physical_volumes_encryption end - # Encryption from the first encrypted partition. + # Partition config for root. # - # @return [Configs::Encryption, nil] nil if there is no encrypted partition. + # @return [Configs::Partition, nil] + def root_partition + config.partitions.find(&:root?) + end + + # Volume group config containing a logical volume for root. + # + # @return [Configs::LogicalVolume, nil] + def root_volume_group + config.volume_groups.find { |v| v.logical_volumes.any?(&:root?) } + end + + # Encryption from the first encrypted partition or from the first volume group with + # automatically generated and encrypted physical volumes. + # + # @return [Configs::Encryption, nil] def first_encryption - config.partitions.find(&:encryption)&.encryption + config.partitions.find(&:encryption)&.encryption || + config.volume_groups.find(&:physical_volumes_encryption)&.physical_volumes_encryption end end end diff --git a/service/lib/agama/storage/config_solvers/boot.rb b/service/lib/agama/storage/config_solvers/boot.rb index ad17840237..40096a2e72 100644 --- a/service/lib/agama/storage/config_solvers/boot.rb +++ b/service/lib/agama/storage/config_solvers/boot.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -82,7 +82,7 @@ def root_volume_group_config # @param config [Configs::Drive] # @return [Boolean] def root_drive_config?(config) - config.partitions.any? { |p| root_config?(p) } + config.partitions.any?(&:root?) end # Whether the given volume group config contains a root logical volume config. @@ -90,15 +90,7 @@ def root_drive_config?(config) # @param config [Configs::VolumeGroup] # @return [Boolean] def root_volume_group_config?(config) - config.logical_volumes.any? { |l| root_config?(l) } - end - - # Whether the given config if for the root filesystem. - # - # @param config [#filesystem] - # @return [Boolean] - def root_config?(config) - config.filesystem&.root? + config.logical_volumes.any?(&:root?) end # Whether the given drive config can be used to allocate physcial volumes. diff --git a/service/lib/agama/storage/configs/drive.rb b/service/lib/agama/storage/configs/drive.rb index 8c6c3a7bcf..edb3d2f265 100644 --- a/service/lib/agama/storage/configs/drive.rb +++ b/service/lib/agama/storage/configs/drive.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -21,6 +21,7 @@ require "agama/storage/configs/search" require "agama/storage/configs/with_alias" +require "agama/storage/configs/with_filesystem" require "agama/storage/configs/with_search" module Agama @@ -30,14 +31,12 @@ module Configs # system and that can be used as a regular disk. class Drive include WithAlias + include WithFilesystem include WithSearch # @return [Encryption, nil] attr_accessor :encryption - # @return [Filesystem, nil] - attr_accessor :filesystem - # @return [Y2Storage::PartitionTables::Type, nil] attr_accessor :ptable_type diff --git a/service/lib/agama/storage/configs/logical_volume.rb b/service/lib/agama/storage/configs/logical_volume.rb index 1f1696a5a1..65120bf269 100644 --- a/service/lib/agama/storage/configs/logical_volume.rb +++ b/service/lib/agama/storage/configs/logical_volume.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -21,6 +21,7 @@ require "agama/storage/configs/size" require "agama/storage/configs/with_alias" +require "agama/storage/configs/with_filesystem" module Agama module Storage @@ -28,6 +29,7 @@ module Configs # Section of the configuration representing a LVM logical volume. class LogicalVolume include WithAlias + include WithFilesystem # @return [String, nil] attr_accessor :name @@ -51,9 +53,6 @@ class LogicalVolume # @return [Encryption, nil] attr_accessor :encryption - # @return [Filesystem, nil] - attr_accessor :filesystem - def initialize @size = Size.new @pool = false diff --git a/service/lib/agama/storage/configs/partition.rb b/service/lib/agama/storage/configs/partition.rb index d615ade999..babbfd0bf7 100644 --- a/service/lib/agama/storage/configs/partition.rb +++ b/service/lib/agama/storage/configs/partition.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -20,8 +20,9 @@ # find current contact information at www.suse.com. require "agama/storage/configs/size" -require "agama/storage/configs/with_search" require "agama/storage/configs/with_alias" +require "agama/storage/configs/with_filesystem" +require "agama/storage/configs/with_search" module Agama module Storage @@ -49,6 +50,7 @@ def self.new_for_shrink_any_if_needed end include WithAlias + include WithFilesystem include WithSearch # @return [Boolean] @@ -68,9 +70,6 @@ def self.new_for_shrink_any_if_needed # @return [Encryption, nil] attr_accessor :encryption - # @return [Filesystem, nil] - attr_accessor :filesystem - def initialize @size = Size.new @delete = false diff --git a/service/lib/agama/storage/configs/with_filesystem.rb b/service/lib/agama/storage/configs/with_filesystem.rb new file mode 100644 index 0000000000..2f48158a0a --- /dev/null +++ b/service/lib/agama/storage/configs/with_filesystem.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# Copyright (c) [2025] 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. + +module Agama + module Storage + module Configs + # Mixin for configs with filesystem. + module WithFilesystem + # @return [Filesystem, nil] + attr_accessor :filesystem + + # Whether the config is for the root filesystem. + # + # @return [Boolean] + def root? + filesystem&.root? + end + end + end + end +end diff --git a/service/test/agama/storage/config_conversions/to_model_test.rb b/service/test/agama/storage/config_conversions/to_model_test.rb index db7317faf9..76661cb653 100644 --- a/service/test/agama/storage/config_conversions/to_model_test.rb +++ b/service/test/agama/storage/config_conversions/to_model_test.rb @@ -745,87 +745,204 @@ end end - context "if the root partition is encrypted" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/" }, - encryption: { - luks1: { password: "12345" } + context "for the global encryption" do + context "if the root partition is encrypted" do + let(:config_json) do + { + drives: [ + { + alias: "vda", + partitions: [ + { + filesystem: { path: "/" }, + encryption: { + luks1: { password: "12345" } + } } - } - ] + ] + } + ], + volumeGroups: [ + { + name: "test", + physicalVolumes: [ + { + generate: { + targetDevices: ["vda"], + encryption: { + luks2: { password: "54321" } + } + } + } + ], + logicalVolumes: [ + { filesystem: { path: "/home" } } + ] + } + ] + } + end + + it "generates the expected JSON for 'encryption'" do + encryption_model = subject.convert[:encryption] + + expect(encryption_model).to eq( + { + method: "luks1", + password: "12345" } - ] - } + ) + end end - it "generates the expected JSON for 'encryption'" do - encryption_model = subject.convert[:encryption] - - expect(encryption_model).to eq( + context "if there is a root logical volume" do + let(:config_json) do { - method: "luks1", - password: "12345" + drives: [ + { + alias: "vda", + partitions: [ + { + filesystem: { path: "/home" }, + encryption: { + luks1: { password: "12345" } + } + } + ] + } + ], + volumeGroups: [ + { + name: "test", + physicalVolumes: physicalVolumes, + logicalVolumes: [ + { filesystem: { path: "/" } } + ] + } + ] } - ) - end - end + end - context "if the root partition is not encrypted but other partition is encrypted" do - let(:config_json) do - { - drives: [ - { - partitions: [ - { - filesystem: { path: "/" } - }, - { - encryption: { - luks1: { password: "12345" } + context "and the volume group has automatically generated and encrypted physical volumes" do + let(:physicalVolumes) do + [ + { + generate: { + targetDevices: ["vda"], + encryption: { + luks2: { password: "54321" } } } - ] - } - ] - } - end + } + ] + end + + it "generates the expected JSON for 'encryption'" do + encryption_model = subject.convert[:encryption] - it "generates the expected JSON for 'encryption'" do - encryption_model = subject.convert[:encryption] + expect(encryption_model).to eq( + { + method: "luks2", + password: "54321" + } + ) + end + end + end - expect(encryption_model).to eq( + context "if there is no encryption for root" do + let(:config_json) do { - method: "luks1", - password: "12345" + drives: [ + { + alias: "vda", + partitions: [ + { + filesystem: { path: "/" } + }, + { + filesystem: { path: "/home" }, + encryption: encryption + } + ] + } + ], + volumeGroups: [ + { + name: "test", + physicalVolumes: physicalVolumes, + logicalVolumes: [ + { filesystem: { path: "swap" } } + ] + } + ] } - ) - end - end + end - context "if there is not an encrypted partition" do - let(:config_json) do - { - drives: [ + let(:physicalVolumes) do + [ { - partitions: [ - { - filesystem: { path: "/" } + generate: { + targetDevices: ["vda"], + encryption: { + luks2: { password: "54321" } } - ] + } } ] - } - end + end - it "generates the expected JSON for 'encryption'" do - encryption_model = subject.convert[:encryption] + context "and there is an encrypted partition" do + let(:encryption) do + { + luks1: { password: "12345" } + } + end + + it "generates the expected JSON for 'encryption'" do + encryption_model = subject.convert[:encryption] + + expect(encryption_model).to eq( + { + method: "luks1", + password: "12345" + } + ) + end + end + + context "and there is no encrypted partition" do + let(:encryption) { nil } + + it "generates the expected JSON for 'encryption'" do + encryption_model = subject.convert[:encryption] + + expect(encryption_model).to eq( + { + method: "luks2", + password: "54321" + } + ) + end - expect(encryption_model).to be_nil + context "if there is no automatically generated and encrypted physical volumes" do + let(:physicalVolumes) do + [ + { + generate: { + targetDevices: ["vda"] + } + } + ] + end + + it "generates the expected JSON for 'encryption'" do + encryption_model = subject.convert[:encryption] + + expect(encryption_model).to be_nil + end + end + end end end From 8668a57a6c24d476d57302809589bf0bc82f6521 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, 7 Mar 2025 11:45:41 +0000 Subject: [PATCH 18/22] service: changelog --- service/package/rubygem-agama-yast.changes | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index a7d674c8b1..9b730ece93 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Fri Mar 7 11:37:47 UTC 2025 - José Iván López González + +- Export LVM config to the storage model and add more checks for + the model supported features (gh#agama-project/agama#2089). + ------------------------------------------------------------------- Wed Mar 5 14:50:04 UTC 2025 - Ladislav Slezák @@ -15,7 +21,7 @@ Wed Mar 5 08:09:28 UTC 2025 - Michal Filka - introduced boot_strategy into storage section of product definition yaml file. It allows to control what boot strategy - will be proposed by storage. Currently works only for BLS. + will be proposed by storage. Currently works only for BLS. ------------------------------------------------------------------- Fri Feb 28 13:03:11 UTC 2025 - Imobach Gonzalez Sosa From 0dd659f2c080feda465f70f0dfb596421e8560ad 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, 7 Mar 2025 11:46:20 +0000 Subject: [PATCH 19/22] rust: changelog --- rust/package/agama.changes | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 5adec11d4f..d99d6d0cea 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Fri Mar 7 11:40:56 UTC 2025 - José Iván López González + +- Extend storage model schema with LVM (gh#agama-project/agama#2089). + ------------------------------------------------------------------- Thu Mar 6 12:51:42 UTC 2025 - Imobach Gonzalez Sosa From 7d64c00a82e1be976d8245866cc936ea562d9772 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, 7 Mar 2025 11:46:40 +0000 Subject: [PATCH 20/22] web: changelog --- web/package/agama-web-ui.changes | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index 8c1d32cd12..8273339a77 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Fri Mar 7 11:42:33 UTC 2025 - José Iván López González + +- Show LVM config in the storage UI (gh#agama-project/agama#2089). + ------------------------------------------------------------------- Thu Mar 6 08:06:17 UTC 2025 - David Diaz From 2dfb8d240933ca69642a3f2f0247bef746a7b608 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, 10 Mar 2025 09:18:51 +0000 Subject: [PATCH 21/22] web: rename utils file --- web/src/components/storage/VolumeGroupEditor.tsx | 2 +- .../storage/utils/{volumeGroup.tsx => volume-group.tsx} | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) rename web/src/components/storage/utils/{volumeGroup.tsx => volume-group.tsx} (99%) diff --git a/web/src/components/storage/VolumeGroupEditor.tsx b/web/src/components/storage/VolumeGroupEditor.tsx index 1cdd49522e..d8568d1968 100644 --- a/web/src/components/storage/VolumeGroupEditor.tsx +++ b/web/src/components/storage/VolumeGroupEditor.tsx @@ -25,7 +25,7 @@ import { _ } from "~/i18n"; import { sprintf } from "sprintf-js"; import * as apiModel from "~/api/storage/types/config-model"; import * as model from "~/types/storage/model"; -import { contentDescription } from "~/components/storage/utils/volumeGroup"; +import { contentDescription } from "~/components/storage/utils/volume-group"; import { useVolumeGroup } from "~/hooks/storage/model"; import { DeviceHeader, diff --git a/web/src/components/storage/utils/volumeGroup.tsx b/web/src/components/storage/utils/volume-group.tsx similarity index 99% rename from web/src/components/storage/utils/volumeGroup.tsx rename to web/src/components/storage/utils/volume-group.tsx index bbb70adb0c..3c58409d71 100644 --- a/web/src/components/storage/utils/volumeGroup.tsx +++ b/web/src/components/storage/utils/volume-group.tsx @@ -20,8 +20,6 @@ * find current contact information at www.suse.com. */ -// @ts-check - import { _, n_, formatList } from "~/i18n"; import * as model from "~/types/storage/model"; import { formattedPath } from "~/components/storage/utils"; From 44a7bb67b1a7b7e29a9dbdfd0d953e20b58c07ff 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, 10 Mar 2025 09:19:33 +0000 Subject: [PATCH 22/22] web: extract components --- web/src/components/storage/DeviceHeader.tsx | 39 +++++++++ .../configEditor.tsx => DeviceMenu.tsx} | 80 ++++--------------- web/src/components/storage/DriveEditor.tsx | 8 +- .../components/storage/MountPathMenuItem.tsx | 77 ++++++++++++++++++ .../components/storage/VolumeGroupEditor.tsx | 8 +- 5 files changed, 139 insertions(+), 73 deletions(-) create mode 100644 web/src/components/storage/DeviceHeader.tsx rename web/src/components/storage/{utils/configEditor.tsx => DeviceMenu.tsx} (54%) create mode 100644 web/src/components/storage/MountPathMenuItem.tsx diff --git a/web/src/components/storage/DeviceHeader.tsx b/web/src/components/storage/DeviceHeader.tsx new file mode 100644 index 0000000000..f0a9a78bf6 --- /dev/null +++ b/web/src/components/storage/DeviceHeader.tsx @@ -0,0 +1,39 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +import React from "react"; +import { Content } from "@patternfly/react-core"; + +export type DeviceHeaderProps = { + title: string; + children: React.ReactNode; +}; + +export default function DeviceHeader({ title, children }: DeviceHeaderProps) { + const [txt1, txt2] = title.split("%s"); + + return ( + + {txt1} {children} {txt2} + + ); +} diff --git a/web/src/components/storage/utils/configEditor.tsx b/web/src/components/storage/DeviceMenu.tsx similarity index 54% rename from web/src/components/storage/utils/configEditor.tsx rename to web/src/components/storage/DeviceMenu.tsx index 0fdafdf573..6492b9ddbf 100644 --- a/web/src/components/storage/utils/configEditor.tsx +++ b/web/src/components/storage/DeviceMenu.tsx @@ -20,25 +20,19 @@ * find current contact information at www.suse.com. */ -// @ts-check - import React, { useId, useRef, useState } from "react"; -import { useNavigate } from "react-router-dom"; -import { useVolume } from "~/queries/storage"; -import * as partitionUtils from "~/components/storage/utils/partition"; -import { Icon } from "../../layout"; +import { Icon } from "~/components/layout"; import { Menu, + MenuProps, MenuContainer, MenuContent, - MenuItem, - MenuItemAction, MenuToggle, MenuToggleProps, MenuToggleElement, } from "@patternfly/react-core"; -export const InlineMenuToggle = React.forwardRef( +const InlineMenuToggle = React.forwardRef( (props: MenuToggleProps, ref: React.Ref) => ( } @@ -50,7 +44,19 @@ export const InlineMenuToggle = React.forwardRef( ), ); -const DeviceMenu = ({ title, ariaLabel = undefined, activeItemId = undefined, children }) => { +export type DeviceMenuProps = { + title: string | React.ReactNode; + ariaLabel?: string; + activeItemId?: MenuProps["activeItemId"]; + children: React.ReactNode; +}; + +export default function DeviceMenu({ + title, + ariaLabel = undefined, + activeItemId = undefined, + children, +}: DeviceMenuProps) { const menuId = useId(); const menuRef = useRef(); const toggleMenuRef = useRef(); @@ -89,56 +95,4 @@ const DeviceMenu = ({ title, ariaLabel = undefined, activeItemId = undefined, ch popperProps={{ appendTo: document.body }} /> ); -}; - -const DeviceHeader = ({ title, children }) => { - const [txt1, txt2] = title.split("%s"); - - return ( -

- {txt1} - {children} - {txt2} -

- ); -}; - -const MountPathMenuItem = ({ device, editPath = undefined, deleteFn = undefined }) => { - const navigate = useNavigate(); - const mountPath = device.mountPath; - const volume = useVolume(mountPath); - const isRequired = volume.outline?.required || false; - const description = device ? partitionUtils.typeWithSize(device) : null; - - return ( - - } - actionId={`edit-${mountPath}`} - aria-label={`Edit ${mountPath}`} - onClick={() => editPath && navigate(editPath)} - /> - {!isRequired && ( - } - actionId={`delete-${mountPath}`} - aria-label={`Delete ${mountPath}`} - onClick={() => deleteFn && deleteFn(mountPath)} - /> - )} - - } - > - {mountPath} - - ); -}; - -export { DeviceHeader, DeviceMenu, MountPathMenuItem }; +} diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx index 3d592a2103..a38baf1175 100644 --- a/web/src/components/storage/DriveEditor.tsx +++ b/web/src/components/storage/DriveEditor.tsx @@ -32,11 +32,9 @@ import { STORAGE as PATHS } from "~/routes/paths"; import { useDrive } from "~/queries/storage/config-model"; import * as driveUtils from "~/components/storage/utils/drive"; import { contentDescription } from "~/components/storage/utils/device"; -import { - DeviceHeader, - DeviceMenu, - MountPathMenuItem, -} from "~/components/storage/utils/configEditor"; +import DeviceMenu from "~/components/storage/DeviceMenu"; +import DeviceHeader from "~/components/storage/DeviceHeader"; +import MountPathMenuItem from "~/components/storage/MountPathMenuItem"; import { MenuHeader } from "~/components/core"; import MenuDeviceDescription from "./MenuDeviceDescription"; import { diff --git a/web/src/components/storage/MountPathMenuItem.tsx b/web/src/components/storage/MountPathMenuItem.tsx new file mode 100644 index 0000000000..6f8ef3ab29 --- /dev/null +++ b/web/src/components/storage/MountPathMenuItem.tsx @@ -0,0 +1,77 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * 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. + */ + +import React from "react"; +import { useNavigate } from "react-router-dom"; +import { useVolume } from "~/queries/storage"; +import * as partitionUtils from "~/components/storage/utils/partition"; +import { Icon } from "~/components/layout"; +import { MenuItem, MenuItemAction } from "@patternfly/react-core"; +import * as apiModel from "~/api/storage/types/config-model"; + +export type MountPathMenuItemProps = { + device: apiModel.Partition | apiModel.LogicalVolume; + editPath?: string; + deleteFn?: (mountPath: string) => void; +}; + +export default function MountPathMenuItem({ + device, + editPath = undefined, + deleteFn = undefined, +}: MountPathMenuItemProps) { + const navigate = useNavigate(); + const mountPath = device.mountPath; + const volume = useVolume(mountPath); + const isRequired = volume.outline?.required || false; + const description = device ? partitionUtils.typeWithSize(device) : null; + + return ( + + } + actionId={`edit-${mountPath}`} + aria-label={`Edit ${mountPath}`} + onClick={() => editPath && navigate(editPath)} + /> + {!isRequired && ( + } + actionId={`delete-${mountPath}`} + aria-label={`Delete ${mountPath}`} + onClick={() => deleteFn && deleteFn(mountPath)} + /> + )} + + } + > + {mountPath} + + ); +} diff --git a/web/src/components/storage/VolumeGroupEditor.tsx b/web/src/components/storage/VolumeGroupEditor.tsx index d8568d1968..4cf7c1800c 100644 --- a/web/src/components/storage/VolumeGroupEditor.tsx +++ b/web/src/components/storage/VolumeGroupEditor.tsx @@ -27,11 +27,9 @@ import * as apiModel from "~/api/storage/types/config-model"; import * as model from "~/types/storage/model"; import { contentDescription } from "~/components/storage/utils/volume-group"; import { useVolumeGroup } from "~/hooks/storage/model"; -import { - DeviceHeader, - DeviceMenu, - MountPathMenuItem, -} from "~/components/storage/utils/configEditor"; +import DeviceMenu from "~/components/storage/DeviceMenu"; +import DeviceHeader from "~/components/storage/DeviceHeader"; +import MountPathMenuItem from "~/components/storage/MountPathMenuItem"; import { Card, CardBody,