[sosreport] support all command line options in /etc/sos.conf

- SoSOptions class changes:
  - from_file method to load load options from file
  - encapsulation of particular options into __init__ method
  - updated merge logic to replace non-default values only

- sosreport.py changes:
  - _parse_args (renamed to _get_parser) newly returns just the
    ArgumentParser object as parsing itself is done elsewhere
  - SoSOptions are built in ordering:
    - defaults from parser (with emtpy cmdline) loaded
    - cmdline options replace defaults
    - config file known even now
    - config.file options replace defaults until already set by
      cmdline (i.e. until SoSOptions marked them as nondefault)
    - presets can be known even now
    - options from given preset replace defaults, until the options
      are already updated from cmdline or config.file
  - some extra logging/formatting added
  - some extra comments added

Fixes: #855
Resolves: #1530

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
Signed-off-by: Bryn M. Reeves <bmr@redhat.com>

2 files changed, 154 insertions, 112 deletions

diff --git a/sos/__init__.py b/sos/__init__.py
index cc795ab2..491d801f 100644
--- a/sos/__init__.py
+++ b/sos/__init__.py
@@ -18,6 +18,13 @@ gettext to internationalize messages.
 import gettext
 import six
 
+from argparse import ArgumentParser
+
+if six.PY3:
+    from configparser import ConfigParser, ParsingError, Error
+else:
+    from ConfigParser import ConfigParser, ParsingError, Error
+
 __version__ = "3.6"
 
 gettext_dir = "/usr/share/locale"
@@ -71,55 +78,34 @@ def _is_seq(val):
 
 
 class SoSOptions(object):
-    add_preset = ""
-    alloptions = False
-    all_logs = False
-    batch = False
-    build = False
-    case_id = ""
-    chroot = _arg_defaults["chroot"]
-    compression_type = _arg_defaults["compression_type"]
-    config_file = ""
-    debug = False
-    del_preset = ""
-    desc = ""
-    enableplugins = []
-    encrypt_key = None
-    encrypt_pass = None
-    experimental = False
-    label = ""
-    list_plugins = False
-    list_presets = False
-    list_profiles = False
-    log_size = _arg_defaults["log_size"]
-    noplugins = []
-    noreport = False
-    note = ""
-    onlyplugins = []
-    plugin_timeout = None
-    plugopts = []
-    preset = ""
-    profiles = []
-    quiet = False
-    sysroot = None
-    threads = 4
-    tmp_dir = ""
-    verbosity = 0
-    verify = False
-
-    def _merge_opt(self, opt, src, replace):
+
+    def _merge_opt(self, opt, src, is_default):
+        def _unset(val):
+            return (val == "" or val is None)
+
         if hasattr(src, opt):
-            value = getattr(src, opt)
-            if replace or not _is_seq(value):
+            newvalue = getattr(src, opt)
+            oldvalue = getattr(self, opt)
+            # overwrite value iff:
+            # - we replace unset option by a real value
+            # - new default is set, or
+            # - non-sequential variable keeps its default value
+            if (_unset(oldvalue) and not _unset(newvalue)) or \
+               is_default or \
+               ((opt not in self._nondefault) and (not _is_seq(newvalue))):
                 # Overwrite atomic values
-                setattr(self, opt, getattr(src, opt))
-            else:
+                setattr(self, opt, newvalue)
+                if is_default:
+                    self._nondefault.discard(opt)
+                else:
+                    self._nondefault.add(opt)
+            elif _is_seq(newvalue):
                 # Concatenate sequence types
-                setattr(self, opt, getattr(self, opt) + getattr(src, opt))
+                setattr(self, opt, newvalue + oldvalue)
 
-    def _merge_opts(self, src, replace):
+    def _merge_opts(self, src, is_default):
         for arg in _arg_names:
-            self._merge_opt(arg, src, replace)
+            self._merge_opt(arg, src, is_default)
 
     def __str(self, quote=False, sep=" ", prefix="", suffix=""):
         """Format a SoSOptions object as a human or machine readable string.
@@ -168,6 +154,42 @@ class SoSOptions(object):
             :param *kwargs: a list of ``SoSOptions`` keyword args.
             :returns: the new ``SoSOptions`` object.
         """
+        self.add_preset = ""
+        self.alloptions = False
+        self.all_logs = False
+        self.batch = False
+        self.build = False
+        self.case_id = ""
+        self.chroot = _arg_defaults["chroot"]
+        self.compression_type = _arg_defaults["compression_type"]
+        self.config_file = ""
+        self.debug = False
+        self.del_preset = ""
+        self.desc = ""
+        self.enableplugins = []
+        self.encrypt_key = None
+        self.encrypt_pass = None
+        self.experimental = False
+        self.label = ""
+        self.list_plugins = False
+        self.list_presets = False
+        self.list_profiles = False
+        self.log_size = _arg_defaults["log_size"]
+        self.noplugins = []
+        self.noreport = False
+        self.note = ""
+        self.onlyplugins = []
+        self.plugin_timeout = None
+        self.plugopts = []
+        self.preset = _arg_defaults["preset"]
+        self.profiles = []
+        self.quiet = False
+        self.sysroot = None
+        self.threads = 4
+        self.tmp_dir = ""
+        self.verbosity = _arg_defaults["verbosity"]
+        self.verify = False
+        self._nondefault = set()
         for arg in kwargs.keys():
             if arg not in _arg_names:
                 raise ValueError("Unknown SoSOptions attribute: %s" % arg)
@@ -186,28 +208,61 @@ class SoSOptions(object):
         opts._merge_opts(args, True)
         return opts
 
-    def merge(self, src, replace=False):
+    @classmethod
+    def from_file(cls, argparser, config_file, is_default=True):
+        opts = SoSOptions()
+        config = ConfigParser()
+        try:
+            try:
+                with open(config_file) as f:
+                    config.readfp(f)
+            except (ParsingError, Error) as e:
+                raise exit('Failed to parse configuration '
+                           'file %s' % config_file)
+        except (OSError, IOError) as e:
+            raise exit('Unable to read configuration file %s '
+                       ': %s' % (config_file, e.args[1]))
+
+        if config.has_section("general"):
+            optlist = []
+            for opt, val in config.items("general"):
+                # assume just long options are specified so prefix them by "--"
+                optlist.append("--" + opt + "=" + val)
+            opts._merge_opts(argparser.parse_args(optlist), is_default)
+
+        if config.has_option("plugins", "disable"):
+            opts.noplugins = []
+            opts.noplugins.extend([plugin.strip() for plugin in
+                                  config.get("plugins", "disable").split(',')])
+
+        if config.has_option("plugins", "enable"):
+            opts.enableplugins = []
+            opts.enableplugins.extend(
+                    [plugin.strip() for plugin in
+                     config.get("plugins", "enable").split(',')])
+
+        if config.has_section("tunables"):
+            opts.plugopts = []
+            for opt, val in config.items("tunables"):
+                if not opt.split('.')[0] in opts.noplugins:
+                    opts.plugopts.append(opt + "=" + val)
+
+        return opts
+
+    def merge(self, src, skip_default=True):
         """Merge another set of ``SoSOptions`` into this object.
 
             Merge two ``SoSOptions`` objects by setting unset or default
             values to their value in the ``src`` object.
 
             :param src: the ``SoSOptions`` object to copy from
-            :param replace: ``True`` if non-default values should be
-                            overwritten.
+            :param is_default: ``True`` if new default values are to be set.
         """
         for arg in _arg_names:
             if not hasattr(src, arg):
                 continue
-            if _is_seq(getattr(self, arg)):
-                self._merge_opt(arg, src, replace)
-                continue
-            if arg in _arg_defaults.keys():
-                if replace or getattr(self, arg) == _arg_defaults[arg]:
-                    self._merge_opt(arg, src, replace)
-            else:
-                if replace or not getattr(self, arg):
-                    self._merge_opt(arg, src, replace)
+            if getattr(src, arg) is not None or not skip_default:
+                self._merge_opt(arg, src, False)
 
     def dict(self):
         """Return this ``SoSOptions`` option values as a dictionary of
diff --git a/sos/sosreport.py b/sos/sosreport.py
index 47fcc0cd..c5f406c2 100644
--- a/sos/sosreport.py
+++ b/sos/sosreport.py
@@ -45,11 +45,6 @@ import six
 from six.moves import zip, input
 from six import print_
 
-if six.PY3:
-    from configparser import ConfigParser, ParsingError, Error
-else:
-    from ConfigParser import ConfigParser, ParsingError, Error
-
 # file system errors that should terminate a run
 fatal_fs_errors = (errno.ENOSPC, errno.EROFS)
 
@@ -201,8 +196,8 @@ class XmlReport(object):
 chroot_modes = ["auto", "always", "never"]
 
 
-def _parse_args(args):
-    """ Parse command line options and arguments"""
+def _get_parser():
+    """ Build ArgumentParser content"""
 
     usage_string = ("%(prog)s [options]\n\n"
                     "Some examples:\n\n"
@@ -235,8 +230,8 @@ def _parse_args(args):
                         help="chroot executed commands to SYSROOT "
                              "[auto, always, never] (default=auto)",
                         default=_arg_defaults["chroot"])
-    parser.add_argument("--config-file", action="store",
-                        dest="config_file",
+    parser.add_argument("--config-file", type=str, action="store",
+                        dest="config_file", default="/etc/sos.conf",
                         help="specify alternate configuration file")
     parser.add_argument("--debug", action="store_true", dest="debug",
                         help="enable interactive debugging using the "
@@ -319,6 +314,7 @@ def _parse_args(args):
     preset_grp.add_argument("--del-preset", type=str, action="store",
                             help="Delete the named command line preset")
 
+    # Group to make tarball encryption (via GPG/password) exclusive
     encrypt_grp = parser.add_mutually_exclusive_group()
     encrypt_grp.add_argument("--encrypt-key",
                              help="Encrypt the final archive using a GPG "
@@ -326,7 +322,7 @@ def _parse_args(args):
     encrypt_grp.add_argument("--encrypt-pass",
                              help="Encrypt the final archive using a password")
 
-    return parser.parse_args(args)
+    return parser
 
 
 class SoSReport(object):
@@ -352,30 +348,47 @@ class SoSReport(object):
         except Exception:
             pass  # not available in java, but we don't care
 
-        cmd_args = _parse_args(args)
-        self.opts = SoSOptions.from_args(cmd_args)
+        # load default options and store them in self.opts
+        parser = _get_parser()
+        self.opts = SoSOptions().from_args(parser.parse_args([]))
+
+        # remove default options now, such that by processing cmdline options
+        # we know what exact options were provided there and should not be
+        # overwritten any time further
+        # then merge these options on top of self.opts
+        # this approach is required since:
+        # - we process the more priority options first (cmdline, then config
+        #   file, then presets) - required to know cfgfile or preset
+        # - we have to apply lower prio options only on top of non-default
+        for option in parser._actions:
+            if option.default != '==SUPPRESS==':
+                option.default = None
+        cmd_opts = SoSOptions().from_args(parser.parse_args(args))
+        self.opts.merge(cmd_opts)
+
+        # load options from config.file and merge them to self.opts
+        self.fileopts = SoSOptions().from_file(parser, self.opts.config_file)
+        self.opts.merge(self.fileopts)
         self._set_debug()
-        self._read_config()
 
+        # load preset and options from it - first, identify policy for that
         try:
             self.policy = sos.policies.load(sysroot=self.opts.sysroot)
         except KeyboardInterrupt:
             self._exit(0)
-
         self._is_root = self.policy.is_root()
 
         # user specified command line preset
-        if cmd_args.preset != _arg_defaults["preset"]:
-            self.preset = self.policy.find_preset(cmd_args.preset)
+        if self.opts.preset != _arg_defaults["preset"]:
+            self.preset = self.policy.find_preset(self.opts.preset)
             if not self.preset:
-                sys.stderr.write("Unknown preset: '%s'\n" % cmd_args.preset)
+                sys.stderr.write("Unknown preset: '%s'\n" % self.opts.preset)
                 self.preset = self.policy.probe_preset()
                 self.opts.list_presets = True
-
         # --preset=auto
         if not self.preset:
             self.preset = self.policy.probe_preset()
-
+        # now merge preset options to self.opts
         self.opts.merge(self.preset.opts)
 
         # system temporary directory to use
@@ -432,7 +445,6 @@ class SoSReport(object):
             'verbosity': self.opts.verbosity,
             'xmlreport': self.xml_report,
             'cmdlineopts': self.opts,
-            'config': self.config,
         }
 
     def get_temp_file(self):
@@ -515,24 +527,6 @@ class SoSReport(object):
         if plugname and func:
             self._log_plugin_exception(plugname, func)
 
-    def _read_config(self):
-        self.config = ConfigParser()
-        if self.opts.config_file:
-            config_file = self.opts.config_file
-        else:
-            config_file = '/etc/sos.conf'
-
-        try:
-            try:
-                with open(config_file) as f:
-                    self.config.readfp(f)
-            except (ParsingError, Error) as e:
-                raise exit('Failed to parse configuration '
-                           'file %s' % config_file)
-        except (OSError, IOError) as e:
-            raise exit('Unable to read configuration file %s '
-                       ': %s' % (config_file, e.args[1]))
-
     def _setup_logging(self):
         # main soslog
         self.soslog = logging.getLogger('sos')
@@ -585,13 +579,6 @@ class SoSReport(object):
             self.archive.add_file(self.sos_ui_log_file,
                                   dest=os.path.join('sos_logs', 'ui.log'))
 
-    def _get_disabled_plugins(self):
-        disabled = []
-        if self.config.has_option("plugins", "disable"):
-            disabled = [plugin.strip() for plugin in
-                        self.config.get("plugins", "disable").split(',')]
-        return disabled
-
     def _is_in_profile(self, plugin_class):
         onlyplugins = self.opts.onlyplugins
         if not len(self.opts.profiles):
@@ -603,8 +590,7 @@ class SoSReport(object):
         return any([p in self.opts.profiles for p in plugin_class.profiles])
 
     def _is_skipped(self, plugin_name):
-        return (plugin_name in self.opts.noplugins or
-                plugin_name in self._get_disabled_plugins())
+        return (plugin_name in self.opts.noplugins)
 
     def _is_inactive(self, plugin_name, pluginClass):
         return (not pluginClass(self.get_commons()).check_enabled() and
@@ -727,13 +713,6 @@ class SoSReport(object):
                         parms["enabled"] = True
 
     def _set_tunables(self):
-        if self.config.has_section("tunables"):
-            if not self.opts.plugopts:
-                self.opts.plugopts = []
-
-            for opt, val in self.config.items("tunables"):
-                if not opt.split('.')[0] in self._get_disabled_plugins():
-                    self.opts.plugopts.append(opt + "=" + val)
         if self.opts.plugopts:
             opts = {}
             for opt in self.opts.plugopts:
@@ -774,8 +753,12 @@ class SoSReport(object):
                             self._exit(1)
                     del opts[plugname]
             for plugname in opts.keys():
-                self.soslog.error('unable to set option for disabled or '
-                                  'non-existing plugin (%s)' % (plugname))
+                self.soslog.error('WARNING: unable to set option for disabled '
+                                  'or non-existing plugin (%s)' % (plugname))
+            # in case we printed warnings above, visually intend them from
+            # subsequent header text
+            if opts.keys():
+                self.soslog.error('')
 
     def _check_for_unknown_plugins(self):
         import itertools
@@ -1020,6 +1003,10 @@ class SoSReport(object):
         msg = "[%s:%s] executing 'sosreport %s'"
         self.soslog.info(msg % (__name__, "setup", " ".join(self._args)))
 
+        msg = "[%s:%s] loaded options from config file: %s'"
+        self.soslog.info(msg % (__name__, "setup",
+                         " ".join(self.fileopts.to_args())))
+
         # Log active preset defaults
         preset_args = self.preset.opts.to_args()
         msg = ("[%s:%s] using '%s' preset defaults (%s)" %