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

Use ynh_systemd_action #711

Merged
merged 2 commits into from Apr 25, 2019

Conversation

Projects
None yet
4 participants
@maniackcrudelis
Copy link
Contributor

commented Apr 13, 2019

The problem

Use ynh_systemd_action helper in other helpers

PR Status

...

How to test

...

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :

@YunoHost/apps

@@ -442,7 +442,7 @@ EOF
ynh_store_file_checksum "$finalfail2banjailconf"
ynh_store_file_checksum "$finalfail2banfilterconf"

systemctl try-reload-or-restart fail2ban
ynh_systemd_action --service_name=fail2ban --action=reload

This comment has been minimized.

Copy link
@Josue-T

Josue-T Apr 13, 2019

Contributor

Well,

I think reload is not same than try-reload-or-restart.

And for this helper maybe we need to manage the error because actually some app install/upgrade fail juste because the fail2ban fail to reload (see YunoHost-Apps/synapse_ynh#88 (comment))

@@ -289,6 +290,6 @@ ynh_psql_test_if_first_run() {
yunohost service add postgresql --log "$logfile"

systemctl enable postgresql
systemctl reload postgresql
ynh_systemd_action --service_name=postgresql --action=reload

This comment has been minimized.

Copy link
@alexAubin

alexAubin Apr 16, 2019

Member

Kinda unrelated but just to say that this stuff about the manipulating postgresql here is a bit weird / misleading : the "real" postgresql service seems to be postgresql@9.6-main (or something like this) compared to just postgresql service which is a different thing ... Dunno if that's an issue or not but would be nice if people are aware of this :P

@alexAubin alexAubin added this to the 3.6.x milestone Apr 16, 2019

@kay0u

kay0u approved these changes Apr 17, 2019

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

Following this issue YunoHost-Apps/etherpad_mypads_ynh#65, I suggest to restart instead of reload fail2ban.
Reload may not be enough if there's a lot of banned IP.

@Josue-T

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2019

Following this issue YunoHost-Apps/etherpad_mypads_ynh#65, I suggest to restart instead of reload fail2ban.
Reload may not be enough if there's a lot of banned IP.

Yes, but restart is sometime really really long one the slow architecture...

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2019

Yes, but restart is sometime really really long one the slow architecture...

Ok then...
Anyway, that's an edge case and this behavior is fixed since version 0.10 with a reload that doesn't re-ban IP.

@alexAubin

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

Soooo are we good with this ?

@maniackcrudelis

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

I'm good with this PR.

@alexAubin

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

Will merge in the coming days except if there are change requests 😉

@alexAubin alexAubin merged commit e4544ea into stretch-unstable Apr 25, 2019

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@alexAubin alexAubin deleted the use_ynh_systemd_action branch Apr 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.