watchcat: Add support for custom ping timeout

[kumy]

Maintainer: me / @neheb, @jtkohl
Compile tested: ATH79 - generic - tag v23.05.5
Run tested: ath79/generic OpenWrt 23.05.4 r24012-d8dd03c46f / LuCI openwrt-23.05 branch git-24.086.45142-09d5a38

Closes #25173

Description:
Allow custom ping timeout

Notes:

• Need to work on the luci-app-watchcat side -> luci-app-watchcat: Add support for ping timeout luci#7337
• Manual test: add the -W to the ping command
• Code tested

$ opkg install /tmp/watchcat_1-18_all.ipk 
Upgrading watchcat on root from 1-17 to 1-18...
Configuring watchcat.
Collected errors:
 * resolve_conffiles: Existing conffile /etc/config/watchcat is different from the conffile in the new package. The new conffile will be placed at /etc/config/watchcat-opkg.

$ cat /etc/config/watchcat

config watchcat
	option period '32'
	option mode 'restart_iface'
	option pinghosts '1.1.1.1'
	option addressfamily 'ipv4'
	option pingperiod '30s'
	option pingsize 'small'
	option interface 'wwan0'
	option mmifacename 'free'

$ cat /etc/config/watchcat-opkg 
config watchcat
	option period '6h'
	option mode 'ping_reboot'
	option pinghosts '8.8.8.8'
	option forcedelay '30'
	option pingtimeout '10'

# this ^ is probably not an error introduced by my change...

# test cases:
$ sh -x /usr/bin/watchcat.sh restart_iface 30 1.1.5.5 3 small wwan0 free 0 ipv4
# properly defaults to 10s
$ sh -x /usr/bin/watchcat.sh restart_iface 30 1.1.5.5 3 small wwan0 free 0 ipv4 1
# properly set to 1s
$ sh -x /usr/bin/watchcat.sh restart_iface 30 1.1.5.5 3 small wwan0 free 0 ipv4 A
# properly defaults to 10s

$ sh -x /usr/bin/watchcat.sh ping_reboot 30 5m 1.1.5.5 3 small ipv4
# properly defaults to 10s
$ sh -x /usr/bin/watchcat.sh ping_reboot 30 5m 1.1.5.5 3 small ipv4 1
# properly set to 1s
$ sh -x /usr/bin/watchcat.sh ping_reboot 30 5m 1.1.5.5 3 small ipv4 A
# properly defaults to 10s

$ sh -x /usr/bin/watchcat.sh run_script 30 1.1.5.5 3 small wwan0 ipv4 /usr/bin/true 
# properly defaults to 10s
$ sh -x /usr/bin/watchcat.sh run_script 30 1.1.5.5 3 small wwan0 ipv4 /usr/bin/true 1
# properly set to 1s
$ sh -x /usr/bin/watchcat.sh run_script 30 1.1.5.5 3 small wwan0 ipv4 /usr/bin/true A
# properly defaults to 10s

$ uci set watchcat.@watchcat[-1] pingtimeout "1"

$ uci show watchcat
watchcat.@watchcat[0]=watchcat
watchcat.@watchcat[0].period='32'
watchcat.@watchcat[0].mode='restart_iface'
watchcat.@watchcat[0].pinghosts='1.1.1.1'
watchcat.@watchcat[0].addressfamily='ipv4'
watchcat.@watchcat[0].pingperiod='30s'
watchcat.@watchcat[0].pingsize='small'
watchcat.@watchcat[0].interface='wwan0'
watchcat.@watchcat[0].mmifacename='free'
watchcat.@watchcat[0].pingtimeout='1'

$ uci commit

$ ps w|grep watchcat
24975 root      1408 S    {watchcat.sh} /bin/sh /usr/bin/watchcat.sh restart_iface 32 1.1.1.1 30 small wwan0 free 0 ipv4 10
# default value 10

$ service watchcat restart

$ ps w|grep watchcat
25561 root      1408 S    {watchcat.sh} /bin/sh /usr/bin/watchcat.sh restart_iface 32 1.1.1.1 30 small wwan0 free 0 ipv4 1
# my new config is applied -> 1

[kumy]

I think this PR and openwrt/luci#7337 are ready for review.

Thanks 🙏

[systemcrash]

You can avoid this altogether by supplying a default if timeout is empty or unsupplied.

${parameter:-[word]}. Param is empty? Fill with default [word]

[systemcrash]

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

[kumy]

This is what I've done first but then changed my mind. The code here also defaults to the default in case someone pass it a non-integer or negative value. If we don't care about this case then yes, shell default value syntax will be simpler 🙂. WDYT?

[kumy]

@systemcrash I pushed a new commit to change that part. But honestly I'm not convinced.

That will silently fail with such case

# use of "A" / "-1" / ""1.5" as last param is not a positive integer and will silently fail the test
sh -x /usr/bin/watchcat.sh restart_iface 30 1.1.5.5 3 small wwan0 free 0 ipv4 A

# would translate to 
ping -4 -I wwan0 -s 1 -c 1 -W A 1.1.5.5

# previous implementation would have set the default value
ping -4 -I wwan0 -s 1 -c 10 -W 10 1.1.5.5

[kumy]

thanks @Neustradamus

@systemcrash @Neustradamus Regarding comments above. We still need to decide on ab35bd7 (xref #25174 (comment)). I would prefer to remove it and keep previous implementation. But final word is yours :)

Then we have openwrt/luci#7337 to check. (It's now having merge conflicts but I will resolve them lately)

Thanks

[Neustradamus]

@kumy: Nice PR, thanks!