Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
joseivanlopez committed Oct 8, 2024
1 parent cf82e99 commit 7ea8721
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 10 deletions.
62 changes: 61 additions & 1 deletion service/lib/agama/storage/config_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# find current contact information at www.suse.com.

require "agama/issue"
require "agama/storage/volume_templates_builder"
require "yast/i18n"
require "y2storage/mount_point"

Expand All @@ -30,9 +31,11 @@ class ConfigChecker
include Yast::I18n

# @param config [Storage::Config]
def initialize(config)
# @param product_config [Agama::Config]
def initialize(config, product_config)
textdomain "agama"
@config = config
@product_config = product_config || Agama::Config.new
end

# Issues detected in the config.
Expand All @@ -47,6 +50,9 @@ def issues
# @return [Storage::Config]
attr_reader :config

# @return [Agama::Config]
attr_reader :product_config

# Issues from drives.
#
# @return [Array<Issue>]
Expand All @@ -61,6 +67,7 @@ def drives_issues
def drive_issues(config)
[
search_issue(config),
filesystem_issues(config),
encryption_issues(config),
partitions_issues(config)
].flatten.compact
Expand All @@ -86,6 +93,48 @@ def search_issue(config)
end
end

def filesystem_issues(config)
[
missing_filesystem_issue(config),
invalid_filesystem_issue(config)
].compact
end

def missing_filesystem_issue(config)
filesystem = config.filesystem
return if filesystem.nil? || filesystem.reuse?

type = filesystem.type&.fs_type
return if type

error(
format(
# TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device
# (e.g., 'luks1', 'random_swap').
_("Missing file system type for '%s'"),
filesystem.path
)
)
end

def invalid_filesystem_issue(config)
type = config.filesystem&.type&.fs_type
return if type.nil? || config.filesystem.reuse?

path = config.filesystem.path
return if suitable_filesystem_types(path).include?(type)

error(
format(
# TRANSLATORS: 'crypt_method' is the identifier of the method to encrypt the device
# (e.g., 'luks1', 'random_swap').
_("The file system type '%{filesystem}' is not suitable for '%{path}'"),
filesystem: type.to_human_string,
path: path
)
)
end

# Issues related to encryption.
#
# @param config [Configs::Drive, Configs::Partition, Configs::LogicalVolume]
Expand Down Expand Up @@ -169,6 +218,7 @@ def partitions_issues(config)
def partition_issues(config)
[
search_issue(config),
filesystem_issues(config),
encryption_issues(config)
].flatten.compact
end
Expand Down Expand Up @@ -247,6 +297,7 @@ def logical_volumes_issues(config)
# @return [Array<Issue>]
def logical_volume_issues(lv_config, vg_config)
[
filesystem_issues(lv_config),
encryption_issues(lv_config),
missing_thin_pool_issue(lv_config, vg_config)
].compact.flatten
Expand Down Expand Up @@ -375,6 +426,15 @@ def valid_physical_volumes_encryption_method?(method)
valid_methods.include?(method)
end

def suitable_filesystem_types(path = nil)
volume_builder.for(path || "").outline.filesystems
end

# @return [VolumeTemplatesBuilder]
def volume_builder
@volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config)
end

# Creates a warning issue.
#
# @param message [String]
Expand Down
5 changes: 4 additions & 1 deletion service/lib/y2storage/agama_proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ def calculate_proposal
.new(initial_devicegraph, product_config)
.solve(config)

issues = Agama::Storage::ConfigChecker.new(config).issues
issues = Agama::Storage::ConfigChecker
.new(config, product_config)
.issues

issues_list.concat(issues)

if fatal_error?
Expand Down
139 changes: 134 additions & 5 deletions service/test/agama/storage/config_checker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@
require "agama/storage/config_checker"
require "agama/storage/config_solver"
require "y2storage"
require "y2storage/refinements"

using Y2Storage::Refinements::SizeCasts

shared_examples "encryption issues" do
let(:filesystem) { nil }
Expand Down Expand Up @@ -110,10 +107,91 @@
end
end

shared_examples "filesystem issues" do |filesystem_proc|
context "with invalid type" do
let(:filesystem) do
{
path: "/",
type: "vfat",
reuseIfPossible: reuse
}
end

context "and without reusing the filesystem" do
let(:reuse) { false }

it "includes the expected issue" do
issues = subject.issues
expect(issues.size).to eq(1)

issue = issues.first
expect(issue.error?).to eq(true)
expect(issue.description).to match("type 'FAT' is not suitable for '/'")
end
end

context "and reusing the filesystem" do
let(:reuse) { true }

it "does not include an issue" do
expect(subject.issues.size).to eq(0)
end
end
end

context "with valid type" do
let(:filesystem) do
{
path: "/",
type: "btrfs"
}
end

it "does not include an issue" do
expect(subject.issues.size).to eq(0)
end
end

context "without a filesystem type" do
let(:filesystem) do
{
path: "/",
reuseIfPossible: reuse
}
end

before do
# Explicitly remove the filesystem type. Otherwise the JSON conversion assigns a default type.
filesystem_proc.call(config).type = nil
end

context "and without reusing the filesystem" do
let(:reuse) { false }

it "includes the expected issue" do
issues = subject.issues
expect(issues.size).to eq(1)

issue = issues.first
expect(issue.error?).to eq(true)
expect(issue.description).to eq("Missing file system type for '/'")
end
end

context "and reusing the filesystem" do
let(:reuse) { true }

it "does not include an issue" do
expect(subject.issues.size).to eq(0)
end
end
end
end

describe Agama::Storage::ConfigChecker do
include Agama::RSpec::StorageHelpers

subject { described_class.new(config) }
subject { described_class.new(config, product_config) }

let(:config) do
Agama::Storage::ConfigConversions::FromJSON
Expand All @@ -123,6 +201,22 @@

let(:config_json) { nil }

let(:product_config) { Agama::Config.new(product_data) }

let(:product_data) do
{
"storage" => {
"volume_templates" => [
{
"mount_path" => "/",
"filesystem" => "btrfs",
"outline" => { "filesystems" => ["btrfs", "xfs"] }
}
]
}
}
end

before do
mock_storage(devicegraph: scenario)
# To speed-up the tests
Expand All @@ -135,7 +229,6 @@
before do
# Solves the config before checking.
devicegraph = Y2Storage::StorageManager.instance.probed
product_config = Agama::Config.new

Agama::Storage::ConfigSolver
.new(devicegraph, product_config)
Expand Down Expand Up @@ -214,6 +307,22 @@
include_examples "encryption issues"
end

context "if a drive has filesystem" do
let(:config_json) do
{
drives: [
{
filesystem: filesystem
}
]
}
end

filesystem_proc = proc { |c| c.drives.first.filesystem }

include_examples "filesystem issues", filesystem_proc
end

context "if a drive has partitions" do
let(:config_json) do
{
Expand Down Expand Up @@ -272,6 +381,16 @@
end
end

context "if a partition has filesystem" do
let(:partition) do
{ filesystem: filesystem }
end

filesystem_proc = proc { |c| c.drives.first.partitions.first.filesystem }

include_examples "filesystem issues", filesystem_proc
end

context "and a partition has encryption" do
let(:partition) do
{
Expand Down Expand Up @@ -301,6 +420,16 @@
}
end

context "if a logical volume has filesystem" do
let(:logical_volume) do
{ filesystem: filesystem }
end

filesystem_proc = proc { |c| c.volume_groups.first.logical_volumes.first.filesystem }

include_examples "filesystem issues", filesystem_proc
end

context "and a logical volume has encryption" do
let(:logical_volume) do
{
Expand Down
7 changes: 4 additions & 3 deletions service/test/y2storage/agama_proposal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def partition_config(name: nil, filesystem: nil, size: nil)
},
"outline" => {
"required" => true, "snapshots_configurable" => true,
"filesystems" => ["btrfs"],
"auto_size" => {
"base_min" => "5 GiB", "base_max" => "10 GiB",
"min_fallback_for" => ["/home"], "max_fallback_for" => ["/home"],
Expand All @@ -123,14 +124,14 @@ def partition_config(name: nil, filesystem: nil, size: nil)
},
{
"mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" },
"filesystem" => "xfs", "outline" => { "required" => false }
"filesystem" => "xfs", "outline" => { "required" => false, "filesystems" => ["xfs", "ext4"] }
},
{
"mount_path" => "swap", "filesystem" => "swap",
"outline" => { "required" => false }
"outline" => { "required" => false, "filesystems" => ["swap"] }
},
{ "mount_path" => "", "filesystem" => "ext4",
"size" => { "min" => "100 MiB" } }
"size" => { "min" => "100 MiB" }, "outline" => { "filesystems" => ["ext4"] } }
]
}
}
Expand Down

0 comments on commit 7ea8721

Please sign in to comment.