commit 335a1610fae8cd74720e2d9841d904deb8ff1881
Author: Lukas Zapletal <lzap+git@redhat.com>
Date:   Tue Aug 11 15:44:39 2015 +0200

    Fixes #11323 - fixed PID writing, interrupt trap and daemon logging

diff --git a/config/settings.yml.example b/config/settings.yml.example
index 42e8def..23709a9 100644
--- a/config/settings.yml.example
+++ b/config/settings.yml.example
@@ -51,7 +51,7 @@
 :virsh_network: default
 
 # Log configuration
-# Uncomment and modify if you want to change the location of the log file or use STDOUT
+# Uncomment and modify if you want to change the location of the log file or use STDOUT or SYSLOG values
 #:log_file: /var/log/foreman-proxy/proxy.log
 # Uncomment and modify if you want to change the log level
 # WARN, DEBUG, ERROR, FATAL, INFO, UNKNOWN
diff --git a/lib/launcher.rb b/lib/launcher.rb
new file mode 100644
index 0000000..a98a258
--- /dev/null
+++ b/lib/launcher.rb
@@ -0,0 +1,144 @@
+require 'proxy/log'
+require 'proxy/settings'
+
+module Proxy
+  class Launcher
+    include ::Proxy::Log
+
+    def pid_path
+      SETTINGS.daemon_pid
+    end
+
+    def https_enabled?
+      SETTINGS.ssl_private_key && SETTINGS.ssl_certificate && SETTINGS.ssl_ca_file
+    end
+
+    def http_app
+      return nil if SETTINGS.http_port.nil?
+      app = Rack::Builder.new do
+        ::Proxy::Plugins.enabled_plugins.each do |p|
+          instance_eval(p.http_rackup)
+        end
+      end
+
+      Rack::Server.new(
+        :app => app,
+        :server => :webrick,
+        :Host => SETTINGS.bind_host,
+        :Port => SETTINGS.http_port,
+        :daemonize => false)
+    end
+
+    def https_app
+      unless https_enabled?
+        logger.warn "Missing SSL setup, https is disabled."
+        nil
+      else
+        app = Rack::Builder.new do
+          ::Proxy::Plugins.enabled_plugins.each {|p| instance_eval(p.https_rackup)}
+        end
+
+        Rack::Server.new(
+          :app => app,
+          :server => :webrick,
+          :Host => SETTINGS.bind_host,
+          :Port => SETTINGS.https_port,
+          :SSLEnable => true,
+          :SSLVerifyClient => OpenSSL::SSL::VERIFY_PEER,
+          :SSLPrivateKey => load_ssl_private_key(SETTINGS.ssl_private_key),
+          :SSLCertificate => load_ssl_certificate(SETTINGS.ssl_certificate),
+          :SSLCACertificateFile => SETTINGS.ssl_ca_file,
+          :daemonize => false)
+      end
+    end
+
+    def load_ssl_private_key(path)
+      OpenSSL::PKey::RSA.new(File.read(path))
+    rescue Exception => e
+      logger.error "Unable to load private SSL key. Are the values correct in settings.yml and do permissions allow reading?: #{e}"
+      raise e
+    end
+
+    def load_ssl_certificate(path)
+      OpenSSL::X509::Certificate.new(File.read(path))
+    rescue Exception => e
+      logger.error "Unable to load SSL certificate. Are the values correct in settings.yml and do permissions allow reading?: #{e}"
+      raise e
+    end
+
+    def pid_status
+      return :exited unless File.exist?(pid_path)
+      pid = ::File.read(pid_path).to_i
+      return :dead if pid == 0
+      Process.kill(0, pid)
+      :running
+    rescue Errno::ESRCH
+      :dead
+    rescue Errno::EPERM
+      :not_owned
+    end
+
+    def check_pid
+      case pid_status
+      when :running, :not_owned
+        logger.error "A server is already running. Check #{pid_path}"
+        exit(2)
+      when :dead
+        File.delete(pid_path)
+      end
+    end
+
+    def write_pid
+      FileUtils.mkdir_p(File.dirname(pid_path)) unless File.exist?(pid_path)
+      File.open(pid_path, ::File::CREAT | ::File::EXCL | ::File::WRONLY){|f| f.write("#{Process.pid}") }
+      at_exit { File.delete(pid_path) if File.exist?(pid_path) }
+    rescue Errno::EEXIST
+      check_pid
+      retry
+    end
+
+    def launch
+      ::Proxy::Plugins.configure_loaded_plugins
+
+      http_app = http_app()
+      https_app = https_app()
+      raise Exception.new("Both http and https are disabled, unable to start.") if http_app.nil? && https_app.nil?
+
+      if SETTINGS.daemon
+        check_pid
+        Process.daemon
+        write_pid
+      end
+
+      t1 = Thread.new { https_app.start } unless https_app.nil?
+      t2 = Thread.new { http_app.start } unless http_app.nil?
+
+      begin
+        if Gem.loaded_specs['rack'].version < Gem::Version.create('1.6.4')
+          # Rack installs its own trap; Sleeping for 5 secs insures we overwrite it with our own
+          sleep 5
+          trap(:INT) do
+            exit(0)
+          end
+        end
+      rescue Exception => e
+        logger.warn "Unable to overwrite interrupt trap: #{e}"
+      end
+
+      (t1 || t2).join
+    rescue SignalException => e
+      # This is to prevent the exception handler below from catching SignalException exceptions.
+      logger.info("Caught #{e}. Exiting")
+      raise
+    rescue SystemExit
+      # do nothing. This is to prevent the exception handler below from catching SystemExit exceptions.
+      raise
+    rescue Exception => e
+      logger.error("Error during startup, terminating. #{e}")
+      logger.debug("#{e}:#{e.backtrace.join("\n")}")
+
+      puts "Errors detected on startup, see log for details. Exiting: #{e}"
+      exit(1)
+    end
+  end
+end
diff --git a/lib/proxy/log.rb b/lib/proxy/log.rb
index b8934fc..a7d2164 100644
--- a/lib/proxy/log.rb
+++ b/lib/proxy/log.rb
@@ -1,6 +1,13 @@
 require 'logger'
+begin
+  require 'syslog/logger'
+  ::Syslog::Logger.class_eval { alias_method :write, :info }
+rescue LoadError
+  puts "Setting log_file=SYSLOG not supported on this platform, ignoring"
+end
 
-Logger.class_eval { alias_method :write, :'<<' } # ::Rack::CommonLogger expects loggers to implement 'write' method
+# ::Rack::CommonLogger expects loggers to implement 'write' method
+Logger.class_eval { alias_method :write, :info }
 
 module Proxy
   module Log
@@ -10,13 +17,27 @@ module Proxy
       @@logger ||= ::Proxy::Log.logger
     end
 
+    def self.default_logger(log_file)
+      # We keep the last 6 10MB log files
+      ::Logger.new(log_file, 6, 1024*1024*10)
+    end
+
     def self.logger
       log_file = ::Proxy::SETTINGS.log_file
       if log_file.upcase == 'STDOUT'
+        if SETTINGS.daemon
+          puts "Settings log_file=STDOUT and daemon=true are incompatible, exiting..."
+          exit 1
+        end
         logger = ::Logger.new(STDOUT)
+      elsif log_file.upcase == 'SYSLOG'
+        begin
+          logger = ::Syslog::Logger.new 'foreman-proxy'
+        rescue
+          logger = default_logger(log_file)
+        end
       else
-        # We keep the last 6 10MB log files
-        logger = ::Logger.new(log_file, 6, 1024*1024*10)
+        logger = default_logger(log_file)
       end
       logger.level = ::Logger.const_get(::Proxy::SETTINGS.log_level.upcase)
       logger
diff --git a/lib/smart_proxy.rb b/lib/smart_proxy.rb
index 1b1d6d9..bb462ef 100644
--- a/lib/smart_proxy.rb
+++ b/lib/smart_proxy.rb
@@ -1,12 +1,13 @@
 APP_ROOT = "#{File.dirname(__FILE__)}/.."
 
 require 'proxy'
+require 'launcher'
 
 require 'fileutils'
 require 'pathname'
 require 'checks'
 require 'webrick/https'
-require 'daemon' # FIXME: Do we still need this?
+require 'daemon'
 
 require 'checks'
 require 'proxy/log'
@@ -63,111 +64,4 @@ module Proxy
   def self.version
     {:version => VERSION}
   end
-
-  class Launcher
-    include ::Proxy::Log
-
-    def pid_path
-      SETTINGS.daemon_pid
-    end
-
-    def create_pid_dir
-      if SETTINGS.daemon
-        FileUtils.mkdir_p(File.dirname(pid_path)) unless File.exist?(pid_path)
-      end
-    end
-
-    def https_enabled?
-      SETTINGS.ssl_private_key && SETTINGS.ssl_certificate && SETTINGS.ssl_ca_file
-    end
-
-    def http_app
-      return nil if SETTINGS.http_port.nil?
-      app = Rack::Builder.new do
-        ::Proxy::Plugins.enabled_plugins.each do |p|
-          instance_eval(p.http_rackup)
-        end
-      end
-
-      Rack::Server.new(
-        :app => app, 
-        :server => :webrick,
-        :Host => SETTINGS.bind_host,
-        :Port => SETTINGS.http_port,
-        :daemonize => false,
-        :pid => (SETTINGS.daemon && !https_enabled?) ? pid_path : nil)
-    end
-
-    def https_app
-      unless https_enabled?
-        logger.warn "Missing SSL setup, https is disabled."
-        nil
-      else
-        app = Rack::Builder.new do
-          ::Proxy::Plugins.enabled_plugins.each {|p| instance_eval(p.https_rackup)}
-        end
-
-        Rack::Server.new(
-          :app => app,
-          :server => :webrick,
-          :Host => SETTINGS.bind_host,
-          :Port => SETTINGS.https_port,
-          :SSLEnable => true,
-          :SSLVerifyClient => OpenSSL::SSL::VERIFY_PEER,
-          :SSLPrivateKey => load_ssl_private_key(SETTINGS.ssl_private_key),
-          :SSLCertificate => load_ssl_certificate(SETTINGS.ssl_certificate),
-          :SSLCACertificateFile => SETTINGS.ssl_ca_file,
-          :daemonize => false,
-          :pid => SETTINGS.daemon ? pid_path : nil)
-      end
-    end
-
-    def load_ssl_private_key(path)
-      OpenSSL::PKey::RSA.new(File.read(path))
-    rescue Exception => e
-      logger.error "Unable to load private SSL key. Are the values correct in settings.yml and do permissions allow reading?: #{e}"
-      raise e
-    end
-
-    def load_ssl_certificate(path)
-      OpenSSL::X509::Certificate.new(File.read(path))
-    rescue Exception => e
-      logger.error "Unable to load SSL certificate. Are the values correct in settings.yml and do permissions allow reading?: #{e}"
-      raise e
-    end
-
-    def launch
-      ::Proxy::Plugins.configure_loaded_plugins
-
-      create_pid_dir
-      http_app = http_app()
-      https_app = https_app()
-      raise Exception.new("Both http and https are disabled, unable to start.") if http_app.nil? && https_app.nil?
-
-      Process.daemon if SETTINGS.daemon
-
-      t1 = Thread.new { https_app.start } unless https_app.nil?
-      t2 = Thread.new { http_app.start } unless http_app.nil?
-
-      sleep 5 # Rack installs its own trap; Sleeping for 5 secs insures we overwrite it with our own
-      trap(:INT) do
-        exit(0)
-      end
-
-      (t1 || t2).join
-    rescue SignalException => e
-      # This is to prevent the exception handler below from catching SignalException exceptions.
-      logger.info("Caught #{e}. Exiting")
-      raise
-    rescue SystemExit
-      # do nothing. This is to prevent the exception handler below from catching SystemExit exceptions.
-      raise
-    rescue Exception => e
-      logger.error("Error during startup, terminating. #{e}")
-      logger.debug("#{e}:#{e.backtrace.join("\n")}")
-
-      puts "Errors detected on startup, see log for details. Exiting."
-      exit(1)
-    end
-  end
 end
diff --git a/test/launcher_test.rb b/test/launcher_test.rb
new file mode 100644
index 0000000..1704917
--- /dev/null
+++ b/test/launcher_test.rb
@@ -0,0 +1,42 @@
+require 'test_helper'
+require 'launcher'
+
+class LauncherTest < Test::Unit::TestCase
+  def setup
+    @launcher = Proxy::Launcher.new
+    @launcher.stubs(:pid_path).returns("launcher_test.pid")
+  end
+
+  def test_pid_status_exited
+    assert_equal :exited, @launcher.pid_status
+  end
+
+  def test_write_pid_success
+    @launcher.write_pid
+    assert File.exist?(@launcher.pid_path)
+  ensure
+    FileUtils.rm_f @launcher.pid_path
+  end
+
+  def test_pid_status_running
+    @launcher.write_pid
+    assert_equal :running, @launcher.pid_status
+  ensure
+    FileUtils.rm_f @launcher.pid_path
+  end
+
+  def test_check_pid_deletes_dead
+    Process.stubs(:kill).returns { raise Errno::ESRCH }
+    @launcher.check_pid
+    assert_equal false, File.exist?(@launcher.pid_path)
+  end
+
+  def test_check_pid_exits_program
+    @launcher.write_pid
+    assert_raises SystemExit do
+      @launcher.check_pid
+    end
+  ensure
+    FileUtils.rm_f @launcher.pid_path
+  end
+end
