New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add `be_monitored_by('god')` for Service resource. #208

Merged
merged 8 commits into from Jul 7, 2013

Conversation

Projects
None yet
2 participants
@kitak
Copy link
Contributor

kitak commented Jul 6, 2013

Serviceリソースのbe_monitored_byマッチャにプロセス監視フレームワークのgodを指定できるようにしました。
以下が記述例になります。

describe service('unicorn') do
  it { should be_monitored_by('god') }
end

各OSのservice_spec.rbにテストを追加して通ることを確認しています。

def check_monitored_by_god(process)
ret = run_command(commands.check_monitored_by_god(process))
ret[:exit_status] == 0 && ret[:stdout] =~ /^#{process}\: up/
end

This comment has been minimized.

@mizzy

mizzy Jul 6, 2013

Owner

該当のプロセスがなければ、exit status 1 を返すみたいだから、exit status 見るだけで十分だと思うんだけど、stdout も見てるのは何か理由があるのかな?

This comment has been minimized.

@kitak

kitak Jul 6, 2013

Contributor

該当プロセスが監視されていない状態で「unmonitored」と出力されてexit status 0を返すので、stdoutの内容を確認しています。

はじめはcommands/base.rbに以下の様なcheck_monitored_by_godメソッドを定義して、method_missingにプロキシさせようと思ったのですが、

def check_monitored_by_god(process)
  "god status #{escape(process)} | grep -- ^#{escape(process)}:\\ up"
end

type/service.rbbackend.respond_to?(:check_monitored_by_god)falseを返してしまい、例外が送出されるので出力を調べる実装をbackend/exec.rbに移しました。
他の解決方法としては、backend/exec.rbrespond_to?メソッドを以下のようにオーバーライドする方法もあると思うのですが、check_monitored_by_godメソッドのためだけにオーバーライドするのも大げさだと思い、現在の形になりました。

def respond_to?(name)
  return true if commands.respond_to? name && name =~ /^check/
  super
end

This comment has been minimized.

@mizzy

mizzy Jul 7, 2013

Owner

あれ、unmonitored だと 1 を返さない?バージョンにもよる?俺が試した0.13.2では1を返すし、master のソースコードでもそうなってるように読めるけど。

https://github.com/mojombo/god/blob/master/lib/god/cli/command.rb#L96

This comment has been minimized.

@mizzy

mizzy Jul 7, 2013

Owner

type/service.rbのbackend.respond_to?(:check_monitored_by_god)がfalseを返してしまい、例外が送出される

については、このせいで backend/exec.rb に明示的に書かないといけない、というのも微妙なので、backend.respond_to? によるチェックは外してもいいかもね。

その場合は、

undefined method `check_monitored_under_hoge' for #<Serverspec::Commands::RedHat:0x007f801c501290>

みたいなエラーになるけど、これでも十分わかるし。

This comment has been minimized.

@mizzy

mizzy Jul 7, 2013

Owner

他の check_running_under_xxxx とかと一貫性ないじゃん、ってツッコミあると思うけど、基本的には、ここで書いた方針に揃えていこうと思ってる。

This comment has been minimized.

@mizzy

mizzy Jul 7, 2013

Owner

試しに supervisor をこんな感じに変えてみた。
#210

こんな風に、backend.respond_to?をcommands.respond_to?に変えると、例外を投げることができるね。

This comment has been minimized.

@kitak

kitak Jul 7, 2013

Contributor

こんな風に、backend.respond_to?をcommands.respond_to?に変えると、例外を投げることができるね。

了解です。commands.respond_to?で確認するように修正しました。

unmonitored だと 1 を返さない?バージョンにもよる?

これは僕の勘違いでした。すいません。Task, Groupを指定せずにgod statusするとunmonitoredなものがあっても0を返すのでその挙動と勘違いしていました。コマンドを叩いてはecho $?で確認していましたが、ソースを直接見にいったほうが確実ですね... 気をつけます。

stdoutを調べないように修正して、method_missingにプロキシさせるようにメソッド定義を削除しました。

describe service('unicorn') do
let(:stdout) { "unicorn: unmonitored\r\n" }
it { should_not be_monitored_by('god') }
its(:command) { should eq "god status unicorn" }

This comment has been minimized.

@mizzy

mizzy Jul 6, 2013

Owner

should_not の場合は its(:command) のテストはなくてもいいよ。should の場合と同じだから。(あっても害はないけど。)

This comment has been minimized.

@kitak

kitak Jul 6, 2013

Contributor

はい!

@mizzy mizzy merged commit 44f6853 into mizzy:master Jul 7, 2013

1 check passed

default The Travis CI build passed
Details
@mizzy

This comment has been minimized.

Copy link
Owner

mizzy commented Jul 7, 2013

ありがとう!マージしました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment