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

Add reload systemd to run unit generator for st2ctl #5286

Merged
merged 3 commits into from
Oct 2, 2021

Conversation

nzlosh
Copy link
Contributor

@nzlosh nzlosh commented Jun 13, 2021

This PR is part of StackStorm/st2-packages#706 to use st2.confto dynamically generate st2api, st2auth and st2stream socket files.

For this to work seamlessly for most users, an update to st2ctl is required to perform a systemctl daemon-reload which will regenerator the .socket files and pickup any modifications to st2.conf.

Fixes: #3356 StackStorm/st2-packages#686 #2676

@pull-request-size pull-request-size bot added the size/XS PR that changes 0-9 lines. Quick fix/merge. label Jun 13, 2021
Comment on lines +112 to +115
if [ -z $SYSTEMD_RELOADED ]; then
#Reload systemd to regenerate socket files from st2.conf
systemctl daemon-reload
SYSTEMD_RELOADED="yes"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering SYSTEMD_RELOADED state is not saved in any file storage or passed externally, what's the idea here?

Will it run the systemctl daemon-reload on every new st2ctl run?

From that point, is the daemon-reload that triggers system-generator then?

Copy link
Contributor Author

@nzlosh nzlosh Jun 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you've understood the logic correctly. The SYSTEMD_RELOADED is to guard against reloading systemd for every daemon process st2ctl will restart in a single invocation. The systemctl daemon-reload triggers the execution of the generators. This way the socket files are regenerated with each st2ctl restart.

While this may seem inefficient, because the likelihood of st2.conf being changed on each restart is low, the logic required to test st2.conf last modified time and compare it with the age the daemon process being restarted would have added unnecessary complexity. The daemon-reload is ~200ms on my dev machine which is insignificant compare with the time it takes to restart all st2 daemon (12.848s on my dev machine).

For the sake of science, here's the time it takes to restart each of the concerned daemons:

root@u1804:~# time systemctl restart st2api
real    0m4.014s
user    0m0.004s
sys     0m0.003s

root@u1804:~# time systemctl restart st2auth
real    0m1.879s
user    0m0.007s
sys     0m0.000s

root@u1804:~# time systemctl restart st2stream
real    0m1.483s
user    0m0.003s
sys     0m0.004s

@arm4b
Copy link
Member

arm4b commented Jun 15, 2021

Thanks, @nzlosh!

Great item for including in the v3.6.0 👍 Marked it as such.

@cognifloyd cognifloyd added this to the 3.6.0 milestone Aug 7, 2021
@CLAassistant
Copy link

CLAassistant commented Sep 6, 2021

CLA assistant check
All committers have signed the CLA.

@cognifloyd cognifloyd enabled auto-merge October 2, 2021 01:27
@cognifloyd cognifloyd merged commit c978e72 into StackStorm:master Oct 2, 2021
@arm4b
Copy link
Member

arm4b commented Oct 2, 2021

This PR comes with the StackStorm/st2-packages#706 and should be merged only once the change in packaging is ready.
In this situation, we merged an unfinished feature which is not ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature size/XS PR that changes 0-9 lines. Quick fix/merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ports for api and authentication services can't be changed
4 participants