[docs|policy] Update Policy to sphinx-style docstrings

Updates the `Policy` and related docstrings to Sphinx-style as part of
the work to bring the API docs up to date.

Related: #2172

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

2 files changed, 405 insertions, 64 deletions

diff --git a/docs/policies.rst b/docs/policies.rst
index 56fb1c23..eeca0f56 100644
--- a/docs/policies.rst
+++ b/docs/policies.rst
@@ -4,5 +4,4 @@
 .. automodule:: sos.policies
     :noindex:
     :members:
-    :undoc-members:
     :show-inheritance:
diff --git a/sos/policies/__init__.py b/sos/policies/__init__.py
index b0b1d508..647aa93f 100644
--- a/sos/policies/__init__.py
+++ b/sos/policies/__init__.py
@@ -63,6 +63,22 @@ class ContainerRuntime(object):
     """Encapsulates a container runtime that provides the ability to plugins to
     check runtime status, check for the presence of specific containers, and
     to format commands to run in those containers
+
+    :param policy: The loaded policy for the system
+    :type policy: ``Policy()``
+
+    :cvar name: The name of the container runtime, e.g. 'podman'
+    :vartype name: ``str``
+
+    :cvar containers: A list of containers known to the runtime
+    :vartype containers: ``list``
+
+    :cvar images: A list of images known to the runtime
+    :vartype images: ``list``
+
+    :cvar binary: The binary command to run for the runtime, must exit within
+                  $PATH
+    :vartype binary: ``str``
     """
 
     name = 'Undefined'
@@ -89,6 +105,9 @@ class ContainerRuntime(object):
 
         Active in this sense means that the runtime can be used to glean
         information about the runtime itself and containers that are running.
+
+        :returns: ``True`` if the runtime is active, else ``False``
+        :rtype: ``bool``
         """
         if is_executable(self.binary):
             self.active = True
@@ -98,7 +117,8 @@ class ContainerRuntime(object):
     def get_containers(self, get_all=False):
         """Get a list of containers present on the system.
 
-        If `get_all` is `True`, also include non-running containers
+        :param get_all: If set, include stopped containers as well
+        :type get_all: ``bool``
         """
         containers = []
         _cmd = "%s ps %s" % (self.binary, '-a' if get_all else '')
@@ -114,6 +134,12 @@ class ContainerRuntime(object):
     def get_container_by_name(self, name):
         """Get the container ID for the container matching the provided
         name
+
+        :param name: The name of the container, note this can be a regex
+        :type name: ``str``
+
+        :returns: The id of the first container to match `name`, else ``None``
+        :rtype: ``str``
         """
         if not self.active or name is None:
             return None
@@ -124,6 +150,9 @@ class ContainerRuntime(object):
 
     def get_images(self):
         """Get a list of images present on the system
+
+        :returns: A list of 2-tuples containing (image_name, image_id)
+        :rtype: ``list``
         """
         images = []
         fmt = '{{lower .Repository}}:{{lower .Tag}} {{lower .ID}}'
@@ -139,6 +168,9 @@ class ContainerRuntime(object):
 
     def get_volumes(self):
         """Get a list of container volumes present on the system
+
+        :returns: A list of volume IDs on the system
+        :rtype: ``list``
         """
         vols = []
         if self.active:
@@ -150,16 +182,34 @@ class ContainerRuntime(object):
         return vols
 
     def fmt_container_cmd(self, container, cmd):
+        """Format a command to run inside a container using the runtime
+
+        :param container: The name or ID of the container in which to run
+        :type container: ``str``
+
+        :param cmd: The command to run inside `container`
+        :type cmd: ``str``
+
+        :returns: Formatted string to run `cmd` inside `container`
+        :rtype: ``str``
+        """
         return "%s %s %s" % (self.run_cmd, container, quote(cmd))
 
     def get_logs_command(self, container):
-        """Return the command string used to dump container logs from the
+        """Get the command string used to dump container logs from the
         runtime
+
+        :param container: The name or ID of the container to get logs for
+        :type container: ``str``
+
+        :returns: Formatted runtime command to get logs from `container`
+        :type: ``str``
         """
         return "%s logs -t %s" % (self.binary, container)
 
 
 class DockerContainerRuntime(ContainerRuntime):
+    """Runtime class to use for systems running Docker"""
 
     name = 'docker'
     binary = 'docker'
@@ -174,6 +224,7 @@ class DockerContainerRuntime(ContainerRuntime):
 
 
 class PodmanContainerRuntime(ContainerRuntime):
+    """Runtime class to use for systems running Podman"""
 
     name = 'podman'
     binary = 'podman'
@@ -185,9 +236,21 @@ class InitSystem(object):
 
     This should be used to query the status of services, such as if they are
     enabled or disabled on boot, or if the service is currently running.
+
+    :param init_cmd: The binary used to interact with the init system
+    :type init_cmd: ``str``
+
+    :param list_cmd: The list subcmd given to `init_cmd` to list services
+    :type list_cmd: ``str``
+
+    :param query_cmd: The query subcmd given to `query_cmd` to query the
+                      status of services
+    :type query_cmd: ``str``
+
     """
 
     def __init__(self, init_cmd=None, list_cmd=None, query_cmd=None):
+        """Initialize a new InitSystem()"""
 
         self.services = {}
 
@@ -196,13 +259,26 @@ class InitSystem(object):
         self.query_cmd = "%s %s" % (self.init_cmd, query_cmd) or None
 
     def is_enabled(self, name):
-        """Check if given service name is enabled """
+        """Check if given service name is enabled
+
+        :param name: The name of the service
+        :type name: ``str``
+
+        :returns: ``True`` if the service is enabled, else ``False``
+        :rtype: ``bool``
+        """
         if self.services and name in self.services:
             return self.services[name]['config'] == 'enabled'
         return False
 
     def is_disabled(self, name):
-        """Check if a given service name is disabled """
+        """Check if a given service name is disabled
+        :param name: The name of the service
+        :type name: ``str``
+
+        :returns: ``True`` if the service is disabled, else ``False``
+        :rtype: ``bool``
+        """
         if self.services and name in self.services:
             return self.services[name]['config'] == 'disabled'
         return False
@@ -210,6 +286,12 @@ class InitSystem(object):
     def is_service(self, name):
         """Checks if the given service name exists on the system at all, this
         does not check for the service status
+
+        :param name: The name of the service
+        :type name: ``str``
+
+        :returns: ``True`` if the service exists, else ``False``
+        :rtype: ``bool``
         """
         return name in self.services
 
@@ -217,6 +299,12 @@ class InitSystem(object):
         """Checks if the given service name is in a running state.
 
         This should be overridden by initsystems that subclass InitSystem
+
+        :param name: The name of the service
+        :type name: ``str``
+
+        :returns: ``True`` if the service is running, else ``False``
+        :rtype: ``bool``
         """
         # This is going to be primarily used in gating if service related
         # commands are going to be run or not. Default to always returning
@@ -228,6 +316,9 @@ class InitSystem(object):
         """This loads all services known to the init system into a dict.
         The dict should be keyed by the service name, and contain a dict of the
         name and service status
+
+        This must be overridden by anything that subclasses `InitSystem` in
+        order for service methods to function properly
         """
         pass
 
@@ -245,19 +336,35 @@ class InitSystem(object):
         determination of what the state of the service is
 
         This should be overriden by anything that subclasses InitSystem
+
+        :param output: The raw output from querying the service with the
+                       configured `query_cmd`
+        :type output: ``str``
+
+        :returns: A state for the service, e.g. 'active', 'disabled', etc...
+        :rtype: ``str``
         """
         return output
 
     def get_service_names(self, regex):
         """Get a list of all services discovered on the system that match the
         given regex.
+
+        :param regex: The service name regex to match against
+        :type regex: ``str``
         """
         reg = re.compile(regex, re.I)
         return [s for s in self.services.keys() if reg.match(s)]
 
     def get_service_status(self, name):
-        """Returns the status for the given service name along with the output
+        """Get the status for the given service name along with the output
         of the query command
+
+        :param name: The name of the service
+        :type name: ``str``
+
+        :returns: Service status and query_cmd output from the init system
+        :rtype: ``dict`` with keys `name`, `status`, and `output`
         """
         _default = {
             'name': name,
@@ -278,6 +385,7 @@ class InitSystem(object):
 
 
 class SystemdInit(InitSystem):
+    """InitSystem abstraction for SystemD systems"""
 
     def __init__(self):
         super(SystemdInit, self).__init__(
@@ -320,6 +428,26 @@ class PackageManager(object):
 
     You may also subclass this class and provide a get_pkg_list method to
     build the list of packages and versions.
+
+    :cvar query_command: The command to use for querying packages
+    :vartype query_command: ``str`` or ``None``
+
+    :cvar verify_command: The command to use for verifying packages
+    :vartype verify_command: ``str`` or ``None``
+
+    :cvar verify_filter: Optional filter to use for controlling package
+                         verification
+    :vartype verify_filter: ``str or ``None``
+
+    :cvar files_command: The command to use for getting file lists for packages
+    :vartype files_command: ``str`` or ``None``
+
+    :cvar chroot: Perform a chroot when executing `files_command`
+    :vartype chroot: ``bool``
+
+    :cvar remote_exec: If package manager is on a remote system (e.g. for
+                       sos collect), prepend this SSH command to run remotely
+    :vartype remote_exec: ``str`` or ``None``
     """
 
     query_command = None
@@ -352,20 +480,40 @@ class PackageManager(object):
 
     def all_pkgs_by_name(self, name):
         """
-        Return a list of packages that match name.
+        Get a list of packages that match name.
+
+        :param name: The name of the package
+        :type name: ``str``
+
+        :returns: List of all packages matching `name`
+        :rtype: ``list``
         """
         return fnmatch.filter(self.all_pkgs().keys(), name)
 
     def all_pkgs_by_name_regex(self, regex_name, flags=0):
         """
-        Return a list of packages that match regex_name.
+        Get a list of packages that match regex_name.
+
+        :param regex_name: The regex to use for matching package names against
+        :type regex_name: ``str``
+
+        :param flags: Flags for the `re` module when matching `regex_name`
+
+        :returns: All packages matching `regex_name`
+        :rtype: ``list``
         """
         reg = re.compile(regex_name, flags)
         return [pkg for pkg in self.all_pkgs().keys() if reg.match(pkg)]
 
     def pkg_by_name(self, name):
         """
-        Return a single package that matches name.
+        Get a single package that matches name.
+
+        :param name: The name of the package
+        :type name: ``str``
+
+        :returns: The first package that matches `name`
+        :rtype: ``str``
         """
         pkgmatches = self.all_pkgs_by_name(name)
         if (len(pkgmatches) != 0):
@@ -406,6 +554,12 @@ class PackageManager(object):
 
     def pkg_version(self, pkg):
         """Returns the entry in self.packages for pkg if it exists
+
+        :param pkg: The name of the package
+        :type pkg: ``str``
+
+        :returns: Package name and version, if package exists
+        :rtype: ``dict`` if found, else ``None``
         """
         pkgs = self.all_pkgs()
         if pkg in pkgs:
@@ -414,13 +568,24 @@ class PackageManager(object):
 
     def all_pkgs(self):
         """
-        Return a list of all packages.
+        Get a list of all packages.
+
+        :returns: All packages, with name and version, installed on the system
+        :rtype: ``dict``
         """
         if not self.packages:
             self.packages = self.get_pkg_list()
         return self.packages
 
     def pkg_nvra(self, pkg):
+        """Get the name, version, release, and architecture for a package
+
+        :param pkg: The name of the package
+        :type pkg: ``str``
+
+        :returns: name, version, release, and arch of the package
+        :rtype: ``tuple``
+        """
         fields = pkg.split("-")
         version, release, arch = fields[-3:]
         name = "-".join(fields[:-3])
@@ -428,7 +593,10 @@ class PackageManager(object):
 
     def all_files(self):
         """
-        Returns a list of files known by the package manager
+        Get a list of files known by the package manager
+
+        :returns: All files known by the package manager
+        :rtype: ``list``
         """
         if self.files_command and not self.files:
             cmd = self.files_command
@@ -451,7 +619,7 @@ class PackageManager(object):
             :returns: a string containing an executable command
                       that will perform verification of the given
                       packages.
-            :returntype: str or ``NoneType``
+            :rtype: str or ``NoneType``
         """
         if not self.verify_command:
             return None
@@ -485,7 +653,20 @@ OPTS = "args"
 
 
 class PresetDefaults(object):
-    """Preset command line defaults.
+    """Preset command line defaults to allow for quick reference to sets of
+    commonly used options
+
+    :param name: The name of the new preset
+    :type name: ``str``
+
+    :param desc: A description for the new preset
+    :type desc: ``str``
+
+    :param note: Note for the new preset
+    :type note: ``str``
+
+    :param opts: Options set for the new preset
+    :type opts: ``SoSOptions``
     """
     #: Preset name, used for selection
     name = None
@@ -517,10 +698,6 @@ class PresetDefaults(object):
         """Initialise a new ``PresetDefaults`` object with the specified
             arguments.
 
-            :param name: The name of the new preset
-            :param desc: A description for the new preset
-            :param note: Note for the new preset
-            :param opts: Options set for the new preset
             :returns: The newly initialised ``PresetDefaults``
         """
         self.name = name
@@ -531,8 +708,8 @@ class PresetDefaults(object):
     def write(self, presets_path):
         """Write this preset to disk in JSON notation.
 
-            :param presets_path: the directory where the preset will be
-                                 written.
+        :param presets_path: the directory where the preset will be written
+        :type presets_path: ``str``
         """
         if self.builtin:
             raise TypeError("Cannot write built-in preset")
@@ -548,6 +725,11 @@ class PresetDefaults(object):
             json.dump(pdict, pfile)
 
     def delete(self, presets_path):
+        """Delete a preset from disk
+
+        :param presets_path: the directory where the preset is saved
+        :type presets_path: ``str``
+        """
         os.unlink(os.path.join(presets_path, self.name))
 
 
@@ -562,6 +744,41 @@ GENERIC_PRESETS = {
 
 
 class Policy(object):
+    """Policies represent distributions that sos supports, and define the way
+    in which sos behaves on those distributions. A policy should define at
+    minimum a way to identify the distribution, and a package manager to allow
+    for package based plugin enablement.
+
+    Policies also control preferred ContainerRuntime()'s, upload support to
+    default locations for distribution vendors, disclaimer text, and default
+    presets supported by that distribution or vendor's products.
+
+    Every Policy will also need at least one "tagging class" for plugins.
+
+    :param sysroot: Set the sysroot for the system, if not /
+    :type sysroot: ``str`` or ``None``
+
+    :param probe_runtime: Should the Policy try to load a ContainerRuntime
+    :type probe_runtime: ``bool``
+
+    :cvar distro: The name of the distribution the Policy represents
+    :vartype distro: ``str``
+
+    :cvar vendor: The name of the vendor producing the distribution
+    :vartype vendor: ``str``
+
+    :cvar vendor_url: URL for the vendor's website, or support portal
+    :vartype vendor_url: ``str``
+
+    :cvar vendor_text: Additional text to add to the banner message
+    :vartype vendor_text: ``str``
+
+    :cvar name_pattern: The naming pattern to be used for naming archives
+                        generated by sos. Values of `legacy`, and `friendly`
+                        are preset patterns. May also be set to an explicit
+                        custom pattern, see `get_archive_name()`
+    :vartype name_pattern: ``str``
+    """
 
     msg = _("""\
 This command will collect system configuration and diagnostic information \
@@ -630,15 +847,26 @@ any third party.
         If `remote` is provided, it should be the contents of os-release from
         a remote host, or a similar vendor-specific file that can be used in
         place of a locally available file.
+
+        :returns: ``True`` if the Policy should be loaded, else ``False``
+        :rtype: ``bool``
         """
         return False
 
     def in_container(self):
-        """ Returns True if sos is running inside a container environment.
+        """Are we running inside a container?
+
+        :returns: ``True`` if in a container, else ``False``
+        :rtype: ``bool``
         """
         return self._in_container
 
     def host_sysroot(self):
+        """Get the host's default sysroot
+
+        :returns: Host sysroot
+        :rtype: ``str`` or ``None``
+        """
         return self._host_sysroot
 
     def dist_version(self):
@@ -660,26 +888,29 @@ any third party.
         This function should return the filename of the archive without the
         extension.
 
-        This uses the policy's name_pattern attribute to determine the name.
-        There are two pre-defined naming patterns - 'legacy' and 'friendly'
+        This uses the policy's `name_pattern` attribute to determine the name.
+        There are two pre-defined naming patterns - `legacy` and `friendly`
         that give names like the following:
 
-        legacy - 'sosreport-tux.123456-20171224185433'
-        friendly - 'sosreport-tux-mylabel-123456-2017-12-24-ezcfcop.tar.xz'
+        * legacy - `sosreport-tux.123456-20171224185433`
+        * friendly - `sosreport-tux-mylabel-123456-2017-12-24-ezcfcop.tar.xz`
 
         A custom name_pattern can be used by a policy provided that it
         defines name_pattern using a format() style string substitution.
 
         Usable substitutions are:
 
-            name  - the short hostname of the system
-            label - the label given by --label
-            case  - the case id given by --case-id or --ticker-number
-            rand  - a random string of 7 alpha characters
+            * name  - the short hostname of the system
+            * label - the label given by --label
+            * case  - the case id given by --case-id or --ticker-number
+            * rand  - a random string of 7 alpha characters
 
         Note that if a datestamp is needed, the substring should be set
-        in the name_pattern in the format accepted by strftime().
+        in `name_pattern` in the format accepted by ``strftime()``.
 
+        :returns: A name to be used for the archive, as expanded from
+                  the Policy `name_pattern`
+        :rtype: ``str``
         """
         name = self.get_local_name().split('.')[0]
         case = self.case_id
@@ -715,6 +946,17 @@ any third party.
         return binary
 
     def get_cmd_for_compress_method(self, method, threads):
+        """Determine the command to use for compressing the archive
+
+        :param method: The compression method/binary to use
+        :type method: ``str``
+
+        :param threads: Number of threads compression should use
+        :type threads: ``int``
+
+        :returns: Full command to use to compress the archive
+        :rtype: ``str``
+        """
         cmd = method
         if cmd.startswith("xz"):
             # XZ set compression to -2 and use threads
@@ -730,6 +972,16 @@ any third party.
         return self.default_scl_prefix
 
     def match_plugin(self, plugin_classes):
+        """Determine what subclass of a Plugin should be used based on the
+        tagging classes assigned to the Plugin
+
+        :param plugin_classes: The classes that the Plugin subclasses
+        :type plugin_classes: ``list``
+
+        :returns: The first subclass that matches one of the Policy's
+                  `valid_subclasses`
+        :rtype: A tagging class for Plugins
+        """
         if len(plugin_classes) > 1:
             for p in plugin_classes:
                 # Give preference to the first listed tagging class
@@ -742,6 +994,12 @@ any third party.
     def validate_plugin(self, plugin_class, experimental=False):
         """
         Verifies that the plugin_class should execute under this policy
+
+        :param plugin_class: The tagging class being checked
+        :type plugin_class: A Plugin() tagging class
+
+        :returns: ``True`` if the `plugin_class` is allowed by the policy
+        :rtype: ``bool``
         """
         valid_subclasses = [IndependentPlugin] + self.valid_subclasses
         if experimental:
@@ -762,6 +1020,14 @@ any third party.
         pass
 
     def pkg_by_name(self, pkg):
+        """Wrapper to retrieve a package from the Policy's package manager
+
+        :param pkg: The name of the package
+        :type pkg: ``str``
+
+        :returns: The first package that matches `pkg`
+        :rtype: ``str``
+        """
         return self.package_manager.pkg_by_name(pkg)
 
     def _parse_uname(self):
@@ -774,6 +1040,8 @@ any third party.
         self.machine = machine
 
     def set_commons(self, commons):
+        """Set common host data for the Policy to reference
+        """
         self.commons = commons
 
     def _set_PATH(self, path):
@@ -784,7 +1052,11 @@ any third party.
 
     def is_root(self):
         """This method should return true if the user calling the script is
-        considered to be a superuser"""
+        considered to be a superuser
+
+        :returns: ``True`` if user is superuser, else ``False``
+        :rtype: ``bool``
+        """
         return (os.getuid() == 0)
 
     def get_preferred_hash_name(self):
@@ -794,8 +1066,24 @@ any third party.
 
     def display_results(self, archive, directory, checksum, archivestat=None,
                         map_file=None):
-        # Display results is called from the tail of SoSReport.final_work()
-        #
+        """Display final information about a generated archive
+
+        :param archive: The name of the archive that was generated
+        :type archive: ``str``
+
+        :param directory: The build directory for sos if --build was used
+        :type directory: ``str``
+
+        :param checksum: The checksum of the archive
+        :type checksum: ``str``
+
+        :param archivestat: stat() information for the archive
+        :type archivestat: `os.stat_result`
+
+        :param map_file: If sos clean was invoked, the location of the mapping
+                         file for this run
+        :type map_file: ``str``
+        """
         # Logging is already shutdown and all terminal output must use the
         # print() call.
 
@@ -839,7 +1127,11 @@ any third party.
         """This method is used to prepare the preamble text to display to
         the user in non-batch mode. If your policy sets self.distro that
         text will be substituted accordingly. You can also override this
-        method to do something more complicated."""
+        method to do something more complicated.
+
+        :returns: Formatted banner message string
+        :rtype: ``str``
+        """
         if self.commons['cmdlineopts'].allow_system_changes:
             changes_text = "Changes CAN be made to system configuration."
         else:
@@ -1144,7 +1436,8 @@ class LinuxPolicy(Policy):
             self.upload_password = getpass(msg)
 
     def upload_archive(self, archive):
-        """Entry point for sos attempts to upload the generated archive to a
+        """
+        Entry point for sos attempts to upload the generated archive to a
         policy or user specified location.
 
         Curerntly there is support for HTTPS, SFTP, and FTP. HTTPS uploads are
@@ -1154,34 +1447,39 @@ class LinuxPolicy(Policy):
         respective upload_https(), upload_sftp(), and/or upload_ftp() methods
         and should NOT override this method.
 
+        :param archive: The archive filepath to use for upload
+        :type archive: ``str``
+
         In order to enable this for a policy, that policy needs to implement
         the following:
 
-        Required:
-            Class Attrs:
-                _upload_url                 The default location to use. Note
-                                            these MUST include protocol header
-                _upload_user                Default username, if any else None
-                _upload_password            Default password, if any else None
-                _use_https_streaming        Set to True if the HTTPS endpoint
-                                            supports streaming data
-
-        Optional:
-            Class Attrs:
-                _upload_directory   Default FTP server directory, if any
-
-            Methods:
-                prompt_for_upload_user()    Determines if sos should prompt
-                                            for a username or not.
-                get_upload_user()           Determines if the default or a
-                                            different username should be used
-                get_upload_https_auth()     Format authentication data for
-                                            HTTPS uploads
-                get_upload_url_string()     If you want your policy to print
-                                            a string other than the default URL
-                                            for your vendor/distro, override
-                                            this method
+        Required Class Attrs
+
+        :_upload_url:     The default location to use. Note these MUST include
+                          protocol header
+        :_upload_user:    Default username, if any else None
+        :_upload_password: Default password, if any else None
+        :_use_https_streaming: Set to True if the HTTPS endpoint supports
+                               streaming data
 
+        The following Class Attrs may optionally be overidden by the Policy
+
+        :_upload_directory:     Default FTP server directory, if any
+
+
+        The following methods may be overridden by ``Policy`` as needed
+
+        `prompt_for_upload_user()`
+            Determines if sos should prompt for a username or not.
+
+        `get_upload_user()`
+            Determines if the default or a different username should be used
+
+        `get_upload_https_auth()`
+            Format authentication data for HTTPS uploads
+
+        `get_upload_url_string()`
+            Print a more human-friendly string than vendor URLs
         """
         self.upload_archive = archive
         self.upload_url = self.get_upload_url()
@@ -1213,6 +1511,15 @@ class LinuxPolicy(Policy):
 
     def get_upload_https_auth(self, user=None, password=None):
         """Formats the user/password credentials using basic auth
+
+        :param user: The username for upload
+        :type user: ``str``
+
+        :param password: Password for `user` to use for upload
+        :type password: ``str``
+
+        :returns: The user/password auth suitable for use in reqests calls
+        :rtype: ``requests.auth.HTTPBasicAuth()``
         """
         if not user:
             user = self.get_upload_user()
@@ -1224,6 +1531,9 @@ class LinuxPolicy(Policy):
     def get_upload_url(self):
         """Helper function to determine if we should use the policy default
         upload url or one provided by the user
+
+        :returns: The URL to use for upload
+        :rtype: ``str``
         """
         return self.upload_url or self._upload_url
 
@@ -1236,12 +1546,18 @@ class LinuxPolicy(Policy):
     def get_upload_user(self):
         """Helper function to determine if we should use the policy default
         upload user or one provided by the user
+
+        :returns: The username to use for upload
+        :rtype: ``str``
         """
         return self.upload_user or self._upload_user
 
     def get_upload_password(self):
         """Helper function to determine if we should use the policy default
         upload password or one provided by the user
+
+        :returns: The password to use for upload
+        :rtype: ``str``
         """
         return self.upload_password or self._upload_password
 
@@ -1263,8 +1579,7 @@ class LinuxPolicy(Policy):
 
         Policies should override this method instead of the base upload_https()
 
-        Positional arguments:
-            :param archive:     The open archive file object
+        :param archive:     The open archive file object
         """
         return requests.put(self.get_upload_url(), data=archive,
                             auth=self.get_upload_https_auth())
@@ -1280,8 +1595,7 @@ class LinuxPolicy(Policy):
 
         Policies should override this method instead of the base upload_https()
 
-        Positional arguments:
-            :param archive:     The open archive file object
+        :param archive:     The open archive file object
         """
         files = {
             'file': (archive.name.split('/')[-1], archive,
@@ -1296,6 +1610,11 @@ class LinuxPolicy(Policy):
         Policies may define whether this upload attempt should use streaming
         or non-streaming data by setting the `use_https_streaming` class
         attr to True
+
+        :returns: ``True`` if upload is successful
+        :rtype: ``bool``
+
+        :raises: ``Exception`` if upload was unsuccessful
         """
         if not REQUESTS_LOADED:
             raise Exception("Unable to upload due to missing python requests "
@@ -1318,6 +1637,23 @@ class LinuxPolicy(Policy):
     def upload_ftp(self, url=None, directory=None, user=None, password=None):
         """Attempts to upload the archive to either the policy defined or user
         provided FTP location.
+
+        :param url: The URL to upload to
+        :type url: ``str``
+
+        :param directory: The directory on the FTP server to write to
+        :type directory: ``str`` or ``None``
+
+        :param user: The user to authenticate with
+        :type user: ``str``
+
+        :param password: The password to use for `user`
+        :type password: ``str``
+
+        :returns: ``True`` if upload is successful
+        :rtype: ``bool``
+
+        :raises: ``Exception`` if upload in unsuccessful
         """
         try:
             import ftplib
@@ -1394,7 +1730,7 @@ class LinuxPolicy(Policy):
         return ''
 
     def restart_sos_container(self):
-        """Restarts the container created for sos-collector if it has stopped.
+        """Restarts the container created for sos collect if it has stopped.
 
         This is called immediately after create_sos_container() as the command
         to create the container will exit and the container will stop. For
@@ -1407,7 +1743,13 @@ class LinuxPolicy(Policy):
 
     def format_container_command(self, cmd):
         """Returns the command that allows us to exec into the created
-        container for sos-collector.
+        container for sos collect.
+
+        :param cmd: The command to run in the sos container
+        :type cmd: ``str``
+
+        :returns: The command to execute to run `cmd` in the container
+        :rtype: ``str``
         """
         if self.container_runtime:
             return '%s exec %s %s' % (self.container_runtime,