Skip to content

Commit

Permalink
Storage: Do not register issues when skipping entries at the config (#…
Browse files Browse the repository at this point in the history
…1696)

## Problem

At the storage configuration, it's possible to specify a section
describing a device (drive, partition, volume group, logical volume,
etc.) with a `search` entry to indicate such a description corresponds
to one or several previously existing device(s) that should be found in
the system.

That `search` entry can contain `ifNotFound: "skip"` to indicate the
description should be ignored if the corresponding devices are not
found.

If that happens, a warning is added to the list of found issues with a
message like "_No device found for an optional
drive/partition/whatever_".

That warning looks overkill. Especially having into account the default
value for `ifNotFound` is "error", which means profiles using "skip"
really mean it.

## Solution

Reduce the noise by not longer generating the issue for missing optional
devices.
  • Loading branch information
ancorgs authored Oct 25, 2024
2 parents 825ad0c + 811ecb9 commit d0e1ca2
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 51 deletions.
23 changes: 3 additions & 20 deletions service/lib/agama/storage/config_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module Storage
# Class for checking a storage config.
#
# TODO: Split in smaller checkers, for example: ConfigFilesystemChecker, etc.
class ConfigChecker # rubocop:disable Metrics/ClassLength
class ConfigChecker
include Yast::I18n

# @param config [Storage::Config]
Expand Down Expand Up @@ -81,15 +81,10 @@ def drive_issues(config)
# @return [Agama::Issue]
def search_issue(config)
return if !config.search || config.found_device
return if config.search.skip_device?

if config.is_a?(Agama::Storage::Configs::Drive)
if config.search.skip_device?
warning(_("No device found for an optional drive"))
else
error(_("No device found for a mandatory drive"))
end
elsif config.search.skip_device?
warning(_("No device found for an optional partition"))
error(_("No device found for a mandatory drive"))
else
error(_("No device found for a mandatory partition"))
end
Expand Down Expand Up @@ -470,18 +465,6 @@ def volume_builder
@volume_builder ||= VolumeTemplatesBuilder.new_from_config(product_config)
end

# Creates a warning issue.
#
# @param message [String]
# @return [Issue]
def warning(message)
Agama::Issue.new(
message,
source: Agama::Issue::Source::CONFIG,
severity: Agama::Issue::Severity::WARN
)
end

# Creates an error issue.
#
# @param message [String]
Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Thu Oct 24 14:44:35 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

- Storage: do not report issues when intentionally skipping entries
at the storage config (gh#agama-project/agama#1696).

-------------------------------------------------------------------
Thu Oct 24 13:07:50 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

Expand Down
20 changes: 5 additions & 15 deletions service/test/agama/storage/config_checker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

require_relative "./storage_helpers"
require "agama/config"
require "agama/storage/config_conversions/from_json"
require "agama/storage/config_conversions"
require "agama/storage/config_checker"
require "agama/storage/config_solver"
require "y2storage"
Expand Down Expand Up @@ -281,13 +281,8 @@
context "and the drive should be skipped" do
let(:if_not_found) { "skip" }

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

issue = issues.first
expect(issue.error?).to eq(false)
expect(issue.description).to eq("No device found for an optional drive")
it "does not include any issue" do
expect(subject.issues).to be_empty
end
end

Expand Down Expand Up @@ -374,13 +369,8 @@
context "and the partition should be skipped" do
let(:if_not_found) { "skip" }

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

issue = issues.first
expect(issue.error?).to eq(false)
expect(issue.description).to eq("No device found for an optional partition")
it "does not include any issue" do
expect(subject.issues).to be_empty
end
end

Expand Down
7 changes: 2 additions & 5 deletions service/test/y2storage/agama_proposal_search_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,9 @@
expect(disk.partitions.size).to eq 1
end

it "register a warning about non-existent partitions" do
it "does not include any issue about non-existent partitions" do
proposal.propose
expect(proposal.issues_list).to include an_object_having_attributes(
description: /optional partition/,
severity: Agama::Issue::Severity::WARN
)
expect(proposal.issues_list).to be_empty
end
end
end
Expand Down
16 changes: 5 additions & 11 deletions service/test/y2storage/agama_proposal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
require_relative "../agama/storage/storage_helpers"
require "agama/config"
require "agama/storage/config"
require "agama/storage/config_conversions/from_json"
require "agama/storage/config_conversions"
require "y2storage"
require "y2storage/agama_proposal"

Expand Down Expand Up @@ -484,12 +484,9 @@ def partition_config(name: nil, filesystem: nil, size: nil)
expect(proposal.failed?).to eq false
end

it "registers a non-critical issue" do
it "does not register any issue about missing disks" do
proposal.propose
expect(proposal.issues_list).to include an_object_having_attributes(
description: /optional drive/,
severity: Agama::Issue::Severity::WARN
)
expect(proposal.issues_list).to be_empty
end
end

Expand Down Expand Up @@ -576,12 +573,9 @@ def partition_config(name: nil, filesystem: nil, size: nil)
expect(proposal.failed?).to eq false
end

it "registers a non-critical issue" do
it "does not register any issue about missing partitions" do
proposal.propose
expect(proposal.issues_list).to include an_object_having_attributes(
description: /optional partition/,
severity: Agama::Issue::Severity::WARN
)
expect(proposal.issues_list).to be_empty
end
end

Expand Down

0 comments on commit d0e1ca2

Please sign in to comment.