Skip to content

Commit

Permalink
refactor(storage): make proposal and planner code more uniform
Browse files Browse the repository at this point in the history
- Use the same parameters in constructors.
- Use config param instead of settings.
  • Loading branch information
joseivanlopez committed Sep 12, 2024
1 parent e4c8cda commit 979615d
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 72 deletions.
32 changes: 16 additions & 16 deletions service/lib/y2storage/agama_proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,24 +76,24 @@ class AgamaProposal < Proposal::Base
include Proposal::PlannedDevicesHandler

# @return [Agama::Storage::Config]
attr_reader :settings
attr_reader :config

# @return [Array<Agama::Issue>] List of found issues
attr_reader :issues_list

# Constructor
#
# @param initial_settings [Agama::Storage::Config] Agama storage settings
# @param initial_config [Agama::Storage::Config] Agama storage config
# @param devicegraph [Devicegraph] starting point. If nil, then probed devicegraph
# will be used
# @param disk_analyzer [DiskAnalyzer] by default, the method will create a new one
# based on the initial devicegraph or will use the one from the StorageManager if
# starting from probed (i.e. 'devicegraph' argument is also missing)
# @param issues_list [Array<Agama::Issue] Array to register issues found during the process
def initialize(initial_settings, devicegraph: nil, disk_analyzer: nil, issues_list: nil)
def initialize(initial_config, devicegraph: nil, disk_analyzer: nil, issues_list: nil)
super(devicegraph: devicegraph, disk_analyzer: disk_analyzer)
@issues_list = issues_list || []
@settings = initial_settings
@config = initial_config
end

private
Expand All @@ -112,20 +112,20 @@ def fatal_error?
#
# @raise [NoDiskSpaceError] if there is no enough space to perform the installation
def calculate_proposal
# TODO: Could the search be moved to the devices planner? If so, the settings object might
# keep untouched, directly generating planned devices associated to the found device and
# skipping planned devices for searches with skip if not found.
# TODO: Could the search be moved to the devices planner? If so, the config object can keep
# untouched, directly generating planned devices associated to the found device and skipping
# planned devices for searches with skip if not found.
Proposal::AgamaSearcher
.new(initial_devicegraph)
.search(settings, issues_list)
.search(config, issues_list)

if fatal_error?
# This means some IfNotFound is set to "error" and we failed to find a match
@devices = nil
return @devices
end

@space_maker = Proposal::AgamaSpaceMaker.new(disk_analyzer, settings)
@space_maker = Proposal::AgamaSpaceMaker.new(disk_analyzer, config)
@devices = propose_devicegraph
end

Expand All @@ -152,8 +152,8 @@ def propose_devicegraph
#
# @return [Planned::DevicesCollection]
def calculate_initial_planned(devicegraph)
planner = Proposal::AgamaDevicesPlanner.new(settings, issues_list)
@planned_devices = planner.initial_planned_devices(devicegraph)
planner = Proposal::AgamaDevicesPlanner.new(devicegraph, issues_list)
@planned_devices = planner.planned_devices(config)
end

# Performs the mandatory space-making actions on the given devicegraph
Expand All @@ -171,7 +171,7 @@ def clean_graph(devicegraph)
#
# @param devicegraph [Devicegraph] the graph gets modified
def configure_ptable_types(devicegraph)
configured = settings.drives.select(&:ptable_type)
configured = config.drives.select(&:ptable_type)
configured.each do |drive|
dev = device_for(drive, devicegraph)
next unless dev
Expand All @@ -185,7 +185,7 @@ def configure_ptable_types(devicegraph)
#
# @param devicegraph [Devicegraph]
def complete_planned(devicegraph)
if settings.boot.configure?
if config.boot.configure?
@planned_devices = planned_devices.prepend(boot_partitions(devicegraph))
end

Expand All @@ -197,7 +197,7 @@ def boot_partitions(devicegraph)
checker = BootRequirementsChecker.new(
devicegraph,
planned_devices: planned_devices.mountable_devices,
boot_disk_name: settings.boot_device
boot_disk_name: config.boot_device
)
# NOTE: Should we try with :desired first?
checker.needed_partitions(:min)
Expand All @@ -220,7 +220,7 @@ def remove_empty_partition_tables(devicegraph)
# @param devicegraph [Y2Storage::Devicegraph]
# @return [Array<Y2Storage::BlkDevice>]
def drives_with_empty_partition_table(devicegraph)
devices = settings.drives.map { |d| device_for(d, devicegraph) }.compact
devices = config.drives.map { |d| device_for(d, devicegraph) }.compact
devices.select { |d| d.partition_table && d.partitions.empty? }
end

Expand All @@ -244,7 +244,7 @@ def protect_sids
# @param devicegraph [Devicegraph] the graph gets modified
def create_devices(devicegraph)
devices_creator = Proposal::AgamaDevicesCreator.new(devicegraph, issues_list)
names = settings.drives.map(&:found_device).compact.map(&:name)
names = config.drives.map(&:found_device).compact.map(&:name)
protect_sids
result = devices_creator.populated_devicegraph(planned_devices, names, space_maker)
end
Expand Down
77 changes: 39 additions & 38 deletions service/lib/y2storage/proposal/agama_device_planner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

require "y2storage/planned"
require "agama/issue"
require "yast/i18n"
require "y2storage/planned"

module Y2Storage
module Proposal
Expand Down Expand Up @@ -49,10 +50,10 @@ def initialize(devicegraph, issues_list)
@issues_list = issues_list
end

# Planned devices according to the given settings.
# Planned devices according to the given config.
#
# @return [Array] Array of planned devices.
def planned_devices(_setting)
def planned_devices(_config)
raise NotImplementedError
end

Expand Down Expand Up @@ -81,50 +82,50 @@ def reformat?(device, config)
end

# @param planned [Planned::Disk, Planned::Partition]
# @param settings [#encryption, #filesystem]
def configure_device(planned, settings)
configure_encryption(planned, settings.encryption) if settings.encryption
configure_filesystem(planned, settings.filesystem) if settings.filesystem
# @param config [#encryption, #filesystem]
def configure_block_device(planned, config)
configure_encryption(planned, config.encryption) if config.encryption
configure_filesystem(planned, config.filesystem) if config.filesystem
end

# @param planned [Planned::Disk, Planned::Partition]
# @param settings [Agama::Storage::Configs::Filesystem]
def configure_filesystem(planned, settings)
planned.mount_point = settings.path
planned.mount_by = settings.mount_by
planned.fstab_options = settings.mount_options
planned.mkfs_options = settings.mkfs_options.join(",")
planned.label = settings.label
configure_filesystem_type(planned, settings.type) if settings.type
# @param config [Agama::Storage::Configs::Filesystem]
def configure_filesystem(planned, config)
planned.mount_point = config.path
planned.mount_by = config.mount_by
planned.fstab_options = config.mount_options
planned.mkfs_options = config.mkfs_options.join(",")
planned.label = config.label
configure_filesystem_type(planned, config.type) if config.type
end

# @param planned [Planned::Disk, Planned::Partition]
# @param settings [Agama::Storage::Configs::FilesystemType]
def configure_filesystem_type(planned, settings)
planned.filesystem_type = settings.fs_type
configure_btrfs(planned, settings.btrfs) if settings.btrfs
# @param config [Agama::Storage::Configs::FilesystemType]
def configure_filesystem_type(planned, config)
planned.filesystem_type = config.fs_type
configure_btrfs(planned, config.btrfs) if config.btrfs
end

# @param planned [Planned::Disk, Planned::Partition]
# @param settings [Agama::Storage::Configs::Btrfs]
def configure_btrfs(planned, settings)
# @param config [Agama::Storage::Configs::Btrfs]
def configure_btrfs(planned, config)
# TODO: we need to discuss what to do with transactional systems and the read_only
# property. We are not sure whether those things should be configurable by the user.
# planned.read_only = settings.read_only?
planned.snapshots = settings.snapshots?
planned.default_subvolume = settings.default_subvolume
planned.subvolumes = settings.subvolumes
# planned.read_only = config.read_only?
planned.snapshots = config.snapshots?
planned.default_subvolume = config.default_subvolume
planned.subvolumes = config.subvolumes
end

# @param planned [Planned::Disk, Planned::Partition]
# @param settings [Agama::Storage::Configs::Encryption]
def configure_encryption(planned, settings)
planned.encryption_password = settings.password
planned.encryption_method = settings.method
planned.encryption_pbkdf = settings.pbkd_function
planned.encryption_label = settings.label
planned.encryption_cipher = settings.cipher
planned.encryption_key_size = settings.key_size
# @param config [Agama::Storage::Configs::Encryption]
def configure_encryption(planned, config)
planned.encryption_password = config.password
planned.encryption_method = config.method
planned.encryption_pbkdf = config.pbkd_function
planned.encryption_label = config.label
planned.encryption_cipher = config.cipher
planned.encryption_key_size = config.key_size

check_encryption(planned)
end
Expand Down Expand Up @@ -191,10 +192,10 @@ def encryption_issue(message)
end

# @param planned [Planned::Partition]
# @param settings [Agama::Storage::Configs::Size]
def configure_size(planned, settings)
planned.min_size = settings.min
planned.max_size = settings.max
# @param config [Agama::Storage::Configs::Size]
def configure_size(planned, config)
planned.min_size = config.min
planned.max_size = config.max
planned.weight = 100
end

Expand All @@ -216,7 +217,7 @@ def planned_partition(config)
Planned::Partition.new(nil, nil).tap do |planned|
planned.partition_id = config.id
configure_reuse(planned, config)
configure_device(planned, config)
configure_block_device(planned, config)
configure_size(planned, config.size)
end
end
Expand Down
34 changes: 17 additions & 17 deletions service/lib/y2storage/proposal/agama_devices_planner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,10 @@ module Proposal
class AgamaDevicesPlanner
include Yast::Logger

# Settings used to calculate the planned devices.
#
# @return [Agama::Storage::Config]
attr_reader :settings

# @param settings [Agama::Storage::Config]
# @param devicegraph [Devicegraph]
# @param issues_list [Array<Agama::Issue>]
def initialize(settings, issues_list)
@settings = settings
def initialize(devicegraph, issues_list)
@devicegraph = devicegraph
@issues_list = issues_list
end

Expand All @@ -46,28 +41,33 @@ def initialize(settings, issues_list)
# For the time being, this implements only stuff coming from partitition elements within
# drive elements.
#
# @param devicegraph [Devicegraph]
# @param config [Agama::Storage::Config]
# @return [Planned::DevicesCollection]
def initial_planned_devices(devicegraph)
def planned_devices(config)
# In the future this will also include planned devices that are equivalent to
# those typically generated by the Guided Proposal. For those, note that:
# - For dedicated VGs it creates a Planned VG containing a Planned LV, but no PVs
# - For LVM volumes it create a Planned LV but associated to no planned VG
# - For partition volumes, it creates a planned partition, of course

devs = settings.drives.flat_map { |d| planned_for_drive(d, devicegraph) }.compact
Planned::DevicesCollection.new(devs)
planned = planned_for_drives(config)
Planned::DevicesCollection.new(planned)
end

protected

# @return [Y2Storage::Devicegraph]
attr_reader :devicegraph

# @return [Array<Agama::Issue>] List to register any found issue
attr_reader :issues_list

# @see #initial_planned_devices
def planned_for_drive(drive, devicegraph)
planner = AgamaDrivePlanner.new(devicegraph, issues_list)
planner.planned_devices(drive)
# @param config [Agama::Storage::Config]
# @return [Array<Planned::Device>]
def planned_for_drives(config)
config.drives.flat_map do |drive|
planner = AgamaDrivePlanner.new(devicegraph, issues_list)
planner.planned_devices(drive)
end
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion service/lib/y2storage/proposal/agama_drive_planner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

require "y2storage/planned/disk"
require "y2storage/proposal/agama_device_planner"

module Y2Storage
Expand Down Expand Up @@ -49,7 +50,7 @@ def planned_drive(settings)
def planned_full_drive(settings)
Planned::Disk.new.tap do |planned|
configure_reuse(planned, settings)
configure_device(planned, settings)
configure_block_device(planned, settings)
end
end

Expand Down

0 comments on commit 979615d

Please sign in to comment.