Remove the stale Puma pidfile if it exists

CHANGELOG.md

@@ -47,6 +47,8 @@ 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)
+- Fixed a bug that causes docker restart of conjur server to fail sometimes due to a stale puma pidfile
+  [#2381](https://github.com/cyberark/conjur/issues/2381)
 
 ### 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,19 @@ 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
+      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...")
@@ -136,5 +152,13 @@ def fork_rotation_process
     def wait_for_child_processes
       Process.waitall
     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
   end
 end

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'