From fcbc657ade3abaa28667f0b4c26a983f83f36117 Mon Sep 17 00:00:00 2001 From: Christian Simon Date: Tue, 2 Dec 2014 19:43:12 +0100 Subject: [PATCH] * Refactor specs --- lib/php_fpm_docker/application.rb | 2 + lib/php_fpm_docker/docker_container.rb | 14 +++- lib/php_fpm_docker/docker_image.rb | 3 +- lib/php_fpm_docker/launcher.rb | 14 +++- lib/php_fpm_docker/pool.rb | 4 +- spec/helper.rb | 52 ++++++++++-- spec/integration/application_spec.rb | 106 +++++++++++++++++++++++-- spec/integration/docker_spec.rb | 6 ++ spec/spec_helper.rb | 8 +- spec/unit/docker_container_spec.rb | 82 ++++++++++++++++--- spec/unit/launcher_spec.rb | 31 -------- spec/unit/pool_spec.rb | 4 +- 12 files changed, 263 insertions(+), 63 deletions(-) diff --git a/lib/php_fpm_docker/application.rb b/lib/php_fpm_docker/application.rb index f9bbcc6..75cf9d7 100644 --- a/lib/php_fpm_docker/application.rb +++ b/lib/php_fpm_docker/application.rb @@ -28,6 +28,7 @@ def self.log_dir_path def initialize @name = 'php_fpm_docker' @longname = 'PHP FPM Docker Wrapper' + @launchers = [] end def start @@ -41,6 +42,7 @@ def start # init l = Launcher.new php_name, self + @launchers << l # run daemon self.pid = l.run diff --git a/lib/php_fpm_docker/docker_container.rb b/lib/php_fpm_docker/docker_container.rb index 925534e..06b5c1f 100644 --- a/lib/php_fpm_docker/docker_container.rb +++ b/lib/php_fpm_docker/docker_container.rb @@ -90,14 +90,22 @@ def method_missing(sym, *args, &block) end end - def create(cmd, opts = {}) + def create(opts = {}) + opts = { 'Cmd' => opts } if opts.is_a? Array + + fail( + ArgumentError, + 'Argument has to be a hash or array of strings' + ) if opts.nil? + my_opts = options my_opts.merge! opts - fail(ArgumentError, "cmd is no array: #{cmd}") unless cmd.is_a? Array + fail(ArgumentError, "cmd is no array: #{my_opts['Cmd']}") \ + unless my_opts['Cmd'].is_a? Array # ensure there are only strings - my_opts['Cmd'] = cmd.map(&:to_s) + my_opts['Cmd'] = my_opts['Cmd'].map(&:to_s) @container = Docker::Container.create(my_opts) logger.debug do diff --git a/lib/php_fpm_docker/docker_image.rb b/lib/php_fpm_docker/docker_image.rb index 7cc85f0..e79e99f 100644 --- a/lib/php_fpm_docker/docker_image.rb +++ b/lib/php_fpm_docker/docker_image.rb @@ -1,6 +1,7 @@ # coding: utf-8 require 'docker' require 'php_fpm_docker/logging' +require 'php_fpm_docker/docker_container' module PhpFpmDocker # Wraps the docker connection @@ -91,7 +92,7 @@ def detect_find_path(sym) private def detect_fetch - output = cmd(['/bin/sh', '-c', detect_cmd]) + output = cmd('Cmd' => ['/bin/sh', '-c', detect_cmd]) output[:stdout].split("\n") end diff --git a/lib/php_fpm_docker/launcher.rb b/lib/php_fpm_docker/launcher.rb index 5d49d84..4f95a8f 100644 --- a/lib/php_fpm_docker/launcher.rb +++ b/lib/php_fpm_docker/launcher.rb @@ -1,6 +1,7 @@ # coding: utf-8 require 'pathname' require 'php_fpm_docker/config_parser' +require 'php_fpm_docker/docker_image' require 'php_fpm_docker/logging' module PhpFpmDocker @@ -13,6 +14,8 @@ class Launcher # rubocop:disable ClassLength def initialize(name, app) # rubocop:disable MethodLength @name = name @app = app + + # Set log file for all sub objects Application.log_path = Application.log_dir_path.join("#{@name}") end @@ -120,9 +123,14 @@ def pools_action(pools, pools_hashes, action) begin pool[:object].send(action) rescue => e - logger.warn(pool[:object].to_s) do + logger.warn do "Failed to #{action}: #{e.message}" end + e.backtrace.each do |line| + logger.debug do + line + end + end end end else @@ -185,11 +193,11 @@ def pools_config def config @config ||= ConfigParser.new(config_path) - @config.config + @config.config['global'] end def docker_image - @docker_image ||= docker_image_get + @docker_image ||= DockerImage.new(config['image_name']) end end end diff --git a/lib/php_fpm_docker/pool.rb b/lib/php_fpm_docker/pool.rb index a6f704c..08fb887 100644 --- a/lib/php_fpm_docker/pool.rb +++ b/lib/php_fpm_docker/pool.rb @@ -111,7 +111,7 @@ def gid # Build the spawn command def spawn_command [ - @launcher.spawn_cmd_path, + @launcher.docker_image.spawn_fcgi_path, '-s', @config['listen'], '-U', listen_uid.to_s, '-G', listen_gid.to_s, @@ -134,7 +134,7 @@ def php_command end - [@launcher.php_cmd_path] + admin_options + [@launcher.docker_image.php_path] + admin_options end def container diff --git a/spec/helper.rb b/spec/helper.rb index 5adbcd9..018cc95 100644 --- a/spec/helper.rb +++ b/spec/helper.rb @@ -60,8 +60,6 @@ def dbl_launcher_options :bind_mounts => ['/mnt/bind'], :web_path => '/var/webpath', :docker_image => dbl_docker_image, - :spawn_cmd_path => '/usr/bin/fcgi-bin', - :php_cmd_path => '/usr/bin/php', } end @@ -74,6 +72,8 @@ def dbl_docker_image return @dbl_docker_image unless @dbl_docker_image.nil? @dbl_docker_image = instance_double('PhpFpmDocker::DockerImage', :create => nil, + :spawn_fcgi_path => '/usr/bin/fcgi-bin', + :php_path => '/usr/bin/php', ) @dbl_docker_image end @@ -95,11 +95,49 @@ def dbl_logger @dbl_logger ||= logger end + def dbl_docker_api_image + return @dbl_docker_api_image unless @dbl_docker_api_image.nil? + d = @dbl_docker_api_image = double(Docker::Image) + allow(Docker::Image).to receive(:get) do |name| + double(name, + :id => "id_#{name}" + ) + end + d + end + + def dbl_docker_api_container + return @dbl_docker_api_container unless @dbl_docker_api_container.nil? + allow(Docker::Container).to receive(:create) do |*create_args| + name = create_args.first['name'] || 'unknown' + c = instance_double( + Docker::Container, + :id => name + ) + allow(c).to receive(:start) do |*start_args| + @docker_api_containers[name][:start_args] = start_args + end + @docker_api_containers = {} if @docker_api_containers.nil? + @docker_api_containers[name] = { + :object => c, + :create_args => create_args, + } + c + end + end + + def mock_docker_api + dbl_docker_api_image + dbl_docker_api_container + end + def mock_users allow(Etc).to receive(:getpwnam) do |user| - uid = @users.select do |key, value| + result = @users.select do |key, value| value == user - end.first.first.to_i + end + raise "User #{user} not found" if result.length < 1 + uid = result.first.first.to_i d = double(user) allow(d).to receive(:uid).and_return(uid) d @@ -108,9 +146,11 @@ def mock_users def mock_groups allow(Etc).to receive(:getgrnam) do |group| - gid = @groups.select do |key, value| + result = @groups.select do |key, value| value == group - end.first.first.to_i + end + raise "Group #{group} not found" if result.length < 1 + gid = result.first.first.to_i d = double(group) allow(d).to receive(:gid).and_return(gid) d diff --git a/spec/integration/application_spec.rb b/spec/integration/application_spec.rb index 8bd3840..4f40791 100644 --- a/spec/integration/application_spec.rb +++ b/spec/integration/application_spec.rb @@ -11,10 +11,37 @@ module PhpFpmDocker dir: Pathname.new(Dir.mktmpdir) } + # Mock docker api + mock_docker_api + + # Mock users + @users = { + 1001 => 'user1', + 1002 => 'user2', + 2001 => 'luser1', + 2002 => 'luser2', + } + mock_users + + @groups = {} + @users.each do |key,value| + @groups[key] = value.sub('user','group') + end + mock_groups + + # Mock path detection + allow_any_instance_of(DockerImage).to receive(:detect_output).and_return([ + 'spawn_fcgi_path=/spawn_fcgi_path', + 'php_path=/php_path', + 'PHP 5.6.3 (cgi-fcgi) (built: Nov 17 2014 14:14:17)', + ]) + # Mock log dir @options[:log_dir] = @options[:dir].join 'log' allow(Application).to receive(:log_dir_path).and_return(@options[:log_dir]) - allow(Application).to receive(:log_path).and_return(STDOUT) + allow(Application).to receive(:log_path).and_return(Pathname.new( + File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'tmp', 'rspec.log')) + )) # Wrap creation of Launchers allow(Launcher).to receive(:new).and_wrap_original do |m, *args| @@ -49,10 +76,79 @@ module PhpFpmDocker @options[:pools] = 2 create_config @options end - describe '#run' do - it 'starts containers' do - puts @options[:dir] - method(['launcher1','start']) + describe 'init.d script' do + let(:start) do + expect{start_method}.to output.to_stdout + start_method + end + let(:start_method) do + allow(a_i).to receive(:exit).with(0) + a_i.run(['launcher1','start']) + end + + context 'one launcher' do + context 'start' do + let(:method) do + start + end + it 'should return successful return value' do + expect(a_i).to receive(:exit).with(0) + method + end + context 'starts two containers' do + before(:example) do + method + end + it 'should be started' do + expect(@docker_api_containers.keys).to contain_exactly( + match(/web1_/), + match(/web2_/) + ) + end + it 'should get the correct bind_mounts' do + matching = [ + match(%r{^/var/lib/php5-fpm(:|$)}), + match(%r{^/var/www/web[0-9]+domain\.com(:|$)}), + ] + @docker_api_containers.each do |key,value| + expect(value[:start_args].first['Binds']).to contain_exactly( + *matching + ) + expect(value[:create_args].first['Volumes'].keys).to contain_exactly( + *matching + ) + end + end + it 'should get the correct command' do + @docker_api_containers.each do |key,value| + command = value[:create_args].first['Cmd'] + expect(command.join(' ')).to match(/-M 0660/) + expect(command.join(' ')).to match(%r{/spawn_fcgi_path.+--.+/php_path}) + ['u','U','g','G'].each do |char| + expect(command.join(' ')).to match(/-U [0-9]+/) + end + end + end + end + end + context 'start and stop' do + let(:method) do + start + end + let(:launchers) do + inst_get(:@launchers) + end + it 'should return successful return value' do + expect(a_i).to receive(:exit).with(0) + method + + # Loop containers + @docker_api_containers.each do |key,value| + expect(value[:object]).to receive(:delete).with(hash_including(:force => true)) + end + launchers.first.stop_pools + end + end end end end diff --git a/spec/integration/docker_spec.rb b/spec/integration/docker_spec.rb index 3ae3648..9a81937 100644 --- a/spec/integration/docker_spec.rb +++ b/spec/integration/docker_spec.rb @@ -17,6 +17,12 @@ module PhpFpmDocker DockerImage.new @image_name end describe 'docker_integration' do + before(:example) do + # Mock log dir + allow(Application).to receive(:log_path).and_return(Pathname.new( + File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'tmp', 'rspec.log')) + )) + end describe DockerImage do let(:a_i) do image diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 52954cd..69d7c56 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,13 @@ require 'coveralls' require 'helper' +require 'simplecov' +require 'coveralls' -Coveralls.wear! do +SimpleCov.formatter = SimpleCov::Formatter::MultiFormatter[ + Coveralls::SimpleCov::Formatter, + SimpleCov::Formatter::HTMLFormatter, +] +SimpleCov.start do add_filter 'spec/' end diff --git a/spec/unit/docker_container_spec.rb b/spec/unit/docker_container_spec.rb index 5889789..b160868 100644 --- a/spec/unit/docker_container_spec.rb +++ b/spec/unit/docker_container_spec.rb @@ -83,6 +83,23 @@ } end end + describe '#method_missing' do + before(:example) do + @cont = instance_double(Docker::Container) + allow(a_i).to receive(:container).and_return(@cont) + end + context 'forward methods' do + [:start, :stop, :delete, :wait, :logs, :attach].each do |method| + it "should forward :#{method}" do + expect(@cont).to receive(method).with(:args) + a_i.send(method, :args) + end + end + it "should not forward :not_existing_method" do + expect{a_i.send(:not_existing_method, :args)}.to raise_error(NoMethodError) + end + end + end describe '#create' do before(:example) do a_i @@ -90,33 +107,80 @@ 'Image' => @image.id, 'Cmd' => ['test','me'], } - @input = deep_clone @output['Cmd'] + @input = { 'Cmd' => deep_clone(@output['Cmd']) } @opts = {} end context 'creates container' do after(:example) do expect(Docker::Container).to receive(:create).with(deep_clone @output).and_return(:container) - expect(method(@input,@opts)).to eq(:container) + expect(method(@input)).to eq(:container) end - it 'creates container' do + it 'creates container and appends options' do + @input['name'] = 'name1' + @output['name'] = 'name1' end it 'creates container and appends options' do - @opts['name'] = 'name1' + @input['name'] = 'name1' @output['name'] = 'name1' end + it 'creates container with array argument' do + @input = @input['Cmd'] + end + it 'creates container with hash argument' do + end it 'creates container and string only cmd array' do + @input['Cmd'] << 100 @output['Cmd'] << '100' - @input << 100 end it 'overrides default options' do - @opts['Image'] = 'dead' + @input['Image'] = 'dead' @output['Image'] = 'dead' + end + it 'logs container creation' do + expect(dbl_logger).to receive(:debug) do |&block| + expect(block.call).to match(/created container/) + end end end - it 'fail if nil cmd' do + it 'fail if nil input' do @input = nil - @fail = true - expect{method(@input,@opts)}.to raise_error(ArgumentError, /no array/) + expect{method(@input)}.to raise_error(ArgumentError, /has to be a hash/) + end + it 'fail if nil cmd' do + @input['Cmd'] = nil + expect{method(@input)}.to raise_error(ArgumentError, /no array/) + end + end + describe '#to_s' do + before(:example) do + expect(a_i).to receive(:id).and_return(:id1) + d = instance_double('DockerImage', :name => :name1) + inst_set(:@image, d) + end + after(:example) do + result = method + expect(result).to be_a(String) + expect(result).to match(/id1/) + expect(result).to match(/name1/) + end + it 'should return string repr' do + end + end + describe '#id' do + before(:example) do + @dbl_container = instance_double(Docker::Container) + end + after(:example) do + expect(a_i).to receive(:container).and_return(@dbl_container) + expect(method).to eq(@result) + end + it 'should return id' do + allow(@dbl_container).to receive(:id).and_return(:id1) + @result = :id1 + end + it 'should handle errors' do + allow(@dbl_container).to receive(:id).and_raise(RuntimeError,:test_error) + @result = 'unknown' end end end diff --git a/spec/unit/launcher_spec.rb b/spec/unit/launcher_spec.rb index 994e468..073bd6e 100644 --- a/spec/unit/launcher_spec.rb +++ b/spec/unit/launcher_spec.rb @@ -27,37 +27,6 @@ let (:a_c){ described_class } - xdescribe '#initialize' do - let (:method){ - expect_any_instance_of(described_class).to receive(:test) - a_i_only - } - it 'should not raise error' do - expect{method}.not_to raise_error - end - end - xdescribe '#test' do - before(:example) { - expect_any_instance_of(described_class).to receive(:test).and_call_original - @downstream_methods = [:test_directories] - } - let (:method){ - a_i_only - } - it 'should call down stream functions' do - @downstream_methods.each do |m| - expect_any_instance_of(described_class).to receive(m) - end - method - end - it 'should exit on runtime errors' do - @downstream_methods.each do |m| - allow_any_instance_of(described_class).to receive(m).and_raise(RuntimeError,"error") - end - expect_any_instance_of(described_class).to receive(:exit).with(1) - method - end - end describe '#run' do before (:example) { @pid = 12345 diff --git a/spec/unit/pool_spec.rb b/spec/unit/pool_spec.rb index bae4dd0..a4545d7 100644 --- a/spec/unit/pool_spec.rb +++ b/spec/unit/pool_spec.rb @@ -70,7 +70,7 @@ @list = a_i.spawn_command } it 'list include spawn_fcgi' do - expect(@list).to include(dbl_launcher.spawn_cmd_path) + expect(@list).to include(dbl_docker_image.spawn_fcgi_path) end it 'list include socket path' do expect(@list).to include(default_config['listen']) @@ -108,7 +108,7 @@ @list = method } it 'list include php cmd' do - expect(@list).to include(dbl_launcher.php_cmd_path) + expect(@list).to include(dbl_docker_image.php_path) end it 'list is flat' do expect(@list.flatten).to eq(@list)