WIP: more tests adaptations and small fixes

service/lib/agama/dbus/storage/volume_conversion/to_dbus.rb

@@ -34,18 +34,21 @@ def initialize(volume)
           #
           # @return [Hash]
           def convert
-            {
+            hash = {
               "MountPath"    => volume.mount_path.to_s,
               "MountOptions" => volume.mount_options,
               "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,
               "AutoSize"     => volume.auto_size?,
               "Snapshots"    => volume.btrfs.snapshots?,
               "Outline"      => outline_conversion
             }
+            return hash if volume.max_size.nil? || volume.max_size.unlimited?
+
+            hash["MaxSize"] = volume.max_size.to_i
+            hash
           end
 
         private

service/lib/agama/storage/proposal_settings_reader.rb

@@ -85,10 +85,16 @@ def space_policy_reader(settings, value)
       # @param volumes [Array<Hash>]
       def volumes_reader(settings, volumes)
         builder = VolumeTemplatesBuilder.new_from_config(config)
-        mount_paths = volumes.map { |v| v["mount_path"] }.compact
+        mount_paths = volumes.map { |v| volume_path(v) }.compact
 
         settings.volumes = mount_paths.map { |mp| builder.for(mp) }
       end
+
+      def volume_path(volume)
+        return volume if volume.is_a?(String)
+
+        volume["mount_path"]
+      end
     end
   end
 end

service/lib/agama/storage/volume.rb

@@ -95,7 +95,7 @@ def initialize(mount_path)
         @mount_options = []
         @auto_size = false
         @min_size = Y2Storage::DiskSize.zero
-        @max_size = Y2Storage::DiskSize.zero
+        @max_size = Y2Storage::DiskSize.unlimited
         @btrfs = BtrfsSettings.new
         @outline = VolumeOutline.new
       end

service/lib/agama/storage/volume_conversion/to_y2storage.rb

@@ -73,8 +73,8 @@ def sizes_conversion(target)
         def btrfs_conversion(target)
           target.snapshots = volume.btrfs.snapshots?
           target.snapshots_configurable = volume.outline.snapshots_configurable?
-          target.snapshots_size = volume.outline.snapshots_size
-          target.snapshots_percentage = volume.outline.snapshots_percentage
+          target.snapshots_size = volume.outline.snapshots_size || Y2Storage::DiskSize.zero
+          target.snapshots_percentage = volume.outline.snapshots_percentage || 0
           target.subvolumes = volume.btrfs.subvolumes
           target.btrfs_default_subvolume = volume.btrfs.default_subvolume
           target.btrfs_read_only = volume.btrfs.read_only?

service/lib/agama/storage/volume_templates_builder.rb

@@ -147,8 +147,8 @@ def outline(data) # rubocop:disable Metrics/AbcSize
           outline.snapshots_configurable = fetch(outline_data, "snapshots_configurable", true)
 
           size = fetch(outline_data, "auto_size", {})
-          min = parse_disksize(fetch(size, :min))
-          max = parse_disksize(fetch(size, :max))
+          min = parse_disksize(fetch(size, :base_min))
+          max = parse_disksize(fetch(size, :base_max))
           outline.base_min_size = min if min
           outline.base_max_size = max if max
           outline.adjust_by_ram = fetch(size, :adjust_by_ram, false)

service/test/agama/dbus/storage/manager_volumes_test.rb

@@ -0,0 +1,229 @@
+# frozen_string_literal: true
+
+# Copyright (c) [2023] SUSE LLC
+#
+# All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, contact SUSE LLC.
+#
+# To contact SUSE LLC about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+
+require_relative "../../../test_helper"
+require "agama/dbus/storage/manager"
+require "agama/dbus/storage/proposal"
+require "agama/storage/manager"
+require "agama/storage/proposal"
+require "agama/storage/proposal_settings"
+require "agama/storage/volume"
+require "y2storage"
+require "dbus"
+
+describe Agama::DBus::Storage::Manager do
+  subject(:manager) { described_class.new(backend, logger) }
+
+  let(:logger) { Logger.new($stdout, level: :warn) }
+
+  let(:backend) do
+    instance_double(Agama::Storage::Manager,
+      proposal:                    proposal,
+      iscsi:                       iscsi,
+      software:                    software,
+      config:                      config,
+      on_probe:                    nil,
+      on_progress_change:          nil,
+      on_progress_finish:          nil,
+      on_issues_change:            nil,
+      on_deprecated_system_change: nil)
+  end
+
+  let(:iscsi) do
+    instance_double(Agama::Storage::ISCSI::Manager,
+      on_activate:        nil,
+      on_probe:           nil,
+      on_sessions_change: nil)
+  end
+
+  let(:software) { instance_double(Agama::DBus::Clients::Software, on_product_selected: nil) }
+
+  let(:config) { Agama::Config.new(config_data) }
+
+  let(:config_data) do
+    { "storage" => { "volumes" => cfg_volumes, "volume_templates" => cfg_templates } }
+  end
+
+  let(:cfg_volumes) { ["/", "/home", "swap"] }
+
+  let(:cfg_templates) do
+    [
+      {
+        "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true },
+        "mount_options" => ["whatever=foo"],
+        "outline" => {
+          "required" => true,
+          "auto_size" => {
+            "base_min" => "5 GiB", "base_max" => "20 GiB", "min_fallback_for" => ["/home"]
+          },
+          "filesystems" => ["btrfs"]
+        }
+      },
+      {
+        "mount_path" => "swap", "filesystem" => "swap",
+        "size" => { "auto" => false, "min" => "1 GiB", "max" => "2 GiB" },
+        "outline" => { "required" => true, "filesystems" => ["swap"] }
+      },
+      {
+        "mount_path" => "/home", "filesystem" => "xfs",
+        "size" => { "auto" => false, "min" => "10 GiB" },
+        "outline" => { "required" => false, "filesystems" => ["xfs", "ext3", "ext4"] }
+      },
+      {
+        "mount_path" => "/var", "filesystem" => "xfs",
+        "size" => { "auto" => false, "min" => "5 GiB" },
+        "outline" => { "required" => false }
+      },
+      {
+        "filesystem" => "ext4",
+        "size" => { "auto" => false, "min" => "10 GiB" },
+        "outline" => { "filesystems" => ["ext3", "ext4", "xfs"] }
+      }
+    ]
+  end
+
+  let(:proposal) do
+    instance_double(Agama::Storage::Proposal, on_calculate: nil, settings: settings)
+  end
+
+  let(:settings) { nil }
+
+  before do
+    allow(Yast::Arch).to receive(:s390).and_return false
+  end
+
+  describe "#calculate_proposal" do
+    context "when the D-Bus settings do not include information about volumes" do
+      let(:dbus_settings) { {} }
+
+      it "calculates a proposal using the default volumes" do
+        expect(proposal).to receive(:calculate) do |settings|
+          expect(settings.volumes.map(&:mount_path)).to eq cfg_volumes
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+
+      it "calculates a proposal completely ignoring templates of non-default volumes" do
+        expect(proposal).to receive(:calculate) do |settings|
+          expect(settings.volumes.map(&:mount_path)).to_not include "/var"
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+    end
+
+    context "when the D-Bus settings omit some mandatory volumes" do
+      let(:dbus_settings) { { "Volumes" => dbus_volumes } }
+      let(:dbus_volumes) { [dbus_root_vol, dbus_foo_vol ] }
+      let(:dbus_root_vol) do
+        {
+          "MountPath"  => "/",
+          "AutoSize"   => false,
+          "MinSize"    => 1024,
+          "MaxSize"    => 2048,
+          "Snapshots"  => true
+        }
+      end
+      let(:dbus_foo_vol) { { "MountPath" => "/foo" } }
+
+      it "calculates a proposal including all the mandatory volumes" do
+        expect(proposal).to receive(:calculate) do |settings|
+          expect(settings.volumes.map(&:mount_path)).to include("/", "swap")
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+
+      it "calculates a proposal including the extra volumes specified via D-Bus" do
+        expect(proposal).to receive(:calculate) do |settings|
+          expect(settings.volumes.map(&:mount_path)).to include("/foo")
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+
+      it "calculates a proposal ignoring ommitted default values that are not mandatory" do
+        expect(proposal).to receive(:calculate) do |settings|
+          expect(settings.volumes.map(&:mount_path)).to_not include("/home")
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+
+      it "calculates a proposal ignoring templates for excluded volumes" do
+        expect(proposal).to receive(:calculate) do |settings|
+          expect(settings.volumes.map(&:mount_path)).to_not include "/var"
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+
+      it "takes all volume attributes from the provided D-Bus settings" do
+        expect(proposal).to receive(:calculate) do |settings|
+          root = settings.volumes.find { |v| v.mount_path == "/" }
+
+          expect(root.auto_size).to eq(false)
+          expect(root.min_size.to_i).to eq(1024)
+          expect(root.max_size.to_i).to eq(2048)
+          expect(root.btrfs.snapshots).to eq(true)
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+
+      it "completes missing volume attributes with values from the configuration" do
+        expect(proposal).to receive(:calculate) do |settings|
+          root = settings.volumes.find { |v| v.mount_path == "/" }
+          expect(root.fs_type).to eq Y2Storage::Filesystems::Type::BTRFS
+          expect(root.mount_options).to eq ["whatever=foo"]
+
+          swap = settings.volumes.find { |v| v.mount_path == "swap" }
+          expect(swap.auto_size).to eq(false)
+          expect(swap.min_size.to_i).to eq(1024**3)
+          expect(swap.max_size.to_i).to eq(2*(1024**3))
+
+          foo = settings.volumes.find { |v| v.mount_path == "/foo" }
+          expect(foo.auto_size).to eq(false)
+          expect(foo.min_size.to_i).to eq(10*(1024**3))
+          expect(foo.max_size).to eq Y2Storage::DiskSize.unlimited
+          expect(foo.fs_type).to eq Y2Storage::Filesystems::Type::EXT4
+        end
+
+        subject.calculate_proposal(dbus_settings)
+      end
+    end
+
+    xcontext "when the D-Bus settings include changes in the volume outline" do
+    end
+
+    xcontext "when the D-Bus settings specify auto_size for an unsupported volume" do
+    end
+
+    xcontext "when the D-Bus settings specify a filesystem type not listed in the outline" do
+      # NOTE: do we have some mechanism to specify that any type is allowed (for example,
+      # empty or omitted #filesystems in an outline
+    end
+
+    xcontext "when the D-Bus settings specify a forbidden configuration for snapshots" do
+    end
+  end
+end

service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb

@@ -74,7 +74,6 @@
             "TargetVG"     => "",
             "FsType"       => "",
             "MinSize"      => 0,
-            "MaxSize"      => 0,
             "AutoSize"     => false,
             "Snapshots"    => false,
             "Outline"      => {

service/test/agama/dbus/storage/volume_conversion/to_dbus_test.rb

@@ -62,7 +62,6 @@
         "TargetVG"     => "",
         "FsType"       => "",
         "MinSize"      => 0,
-        "MaxSize"      => 0,
         "AutoSize"     => false,
         "Snapshots"    => false,
         "Outline"      => {