daemon-reload on call to service.avaliable

[clinta]

What does this PR do?

Calls _check_for_unit_changes on service.avaliable

What issues does this PR fix or reference?

Related to #34927, but affecting the service.running with enable=true rather than the service.enable state.

Previous Behavior

The following state would automatically detect if a unit file existed and execute a daemon-reload if required to make the service available before enabling it.

myservice:
  service.enabled: []

But this similar state would not.

myservice:
  service.running:
    - enable: True

New Behavior

Both states listed above do daemon-reload before trying to enable the service if changes in the unit file are detected.

Tests written?

No

Please review Salt's Contributing Guide for best practices.

[cachedout]

Looks good to me but would also like @terminalmage to have a look.

[terminalmage]

@clinta I feel like causing available() to call itself (as part of the helper functions) that are called through the course of _check_for_unit_changes() is potentially dangerous. Any changes to _check_for_unit_changes() (or any of the functions that it calls) which invoke available() would need to properly be handled or we'd end up with an "endless" loop, ultimately resulting in exception being raised for exceeding the maximum recursion depth.

I've opened #36626 as a potential alternative. What this does is expose the _check_for_unit_changes() func as a newly-renamed refresh() func. Then, the service.running state will call this function if it is implemented in a service virtual module (like systemd.py).

Would you mind testing my alternate solution?

[clinta]

@terminalmage: Sure, I'll put it in place and let you know.

[clinta]

@terminalmage, This does work. However, with that fix service.available will still return false if there is a unit file available awaiting a daemon_reload.

I use service.available in quite a few jinja templates and this is not the way I'd expect it to behave. I can't think of any situations in my current usages where I'm testing for a service that would be waiting on a daemon-reload, but I'd expect it to work.

[terminalmage]

OK, I just thought of a middle ground. I opened a PR against your fork, see here: clinta#1

What this does is move the logic that checks to see if a unit is available to a separate function that _untracked_custom_unit_found() can also invoke, without re-invoking available().

[clinta]

That looks perfect, and I tested and it works exactly as I expect. Thanks.

[terminalmage]

Great, we'll wait for tests to complete and get this one merged. I've closed my alternative PR.