Skip to content

Commit

Permalink
Merge d08ab34 into 055f581
Browse files Browse the repository at this point in the history
  • Loading branch information
imobachgs committed Mar 10, 2020
2 parents 055f581 + d08ab34 commit 683f352
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 4 deletions.
7 changes: 7 additions & 0 deletions package/yast2-storage-ng.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Mon Mar 9 09:53:50 UTC 2020 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

- AutoYaST: show an error when no suitable physical volumes are
found for a given volume group (bsc#1162043).
- 4.1.93

-------------------------------------------------------------------
Tue Mar 3 12:56:34 UTC 2020 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-storage-ng.spec
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#

Name: yast2-storage-ng
Version: 4.1.92
Version: 4.1.93
Release: 0

BuildRoot: %{_tmppath}/%{name}-%{version}-build
Expand Down
1 change: 1 addition & 0 deletions src/lib/y2storage/autoinst_issues.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ module AutoinstIssues
require "y2storage/autoinst_issues/no_disk_space"
require "y2storage/autoinst_issues/no_partitionable"
require "y2storage/autoinst_issues/no_proposal"
require "y2storage/autoinst_issues/no_pvs"
require "y2storage/autoinst_issues/shrinked_planned_devices"
require "y2storage/autoinst_issues/surplus_partitions"
require "y2storage/autoinst_issues/thin_pool_not_found"
Expand Down
54 changes: 54 additions & 0 deletions src/lib/y2storage/autoinst_issues/no_pvs.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright (c) [2020] 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
# No suitable physical volumes were found for this volume group
class NoPvs < Issue
attr_reader :planned_vg

# @param planned_vg [Planned::LvmVg] Planned volume group
def initialize(planned_vg)
textdomain "storage"

@planned_vg = planned_vg
end

# Fatal problem
#
# @return [Symbol] :fatal
# @see Issue#severity
def severity
:fatal
end

# Return the error message to be displayed
#
# @return [String] Error message
# @see Issue#message
def message
format(
_("Could not find a suitable physical volume for volume group '%{vg_name}'."),
vg_name: planned_vg.volume_group_name
)
end
end
end
end
2 changes: 1 addition & 1 deletion src/lib/y2storage/autoinst_proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def guided_devicegraph_for_target(devicegraph, drives, target)
# @return [Devicegraph] Copy of devicegraph containing the planned devices
def create_devices(devicegraph, planned_devices, disk_names)
boot_parts = boot_devices(devicegraph, @planned_devices)
devices_creator = Proposal::AutoinstDevicesCreator.new(devicegraph)
devices_creator = Proposal::AutoinstDevicesCreator.new(devicegraph, issues_list)
begin
planned_with_boot = planned_devices.prepend(boot_parts)
result = devices_creator.populated_devicegraph(planned_with_boot, disk_names)
Expand Down
12 changes: 11 additions & 1 deletion src/lib/y2storage/proposal/autoinst_devices_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,16 @@ module Proposal
class AutoinstDevicesCreator
include Yast::Logger

# @return [AutoinstIssues::List] List of found AutoYaST issues
attr_reader :issues_list

# Constructor
#
# @param original_graph [Devicegraph] Devicegraph to be used as starting point
def initialize(original_graph)
# @param issues_list [AutoinstIssues::List] List of AutoYaST issues to register them
def initialize(original_graph, issues_list)
@original_graph = original_graph
@issues_list = issues_list
end

# Devicegraph including all the specified planned devices
Expand Down Expand Up @@ -358,6 +363,11 @@ def set_up_lvm(vgs, devs_to_reuse)
vgs.reduce(creator_result) do |result, vg|
pvs = creator_result.created_names { |d| d.pv_for?(vg.volume_group_name) }
pvs += devs_to_reuse.select { |d| d.pv_for?(vg.volume_group_name) }.map(&:reuse_name)
if pvs.empty?
issues_list.add(:no_pvs, vg)
next result
end

result.merge(create_logical_volumes(result.devicegraph, vg, pvs))
end
end
Expand Down
38 changes: 38 additions & 0 deletions test/y2storage/autoinst_issues/no_pvs_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Copyright (c) [2020] 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"

describe Y2Storage::AutoinstIssues::NoPvs do
subject(:issue) { described_class.new(vg0) }

let(:vg0) { planned_vg(volume_group_name: "vg0") }

describe "#message" do
it "returns a description of the issue" do
expect(issue.message).to include("Could not find a suitable physical volume")
end
end

describe "#severity" do
it "returns :fatal" do
expect(issue.severity).to eq(:fatal)
end
end
end
16 changes: 16 additions & 0 deletions test/y2storage/autoinst_proposal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,22 @@
end
end

context "when the volume group name does not match" do
let(:lvm_pv) do
{ "create" => true, "lvm_group" => "another", "size" => "max" }
end

let(:lvs) { [root_spec] }

it "does not crash" do
proposal.propose
issues = issues_list.select do |i|
i.is_a?(Y2Storage::AutoinstIssues::NoPvs)
end
expect(issues.size).to eq(1)
end
end

context "when using a thin pool" do
let(:root_spec) do
{
Expand Down
14 changes: 13 additions & 1 deletion test/y2storage/proposal/autoinst_devices_creator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,12 @@

let(:planned_devices) { Y2Storage::Planned::DevicesCollection.new([disk]) }

let(:issues_list) { Y2Storage::AutoinstIssues::List.new }

before { fake_scenario(scenario) }

subject(:creator) do
described_class.new(Y2Storage::StorageManager.instance.probed)
described_class.new(Y2Storage::StorageManager.instance.probed, issues_list)
end

describe "#populated_devicegraph" do
Expand Down Expand Up @@ -245,6 +247,16 @@
expect(lvm_vgs.size).to eq(2)
end
end

context "when no suitable pvs are found" do
let(:planned_devices) { Y2Storage::Planned::DevicesCollection.new([disk, vg]) }
let(:vg) { planned_vg(volume_group_name: "vg1", lvs: [lv_root]) }

it "registers an issue" do
expect(issues_list).to receive(:add).with(:no_pvs, vg)
creator.populated_devicegraph(planned_devices, ["/dev/sda"])
end
end
end

describe "using RAID" do
Expand Down

0 comments on commit 683f352

Please sign in to comment.