Skip to content

Commit

Permalink
Several small fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
ancorgs committed Jul 25, 2023
1 parent 9b3a7e0 commit 86b9ef1
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def initialize(settings)
def convert # rubocop:disable Metrics/AbcSize
{
"BootDevice" => settings.boot_device.to_s,
"LVM" => settings.lvm,
"LVM" => settings.lvm.enabled,
"SystemVGDevices" => settings.lvm.system_vg_devices,
"EncryptionPassword" => settings.encryption.password,
"EncryptionMethod" => settings.encryption.method.id.to_s,
Expand Down
8 changes: 4 additions & 4 deletions service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ def convert
{
"MountPath" => volume.mount_path.to_s,
"MountOptions" => volume.mount_options,
"TargetDevice" => volume.target_device.to_s,
"TargetVG" => volume.target_vg.to_s,
"TargetDevice" => volume.device.to_s,
"TargetVG" => volume.separate_vg_name.to_s,
"FsType" => volume.fs_type&.to_human_string,
"MinSize" => volume.min_size&.to_i,
"MaxSize" => volume.max_size&.to_i,
Expand All @@ -58,8 +58,8 @@ def outline_conversion
outline = volume.outline
{
"Required" => outline.required?,
"FsTypes" => outline.fs_types.map(&:to_human_string),
"SupportAutoSize" => outline.support_auto_size?,
"FsTypes" => outline.filesystems.map(&:to_human_string),
"SupportAutoSize" => outline.adaptive_sizes?,
"SnapshotsConfigurable" => outline.snapshots_configurable?,
"SnapshotsAffectSizes" => outline.snapshots_affect_sizes?,
"SizeRelevantVolumes" => outline.size_relevant_volumes
Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/storage/btrfs_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class BtrfsSettings
#
# @return [Boolean]
attr_accessor :snapshots
alias_method :snapshtos?, :snapshots
alias_method :snapshots?, :snapshots

# @return [Boolean]
attr_accessor :read_only
Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/storage/proposal_settings_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def read
"volumes" => :volumes_reader
}.freeze

private_constant :CONFIG_READERS
private_constant :READERS

# @param settings [Agama::Storage::ProposalSettings]
# @param value [Boolean]
Expand Down
8 changes: 6 additions & 2 deletions service/lib/agama/storage/volume.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,12 @@ def initialize(mount_path)
# Whether it makes sense to have automatic size limits for the volume
#
# @return [Boolean]
def adaptive_sizes?
outlilne.snapshots_affect_sizes?(btrfs.snapshots?) || outline.size_relevant_volumes.any?
def auto_size_supported?
# At some point, this method contained logic for ignoring outline.snapshots_affect_sizes?
# when both btrfs.snapshots and outline.snapshots_configurable? were false. But we don't
# need to have such a senseless scenario into account (a product that contains size rules
# for the snapshots but does not allow to enable them).
outline.adaptive_sizes?
end
end
end
Expand Down
18 changes: 10 additions & 8 deletions service/lib/agama/storage/volume_outline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class VolumeOutline
def initialize
@filesystems = []
@base_min_size = Y2Storage::DiskSize.zero
@base_max_size = Y2Storage::DiskSize.Unlimited
@base_max_size = Y2Storage::DiskSize.unlimited
@size_relevant_volumes = []
@max_size_fallback_for = []
@min_size_fallback_for = []
Expand All @@ -89,22 +89,24 @@ def initialize
#
# @return [Array<String>]
def size_relevant_volumes
(max_size_fallbacks_for + min_size_fallbacks_for).sort.uniq
(max_size_fallback_for + min_size_fallback_for).sort.uniq
end

# Whether snapshots affect the automatic calculation of the size limits
#
# @param snapshots [Booelan] Whether snapshots is active
# @return [Boolean]
def snapshots_affect_sizes?(snapshots)
# FIXME: this should be a responsibility of the Proposal (since it's calculated by
# Proposal::DevicesPlanner)
return false unless snapshots || snapshots_configurable

def snapshots_affect_sizes?
return true if snapshots_size && !snapshots_size.zero?

snapshots_percentage && !snapshots_percentage.zero?
end

# Whether it makes sense to have automatic size limits for the volume
#
# @return [Boolean]
def adaptive_sizes?
size_relevant_volumes.any? || adjust_by_ram? || snapshots_affect_sizes?
end
end
end
end
2 changes: 1 addition & 1 deletion service/lib/agama/storage/volume_templates_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def for(path)
volume.filesystem = data[:filesystem]
volume.mount_options = data[:mount_options]

if data[:auto_size] && volume.adaptive_sizes?
if data[:auto_size] && volume.auto_size_supported?
volume.auto_size = true
else
volume.auto_size = false
Expand Down
35 changes: 12 additions & 23 deletions service/test/agama/dbus/storage/proposal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,27 @@
let(:logger) { Logger.new($stdout, level: :warn) }

let(:backend) do
instance_double(Agama::Storage::Proposal, calculated_settings: settings)
instance_double(Agama::Storage::Proposal, settings: settings)
end

let(:settings) { nil }

describe "#candidate_devices" do
describe "#boot_device" do
context "if a proposal has not been calculated yet" do
let(:settings) { nil }

it "returns an empty list" do
expect(subject.candidate_devices).to eq([])
it "returns an empty string" do
expect(subject.boot_device).to eq ""
end
end

context "if a proposal has been calculated" do
let(:settings) do
instance_double(Agama::Storage::ProposalSettings,
candidate_devices: ["/dev/vda", "/dev/vdb"])
Agama::Storage::ProposalSettings.new.tap { |s| s.boot_device = "/dev/vda" }
end

it "returns the candidate devices used by the proposal" do
expect(subject.candidate_devices).to contain_exactly("/dev/vda", "/dev/vdb")
expect(subject.boot_device).to eq "/dev/vda"
end
end
end
Expand All @@ -69,7 +68,7 @@

context "if a proposal has been calculated" do
let(:settings) do
instance_double(Agama::Storage::ProposalSettings, lvm: true)
Agama::Storage::ProposalSettings.new.tap { |s| s.lvm.enabled = true }
end

it "return whether LVM was used" do
Expand All @@ -89,7 +88,7 @@

context "if a proposal has been calculated" do
let(:settings) do
instance_double(Agama::Storage::ProposalSettings, encryption_password: "n0ts3cr3t")
Agama::Storage::ProposalSettings.new.tap { |s| s.encryption.password = "n0ts3cr3t" }
end

it "return the encryption password used by the proposal" do
Expand All @@ -113,27 +112,17 @@

context "if the calculated settings has volumes" do
let(:calculated_volumes) { [calculated_volume1, calculated_volume2] }

let(:calculated_volume1) do
Agama::Storage::Volume.new.tap do |volume|
volume.mount_point = "/test1"
end
end

let(:calculated_volume2) do
Agama::Storage::Volume.new.tap do |volume|
volume.mount_point = "/test2"
end
end
let(:calculated_volume1) { Agama::Storage::Volume.new("/test1") }
let(:calculated_volume2) { Agama::Storage::Volume.new("/test2") }

it "returns a list with a hash for each volume" do
expect(subject.volumes.size).to eq(2)
expect(subject.volumes).to all(be_a(Hash))

volume1, volume2 = subject.volumes

expect(volume1).to include("MountPoint" => "/test1")
expect(volume2).to include("MountPoint" => "/test2")
expect(volume1).to include("MountPath" => "/test1")
expect(volume2).to include("MountPath" => "/test2")
end
end
end
Expand Down

0 comments on commit 86b9ef1

Please sign in to comment.