[Reviewer: EM] Don't run Ralf if there is no Ralf hostname.

[richardwhiuk]

Ellie,

As discussed the other day, we'd like to be able to configured whether Ralf is enabled based on whether there is a configured host name. If it isn't enabled, we shouldn't start it or allow it to be started, and we shouldn't monitor it, or any of it's children.

To this end, I've made two changes:

• Add/remove the monit script based on whether Ralf is present
• Prevent Ralf from starting if there is no configured Ralf hostname

[eleanor-merry]

I've commented on your PR.

As we discussed before though, I don't think this is the right way to do this.

You're using ralf_hostname for too many things. It currently controls where Sprout/Bono send ACR events and whether Sprout/Bono should send Ralf ACR events at all, and now you're making it control whether Ralf should be running. This means there's no clean way to turn on Ralf support - it's entirely plausible that your Sprout nodes start sending ACR events to Dime nodes before they've restarted their Ralfs.

I would rather see one of the following:

1. Separate builds for each of the Dime node types. This means that there's no way to enable Ralf if you don't have the right Dime node - if customers want to use Ralf they must wait until they upgrade.

2. Splitting up what ralf_hostname means.

• Add a new ralf_enabled option - this controls whether Sprout/Bono send to Ralf independently of ralf_hostname.
• Change Ralf so that it doesn't exit if ralf_hostname isn't set (but add a poll_rf script like the poll_cx script to detect diameter configuration errors). (Note - I'd expect the build process to set ralf_hostname to the Dime nodes anyway - it just wouldn't process any traffic)
• Add a documented process for enabling/disabling Ralf (shared_config upgrade to set ralf_hostname if not already set, then shared_config upgrade to set ralf_enabled).

Did you check with @steelyvoid what support's preferences for turning on/off Ralf support would be, and what use case they think customers will have?

[eleanor-merry]

This should probably be in main.cpp with the verification of the other parameters.

[eleanor-merry]

else branch has incorrect indentation.

[eleanor-merry]

Thinking about this some more, I'd be happy with the monit changes if you also added the ralf_enabled option (so no need to change ralf's behaviour when it runs with no ralf_hostname set).