[PATCH] Fix default value for enable_environment_macros

[Andreas Ericsson]

On 01/11/2013 02:43 PM, Ton Voon wrote:
> 
> 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.
> 

Those scripts are broken, since environment variables are an option
in Nagios and can't always be enabled.

>> 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.
> 

Well, for small systems it's certainly an option, but please realize
that the cost for firing up a single check rises enormously when we
enable macros as environment variables. It really is a very bad idea
to use them.

>> 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

Sounds sensible, but I'll also make it print a warning in the logs
and to console when we encounter the option to enable it, saying
that it's deprecated and that things relying on it are currently
considered broken by design.

> 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 was thinking more along the lines of

define command {
   env          ORACLE_HOME=/foo/bar/lala
   env          HOST_NAME=$HOSTNAME$
   command_name check_something
   command_line /path/to/some-check
}

since that also lets users set variables that aren't exactly Nagios related.

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

If you get the macros to the workers as kvvec key/value pairs like this:

  k=env
  v=NAGIOS_(MACRO_NAME)=macro value

then I'll make sure the workers set up the environment properly. I'll
make an addendum to the worker rules though so the (possibly huge)
macro values don't have to be passed back to Nagios for every check
executed.

I recommend using a pre-filled bitmap to denote which macros should
be available for which type of commands so a generic function can be
used to iterate over them and skip the ones we can't possibly have
access to for the current command.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.

------------------------------------------------------------------------------
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