Replacing deferrable reboot with watchcat

[AguTrachta]

This implementation covers the suggestion made here. The code configures watchcat every time lime-config is run.

For the moment deferrable-reboot has not been removed.
Please review and let me know if you think it is safe to remove deferrable-reboot entirely, or if further adjustments are needed.

[ilario]

Hi!
Thanks for this work!! :D

I did not test yet. Just trying to comment the code by eye, I could be wrong:

For the Makefile, there is a "new" and more compact format using the libremesh.mk file, you can see many examples in the files modified by this pull request: #1139
As this package will be just one Lua file, you can take inspiration by a simple Makefile like this one:
https://github.com/libremesh/lime-packages/blob/d1642f198ac94947d818ed5c97fbce1cd01ca996/packages/altermundi-grafana/Makefile

It would be helpful to add an example of configuration in lime-example.txt https://github.com/libremesh/lime-packages/blob/master/packages/lime-docs/files/www/docs/lime-example.txt

It looks like all the settings are currently hardcoded, is it so?
What I would allow the user to customize is:
The ping target IP (e.g. someone could ping an IP internal in the local net, which would make even more sense than pinging an IP on the internet)
The period, as maybe someone could want something longer or shorter than 6 hours.
The pingperiod of 30s is likely good for everybody... But I am not super sure of that. We can leave that hardcoded until when someone complains or we can add it as a customizable parameter.

I would not use Google's 8.8.8.8 as the default ping target. Rather, we can use the same that is in place for deferrable-reboot (4.2.2.2 b.resolvers.Level3.net) https://github.com/libremesh/lime-packages/blob/master/packages/lime-docs/files/www/docs/lime-example.txt#L25

[AguTrachta]

Nice, so I have a doubt here, do we need to add a fallback configuration if it doesnt find a config hwd_watchcat in lime-community? or we leave the default open wrt config?

If we do the last option it will stay with this configuration

[ilario]

Nice, so I have a doubt here, do we need to add a fallback configuration if it doesnt find a config hwd_watchcat in lime-community? or we leave the default open wrt config?

If we do the last option it will stay with this configuration

In case an option is not specified in /etc/config/lime-*, it seems more than reasonable to use the fallback from the upstream package actually... Great proposal, thanks!

So, if we want to propose a default alternative to 8.8.8.8 we should include it in /etc/config/lime-defaults

Also, what about including an "enabled" true/false option? I mean: if, in the future, we decide to include watchcat by default in the firmware images, it should not run before the user chooses the target IP and explicitly decides to enable it.
That option would cause the execution (when lime-config is run) of "service watchcat enable; service watchcat start" from within the same Lua file when it is true and a "service watchcat stop; service watchcat disable" when it is false. But this can be implemented in the future: until when watchcat is not included by default in the firmware images, I believe it can be spared.

[AguTrachta]

• In these new changes I replaced the Makefile with a minimal one based on libremesh.mk
• /etc/config/watchcat is left intact unless the admin defines config hwd_watchcat
• And added a new example block under “Watchcat specific sections” showing exactly how to define a config hwd_watchcat

I'm still analyzing the enable option, but I wanted to make this push for you to help me testing if its possible :)

[ilario]

The code looks great! :D

I took the liberty to edit myself the Makefile, I hope you don't mind. If you prefer the things to be different, feel free to revert my edits.

Adding a configuration piece in /etc/config/lime-node and running lime-config resulted in two rules being present in /etc/config/watchcat:

config watchcat
	option period '6h'
	option mode 'ping_reboot'
	option pinghosts '8.8.8.8'
	option forcedelay '30'

config watchcat 'lm_hwd_watchcat_default'
	option pingperiod '5m'
	option period '2h'
	option pinghosts '4.2.2.2'
	option mode 'ping_reboot'
	option forcedelay '1m'

But what I would expect is that the original rule disappears, do you agree that it would be reasonable? What do you think about adding this "delete also the original rule" in the watchcat.clean function.

Also, it would be convenient that service watchcat reload is executed if changes are present. This would apply the new watchcat rule without the need to reboot the device or to run service watchcat reload manually. But I am not sure this is the elegant thing to do...

Seems that the clean function is executed every time lime-config is run:

lime-packages/packages/lime-system/files/usr/lib/lua/lime/config.lua

Line 217 in e41d259

xpcall(module.clean, function(errmsg) print(errmsg) ; print(debug.traceback()) end)

So there would be no need for an enable/disable option, as deleting the configuration section would be the simple way for disabling watchcat.

[AguTrachta]

Thanks for the Makefile changes!

Yes I agree with the original rule, I'll work on it.

About the service watchcat reload, maybe using something like os.execute("/etc/init.d/watchcat reload >/dev/null") ?

[AguTrachta]

In these new commits I modified the clear function, now it really clears the original rule, I tested it but it will be great if you check it too :)

Also I added a simple function to use the reload command, I noticed that it doesn't throw any log so I didn't use the >/dev/null that was proposed

[ilario]

Tested. It works perfectly, congratulations!