Code cleanup for code climate

lib/linux_admin.rb

@@ -16,6 +16,7 @@
 require 'linux_admin/distro'
 require 'linux_admin/system'
 require 'linux_admin/fstab'
+require 'linux_admin/volume'
 require 'linux_admin/logical_volume'
 require 'linux_admin/physical_volume'
 require 'linux_admin/volume_group'

lib/linux_admin/disk.rb

@@ -7,8 +7,27 @@
 
 class LinuxAdmin
   class Disk < LinuxAdmin
+    PARTED_FIELDS =
+      [:id, :start_sector, :end_sector,
+       :size, :partition_type, :fs_type]
+
     attr_accessor :path
 
+    private
+
+    def str_to_bytes(val, unit)
+      case unit
+      when 'K' then
+        val.to_f.kilobytes
+      when 'M' then
+        val.to_f.megabytes
+      when 'G' then
+        val.to_f.gigabytes
+      end
+    end
+
+    public
+
     def self.local
       Dir.glob('/dev/[vhs]d[a-z]').collect do |d|
         Disk.new :path => d
@@ -25,14 +44,7 @@ def size
         out = run!(cmd(:fdisk), :params => {"-l" => nil}).output
         out.each_line { |l|
           if l =~ /Disk #{path}: ([0-9\.]*) ([KMG])B.*/
-            size = case $2
-                   when 'K' then
-                     $1.to_f.kilobytes
-                   when 'M' then
-                     $1.to_f.megabytes
-                   when 'G' then
-                     $1.to_f.gigabytes
-                   end
+            size = str_to_bytes($1, $2)
             break
           end
         }
@@ -41,53 +53,52 @@ def size
     end
 
     def partitions
-      @partitions ||= begin
-        partitions = []
-
-        # TODO: Should this really catch non-zero RC, set output to the default "" and silently return [] ?
-        #   If so, should other calls to parted also do the same?
-        # requires sudo
-        out = run(cmd(:parted),
-                  :params => { nil => [@path, 'print'] }).output
-
-        out.each_line do |l|
-          if l =~ /^ [0-9].*/
-            p = l.split
-            args = {:disk => self}
-            fields = [:id, :start_sector, :end_sector,
-                      :size, :partition_type, :fs_type]
-
-            fields.each_index do |i|
-              val = p[i]
-              case fields[i]
-              when :start_sector, :end_sector, :size
-                if val =~ /([0-9\.]*)([KMG])B/
-                  val = case $2
-                        when 'K' then
-                          $1.to_f.kilobytes
-                        when 'M' then
-                          $1.to_f.megabytes
-                        when 'G' then
-                          $1.to_f.gigabytes
-                        end
-                end
-
-              when :id
-                val = val.to_i
-
-              end
-
-              args[fields[i]] = val
-            end
-            partitions << Partition.new(args)
+      @partitions ||=
+        parted_output.collect { |disk|
+          partition_from_parted(disk)
+        }
+    end
 
-          end
+    private
+
+    def parted_output
+      # TODO: Should this really catch non-zero RC, set output to the default "" and silently return [] ?
+      #   If so, should other calls to parted also do the same?
+      # requires sudo
+      out = run(cmd(:parted),
+                :params => { nil => [@path, 'print'] }).output
+      split = []
+      out.each_line do |l|
+        if l =~ /^ [0-9].*/
+          split << l.split
         end
+      end
+      split
+    end
+
+
+    def partition_from_parted(output_disk)
+      args = {:disk => self}
+      PARTED_FIELDS.each_index do |i|
+        val = output_disk[i]
+        case PARTED_FIELDS[i]
+        when :start_sector, :end_sector, :size
+          if val =~ /([0-9\.]*)([KMG])B/
+            val = str_to_bytes($1, $2)
+          end
+
+        when :id
+          val = val.to_i
 
-        partitions
+        end
+        args[PARTED_FIELDS[i]] = val
       end
+
+      Partition.new(args)
     end
 
+    public
+
     def create_partition_table(type = "msdos")
       run!(cmd(:parted), :params => { nil => [path, "mklabel", type]})
     end

lib/linux_admin/fstab.rb

@@ -13,6 +13,26 @@ class FSTabEntry < LinuxAdmin
     attr_accessor :mount_options
     attr_accessor :dumpable
     attr_accessor :fsck_order
+
+    def initialize(args = {})
+      @device        = args[:device]
+      @mount_point   = args[:mount_point]
+      @fs_type       = args[:fs_type]
+      @mount_options = args[:mount_options]
+      @dumpable      = args[:dumpable]
+      @fsck_order    = args[:fsck_order]
+    end
+
+    def self.from_line(fstab_line)
+      columns = fstab_line.chomp.split
+      FSTabEntry.new :device        => columns[0],
+                     :mount_point   => columns[1],
+                     :fs_type       => columns[2],
+                     :mount_options => columns[3],
+                     :dumpable      => columns[4].to_i,
+                     :fsck_order    => columns[5].to_i
+
+    end
   end
 
   class FSTab < LinuxAdmin
@@ -35,24 +55,21 @@ def write!
 
     private
 
-    def refresh
-      @entries = []
-      f = File.read('/etc/fstab')
-      f.each_line { |line|
-        first_char = line.strip[0] 
-        if first_char != '#' && first_char !~ /\s/
-          columns = line.split
-          entry   = FSTabEntry.new
-          entry.device         = columns[0]
-          entry.mount_point    = columns[1]
-          entry.fs_type        = columns[2]
-          entry.mount_options  = columns[3]
-          entry.dumpable       = columns[4].to_i
-          entry.fsck_order     = columns[5].to_i
-          @entries << entry
-        end
+    def read
+      contents = File.read('/etc/fstab')
+      contents = contents.lines.to_a
+      contents.reject! { |line|
+        first_char = line.strip[0]
+        first_char == '#' || first_char =~ /\s/
       }
-      self
+      contents
+    end
+
+    def refresh
+      @entries = 
+        read.collect { |line|
+          FSTabEntry.from_line line
+        }
     end
   end
 end

lib/linux_admin/logical_volume.rb

@@ -4,7 +4,7 @@
 # Licensed under the MIT License
 
 class LinuxAdmin
-  class LogicalVolume < LinuxAdmin
+  class LogicalVolume < Volume
     # logical volume name
     attr_accessor :name
 
@@ -51,22 +51,11 @@ def self.create(name, vg, size)
 
     def self.scan
       @lvs ||= begin
-        vgs = VolumeGroup.scan
-        lvs = []
-
-        out = run!(cmd(:lvdisplay), :params => { '-c' => nil}).output
-
-        out.each_line do |line|
-          fields = line.split(':')
-          vgname = fields[1] 
-          vg = vgs.find { |vg| vg.name == vgname }
-
-          lvs << LogicalVolume.new(:name         => fields[0],
-                                   :volume_group => vg,
-                                   :sectors      => fields[6].to_i)
+        scan_volumes(cmd(:lvdisplay)) do |fields, vg|
+          LogicalVolume.new(:name         => fields[0],
+                            :volume_group => vg,
+                            :sectors      => fields[6].to_i)
         end
-
-        lvs
       end
     end
   end

lib/linux_admin/physical_volume.rb

@@ -4,7 +4,7 @@
 # Licensed under the MIT License
 
 class LinuxAdmin
-  class PhysicalVolume < LinuxAdmin
+  class PhysicalVolume < Volume
     # physical volume device name
     attr_accessor :device_name
 
@@ -51,22 +51,11 @@ def self.create(device)
 
     def self.scan
       @pvs ||= begin
-        vgs = VolumeGroup.scan
-        pvs = []
-
-        out = run!(cmd(:pvdisplay), :params => { '-c' => nil}).output
-
-        out.each_line do |line|
-          fields = line.split(':')
-          vgname = fields[1]
-          vg = vgs.find { |vg| vg.name == vgname}
-
-          pvs << PhysicalVolume.new(:device_name  => fields[0],
-                                    :volume_group => vg,
-                                    :size         => fields[2].to_i)
-        end
-
-        pvs
+        scan_volumes(cmd(:pvdisplay)) do |fields, vg|
+          PhysicalVolume.new(:device_name  => fields[0],
+                             :volume_group => vg,
+                             :size         => fields[2].to_i)
+         end
       end
     end
   end

lib/linux_admin/service.rb

@@ -7,6 +7,15 @@ class LinuxAdmin
   class Service < LinuxAdmin
     attr_accessor :id
 
+    private
+
+    def systemctl(cmd)
+      run!(cmd(:systemctl),
+          :params => { nil => [cmd, "#{id}.service"] })
+    end
+
+    public
+
     def initialize(id)
       @id = id
     end
@@ -17,14 +26,12 @@ def running?
     end
 
     def enable
-      run!(cmd(:systemctl),
-          :params => { nil => ["enable", "#{id}.service"] })
+      systemctl 'enable'
       self
     end
 
     def disable
-      run!(cmd(:systemctl),
-          :params => { nil => ["disable", "#{id}.service"] })
+      systemctl 'disable'
       self
     end
 

lib/linux_admin/volume.rb

@@ -0,0 +1,33 @@
+# LinuxAdmin Logical Volume Representation
+#
+# Copyright (C) 2013 Red Hat Inc.
+# Licensed under the MIT License
+
+class LinuxAdmin
+  class Volume < LinuxAdmin
+    private
+
+    def self.process_volume_display_line(line)
+      groups = VolumeGroup.scan
+      fields = line.split(':')
+      vgname = fields[1] 
+      vg = groups.find { |vg| vg.name == vgname }
+      return fields, vg 
+    end
+
+    protected
+
+    def self.scan_volumes(cmd)
+      volumes = []
+
+      out = run!(cmd, :params => { '-c' => nil}).output
+
+      out.each_line do |line|
+        fields, vg = process_volume_display_line(line)
+        volumes << yield(fields, vg)
+      end
+
+      volumes
+    end
+  end
+end