DietPi-Service | Stopping a random services will stop NextCloud as well

[Joulinar]

Hi @MichaIng,

I'm opening the report based on 2 forum post.

https://dietpi.com/phpbb/viewtopic.php?f=9&t=7320
https://dietpi.com/phpbb/viewtopic.php?f=11&t=8020

Issue is, that dietpi-services stop <service> will stop NextCloud as well, even if the service is not related to NextCloud

root@DietPi3:~# dietpi-services stop qbittorrent

 DietPi-Services
─────────────────────────────────────────────────────
 Mode: stop qbittorrent

[  OK  ] DietPi-Services | ncc maintenance:mode --on
[  OK  ] DietPi-Services | stop : qbittorrent
root@DietPi3:~# dietpi-services start qbittorrent

The other way around of service going to be started.

root@DietPi3:~# dietpi-services start qbittorrent

 DietPi-Services
─────────────────────────────────────────────────────
 Mode: start qbittorrent

[  OK  ] DietPi-Services | start : qbittorrent
[  OK  ] DietPi-Services | ncc maintenance:mode --off

This did not happen on dietpi-services restart <service>

root@DietPi3:~# dietpi-services restart qbittorrent

[ DietPi-Services
─────────────────────────────────────────────────────
 Mode: restart qbittorrent

[  OK  ] DietPi-Services | restart : qbittorrent
root@DietPi3:~#

I guess this is related to this line. But If I'm not mistaken it should happen for start/restart/stop
https://github.com/MichaIng/DietPi/blob/f1cfa3c843ebfa28b8edd8ba46dccd35742de52c/dietpi/dietpi-services#L392

[MichaIng]

Indeed, was reported somewhere else already. I have to think through it since in case of stopped database or redis or php-fpm it makes sense to put Nextcloud into maintenance mode. So how to handle this:

• At least two services are stopped that requires Nextcloud to be put in maintenance (to prevent access error via cron or webserver access).
• Then a single of those services is started, so Nextcloud can still not be successfully accessed without producing a bunch of ugly error messages.
• We cannot simply check the single service name to know whether to disable maintenance mode since stopping a single service requires maintenance mode enabled but starting a single service does not necessarily allow it to be disabled. We'd need to check for all required services, which includes checking for any PHP version or checking which one is used by the webserver, which means checking which webserver is in use etc etc.

Probably like that is easy and good enough in most cases:

• Stopping any of MariaDB, Redis or PHP enables maintenance mode.
• Only starting all services disables it.
• We could print a message when a single service is started and maintenance mode is enabled with a hint that dietpi-services start is required to disable maintenance mode.

[Joulinar]

Indeed we should keep it simple and just set maintenance mode if MariaDB, Redis or PHP are stopped. That's basically what NextCloud is depending on. (next to a functioning web server).

[MichaIng]

Jep. If the webserver is stopped, then only cron via php-cli or php-cli directly can "reach" Nextcloud anyway, and for this no webserver is required.

[MichaIng]

Strange, actually starting/stopping a single service should not toggle maintenance mode:

[[ $command == 'stop' || $command == 'restart' && ! $index ]]

According to what I through I'd know about bash syntax, $index must be empty for this to succeed, regardless if "$command" is "stop" or "restart". This should be the case for the same reason why [[ A && B || C ]] does not equal if A then B else C, since C is as well checked when A is true but B is false. Basically bash should proceed with any later && as long as its previous check was positiv and should proceed with any next || if its last check was negative.
However in this case, ! $index is only checked if "command" is "restart" but not if it's "stop":

2020-08-26 16:01:03 root@micha:/var/log# index=1
2020-08-26 16:08:55 root@micha:/var/log# command=stop
2020-08-26 16:09:00 root@micha:/var/log# [[ $command == 'stop' || $command == 'restart' && ! $index ]] && echo true
true
2020-08-26 16:09:05 root@micha:/var/log# command=restart
2020-08-26 16:09:10 root@micha:/var/log# [[ $command == 'stop' || $command == 'restart' && ! $index ]] && echo true
2020-08-26 16:09:11 root@micha:/var/log#

Clearer test:

2020-08-26 16:13:30 root@micha:/var/log# [[ 1 == 2 || 2 == 2 && 3 == 2 ]] && echo yes
2020-08-26 16:13:35 root@micha:/var/log# [[ 2 == 2 || 1 == 2 && 3 == 2 ]] && echo yes
yes

The last test is only checked if the second test is true and ignored if the first test is true.

2020-08-26 16:15:17 root@micha:/var/log# [[ 1 == 2 && 2 == 2 || 3 == 2 ]] && echo yes
2020-08-26 16:17:02 root@micha:/var/log# [[ 2 == 2 && 1 == 2 || 3 == 2 ]] && echo yes
2020-08-26 16:17:09 root@micha:/var/log# [[ 2 == 2 && 1 == 2 || 3 == 3 ]] && echo yes
yes
2020-08-26 16:17:35 root@micha:/var/log# [[ 1 == 2 && 2 == 2 || 3 == 3 ]] && echo yes
yes
2020-08-26 16:20:35 root@micha:/var/log# [[ 1 == 1 && 2 == 2 || 2 == 3 ]] && echo yes
yes

Here, regardless if the first test is false or the second test is false, the 3rd test is performed in both cases.

This is inconsistent. Either the checks must end completely at && if the last was false and at || when the last was true, or it must search for any following && and || statement in both cases 🤔.

Separating the bash conditionals behaves as expected:

2020-08-26 16:23:23 root@micha:/var/log# [[ 2 == 2 ]] || [[ 1 == 2 ]] && [[ 3 == 2 ]] && echo yes
2020-08-26 16:23:39 root@micha:/var/log# [[ 2 == 2 ]] || [[ 1 == 2 ]] && [[ 3 == 3 ]] && echo yes
yes

Made some more checks:

• Outside of double square brackets, when any command/test returns true, the shell looks for any next && ... to continue, even when located after ||.
• Accordingly when any command/test returns false, it looks for any next || ... to continue, even when located after &&.
• Inside double square brackets, when a test returns false, it looks for any next || ... to continue, so same as above.
• But when a command test returns true, it only continues with a directly following && ... but does not continue to perform any && ... after ||, so differently compared to the first case outside of double square backets.

Good to know. Solution is quite easy by separating and by this grouping the checks, but I completely dislike this inconsistency 😄.

[MichaIng]

Okay fixed it, i.e. it behaves now like it was intended: https://github.com/MichaIng/DietPi/commit/2fd9a8744622a8c5a4589cbfe78a4e2df88895b1
This means that stopping any single service will not enable NC maintenance mode. I was a bid lazy to implement checking further for MariaDB/Redis/PHP but I think this is fine since we can expect people knowing what they're doing when stopping such a service. Also it is more consistent compared to enabling maintenance when stopping the database but not disabling maintenance when restarting database.

[Joulinar]

ok it's working on Beta that way.

[MichaIng]

Thanks for testing. I hope there are no other cases where things do no behave as expected due to this wrong understood bash behaviour 😅.