Merge pull request #13 from Shopify/refactoring

Refactoring

15 files changed, 73 insertions, 60 deletions

diff --git a/README.md b/README.md
index aea4326..3b7083f 100644
--- a/README.md
+++ b/README.md
@@ -113,7 +113,7 @@ Oxidized supports ```CSV```, ```SQLite``` and ```HTTP``` as source backends. The
 
 Possible outputs are either ```file``` or ```git```. The file backend takes a destination directory as argument and will keep a file per device, with most recent running version of a device. The GIT backend (recommended) will initialize an empty GIT repository in the specified path and create a new commit on every configuration change. Take a look at the [Cookbook](#cookbook) for more details.
 
-Maps define how to map a model's fields to model [model fields](https://github.com/ytti/oxidized/tree/master/lib/oxidized/model). Most of the settings should be self explanatory, log is ignored if Syslog::Logger exists (>=2.0) and syslog is used instead.
+Maps define how to map a model's fields to model [model fields](https://github.com/ytti/oxidized/tree/master/lib/oxidized/model). Most of the settings should be self explanatory, log is ignored if `use_syslog`(requires Ruby >= 2.0) is set to `true`.
 
 First create the directory where the CSV ```output``` is going to store device configs and start Oxidized once.
 ```
diff --git a/bin/oxidized b/bin/oxidized
index e5d9bec..2695a52 100755
--- a/bin/oxidized
+++ b/bin/oxidized
@@ -9,5 +9,5 @@ begin
   Oxidized::CLI.new.run
 rescue => error
   warn "#{error}"
-  raise if Oxidized::CFG.debug
+  raise if Oxidized.config.debug
 end
diff --git a/lib/oxidized.rb b/lib/oxidized.rb
index df30602..e92224a 100644
--- a/lib/oxidized.rb
+++ b/lib/oxidized.rb
@@ -1,6 +1,15 @@
 module Oxidized
   class OxidizedError < StandardError; end
-  Directory = File.expand_path File.join File.dirname(__FILE__), '../'
+
+  Directory = File.expand_path(File.join(File.dirname(__FILE__), '../'))
+
+  require 'oxidized/string'
+  require 'oxidized/config'
+  require 'oxidized/config/vars'
+  require 'oxidized/worker'
+  require 'oxidized/nodes'
+  require 'oxidized/manager'
+  require 'oxidized/hook'
   require 'oxidized/core'
 
   def self.asetus
@@ -14,4 +23,27 @@ module Oxidized
   def self.config
     asetus.cfg
   end
+
+  def self.logger
+    @@logger
+  end
+
+  def self.logger=(val)
+    @@logger = val
+  end
+
+  def self.setup_logger
+    self.logger = if config.has_key?('use_syslog') && config.use_syslog
+                    require 'syslog/logger'
+                    Syslog::Logger.new('oxidized')
+                  else
+                    require 'logger'
+                    if config.has_key?('log')
+                      Logger.new(config.log)
+                    else
+                      Logger.new(STDERR)
+                    end
+                  end
+    logger.level = Logger::INFO unless config.debug
+  end
 end
diff --git a/lib/oxidized/cli.rb b/lib/oxidized/cli.rb
index fd766e3..15d20c5 100644
--- a/lib/oxidized/cli.rb
+++ b/lib/oxidized/cli.rb
@@ -1,13 +1,14 @@
 module Oxidized
   class CLI
-    require 'oxidized'
     require 'slop'
+    require 'oxidized'
 
     def run
       check_pid
       Process.daemon if @opts[:daemonize]
       write_pid
       begin
+        Oxidized.logger.info "Oxidized starting, running as pid #{$$}"
         Oxidized.new
       rescue => error
         crash error
@@ -18,15 +19,16 @@ module Oxidized
     private
 
     def initialize
-      Log.info "Oxidized starting, running as pid #{$$} by #{ENV['USER']}"
       _args, @opts = parse_opts
-      Oxidized.config.debug = true if @opts[:debug]
+
+      Config.load(@opts)
+      Oxidized.setup_logger
+
       @pidfile = File.expand_path("pid")
-      Log.debug "pidfile: #{@pidfile}"
     end
 
     def crash error
-      Log.fatal "Oxidized crashed, crashfile written in #{Config::Crash}"
+      Oxidized.logger.fatal "Oxidized crashed, crashfile written in #{Config::Crash}"
       open Config::Crash, 'w' do |file|
         file.puts '-' * 50
         file.puts Time.now.utc
diff --git a/lib/oxidized/config.rb b/lib/oxidized/config.rb
index 36c1a8f..c850059 100644
--- a/lib/oxidized/config.rb
+++ b/lib/oxidized/config.rb
@@ -12,7 +12,7 @@ module Oxidized
     HookDir   = File.join Directory, %w(lib oxidized hook)
     Sleep     = 1
 
-    def self.load
+    def self.load(cmd_opts={})
       asetus = Asetus.new(name: 'oxidized', load: false, key_to_s: true)
       Oxidized.asetus = asetus
 
@@ -20,7 +20,7 @@ module Oxidized
       asetus.default.password      = 'password'
       asetus.default.model         = 'junos'
       asetus.default.interval      = 3600
-      asetus.default.log           = File.join Config::Root, 'log'
+      asetus.default.use_syslog    = false
       asetus.default.debug         = false
       asetus.default.threads       = 30
       asetus.default.timeout       = 20
@@ -48,9 +48,10 @@ module Oxidized
         raise InvalidConfig, "Error loading config: #{error.message}"
       end
 
-      Log.level = Logger::INFO unless asetus.cfg.debug
       raise NoConfig, 'edit ~/.config/oxidized/config' if asetus.create
-      Log.file = asetus.cfg.log if asetus.cfg.log
+
+      # override if comand line flag given
+      asetus.cfg.debug = cmd_opts[:debug] if cmd_opts[:debug]
 
       asetus
     end
diff --git a/lib/oxidized/core.rb b/lib/oxidized/core.rb
index 1b918d1..d57c5cb 100644
--- a/lib/oxidized/core.rb
+++ b/lib/oxidized/core.rb
@@ -1,12 +1,4 @@
 module Oxidized
-  require 'oxidized/log'
-  require 'oxidized/string'
-  require 'oxidized/config'
-  require 'oxidized/config/vars'
-  require 'oxidized/worker'
-  require 'oxidized/nodes'
-  require 'oxidized/manager'
-  require 'oxidized/hook'
   class << self
     def new *args
       Core.new args
@@ -17,7 +9,6 @@ module Oxidized
     class NoNodesFound < OxidizedError; end
 
     def initialize args
-      Config.load
       Oxidized.mgr = Manager.new
       Oxidized.Hooks = HookManager.from_config(Oxidized.config)
       nodes        = Nodes.new
@@ -28,7 +19,8 @@ module Oxidized
         begin
           require 'oxidized/web'
         rescue LoadError
-          raise OxidizedError, 'oxidized-web not found: sudo gem install oxidized-web - or disable web support by setting "rest: false" in your configuration'
+          raise OxidizedError, 'oxidized-web not found: sudo gem install oxidized-web - \
+          or disable web support by setting "rest: false" in your configuration'
         end
         @rest        = API::Web.new nodes, Oxidized.config.rest
         @rest.run
diff --git a/lib/oxidized/hook.rb b/lib/oxidized/hook.rb
index d774653..029688d 100644
--- a/lib/oxidized/hook.rb
+++ b/lib/oxidized/hook.rb
@@ -46,7 +46,7 @@ class HookManager
     hook.cfg = cfg
 
     @registered_hooks[event] << RegisteredHook.new(name, hook)
-    Log.debug "Hook #{name.inspect} registered #{hook.class} for event #{event.inspect}"
+    Oxidized.logger.debug "Hook #{name.inspect} registered #{hook.class} for event #{event.inspect}"
   end
 
   def handle event, ctx_params={}
@@ -57,7 +57,7 @@ class HookManager
       begin
         r_hook.hook.run_hook ctx
       rescue => e
-        Log.error "Hook #{r_hook.name} (#{r_hook.hook}) failed " +
+        Oxidized.logger.error "Hook #{r_hook.name} (#{r_hook.hook}) failed " +
                   "(#{e.inspect}) for event #{event.inspect}"
       end
     end
@@ -81,7 +81,7 @@ class Hook
   end
 
   def log(msg, level=:info)
-    Log.send(level, "#{self.class.name}: #{msg}")
+    Oxidized.logger.send(level, "#{self.class.name}: #{msg}")
   end
 
 end
diff --git a/lib/oxidized/input/ftp.rb b/lib/oxidized/input/ftp.rb
index a3d7ac8..93cdb38 100644
--- a/lib/oxidized/input/ftp.rb
+++ b/lib/oxidized/input/ftp.rb
@@ -28,7 +28,7 @@ module Oxidized
     end
 
     def cmd file
-      Log.debug "FTP: #{file} @ #{@node.name}"
+      Oxidized.logger.debug "FTP: #{file} @ #{@node.name}"
       @ftp.getbinaryfile file, nil
     end
 
diff --git a/lib/oxidized/input/ssh.rb b/lib/oxidized/input/ssh.rb
index e45adb6..476a786 100644
--- a/lib/oxidized/input/ssh.rb
+++ b/lib/oxidized/input/ssh.rb
@@ -43,7 +43,7 @@ module Oxidized
     end
 
     def cmd cmd, expect=node.prompt
-      Log.debug "SSH: #{cmd} @ #{node.name}"
+      Oxidized.logger.debug "SSH: #{cmd} @ #{node.name}"
       if @exec
         @ssh.exec! cmd
       else
diff --git a/lib/oxidized/input/telnet.rb b/lib/oxidized/input/telnet.rb
index b92d703..e9fd7d9 100644
--- a/lib/oxidized/input/telnet.rb
+++ b/lib/oxidized/input/telnet.rb
@@ -37,7 +37,7 @@ module Oxidized
     end
 
     def cmd cmd, expect=@node.prompt
-      Log.debug "Telnet: #{cmd} @#{@node.name}"
+      Oxidized.logger.debug "Telnet: #{cmd} @#{@node.name}"
       args = { 'String' => cmd }
       args.merge!({ 'Match' => expect, 'Timeout' => @timeout }) if expect
       @telnet.cmd args
diff --git a/lib/oxidized/log.rb b/lib/oxidized/log.rb
deleted file mode 100644
index 04a2d65..0000000
--- a/lib/oxidized/log.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-module Oxidized
-  require 'logger'
-
-  class Logger < Logger
-    def initialize target=STDOUT
-      super target
-    end
-    def file= target
-      FileUtils.mkdir_p File.dirname(target)
-      @logdev = LogDevice.new target
-    end
-  end
-
-  Log = Logger.new
-end
diff --git a/lib/oxidized/node.rb b/lib/oxidized/node.rb
index 1bbf6dc..c16e3d0 100644
--- a/lib/oxidized/node.rb
+++ b/lib/oxidized/node.rb
@@ -62,7 +62,7 @@ module Oxidized
           level = rescue_fail[resc]
           resc  = " (rescued #{resc})"
         end
-        Log.send(level, '%s raised %s%s with msg "%s"' % [self.ip, err.class, resc, err.message])
+        Oxidized.logger.send(level, '%s raised %s%s with msg "%s"' % [self.ip, err.class, resc, err.message])
         return false
       rescue => err
         file = Oxidized::Config::Crash + '.' + self.ip.to_s
@@ -72,7 +72,7 @@ module Oxidized
           fh.puts '-' * 50
           fh.puts err.backtrace
         end
-        Log.error '%s raised %s with msg "%s", %s saved' % [self.ip, err.class, err.message, file]
+        Oxidized.logger.error '%s raised %s with msg "%s", %s saved' % [self.ip, err.class, err.message, file]
         return false
       end
     end
diff --git a/lib/oxidized/nodes.rb b/lib/oxidized/nodes.rb
index f0c5d56..0c02d7a 100644
--- a/lib/oxidized/nodes.rb
+++ b/lib/oxidized/nodes.rb
@@ -1,8 +1,8 @@
 module Oxidized
- require 'ipaddr'
- require 'oxidized/node'
- class Oxidized::NotSupported < OxidizedError; end
- class Oxidized::NodeNotFound < OxidizedError; end
+  require 'ipaddr'
+  require 'oxidized/node'
+  class Oxidized::NotSupported < OxidizedError; end
+  class Oxidized::NodeNotFound < OxidizedError; end
   class Nodes < Array
     attr_accessor :source
     alias :put :unshift
@@ -18,13 +18,13 @@ module Oxidized
             _node = Node.new node
             new.push _node
           rescue ModelNotFound => err
-            Log.error "node %s raised %s with message '%s'" % [node, err.class, err.message]
+            Oxidized.logger.error "node %s raised %s with message '%s'" % [node, err.class, err.message]
           rescue Resolv::ResolvError => err
-            Log.error "node %s is not resolvable, raised %s with message '%s'" % [node, err.class, err.message]
+            Oxidized.logger.error "node %s is not resolvable, raised %s with message '%s'" % [node, err.class, err.message]
           end
         end
         size == 0 ? replace(new) : update_nodes(new)
-	Log.info "Loaded #{size} nodes"
+        Oxidized.logger.info "Loaded #{size} nodes"
       end
     end
 
@@ -148,9 +148,9 @@ module Oxidized
         end
       end
     end
-    
+
     public
-    
+
     def version node, group
       with_lock do
         i = find_node_index node
@@ -159,7 +159,7 @@ module Oxidized
         output.version node, group
       end
     end
-    
+
     def get_version node, group, oid
       with_lock do
         i = find_node_index node
@@ -168,7 +168,7 @@ module Oxidized
         output.get_version node, group, oid
       end
     end
-    
+
     def get_diff node, group, oid1, oid2
       with_lock do
         i = find_node_index node
diff --git a/lib/oxidized/worker.rb b/lib/oxidized/worker.rb
index b36929f..15977e2 100644
--- a/lib/oxidized/worker.rb
+++ b/lib/oxidized/worker.rb
@@ -14,7 +14,7 @@ module Oxidized
       ended.each      { |job| process job }
       @jobs.work
       while @jobs.size < @jobs.want
-        Log.debug "Jobs #{@jobs.size}, Want: #{@jobs.want}"
+        Oxidized.logger.debug "Jobs #{@jobs.size}, Want: #{@jobs.want}"
         # ask for next node in queue non destructive way
         nextnode = @nodes.first
         unless nextnode.last.nil?
@@ -41,7 +41,7 @@ module Oxidized
         msg += " with message '#{node.msg}'" if node.msg
         if node.output.new.store node.name, job.config,
                               :msg => msg, :user => node.user, :group => node.group
-          Log.info "Configuration updated for #{node.group}/#{node.name}"
+          Oxidized.logger.info "Configuration updated for #{node.group}/#{node.name}"
           Oxidized.Hooks.handle :post_store, :node => node,
                                              :job => job
         end
@@ -58,10 +58,10 @@ module Oxidized
           Oxidized.Hooks.handle :node_fail, :node => node,
                                             :job => job
         end
-        Log.warn msg
+        Oxidized.logger.warn msg
       end
     rescue NodeNotFound
-      Log.warn "#{node.name} not found, removed while collecting?"
+      Oxidized.logger.warn "#{node.name} not found, removed while collecting?"
     end
 
   end
diff --git a/spec/githubrepo_spec.rb b/spec/githubrepo_spec.rb
index a3b4953..5786d73 100644
--- a/spec/githubrepo_spec.rb
+++ b/spec/githubrepo_spec.rb
@@ -6,6 +6,7 @@ describe Oxidized::Node do
   before(:each) do
     Oxidized.asetus = Asetus.new
     Oxidized.config.output.git.repo = 'foo.git'
+    Oxidized.setup_logger
 
     @credentials = mock()