WIP: tests and fixes for size calculation

service/lib/agama/storage/config.rb

@@ -100,11 +100,12 @@ def default_size_devices
  end
 
  def default_size(device, attr, builder)
- # TODO: what to do if path is nil or empty?
- path = device.filesystem&.path
- # TODO: what to do if there is no default volume?
+ path = device.filesystem&.path || ""
  vol = builder.for(path)
+ return fallback_size(attr) unless vol
 
+ # Theoretically, neither Volume#min_size or Volume#max_size can be nil
+ # At most they will be zero or unlimited, respectively
  return vol.send(:"#{attr}_size") unless vol.auto_size?
 
  outline = vol.outline
@@ -113,6 +114,14 @@ def default_size(device, attr, builder)
  size_with_snapshots(size, device, outline)
  end
 
+ # TODO: these are the fallbacks used when constructing volumes, not sure if repeating them
+ # here is right
+ def fallback_size(attr)
+ return Y2Storage::DiskSize.zero if attr == :min
+
+ Y2Storage::DiskSize.unlimited
+ end
+
  def size_with_fallbacks(device, outline, attr, builder)
  proposed_paths = filesystems.map(&:path)
 

service/lib/agama/storage/config_conversions/filesystem_type/from_json.rb

@@ -72,7 +72,7 @@ def convert_btrfs(default = nil)
  default_config.tap do |config|
  snapshots = btrfs_json[:snapshots]
 
- config.snapshots = snapshots if snapshots
+ config.snapshots = snapshots unless snapshots.nil?
  end
  end
  end

service/test/agama/storage/config_conversions/from_json_test.rb

@@ -43,6 +43,10 @@
  "volume_templates" => [
  {
  "mount_path" => "/", "filesystem" => "btrfs", "size" => { "auto" => true },
+ "btrfs" => {
+ "snapshots" => true, "default_subvolume" => "@",
+ "subvolumes" => ["home", "opt", "root", "srv"]
+ },
  "outline" => {
  "required" => true, "snapshots_configurable" => true,
  "auto_size" => {
@@ -53,22 +57,29 @@
  }
  },
  {
- "mount_path" => "/home",
- "outline" => {
- "required" => false, "filesystem" => "xfs",
- "size" => { "auto" => false, "min" => "5 GiB" }
- }
+ "mount_path" => "/home", "size" => { "auto" => false, "min" => "5 GiB" },
+ "outline" => { "required" => false, "filesystem" => "xfs" }
  },
  {
  "mount_path" => "swap",
- "outline" => { "required" => false }
- }
+ "outline" => { "required" => false, "filesystem" => "swap" }
+ },
+ { "mount_path" => "", "size" => { "min" => "100 MiB" } }
  ]
  }
  }
  end
 
  describe "#convert" do
+ using Y2Storage::Refinements::SizeCasts
+
+ # TODO:
+ # Encryption
+ # Filesystem type (btrfs, etc)
+ # Filesystem at disk (including default types based on config, etc.)
+ # Filesystem at partition
+ # Partition id
+
  context "with an empty JSON configuration" do
  let(:config_json) { {} }
 
@@ -94,18 +105,11 @@
  context "with some drives and boot configuration at JSON" do
  let(:config_json) do
  {
- boot: {
- configure: true,
- device: "/dev/sdb"
- },
+ boot: { configure: true, device: "/dev/sdb" },
  drives: [
  {
  ptableType: "gpt",
- partitions: [
- {
- filesystem: { path: "/", type: { btrfs: { snapshots: false } } }
- }
- ]
+ partitions: [{ filesystem: { path: "/" } }]
  }
  ]
  }
@@ -122,6 +126,132 @@
  expect(config.boot.configure).to eq true
  expect(config.boot.device).to eq "/dev/sdb"
  end
+
+ it "includes the corresponding drives" do
+ config = subject.convert
+ expect(config.drives.size).to eq 1
+ drive = config.drives.first
+ expect(drive).to be_a(Agama::Storage::Configs::Drive)
+ expect(drive.ptable_type).to eq Y2Storage::PartitionTables::Type::GPT
+ expect(drive.partitions.size).to eq 1
+ partition = drive.partitions.first
+ expect(partition.filesystem.path).to eq "/"
+ end
+ end
+
+ context "omitting sizes for the partitions" do
+ let(:config_json) do
+ {
+ drives: [
+ {
+ partitions: [
+ {
+ filesystem: { path: "/", type: { btrfs: { snapshots: false } } }
+ },
+ {
+ filesystem: { path: "/home" }
+ },
+ {
+ filesystem: { path: "/opt" }
+ },
+ {
+ filesystem: { path: "swap" }
+ }
+ ]
+ }
+ ]
+ }
+ end
+
+ it "uses default sizes" do
+ config = subject.convert
+ partitions = config.drives.first.partitions
+ expect(partitions).to contain_exactly(
+ an_object_having_attributes(
+ filesystem: have_attributes(path: "/"),
+ size: have_attributes(default: true, min: 5.GiB, max: 10.GiB)
+ ),
+ an_object_having_attributes(
+ filesystem: have_attributes(path: "/home"),
+ size: have_attributes(default: true, min: 5.GiB, max: Y2Storage::DiskSize.unlimited)
+ ),
+ an_object_having_attributes(
+ filesystem: have_attributes(path: "/opt"),
+ size: have_attributes(default: true, min: 100.MiB, max: Y2Storage::DiskSize.unlimited)
+ ),
+ an_object_having_attributes(
+ filesystem: have_attributes(path: "swap"),
+ size: have_attributes(
+ default: true, min: Y2Storage::DiskSize.zero, max: Y2Storage::DiskSize.unlimited
+ )
+ )
+ )
+ end
+ end
+
+ # Note the min is mandatory
+ context "specifying size limits for the partitions" do
+ let(:config_json) do
+ {
+ drives: [
+ {
+ partitions: [
+ {
+ filesystem: { path: "/", type: { btrfs: { snapshots: false } } },
+ size: { min: "3 GiB" }
+ },
+ {
+ filesystem: { path: "/home" },
+ size: { min: "6 GiB", max: "9 GiB" }
+ }
+ ]
+ }
+ ]
+ }
+ end
+
+ it "sets both min and max limits as requested" do
+ config = subject.convert
+ partitions = config.drives.first.partitions
+ expect(partitions).to include(
+ an_object_having_attributes(
+ filesystem: have_attributes(path: "/home"),
+ size: have_attributes(default: false, min: 6.GiB, max: 9.GiB)
+ )
+ )
+ end
+
+ it "uses unlimited for the omitted max sizes" do
+ config = subject.convert
+ partitions = config.drives.first.partitions
+ expect(partitions).to include(
+ an_object_having_attributes(
+ filesystem: have_attributes(path: "/"),
+ size: have_attributes(default: false, min: 3.GiB, max: Y2Storage::DiskSize.unlimited)
+ )
+ )
+ end
+ end
+
+ context "specifying a filesystem for a drive" do
+ let(:config_json) do
+ {
+ drives: [{ filesystem: filesystem }]
+ }
+ end
+
+ context "if the filesystem specification only contains a path" do
+ let(:filesystem) { { path: "/" } }
+
+ it "uses the default type and btrfs attributes for that path" do
+ config = subject.convert
+ filesystem = config.drives.first.filesystem
+ expect(filesystem.type.fstype).to eq Y2Storage::Filesystems::Type::BTRFS
+ expect(filesystem.type.btrfs.snapshots).to eq true
+ expect(filesystem.type.btrfs.default_subvolume).to eq "@"
+ expect(filesystem.type.btrfs.subvolumes.map(&:path)).to eq ["home", "opt", "root", "srv"]
+ end
+ end
  end
  end
 end