[service] Export space actions on D-Bus

- Moreover, the list of actions is always recovered from Y2Storage instead of remembering the actions passed in the settings to calculate a proposal.
agama-project · Feb 7, 2024 · fd4d7d2 · fd4d7d2
1 parent f3aadda
commit fd4d7d2
Show file tree

Hide file tree

Showing 11 changed files with 101 additions and 31 deletions.
diff --git a/service/lib/agama/dbus/storage/proposal.rb b/service/lib/agama/dbus/storage/proposal.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2023] SUSE LLC
+# Copyright (c) [2022-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -58,6 +58,8 @@ def initialize(backend, logger)
 
           dbus_reader :space_policy, "s"
 
+          dbus_reader :space_actions, "aa{sv}"
+
           dbus_reader :volumes, "aa{sv}"
 
           dbus_reader :actions, "aa{sv}"
@@ -109,6 +111,13 @@ def space_policy
           dbus_settings.fetch("SpacePolicy", "")
         end
 
+        # Space actions
+        #
+        # @return [Array<Hash>]
+        def space_actions
+          dbus_settings.fetch("SpaceActions", [])
+        end
+
         # Volumes used to calculate the storage proposal
         #
         # @return [Array<Hash>]

diff --git a/service/lib/agama/dbus/storage/proposal_settings_conversion/from_dbus.rb b/service/lib/agama/dbus/storage/proposal_settings_conversion/from_dbus.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -130,9 +130,11 @@ def space_policy_conversion(target, value)
           end
 
           # @param target [Agama::Storage::ProposalSettings]
-          # @param value [Hash]
+          # @param value [Array<Hash>]
           def space_actions_conversion(target, value)
-            target.space.actions = value
+            target.space.actions = value.each_with_object({}) do |v, result|
+              result[v["Device"]] = v["Action"].to_sym
+            end
           end
 
           # @param target [Agama::Storage::ProposalSettings]

diff --git a/service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb b/service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -35,7 +35,7 @@ def initialize(settings)
           # Performs the conversion to D-Bus format.
           #
           # @return [Hash]
-          def convert # rubocop:disable Metrics/AbcSize
+          def convert
             {
               "BootDevice"             => settings.boot_device.to_s,
               "LVM"                    => settings.lvm.enabled?,
@@ -44,7 +44,7 @@ def convert # rubocop:disable Metrics/AbcSize
               "EncryptionMethod"       => settings.encryption.method.id.to_s,
               "EncryptionPBKDFunction" => settings.encryption.pbkd_function&.value || "",
               "SpacePolicy"            => settings.space.policy.to_s,
-              "SpaceActions"           => settings.space.actions,
+              "SpaceActions"           => space_actions_conversion,
               "Volumes"                => settings.volumes.map { |v| VolumeConversion.to_dbus(v) }
             }
           end
@@ -53,6 +53,13 @@ def convert # rubocop:disable Metrics/AbcSize
 
           # @return [Agama::Storage::ProposalSettings]
           attr_reader :settings
+
+          # @return [Array<Hash>]
+          def space_actions_conversion
+            settings.space.actions.each_with_object([]) do |(device, action), actions|
+              actions << { "Device" => device, "Action" => action.to_s }
+            end
+          end
         end
       end
     end

diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2023] SUSE LLC
+# Copyright (c) [2022-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -85,10 +85,9 @@ def settings
           proposal.settings,
           config: config
         ).tap do |settings|
-          # FIXME: The conversion from Y2Storage cannot infer the space policy. Copying space
-          #   settings from the original settings.
+          # The conversion from Y2Storage cannot infer the space policy. Copying space policy from
+          # the original settings.
           settings.space.policy = original_settings.space.policy
-          settings.space.actions = original_settings.space.actions
           # FIXME: The conversion from Y2Storage cannot reliably infer the system VG devices in all
           #   cases. Copying system VG devices from the original settings.
           settings.lvm.system_vg_devices = original_settings.lvm.system_vg_devices

diff --git a/service/lib/agama/storage/proposal_settings_conversion/from_y2storage.rb b/service/lib/agama/storage/proposal_settings_conversion/from_y2storage.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -82,9 +82,10 @@ def encryption_conversion(target)
           target.encryption.pbkd_function = settings.encryption_pbkdf
         end
 
+        # @note The space policy cannot be inferred from Y2Storage settings.
         # @param target [Agama::Storage::ProposalSettings]
         def space_settings_conversion(target)
-          # FIXME: No way to infer space settings from Y2Storage.
+          target.space.actions = settings.space_settings.actions
         end
 
         # @param target [Agama::Storage::ProposalSettings]

diff --git a/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb b/service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb
@@ -162,7 +162,7 @@ def find_max_size_fallback(mount_path)
 
         # All block devices affected by the space policy.
         #
-        # This includes the partitions from the root device, the candidate devices and from the
+        # This includes the partitions from the boot device, the candidate devices and from the
         # devices directly assigned to a volume as target device. If a device is not partitioned,
         # then the device itself is included.
         #

diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/from_dbus_test.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -69,7 +69,16 @@
         "EncryptionMethod"       => "luks1",
         "EncryptionPBKDFunction" => "pbkdf2",
         "SpacePolicy"            => "custom",
-        "SpaceActions"           => { "/dev/sda" => "force_delete" },
+        "SpaceActions"           => [
+          {
+            "Device" => "/dev/sda",
+            "Action" => "force_delete"
+          },
+          {
+            "Device" => "/dev/sdb1",
+            "Action" => "resize"
+          }
+        ],
         "Volumes"                => [
           { "MountPath" => "/" },
           { "MountPath" => "/test" }
@@ -87,7 +96,9 @@
       expect(settings.encryption.method).to eq(Y2Storage::EncryptionMethod::LUKS1)
       expect(settings.encryption.pbkd_function).to eq(Y2Storage::PbkdFunction::PBKDF2)
       expect(settings.space.policy).to eq(:custom)
-      expect(settings.space.actions).to eq({ "/dev/sda" => "force_delete" })
+      expect(settings.space.actions).to eq({
+        "/dev/sda" => :force_delete, "/dev/sdb1" => :resize
+      })
       expect(settings.volumes.map(&:mount_path)).to contain_exactly("/", "/test")
     end
 

diff --git a/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb b/service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -38,7 +38,7 @@
       settings.encryption.method = Y2Storage::EncryptionMethod::LUKS2
       settings.encryption.pbkd_function = Y2Storage::PbkdFunction::ARGON2ID
       settings.space.policy = :custom
-      settings.space.actions = { "/dev/sda" => :force_delete }
+      settings.space.actions = { "/dev/sda" => :force_delete, "/dev/sdb1" => "resize" }
       settings.volumes = [Agama::Storage::Volume.new("/test")]
     end
   end
@@ -53,7 +53,7 @@
         "EncryptionMethod"       => "luks2",
         "EncryptionPBKDFunction" => "pbkdf2",
         "SpacePolicy"            => "keep",
-        "SpaceActions"           => {},
+        "SpaceActions"           => [],
         "Volumes"                => []
       )
 
@@ -65,7 +65,16 @@
         "EncryptionMethod"       => "luks2",
         "EncryptionPBKDFunction" => "argon2id",
         "SpacePolicy"            => "custom",
-        "SpaceActions"           => { "/dev/sda" => :force_delete },
+        "SpaceActions"           => [
+          {
+            "Device" => "/dev/sda",
+            "Action" => "force_delete"
+          },
+          {
+            "Device" => "/dev/sdb1",
+            "Action" => "resize"
+          }
+        ],
         "Volumes"                => [
           {
             "MountPath"     => "/test",

diff --git a/service/test/agama/dbus/storage/proposal_test.rb b/service/test/agama/dbus/storage/proposal_test.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2023] SUSE LLC
+# Copyright (c) [2022-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -181,6 +181,34 @@
     end
   end
 
+  describe "#space_actions" do
+    context "if a proposal has not been calculated yet" do
+      let(:settings) { nil }
+
+      it "returns an empty list" do
+        expect(subject.space_actions).to eq([])
+      end
+    end
+
+    context "if a proposal has been calculated" do
+      let(:settings) do
+        Agama::Storage::ProposalSettings.new.tap do |settings|
+          settings.space.actions = {
+            "/dev/vda1" => :force_delete,
+            "/dev/vda2" => :resize
+          }
+        end
+      end
+
+      it "return a list with a hash for each action" do
+        expect(subject.space_actions).to contain_exactly(
+          { "Device" => "/dev/vda1", "Action" => "force_delete" },
+          { "Device" => "/dev/vda2", "Action" => "resize" }
+        )
+      end
+    end
+  end
+
   describe "#volumes" do
     let(:settings) do
       Agama::Storage::ProposalSettings.new.tap { |s| s.volumes = calculated_volumes }

diff --git a/service/test/agama/storage/proposal_settings_conversion/from_y2storage_test.rb b/service/test/agama/storage/proposal_settings_conversion/from_y2storage_test.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -38,6 +38,10 @@
         settings.encryption_password = "notsecret"
         settings.encryption_method = Y2Storage::EncryptionMethod::LUKS2
         settings.encryption_pbkdf = Y2Storage::PbkdFunction::ARGON2ID
+        settings.space_settings.actions = {
+          "/dev/sda"  => :force_delete,
+          "/dev/sdb1" => :resize
+        }
         settings.volumes = []
       end
     end
@@ -57,6 +61,9 @@
           method:        Y2Storage::EncryptionMethod::LUKS2,
           pbkd_function: Y2Storage::PbkdFunction::ARGON2ID
         ),
+        space:       an_object_having_attributes(
+          actions: { "/dev/sda" => :force_delete, "/dev/sdb1" => :resize }
+        ),
         volumes:     []
       )
     end

diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2022-2023] SUSE LLC
+# Copyright (c) [2022-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -155,13 +155,10 @@
           volumes:     contain_exactly(
             an_object_having_attributes(mount_path: "/")
           ),
-          # Checking space settings explicitly here because the settings converter cannot infer the
-          # space settings from the Y2Storage settings. The space settings are directly recovered
-          # from the original settings passed to #calculate.
-          space:       an_object_having_attributes(
-            policy:  :custom,
-            actions: { "/dev/sda" => :force_delete }
-          )
+          # Checking space policy explicitly here because the settings converter cannot infer the
+          # space policy from the Y2Storage settings. The space policy is directly recovered from
+          # the original settings passed to #calculate.
+          space:       an_object_having_attributes(policy: :custom)
         )
       end