Correctly handle config-file-only BooleanOptions which default to True

In Bcfg2.Options.Parser._parse_config_options, if the "default" of the
option (perhaps set by the config file) evaluates to True, then it runs
the actions associated with the option. Otherwise, it just sets the
option to the default directly with setattr.

Since the default of a store_false action is True, the code will run
argparse's _StoreFalseAction function. Well, _StoreFalseAction, is a
subclass of _StoreConstAction where const=False. _StoreConstAction has a
call method which looks like this:

def __call__(self, parser, namespace, values, option_string=None):
    setattr(namespace, self.dest, self.const)

So it completely ignores the value passed in and just sets the value of
the option to const (i.e. False).

Looking at fam_blocking
{None: _StoreFalseAction(option_strings='fam_blocking',
dest='fam_blocking', nargs=0, const=False, default=True, type=None,
choices=None, help='FAM blocks on startup until all events are
processed', metavar=None)}
start phase 3
fam_blocking config val is True
start phase 3
_parse_config_options: fam_blocking default is True
_parse_config_options: calling argparse.<class
'argparse._StoreFalseAction'> with value True on fam_blocking
_parse_config_options: after calling argparse.<class
'argparse._StoreFalseAction'> fam_blocking is False
after _parse_config_options fam_blocking is False
after phase 3 fam_blocking is False
after phase 4 fam_blocking is False
end of parser fam_blocking is False
CLI init fam_blocking is False

So how to fix it? Define a new Action? That seems like the most direct
approach since the problem really is that _StoreFalseAction does what it
says on the tin, it stores false no matter what.

The new action BooleanOptionAction works like store_true and store_false,
except that it stores the value that was passed in, or the default if there
was no value passed in.

1 files changed, 25 insertions, 4 deletions

diff --git a/src/lib/Bcfg2/Options/Options.py b/src/lib/Bcfg2/Options/Options.py
index 11388fb4d..fcc76dca5 100644
--- a/src/lib/Bcfg2/Options/Options.py
+++ b/src/lib/Bcfg2/Options/Options.py
@@ -306,6 +306,26 @@ class PathOption(Option):
         Option.__init__(self, *args, **kwargs)
 
 
+class _BooleanOptionAction(argparse.Action):
+    """ BooleanOptionAction sets a boolean value in the following ways:
+    - if None is passed, store the default
+    - if the option_string is not None, then the option was passed on the
+      command line, thus store the opposite of the default (this is the
+      argparse store_true and store_false behavior)
+    - if a boolean value is passed, use that
+
+    Defined here instead of :mod:`Bcfg2.Options.Actions` because otherwise
+    there is a circular import Options -> Actions -> Parser -> Options """
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        if values is None:
+            setattr(namespace, self.dest, self.default)
+        elif option_string is not None:
+            setattr(namespace, self.dest, not self.default)
+        else:
+            setattr(namespace, self.dest, bool(values))
+
+
 class BooleanOption(Option):
     """ Shortcut for boolean options.  The default is False, but this
     can easily be overridden:
@@ -317,11 +337,12 @@ class BooleanOption(Option):
                 "--dwim", default=True, help="Do What I Mean")]
     """
     def __init__(self, *args, **kwargs):
-        if 'default' in kwargs and kwargs['default']:
-            kwargs.setdefault('action', 'store_false')
-        else:
-            kwargs.setdefault('action', 'store_true')
+        kwargs.setdefault('action', _BooleanOptionAction)
+        kwargs.setdefault('nargs', 0)
+
+        if 'default' not in kwargs:
             kwargs.setdefault('default', False)
+
         Option.__init__(self, *args, **kwargs)