# Ansible.builtin.service\_facts incorrect and simplified state

**URL:** <https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836>\
**Category:** Project Discussions\
**Tags:** bug, module, ansible-core\
**Created:** [January 26, 2025, 4:52pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836 "2025-01-26T16:52:05Z")\
**Posts on this page:** 8\
**Page:** 1

<div class="post-metadata">

**Author:** ![NomakCooper](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/nomakcooper/32/7309_2.png) [@NomakCooper](https://forum.ansible.com/u/NomakCooper)\
**Post date:** [January 26, 2025, 4:52pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/1 "2025-01-26T16:52:05Z")

</div>

Hello everyone,

Two days ago, I created a new issue ([**#84607**](https://github.com/ansible/ansible/issues/84607)) on the **ansible/ansible** repository regarding the **`ansible.builtin.service_facts`** module.

I also posted about it on **[r/ansible](https://www.reddit.com/r/ansible/comments/1i97f8w/ansiblebuiltinservice_facts_new_issue_84607_for/)**.

While working with hosts that use `systemd` , I noticed that the module can only return two possible state values:

- `stopped`
- `running`

In the module’s code, within the **`SystemctlScanService`** class, the **`state_val`** value is set to **`stopped`** by default and only changes to **`running`** if the **`SUB`** parameter from **`systemctl`** is explicitly **`running`**.

at **276** line :

```python
state_val = "stopped"

```

at **292** : **293** lines:

```python
if fields[3] == "running":
    state_val = "running"

```

**`fields[3]`** is the real state of **`SUB`** from this **`systemctl`** command:

```bash
systemctl list-units --no-pager --type service --all --plain

```

This approach means the module will never reflect the actual state of the service. Instead, any service that isn’t **`running`** is simply reported as **`stopped`**.

Currently, the **`SUB`** parameter from **`systemctl`** can represent various states, such as:

- **`dead`**
- **`exited`**
- **`reloaded`**
- **`failed`**

This makes it impossible to use the module for performing precise checks on the state of services managed by **`systemd`** and, consequently, to implement solutions based on their actual state.

Am I the only one who thinks this approach is overly simplistic?

---

<div class="post-metadata">

**Author:** ![felixfontein](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/felixfontein/32/7530_2.png) [@felixfontein](https://forum.ansible.com/u/felixfontein)\
**Post date:** [January 26, 2025, 5:51pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/2 "2025-01-26T17:51:17Z")

</div>

It really depends on what you want to do with the result. If you are just interested in whether the service is running or not, the current values are perfect. (_Especially_ the value `reloaded` is often not helpful, you want `running` instead.) If you want more details on why a service is not running, they are not.

It’s probably best to add a new return value that gives the exact state. Changing the existing state potentially breaks a lot of playbooks/roles that assume there are only `running` or `stopped`.

(There are more states currently returned BTW: on my system I also see `active`, `inactive`, and `unknown`.)

---

<div class="post-metadata">

**Author:** ![NomakCooper](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/nomakcooper/32/7309_2.png) [@NomakCooper](https://forum.ansible.com/u/NomakCooper)\
**Post date:** [January 26, 2025, 6:09pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/3 "2025-01-26T18:09:08Z")

</div>

What I expect from a module like this is that the state value reflects the exact state, not a “placeholder” value.

To give an example, the main issue is that a service in a **`failed`** or **`exited`** state is not the same as being **`stopped`**.

A service is considered **`stopped`** only if the shutdown process completed without errors.  
**`Failed`** or **`exited`** are not equivalent to **`stopped`** and indicate an issue with the service.

In the issue I opened, I shared my thoughts and proposed a change to replace the **`state_val`** value with the **`SUB`** state.

As you mentioned, changing the existing **`state`** field might cause problems for all currently implemented playbooks/roles.

Perhaps the best solution would be to add a new value, **`sub_state`** , that reflects the exact state from **`systemctl`** , but only when the source is **`systemd`**.

For example, like this:

```python
class SystemctlScanService(BaseService):

    BAD_STATES = frozenset(['not-found', 'masked', 'failed'])

    def systemd_enabled(self):
        return is_systemd_managed(self.module)

    def _list_from_units(self, systemctl_path, services):

        # list units as systemd sees them
        rc, stdout, stderr = self.module.run_command("%s list-units --no-pager --type service --all --plain" % systemctl_path, use_unsafe_shell=True)
        if rc != 0:
            self.module.warn("Could not list units from systemd: %s" % stderr)
        else:
            for line in [svc_line for svc_line in stdout.split('\n') if '.service' in svc_line]:

                state_val = "stopped"
                status_val = "unknown"
                fields = line.split()

                # systemd sometimes gives misleading status
                # check all fields for bad states
                for bad in self.BAD_STATES:
                    # except description
                    if bad in fields[:-1]:
                        status_val = bad
                        break
                else:
                    # active/inactive
                    status_val = fields[2]

                service_name = fields[0]
                sub_state_val = fields[3]
                if sub_state_val == "running":
                    state_val = "running"

                services[service_name] = {"name": service_name, "state": state_val, "sub_state": sub_state_val, "status": status_val, "source": "systemd"}

```

---

<div class="post-metadata">

**Author:** ![NomakCooper](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/nomakcooper/32/7309_2.png) [@NomakCooper](https://forum.ansible.com/u/NomakCooper)\
**Post date:** [January 30, 2025, 7:53pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/4 "2025-01-30T19:53:47Z")

</div>

I created [#84618](https://github.com/ansible/ansible/pull/84618) PR , adding the return of the new **`sub_state`** value for hosts using **`systemd`**.

- Added the **`sub_state`** value to the module.
- Added documentation in module.
- Added the **`changelogs/fragments`** file.
- Included comments on the test playbook and result in **`ansible-core 2.18.2`**.

All checks have passed ✅

---

<div class="post-metadata">

**Author:** ![felixfontein](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/felixfontein/32/7530_2.png) [@felixfontein](https://forum.ansible.com/u/felixfontein)\
**Post date:** [February 1, 2025, 10:07am UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/5 "2025-02-01T10:07:01Z")

</div>

I added a few review comments. You should also include some basic integration tests for this (since it’s a new feature), i.e. make sure that the field is present for systemd services and make sure that the values are as expected at least in a few cases where it can be reliably guessed.

Note that the core team might ignore the PR (or at least not merge it) until data tagging gets merged, since they want to avoid creating additional conflicts to the data tagging PR (which touches almost all files).

---

<div class="post-metadata">

**Author:** ![NomakCooper](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/nomakcooper/32/7309_2.png) [@NomakCooper](https://forum.ansible.com/u/NomakCooper)\
**Post date:** [February 1, 2025, 2:14pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/6 "2025-02-01T14:14:47Z")

</div>

@felixfontein thank you very much for the review comments (my mistake 😅 )  
I’ll definitely commit all the suggested changes.

Regarding the **`integration test`** , should I modify the current test by **adding** a check for the **`sub_state`** value, or should I completely replace the **existing** test with one focused only on the **`sub_state`** value?

---

<div class="post-metadata">

**Author:** ![NomakCooper](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/nomakcooper/32/7309_2.png) [@NomakCooper](https://forum.ansible.com/u/NomakCooper)\
**Post date:** [February 1, 2025, 9:25pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/7 "2025-02-01T21:25:30Z")

</div>

I have added changes to run integration tests for the **`sub_state`** value:

- Modified the original file **`test/integration/targets/service_facts/tasks/main.yml`**
  - Added a new block to avoid interfering with the existing **`disabled test`**

- Added a new file **`test/integration/targets/service_facts/tasks/subtests.yml`**

In this new test, the **`sub_state`** value will be extracted in several key scenarios to observe how the **`sub_state`** value reflects on the state or takes on different values.

The test uses the already present **`ansible_test`** service in integration.

Scenarios:

| state | enabled |
| --- | --- |
| started | yes |
| stopped | yes |
| stopped | no |
| started | no |

- In the penultimate test, the main process is intentionally **`killed`** to force the service into a **`failed`** state.

- In the final test, the **`.service`** file is modified, then reload and restart to force the service into an **`exited`** state immediately after starting.

In this last test, you can see how the module reports the following values:

```json
"ansible_test.service": {
    "name": "ansible_test.service", 
    "source": "systemd", 
    "state": "stopped", 
    "status": "enabled", 
    "sub_state": "exited"
}

```

---

<div class="post-metadata">

**Author:** ![NomakCooper](https://sea1.discourse-cdn.com/ansible/user_avatar/forum.ansible.com/nomakcooper/32/7309_2.png) [@NomakCooper](https://forum.ansible.com/u/NomakCooper)\
**Post date:** [February 2, 2025, 8:54pm UTC](https://forum.ansible.com/t/ansible-builtin-service-facts-incorrect-and-simplified-state/39836/8 "2025-02-02T20:54:13Z")

</div>

FYI

I have rewritten the **`integration test`** to make it more **`efficient`** and easier to **`maintain`**.

- Separated the standard operation tests into **`standard_operation.yml`** using **`include_tasks`** in a **`loop`**.
- Isolated the kill test into **`kill_operation.yml`**.
- Moved the test for the **`exited`** service into **`exited_operation.yml`**.
  - This test replaces the **`ansible_test.systemd`** file with **`exited_ansible.systemd`**.

new tree:

```gitbash
test/integration/targets/service_facts/
├── aliases
├── files
│ ├── ansible.systemd
│ ├── ansible_test_service.py
│ └── exited_ansible.systemd
└── tasks
    ├── exited_operation.yml
    ├── kill_operation.yml
    ├── main.yml
    ├── standard_operation.yml
    ├── subtests.yml
    ├── systemd_cleanup.yml
    ├── systemd_setup.yml
    └── tests.yml

```

All checks have passed ✅