Merge pull request #896 from joseivanlopez/autoyast_new_nfs_format

Add support for NFS in Autoyast (new format)

package/yast2-storage-ng.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Apr 10 11:04:44 UTC 2019 - José Iván López González <jlopez@suse.com>
+
+- AutoYaST: new format for importing/exporting NFS drives.
+- Related to bsc#1130256.
+- 4.1.82
+
 -------------------------------------------------------------------
 Wed Apr 10 07:55:26 UTC 2019 - David Diaz <dgonzalez@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:		yast2-storage-ng
-Version:	4.1.81
+Version:	4.1.82
 Release:	0
 
 BuildRoot:	%{_tmppath}/%{name}-%{version}-build

src/lib/y2storage/autoinst_profile/drive_section.rb

@@ -138,6 +138,13 @@ def init_from_hashes(hash)
         end
       end
 
+      # Default drive type depending on the device name
+      #
+      # For NFS, the default type can only be inferred when using the old format. With the new
+      # format, type attribute is mandatory.
+      #
+      # @param hash [Hash]
+      # @return [Symbol]
       def default_type_for(hash)
         device_name = hash["device"].to_s
 
@@ -159,7 +166,7 @@ def default_type_for(hash)
       #
       # @param device [BlkDevice] a block device that can be cloned into a
       #   <drive> section, like a disk, a DASD or an LVM volume group.
-      # @return [DriveSection]
+      # @return [DriveSection, nil] nil if the device cannot be exported
       def self.new_from_storage(device)
         result = new
         # So far, only disks (and DASD) are supported
@@ -185,6 +192,8 @@ def init_from_device(device)
           init_from_stray_blk_device(device)
         elsif device.is?(:bcache)
           init_from_bcache(device)
+        elsif device.is?(:nfs)
+          init_from_nfs(device)
         else
           init_from_disk(device)
         end
@@ -275,10 +284,14 @@ def bcache_name?(device_name)
 
       # Whether the given name is a NFS name
       #
+      # Note that this method only recognizes a NFS name when the old format is used,
+      # that is, device attribute contains "/dev/nfs". With the new format, device
+      # contains the NFS share name (server:path), but in this case the type attribute
+      # is mandatory to identify the drive type.
+      #
       # @param device_name [String]
       # @return [Boolean]
       def nfs_name?(device_name)
-        # TODO: with the new format, device_name would be "server:path"
         device_name == "/dev/nfs"
       end
 
@@ -391,6 +404,16 @@ def init_from_stray_blk_device(device)
         true
       end
 
+      def init_from_nfs(device)
+        @type = :CT_NFS
+        @device = device.share
+        @use = "all"
+        @disklabel = "none"
+        @partitions = [PartitionSection.new_from_storage(device)]
+
+        true
+      end
+
       def partitions_from_hash(hash)
         return [] unless hash["partitions"]
         hash["partitions"].map { |part| PartitionSection.new_from_hashes(part, self) }

src/lib/y2storage/autoinst_profile/partition_section.rb

@@ -160,7 +160,7 @@ def self.attributes
 
       # @!attribute device
       #   @return [String, nil] undocumented attribute, but used to indicate a NFS
-      #     share when installing over NFS
+      #     share when installing over NFS (with the old profile format)
 
       def init_from_hashes(hash)
         super
@@ -177,12 +177,12 @@ def init_from_hashes(hash)
       end
 
       # Clones a device into an AutoYaST profile section by creating an instance
-      # of this class from the information in a partition or LVM logical volume.
+      # of this class from the information of a device
       #
       # @see PartitioningSection.new_from_storage for more details
       #
-      # @param device [BlkDevice] a block device that can be cloned into a
-      #   <partition> section, like a partition, an LVM logical volume or an MD RAID.
+      # @param device [Device] a device that can be cloned into a <partition> section,
+      #   like a partition, an LVM logical volume, an MD RAID or a NFS filesystem.
       # @return [PartitionSection]
       def self.new_from_storage(device)
         result = new
@@ -248,15 +248,20 @@ def name_for_md
       # As usual, it keeps the behavior of the old clone functionality, check
       # the implementation of this class for details.
       #
-      # @param device [BlkDevice] a block device that can be cloned into a
-      #   <partition> section, like a partition, an LVM logical volume or an MD RAID.
+      # @param device [Device] a device that can be cloned into a <partition> section,
+      #   like a partition, an LVM logical volume, an MD RAID or a NFS filesystem.
       def init_from_device(device)
         @create = true
         @resize = false
 
         init_fields_by_type(device)
-        init_encryption_fields(device)
-        init_filesystem_fields(device)
+
+        # Exporting these values only makes sense when the device is a block device. Note
+        # that some exported devices (e.g., NFS filesystems) are not block devices.
+        if device.is?(:blk_device)
+          init_encryption_fields(device)
+          init_filesystem_fields(device)
+        end
 
         # NOTE: The old AutoYaST exporter does not report the real size here.
         # It intentionally reports one cylinder less. Cylinders is an obsolete
@@ -300,6 +305,8 @@ def init_fields_by_type(device)
           init_lv_fields(device)
         elsif device.is?(:disk_device, :software_raid, :stray_blk_device, :bcache)
           init_disk_device_fields(device)
+        elsif device.is?(:nfs)
+          init_nfs_fields(device)
         else
           init_partition_fields(device)
         end
@@ -386,6 +393,11 @@ def init_bcache_fields(device)
         end
       end
 
+      def init_nfs_fields(device)
+        @create = false
+        init_mount_options(device)
+      end
+
       # Whether the given existing partition should be reported as GRUB (GPT
       # Bios Boot) in the cloned profile.
       #

src/lib/y2storage/autoinst_profile/partitioning_section.rb

@@ -82,13 +82,8 @@ def self.new_from_hashes(drives_array)
       # @return [PartitioningSection]
       def self.new_from_storage(devicegraph)
         result = new
-        # TODO: consider also NFS and TMPFS
-        devices = devicegraph.bcaches + devicegraph.software_raids + devicegraph.lvm_vgs +
-          devicegraph.disk_devices + devicegraph.stray_blk_devices
-        result.drives = devices.each_with_object([]) do |dev, array|
-          drive = DriveSection.new_from_storage(dev)
-          array << drive if drive
-        end
+        result.drives = drives_from_storage(devicegraph)
+
         result
       end
 
@@ -141,6 +136,36 @@ def nfs_drives
       def section_name
         "partitioning"
       end
+
+      # All drive sections generated from a given devicegraph
+      #
+      # It creates a drive section for each exportable device, see {#exportable_devices}.
+      #
+      # @param devicegraph [Devicegraph]
+      # @return [Array<DriveSection>]
+      def self.drives_from_storage(devicegraph)
+        devices = exportable_devices(devicegraph)
+
+        devices.map { |d| DriveSection.new_from_storage(d) }.compact
+      end
+
+      # All devices that can be exported by AutoYaST
+      #
+      # @param devicegraph [Devicegraph]
+      # @return [Array<Device>]
+      def self.exportable_devices(devicegraph)
+        # TODO: consider also TMPFS
+        [].concat(
+          devicegraph.bcaches,
+          devicegraph.software_raids,
+          devicegraph.lvm_vgs,
+          devicegraph.disk_devices,
+          devicegraph.stray_blk_devices,
+          devicegraph.nfs_mounts
+        )
+      end
+
+      private_class_method :drives_from_storage, :exportable_devices
     end
   end
 end

src/lib/y2storage/proposal/autoinst_devices_planner.rb

@@ -149,7 +149,7 @@ def planned_for_nfs(drive)
       def remove_shadowed_subvols(planned_devices)
         planned_devices.each do |device|
           # Some planned devices could be mountable but not formattable (e.g., {Planned::Nfs}).
-          # Those devices might shadow some subvolumes but they do no have any subvolume to
+          # Those devices might shadow some subvolumes but they do not have any subvolume to
           # be shadowed.
           next unless device.respond_to?(:shadowed_subvolumes)
 

src/lib/y2storage/proposal/autoinst_nfs_planner.rb

@@ -31,9 +31,9 @@ class AutoinstNfsPlanner < AutoinstDrivePlanner
       # @param drive [AutoinstProfile::DriveSection] drive section describing NFS filesystems
       # @return [Array<Planned::Nfs>] Planned NFS filesystems
       def planned_devices(drive)
-        # TODO: planned device with new format
+        return planned_devices_old_format(drive) if old_format?(drive)
 
-        planned_devices_old_format(drive)
+        planned_devices_new_format(drive)
       end
 
     private
@@ -57,11 +57,25 @@ def planned_devices(drive)
 
       # Similar to {OLD_FORMAT_MANDATORY_VALUES}, but for the new AutoYaST style.
       #
-      # TODO
-      NEW_FORMAT_MANDATORY_VALUES = {}.freeze
+      # With new format, drive section must contain a device and partition section must contain
+      # a mount value, e.g.:
+      #
+      # <drive>
+      #   <device>192.168.56.1:/root_fs</device>
+      #   <partitions>
+      #     <partition>
+      #       <mount>/</mount>
+      #     </partition>
+      #   </partitions>
+      # </drive>
+      NEW_FORMAT_MANDATORY_VALUES = { drive: [:device], partition: [:mount] }.freeze
 
       private_constant :OLD_FORMAT_MANDATORY_VALUES, :NEW_FORMAT_MANDATORY_VALUES
 
+      def old_format?(drive)
+        drive.device == "/dev/nfs" || drive.partitions.any?(&:device)
+      end
+
       # Returns a list of planned NFS filesystems from the old-style AutoYaST profile
       #
       # Using `/dev/nfs` as device name means that the whole drive section should be treated as an
@@ -77,7 +91,6 @@ def planned_devices_old_format(drive)
 
       # Creates a planned NFS filesystem from the old-style AutoYaST profile
       #
-      #
       # @param drive [AutoinstProfile::DriveSection] drive section describing the list of NFS
       #   filesystems
       # @param partition_section [AutoinstProfile::PartitionSection] partition section describing
@@ -87,22 +100,51 @@ def planned_devices_old_format(drive)
       def planned_device_old_format(drive, partition_section)
         return nil unless valid_drive?(drive, partition: partition_section, profile_format: :old)
 
+        issues_list.add(:no_partitionable, drive) if drive.wanted_partitions?
+
         share = partition_section.device
 
-        planned_nfs = Planned::Nfs.new(server(share), path(share))
-        add_options(planned_nfs, partition_section)
+        create_planned_nfs(share, partition_section)
+      end
 
-        planned_nfs
+      # Returns a list of planned NFS filesystems from the new-style AutoYaST profile
+      #
+      # @param drive [AutoinstProfile::DriveSection] drive section describing a NFS share
+      # @return [Array<Planned::Nfs>] List containing the planned NFS filesystem
+      def planned_devices_new_format(drive)
+        [planned_device_new_format(drive)].compact
+      end
+
+      # Creates a planned NFS filesystem from the new-style AutoYaST profile
+      #
+      # @param drive [AutoinstProfile::DriveSection] drive section describing a NFS share
+      # @return [Planned::Nfs]
+      def planned_device_new_format(drive)
+        return nil unless valid_drive?(drive, profile_format: :new)
+
+        issues_list.add(:no_partitionable, drive) if drive.wanted_partitions?
+        issues_list.add(:surplus_partitions, drive) if drive.partitions.size > 1
+
+        share = drive.device
+        master_partition = drive.partitions.first
+
+        create_planned_nfs(share, master_partition)
       end
 
-      # Adds options to the planned NFS filesystem
+      # Creates a planned NFS filesystem
       #
-      # @param planned_nfs [Planned::Nfs]
+      # @param share [String] NFS share name with the format server:path
       # @param partition_section [AutoinstProfile::PartitionSection] partition section describing
       #   a NFS filesystem
-      def add_options(planned_nfs, partition_section)
+      #
+      # @return [Planned::Nfs]
+      def create_planned_nfs(share, partition_section)
+        planned_nfs = Planned::Nfs.new(server(share), path(share))
+
         planned_nfs.mount_point = partition_section.mount
         planned_nfs.fstab_options = partition_section.fstab_options || []
+
+        planned_nfs
       end
 
       # Whether the drive section is valid