Skip to content

Commit

Permalink
Use return code to determine service status in Upstart module.
Browse files Browse the repository at this point in the history
This also broke the "dead" state.

This is because `service` will call `/etc/init.d/<service-name>` for
System V style services. Each of which could have different output.

For example:

    $ sudo invoke-rc.d exim4 stop
     * Stopping MTA                                               [ OK ]
    $ sudo service exim4 status
     * checking separate queue runner daemon...                   [ OK ]
     * checking combined SMTP listener and queue runner daemon... [ OK ]
    $ echo $?
    3
    $ sudo invoke-rc.d exim4 start
     * Starting MTA                                               [ OK ]
    $ sudo service exim4 status
     * checking separate queue runner daemon...                   [ OK ]
     * checking combined SMTP listener and queue runner daemon... [ OK ]
    $ echo $?
    0
  • Loading branch information
Tom Vaughan committed Apr 24, 2012
1 parent 18b7cb7 commit f5cb036
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion salt/modules/upstart.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def status(name, sig=None):
salt '*' service.status <service name>
'''
cmd = 'service {0} status'.format(name)
return 'start/running' in __salt__['cmd.run'](cmd)
return not bool(__salt__['cmd.retcode'](cmd))


def _get_service_exec():
Expand Down

4 comments on commit f5cb036

@chapel
Copy link

@chapel chapel commented on f5cb036 May 1, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my usage, upstart status is returning true and causing this to think it is running when it isn't.

There needs to be better logic on this, at the very least make sure it is working for upstart as well as System V.

@thatch45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we might need some more serious auditing here, thanks for bringing it up

@chapel
Copy link

@chapel chapel commented on f5cb036 May 1, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tvaughan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This at least restores the default behavior for Upstart jobs. #1161

Please sign in to comment.