From 0ec440bbeb1152c193b079d8c355d27aec34a302 Mon Sep 17 00:00:00 2001 From: uu59 Date: Tue, 18 Nov 2014 11:12:10 +0900 Subject: [PATCH 1/5] fluent-gem commannd move to FluentGem module from Plugin --- app/models/fluent_gem.rb | 53 ++++++++++++++++++++++++++++++++++++++ app/models/plugin.rb | 42 +++--------------------------- spec/models/plugin_spec.rb | 18 ++++++------- 3 files changed, 65 insertions(+), 48 deletions(-) create mode 100644 app/models/fluent_gem.rb diff --git a/app/models/fluent_gem.rb b/app/models/fluent_gem.rb new file mode 100644 index 0000000..6db39b8 --- /dev/null +++ b/app/models/fluent_gem.rb @@ -0,0 +1,53 @@ +module FluentGem + class GemError < StandardError; end + + class << self + def install(*args) + run("install", *args) + end + + def uninstall(*args) + run("uninstall", *args) + end + + def list + output = `#{gem} list` + unless $?.exitstatus.zero? + raise GemError, "failed command `#{gem} list`" + end + output.lines + end + + def run(*args) + # NOTE: use `fluent-gem` instead of `gem` + Bundler.with_clean_env do + # NOTE: this app is under the Bundler, so call `system` in with_clean_env is Bundler jail breaking + unless system(* [gem, *args]) + raise GemError, "failed command #{gem} #{args.join(" ")}" + end + end + true + end + + def gem + # Not yet setup any fluentd/td-agent + return "fluent-gem" unless Fluentd.instance + + # On installed both td-agent and fluentd system, decide which fluent-gem command should be used depend on setup(Fluentd.instance) + if Fluentd.instance && Fluentd.instance.fluentd? + return "fluent-gem" # maybe `fluent-gem` command is in the $PATH + end + + # NOTE: td-agent has a command under the /usr/lib{,64}, td-agent2 has under /opt/td-agent + %W( + /usr/sbin/td-agent-gem + /opt/td-agent/embedded/bin/fluent-gem + /usr/lib/fluent/ruby/bin/fluent-gem + /usr/lib64/fluent/ruby/bin/fluent-gem + fluent-gem + ).find do |path| + system("which #{path}", out: File::NULL, err: File::NULL) + end + end + end +end diff --git a/app/models/plugin.rb b/app/models/plugin.rb index a4b1cfc..5722baa 100644 --- a/app/models/plugin.rb +++ b/app/models/plugin.rb @@ -89,10 +89,7 @@ class Plugin def self.installed Rails.cache.fetch("installed_gems", expires_in: 3.seconds) do Bundler.with_clean_env do - fluent_gem = fluent_gem_path - return [] unless fluent_gem - gems = `#{fluent_gem} list`.try(:lines) - return [] unless gems + gems = FluentGem.list gems.grep(/fluent-plugin/).map do |gem| name, versions_str = gem.strip.split(" ") version = versions_str[/[^(), ]+/] @@ -141,24 +138,6 @@ class Plugin "https://rubygems.org/api/v1/versions/#{gem_name}.json" end - def self.fluent_gem_path - # On installed both td-agent and fluentd system, decide which fluent-gem command should be used depend on setup(Fluentd.instance) - if Fluentd.instance && Fluentd.instance.fluentd? - return "fluent-gem" # maybe `fluent-gem` command is in the $PATH - end - - # NOTE: td-agent has a command under the /usr/lib{,64}, td-agent2 has under /opt/td-agent - %W( - /usr/sbin/td-agent-gem - /opt/td-agent/embedded/bin/fluent-gem - /usr/lib/fluent/ruby/bin/fluent-gem - /usr/lib64/fluent/ruby/bin/fluent-gem - fluent-gem - ).find do |path| - system("which #{path}", out: File::NULL, err: File::NULL) - end - end - private def gem_install @@ -166,7 +145,7 @@ class Plugin return if processing? return if installed? WORKING.push(data) - fluent_gem("install", gem_name, "--no-document", "-v", version) + FluentGem.install(gem_name, "--no-document", "-v", version) ensure WORKING.delete(data) end @@ -176,23 +155,8 @@ class Plugin return if processing? return unless installed? WORKING.push(data) - fluent_gem("uninstall", gem_name, "-x", "-a") + FluentGem.uninstall(gem_name, "-x", "-a") ensure WORKING.delete(data) end - - def fluent_gem(*commands) - # NOTE: use `fluent-gem` instead of `gem` - Bundler.with_clean_env do - # NOTE: this app is under the Bundler, so call `system` in with_clean_env is Bundler jail breaking - unless system(* [fluent_gem_path, *commands]) - raise GemError, "failed command #{commands.join(" ")}" - end - end - true - end - - def fluent_gem_path - self.class.fluent_gem_path - end end diff --git a/spec/models/plugin_spec.rb b/spec/models/plugin_spec.rb index f352a68..d82872c 100644 --- a/spec/models/plugin_spec.rb +++ b/spec/models/plugin_spec.rb @@ -4,7 +4,7 @@ describe Plugin do let(:plugin) { build(:plugin) } describe ".installed" do - before { Plugin.stub(:"`").and_return(gem_list) } + before { FluentGem.stub(:"`").and_return(gem_list) } context "fluent-plugin-foo 0.1.2" do let(:target) { Plugin.new(gem_name: "fluent-plugin-foo", version: "0.1.2") } @@ -73,12 +73,12 @@ describe Plugin do context "installed" do let(:installed) { true } - it { plugin.should_not_receive(:fluent_gem) } + it { FluentGem.should_not_receive(:install) } end context "not installed" do let(:installed) { false } - it { plugin.should_receive(:fluent_gem) } + it { FluentGem.should_receive(:install) } end end @@ -87,22 +87,22 @@ describe Plugin do context "installed" do let(:installed) { true } - it { plugin.should_not_receive(:fluent_gem) } + it { FluentGem.should_not_receive(:install) } end context "not installed" do let(:installed) { false } - it { plugin.should_not_receive(:fluent_gem) } + it { FluentGem.should_not_receive(:installed) } end end end context "system command error" do - before { plugin.should_receive(:system).at_least(1).and_return(false) } + before { FluentGem.should_receive(:system).at_least(1).and_return(false) } subject { expect { plugin.install! } } it "raise GemError" do - subject.to raise_error(Plugin::GemError) + subject.to raise_error(FluentGem::GemError) end it "error message contains gem name" do @@ -139,10 +139,10 @@ describe Plugin do before do # NOTE: not `plugin.stub` because upgrade! creates new Plugin instance internally installed_plugin.stub(:installed?).and_return(true) - Plugin.any_instance.stub(:fluent_gem).and_return(true) + FluentGem.stub(:run).and_return(true) installed_plugin.should_receive(:uninstall!) - Plugin.any_instance.should_receive(:install!) + FluentGem.should_receive(:install) end it { installed_plugin.upgrade!(target_version) } From 2df1991d2b63d61d7dc8dbfac6f87ba726dc8046 Mon Sep 17 00:00:00 2001 From: uu59 Date: Tue, 18 Nov 2014 13:11:58 +0900 Subject: [PATCH 2/5] Add specs --- app/models/fluent_gem.rb | 9 ++- spec/models/fluent_gem_spec.rb | 112 +++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 spec/models/fluent_gem_spec.rb diff --git a/app/models/fluent_gem.rb b/app/models/fluent_gem.rb index 6db39b8..f1124c1 100644 --- a/app/models/fluent_gem.rb +++ b/app/models/fluent_gem.rb @@ -22,8 +22,9 @@ module FluentGem # NOTE: use `fluent-gem` instead of `gem` Bundler.with_clean_env do # NOTE: this app is under the Bundler, so call `system` in with_clean_env is Bundler jail breaking - unless system(* [gem, *args]) - raise GemError, "failed command #{gem} #{args.join(" ")}" + cmd = [gem, *args].compact + unless system(*cmd) + raise GemError, "failed command: `#{cmd.join(" ")}`" end end true @@ -36,8 +37,12 @@ module FluentGem # On installed both td-agent and fluentd system, decide which fluent-gem command should be used depend on setup(Fluentd.instance) if Fluentd.instance && Fluentd.instance.fluentd? return "fluent-gem" # maybe `fluent-gem` command is in the $PATH + else + detect_td_agent_gem end + end + def detect_td_agent_gem # NOTE: td-agent has a command under the /usr/lib{,64}, td-agent2 has under /opt/td-agent %W( /usr/sbin/td-agent-gem diff --git a/spec/models/fluent_gem_spec.rb b/spec/models/fluent_gem_spec.rb new file mode 100644 index 0000000..10c0f8a --- /dev/null +++ b/spec/models/fluent_gem_spec.rb @@ -0,0 +1,112 @@ +require 'spec_helper' + +describe FluentGem do + describe "invoker" do + describe "#install" do + let(:gem) { FluentGem.gem } + + context "no argument" do + after { FluentGem.install } + it { FluentGem.should_receive(:run).with("install") } + end + + context "with arguments" do + after { FluentGem.install(*args) } + + context "1" do + let(:args) { ["plugin-foo"] } + it { FluentGem.should_receive(:run).with("install", *args) } + end + + context "2" do + let(:args) { ["plugin-foo", "--no-document"] } + it { FluentGem.should_receive(:run).with("install", *args) } + end + end + end + + describe "#uninstall" do + let(:gem) { FluentGem.gem } + + context "no argument" do + after { FluentGem.uninstall } + it { FluentGem.should_receive(:run).with("uninstall") } + end + + context "with arguments" do + after { FluentGem.uninstall(*args) } + + context "1" do + let(:args) { ["plugin-foo"] } + it { FluentGem.should_receive(:run).with("uninstall", *args) } + end + + context "2" do + let(:args) { ["plugin-foo", "--no-document"] } + it { FluentGem.should_receive(:run).with("uninstall", *args) } + end + end + end + end + + describe "#list" do + before { FluentGem.stub(:`).and_return(gem_list) } + subject { FluentGem.list } + + context "no list" do + let(:gem_list) { "" } + it { subject.should == [] } + end + + context "some lines" do + let(:gem_list) { <<-GEM.strip_heredoc } + dummy (3.3.3) + fluent-plugin-foo (0.1.2) + more_dummy (0.0.1) + GEM + it { subject.should == gem_list.lines } + end + + context "failed" do + let(:gem_list) { "" } + before { $?.stub(:exitstatus).and_return(128) } + it { expect{ subject }.to raise_error(FluentGem::GemError) } + end + end + + describe "#run" do + before { FluentGem.stub(:system).and_return(ret) } + let(:args) { ["install", "foobar"] } + + describe "success" do + let(:ret) { true } + after { FluentGem.run(*args) } + it { FluentGem.should_receive(:system) } + end + + describe "failed" do + let(:ret) { false } + it { expect{ FluentGem.run(*args) }.to raise_error(FluentGem::GemError) } + end + end + + describe "#gem" do + before { Fluentd.stub(:instance).and_return(instance) } + subject { FluentGem.gem } + + context "any instance not setup yet" do + let(:instance) { nil } + it { should == "fluent-gem" } + end + + context "fluentd setup" do + let(:instance) { Fluentd.new(id: nil, variant: "fluentd_gem", log_file: "dummy.log", pid_file: "dummy.pid", config_file: "dummy.conf") } + it { should == "fluent-gem" } + end + + context "td-agent 2 setup" do + let(:instance) { Fluentd.new(id: nil, variant: "td_agent", log_file: "dummy.log", pid_file: "dummy.pid", config_file: "dummy.conf") } + it { should == FluentGem.detect_td_agent_gem } + end + end +end From abb98e70c1d754c92fb931e6a308f72d3a221a50 Mon Sep 17 00:00:00 2001 From: uu59 Date: Tue, 18 Nov 2014 13:15:40 +0900 Subject: [PATCH 3/5] Remove deprecation warnings --- spec/lib/file_reverse_reader_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/file_reverse_reader_spec.rb b/spec/lib/file_reverse_reader_spec.rb index cc5a1f5..9f6b481 100644 --- a/spec/lib/file_reverse_reader_spec.rb +++ b/spec/lib/file_reverse_reader_spec.rb @@ -39,12 +39,12 @@ describe FileReverseReader do context "contain ascii only" do let(:content) { "ABCDE" } - it { should be_false } + it { should == false } end context "contain non-ascii" do let(:content) { "\x89NG" } - it { should be_true } + it { should == true } end end From 0e922982a86f2b88d7bfb0b3a949e0211402ce54 Mon Sep 17 00:00:00 2001 From: uu59 Date: Tue, 18 Nov 2014 13:19:49 +0900 Subject: [PATCH 4/5] cosme --- app/models/fluent_gem.rb | 4 +- spec/models/fluent_gem_spec.rb | 70 +++++++++++++++++----------------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/app/models/fluent_gem.rb b/app/models/fluent_gem.rb index f1124c1..d0241c5 100644 --- a/app/models/fluent_gem.rb +++ b/app/models/fluent_gem.rb @@ -13,7 +13,7 @@ module FluentGem def list output = `#{gem} list` unless $?.exitstatus.zero? - raise GemError, "failed command `#{gem} list`" + raise GemError, "failed command: `#{gem} list`" end output.lines end @@ -36,7 +36,7 @@ module FluentGem # On installed both td-agent and fluentd system, decide which fluent-gem command should be used depend on setup(Fluentd.instance) if Fluentd.instance && Fluentd.instance.fluentd? - return "fluent-gem" # maybe `fluent-gem` command is in the $PATH + "fluent-gem" # maybe `fluent-gem` command is in the $PATH else detect_td_agent_gem end diff --git a/spec/models/fluent_gem_spec.rb b/spec/models/fluent_gem_spec.rb index 10c0f8a..aea5cd1 100644 --- a/spec/models/fluent_gem_spec.rb +++ b/spec/models/fluent_gem_spec.rb @@ -1,50 +1,48 @@ require 'spec_helper' describe FluentGem do - describe "invoker" do - describe "#install" do - let(:gem) { FluentGem.gem } + describe "#install" do + let(:gem) { FluentGem.gem } - context "no argument" do - after { FluentGem.install } - it { FluentGem.should_receive(:run).with("install") } - end - - context "with arguments" do - after { FluentGem.install(*args) } - - context "1" do - let(:args) { ["plugin-foo"] } - it { FluentGem.should_receive(:run).with("install", *args) } - end - - context "2" do - let(:args) { ["plugin-foo", "--no-document"] } - it { FluentGem.should_receive(:run).with("install", *args) } - end - end + context "no argument" do + after { FluentGem.install } + it { FluentGem.should_receive(:run).with("install") } end - describe "#uninstall" do - let(:gem) { FluentGem.gem } + context "with arguments" do + after { FluentGem.install(*args) } - context "no argument" do - after { FluentGem.uninstall } - it { FluentGem.should_receive(:run).with("uninstall") } + context "1" do + let(:args) { ["plugin-foo"] } + it { FluentGem.should_receive(:run).with("install", *args) } end - context "with arguments" do - after { FluentGem.uninstall(*args) } + context "2" do + let(:args) { ["plugin-foo", "--no-document"] } + it { FluentGem.should_receive(:run).with("install", *args) } + end + end + end - context "1" do - let(:args) { ["plugin-foo"] } - it { FluentGem.should_receive(:run).with("uninstall", *args) } - end + describe "#uninstall" do + let(:gem) { FluentGem.gem } - context "2" do - let(:args) { ["plugin-foo", "--no-document"] } - it { FluentGem.should_receive(:run).with("uninstall", *args) } - end + context "no argument" do + after { FluentGem.uninstall } + it { FluentGem.should_receive(:run).with("uninstall") } + end + + context "with arguments" do + after { FluentGem.uninstall(*args) } + + context "1" do + let(:args) { ["plugin-foo"] } + it { FluentGem.should_receive(:run).with("uninstall", *args) } + end + + context "2" do + let(:args) { ["plugin-foo", "--no-document"] } + it { FluentGem.should_receive(:run).with("uninstall", *args) } end end end From 475942f868abd078e4d57db4b68337383a1ddf32 Mon Sep 17 00:00:00 2001 From: uu59 Date: Tue, 18 Nov 2014 13:21:49 +0900 Subject: [PATCH 5/5] Fix assertion as Enumerator#to_a --- spec/models/fluent_gem_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/fluent_gem_spec.rb b/spec/models/fluent_gem_spec.rb index aea5cd1..668b1ba 100644 --- a/spec/models/fluent_gem_spec.rb +++ b/spec/models/fluent_gem_spec.rb @@ -53,7 +53,7 @@ describe FluentGem do context "no list" do let(:gem_list) { "" } - it { subject.should == [] } + it { subject.to_a.should == [] } end context "some lines" do @@ -62,7 +62,7 @@ describe FluentGem do fluent-plugin-foo (0.1.2) more_dummy (0.0.1) GEM - it { subject.should == gem_list.lines } + it { subject.to_a.should == gem_list.lines.to_a } end context "failed" do