asterisk: improve init.d script and fix #681

[dhewg]

A few small and (hopefully) self-explanatory patches.

@jospezial: Does that work for you?

[jospezial]

Seems to work so far (asterisk-19.0.0). Can we do anything with that procd message of "/etc/init.d/asterisk stop" ?

Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]: [Nov  5 00:00:31] DEBUG[2515]: cdr.c:4602 ast_cdr_engine_term: CDR Engine termination request received; waiting on messages...
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]: Asterisk cleanly ending (0).
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]: Executing last minute cleanups
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]:   == Manager unregistered action DBGet
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]:   == Manager unregistered action DBPut
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]:   == Manager unregistered action DBDel
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]:   == Manager unregistered action DBDelTree
Fri Nov  5 01:00:31 2021 daemon.info asterisk[2515]: [Nov  5 00:00:31] DEBUG[2515]: asterisk.c:2051 really_quit: Asterisk ending (0).
Fri Nov  5 01:00:36 2021 daemon.info procd: Instance asterisk::instance1 pid 2515 not stopped on SIGTERM, sending SIGKILL instead

[jospezial]

Sometimes after a service asterisk restart
I see:
Command failed: Not found
Also there:

root@OpenWrt6431:~# /etc/init.d/asterisk stop
Command failed: Not found

But in logread I see asterisk starting/stopping normal.

Maybe we could also use the green:telefon LED with Asterisk if all is "ok", flash on "error" for example?
chan_lantiq.c has something with led control.
kochstefan wrote me "Look at lines 2124-2133, it seems that LED named using a fixed string + channel number."
But I don't know how or where to activate it.

[dhewg]

Nice, thanks for testing it.
IIRC I had similar signal/commands errors with asterisk but without astcanary. Try installing the latter.

Dunno about the LED idea, that's likely a can of worms ;) For a reliable solution you'd have to integrate it in your dialplan I guess (switch the LED status on a trunk registration event)

[micmac1]

I don't think this flies, definitely not as default as the next commit makes it out to be. This is expecting that pjsip is used for outbound connections. But users may use all sorts of channels, iax2, dahdi, chan-sip and whatnot.

If made optional and into some sort of template that'd be fine.

[micmac1]

Sometimes after a service asterisk restart I see: Command failed: Not found Also there:

root@OpenWrt6431:~# /etc/init.d/asterisk stop
Command failed: Not found

But in logread I see asterisk starting/stopping normal.

Where is this coming from, then? Is this in any way related to this pull request? Do you see this when issuing a stop or restart while procd has an asterisk instance up and running?

[jospezial]

Sometimes after a service asterisk restart I see: Command failed: Not found Also there:

root@OpenWrt6431:~# /etc/init.d/asterisk stop
Command failed: Not found

But in logread I see asterisk starting/stopping normal.

Where is this coming from, then? Is this in any way related to this pull request? Do you see this when issuing a stop or restart while procd has an asterisk instance up and running?

The message "Command failed: Not found" seems to come directly from asterisk when issuing a stop or restart while asterisk is not running.

I don't think this flies, definitely not as default as the next commit makes it out to be. This is expecting that pjsip is used for outbound connections. But users may use all sorts of channels, iax2, dahdi, chan-sip and whatnot.

Let's do the restart simpler without using the asterisk internal pjsip command. Let's always do a fresh full start of asterisk.
No matter whether "start" or "restart" is used or triggered by interface. Maybe start_service() should first stop then start asterisk, but I'm not sure.

[jospezial]

After reboot we could let the asterisk start wait for the interface up if an interface is set in /etc/config/asterisk.
Then pjsip does not need to use its retry_interval to connect before the interface up.
Asterisk could be stopped if the interface goes down. (I have to check if the wan interface goes down on daily DSL break)

[dhewg]

I don't think this flies, definitely not as default as the next commit makes it out to be. This is expecting that pjsip is used for outbound connections. But users may use all sorts of channels, iax2, dahdi, chan-sip and whatnot.

If made optional and into some sort of template that'd be fine.

I'd leave it at pjsip only for now, that'll cover most if not all asterisk users on OpenWrt I guess.

It's already optional in the sense that it'll only do anything if you use the interface config entry. We could just not set that to wan by default? Or alternatively check for the pjsip module before running the two commands?

[dhewg]

Let's do the restart simpler without using the asterisk internal pjsip command. Let's always do a fresh full start of asterisk.

I don't think that's the way to go. Connecting to asterisk without it itself being able to connect to upstreams is still valid, and it'll lose its runtime stats that way too, like used in the prometheus module which I'm using

[dhewg]

How about this? The wan default is kept, but the trigger now checks is res_pjsip.so is running, and only then running the two commands. That can easily be extended for whatever channels.

[micmac1]

I got an idea. You know OpenWrt has this file /etc/firewall.user. You add a file like that, maybe call it /etc/asterisk-if-trigger.user. Make it a "#!/bin/sh" script. Not sure if procd_add_interface_trigger cares about exit code, so maybe add "exit 0" at the end of the script. You add your pjsip commands in there, but commented out, as an example. Add the file to conffiles, too, so user changes won't get overwritten.

Then you setup the trigger in the init script, pointing the trigger to this .user script. And in /etc/config/asterisk you add a comment to your new option that points users to the file. I think the option should have a more pointy name than "interface". Maybe "interface-trigger" or "if-trigger".

This way there's no change of behavior, it's not hacky and users can add any command they like to run to the file that already contains an example for pjsip.

[dhewg]

But we can already do that now with a script in /etc/hotplug.d/iface without any changes whatsoever :)

I agree that interface-trigger is more appropriate and self explanatory for this use case. But I also think that providing an easy solution for this problem is more user friendly without having users fiddle around with scripts.

Do you believe this is hacky? I mean the fact that it's now parsing the output of module show isn't as nice and tidy as I'd like, but it's not so bad either and with this all the user has to do is set a uci config knob. Maybe even someday with a luci app?

[micmac1]

But we can already do that now with a script in `/etc/hotplug.d/iface` without any changes whatsoever :)

I don't think we can, for the same reason mentioned yesterday: We don't know what kind of outbound channels the user has in use.

I agree that `interface-trigger` is more appropriate and self explanatory for this use case. But I also think that providing an easy solution for this problem is more user friendly without having users fiddle around with scripts. Do you believe this is hacky? I mean the fact that it's now parsing the output of `module show` isn't as nice and tidy as I'd like, but it's not so bad either and with this all the user has to do is set a uci config knob. Maybe even someday with a luci app?

Yes, I do believe it's hacky and I wouldn't merge this as is.

[dhewg]

Alright, I would still prefer this over custom scripts though. So let's not then :\

[jospezial]

That is sad to read.
Are the first 2 commits at least good to go in? These were not part of the discussion.
8391b99
2438ff5

I hope we find a solving way that makes everybody happy some day.
Thank you for your work.

[dhewg]

Yeah, those 2 are independent of the fix for #681 and are still worth having imho. The branch is still there if anyone cares

[jospezial]

Yeah, those 2 are independent of the fix for #681 and are still worth having imho. The branch is still there if anyone cares

@micmac1 or @jslachta would you like to merge these 2 commits of this PR?
8391b99
2438ff5

[micmac1]

Yeah, those 2 are independent of the fix for #681 and are still worth having imho. The branch is still there if anyone cares

@micmac1 or @jslachta would you like to merge these 2 commits of this PR? 8391b99 2438ff5

I cherry-picked the sighup commit. With the other one I have some doubts. Like, the level is not checked in the function and you'd need to remember to put the log level before the log message. This seems to be a bit of an overkill, because we're only using the logger just a single time right now. I added a commit to change the facility to deamon anyway, to smooth things over. :)