Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Re-)start service with notify #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wsmirnow
Copy link
Contributor

It is a good idea using Ansible good practices re-/starting services only if needed. This should be done with notify.

It is a good idea using Ansible good practices re-/starting services only if needed. This should be done with notify.
@lkiesow
Copy link
Contributor

lkiesow commented Dec 17, 2024

This doesn't make sense to me.

  • A Debian package install should automatically start the service. But why should this trigger an additional restart?
  • Similar for RPMs: It doesn't start automatically for an install, but would do a restart on update if necessary. Why should ansible trigger an additional restart?
  • You removed state: started. Ansible should describe the expected state. At the end of the run, I expect the service to be started. If I stop the service and then run the script, with your changes, you wouldn't get a running OpenSearch.

@wsmirnow
Copy link
Contributor Author

This doesn't make sense to me.

But it is. Not using the power of notify/handlers in Ansible is like not using a for-each loops in any programming language.

  • A Debian package install should automatically start the service. But why should this trigger an additional restart?

Yes, at least with the PR #4 (after your requested change installing plugin with OS package manager). Let me give you an example. You upgrade your Opencast instance from OC <17 to 17. Opensearch is already installed. But #4 will also install the alnalysis-icu plugin. Not having a restart-service notify will end up with running service (due to service state=started) but the plugin is not loaded. With notify, any installation (including dependencies and plugins) will trigger an restart as it should. For Debian it is possible to change the start-service-on-install Policy globally if you want (look somewhere here). Force starting the service (and not follow the System policy) is not a good practice.

  • Similar for RPMs: It doesn't start automatically for an install, but would do a restart on update if necessary. Why should ansible trigger an additional restart?

Same as for Debian.

  • You removed state: started. Ansible should describe the expected state. At the end of the run, I expect the service to be started. If I stop the service and then run the script, with your changes, you wouldn't get a running OpenSearch.

Yes, I would also expect a running service at the end of the playbook run (or at least at the end of the block). And this is the case. The difference is, the service will be started with the new configuration at the end of the Ansible block, not the Ansible role assignment. Then you have a chance to configure it properly before the first run. An other real example: An elasticsearch package (not tested opensearch yet) from OC repo runs fine on EL9 instances. But in some cases the system policy forcing /tmp to reject executable files (and directories?). This will break the elasticsearch service (maybe opensearch as well). Setting tmp storage dir for the service to be /var/lib... will do the job but this must be done before starting the service. If the role force start the service at the end, this will fail and the playbook as well. Using notify will trigger the first service start at the end of the Ansible block. Then you can apply your configuration before handler were running like this:

- name: Setup OpenSearch
  hosts: opensearch
  roles:
    - elan.opencast_opensearch <--will trigger restart service
    - opensearch_org_settings <--will trigger restart service
  <-- The triggers will be applied here

If you stop the service manually, then it is on your own starting it again. Ansible is not a tool to monitor the state. Please use an other tool (e.g. Puppet) for that kind of automation.

@wsmirnow wsmirnow requested a review from lkiesow December 20, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants