[PATCH] Fix default value for enable_environment_macros

[Ton Voon]

On 11 Jan 2013, at 12:17, Andreas Ericsson wrote:

> On 01/10/2013 11:31 PM, Ton Voon wrote:
>> However, even with this set to true, it looks like Nagios 4 doesn't
>> set the envvars. In host/service checks, it looks like no envvars are
>> set. Is there a technical reason why this is not done, or is this a
>> bug?
>> 
> 
> The technical reason is twofold: I'm lazy, and environment macros cause
> a lot of additional problems for anything but very small systems, while
> providing a mediocre benefit at best.
> 
> The problem people have been seeing with Nagios 3 where certain checks
> fail to run when they add an extra service to a servicegroup is due to
> environment macros being enabled and the environment space being eaten.
> 
> To enable them again means transferring the entire environment to the
> designated worker process, parsing it from there and setting it in a
> new heap which we pass to execvpe().
> 
> Since we will always pay *all* overhead for all checks but very, very
> few of them use more than one or two of the variables set, the benefit
> really isn't worth the additional overhead.

I think this "bug" is a blocker for upgrading to Nagios 4 since certain checks/notification scripts/event handlers expect the environment variables to be set.

> A different (and far niftier) idea is to use "env" statements for
> commands, where one can determine certain variables to pass through
> the environment. That way we'd pay very little overhead and only for
> the variables people actually want exported via the environment. It
> would also make it possible to set certain environment variables for
> plugins that require them (like many of the oracle checks do).

I completely agree about the performance overhead.

The reason I dislike parsing at the command line is that "funny characters" can interfere with the processing or affect the command executed. Passing through the environment is a lot more consistent.

> So.. yes, it's a bug, but consider environment variable support of the
> current notion (all or nothing) to be deprecated and scheduled for
> removal in Nagios 5 with a more powerful way to control environment
> variables replacing it.

I've had a look at how the environment is handled in the workers and it does look like there is some support for it, though a FIXME says it hasn't been enabled. However, I've managed to get it working so that I can pass some envvars from the main process to the worker, and it gets set at time of execution. I need some help to make it work for all of the macros though and I'm not sure if there are some limits breached in the kvvec_addkv() calls.

So I propose that:
  1) enable_environment_macros=1 passes all the environment data in the wproc_run_job() to retain backwards compatibility
  2) an env_macros field is available in all objects to define specifically which environment macros. This is a comma separated list, defaulting to ALL for backwards compatibility

I'm happy to do some of this work, but will require agreement on the design and some technical assistance.

Ton


------------------------------------------------------------------------------
Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
much more. Get web development skills now with LearnDevNow -
350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122812