Merge pull request #766 from yast/fix/support-old-md-raid-ay-format

Fix/support old md raid ay format

src/lib/y2storage/autoinst_profile/drive_section.rb

@@ -126,6 +126,7 @@ def init_from_hashes(hash)
         @skip_list = SkipListSection.new_from_hashes(hash.fetch("skip_list", []), self)
         if hash["raid_options"]
           @raid_options = RaidOptionsSection.new_from_hashes(hash["raid_options"], self)
+          @raid_options.raid_name = nil # This element is not supported here
         end
       end
 
@@ -279,16 +280,21 @@ def init_from_vg(vg)
       # Method used by {.new_from_storage} to populate the attributes when
       # cloning a MD RAID.
       #
-      # AutoYaST does not support multiple partitions on a MD RAID.
-      #
       # @param md [Y2Storage::Md] RAID
       # @return [Boolean]
       def init_from_md(md)
         @type = :CT_MD
         @device = md.name
-        @partitions = partitions_from_collection([md])
-        @enable_snapshots = enabled_snapshots?([md.filesystem])
+        collection =
+          if md.filesystem
+            [md]
+          else
+            md.partitions
+          end
+        @partitions = partitions_from_collection(collection)
+        @enable_snapshots = enabled_snapshots?(collection.map(&:filesystem).compact)
         @raid_options = RaidOptionsSection.new_from_storage(md)
+        @raid_options.raid_name = nil if @raid_options # This element is not supported here
         true
       end
 

src/lib/y2storage/autoinst_profile/partition_section.rb

@@ -287,10 +287,8 @@ def partition_id_from(partition)
       def init_fields_by_type(device)
         if device.is?(:lvm_lv)
           init_lv_fields(device)
-        elsif device.is?(:md)
-          init_md_fields(device)
         elsif device.is?(:disk_device)
-          init_disk_fields(device)
+          init_disk_device_fields(device)
         else
           init_partition_fields(device)
         end
@@ -305,10 +303,10 @@ def init_partition_fields(partition)
         @raid_name = partition.md.name if partition.md
       end
 
-      def init_disk_fields(disk)
+      def init_disk_device_fields(disk)
         @create = false
         @lvm_group = lvm_group_name(disk)
-        @raid_name = disk.md.name if disk.md
+        @raid_name = disk.md.name if disk.respond_to?(:md) && disk.md
       end
 
       def init_lv_fields(lv)
@@ -320,12 +318,6 @@ def init_lv_fields(lv)
         @used_pool = parent.lv_name if lv.lv_type == LvType::THIN && parent.is?(:lvm_lv)
       end
 
-      def init_md_fields(md)
-        @partition_nr = md.number if md.numeric?
-        @raid_options = RaidOptionsSection.new_from_storage(md)
-        @lvm_group = lvm_group_name(md)
-      end
-
       def init_encryption_fields(partition)
         return unless partition.encrypted?
 

src/lib/y2storage/proposal/autoinst_md_planner.rb

@@ -31,36 +31,56 @@ class AutoinstMdPlanner < AutoinstDrivePlanner
       # @param drive [AutoinstProfile::DriveSection] drive section describing the MD RAID
       # @return [Array<Planned::Md>] Planned MD RAID devices
       def planned_devices(drive)
-        md =
-          if drive.device == "/dev/md" || !drive.partition_table?
+        mds =
+          if drive.device == "/dev/md"
+            non_partitioned_md_old_style(drive)
+          elsif !drive.partition_table?
             non_partitioned_md(drive)
           else
             partitioned_md(drive)
           end
 
-        Array(md)
+        Array(mds)
       end
 
     private
 
+      # Returns a list of non partitioned MD RAID devices from old-style AutoYaST profile
+      #
+      # Using `/dev/md` as device name means that the whole drive section should be treated as an
+      # old-style AutoYaST MD RAID description.
+      #
+      #   * Each partition represents an MD RAID and the `partition_nr` is used to indicate
+      #     the kernel name: `/dev/md0`, `/dev/md1`, etc.
+      #   * Partitioned MD RAID devices are not supported
+      #
+      # @param drive [AutoinstProfile::DriveSection] drive section describing the list of MD RAID
+      #   devices (old-style AutoYaST)
+      # @return [Array<Planned::Md>] List of planned MD RAID devices
+      def non_partitioned_md_old_style(drive)
+        drive.partitions.map do |part_section|
+          md_from_partition_section(drive, part_section)
+        end
+      end
+
       # Returns a non partitioned MD RAID
       #
       # @param drive [AutoinstProfile::DriveSection] drive section describing the MD RAID
-      # @return [Planned::Md]
+      # @return [Planned::Md] Planned MD RAID device
       def non_partitioned_md(drive)
         md = Planned::Md.new(name: drive.name_for_md)
         part_section = drive.partitions.first
         device_config(md, part_section, drive)
         md.lvm_volume_group_name = part_section.lvm_group
         add_md_reuse(md, part_section) if part_section.create == false
-        add_raid_options(md, part_section.raid_options)
+        add_raid_options(md, drive.raid_options || part_section.raid_options)
         md
       end
 
       # Returns a partitioned MD RAID
       #
       # @param drive [AutoinstProfile::DriveSection] drive section describing the MD RAID
-      # @return [Planned::Md]
+      # @return [Planned::Md] Planned MD RAID device
       def partitioned_md(drive)
         md = Planned::Md.new(name: drive.device)
         md.ptable_type = Y2Storage::PartitionTables::Type.find(drive.disklabel) if drive.disklabel
@@ -72,6 +92,22 @@ def partitioned_md(drive)
         md
       end
 
+      # Returns a non partitioned MD RAID from an AutoYaST partition section
+      #
+      # @param drive        [AutoinstProfile::DriveSection] drive section describing the list of MD
+      #   RAID devices (old-style AutoYaST)
+      # @param part_section [AutoinstProfile::PartitionSection] partition section describing the
+      #   an MD RAID
+      # @return [Array<Planned::Md>] List of planned MD RAID devices
+      def md_from_partition_section(drive, part_section)
+        md = Planned::Md.new(name: part_section.name_for_md)
+        device_config(md, part_section, drive)
+        md.lvm_volume_group_name = part_section.lvm_group
+        add_md_reuse(md, part_section) if part_section.create == false
+        add_raid_options(md, part_section.raid_options)
+        md
+      end
+
       # Adds RAID options to a planned RAID
       #
       # @param md [Planned::Md] Planned RAID

test/y2storage/autoinst_profile/drive_section_test.rb

@@ -66,7 +66,18 @@ def lvm_vg(name)
       it "initializes raid options" do
         expect(Y2Storage::AutoinstProfile::RaidOptionsSection).to receive(:new_from_hashes)
           .with(raid_options, Y2Storage::AutoinstProfile::DriveSection)
-        described_class.new_from_hashes(hash)
+          .and_call_original
+        section = described_class.new_from_hashes(hash)
+        expect(section.raid_options.raid_type).to eq("raid0")
+      end
+
+      context "when the raid_name is specified in the raid_options" do
+        let(:raid_options) { { "raid_type" => "raid0" } }
+
+        it "ignores the raid_name element" do
+          section = described_class.new_from_hashes(hash)
+          expect(section.raid_options.raid_name).to be_nil
+        end
       end
     end
 

test/y2storage/autoinst_profile/partition_section_test.rb

@@ -196,30 +196,6 @@ def section_for(name)
           .and_return(raid_options)
       end
 
-      it "initializes #raid_options" do
-        expect(Y2Storage::AutoinstProfile::RaidOptionsSection).to receive(:new_from_storage)
-          .with(md).and_return(raid_options)
-        expect(described_class.new_from_storage(md).raid_options).to eq(raid_options)
-      end
-
-      context "when it is not a named RAID" do
-        let(:numeric?) { true }
-
-        it "initializes #partition_nr to the RAID number" do
-          section = described_class.new_from_storage(md)
-          expect(section.partition_nr).to eq(md.number)
-        end
-      end
-
-      context "when it is a named RAID" do
-        let(:numeric?) { false }
-
-        it "does not initialize #partition_nr" do
-          section = described_class.new_from_storage(md)
-          expect(section.partition_nr).to be_nil
-        end
-      end
-
       context "when it is used as an LVM physical volume" do
         let(:lvm_vg) { instance_double(Y2Storage::LvmVg, basename: "vg0") }
         let(:lvm_pv) do

test/y2storage/proposal/autoinst_md_planner_test.rb

@@ -40,25 +40,89 @@
     let(:drive) { Y2Storage::AutoinstProfile::DriveSection.new_from_hashes(raid) }
 
     let(:raid) do
-      { "device" => "/dev/md2", "raid_options" => raid_options, "partitions" => [home_spec] }
+      {
+        "device" => device, "raid_options" => raid_options, "disklabel" => disklabel,
+        "partitions" => [home_spec]
+      }
     end
 
     let(:home_spec) do
-      {
-        "mount" => "/home", "filesystem" => "xfs", "size" => "max", "partition_nr" => 1,
-        "raid_options" => raid_options
-      }
+      { "mount" => "/home", "filesystem" => "xfs", "size" => "max", "partition_nr" => 1 }
     end
 
     let(:raid_options) do
       { "raid_type" => "raid5" }
     end
 
+    let(:disklabel) { nil }
+
+    let(:device) { "/dev/md2" }
+
     it "returns a planned RAID with the given device name" do
       md = planner.planned_devices(drive).first
       expect(md.name).to eq("/dev/md2")
     end
 
+    context "when a partition table type is specified" do
+      let(:disklabel) { "msdos" }
+
+      it "returns a planned MD RAID with partitions" do
+        md = planner.planned_devices(drive).first
+        expect(md.partitions).to contain_exactly(
+          an_object_having_attributes("mount_point" => "/home")
+        )
+      end
+
+      it "sets the partition table type" do
+        md = planner.planned_devices(drive).first
+        expect(md.ptable_type).to eq(Y2Storage::PartitionTables::Type.find("msdos"))
+      end
+    end
+
+    context "when no partition table type is specified" do
+      it "returns a planned MD RAID with partitions" do
+        md = planner.planned_devices(drive).first
+        expect(md.partitions).to contain_exactly(
+          an_object_having_attributes("mount_point" => "/home")
+        )
+      end
+
+      it "does not set the partition table type" do
+        md = planner.planned_devices(drive).first
+        expect(md.ptable_type).to be_nil
+      end
+    end
+
+    context "when the partition table type is set to 'none'" do
+      let(:disklabel) { "none" }
+
+      it "returns a planned MD RAID with filesystem settings (no partitions)" do
+        md = planner.planned_devices(drive).first
+        expect(md.mount_point).to eq("/home")
+        expect(md.filesystem_type).to eq(Y2Storage::Filesystems::Type::XFS)
+      end
+
+      it "does not set the partition table type" do
+        md = planner.planned_devices(drive).first
+        expect(md.ptable_type).to be_nil
+      end
+
+      context "and RAID options are not specified at drive level" do
+        let(:raid_options) { nil }
+        let(:home_spec) do
+          {
+            "mount" => "/home", "filesystem" => "xfs", "size" => "max", "partition_nr" => 1,
+            "raid_options" => { "raid_type" => "raid5" }
+          }
+        end
+
+        it "reads options from the partition section" do
+          md = planner.planned_devices(drive).first
+          expect(md.md_level).to eq(Y2Storage::MdLevel::RAID5)
+        end
+      end
+    end
+
     context "when no RAID level is specified" do
       let(:raid_options) { nil }
 
@@ -85,62 +149,48 @@
       end
     end
 
-    it "returns a planned RAID including filesystem settings" do
-      md = planner.planned_devices(drive).first
-      home = md.partitions.first
-      expect(home.mount_point).to eq("/home")
-      expect(home.filesystem_type).to eq(Y2Storage::Filesystems::Type::XFS)
-    end
-
     context "when using a named RAID" do
-      let(:raid_options) do
-        { "raid_name" => "/dev/md/data", "raid_type" => "raid5" }
-      end
+      let(:device) { "/dev/md/data" }
 
       it "uses the name instead of a number" do
         md = planner.planned_devices(drive).first
         expect(md.name).to eq("/dev/md/data")
       end
     end
 
-    context "when using a partitioned RAID" do
+    context "using the old schema" do
       let(:raid) do
-        { "device" => "/dev/md2", "partitions" => [home_spec] }
-      end
-
-      it "returns a planned RAID using the specified device as name" do
-        md = planner.planned_devices(drive).first
-        expect(md.name).to eq("/dev/md2")
+        { "device" => "/dev/md", "partitions" => [root_raid_spec, home_raid_spec] }
       end
 
-      it "returns a planned RAID including partitions" do
-        md = planner.planned_devices(drive).first
-        expect(md.partitions).to contain_exactly(
-          an_object_having_attributes("mount_point" => "/home")
-        )
+      let(:raid_options) do
+        { "raid_type" => "raid5" }
       end
-    end
 
-    context "using the old schema" do
-      let(:raid) do
-        { "device" => "/dev/md", "partitions" => [home_spec] }
+      let(:root_raid_spec) do
+        {
+          "mount" => "/", "filesystem" => "ext4", "size" => "max", "partition_nr" => 1,
+          "raid_options" => { "raid_type" => "raid5" }
+        }
       end
 
-      let(:home_spec) do
+      let(:home_raid_spec) do
         {
-          "mount" => "/home", "filesystem" => "xfs", "size" => "max", "partition_nr" => 1,
-          "raid_options" => raid_options
+          "mount" => "/home", "filesystem" => "xfs", "size" => "max", "partition_nr" => 2,
+          "raid_options" => { "raid_type" => "raid1" }
         }
       end
 
       it "returns a planned RAID using /dev/md + partition_nr as device name" do
-        md = planner.planned_devices(drive).first
-        expect(md.name).to eq("/dev/md1")
-      end
-
-      it "returns a planned RAID of the wanted type" do
-        md = planner.planned_devices(drive).first
-        expect(md.md_level).to eq(Y2Storage::MdLevel::RAID5)
+        mds = planner.planned_devices(drive)
+        expect(mds).to contain_exactly(
+          an_object_having_attributes(
+            "name" => "/dev/md1", "md_level" => Y2Storage::MdLevel::RAID5
+          ),
+          an_object_having_attributes(
+            "name" => "/dev/md2", "md_level" => Y2Storage::MdLevel::RAID1
+          )
+        )
       end
     end
   end