From 51e01233ac11fae04b105757b6b29e9bd2e710b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 21 Nov 2017 17:17:47 +0000 Subject: [PATCH] Add warning messages with resizing is not possible --- src/lib/y2storage/autoinst_issues.rb | 1 + .../autoinst_issues/invalid_value.rb | 48 ++++++++++++--- .../autoinst_issues/not_resizable.rb | 61 +++++++++++++++++++ .../proposal/autoinst_devices_planner.rb | 20 ++++-- .../autoinst_issues/invalid_value_test.rb | 34 ++++++++++- .../autoinst_issues/not_resizable_test.rb | 46 ++++++++++++++ test/y2storage/autoinst_proposal_test.rb | 49 +++++++++++++-- .../proposal/autoinst_devices_planner_test.rb | 11 ++++ 8 files changed, 248 insertions(+), 22 deletions(-) create mode 100644 src/lib/y2storage/autoinst_issues/not_resizable.rb create mode 100644 test/y2storage/autoinst_issues/not_resizable_test.rb diff --git a/src/lib/y2storage/autoinst_issues.rb b/src/lib/y2storage/autoinst_issues.rb index 636ea084ff..3b3f6615b9 100644 --- a/src/lib/y2storage/autoinst_issues.rb +++ b/src/lib/y2storage/autoinst_issues.rb @@ -42,5 +42,6 @@ module AutoinstIssues require "y2storage/autoinst_issues/exception" require "y2storage/autoinst_issues/no_disk_space" require "y2storage/autoinst_issues/no_disk" +require "y2storage/autoinst_issues/not_resizable" require "y2storage/autoinst_issues/missing_reusable_device" require "y2storage/autoinst_issues/missing_reuse_info" diff --git a/src/lib/y2storage/autoinst_issues/invalid_value.rb b/src/lib/y2storage/autoinst_issues/invalid_value.rb index 2ed50e473b..bfc55491b1 100644 --- a/src/lib/y2storage/autoinst_issues/invalid_value.rb +++ b/src/lib/y2storage/autoinst_issues/invalid_value.rb @@ -33,16 +33,22 @@ module AutoinstIssues class InvalidValue < Issue # @return [Symbol] Name of the missing attribute attr_reader :attr - # @return [Object] New value or :skip to skip the section. + # @return [Integer,String,Symbol] New value or :skip to skip the section attr_reader :new_value + # @return [Array,Range] Expected values + attr_reader :expected_values - # @param section [#parent,#section_name] Section where it was detected (see {AutoinstProfile}) - # @param attr [Symbol] Name of the invalid attribute - # @param new_value [Integer,String,Symbol] New value or :skip to skip the section - def initialize(section, attr, new_value = :skip) + # @param section [#parent,#section_name] Section where it was detected + # (see {AutoinstProfile}) + # @param attr [Symbol] Name of the invalid attribute + # @param new_value [Integer,String,Symbol] New value or :skip to skip the section + # @param expected_values [Array,Range,nil] Expected values + attr_reader :expected_values + def initialize(section, attr, new_value = :skip, expected_values = nil) @section = section @attr = attr @new_value = new_value + @expected_values = expected_values end # Return the invalid value @@ -65,9 +71,11 @@ def severity # @see Issue#message def message # TRANSLATORS: 'value' is a generic value (number or string) 'attr' is an AutoYaST element - # name; 'new_value_message' is a short explanation about what should be done with the value. - _("Invalid value '%{value}' for attribute '%{attr}' (%{new_value_message}).") % - { value: value, attr: attr, new_value_message: new_value_message } + # name. + message = _("Invalid value '%{value}' for attribute '%{attr}'. ") % + { value: value, attr: attr } + message += expected_values_message if expected_values + message + new_value_message end private @@ -76,10 +84,30 @@ def message def new_value_message if new_value == :skip # TRANSLATORS: it refers to an AutoYaST profile section - _("the section will be skipped") + _("The section will be skipped.") else # TRANSLATORS: 'value' is the value for an AutoYaST element (a number or a string) - _("replaced by '%{value}'") % { value: new_value } + _("Replaced by '%{value}'.") % { value: human_value(new_value) } + end + end + + # Return a messsage explaining expected values + def expected_values_message + case expected_values + when Range + _("Expected a value between '%{min}' and '%{max}'. ") % + { min: human_value(expected_values.first), max: human_value(expected_values.last) } + when Array + _("Expected one of these values: %{values}. ") % + { values: expected_values.join(", ") } + end + end + + def human_value(value) + if value.respond_to?(:to_human_string) + value.to_human_string + else + value.to_s end end end diff --git a/src/lib/y2storage/autoinst_issues/not_resizable.rb b/src/lib/y2storage/autoinst_issues/not_resizable.rb new file mode 100644 index 0000000000..74e257165c --- /dev/null +++ b/src/lib/y2storage/autoinst_issues/not_resizable.rb @@ -0,0 +1,61 @@ +# encoding: utf-8 + +# Copyright (c) [2017] 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/autoinst_issues/issue" + +module Y2Storage + module AutoinstIssues + # Represents an AutoYaST situation where an invalid value was given. + # + # @example Invalid value 'auto' for attribute :size on /home partition + # section = AutoinstProfile::PartitioningSection.new_from_hashes({"size" => "auto"}) + # problem = InvalidValue.new(section, :size) + # problem.value #=> "auto" + # problem.attr #=> :size + class NotResizable < Issue + # @return [String] Name of the device that cannot be resized + attr_reader :device + + # @param section [#parent,#section_name] Section where it was detected (see {AutoinstProfile}) + # @param device [String] Name of the device that cannot be resized + def initialize(section, device) + @section = section + @device = device + end + + # Return problem severity + # + # @return [Symbol] :warn + def severity + :warn + end + + # Return the error message to be displayed + # + # @return [String] Error message + # @see Issue#message + def message + # TRANSLATORS: kernel device name (eg. '/dev/sda1') + _("Device '%s' cannot be resized.") % device + end + end + end +end diff --git a/src/lib/y2storage/proposal/autoinst_devices_planner.rb b/src/lib/y2storage/proposal/autoinst_devices_planner.rb index 0da6e1d75c..77b96f2277 100644 --- a/src/lib/y2storage/proposal/autoinst_devices_planner.rb +++ b/src/lib/y2storage/proposal/autoinst_devices_planner.rb @@ -244,7 +244,7 @@ def add_partition_reuse(partition, section) return unless partition_to_reuse partition.filesystem_type ||= partition_to_reuse.filesystem_type add_device_reuse(partition, partition_to_reuse.name, !!section.format) - resize_device(partition, partition_to_reuse) if section.resize + resize_device(partition, partition_to_reuse, section) if section.resize end # Set 'reusing' attributes for a logical volume @@ -254,20 +254,27 @@ def add_partition_reuse(partition, section) # # @param lv [Planned::LvmLv] Planned logical volume # @param vg_name [String] Volume group name to search for the logical volume to reuse - # @param section [AutoinstProfile::PartitionSection] AutoYaST specification + # @param section [AutoinstProfile::PartitionSection] AutoYaST specification def add_lv_reuse(lv, vg_name, section) lv_to_reuse = find_lv_to_reuse(devicegraph, vg_name, section) return unless lv_to_reuse lv.logical_volume_name ||= lv_to_reuse.lv_name lv.filesystem_type ||= lv_to_reuse.filesystem_type add_device_reuse(lv, lv_to_reuse.name, !!section.format) - resize_device(lv, lv_to_reuse) if section.resize + resize_device(lv, lv_to_reuse, section) if section.resize end # @param lv [Planned::LvmLv] Planned logical volume # @param device [BlkDevice] Device - def resize_device(planned, device) + # @param section [AutoinstProfile::PartitionSection] AutoYaST specification + def resize_device(planned, device, section) resize_info = device.detect_resize_info + + unless resize_info.resize_ok? + issues_list.add(:not_resizable, section, device.name) + return + end + planned.size = if planned.max_size <= resize_info.min_size resize_info.min_size @@ -276,6 +283,11 @@ def resize_device(planned, device) else planned.max_size end + + if planned.max_size != planned.size + values = (resize_info.min_size..resize_info.max_size) + issues_list.add(:invalid_value, section, :size, planned.size, values) + end end def add_device_reuse(device, name, format) diff --git a/test/y2storage/autoinst_issues/invalid_value_test.rb b/test/y2storage/autoinst_issues/invalid_value_test.rb index 85c4e241f1..9a4043ee08 100644 --- a/test/y2storage/autoinst_issues/invalid_value_test.rb +++ b/test/y2storage/autoinst_issues/invalid_value_test.rb @@ -25,6 +25,8 @@ require "y2storage/autoinst_profile/partition_section" describe Y2Storage::AutoinstIssues::InvalidValue do + using Y2Storage::Refinements::SizeCasts + subject(:issue) { described_class.new(section, :size) } let(:section) do @@ -40,7 +42,7 @@ context "when no new value was given" do it "includes a warning about the section being skipped" do - expect(issue.message).to include "the section will be skipped" + expect(issue.message).to include "The section will be skipped" end end @@ -48,7 +50,7 @@ subject(:issue) { described_class.new(section, :size, "some-value") } it "includes a warning about the section being skipped" do - expect(issue.message).to include "replaced by 'some-value'" + expect(issue.message).to include "Replaced by 'some-value'" end end @@ -56,7 +58,33 @@ subject(:issue) { described_class.new(section, :size, :skip) } it "includes a warning about the section being skipped" do - expect(issue.message).to include "the section will be skipped" + expect(issue.message).to include "The section will be skipped" + end + end + + context "when expected values are specified" do + context "as a range" do + subject(:issue) { described_class.new(section, :size, :skip, (0..10)) } + + it "includes a warning about the section being skipped" do + expect(issue.message).to include "Expected a value between '0' and '10'." + end + end + + context "as an array" do + subject(:issue) { described_class.new(section, :size, :skip, [0, 1, 2]) } + + it "includes a warning about the section being skipped" do + expect(issue.message).to include "Expected one of these values: 0, 1, 2." + end + end + + context "and values has an special human representation" do + subject(:issue) { described_class.new(section, :size, :skip, (450.KiB..1.GiB)) } + + it "includes a warning about the section being skipped" do + expect(issue.message).to include "Expected a value between '450.00 KiB' and '1.00 GiB'." + end end end end diff --git a/test/y2storage/autoinst_issues/not_resizable_test.rb b/test/y2storage/autoinst_issues/not_resizable_test.rb new file mode 100644 index 0000000000..e26a457120 --- /dev/null +++ b/test/y2storage/autoinst_issues/not_resizable_test.rb @@ -0,0 +1,46 @@ +#!/usr/bin/env rspec +# encoding: utf-8 + +# Copyright (c) [2017] 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 "../../spec_helper" +require "y2storage/autoinst_issues/not_resizable" + +describe Y2Storage::AutoinstIssues::NotResizable do + subject(:issue) { described_class.new(section, "/dev/sda1") } + + let(:section) do + instance_double(Y2Storage::AutoinstProfile::PartitionSection) + end + + let(:device_name) { nil } + + describe "#message" do + it "returns a description including the device name" do + expect(issue.message).to eq("Device '/dev/sda1' cannot be resized.") + end + end + + describe "#severity" do + it "returns :warn" do + expect(issue.severity).to eq(:warn) + end + end +end diff --git a/test/y2storage/autoinst_proposal_test.rb b/test/y2storage/autoinst_proposal_test.rb index 9d7a0f6f61..ebcd80af02 100644 --- a/test/y2storage/autoinst_proposal_test.rb +++ b/test/y2storage/autoinst_proposal_test.rb @@ -193,13 +193,15 @@ end let(:size) { "100GiB" } - + let(:resize_ok) { true } let(:resize_info) do - instance_double(Y2Storage::ResizeInfo, min_size: 512.MiB, max_size: 2.GiB) + instance_double( + Y2Storage::ResizeInfo, min_size: 512.MiB, max_size: 245.GiB, resize_ok?: resize_ok + ) end before do - allow_any_instance_of(Y2Storage::LvmLv).to receive(:detect_resize_info) + allow_any_instance_of(Y2Storage::Partition).to receive(:detect_resize_info) .and_return(resize_info) end @@ -211,7 +213,7 @@ end context "when requested size smaller than the minimal resize limit" do - let(:size) { "0.5GiB" } + let(:size) { "256MB" } it "sets the size to the minimal allowed size" do proposal.propose @@ -219,6 +221,17 @@ reused_part = devicegraph.partitions.find { |p| p.name == "/dev/sda3" } expect(reused_part.size).to eq(resize_info.min_size) end + + it "registers an issue" do + expect(proposal.issues_list).to be_empty + proposal.propose + issue = proposal.issues_list.find do |i| + i.is_a?(Y2Storage::AutoinstIssues::InvalidValue) + end + expect(issue.attr).to eq(:size) + expect(issue.new_value).to eq(resize_info.min_size) + expect(issue.expected_values).to eq((resize_info.min_size..resize_info.max_size)) + end end context "when requested size smaller than the maximal resize limit" do @@ -230,6 +243,30 @@ reused_part = devicegraph.partitions.find { |p| p.name == "/dev/sda3" } expect(reused_part.size).to eq(resize_info.max_size) end + + it "registers an issue" do + expect(proposal.issues_list).to be_empty + proposal.propose + issue = proposal.issues_list.find do |i| + i.is_a?(Y2Storage::AutoinstIssues::InvalidValue) + end + expect(issue.attr).to eq(:size) + expect(issue.new_value).to eq(resize_info.max_size) + expect(issue.expected_values).to eq((resize_info.min_size..resize_info.max_size)) + end + end + + context "when device cannot be resized" do + let(:resize_ok) { false } + + it "registers an issue" do + expect(proposal.issues_list).to be_empty + proposal.propose + issue = proposal.issues_list.find do |i| + i.is_a?(Y2Storage::AutoinstIssues::NotResizable) + end + expect(issue).to_not be_nil + end end end @@ -361,7 +398,9 @@ let(:size) { "1GiB" } let(:resize_info) do - instance_double(Y2Storage::ResizeInfo, min_size: 512.MiB, max_size: 2.GiB) + instance_double( + Y2Storage::ResizeInfo, min_size: 512.MiB, max_size: 2.GiB, resize_ok?: true + ) end before do diff --git a/test/y2storage/proposal/autoinst_devices_planner_test.rb b/test/y2storage/proposal/autoinst_devices_planner_test.rb index d5a812d6ec..1717e6f11a 100644 --- a/test/y2storage/proposal/autoinst_devices_planner_test.rb +++ b/test/y2storage/proposal/autoinst_devices_planner_test.rb @@ -136,6 +136,17 @@ "resize" => true, "size" => "5GB" } end + let(:resize_info) do + instance_double( + Y2Storage::ResizeInfo, min_size: 512.MiB, max_size: 10.GiB, resize_ok?: true + ) + end + + before do + allow_any_instance_of(Y2Storage::Partition).to receive(:detect_resize_info) + .and_return(resize_info) + end + it "reuses the partition with that number" do devices = planner.planned_devices(drives_map) root = devices.find { |d| d.mount_point == "/" }