[sos-devel] [RESEND RFC PATCH 1/1] [policies] Add IBM PowerKVM policy

[Bryn M. Reeves]

On Wed, Mar 04, 2015 at 09:32:32AM +0530, Kamalesh Babulal wrote:
> Resending the patch, as previous patch had formating issues and duplicate
> imports.

Thanks for resending Kamlesh - I've added some review comments in-line below.
 
> +class PowerKVMPlugin(object):
> +    """Tagging class to indicate that this plugin works with IBM PowerKVM Linux"""
> +    pass
> +
> +

This should derive from the RedHatPlugin class since that is the base class
for all Red Hat derived RPM based distributions. This simplifies plugin
enablement and reduces the clutter and maintenance costs of introducing a
new top-level tagging class.

> -# vim: et ts=4 sw=4
> +# vim: set ts=4 sw=4

Good catch but we should fix this in a separate patch from the PowerKVM
policy work (and fix it across the tree in a single change).

> +from __future__ import print_function
> +from sos.plugins import PowerKVMPlugin

Need to import RedHatPlugin here too.

> +class PowerKVMPolicy(LinuxPolicy):

This should derive from the RedHatPolicy since the __init__ method is
virtually identical (they only differ in the exact set of
valid_subclasses).

> +    distro = "PowerKVM"
> +    vendor = "IBM"
> +    vendor_url = "http://www-03.ibm.com/systems/power/software/linux/powerkvm"
> +
> +    def __init__(self):
> +        super(PowerKVMPolicy, self).__init__()
> +        self.report_name = ""
> +        self.ticket_number = ""
> +        self.package_manager = PackageManager(
> +            'rpm -qa --queryformat "%{NAME}|%{VERSION}\\n"')
> +        self.valid_subclasses = [PowerKVMPlugin]
> +
> +        pkgs = self.package_manager.all_pkgs()
> +
> +        if not pkgs:
> +            print("Could not obtain installed package list", file=sys.stderr)
> +            sys.exit(1)
> +
> +        if pkgs['filesystem']['version'][0] == '3':
> +            self.PATH = "/usr/sbin:/usr/bin:/root/bin"
> +        else:
> +            self.PATH = "/sbin:/bin:/usr/sbin:/usr/bin:/root/bin"
> +        self.PATH = "/sbin:/bin:/usr/sbin:/usr/bin:/root/bin"
> +        self.PATH += os.pathsep + "/usr/local/sbin"
> +        self.set_exec_path()

This can be simplified to something like:

    def __init__(self):
        super(PowerKVMPolicy, self).__init__()
        self.valid_subclasses = [PowerKVMPlugin, RedHatPlugin]

The PowerKVMPlugin tagging class must come first so that if there is a
case where a separate PowerKvmPlugin and RedHatPlugin class exist for
a plugin the PowerKVMPlugin version will be prefered.

Regards,
Bryn.