Remove the stale puma pidfile before starting the server process

CHANGELOG.md

@@ -47,6 +47,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed
 - IAM Authn bug fix - Take rexml gem to production configuration [#2493](https://github.com/cyberark/conjur/pull/2493)
+- Previously, a stale puma pid file would prevent the Conjur server from starting 
+ successfully. Conjur now removes a stale pid file at startup, if it exists.
+ [#2498](https://github.com/cyberark/conjur/pull/2498)
 
 ### Security
 - Updated nokogiri to 1.13.3 to resolve CVE-2022-23308 and CVE-2021-30560

bin/conjur-cli/commands/server.rb

@@ -31,6 +31,9 @@ def call
  create_account
  load_bootstrap_policy
 
+ # Remove a stale puma PID file, if it exists
+ cleanup_pidfile
+
  # Start the Conjur API and service
  # processes
  fork_server_process
@@ -85,6 +88,24 @@ def load_bootstrap_policy
  ) || exit(($CHILD_STATUS.exitstatus))
  end
 
+ # This method is needed because in some versions of conjur server it has been observed that
+ # docker restart of the conjur server results in an error stating that the puma PID file is still present.
+ # Hence we check to see if this stale PID File exists and delete it, which ensures a smooth restart.
+ # This issue is described in detail in Issue 2381.
+
+ def cleanup_pidfile
+ # Get the path to conjurctl
+ conjurctl_path = `readlink -f $(which conjurctl)`
+
+ # Navigate from its directory (/bin) to the root Conjur server directory
+ conjur_server_dir = Pathname.new(File.join(File.dirname(conjurctl_path), '..')).cleanpath
+ pid_file_path = File.join(conjur_server_dir, 'tmp/pids/server.pid')
+ return unless File.exist?(pid_file_path)
+
+ puts("Removing existing PID file: #{pid_file_path}")
+ File.delete(pid_file_path)
+ end
+
  def fork_server_process
  Process.fork do
  puts("Conjur v#{conjur_version} starting up...")

spec/conjurctl/server_spec.rb

@@ -47,5 +47,44 @@ def wait_for_conjur
  expect(Slosilo["authn:demo"]).to be
  expect(Role["demo:user:admin"]).to be
  end
+
+ it "deletes an existing PID file on start up" do
+ pid_file_path = File.join(conjur_server_dir, 'tmp/pids/server.pid')
+
+ # Ensure the pid file exists before starting Conjur
+ FileUtils.mkdir_p(File.dirname(pid_file_path))
+ FileUtils.touch(pid_file_path)
+
+ # Start Conjur and wait for it to finish its initialization
+ output = with_background_process(
+ 'conjurctl server --account demo'
+ ) do
+ wait_for_conjur
+ end
+
+ # Ensure that the Conjur output reports that the PID was removed
+ expect(output).to include(
+ "Removing existing PID file: #{pid_file_path}"
+ )
+ end
+
+ it "doesn't attempt to delete a non-existent PID file" do
+ pid_file_path = File.join(conjur_server_dir, 'tmp/pids/server.pid')
+
+ # Ensure the pid file doesn't exist before starting Conjur
+ File.delete(pid_file_path) if File.exist?(pid_file_path)
+
+ # Start Conjur and wait for it to finish its initialization
+ output = with_background_process(
+ 'conjurctl server --account demo'
+ ) do
+ wait_for_conjur
+ end
+
+ # Ensure that the Conjur output doesn't report that the PID was removed
+ expect(output).not_to include(
+ "Removing existing PID file: #{pid_file_path}"
+ )
+ end
  end
 end

spec/spec_helper.rb

@@ -82,4 +82,75 @@ def access_token_for(user, account: 'rspec')
  "Token token=\"#{Base64.strict_encode64(bearer_token.to_json)}\""
 end
 
+def with_background_process(cmd, &block)
+ puts("Running: #{cmd}")
+ Open3.popen2e(
+ cmd,
+ pgroup: true
+ ) do |stdin, stdout_and_err, wait_thr|
+ # We don't need to interact with stdin for the background process,
+ # so close that stream immediately.
+ stdin.close_write
+
+ # Read the output of the background process in a thread to run
+ # the given block in parallel.
+ out_reader = Thread.new do
+ output = StringIO.new
+
+ loop do
+ ready = IO.select(
+ [stdout_and_err], # Watch for reading
+ [], # Not watching any files for writing
+ [], # Not watching any files for exceptions
+ # When the background process is killed, it doesn't end the
+ # the output stream, so we need a timeout here to recognize the
+ # stream has closed:
+ 1 # 1 second timeout
+ )
+
+ # If the stream has closed, break the read loop.
+ break if stdout_and_err.closed?
+
+ # If we've reached the end of the stream, break the read loop.
+ break if stdout_and_err.eof?
+
+ # If this was the result of a IO#select timeout, enter the select
+ # loop again.
+ next unless ready
+
+ # Read the next available output on the stream
+ output << stdout_and_err.read_nonblock(1024)
+ end
+
+ # Return the collected output as the result of the read thread
+ output.string
+ end
+
+ # Call the given block
+ block.call
+
+ # Kill the background process and any children processes
+ pgid = Process.getpgid(wait_thr.pid)
+ Process.kill("-TERM", pgid) if wait_thr.alive?
+
+ # Wait for the background process to end
+ wait_thr.value
+
+ # Close the output thread
+ stdout_and_err.close
+
+ # Wait for the result from the reader thread
+ out_reader.value
+ end
+end
+
+def conjur_server_dir
+ # Get the path to conjurctl
+ conjurctl_path = `readlink -f $(which conjurctl)`
+
+ # Navigate from its directory (/bin) to the root Conjur server directory
+ Pathname.new(File.join(File.dirname(conjurctl_path), '..')).cleanpath
+end
+
+
 require 'stringio'