From ddcaac87498ae173597e7a529020d92a903b0014 Mon Sep 17 00:00:00 2001 From: Thom May Date: Thu, 1 Sep 2011 16:41:14 +0100 Subject: [PATCH 1/5] Move options into class initialisation --- lib/foreman/cli.rb | 6 +++--- lib/foreman/engine.rb | 19 ++++++++++--------- spec/foreman/cli_spec.rb | 2 +- spec/foreman/engine_spec.rb | 35 ++++++++++++++++++++++------------- 4 files changed, 36 insertions(+), 26 deletions(-) diff --git a/lib/foreman/cli.rb b/lib/foreman/cli.rb index 042212b..e36d3b9 100644 --- a/lib/foreman/cli.rb +++ b/lib/foreman/cli.rb @@ -18,9 +18,9 @@ class Foreman::CLI < Thor check_procfile! if process - engine.execute(process, options) + engine.execute(process) else - engine.start(options) + engine.start end end @@ -64,7 +64,7 @@ private ###################################################################### end def engine - @engine ||= Foreman::Engine.new(procfile) + @engine ||= Foreman::Engine.new(procfile, options) end def procfile diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index 9e5eb1e..352eede 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -16,9 +16,10 @@ class Foreman::Engine COLORS = [ cyan, yellow, green, magenta, red ] - def initialize(procfile) + def initialize(procfile, options={}) @procfile = read_procfile(procfile) @directory = File.expand_path(File.dirname(procfile)) + @options = options end def processes @@ -50,13 +51,13 @@ class Foreman::Engine end end - def start(options={}) - environment = read_environment(options[:env]) + def start + environment = read_environment(@options[:env]) proctitle "ruby: foreman master" processes_in_order.each do |name, process| - fork process, options, environment + fork process, @options, environment end trap("TERM") { puts "SIGTERM received"; terminate_gracefully } @@ -65,10 +66,10 @@ class Foreman::Engine watch_for_termination end - def execute(name, options={}) - environment = read_environment(options[:env]) + def execute(name) + environment = read_environment(@options[:env]) - fork processes[name], options, environment + fork processes[name], @options, environment trap("TERM") { puts "SIGTERM received"; terminate_gracefully } trap("INT") { puts "SIGINT received"; terminate_gracefully } @@ -85,10 +86,10 @@ class Foreman::Engine private ###################################################################### def fork(process, options={}, environment={}) - concurrency = Foreman::Utils.parse_concurrency(options[:concurrency]) + concurrency = Foreman::Utils.parse_concurrency(@options[:concurrency]) 1.upto(concurrency[process.name]) do |num| - fork_individual(process, num, port_for(process, num, options[:port]), environment) + fork_individual(process, num, port_for(process, num, @options[:port]), environment) end end diff --git a/spec/foreman/cli_spec.rb b/spec/foreman/cli_spec.rb index 8858ee3..7663a12 100644 --- a/spec/foreman/cli_spec.rb +++ b/spec/foreman/cli_spec.rb @@ -19,7 +19,7 @@ describe "Foreman::CLI" do it "runs successfully" do dont_allow(subject).error - mock.instance_of(Foreman::Engine).start({}) + mock.instance_of(Foreman::Engine).start subject.start end end diff --git a/spec/foreman/engine_spec.rb b/spec/foreman/engine_spec.rb index 7e76f59..660265e 100644 --- a/spec/foreman/engine_spec.rb +++ b/spec/foreman/engine_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" require "foreman/engine" describe "Foreman::Engine" do - subject { Foreman::Engine.new("Procfile") } + subject { Foreman::Engine.new("Procfile", {}) } describe "initialize" do describe "without an existing Procfile" do @@ -45,11 +45,12 @@ describe "Foreman::Engine" do it "handles concurrency" do write_procfile - mock(subject).fork_individual(subject.processes["alpha"], 1, 5000, {}) - mock(subject).fork_individual(subject.processes["alpha"], 2, 5001, {}) - mock(subject).fork_individual(subject.processes["bravo"], 1, 5100, {}) - mock(subject).watch_for_termination - subject.start(:concurrency => "alpha=2") + engine = Foreman::Engine.new("Procfile",:concurrency => "alpha=2") + mock(engine).fork_individual(engine.processes["alpha"], 1, 5000, {}) + mock(engine).fork_individual(engine.processes["alpha"], 2, 5001, {}) + mock(engine).fork_individual(engine.processes["bravo"], 1, 5100, {}) + mock(engine).watch_for_termination + engine.start end end @@ -63,27 +64,35 @@ describe "Foreman::Engine" do end describe "environment" do + before(:each) do write_procfile stub(Process).fork - stub(subject).info - mock(subject).watch_for_termination end it "should read if specified" do File.open("/tmp/env", "w") { |f| f.puts("FOO=baz") } - subject.execute("alpha", :env => "/tmp/env") + engine = Foreman::Engine.new("Procfile", :env => "/tmp/env") + stub(engine).info + mock(engine).watch_for_termination + engine.execute("alpha") end it "should fail if specified and doesnt exist" do - mock(subject).error("No such file: /tmp/env") - subject.execute("alpha", :env => "/tmp/env") + mock.instance_of(Foreman::Engine).error("No such file: /tmp/env") + engine = Foreman::Engine.new("Procfile", :env => "/tmp/env") + stub(engine).info + mock(engine).watch_for_termination + engine.execute("alpha") end it "should read .env if none specified" do File.open(".env", "w") { |f| f.puts("FOO=qoo") } - mock(subject).fork_individual(anything, anything, anything, { "FOO" => "qoo" }) - subject.execute("bravo") + engine = Foreman::Engine.new("Procfile") + stub(engine).info + mock(engine).watch_for_termination + mock(engine).fork_individual(anything, anything, anything, { "FOO" => "qoo" }) + engine.execute("bravo") end end end From 7fc6d02e7b571c8b426d0b64a87269fc209081c9 Mon Sep 17 00:00:00 2001 From: Thom May Date: Thu, 1 Sep 2011 17:26:50 +0100 Subject: [PATCH 2/5] Read environment at initialisation This allows us to expose the environment attribute from the engine object and utilise it to build exported startup files. --- lib/foreman/engine.rb | 18 +++++++++--------- spec/foreman/engine_spec.rb | 17 +++++++---------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/lib/foreman/engine.rb b/lib/foreman/engine.rb index 352eede..5cf82f3 100644 --- a/lib/foreman/engine.rb +++ b/lib/foreman/engine.rb @@ -11,6 +11,8 @@ class Foreman::Engine attr_reader :procfile attr_reader :directory + attr_reader :environment + attr_reader :options extend Term::ANSIColor @@ -20,6 +22,7 @@ class Foreman::Engine @procfile = read_procfile(procfile) @directory = File.expand_path(File.dirname(procfile)) @options = options + @environment = read_environment(options[:env]) end def processes @@ -52,12 +55,10 @@ class Foreman::Engine end def start - environment = read_environment(@options[:env]) - proctitle "ruby: foreman master" processes_in_order.each do |name, process| - fork process, @options, environment + fork process end trap("TERM") { puts "SIGTERM received"; terminate_gracefully } @@ -67,9 +68,8 @@ class Foreman::Engine end def execute(name) - environment = read_environment(@options[:env]) - fork processes[name], @options, environment + fork processes[name] trap("TERM") { puts "SIGTERM received"; terminate_gracefully } trap("INT") { puts "SIGINT received"; terminate_gracefully } @@ -85,16 +85,16 @@ class Foreman::Engine private ###################################################################### - def fork(process, options={}, environment={}) + def fork(process) concurrency = Foreman::Utils.parse_concurrency(@options[:concurrency]) 1.upto(concurrency[process.name]) do |num| - fork_individual(process, num, port_for(process, num, @options[:port]), environment) + fork_individual(process, num, port_for(process, num, @options[:port])) end end - def fork_individual(process, num, port, environment) - environment.each { |k,v| ENV[k] = v } + def fork_individual(process, num, port) + @environment.each { |k,v| ENV[k] = v } ENV["PORT"] = port.to_s ENV["PS"] = "#{process.name}.#{num}" diff --git a/spec/foreman/engine_spec.rb b/spec/foreman/engine_spec.rb index 660265e..6626924 100644 --- a/spec/foreman/engine_spec.rb +++ b/spec/foreman/engine_spec.rb @@ -37,8 +37,8 @@ describe "Foreman::Engine" do describe "start" do it "forks the processes" do write_procfile - mock(subject).fork(subject.processes["alpha"], {}, {}) - mock(subject).fork(subject.processes["bravo"], {}, {}) + mock(subject).fork(subject.processes["alpha"]) + mock(subject).fork(subject.processes["bravo"]) mock(subject).watch_for_termination subject.start end @@ -46,9 +46,9 @@ describe "Foreman::Engine" do it "handles concurrency" do write_procfile engine = Foreman::Engine.new("Procfile",:concurrency => "alpha=2") - mock(engine).fork_individual(engine.processes["alpha"], 1, 5000, {}) - mock(engine).fork_individual(engine.processes["alpha"], 2, 5001, {}) - mock(engine).fork_individual(engine.processes["bravo"], 1, 5100, {}) + mock(engine).fork_individual(engine.processes["alpha"], 1, 5000) + mock(engine).fork_individual(engine.processes["alpha"], 2, 5001) + mock(engine).fork_individual(engine.processes["bravo"], 1, 5100) mock(engine).watch_for_termination engine.start end @@ -57,7 +57,7 @@ describe "Foreman::Engine" do describe "execute" do it "runs the processes" do write_procfile - mock(subject).fork(subject.processes["alpha"], {}, {}) + mock(subject).fork(subject.processes["alpha"]) mock(subject).watch_for_termination subject.execute("alpha") end @@ -81,9 +81,6 @@ describe "Foreman::Engine" do it "should fail if specified and doesnt exist" do mock.instance_of(Foreman::Engine).error("No such file: /tmp/env") engine = Foreman::Engine.new("Procfile", :env => "/tmp/env") - stub(engine).info - mock(engine).watch_for_termination - engine.execute("alpha") end it "should read .env if none specified" do @@ -91,7 +88,7 @@ describe "Foreman::Engine" do engine = Foreman::Engine.new("Procfile") stub(engine).info mock(engine).watch_for_termination - mock(engine).fork_individual(anything, anything, anything, { "FOO" => "qoo" }) + mock(engine).fork_individual(anything, anything, anything) engine.execute("bravo") end end From 8fd9b753f4edc6ca6bfe9663f7b74e307435bf6b Mon Sep 17 00:00:00 2001 From: Thom May Date: Tue, 6 Sep 2011 15:59:26 +0100 Subject: [PATCH 3/5] Actually test that the environment is set correctly --- spec/foreman/engine_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/foreman/engine_spec.rb b/spec/foreman/engine_spec.rb index 6626924..17273de 100644 --- a/spec/foreman/engine_spec.rb +++ b/spec/foreman/engine_spec.rb @@ -75,6 +75,7 @@ describe "Foreman::Engine" do engine = Foreman::Engine.new("Procfile", :env => "/tmp/env") stub(engine).info mock(engine).watch_for_termination + engine.environment.should == {"FOO"=>"baz"} engine.execute("alpha") end @@ -89,6 +90,7 @@ describe "Foreman::Engine" do stub(engine).info mock(engine).watch_for_termination mock(engine).fork_individual(anything, anything, anything) + engine.environment.should == {"FOO"=>"qoo"} engine.execute("bravo") end end From 9df93a64cce361ba5c0870e14182537a4d8b6b30 Mon Sep 17 00:00:00 2001 From: Thom May Date: Mon, 5 Sep 2011 14:37:52 +0100 Subject: [PATCH 4/5] Export environment to upstart jobs --- data/export/upstart/process.conf.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/export/upstart/process.conf.erb b/data/export/upstart/process.conf.erb index fb7560c..fac2875 100644 --- a/data/export/upstart/process.conf.erb +++ b/data/export/upstart/process.conf.erb @@ -2,4 +2,4 @@ start on starting <%= app %>-<%= process.name %> stop on stopping <%= app %>-<%= process.name %> respawn -exec su - <%= user %> -c 'cd <%= engine.directory %>; export PORT=<%= port %>; <%= process.command %> >> <%= log_root %>/<%=process.name%>-<%=num%>.log 2>&1' +exec su - <%= user %> -c 'cd <%= engine.directory %>; export PORT=<%= port %>;<% engine.environment.each_pair do |var,env| -%> export <%= var.upcase %>=<%= env %>; <% end -%> <%= process.command %> >> <%= log_root %>/<%=process.name%>-<%=num%>.log 2>&1' From 6da8aca6092051baf49548b7224aa4ef260d9bcf Mon Sep 17 00:00:00 2001 From: Thom May Date: Mon, 5 Sep 2011 15:04:53 +0100 Subject: [PATCH 5/5] foreman erb doesn't have the -%> extension enabled. --- data/export/upstart/process.conf.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/export/upstart/process.conf.erb b/data/export/upstart/process.conf.erb index fac2875..e16ed3a 100644 --- a/data/export/upstart/process.conf.erb +++ b/data/export/upstart/process.conf.erb @@ -2,4 +2,4 @@ start on starting <%= app %>-<%= process.name %> stop on stopping <%= app %>-<%= process.name %> respawn -exec su - <%= user %> -c 'cd <%= engine.directory %>; export PORT=<%= port %>;<% engine.environment.each_pair do |var,env| -%> export <%= var.upcase %>=<%= env %>; <% end -%> <%= process.command %> >> <%= log_root %>/<%=process.name%>-<%=num%>.log 2>&1' +exec su - <%= user %> -c 'cd <%= engine.directory %>; export PORT=<%= port %>;<% engine.environment.each_pair do |var,env| %> export <%= var.upcase %>=<%= env %>; <% end %> <%= process.command %> >> <%= log_root %>/<%=process.name%>-<%=num%>.log 2>&1'