Merge pull request #197 from jsantos/bugfix/pidfiles_are_not_being_ac…

…counted_correctly Fix accounting of pidfiles per process (when using multiple processes)
seuros · Apr 4, 2018 · 7edb22a · 7edb22a
2 parents ddb86ec + e6ffa33
commit 7edb22a
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,13 +5,16 @@
 - Support custom monit filename @zocoi
 - Systemd Integration @baierjan
 - Fix regression in sidekiq_roles variable
+- Fixed pidfile accounting per process @jsantos
+- Rubocop corrections for main task @jsantos
 
 ## 1.0.0
 
 - Prepend underscore before service name index @Tensho
 - Convert CHANGELOG to Markdown @Tensho
 - Drop support for capistrano 2.0 @Tensho
 - *BREAKING CHANGE* If people used custom monit template, they should adjust it to use pid_files variable instead of processes_pids. @Tensho
+- *BREAKING CHANGE* `:sidekiq_role` has been renamed to its plural form, `:sidekiq_roles`
 
 ## 0.20.0
 
@@ -23,21 +26,21 @@
 - `sidekiq:stop` task perpertually callable @Tensho
 
 ## 0.5.4
- 
+
  - Add support for custom count of processes per host in monit task @okoriko
- 
+
 ## 0.5.3
- 
+
  - Custom count of processes per each host
- 
+
 ## 0.5.0
- 
+
  - Multiple processes @mrsimo
- 
+
 ## 0.3.9
- 
+
  - Restore daemon flag from Monit template
- 
+
 ## 0.3.8
 
 - Update monit template: use su instead of sudo / permit all Sidekiq options @bensie
@@ -49,59 +52,59 @@
 - Run Sidekiq as daemon from Monit @dpaluy
 
 ## 0.3.5
- 
+
 - Added `:sidekiq_tag` for capistrano 2 @OscarBarrett
- 
+
 ## 0.3.4
- 
+
 - Fix bug in `sidekiq:start` for capistrano 2 task
- 
+
 ## 0.3.3
- 
+
 - `sidekiq:restart` after `deploy:restart` added to default hooks
- 
+
 ## 0.3.2
- 
+
 - `:sidekiq_queue` accept an array
- 
+
 ## 0.3.1
- 
+
 - Fix logs @rottman
 - Add concurrency option support @ungsophy
- 
+
 ## 0.3.0
- 
+
 - Fix monit task @andreygerasimchuk
- 
+
 ## 0.2.9
- 
+
 - Check if current directory exist @alexdunae
- 
+
 ## 0.2.8
- 
+
 - Added `:sidekiq_queue` & `:sidekiq_config`
- 
+
 ## 0.2.7
- 
+
 - Signal usage @penso
- 
+
 ## 0.2.6
- 
+
 - `sidekiq:start` check if sidekiq is running
- 
+
 ## 0.2.5
- 
+
 - Bug fixes
- 
+
 ## 0.2.4
- 
+
 - Fast deploy with `:sidekiq_run_in_background`
- 
+
 ## 0.2.3
- 
+
 - Added monit tasks (alpha)
- 
+
 ## 0.2.0
- 
+
 - Added `sidekiq:rolling_restart` @jlecour
- 
+
diff --git a/lib/capistrano/tasks/sidekiq.rake b/lib/capistrano/tasks/sidekiq.rake
@@ -11,11 +11,11 @@ namespace :load do
     set :sidekiq_options_per_process, nil
     set :sidekiq_user, nil
     # Rbenv, Chruby, and RVM integration
-    set :rbenv_map_bins, fetch(:rbenv_map_bins).to_a.concat(%w(sidekiq sidekiqctl))
-    set :rvm_map_bins, fetch(:rvm_map_bins).to_a.concat(%w(sidekiq sidekiqctl))
-    set :chruby_map_bins, fetch(:chruby_map_bins).to_a.concat(%w{ sidekiq sidekiqctl })
+    set :rbenv_map_bins, fetch(:rbenv_map_bins).to_a.concat(%w[sidekiq sidekiqctl])
+    set :rvm_map_bins, fetch(:rvm_map_bins).to_a.concat(%w[sidekiq sidekiqctl])
+    set :chruby_map_bins, fetch(:chruby_map_bins).to_a.concat(%w[sidekiq sidekiqctl])
     # Bundler integration
-    set :bundle_bins, fetch(:bundle_bins).to_a.concat(%w(sidekiq sidekiqctl))
+    set :bundle_bins, fetch(:bundle_bins).to_a.concat(%w[sidekiq sidekiqctl])
     # Init system integration
     set :init_system, -> { nil }
     # systemd integration
@@ -52,7 +52,7 @@ namespace :sidekiq do
           execute :systemctl, "--user", "reload", fetch(:service_unit_name), raise_on_non_zero_exit: false
         else
           if test("[ -d #{release_path} ]")
-            each_process_with_index(reverse: true) do |pid_file, idx|
+            each_process_with_index(reverse: true) do |pid_file, _idx|
               if pid_file_exists?(pid_file) && process_exists?(pid_file)
                 quiet_sidekiq(pid_file)
               end
@@ -72,7 +72,7 @@ namespace :sidekiq do
           execute :systemctl, "--user", "stop", fetch(:service_unit_name)
         else
           if test("[ -d #{release_path} ]")
-            each_process_with_index(reverse: true) do |pid_file, idx|
+            each_process_with_index(reverse: true) do |pid_file, _idx|
               if pid_file_exists?(pid_file) && process_exists?(pid_file)
                 stop_sidekiq(pid_file)
               end
@@ -89,7 +89,7 @@ namespace :sidekiq do
       switch_user(role) do
         case fetch(:init_system)
         when :systemd
-          execute :systemctl, "--user", "start", fetch(:service_unit_name)
+          execute :systemctl, '--user', 'start', fetch(:service_unit_name)
         else
           each_process_with_index do |pid_file, idx|
             unless pid_file_exists?(pid_file) && process_exists?(pid_file)
@@ -125,11 +125,10 @@ namespace :sidekiq do
   task :cleanup do
     on roles fetch(:sidekiq_roles) do |role|
       switch_user(role) do
-        each_process_with_index do |pid_file, idx|
+        each_process_with_index do |pid_file, _idx|
           unless process_exists?(pid_file)
-            if pid_file_exists?(pid_file)
-              execute "rm #{pid_file}"
-            end
+            next unless pid_file_exists?(pid_file)
+            execute "rm #{pid_file}"
           end
         end
       end
@@ -143,9 +142,7 @@ namespace :sidekiq do
     on roles fetch(:sidekiq_roles) do |role|
       switch_user(role) do
         each_process_with_index do |pid_file, idx|
-          unless pid_file_exists?(pid_file)
-            start_sidekiq(pid_file, idx)
-          end
+          start_sidekiq(pid_file, idx) unless pid_file_exists?(pid_file)
         end
       end
     end
@@ -176,9 +173,9 @@ namespace :sidekiq do
   end
 
   def each_process_with_index(reverse: false)
-    _pid_files = pid_files
-    _pid_files.reverse! if reverse
-    _pid_files.each_with_index do |pid_file, idx|
+    pid_file_list = pid_files
+    pid_file_list.reverse! if reverse
+    pid_file_list.each_with_index do |pid_file, idx|
       within release_path do
         yield(pid_file, idx)
       end
@@ -209,7 +206,7 @@ namespace :sidekiq do
   end
 
   def pid_files
-    sidekiq_roles = Array(fetch(:sidekiq_roles))
+    sidekiq_roles = Array(fetch(:sidekiq_roles)).dup
     sidekiq_roles.select! { |role| host.roles.include?(role) }
     sidekiq_roles.flat_map do |role|
       processes = fetch(:"#{ role }_processes") || fetch(:sidekiq_processes)
@@ -227,15 +224,15 @@ namespace :sidekiq do
 
   def quiet_sidekiq(pid_file)
     begin
-      execute :sidekiqctl, 'quiet', "#{pid_file}"
+      execute :sidekiqctl, 'quiet', pid_file.to_s
     rescue SSHKit::Command::Failed
       # If gems are not installed (first deploy) and sidekiq_default_hooks is active
       warn 'sidekiqctl not found (ignore if this is the first deploy)'
     end
   end
 
   def stop_sidekiq(pid_file)
-    execute :sidekiqctl, 'stop', "#{pid_file}", fetch(:sidekiq_timeout)
+    execute :sidekiqctl, 'stop', pid_file.to_s, fetch(:sidekiq_timeout)
   end
 
   def start_sidekiq(pid_file, idx = 0)
@@ -251,7 +248,7 @@ namespace :sidekiq do
     end
     args.push "--config #{fetch(:sidekiq_config)}" if fetch(:sidekiq_config)
     args.push "--concurrency #{fetch(:sidekiq_concurrency)}" if fetch(:sidekiq_concurrency)
-    if process_options = fetch(:sidekiq_options_per_process)
+    if (process_options = fetch(:sidekiq_options_per_process))
       args.push process_options[idx]
     end
     # use sidekiq_options for special options
@@ -267,22 +264,22 @@ namespace :sidekiq do
     execute :sidekiq, args.compact.join(' ')
   end
 
-  def switch_user(role, &block)
+  def switch_user(role)
     su_user = sidekiq_user(role)
     if su_user == role.user
-      block.call
+      yield
     else
       as su_user do
-        block.call
+        yield
       end
     end
   end
 
   def sidekiq_user(role)
     properties = role.properties
     properties.fetch(:sidekiq_user) || # local property for sidekiq only
-    fetch(:sidekiq_user) ||
-    properties.fetch(:run_as) || # global property across multiple capistrano gems
-    role.user
+      fetch(:sidekiq_user) ||
+      properties.fetch(:run_as) || # global property across multiple capistrano gems
+      role.user
   end
 end