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

Fix checking for service started status on Debian #6

Merged
merged 2 commits into from Dec 4, 2013

Conversation

Projects
None yet
4 participants
@benlangfeld
Contributor

benlangfeld commented Dec 3, 2013

service SERVICE_NAME status should return a 0 exit code when running, or non-zero when not running. Piping into grep swallows this, and serves no apparent purpose. This results in services always appearing running as long as the command outputs something including 'running' to stdout, including strings like 'SERVICE_NAME is not running'.

Fix checking for service started status on Debian
`service SERVICE_NAME status` should return a 0 exit code when running, or non-zero when not running. Piping into grep swallows this, and serves no apparent purpose. This results in services *always* appearing running as long as the command outputs something including 'running' to stdout, including strings like 'SERVICE_NAME is not running'.
@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Dec 3, 2013

Owner

Upstart base services return exit code 0 even if they are stopped.

For example,

$ service mysql status
mysql stop/waiting

$ echo $?
0

So you cannot know whether service is running or not only with exit code.

Please see mizzy/serverspec#130

Owner

mizzy commented Dec 3, 2013

Upstart base services return exit code 0 even if they are stopped.

For example,

$ service mysql status
mysql stop/waiting

$ echo $?
0

So you cannot know whether service is running or not only with exit code.

Please see mizzy/serverspec#130

@mizzy mizzy closed this Dec 3, 2013

@benlangfeld

This comment has been minimized.

Show comment
Hide comment
@benlangfeld

benlangfeld Dec 3, 2013

Contributor

So what happens when using SysV, which is what Debian still uses (not Ubuntu)? This isn't resolved, I'd appreciate the issue being left open until we figure out how to fix this since it's a real bug.

Contributor

benlangfeld commented Dec 3, 2013

So what happens when using SysV, which is what Debian still uses (not Ubuntu)? This isn't resolved, I'd appreciate the issue being left open until we figure out how to fix this since it's a real bug.

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Dec 3, 2013

Owner

OK, I will reopen this issue.

Owner

mizzy commented Dec 3, 2013

OK, I will reopen this issue.

@mizzy mizzy reopened this Dec 3, 2013

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Dec 3, 2013

Owner

I don't use Debian, so I don't know the detail of this problem.But removing the lines breaks the compatibility, so please fix it by another way.

Owner

mizzy commented Dec 3, 2013

I don't use Debian, so I don't know the detail of this problem.But removing the lines breaks the compatibility, so please fix it by another way.

@benlangfeld

This comment has been minimized.

Show comment
Hide comment
@benlangfeld

benlangfeld Dec 3, 2013

Contributor

I guess if we can assume Ubuntu is always Upstart and Debian is always SysV, we could implement a Ubuntu provider and move this there... Does that sound like an acceptable solution? @bklang, thoughts?

Contributor

benlangfeld commented Dec 3, 2013

I guess if we can assume Ubuntu is always Upstart and Debian is always SysV, we could implement a Ubuntu provider and move this there... Does that sound like an acceptable solution? @bklang, thoughts?

Checking services running on Ubuntu should assume Upstart
Upstart does not return non-zero when a service is not running, so must grep for 'running'. On debian, this hides services that are not running but which log something like "not running".
@benlangfeld

This comment has been minimized.

Show comment
Hide comment
@benlangfeld

benlangfeld Dec 3, 2013

Contributor

How does that solution look?

Contributor

benlangfeld commented Dec 3, 2013

How does that solution look?

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Dec 4, 2013

Owner

Thanks! It seems good to me.

Owner

mizzy commented Dec 4, 2013

Thanks! It seems good to me.

mizzy added a commit that referenced this pull request Dec 4, 2013

Merge pull request #6 from benlangfeld/patch-1
Fix checking for service started status on Debian

@mizzy mizzy merged commit 0fbbee3 into mizzy:master Dec 4, 2013

1 check passed

default The Travis CI build passed
Details
@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Dec 4, 2013

Owner

I've just released as v0.0.7. Thanks!

Owner

mizzy commented Dec 4, 2013

I've just released as v0.0.7. Thanks!

mizzy added a commit to mizzy/serverspec that referenced this pull request Dec 4, 2013

@martinb3

This comment has been minimized.

Show comment
Hide comment
@martinb3

martinb3 Jul 29, 2014

Contributor

Not all init scripts return 'running,' so this doesn't work for all cases.

Contributor

martinb3 commented Jul 29, 2014

Not all init scripts return 'running,' so this doesn't work for all cases.

@mizzy

This comment has been minimized.

Show comment
Hide comment
@mizzy

mizzy Jul 29, 2014

Owner

Patches are welcome.

Owner

mizzy commented Jul 29, 2014

Patches are welcome.

@jorhett

This comment has been minimized.

Show comment
Hide comment
@jorhett

jorhett Mar 30, 2016

Easy fix: also check for "done", as init scripts are supposed to use the lsb functions, and check out line 118 here http://apt-browse.org/browse/ubuntu/trusty/main/all/lsb-base/4.1%2BDebian11ubuntu6/file/lib/lsb/init-functions.d/50-ubuntu-logging

jorhett commented Mar 30, 2016

Easy fix: also check for "done", as init scripts are supposed to use the lsb functions, and check out line 118 here http://apt-browse.org/browse/ubuntu/trusty/main/all/lsb-base/4.1%2BDebian11ubuntu6/file/lib/lsb/init-functions.d/50-ubuntu-logging

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