Small fixes to RPM module

lib/linux_admin.rb

@@ -16,6 +16,7 @@
 require 'linux_admin/disk'
 require 'linux_admin/hosts'
 require 'linux_admin/partition'
+require 'linux_admin/etc_issue'
 require 'linux_admin/distro'
 require 'linux_admin/system'
 require 'linux_admin/fstab'

lib/linux_admin/common.rb

@@ -3,7 +3,7 @@
 class LinuxAdmin
   module Common
     def cmd(cmd)
-      Distro.local.class::COMMANDS[cmd]
+      Distros::Distro.local.class::COMMANDS[cmd]
     end
 
     def run(cmd, options = {})

lib/linux_admin/distro.rb

@@ -3,35 +3,9 @@
 # Copyright (C) 2013 Red Hat Inc.
 # Licensed under the MIT License
 
-class LinuxAdmin
-  class Distro < LinuxAdmin
-    attr_accessor :id
-
-    def initialize(id)
-      @id = id
-    end
-
-    def self.local
-      @local ||= begin
-        if File.exists?('/etc/issue')
-          issue = File.read('/etc/issue')
-          if issue.include?('ubuntu')
-            return Distros.ubuntu
-          elsif ['fedora', 'red hat', 'centos'].any? { |d| issue.downcase.include?(d) }
-            return Distros.redhat
-          end
-
-        elsif File.exists?('/etc/redhat-release') ||
-              File.exists?('/etc/fedora-release')
-          return Distros.redhat
-        end
-
-        Distros.generic
-      end
-    end
-
-  end
+require 'linux_admin/etc_issue'
 
+class LinuxAdmin
   module Distros
     def self.generic
       @generic ||= Generic.new
@@ -41,6 +15,14 @@ def self.redhat
       @redhat ||= RedHat.new
     end
 
+    def self.rhel
+      @rhel ||= RHEL.new
+    end
+
+    def self.fedora
+      @fedora ||= Fedora.new
+    end
+
     def self.ubuntu
       @ubuntu ||= Ubuntu.new
     end
@@ -49,6 +31,49 @@ def self.all
      @distros ||= [generic, redhat, ubuntu]
     end
 
+    def self.local
+      Distro.local
+    end
+
+    class Distro
+      RELEASE_FILE = ''
+      ETC_ISSUE_KEYWORDS = []
+
+      def self.etc_issue_keywords
+        self::ETC_ISSUE_KEYWORDS
+      end
+
+      def self.release_file
+        self::RELEASE_FILE
+      end
+
+      def self.local
+        # this can be cleaned up..
+        @local ||= begin
+          result = nil
+          Distros.constants.each do |cdistro|
+            distro_method = cdistro.to_s.downcase.to_sym
+            distro = Distros.const_get(cdistro)
+            next unless distro < Distro
+            result = Distros.send(distro_method) if distro.detected?
+          end
+          result || Distros.generic
+        end
+      end
+
+      def self.detected?
+        detected_by_etc_issue? || detected_by_etc_release?
+      end
+
+      def self.detected_by_etc_issue?
+        etc_issue_keywords.any? { |k| EtcIssue.instance.to_s.include?(k) }
+      end
+
+      def self.detected_by_etc_release?
+        File.exists?(release_file)
+      end
+    end
+
     class Generic < Distro
       COMMANDS = {}
 
@@ -81,7 +106,33 @@ def initialize
       end
     end
 
+    class RHEL < RedHat
+      RELEASE_FILE =       "/etc/redhat-release"
+      ETC_ISSUE_KEYWORDS = ['red hat', 'Red Hat', 'centos', 'CentOS']
+
+      COMMANDS = COMMANDS.merge(
+                   :rpm => '/bin/rpm'
+                 )
+      def initialize
+        @id = :rhel
+      end
+    end
+
+    class Fedora < RedHat
+      RELEASE_FILE =       "/etc/fedora-release"
+      ETC_ISSUE_KEYWORDS = ['Fedora']
+
+      COMMANDS = COMMANDS.merge(
+                   :rpm => '/usr/bin/rpm'
+                 )
+      def initialize
+        @id = :fedora
+      end
+    end
+
     class Ubuntu < Distro
+      ETC_ISSUE_KEYWORDS = ['ubuntu']
+
       COMMANDS = {}
 
       def initialize

lib/linux_admin/etc_issue.rb

@@ -0,0 +1,28 @@
+# LinuxAdmin /etc/issue Representation
+#
+# Copyright (C) 2014 Red Hat Inc.
+# Licensed under the MIT License
+
+require 'singleton'
+
+class LinuxAdmin
+  class EtcIssue
+    include Singleton
+
+    PATH = '/etc/issue'
+
+    def initialize
+      refresh
+    end
+
+    def to_s
+      @data.to_s
+    end
+
+    private
+
+    def refresh
+      @data = File.exists?(PATH) ? File.read(PATH) : ""
+    end
+  end
+end

lib/linux_admin/package.rb

@@ -6,9 +6,9 @@
 class LinuxAdmin
   class Package < LinuxAdmin
     def self.info(pkg)
-      if Distro.local == Distros.redhat
+      if Distros::Distro.local == Distros.redhat
         return Rpm.info(pkg)
-      elsif Distro.local == Distros.ubuntu
+      elsif Distros::Distro.local == Distros.ubuntu
         return Deb.info(pkg)
       end
 

lib/linux_admin/rpm.rb

@@ -1,9 +1,11 @@
 class LinuxAdmin
   class Rpm < Package
-    RPM_CMD = '/usr/bin/rpm'
+    def self.rpm_cmd
+      Distros::Distro.local.class::COMMANDS[:rpm]
+    end
 
     def self.list_installed
-      out = run!("rpm -qa --qf \"%{NAME} %{VERSION}-%{RELEASE}\n\"").output
+      out = run!("#{rpm_cmd} -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
@@ -21,8 +23,12 @@ def self.import_key(file)
     def self.info(pkg)
       params = { "-qi" => pkg}
       in_description = false
-      out = run!(RPM_CMD, :params => params).output
+      out = run!(rpm_cmd, :params => params).output
+      # older versions of rpm may have multiple fields per line,
+      # split up lines with multiple tags/values:
+      out.gsub!(/(^.*:.*)\s\s+(.*:.*)$/, "\\1\n\\2")
       out.split("\n").each.with_object({}) do |line, rpm|
+        next if !line || line.empty?
         tag,value = line.split(':')
         tag = tag.strip
         if tag == 'Description'

spec/common_spec.rb

@@ -15,9 +15,9 @@ class TestClass
 
   context "#cmd" do
     it "looks up local command from id" do
-      d = double(LinuxAdmin::Distro)
+      d = double(LinuxAdmin::Distros::Distro)
       d.class::COMMANDS = {:sh => '/bin/sh'}
-      LinuxAdmin::Distro.should_receive(:local).and_return(d)
+      LinuxAdmin::Distros::Distro.should_receive(:local).and_return(d)
       subject.cmd(:sh).should == '/bin/sh'
     end
   end

spec/distro_spec.rb

@@ -1,53 +1,53 @@
 require 'spec_helper'
 
-describe LinuxAdmin::Distro do
+describe LinuxAdmin::Distros::Distro do
   describe "#local" do
-    after(:each) do
-      # distro generates a local copy, reset after each run
-      LinuxAdmin::Distro.instance_variable_set(:@local, nil)
-    end
-
     [['ubuntu',  :ubuntu],
-     ['Fedora',  :redhat],
-     ['red hat', :redhat],
-     ['CentOS',  :redhat],
-     ['centos',  :redhat]].each do |i,d|
+     ['Fedora',  :fedora],
+     ['red hat', :rhel],
+     ['CentOS',  :rhel],
+     ['centos',  :rhel]].each do |i, d|
       context "/etc/issue contains '#{i}'" do
         before(:each) do
-          File.should_receive(:exists?).with('/etc/issue').and_return(true)
-          File.should_receive(:read).with('/etc/issue').and_return(i)
+          LinuxAdmin::EtcIssue.instance.should_receive(:to_s).at_least(:once).and_return(i)
+          File.should_receive(:exists?).at_least(:once).and_return(false)
         end
 
         it "returns Distros.#{d}" do
-          LinuxAdmin::Distro.local.should == LinuxAdmin::Distros.send(d)
+          distro = LinuxAdmin::Distros.send(d)
+          described_class.local.should == distro
         end
       end
     end
 
     context "/etc/issue did not match" do
       before(:each) do
-        File.should_receive(:exists?).with('/etc/issue').and_return(false)
+        LinuxAdmin::EtcIssue.instance.should_receive(:to_s).at_least(:once).and_return('')
       end
 
       context "/etc/redhat-release exists" do
-        it "returns Distros.redhat" do
+        it "returns Distros.rhel" do
           File.should_receive(:exists?).with('/etc/redhat-release').and_return(true)
-          LinuxAdmin::Distro.local.should == LinuxAdmin::Distros.redhat
+          LinuxAdmin::Distros::Fedora.should_receive(:detected?).and_return(false)
+          File.should_receive(:exists?).at_least(:once).and_call_original
+          described_class.local.should == LinuxAdmin::Distros.rhel
         end
       end
 
       context "/etc/fedora-release exists" do
-        it "returns Distros.redhat" do
+        it "returns Distros.fedora" do
           File.should_receive(:exists?).with('/etc/redhat-release').and_return(false)
           File.should_receive(:exists?).with('/etc/fedora-release').and_return(true)
-          LinuxAdmin::Distro.local.should == LinuxAdmin::Distros.redhat
+          File.should_receive(:exists?).at_least(:once).and_call_original
+          described_class.local.should == LinuxAdmin::Distros.fedora
         end
       end
     end
 
     it "returns Distros.generic" do
-      File.stub(:exists?).and_return(false)
-      LinuxAdmin::Distro.local.should == LinuxAdmin::Distros.generic
+      LinuxAdmin::EtcIssue.instance.should_receive(:to_s).at_least(:once).and_return('')
+      File.should_receive(:exists?).at_least(:once).and_return(false)
+      described_class.local.should == LinuxAdmin::Distros.generic
     end
   end
 end

spec/logical_volume_spec.rb

@@ -2,7 +2,7 @@
 
 describe LinuxAdmin::LogicalVolume do
   before(:each) do
-    LinuxAdmin::Distro.stub(:local => LinuxAdmin::Distros::Test.new)
+    LinuxAdmin::Distros::Distro.stub(:local => LinuxAdmin::Distros::Test.new)
 
     @logical_volumes = <<eos
   /dev/vg_foobar/lv_swap:vg_foobar:3:1:-1:2:4128768:63:-1:0:-1:253:0

spec/package_spec.rb

@@ -3,13 +3,13 @@
 describe LinuxAdmin::Package do
   describe "#info" do
     it "dispatches to redhat lookup mechanism" do
-      LinuxAdmin::Distro.should_receive(:local).and_return(LinuxAdmin::Distros.redhat)
+      LinuxAdmin::Distros::Distro.should_receive(:local).and_return(LinuxAdmin::Distros.redhat)
       LinuxAdmin::Rpm.should_receive(:info).with('ruby')
       described_class.info 'ruby'
     end
 
     it "dispatches to ubuntu lookup mechanism" do
-      LinuxAdmin::Distro.should_receive(:local).twice.and_return(LinuxAdmin::Distros.ubuntu)
+      LinuxAdmin::Distros::Distro.should_receive(:local).twice.and_return(LinuxAdmin::Distros.ubuntu)
       LinuxAdmin::Deb.should_receive(:info).with('ruby')
       described_class.info 'ruby'
     end

spec/physical_volume_spec.rb

@@ -2,7 +2,7 @@
 
 describe LinuxAdmin::PhysicalVolume do
   before(:each) do
-    LinuxAdmin::Distro.stub(:local => LinuxAdmin::Distros::Test.new)
+    LinuxAdmin::Distros::Distro.stub(:local => LinuxAdmin::Distros::Test.new)
 
     @physical_volumes = <<eos
   /dev/vda2:vg_foobar:24139776:-1:8:8:-1:32768:368:0:368:pxR32D-YkC2-PfHe-zOwb-eaGD-9Ar0-mAOl9u

spec/rpm_spec.rb

@@ -59,9 +59,9 @@
 files and to do system management tasks (as in Perl).  It is simple,
 straight-forward, and extensible.
 EOS
-      described_class.should_receive(:run!).
-                      with(described_class::RPM_CMD, :params => {"-qi" => "ruby"}).
-                      and_return(double(:output => data))
+      arguments = [described_class.rpm_cmd, :params => {"-qi" => "ruby"}]
+      result = AwesomeSpawn::CommandResult.new("", data, "", 0)
+      described_class.should_receive(:run!).with(*arguments).and_return(result)
       metadata = described_class.info("ruby")
       metadata['name'].should == 'ruby'
       metadata['version'].should == '2.0.0.247'

spec/service_spec.rb

@@ -3,8 +3,8 @@
 describe LinuxAdmin::Service do
   before(:each) do
     # stub distro.local to return test distro for command lookup
-    LinuxAdmin::Distro.stub(:local).
-                       and_return(LinuxAdmin::Distros::Test.new)
+    LinuxAdmin::Distros::Distro.stub(:local)
+                               .and_return(LinuxAdmin::Distros::Test.new)
 
     @service = LinuxAdmin::Service.new 'foo'
   end

spec/spec_helper.rb

@@ -21,6 +21,9 @@
 
   config.after do
     clear_caches
+
+    # reset the distro, tested in various placed & used extensively
+    LinuxAdmin::Distros::Distro.instance_variable_set(:@local, nil)
   end
 end
 

spec/volume_group_spec.rb

@@ -2,7 +2,7 @@
 
 describe LinuxAdmin::VolumeGroup do
   before(:each) do
-    LinuxAdmin::Distro.stub(:local => LinuxAdmin::Distros::Test.new)
+    LinuxAdmin::Distros::Distro.stub(:local => LinuxAdmin::Distros::Test.new)
 
     @groups = <<eos
   vg_foobar:r/w:772:-1:0:2:2:-1:0:1:1:12058624:32768:368:368:0:tILZUF-IspH-H90I-pT5j-vVFl-b76L-zWx3CW