Change LinuxAdmin.run to return a CommandResult object.

lib/linux_admin.rb

@@ -5,6 +5,7 @@
 
 require 'linux_admin/common'
 require 'linux_admin/exceptions'
+require 'linux_admin/command_result'
 require 'linux_admin/rpm'
 require 'linux_admin/version'
 require 'linux_admin/yum'

lib/linux_admin/command_result.rb

@@ -0,0 +1,9 @@
+class CommandResult
+  attr_reader :output, :error, :exit_status
+
+  def initialize(output, error, exit_status)
+    @output      = output
+    @error       = error
+    @exit_status = exit_status
+  end
+end

lib/linux_admin/common.rb

@@ -9,24 +9,35 @@ def cmd(cmd)
     def run(cmd, options = {})
       params = options[:params] || options[:parameters]
 
+      launch_params = {}
+      launch_params[:chdir] = options[:chdir] if options[:chdir]
+
+      output = ""
+      error  = ""
+      status = nil
+
       begin
-        launch_params = {}
-        launch_params[:chdir] = options[:chdir] if options[:chdir]
-        out = launch(build_cmd(cmd, params), launch_params)
-
-        if options[:return_output] && exitstatus == 0
-          out
-        elsif options[:return_exitstatus] || exitstatus == 0
-          exitstatus
-        else
-          raise CommandError, "#{build_cmd(cmd, params)}: exit code: #{exitstatus}"
-        end
-      rescue
-        return nil if options[:return_exitstatus]
-        raise
+        output, error = launch(build_cmd(cmd, params), launch_params)
+        status = exitstatus
       ensure
+        output ||= ""
+        error  ||= ""
         self.exitstatus = nil
       end
+
+      CommandResult.new(output, error, status)
+    end
+
+    def run!(cmd, options = {})
+      params = options[:params] || options[:parameters]
+      command_result = run(cmd, options)
+
+      if command_result.exit_status != 0
+        message = "#{cmd} exit code: #{command_result.exit_status}"
+        raise CommandResultError.new(message, command_result)
+      end
+
+      command_result
     end
 
     private
@@ -61,25 +72,28 @@ def build_cmd(cmd, params = nil)
     THREAD_SYNC_KEY = "LinuxAdmin-exitstatus"
 
     def launch(cmd, spawn_options = {})
-      pipe_r, pipe_w = IO.pipe
-      pid = Kernel.spawn(cmd, {:err => [:child, :out], :out => pipe_w}.merge(spawn_options))
-      wait_for_process(pid, pipe_w)
-      wait_for_output(pipe_r)
+      out_r, out_w = IO.pipe
+      err_r, err_w = IO.pipe
+      pid = Kernel.spawn(cmd, {:err => err_w, :out => out_w}.merge(spawn_options))
+      wait_for_process(pid, out_w, err_w)
+      wait_for_pipes(out_r, err_r)
     end
 
-    def wait_for_process(pid, pipe_w)
+    def wait_for_process(pid, out_w, err_w)
       self.exitstatus = :not_done
       Thread.new(Thread.current) do |parent_thread|
         _, status = Process.wait2(pid)
-        pipe_w.close
+        out_w.close
+        err_w.close
         parent_thread[THREAD_SYNC_KEY] = status.exitstatus
       end
     end
 
-    def wait_for_output(pipe_r)
-      out = pipe_r.read
+    def wait_for_pipes(out_r, err_r)
+      out = out_r.read
+      err = err_r.read
       sleep(0.1) while exitstatus == :not_done
-      return out
+      return out, err
     end
 
     def exitstatus

lib/linux_admin/disk.rb

@@ -22,9 +22,7 @@ def initialize(args = {})
     def size
       @size ||= begin
         size = nil
-        out = run(cmd(:fdisk),
-                  :return_output => true,
-                  :params => {"-l" => nil})
+        out = run!(cmd(:fdisk), :params => {"-l" => nil}).output
         out.each_line { |l|
           if l =~ /Disk #{path}: ([0-9\.]*) ([KMG])B.*/
             size = case $2
@@ -46,13 +44,11 @@ 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),
-                  :return_exitstatus => true,
-                  :return_output => true,
-                  :params => { nil => [@path, 'print'] })
-
-        return [] if out.kind_of?(Fixnum)
+                  :params => { nil => [@path, 'print'] }).output
 
         out.each_line do |l|
           if l =~ /^ [0-9].*/
@@ -98,7 +94,7 @@ def create_partition(partition_type, size)
           [(partitions.last.id + 1),
             partitions.last.end_sector]
 
-      run(cmd(:parted),
+      run!(cmd(:parted),
           :params => { nil => [path, 'mkpart', partition_type,
                                start, start + size]})
 
@@ -116,7 +112,7 @@ def clear!
       @partitions = []
 
       # clear partition table
-      run(cmd(:dd),
+      run!(cmd(:dd),
           :params => { 'if=' => '/dev/zero', 'of=' => @path,
                        'bs=' => 512, 'count=' => 1})
 

lib/linux_admin/exceptions.rb

@@ -1 +1,8 @@
-  class CommandError < RuntimeError; end
+class CommandResultError < StandardError
+  attr_reader :result
+
+  def initialize(message, result)
+    super(message)
+    @result = result
+  end
+end

lib/linux_admin/logical_volume.rb

@@ -33,14 +33,14 @@ def initialize(args = {})
     end
 
     def extend_with(vg)
-      run(cmd(:lvextend),
+      run!(cmd(:lvextend),
           :params => [self.name, vg.name])
       self
     end
 
     def self.create(name, vg, size)
       self.scan # initialize local logical volumes
-      run(cmd(:lvcreate),
+      run!(cmd(:lvcreate),
           :params => { '-n' => name, nil => vg.name, '-L' => size})
       lv = LogicalVolume.new :name => name,
                              :volume_group => vg,
@@ -54,9 +54,7 @@ def self.scan
         vgs = VolumeGroup.scan
         lvs = []
 
-        out = run(cmd(:lvdisplay),
-                  :return_output => true,
-                  :params => { '-c' => nil})
+        out = run!(cmd(:lvdisplay), :params => { '-c' => nil}).output
 
         out.each_line do |line|
           fields = line.split(':')

lib/linux_admin/partition.rb

@@ -31,7 +31,7 @@ def path
     end
 
     def format_to(filesystem)
-      run(cmd(:mke2fs),
+      run!(cmd(:mke2fs),
           :params => { '-t' => filesystem, nil => self.path})
       @fs_type = filesystem
     end
@@ -42,12 +42,12 @@ def mount(mount_point=nil)
         "/mnt/#{disk.path.split(File::SEPARATOR).last}#{id}" if mount_point.nil?
       FileUtils.mkdir(@mount_point) unless File.directory?(@mount_point)
 
-      run(cmd(:mount),
+      run!(cmd(:mount),
           :params => { nil => [self.path, @mount_point] })
     end
 
     def umount
-      run(cmd(:umount),
+      run!(cmd(:umount),
           :params => { nil => [@mount_point] })
     end
   end

lib/linux_admin/physical_volume.rb

@@ -31,7 +31,7 @@ def initialize(args = {})
     end
 
     def attach_to(vg)
-      run(cmd(:vgextend),
+      run!(cmd(:vgextend),
           :params => [vg.name, @device_name])
       self.volume_group = vg
       self
@@ -40,7 +40,7 @@ def attach_to(vg)
     # specify disk or partition instance to create physical volume on
     def self.create(device)
       self.scan # initialize local physical volumes
-      run(cmd(:pvcreate),
+      run!(cmd(:pvcreate),
           :params => { nil => device.path})
       pv  = PhysicalVolume.new(:device_name  => device.path,
                                :volume_group => nil,
@@ -54,9 +54,7 @@ def self.scan
         vgs = VolumeGroup.scan
         pvs = []
 
-        out = run(cmd(:pvdisplay),
-                  :return_output => true,
-                  :params => { '-c' => nil})
+        out = run!(cmd(:pvdisplay), :params => { '-c' => nil}).output
 
         out.each_line do |line|
           fields = line.split(':')

lib/linux_admin/registration_system/rhn.rb

@@ -29,7 +29,7 @@ def register(options)
       params["--proxyPassword="]  = options[:proxy_password]  if options[:proxy_password]
       params["--serverUrl="]      = options[:server_url]      if options[:server_url]
 
-      run(cmd, :params => params)
+      run!(cmd, :params => params)
     end
 
     def subscribe(options)
@@ -43,7 +43,7 @@ def subscribe(options)
       params["--password="] = options[:password]
       params                = params.to_a + pools
 
-      run(cmd, :params => params)
+      run!(cmd, :params => params)
     end
 
     private

lib/linux_admin/registration_system/subscription_manager.rb

@@ -8,11 +8,11 @@ def validate_credentials(options)
     end
 
     def registered?
-      run("subscription-manager identity", :return_exitstatus => true) == 0
+      run("subscription-manager identity").exit_status == 0
     end
 
     def refresh
-      run("subscription-manager refresh")
+      run!("subscription-manager refresh")
     end
 
     def organizations(options)
@@ -23,7 +23,7 @@ def organizations(options)
       params.merge!(proxy_params(options))
       params["--serverurl="]  = options[:server_url]  if options[:server_url]
 
-      output = run(cmd, :params => params, :return_output => true)
+      output = run!(cmd, :params => params).output
       parse_output(output).index_by {|i| i[:name]}
     end
 
@@ -36,20 +36,20 @@ def register(options)
       params["--org="]        = options[:org]         if options[:server_url] && options[:org]
       params["--serverurl="]  = options[:server_url]  if options[:server_url]
 
-      run(cmd, :params => params)
+      run!(cmd, :params => params)
     end
 
     def subscribe(options)
       cmd    = "subscription-manager attach"
       pools  = options[:pools].collect {|pool| ["--pool", pool]}
       params = proxy_params(options).to_a + pools
 
-      run(cmd, :params => params)
+      run!(cmd, :params => params)
     end
 
     def available_subscriptions
       cmd     = "subscription-manager list --all --available"
-      output  = run(cmd, :return_output => true)
+      output  = run!(cmd).output
       parse_output(output).index_by {|i| i[:pool_id]}
     end
 

lib/linux_admin/rpm.rb

@@ -1,7 +1,7 @@
 class LinuxAdmin
   class Rpm < LinuxAdmin
     def self.list_installed
-      out = run("rpm -qa --qf \"%{NAME} %{VERSION}-%{RELEASE}\n\"", :return_output => true)
+      out = run!("rpm -qa --qf \"%{NAME} %{VERSION}-%{RELEASE}\n\"").output
       out.split("\n").each_with_object({}) do |line, pkg_hash|
         name, ver = line.split(" ")
         pkg_hash[name] = ver

lib/linux_admin/service.rb

@@ -13,39 +13,37 @@ def initialize(id)
 
     def running?
       run(cmd(:service),
-          :params => { nil => [id, "status"] },
-          :return_exitstatus => true) == 0
+          :params => { nil => [id, "status"] }).exit_status == 0
     end
 
     def enable
-      run(cmd(:systemctl),
+      run!(cmd(:systemctl),
           :params => { nil => ["enable", "#{id}.service"] })
       self
     end
 
     def disable
-      run(cmd(:systemctl),
+      run!(cmd(:systemctl),
           :params => { nil => ["disable", "#{id}.service"] })
       self
     end
 
     def start
-      run(cmd(:service),
+      run!(cmd(:service),
           :params => { nil => [id, "start"] })
       self
     end
 
     def stop
-      run(cmd(:service),
+      run!(cmd(:service),
           :params => { nil => [id, "stop"] })
       self
     end
 
     def restart
       status =
         run(cmd(:service),
-          :params => { nil => [id, "restart"] },
-          :return_exitstatus => true)
+          :params => { nil => [id, "restart"] }).exit_status
 
       # attempt to manually stop/start if restart fails
       if status != 0

lib/linux_admin/system.rb

@@ -6,12 +6,12 @@
 class LinuxAdmin
   class System < LinuxAdmin
     def self.reboot!
-      run(cmd(:shutdown),
+      run!(cmd(:shutdown),
           :params => { "-r" => "now" })
     end
 
     def self.shutdown!
-      run(cmd(:shutdown),
+      run!(cmd(:shutdown),
           :params => { "-h" => "0" })
     end
   end