diff --git a/src/lib/y2storage/proposal/space_maker.rb b/src/lib/y2storage/proposal/space_maker.rb index ff6a8fd77..870d4ae30 100644 --- a/src/lib/y2storage/proposal/space_maker.rb +++ b/src/lib/y2storage/proposal/space_maker.rb @@ -361,14 +361,17 @@ def execute_action(action, graph, skip, planned_partitions = nil, disk_name = ni def execute_shrink(action, devicegraph, planned_partitions, disk_name) log.info "SpaceMaker#execute_shrink - #{action}" - if action.shrink_size.nil? + if action.target_size.nil? + part = devicegraph.find_device(action.sid) if planned_partitions - part = devicegraph.find_device(action.sid) - action.shrink_size = resizing_size(part, planned_partitions, disk_name) + resizing = resizing_size(part, planned_partitions, disk_name) + action.target_size = resizing > part.size ? DiskSize.zero : part.size - resizing else - action.shrink_size = DiskSize.Unlimited + # Mandatory resize + action.target_size = part.size end end + action.shrink(devicegraph) end diff --git a/src/lib/y2storage/proposal/space_maker_actions/bigger_resize_strategy.rb b/src/lib/y2storage/proposal/space_maker_actions/bigger_resize_strategy.rb index 583fafbfa..3feb9f43f 100644 --- a/src/lib/y2storage/proposal/space_maker_actions/bigger_resize_strategy.rb +++ b/src/lib/y2storage/proposal/space_maker_actions/bigger_resize_strategy.rb @@ -35,34 +35,29 @@ def initialize(settings, _disk_analyzer) @to_delete_mandatory = [] @to_delete_optional = [] @to_wipe = [] - @to_resize = [] + @to_shrink_mandatory = [] + @to_shrink_optional = [] end # @param disk [Disk] see {List} def add_mandatory_actions(disk) return unless disk.partition_table? - devices = partitions(disk).select { |p| configured?(p, :force_delete) } - to_delete_mandatory.concat(devices) + add_mandatory_delete(disk) + add_mandatory_shrink(disk) end # @param disk [Disk] see {List} def add_optional_actions(disk, _lvm_helper) add_wipe(disk) - add_resize(disk) + add_optional_shrink(disk) add_optional_delete(disk) end # @return [Action, nil] nil if there are no more actions in the list def next source = source_for_next - dev = send(source).first - return unless dev - - return Shrink.new(dev) if source == :to_resize - return Wipe.new(dev) if source == :to_wipe - - Delete.new(dev, related_partitions: false) + send(source).first end # @param deleted_sids [Array] see {List} @@ -70,7 +65,8 @@ def done(deleted_sids) send(source_for_next).shift cleanup(to_delete_mandatory, deleted_sids) cleanup(to_delete_optional, deleted_sids) - cleanup(to_resize, deleted_sids) + cleanup(to_shrink_mandatory, deleted_sids) + cleanup(to_shrink_optional, deleted_sids) end private @@ -78,16 +74,19 @@ def done(deleted_sids) # @return [ProposalSpaceSettings] proposal settings for making space attr_reader :settings - # @return [Array] list of devices to be deleted (mandatory) + # @return [Array] list of mandatory delete actions attr_reader :to_delete_mandatory - # @return [Array] list of devices to be deleted (optionally) + # @return [Array] list of optional delete actions attr_reader :to_delete_optional - # @return [Array] list of partitions to be shrunk - attr_reader :to_resize + # @return [Array] list of mandatory shrink actions + attr_reader :to_shrink_mandatory - # @return [Array] list of disks to be emptied if needed + # @return [Array] list of optional shrink actions + attr_reader :to_shrink_optional + + # @return [Array] list of actions to wipe disks if needed attr_reader :to_wipe # @see #add_optional_actions @@ -95,32 +94,61 @@ def done(deleted_sids) def add_wipe(disk) return if disk.partition_table? - to_wipe << disk + to_wipe << Wipe.new(disk) end # @see #add_optional_actions # @param disk [Disk] - def add_resize(disk) + def add_optional_shrink(disk) return unless disk.partition_table? - partitions = partitions(disk).select { |p| configured?(p, :resize) } - return if partitions.empty? + actions = optional_shrinks(disk) + return if actions.empty? + + @to_shrink_optional = (to_shrink_optional + actions).sort do |a, b| + preferred_resize(a, b, disk.devicegraph) + end + end + + # @see #add_optional_shrink + # @param disk [Disk] + def optional_shrinks(disk) + partitions(disk).map do |part| + resize = resize_actions.find { |a| a.device == part.name } + next unless resize + next if resize.min_size && resize.min_size > part.size + next if resize.max_size && resize.max_size < part.size - @to_resize = (to_resize + partitions).sort { |a, b| preferred_resize(a, b) } + resize_to_shrink(part, resize) + end.compact end - # Compares two partitions to decide which one should be resized first + # Compares two shrinking operations to decide which one should be executed first # - # @param part1 [Partition] - # @param part2 [Partition] - def preferred_resize(part1, part2) - result = part2.recoverable_size <=> part1.recoverable_size + # @param resize1 [Shrink] + # @param resize2 [Shrink] + def preferred_resize(resize1, resize2, devicegraph) + part1 = devicegraph.find_device(resize1.sid) + part2 = devicegraph.find_device(resize2.sid) + result = recoverable_size(part2, resize2) <=> recoverable_size(part1, resize1) return result unless result.zero? # Just to ensure stable sorting between different executions in case of draw part1.name <=> part2.name end + # Max space that can be recovered from the given partition, having into account the + # restrictions imposed by the its Resize action + # + # @see #preferred_resize + def recoverable_size(partition, resize) + if resize.min_size.nil? || resize.min_size > partition.size + return partition.recoverable_size + end + + [partition.recoverable_size, partition.size - resize.min_size].min + end + # @see #add_optional_actions # # @param disk [Disk] @@ -128,7 +156,9 @@ def add_optional_delete(disk) return unless disk.partition_table? partitions = partitions(disk).select { |p| configured?(p, :delete) } - to_delete_optional.concat(partitions.sort { |a, b| preferred_delete(a, b) }) + partitions.sort! { |a, b| preferred_delete(a, b) } + actions = partitions.map { |p| Delete.new(p, related_partitions: false) } + to_delete_optional.concat(actions) end # Compares two partitions to decide which one should be deleted first @@ -136,10 +166,36 @@ def add_optional_delete(disk) # @param part1 [Partition] # @param part2 [Partition] def preferred_delete(part1, part2) - # Mimic order from the auto strategy. We might consider other approaches in the future. + # FIXME: Currently this mimics the order from the auto strategy. + # We might consider other approaches in the future, like deleting partitions that are + # next to another partition that needs to grow. That circumstance is maybe not so easy + # to evaluate at the start and needs to be reconsidered after every action. part2.region.start <=> part1.region.start end + # @see #add_optional_actions + # @param disk [Disk] + def add_mandatory_shrink(disk) + shrink_actions = partitions(disk).map do |part| + resize = resize_actions.find { |a| a.device == part.name } + next unless resize + next unless resize.max_size + next if part.size <= resize.max_size + + resize_to_shrink(part, resize) + end.compact + + to_shrink_mandatory.concat(shrink_actions) + end + + # @see #add_mandatory_actions + # @param disk [Disk] + def add_mandatory_delete(disk) + devices = partitions(disk).select { |p| configured?(p, :force_delete) } + actions = devices.map { |d| Delete.new(d, related_partitions: false) } + to_delete_mandatory.concat(actions) + end + # Whether the given action is configured for the given device at the proposal settings # # @see ProposalSpaceSettings#actions @@ -148,12 +204,17 @@ def preferred_delete(part1, part2) # @param action [Symbol] :force_delete, :delete or :resize # @return [Boolean] def configured?(device, action) - settings.actions[device.name]&.to_sym == action + case action + when :force_delete + delete_actions.select(&:mandatory).any? { |a| a.device == device.name } + when :delete + delete_actions.reject(&:mandatory).any? { |a| a.device == device.name } + end end # Removes devices with the given sids from a collection # - # @param collection [Array] + # @param collection [Array] # @param deleted_sids [Array] def cleanup(collection, deleted_sids) collection.delete_if { |d| deleted_sids.include?(d.sid) } @@ -167,8 +228,10 @@ def source_for_next :to_delete_mandatory elsif to_wipe.any? :to_wipe - elsif to_resize.any? - :to_resize + elsif to_shrink_mandatory.any? + :to_shrink_mandatory + elsif to_shrink_optional.any? + :to_shrink_optional else :to_delete_optional end @@ -178,6 +241,24 @@ def source_for_next def partitions(disk) disk.partitions.reject { |part| part.type.is?(:extended) } end + + # Trivial conversion + def resize_to_shrink(partition, resize) + Shrink.new(partition).tap do |shrink| + shrink.min_size = resize.min_size + shrink.max_size = resize.max_size + end + end + + # All delete actions from the settings + def delete_actions + settings.actions.select { |a| a.is?(:delete) } + end + + # All resize actions from the settings + def resize_actions + settings.actions.select { |a| a.is?(:resize) } + end end end end diff --git a/src/lib/y2storage/proposal/space_maker_actions/shrink.rb b/src/lib/y2storage/proposal/space_maker_actions/shrink.rb index 727ed9242..13fd7a91b 100644 --- a/src/lib/y2storage/proposal/space_maker_actions/shrink.rb +++ b/src/lib/y2storage/proposal/space_maker_actions/shrink.rb @@ -26,12 +26,25 @@ module SpaceMakerActions # # @see Base class Shrink < Base - # @return [DiskSize] size of the space to substract ideally - attr_accessor :shrink_size + # Minimal size of the device set by the configuration, regardless of the ResizeInfo + # + # If set to nil, there is no artificial limit + # @return [DiskSize, nil] + attr_accessor :min_size + + # Max size of the device set by the configuration, regardless of the ResizeInfo + # + # If set to nil, there is no artificial limit + # @return [DiskSize, nil] + attr_accessor :max_size + + # Ideal final size of the device to satisfy the SpaceMaker algorithm + # @return [DiskSize] + attr_accessor :target_size # Reduces the size of the target partition # - # If possible, it reduces the size of the partition by {#shrink_size}. + # If possible, it reduces the size of the partition to {#adjusted_target_size}. # Otherwise, it reduces the size as much as possible. # # This method does not take alignment into account. @@ -48,9 +61,21 @@ def shrink(devicegraph) # @param partition [Partition] def shrink_partition(partition) - target = shrink_size.unlimited? ? DiskSize.zero : partition.size - shrink_size # Explicitly avoid alignment to keep current behavior (to be reconsidered) - partition.resize(target, align_type: nil) + partition.resize(adjusted_target_size, align_type: nil) + end + + # Real target size taking into account the max and min limits + # + # @return [DiskSize] + def adjusted_target_size + if min_size && min_size > target_size + min_size + elsif max_size && max_size < target_size + max_size + else + target_size + end end end end diff --git a/src/lib/y2storage/proposal_space_settings.rb b/src/lib/y2storage/proposal_space_settings.rb index aa54684b2..5987b599d 100644 --- a/src/lib/y2storage/proposal_space_settings.rb +++ b/src/lib/y2storage/proposal_space_settings.rb @@ -19,6 +19,7 @@ require "yast" require "y2storage/equal_by_instance_variables" +require "y2storage/space_actions" module Y2Storage # Class to encapsulate all the GuidedProposal settings related to the process of making space @@ -84,27 +85,20 @@ def self.delete_modes # What to do with existing partitions if they are involved in the process of making space. # - # Keys are device names (like in BlkDevice#name, no alternative names) that correspond to a - # partition. - # - # The value for each key specifies what to do with the corresponding partition if the storage - # proposal needs to process the corresponding disk. If the device is not explicitly mentioned, - # nothing will be done. Possible values are :resize, :delete and :force_delete. - # # Entries for devices that are not involved in the proposal are ignored. For example, if all # the volumes are configured to be placed at /dev/sda but there is an entry like - # `{"/dev/sdb1" => :force_delete}`, the corresponding /dev/sdb1 partition will NOT be deleted - # because there is no reason for the proposal to process the disk /dev/sdb. + # Delete, the corresponding /dev/sdb1 partition will NOT + # be deleted because there is no reason for the proposal to process the disk /dev/sdb. # # Device names corresponding to extended partitions are also ignored. The storage proposal only # considers actions for primary and logical partitions. # - # @return [Hash{String => Symbol}] + # @return [Array] attr_accessor :actions def initialize @strategy = :auto - @actions = {} + @actions = [] end # Whether the settings disable deletion of a given type of partitions diff --git a/src/lib/y2storage/space_actions.rb b/src/lib/y2storage/space_actions.rb new file mode 100644 index 000000000..6db98c5d0 --- /dev/null +++ b/src/lib/y2storage/space_actions.rb @@ -0,0 +1,27 @@ +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +module Y2Storage + # Namespace for the objects representing the actions of the bigger_resize SpaceMaker strategy + module SpaceActions + end +end + +require "y2storage/space_actions/delete" +require "y2storage/space_actions/resize" diff --git a/src/lib/y2storage/space_actions/base.rb b/src/lib/y2storage/space_actions/base.rb new file mode 100644 index 000000000..c9d92e574 --- /dev/null +++ b/src/lib/y2storage/space_actions/base.rb @@ -0,0 +1,46 @@ +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "y2storage/equal_by_instance_variables" + +module Y2Storage + module SpaceActions + # Base class for representing the actions of the bigger_resize SpaceMaker strategy + class Base + include EqualByInstanceVariables + attr_reader :device + + # Constructor + def initialize(device) + @device = device + end + + # Checks whether this is a concrete kind(s) of action + # @return [Boolean] + def is?(*types) + (types.map(&:to_sym) & types_for_is).any? + end + + # @see #is? + def types_for_is + [] + end + end + end +end diff --git a/src/lib/y2storage/space_actions/delete.rb b/src/lib/y2storage/space_actions/delete.rb new file mode 100644 index 000000000..bbf6ddab5 --- /dev/null +++ b/src/lib/y2storage/space_actions/delete.rb @@ -0,0 +1,42 @@ +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "y2storage/space_actions/base" + +module Y2Storage + module SpaceActions + # Delete action to configure the bigger_resize SpaceMaker strategy + class Delete < Base + # Whether the delete action must always be executed (if the involved disk is processed) + # @return [Boolean] + attr_reader :mandatory + + # Constructor + def initialize(device, mandatory: false) + super(device) + @mandatory = mandatory + end + + # @see #is? + def types_for_is + [:delete] + end + end + end +end diff --git a/src/lib/y2storage/space_actions/resize.rb b/src/lib/y2storage/space_actions/resize.rb new file mode 100644 index 000000000..41e8d2b9a --- /dev/null +++ b/src/lib/y2storage/space_actions/resize.rb @@ -0,0 +1,53 @@ +# Copyright (c) [2024] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "y2storage/space_actions/base" + +module Y2Storage + module SpaceActions + # Resize action to configure the bigger_resize SpaceMaker strategy + class Resize < Base + # Min size the device should have. + # + # Nil is equivalent to the initial size of the device (no shrinking, only growing). + # + # @return [DiskSize, nil] + attr_reader :min_size + + # Max size the device should have. + # + # Nil is equivalent to the initial size of the device (no growing, only shrinking). + # + # @return [DiskSize, nil] + attr_reader :max_size + + # Constructor + def initialize(device, min_size: DiskSize.zero, max_size: nil) + super(device) + @min_size = min_size + @max_size = max_size + end + + # @see #is? + def types_for_is + [:resize] + end + end + end +end diff --git a/test/y2storage/proposal/space_maker_bigger_resize_test.rb b/test/y2storage/proposal/space_maker_bigger_resize_test.rb index 0ce952ac8..c99dff899 100755 --- a/test/y2storage/proposal/space_maker_bigger_resize_test.rb +++ b/test/y2storage/proposal/space_maker_bigger_resize_test.rb @@ -39,8 +39,10 @@ def probed_partition(name) settings.space_settings.actions = settings_actions settings end - let(:settings_actions) { {} } + let(:settings_actions) { [] } let(:analyzer) { Y2Storage::DiskAnalyzer.new(fake_devicegraph) } + let(:delete) { Y2Storage::SpaceActions::Delete } + let(:resize) { Y2Storage::SpaceActions::Resize } subject(:maker) { described_class.new(analyzer, settings) } @@ -48,7 +50,7 @@ def probed_partition(name) let(:scenario) { "complex-lvm-encrypt" } context "if no device is set as :force_delete " do - let(:settings_actions) { { "/dev/sda1" => :delete, "/dev/sda2" => :delete } } + let(:settings_actions) { [delete.new("/dev/sda1"), delete.new("/dev/sda2")] } it "does not delete any partition" do result = maker.prepare_devicegraph(fake_devicegraph) @@ -56,9 +58,9 @@ def probed_partition(name) end end - # :force_delete for disks should be ignored, actions only make sense for partitions and LVs + # Mandatory delete for disks should be ignored, actions only make sense for partitions and LVs context "if :force_delete is specified for a disk that contains partitions" do - let(:settings_actions) { { "/dev/sda" => :force_delete } } + let(:settings_actions) { [delete.new("/dev/sda", mandatory: true)] } it "does not delete any partition" do result = maker.prepare_devicegraph(fake_devicegraph) @@ -68,6 +70,9 @@ def probed_partition(name) context "if :force_delete is specified for several partitions" do let(:settings_actions) { { "/dev/sda2" => :force_delete, "/dev/sde1" => :force_delete } } + let(:settings_actions) do + [delete.new("/dev/sda2", mandatory: true), delete.new("/dev/sde1", mandatory: true)] + end it "does not delete partitions out of SpaceMaker#candidate_devices" do result = maker.prepare_devicegraph(fake_devicegraph) @@ -80,11 +85,14 @@ def probed_partition(name) end end - # :force_delete for disks should be ignored, actions only make sense for partitions and LVs + # Mandatory delete for disks should be ignored, actions only make sense for partitions and LVs context "if :force_delete is specified for a directly formatted disk (no partition table)" do let(:scenario) { "multipath-formatted.xml" } - let(:settings_actions) { { "/dev/mapper/0QEMU_QEMU_HARDDISK_mpath1" => :force_delete } } + let(:settings_actions) do + [delete.new("/dev/mapper/0QEMU_QEMU_HARDDISK_mpath1", mandatory: true)] + end + before do settings.candidate_devices = ["/dev/mapper/0QEMU_QEMU_HARDDISK_mpath1"] settings.root_device = "/dev/mapper/0QEMU_QEMU_HARDDISK_mpath1" @@ -104,7 +112,7 @@ def probed_partition(name) context "when deleting a btrfs partition that is part of a multidevice btrfs" do let(:scenario) { "btrfs-multidevice-over-partitions.xml" } - let(:settings_actions) { { "/dev/sda1" => :force_delete } } + let(:settings_actions) { [delete.new("/dev/sda1", mandatory: true)] } it "deletes the partitions explicitly mentioned in the settings" do result = maker.prepare_devicegraph(fake_devicegraph) @@ -119,7 +127,7 @@ def probed_partition(name) context "when deleting a partition that is part of a raid" do let(:scenario) { "raid0-over-partitions.xml" } - let(:settings_actions) { { "/dev/sda1" => :force_delete } } + let(:settings_actions) { [delete.new("/dev/sda1", mandatory: true)] } it "deletes the partitions explicitly mentioned in the settings" do result = maker.prepare_devicegraph(fake_devicegraph) @@ -134,7 +142,7 @@ def probed_partition(name) context "when deleting a partition that is part of a lvm volume group" do let(:scenario) { "lvm-over-partitions.xml" } - let(:settings_actions) { { "/dev/sda1" => :force_delete } } + let(:settings_actions) { [delete.new("/dev/sda1", mandatory: true)] } it "deletes the partitions explicitly mentioned in the settings" do result = maker.prepare_devicegraph(fake_devicegraph) @@ -146,6 +154,31 @@ def probed_partition(name) expect(result.partitions.map(&:name)).to include "/dev/sda2", "/dev/sda3", "/dev/sdb2" end end + + context "if there is a resize action with a max that is smaller than the partition size" do + using Y2Storage::Refinements::SizeCasts + + let(:scenario) { "irst-windows-linux-gpt" } + let(:settings_actions) do + [resize.new("/dev/sda2"), resize.new("/dev/sda3", max_size: 200.GiB)] + end + + let(:resize_info) do + instance_double("ResizeInfo", resize_ok?: true, min_size: 100.GiB, max_size: 800.GiB) + end + + before do + allow_any_instance_of(Y2Storage::Partition) + .to receive(:detect_resize_info).and_return(resize_info) + end + + it "resizes the partition to the specified max if possible" do + result = maker.prepare_devicegraph(fake_devicegraph) + expect(result.partitions).to include( + an_object_having_attributes(filesystem_label: "other", size: 200.GiB) + ) + end + end end describe "#provide_space" do @@ -254,10 +287,10 @@ def probed_partition(name) context "if resizing some partitions and deleting others is allowed" do let(:settings_actions) do - { - "/dev/sda2" => :resize, "/dev/sda3" => :resize, - "/dev/sda5" => :delete, "/dev/sda6" => :delete - } + [ + resize.new("/dev/sda2"), resize.new("/dev/sda3"), + delete.new("/dev/sda5"), delete.new("/dev/sda6") + ] end context "and resizing one partition is enough" do @@ -285,7 +318,7 @@ def probed_partition(name) end end - context "and resizing one partition is not enough" do + context "and resizing one partition is not enough because more space is needed" do let(:volumes) { [vol1, vol2] } it "resizes subsequent partitions" do @@ -311,6 +344,39 @@ def probed_partition(name) end end + context "and resizing one partition is not enough because the action is limited" do + let(:volumes) { [vol1] } + + let(:settings_actions) do + [ + resize.new("/dev/sda2", min_size: 250.GiB), resize.new("/dev/sda3"), + delete.new("/dev/sda5"), delete.new("/dev/sda6") + ] + end + + it "resizes the more 'productive' partition taking restrictions into account" do + result = maker.provide_space(fake_devicegraph, volumes, lvm_helper) + expect(result[:devicegraph].partitions).to include( + an_object_having_attributes(filesystem_label: "windows", size: 360.GiB), + an_object_having_attributes(filesystem_label: "other", size: 110.GiB) + ) + end + + it "does not delete any partition" do + result = maker.provide_space(fake_devicegraph, volumes, lvm_helper) + expect(result[:devicegraph].partitions.map(&:name)).to contain_exactly( + "/dev/sda1", "/dev/sda2", "/dev/sda3", "/dev/sda4", "/dev/sda5" + ) + end + + it "suggests a distribution using the freed space" do + result = maker.provide_space(fake_devicegraph, volumes, lvm_helper) + distribution = result[:partitions_distribution] + expect(distribution.spaces.size).to eq 1 + expect(distribution.spaces.first.partitions).to eq volumes + end + end + context "and resizing all the allowed partitions is not enough" do let(:volumes) { [vol1, vol2, vol3] } @@ -335,6 +401,24 @@ def probed_partition(name) expect(distribution.spaces.size).to eq 3 expect(distribution.spaces.flat_map(&:partitions)).to contain_exactly(*volumes) end + + context "if resize operations are limited" do + let(:settings_actions) do + [ + resize.new("/dev/sda2", min_size: 150.GiB), + resize.new("/dev/sda3", min_size: 110.GiB), + delete.new("/dev/sda5"), delete.new("/dev/sda6") + ] + end + + it "resizes all allowed partitions their specified limits" do + result = maker.provide_space(fake_devicegraph, volumes, lvm_helper) + expect(result[:devicegraph].partitions).to include( + an_object_having_attributes(filesystem_label: "windows", size: 150.GiB), + an_object_having_attributes(filesystem_label: "other", size: 110.GiB) + ) + end + end end end end diff --git a/test/y2storage/proposal_agama_advanced_test.rb b/test/y2storage/proposal_agama_advanced_test.rb index 99c18bb04..80ee309e1 100755 --- a/test/y2storage/proposal_agama_advanced_test.rb +++ b/test/y2storage/proposal_agama_advanced_test.rb @@ -42,6 +42,9 @@ { "mount_point" => "swap", "fs_type" => "swap", "min_size" => "1 GiB", "max_size" => "2 GiB" } end + let(:delete) { Y2Storage::SpaceActions::Delete } + let(:resize) { Y2Storage::SpaceActions::Resize } + let(:scenario) { "mixed_disks" } before do @@ -70,7 +73,7 @@ before do settings.candidate_devices = ["/dev/sdb"] settings.root_device = "/dev/sda" - settings.space_settings.actions = { "/dev/sda1" => :resize, "/dev/sda2" => :resize } + settings.space_settings.actions = [resize.new("/dev/sda1"), resize.new("/dev/sda2")] allow(storage_arch).to receive(:efiboot?).and_return(true) end @@ -127,7 +130,7 @@ it "tries to use the formatted disk before trying an optional delete" do sda1_sid = fake_devicegraph.find_by_name("/dev/sda1").sid - settings.space_settings.actions = { "/dev/sda1" => :delete } + settings.space_settings.actions = [delete.new("/dev/sda1")] proposal.propose expect(proposal.failed?).to eq false @@ -141,7 +144,7 @@ it "tries to use the formatted disk before trying an optional resize" do orig_sda1 = fake_devicegraph.find_by_name("/dev/sda1") - settings.space_settings.actions = { "/dev/sda1" => :resize } + settings.space_settings.actions = [resize.new("/dev/sda1")] proposal.propose expect(proposal.failed?).to eq false diff --git a/test/y2storage/proposal_agama_basis_test.rb b/test/y2storage/proposal_agama_basis_test.rb index 2e29a352b..74ffcaad4 100755 --- a/test/y2storage/proposal_agama_basis_test.rb +++ b/test/y2storage/proposal_agama_basis_test.rb @@ -252,11 +252,11 @@ before do settings.space_settings.strategy = :bigger_resize - settings.space_settings.actions = { - "/dev/sda1" => :resize, - "/dev/sdb1" => :resize, - "/dev/sdc1" => :resize - } + settings.space_settings.actions = [ + Y2Storage::SpaceActions::Resize.new("/dev/sda1"), + Y2Storage::SpaceActions::Resize.new("/dev/sdb1"), + Y2Storage::SpaceActions::Resize.new("/dev/sdc1") + ] end include_examples "resize volume combinations" @@ -275,11 +275,11 @@ before do settings.space_settings.strategy = :bigger_resize - settings.space_settings.actions = { - "/dev/sda1" => :force_delete, - "/dev/sdb1" => :force_delete, - "/dev/sdc1" => :force_delete - } + settings.space_settings.actions = [ + Y2Storage::SpaceActions::Delete.new("/dev/sda1", mandatory: true), + Y2Storage::SpaceActions::Delete.new("/dev/sdb1", mandatory: true), + Y2Storage::SpaceActions::Delete.new("/dev/sdc1", mandatory: true) + ] end include_examples "delete volume combinations" diff --git a/test/y2storage/proposal_agama_reuse_test.rb b/test/y2storage/proposal_agama_reuse_test.rb index bc3e0745b..739157491 100755 --- a/test/y2storage/proposal_agama_reuse_test.rb +++ b/test/y2storage/proposal_agama_reuse_test.rb @@ -34,7 +34,7 @@ let(:settings_format) { :ng } let(:separate_home) { true } let(:control_file_content) { { "partitioning" => { "volumes" => volumes } } } - let(:space_actions) { {} } + let(:space_actions) { [] } let(:scenario) { "lvm-two-vgs" } let(:resize_info) do @@ -57,6 +57,9 @@ { "mount_point" => "swap", "fs_type" => "swap", "min_size" => "2 GiB", "max_size" => "6 GiB" } end + let(:delete) { Y2Storage::SpaceActions::Delete } + let(:resize) { Y2Storage::SpaceActions::Resize } + before do # Speed-up things by avoiding calls to hwinfo allow_any_instance_of(Y2Storage::Disk).to receive(:hwinfo).and_return(Y2Storage::HWInfoDisk.new) @@ -82,7 +85,9 @@ srv.reformat = reformat end - let(:space_actions) { { "/dev/sda1" => :delete, "/dev/sda2" => :delete, "/dev/sda8" => :delete } } + let(:space_actions) do + [delete.new("/dev/sda1"), delete.new("/dev/sda2"), delete.new("/dev/sda8")] + end let(:original_sda8) { fake_devicegraph.find_by_name("/dev/sda8") } context "keeping its filesystem" do @@ -129,7 +134,7 @@ srv.reformat = reformat end - let(:space_actions) { { "/dev/sda1" => :delete, "/dev/sda4" => :delete } } + let(:space_actions) { [delete.new("/dev/sda1"), delete.new("/dev/sda4")] } let(:original_sda4) { fake_devicegraph.find_by_name("/dev/sda4") } context "keeping its filesystem" do @@ -178,7 +183,9 @@ srv.reformat = reformat end - let(:space_actions) { { "/dev/sda1" => :delete, "/dev/sda2" => :delete, "/dev/sda5" => :delete } } + let(:space_actions) do + [delete.new("/dev/sda1"), delete.new("/dev/sda2"), delete.new("/dev/sda5")] + end let(:original_sda5) { fake_devicegraph.find_by_name("/dev/sda5") } context "keeping its filesystem" do @@ -316,7 +323,7 @@ srv.reuse_name = "/dev/md1" end - let(:space_actions) { { "/dev/sda2" => :delete, "/dev/sdb2" => :delete } } + let(:space_actions) { [delete.new("/dev/sda2"), delete.new("/dev/sdb2")] } let(:original_sda2) { fake_devicegraph.find_by_name("/dev/sda2") } let(:original_sdb2) { fake_devicegraph.find_by_name("/dev/sdb2") } @@ -345,7 +352,7 @@ srv.reuse_name = "/dev/md0" end - let(:space_actions) { { "/dev/sda1" => :delete, "/dev/sdb1" => :delete } } + let(:space_actions) { [delete.new("/dev/sda1"), delete.new("/dev/sdb1")] } let(:original_sda1) { fake_devicegraph.find_by_name("/dev/sda1") } let(:original_sdb1) { fake_devicegraph.find_by_name("/dev/sdb1") } @@ -383,7 +390,7 @@ srv.reformat = false end - let(:space_actions) { { "/dev/sda2" => :delete, "/dev/sdb2" => :delete } } + let(:space_actions) { [delete.new("/dev/sda2"), delete.new("/dev/sdb2")] } let(:original_sda2) { fake_devicegraph.find_by_name("/dev/sda2") } let(:original_sdb2) { fake_devicegraph.find_by_name("/dev/sdb2") } @@ -413,7 +420,7 @@ srv.reformat = false end - let(:space_actions) { { "/dev/sda2" => :delete, "/dev/sdb2" => :delete } } + let(:space_actions) { [delete.new("/dev/sda2"), delete.new("/dev/sdb2")] } let(:original_sda2) { fake_devicegraph.find_by_name("/dev/sda2") } let(:original_sdb2) { fake_devicegraph.find_by_name("/dev/sdb2") } @@ -477,10 +484,10 @@ end let(:space_actions) do - { - "/dev/sda1" => :delete, "/dev/sda2" => :delete, "/dev/sda3" => :delete, - "/dev/sdb1" => :delete, "/dev/sdb2" => :delete - } + [ + delete.new("/dev/sda1"), delete.new("/dev/sda2"), delete.new("/dev/sda3"), + delete.new("/dev/sdb1"), delete.new("/dev/sdb2") + ] end let(:original_sdb1) { fake_devicegraph.find_by_name("/dev/sdb1") }