Skip to content

Commit

Permalink
AgamaProposal: fix bug when partitions are searched but not managed (#…
Browse files Browse the repository at this point in the history
…1767)

## Problem

When a drive was configured with an empty search for partitions like
this (in which there is no configuration about deleting or resizing the
found partitions)...

```json
{
  "storage": {
    "drives": [
      {
        "partitions": [
          { "search": "*" },
          { "filesystem": { "path": "/" } }
        ]
      }
    ]
  } 
} 
```

...all the matched pre-existing partitions are considered as candidates
to be shrunk.

## Solution

With this fix, partitions with no actions do not generate any SpaceMaker
action.

## Bonus

`AgamaProposal` now returns an exception of type
`Y2Storage::NoDiskSpaceError` when the SpaceMaker fails to find space.
That was the documented behavior, but a generic `Y2Storage::Error` was
returned instead.

## Testing

- Added a new unit test
- Tested manually
  • Loading branch information
ancorgs authored Nov 14, 2024
2 parents 16c2ad9 + a8d4d87 commit bbc9ad2
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 13 deletions.
16 changes: 11 additions & 5 deletions service/lib/y2storage/proposal/agama_devices_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,17 @@ def process_devices
def process_existing_partitionables
partitions = partitions_for_existing(planned_devices)

# Check whether there is any chance of getting an unwanted order for the planned partitions
# within a disk
space_result = space_maker.provide_space(
original_graph, partitions: partitions, volume_groups: automatic_vgs
)
begin
# Check whether there is any chance of getting an unwanted order for the planned
# partitions within a disk
space_result = space_maker.provide_space(
original_graph, partitions: partitions, volume_groups: automatic_vgs
)
rescue Error => e
log.info "SpaceMaker was not able to find enough space: #{e}"
raise NoDiskSpaceError
end

self.devicegraph = space_result[:devicegraph]
distribution = space_result[:partitions_distribution]

Expand Down
23 changes: 16 additions & 7 deletions service/lib/y2storage/proposal/agama_space_maker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,22 @@ def delete_actions(config)
# @return [Array<Y2Storage::SpaceActions::Resize>]
def resize_actions(config)
partition_configs = partitions(config).select(&:found_device).select(&:size)
partition_configs.map do |part|
# Resize actions contain information that is potentially useful for the SpaceMaker even
# when they are only about growing and not shrinking
min = current_size?(part, :min) ? nil : part.size.min
max = current_size?(part, :max) ? nil : part.size.max
Y2Storage::SpaceActions::Resize.new(part.found_device.name, min_size: min, max_size: max)
end.compact
# Resize actions contain information that is potentially useful for the SpaceMaker even
# when they are only about growing and not shrinking
partition_configs.map { |p| resize_action(p) }.compact
end

# @see #resize_actions
#
# @param part [Agama::Storage::Configs::Partition]
# @return [Y2Storage::SpaceActions::Resize, nil]
def resize_action(part)
min = current_size?(part, :min) ? nil : part.size.min
max = current_size?(part, :max) ? nil : part.size.max
# If both min and max are equal to the current device size, there is nothing to do
return unless min || max

Y2Storage::SpaceActions::Resize.new(part.found_device.name, min_size: min, max_size: max)
end

# @see #resize_actions
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 Nov 14 15:34:17 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

- Storage: fixed bug when existing partitions were searched at the
config but not deleted or resized (gh#agama-project/agama#1767).

-------------------------------------------------------------------
Thu Nov 14 13:26:23 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

Expand Down
2 changes: 1 addition & 1 deletion service/test/agama/storage/proposal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ def drive(partitions)
subject.calculate_agama(config)

expect(subject.issues).to include(
an_object_having_attributes(description: /A problem ocurred/)
an_object_having_attributes(description: /Cannot accommodate/)
)
end
end
Expand Down
25 changes: 25 additions & 0 deletions service/test/y2storage/agama_proposal_search_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,5 +214,30 @@
end
end
end

context "when searching existing partitions but not specifying any action on them" do
let(:config_json) do
{
boot: { configure: false },
drives: [
{
search: "/dev/vda",
partitions: [
{ search: "*" },
{ size: "20 GiB", filesystem: { path: "/" } }
]
}
]
}
end

# Regression test. In the past the proposal tried to resize some partitions due to the
# search "*" without actions.
it "no partitions are deleted or resized" do
expect_any_instance_of(Y2Storage::Partition).to_not receive(:detect_resize_info)
# There is no way to make 20 GiB fit without resizing or deleting
expect { proposal.propose }.to raise_error(Y2Storage::NoDiskSpaceError)
end
end
end
end

0 comments on commit bbc9ad2

Please sign in to comment.