From e6ffa337d6622ad95813cf3ab2be6198273c4eb2 Mon Sep 17 00:00:00 2001 From: Jorge Santos Date: Thu, 29 Mar 2018 14:32:43 +0800 Subject: [PATCH] Make sure :sidekiq_roles is not mutated on pid_files - Minor corrections suggested by Rubocop Corrected the following infractions: - [Style/PercentLiteralDelimiters](http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/PercentLiteralDelimiters) - [Lint/UnusedBlockArgument](http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Lint/UnusedBlockArgument) - [Style/Next](http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/Next) - [Style/IfUnlessModifier](http://www.rubydoc.info/gems/rubocop/0.9.1/Rubocop/Cop/Style/IfUnlessModifier) - [Lint/UnderscorePrefixedVariableName](http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Lint/UnderscorePrefixedVariableName) - [Style/UnneededInterpolation](http://www.rubydoc.info/github/bbatsov/RuboCop/RuboCop/Cop/Style/UnneededInterpolation) - [Performance/RedundantBlockCall](http://www.rubydoc.info/github/bbatsov/RuboCop/RuboCop/Cop/Performance/RedundantBlockCall) - [Layout/MultilineOperationIndentation](http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Layout/MultilineOperationIndentation) - Updated CHANGELOG.md to include recent bugfixes and 1.0.0 (forgotten) breaking changes --- CHANGELOG.md | 75 ++++++++++++++++--------------- lib/capistrano/tasks/sidekiq.rake | 51 ++++++++++----------- 2 files changed, 63 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b7aa32..3b0e758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - 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 @@ -12,6 +14,7 @@ - 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 index 449a1f9..0b5bea0 100644 --- 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,7 +224,7 @@ 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)' @@ -235,7 +232,7 @@ namespace :sidekiq do 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,13 +264,13 @@ 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 @@ -281,8 +278,8 @@ namespace :sidekiq do 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