From acd081700caa90a51884d89704fd43418138aeb7 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 7 Nov 2024 16:02:28 -0500 Subject: [PATCH 1/6] determin inactive users and clean their PUNs too --- nginx_stage/lib/nginx_stage.rb | 15 +++++++++++++++ .../generators/nginx_clean_generator.rb | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/nginx_stage/lib/nginx_stage.rb b/nginx_stage/lib/nginx_stage.rb index f5cb71ea59..a74ecf0edb 100644 --- a/nginx_stage/lib/nginx_stage.rb +++ b/nginx_stage/lib/nginx_stage.rb @@ -192,6 +192,21 @@ def self.active_users end.compact end + # List of inactive users. + # @return [Array] the list of inactive users. + def self.inactive_users + Dir[pun_pid_path(user: '*')].map do |v| + name = v[/#{pun_pid_path(user: '(.+)')}/, 1] + User.new(name) + + # return nil here becasue we actually _want_ to rescue. + # i.e., a User is inactive if it _can't_ be instanitated. + nil + rescue ArgumentError => e + name + end.compact + end + # Get a hash of all the staged app configs # @example List of all staged app configs # staged_apps diff --git a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb index 60c2ea67ee..5b6e343437 100644 --- a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb +++ b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb @@ -79,6 +79,21 @@ class NginxCleanGenerator < Generator $stderr.puts "#{$!.to_s}" end end + + NginxStage.inactive_users.each do |u| + begin + puts "#{u} (disabled)" + pid_path = PidFile.new NginxStage.pun_pid_path(user: u) + + `kill -s TERM #{pid_path.pid}` + FileUtils.rm_rf(Pathname.new(pid_path.to_s).parent) + FileUtils.rm(NginxStage.pun_secret_key_base_path(user: u).to_s) + FileUtils.rm(NginxStage.pun_config_path(user: u).to_s) + + rescue StandardError => e + warn "Error trying to clean up disabled user #{u}: #{e.message}" + end + end end def cleanup_stale_files(pid_path, socket) From 79e9338ea0877668d62ce0c999d284752223147f Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 7 Nov 2024 16:26:51 -0500 Subject: [PATCH 2/6] fix test --- spec/e2e/nginx_stage_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/e2e/nginx_stage_spec.rb b/spec/e2e/nginx_stage_spec.rb index 4a5f029fc2..dc644ab807 100644 --- a/spec/e2e/nginx_stage_spec.rb +++ b/spec/e2e/nginx_stage_spec.rb @@ -17,6 +17,8 @@ def browser on hosts, 'mkdir /var/run/ondemand-nginx/deleted_user' on hosts, 'chmod 600 /var/run/ondemand-nginx/deleted_user' on hosts, 'echo -n 11111111 > /var/run/ondemand-nginx/deleted_user/passenger.pid' + on hosts, 'echo -n 11111111 > /var/lib/ondemand-nginx/config/puns/deleted_user.conf' + on hosts, 'echo -n 11111111 > /var/lib/ondemand-nginx/config/puns/deleted_user.secret_key_base.txt' end after(:all) do @@ -31,7 +33,10 @@ def browser # Note there's no error here about 'deleted_user' on hosts, '/opt/ood/nginx_stage/sbin/nginx_stage nginx_clean --force' do - assert_equal stdout, "ood\n" + assert_equal stdout, "ood\ndeleted_user (disabled)\n" + refute(File.exists?('/var/run/ondemand-nginx/deleted_user')) + refute(File.exists?('/var/lib/ondemand-nginx/config/puns/deleted_user.conf')) + refute(File.exists?('/var/lib/ondemand-nginx/config/puns/deleted_user.secret_key_base.txt')) end end end From 7bd928613f388d5f7e0a77e6d9445cf1ce9200f7 Mon Sep 17 00:00:00 2001 From: Simon Westersund <85734631+CSC-swesters@users.noreply.github.com> Date: Mon, 11 Nov 2024 12:04:15 +0200 Subject: [PATCH 3/6] nginx_stage: clean up invalid users' PID file directories later - Gather a list of directories which will be empty soon, and rmdir() them after sending the SIGTERM signal to all invalid user PUNs. This will result in the least sleeping, and will avoid using `rm_rf()` as root, which seems very risky. --- .../generators/nginx_clean_generator.rb | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb index 5b6e343437..a245a2ef0e 100644 --- a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb +++ b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb @@ -80,20 +80,40 @@ class NginxCleanGenerator < Generator end end + pid_parent_dirs_to_remove_later = [] NginxStage.inactive_users.each do |u| begin puts "#{u} (disabled)" pid_path = PidFile.new NginxStage.pun_pid_path(user: u) + # Send a SIGTERM to the master nginx process to kill the PUN. + # 'nginx stop' won't work, since getpwnam(3) will cause an error. `kill -s TERM #{pid_path.pid}` - FileUtils.rm_rf(Pathname.new(pid_path.to_s).parent) FileUtils.rm(NginxStage.pun_secret_key_base_path(user: u).to_s) FileUtils.rm(NginxStage.pun_config_path(user: u).to_s) - + pid_path_parent_dir = Pathname.new(pid_path.to_s).parent + pid_parent_dirs_to_remove_later.push(pid_path_parent_dir) rescue StandardError => e warn "Error trying to clean up disabled user #{u}: #{e.message}" end end + + # Remove the PID path parent directories now that the nginx processes have + # had time to clean up their Passenger PID file and socket. + pid_parent_dirs_to_remove_later.each do |dir| + begin + begin + FileUtils.rmdir(dir) + rescue Errno::ENOTEMPTY + # Wait for a short time, while Nginx cleans up its PID file. + sleep(0.05) + # Then try again once. + FileUtils.rmdir(dir) + end + rescue StandardError => e + warn "Error trying to clean up the PID file directory of disabled user #{u}: #{e.message}" + end + end end def cleanup_stale_files(pid_path, socket) From eb8a11d53c1c4d7ceec15a1622863a07bc805fa6 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 12 Dec 2024 15:53:37 -0500 Subject: [PATCH 4/6] more conservative sleep time --- nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb index a245a2ef0e..ae0dd47d05 100644 --- a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb +++ b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb @@ -106,7 +106,7 @@ class NginxCleanGenerator < Generator FileUtils.rmdir(dir) rescue Errno::ENOTEMPTY # Wait for a short time, while Nginx cleans up its PID file. - sleep(0.05) + sleep(0.5) # Then try again once. FileUtils.rmdir(dir) end From 69c8854a3045a4ed72e1d18de9681ea1a3a7db7f Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Thu, 12 Dec 2024 16:32:31 -0500 Subject: [PATCH 5/6] reword this messgae --- nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb index ae0dd47d05..3acbe0a412 100644 --- a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb +++ b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb @@ -111,7 +111,7 @@ class NginxCleanGenerator < Generator FileUtils.rmdir(dir) end rescue StandardError => e - warn "Error trying to clean up the PID file directory of disabled user #{u}: #{e.message}" + warn "Error trying to clean up the PID file #{dir} of disabled user: #{e.message}" end end end From dc2d3c63acef8e0d072626e5f04339e671163c24 Mon Sep 17 00:00:00 2001 From: Jeff Ohrstrom Date: Fri, 13 Dec 2024 11:03:52 -0500 Subject: [PATCH 6/6] fixup this warn message --- nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb index 3acbe0a412..63a812ae76 100644 --- a/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb +++ b/nginx_stage/lib/nginx_stage/generators/nginx_clean_generator.rb @@ -111,7 +111,7 @@ class NginxCleanGenerator < Generator FileUtils.rmdir(dir) end rescue StandardError => e - warn "Error trying to clean up the PID file #{dir} of disabled user: #{e.message}" + warn "Error trying to clean up the PID file directory '#{dir}' of disabled user: #{e.message}" end end end