Services are not restarted if the only change is systemd unit

[Dmitriy Rabotyagov]

systemd_service role handles changed unit restart only when service state is not provided in the structure:
https://opendev.org/openstack/ansible-role-systemd_service/src/commit/59736cb4af5330260bb456e454e08e3d1c9da607/handlers/main.yml#L33

This behaviour is valid, as we might want to have state: stopped, which does not require service restart and will be handled here:
https://opendev.org/openstack/ansible-role-systemd_service/src/branch/master/tasks/systemd_load.yml#L23

We also can't provide state: restarted, as then service will be restarted even when it doesn't need to be.

Let's take uwsgi role as example. 2 possible solutions to solve that bug would be:
1. Add "systemd service changed" to listen. That will result in uwsgi handlers restart the service when unit is changed
   https://opendev.org/openstack/ansible-role-uwsgi/src/commit/e157a116e9f4b4c20d3b49f35e2906b6dc853ab9/handlers/main.yml#L29
2. Remove state from service definition. This will allow systemd handler to restart service whenever needed.
   https://opendev.org/openstack/ansible-role-uwsgi/src/commit/e157a116e9f4b4c20d3b49f35e2906b6dc853ab9/tasks/uwsgi_post_install.yml#L48

I'm inclined to follow first step. As I assume that during initial bootstrap we might want to have services started, but we also need to restart them afterwards. But during initial bootstrap we don't see issues, as changed systemd unit is not the only change, so role handlers are also triggered.

[Damian Dąbrowski (damiandabrowski)]

If I understand correctly, option 1. requires to add a fix to all roles that use of ansible-role-systemd_service, right? If so, it won't solve the issue for people who use that role outside OSA.

Maybe we can change:

- 'services_results.item.state is not defined'

to:

- services_results.item.state is not defined or services_results.item.state != 'stopped'

?

[Dmitriy Rabotyagov (noonedeadpunk)]

After some thinking I've realized that second option is not an option at all. As we want to start service during systemd role runtime, but we also want to restart it afterwards.

So I'm replacing second option with following one:
2. Change systemd role condition to:
"'services_results.item.state is not defined' OR services_results.item.state == 'started'"
https://opendev.org/openstack/ansible-role-systemd_service/src/commit/59736cb4af5330260bb456e454e08e3d1c9da607/handlers/main.yml#L33

UPD: we can't compare to 'stopped' as in case it's state == 'restarted' or 'reloaded' - it will be restarted twice. So then we should compare to 'started'

Eventually more proper fix would be 1st option, but that requires patching of all existing roles. And indeed it's unobvious for usage outside of the osa. And this single condition change should be quite backportable as well.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ansible-role-systemd_service/+/876083

[Dmitriy Rabotyagov (noonedeadpunk)]

Ok, nasty part of the new option 2 (where we're changing condition). So if not only systemd unit is changed - service is gonna be restarted twice - one with systemd role and second time with native role handlers.
So option 1 is kind of the only neat option then.

[Dmitriy Rabotyagov (noonedeadpunk)]

Also for option one, if we want to use in-role handlers that listen for "systemd service changed" - we will need to somehow check if the service is actually should be enabled/started. And we can't filter disabled services out as they won't be passed to systemd role and disabled if they should be.

So it not pleasant situation from any angle. Which makes me think in reverse direction, if we should to always notify `systemd service changed` from roles and drop in-role handlers to prevent mutiple restarts. Just leave handlers for services that are not managed with systemd_service.

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Dmitriy Rabotyagov <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/ansible-role-systemd_service/+/876083

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/876328

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/876328
Committed: https://opendev.org/openstack/openstack-ansible-os_glance/commit/f7c7e4864e899cd2404cea3b08495221e37f523d
Submitter: "Zuul (22348)"
Branch: master

commit f7c7e4864e899cd2404cea3b08495221e37f523d
Author: Dmitriy Rabotyagov <email address hidden>
Date: Fri Mar 3 13:59:06 2023 +0100

    Ensure service is restarted on unit file changes

    At the moment we don't restart services if systemd unit file is changed.

    We knowingly prevent systemd_service role handlers to execute
    by providing `state: started` as otherwise service will be restarted twice.
    With that now we ensure that role handlers will also listen for systemd
    unit changes.

    Change-Id: I5a52c0de14ee3a6215edb64dbc3bd48512d57e2e
    Closes-Bug: #2009029

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/881980

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/882006

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/882035

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Dmitriy Rabotyagov <email address hidden>" on branch: stable/xena
Review: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/882035

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/882006
Committed: https://opendev.org/openstack/openstack-ansible-os_glance/commit/e3350a84aa20b8bcc16d724b13087f66b8ba12c1
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit e3350a84aa20b8bcc16d724b13087f66b8ba12c1
Author: Dmitriy Rabotyagov <email address hidden>
Date: Fri Mar 3 13:59:06 2023 +0100

    Ensure service is restarted on unit file changes

    At the moment we don't restart services if systemd unit file is changed.

    We knowingly prevent systemd_service role handlers to execute
    by providing `state: started` as otherwise service will be restarted twice.
    With that now we ensure that role handlers will also listen for systemd
    unit changes.

    Change-Id: I5a52c0de14ee3a6215edb64dbc3bd48512d57e2e
    Closes-Bug: #2009029

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/openstack-ansible-os_glance/+/881980
Committed: https://opendev.org/openstack/openstack-ansible-os_glance/commit/c4886e01dfd6e960b0621dd1951360228f7612c9
Submitter: "Zuul (22348)"
Branch: stable/zed

commit c4886e01dfd6e960b0621dd1951360228f7612c9
Author: Dmitriy Rabotyagov <email address hidden>
Date: Fri Mar 3 13:59:06 2023 +0100

    Ensure service is restarted on unit file changes

    At the moment we don't restart services if systemd unit file is changed.

    We knowingly prevent systemd_service role handlers to execute
    by providing `state: started` as otherwise service will be restarted twice.
    With that now we ensure that role handlers will also listen for systemd
    unit changes.

    Change-Id: I5a52c0de14ee3a6215edb64dbc3bd48512d57e2e
    Closes-Bug: #2009029

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/openstack-ansible-os_glance yoga-eom release.