From c705b5fbef6917f0d60c7485505aa2086918f03a Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Fri, 6 Apr 2012 15:55:59 -0700 Subject: [PATCH 1/7] Add #stop method on Foreman::Engine for stopping certain named processes This will make embedding foreman "nicer" since the embedder can then stop a specific process (e.g. turning off a service for an integration fail-over test) --- lib/foreman/engine.rb | 18 +++++++++++++++++- spec/foreman/engine_spec.rb | 3 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index 71953fb..e1f1180 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -39,6 +39,22 @@ class Foreman::Engine spawn_processes watch_for_output watch_for_termination + terminate_gracefully + end + + def stop(name) + running_processes.each do |pid, process| + next unless process.name.start_with? name + + process.kill 'SIGTERM' + process = running_processes.delete(pid) + Timeout.timeout(5) do + begin + Process.waitpid(pid) + rescue Errno::ECHILD + end + end + end end def port_for(process, num, base_port=nil) @@ -91,6 +107,7 @@ private ###################################################################### rescue Timeout::Error info "sending SIGKILL to all processes" kill_all "SIGKILL" + rescue Errno::ECHILD end def poll_readers @@ -123,7 +140,6 @@ private ###################################################################### pid, status = Process.wait2 process = running_processes.delete(pid) info "process terminated", process.name - terminate_gracefully rescue Errno::ECHILD end diff --git a/spec/foreman/engine_spec.rb b/spec/foreman/engine_spec.rb index a7e8516..f64e5ba 100644 --- a/spec/foreman/engine_spec.rb +++ b/spec/foreman/engine_spec.rb @@ -35,6 +35,7 @@ describe "Foreman::Engine", :fakefs do mock.instance_of(Foreman::Process).run_process(Dir.pwd, "./bravo", is_a(IO)) mock(subject).watch_for_output mock(subject).watch_for_termination + mock(subject).terminate_gracefully subject.start end @@ -45,6 +46,7 @@ describe "Foreman::Engine", :fakefs do mock.instance_of(Foreman::Process).run_process(Dir.pwd, "./bravo", is_a(IO)).never mock(engine).watch_for_output mock(engine).watch_for_termination + mock(engine).terminate_gracefully engine.start end end @@ -109,6 +111,7 @@ describe "Foreman::Engine", :fakefs do it "should spawn" do stub(subject).watch_for_output stub(subject).watch_for_termination + stub(subject).terminate_gracefully subject.start Process.waitall mock(subject).info(/started with pid \d+/, "utf8.1", anything) From e4a3215257b2e46b5791288a37be352253aea49d Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Tue, 10 Apr 2012 20:19:23 -0700 Subject: [PATCH 2/7] Re-implement #terminate_gracefully with #stop(name) --- lib/foreman/engine.rb | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index e1f1180..a665535 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -42,15 +42,16 @@ class Foreman::Engine terminate_gracefully end - def stop(name) + def stop(name, signal='SIGTERM') running_processes.each do |pid, process| next unless process.name.start_with? name - process.kill 'SIGTERM' + process.kill signal process = running_processes.delete(pid) Timeout.timeout(5) do begin Process.waitpid(pid) + info "process terminated", process.name rescue Errno::ECHILD end end @@ -85,28 +86,21 @@ private ###################################################################### options[:port] || 5000 end - def kill_all(signal="SIGTERM") - running_processes.each do |pid, process| - info "sending #{signal} to pid #{pid}" - process.kill signal - end - end - def terminate_gracefully return if @terminating @terminating = true info "sending SIGTERM to all processes" - kill_all "SIGTERM" Timeout.timeout(5) do - while running_processes.length > 0 - pid, status = Process.wait2 - process = running_processes.delete(pid) - info "process terminated", process.name + running_processes.each do |pid, process| + stop(process.name) end end rescue Timeout::Error info "sending SIGKILL to all processes" - kill_all "SIGKILL" + running_process.each do |pid, process| + info "sending #{signal} to pid #{pid}" + stop(process.name, 'SIGKILL') + end rescue Errno::ECHILD end From 38aecff886504bdfaf3fecac58ef271c66e7fad3 Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Tue, 10 Apr 2012 20:21:43 -0700 Subject: [PATCH 3/7] When executing #stop(nil), all processes should be sent the signal --- lib/foreman/engine.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index a665535..7ca47e2 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -44,7 +44,14 @@ class Foreman::Engine def stop(name, signal='SIGTERM') running_processes.each do |pid, process| - next unless process.name.start_with? name + unless name.nil? + # Comparing against process.entry.name instead of process.name to + # make sure we match the process name exactly for any/all + # concurrently running processes by this name + next unless process.entry.name == name + else + info "sending #{signal} to all processes" + end process.kill signal process = running_processes.delete(pid) @@ -89,18 +96,11 @@ private ###################################################################### def terminate_gracefully return if @terminating @terminating = true - info "sending SIGTERM to all processes" Timeout.timeout(5) do - running_processes.each do |pid, process| - stop(process.name) - end + stop(nil) end rescue Timeout::Error - info "sending SIGKILL to all processes" - running_process.each do |pid, process| - info "sending #{signal} to pid #{pid}" - stop(process.name, 'SIGKILL') - end + stop(nil, 'SIGKILL') rescue Errno::ECHILD end From de62d0655efed0289ad7c5d142478bc2df210d9c Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Tue, 10 Apr 2012 20:26:02 -0700 Subject: [PATCH 4/7] Re-name the main Foreman::Engine method to #run to avoid a name collision with #start(name) --- lib/foreman/cli.rb | 2 +- lib/foreman/engine.rb | 2 +- spec/foreman/cli_spec.rb | 2 +- spec/foreman/engine_spec.rb | 14 +++++++------- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/foreman/cli.rb b/lib/foreman/cli.rb index f7dc59b..cf72b58 100644 --- a/lib/foreman/cli.rb +++ b/lib/foreman/cli.rb @@ -30,7 +30,7 @@ class Foreman::CLI < Thor def start(process=nil) check_procfile! engine.options[:concurrency] = "#{process}=1" if process - engine.start + engine.run end desc "export FORMAT LOCATION", "Export the application to another process management format" diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index 7ca47e2..a6dae6a 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -28,7 +28,7 @@ class Foreman::Engine @output_mutex = Mutex.new end - def start + def run proctitle "ruby: foreman master" termtitle "#{File.basename(@directory)} - foreman" diff --git a/spec/foreman/cli_spec.rb b/spec/foreman/cli_spec.rb index 405b8a2..7a4d9c1 100644 --- a/spec/foreman/cli_spec.rb +++ b/spec/foreman/cli_spec.rb @@ -21,7 +21,7 @@ describe "Foreman::CLI", :fakefs do it "runs successfully" do dont_allow(subject).error - mock.instance_of(Foreman::Engine).start + mock.instance_of(Foreman::Engine).run subject.start end diff --git a/spec/foreman/engine_spec.rb b/spec/foreman/engine_spec.rb index f64e5ba..0c5f6e0 100644 --- a/spec/foreman/engine_spec.rb +++ b/spec/foreman/engine_spec.rb @@ -28,7 +28,7 @@ describe "Foreman::Engine", :fakefs do end end - describe "start" do + describe "run" do it "forks the processes" do write_procfile mock.instance_of(Foreman::Process).run_process(Dir.pwd, "./alpha", is_a(IO)) @@ -36,7 +36,7 @@ describe "Foreman::Engine", :fakefs do mock(subject).watch_for_output mock(subject).watch_for_termination mock(subject).terminate_gracefully - subject.start + subject.run end it "handles concurrency" do @@ -47,7 +47,7 @@ describe "Foreman::Engine", :fakefs do mock(engine).watch_for_output mock(engine).watch_for_termination mock(engine).terminate_gracefully - engine.start + engine.run end end @@ -66,7 +66,7 @@ describe "Foreman::Engine", :fakefs do File.open("/tmp/env", "w") { |f| f.puts("FOO=baz") } engine = Foreman::Engine.new("Procfile", :env => "/tmp/env") engine.environment.should == {"FOO"=>"baz"} - engine.start + engine.run end it "should read more than one if specified" do @@ -74,7 +74,7 @@ describe "Foreman::Engine", :fakefs do File.open("/tmp/env2", "w") { |f| f.puts("BAZ=qux") } engine = Foreman::Engine.new("Procfile", :env => "/tmp/env1,/tmp/env2") engine.environment.should == { "FOO"=>"bar", "BAZ"=>"qux" } - engine.start + engine.run end it "should handle quoted values" do @@ -97,7 +97,7 @@ describe "Foreman::Engine", :fakefs do File.open(".env", "w") { |f| f.puts("FOO=qoo") } engine = Foreman::Engine.new("Procfile") engine.environment.should == {"FOO"=>"qoo"} - engine.start + engine.run end end @@ -112,7 +112,7 @@ describe "Foreman::Engine", :fakefs do stub(subject).watch_for_output stub(subject).watch_for_termination stub(subject).terminate_gracefully - subject.start + subject.run Process.waitall mock(subject).info(/started with pid \d+/, "utf8.1", anything) mock(subject).info("\xff\x03\n", "utf8.1", anything) From 48f764e347222b790d7935ff2a2864a9799af0d8 Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Tue, 10 Apr 2012 20:34:24 -0700 Subject: [PATCH 5/7] Refactor #spawn_processes into #start(name) When passed nil (aka ALL_PROCESSES) #start will start all processes in the Procfile as per existing behavior --- lib/foreman/engine.rb | 33 ++++++++++++++++++++------------- spec/foreman/engine_spec.rb | 2 +- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index a6dae6a..fcaf512 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -15,6 +15,9 @@ class Foreman::Engine attr_reader :directory attr_reader :options + # This constant is here to make the invocations of #start and #stop methods + # clearer + ALL_PROCESSES = nil COLORS = %w( cyan yellow green magenta red blue intense_cyan intense_yellow intense_green intense_magenta intense_red, intense_blue ) @@ -36,12 +39,28 @@ class Foreman::Engine trap("INT") { puts "SIGINT received"; terminate_gracefully } assign_colors - spawn_processes + start ALL_PROCESSES watch_for_output watch_for_termination terminate_gracefully end + def start(name) + concurrency = Foreman::Utils.parse_concurrency(@options[:concurrency]) + + procfile.entries.each do |entry| + unless name == ALL_PROCESSES + next unless entry.name == name + end + + reader, writer = (IO.method(:pipe).arity == 0 ? IO.pipe : IO.pipe("BINARY")) + entry.spawn(concurrency[entry.name], writer, @directory, @environment, port_for(entry, 1, base_port)).each do |process| + running_processes[process.pid] = process + readers[process] = reader + end + end + end + def stop(name, signal='SIGTERM') running_processes.each do |pid, process| unless name.nil? @@ -77,18 +96,6 @@ class Foreman::Engine private ###################################################################### - def spawn_processes - concurrency = Foreman::Utils.parse_concurrency(@options[:concurrency]) - - procfile.entries.each do |entry| - reader, writer = (IO.method(:pipe).arity == 0 ? IO.pipe : IO.pipe("BINARY")) - entry.spawn(concurrency[entry.name], writer, @directory, @environment, port_for(entry, 1, base_port)).each do |process| - running_processes[process.pid] = process - readers[process] = reader - end - end - end - def base_port options[:port] || 5000 end diff --git a/spec/foreman/engine_spec.rb b/spec/foreman/engine_spec.rb index 0c5f6e0..07bfe53 100644 --- a/spec/foreman/engine_spec.rb +++ b/spec/foreman/engine_spec.rb @@ -57,7 +57,7 @@ describe "Foreman::Engine", :fakefs do stub(Process).fork any_instance_of(Foreman::Engine) do |engine| stub(engine).info - stub(engine).spawn_processes + stub(engine).start stub(engine).watch_for_termination end end From b2bf95479e2bf2605a23d6abc21c2bcd359c399c Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Tue, 10 Apr 2012 20:35:03 -0700 Subject: [PATCH 6/7] Refactor #stop to reference ALL_PROCESSES for a bit clearer readability --- lib/foreman/engine.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index fcaf512..bd75cd4 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -63,7 +63,7 @@ class Foreman::Engine def stop(name, signal='SIGTERM') running_processes.each do |pid, process| - unless name.nil? + unless name == ALL_PROCESSES # Comparing against process.entry.name instead of process.name to # make sure we match the process name exactly for any/all # concurrently running processes by this name @@ -104,10 +104,10 @@ private ###################################################################### return if @terminating @terminating = true Timeout.timeout(5) do - stop(nil) + stop ALL_PROCESSES end rescue Timeout::Error - stop(nil, 'SIGKILL') + stop(ALL_PROCESSES, 'SIGKILL') rescue Errno::ECHILD end From 3a2a53be9594ddcab8019adc0a14d0cb5efb4b36 Mon Sep 17 00:00:00 2001 From: "R. Tyler Croy" Date: Wed, 11 Apr 2012 10:25:58 -0700 Subject: [PATCH 7/7] Remove ALL_PROCESSES and default the name arguments to #start/#stop to nil --- lib/foreman/engine.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index bd75cd4..0ca29ac 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -15,9 +15,6 @@ class Foreman::Engine attr_reader :directory attr_reader :options - # This constant is here to make the invocations of #start and #stop methods - # clearer - ALL_PROCESSES = nil COLORS = %w( cyan yellow green magenta red blue intense_cyan intense_yellow intense_green intense_magenta intense_red, intense_blue ) @@ -39,17 +36,17 @@ class Foreman::Engine trap("INT") { puts "SIGINT received"; terminate_gracefully } assign_colors - start ALL_PROCESSES + start watch_for_output watch_for_termination terminate_gracefully end - def start(name) + def start(name=nil) concurrency = Foreman::Utils.parse_concurrency(@options[:concurrency]) procfile.entries.each do |entry| - unless name == ALL_PROCESSES + unless name == nil next unless entry.name == name end @@ -61,7 +58,7 @@ class Foreman::Engine end end - def stop(name, signal='SIGTERM') + def stop(name=nil, signal='SIGTERM') running_processes.each do |pid, process| unless name == ALL_PROCESSES # Comparing against process.entry.name instead of process.name to @@ -104,10 +101,10 @@ private ###################################################################### return if @terminating @terminating = true Timeout.timeout(5) do - stop ALL_PROCESSES + stop end rescue Timeout::Error - stop(ALL_PROCESSES, 'SIGKILL') + stop(nil, 'SIGKILL') rescue Errno::ECHILD end