service.running: failure to start with enable: True reports no changes / no errors

[mgwilliams]

I believe there is a bug in service.running.

If a service fails to start, but enable: is provided, it returns only the status of service._enable and fails to return the failure to start.

[terminalmage]

Thanks, I'll try to reproduce. What distro/version are you running? We have multiple service backends, so it might be an issue with the backend rather than the state itself.

[terminalmage]

Actually, this doesn't look like a bug at all. Did you actually experience this problem, or is this report just from looking at the code? If you look at line 295 in this block, the False result is indeed returned.

    changes = {name: __salt__['service.start'](name)}

    if not changes[name]:
        ret['result'] = False
        ret['comment'] = 'Service {0} failed to start'.format(name)
        if enable is True:
            ret = _enable(name, False, **kwargs)
            ret['result'] = False
        elif enable is False:
            return _disable(name, False, **kwargs)
        else:
            return ret

[terminalmage]

If anything, it looks like there might be an issue with the return data if enable is False.

[mgwilliams]

I did experience it. If I'm reading correctly, if the service fails to start and enable is True, it doesn't actually return in this block. Instead, it flows on to here:

    if enable is True:
        return _enable(name, True, **kwargs)

[terminalmage]

If the service fails to start, then changes[name] will be False, as shown in my example above.

[terminalmage]

Ahh I see what is going on here. There is no return after the result is set to False.

[mgwilliams]

Yes, so it continues on and runs _enable a second time, and returns the result of that.

[terminalmage]

The _enable and _disable functions need to be smarter about how they set the result value. I'll audit them and make sure they're returning the proper info, this should take care of the issue. Thanks for the report.

[basepi]

@mgwilliams Did your pull request resolve this issue? Can it be closed?

[mgwilliams]

@basepi Yes, it did. Thanks!

[ruimarinho]

Using latest git code (0.17.x), I'm having a somewhat similar issue - a failure to start a service is not correctly reported back to the master as such.

Here's the output from a manual call on the minion shell:

/etc/init.d/clamav-daemon start
[FAIL] Clamav signatures not found in /var/lib/clamav ... failed!
[FAIL] Please retrieve them using freshclam ... failed!
[FAIL] Then run '/etc/init.d/clamav-daemon start' ... failed!

And the output on the master:

State: - service
Name:      clamav-daemon
Function:  running
    Result:    True
    Comment:   Service clamav-daemon is already enabled, and is running
    Changes:

The state is basic:

clamav-daemon:
  pkg:
    - installed
  service:
    - running
    - enable: True
    - require:
      - pkg: clamav-daemon

I'm on Debian 7 (Wheezy).

[basepi]

@ruimarinho Could you create a new issue? I'm guessing it's a slightly different bug.

[ruimarinho]

Of course, will do. Thanks for the feedback.