[sos-devel] [PATCH v2] Add an sos plugin for the kimchi platform

[Christy Perez]

On 12/09/2014 10:41 AM, Bryn M. Reeves wrote:
> On Fri, Dec 05, 2014 at 06:49:51PM -0600, Christy Perez wrote:
>> This plugin gathers kimchi logs as well as information
>> about libvirt storage pools defined on the system.
>>
>> Kimchi is an HTML5-based virtualization-management platform
>> For more information, please visit:
>> https://github.com/kimchi-project/kimchi/wiki
>>
>> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
> 
> Hi Christy,
> 
> Thanks for submitting the plugin.

Thanks for the suggestions! I'm sending a v3 implementing them all.
> 
>> ---
>>  sos/plugins/kimchi.py | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>  create mode 100644 sos/plugins/kimchi.py
> 
> You're welcome to add a copyright header here if you wish (we don't have
> a strict policy either way - plugin authors are free to add one or not).
> 
>> +### This program is free software; you can redistribute it and/or modify
>> +## it under the terms of the GNU General Public License as published by
>> +## the Free Software Foundation; either version 2 of the License, or
>> +## (at your option) any later version.
> 
>> +    def setup(self):
>> +        self.add_copy_spec([
>> +            "/etc/kimchi/",
>> +            "/var/log/kimchi*"
>> +        ])
> 
> Because of the possible size of logs (esp. on systems that have been
> running for some time and that do not have log rotation enabled) we try
> to limit the size of files collected from /var/log/ by default.
> 
> The add_copy_spec_limit() method allows you to do this for single files,
> or lists of files (with optional glob expansion). The only limitation is
> that the size restrictions do not work for directories.
> 
> There's also the global 'all_logs' option that overrides this so
> something like this is common:
> 
>     if not self.get_option("all_logs"):
>         self.add_copy_spec_limit("/var/log/kimchi*.log", sizelimit=limit)
>     else:
>         self.add_copy_spec("/var/log/kimchi*.log")
> 
>> +        file_name = self.get_cmd_output_now("virsh -r pool-list --details")
>> +        if file_name is not None:
>> +            with open(file_name, 'r') as pools:
>> +                for pool in list(pools)[2:]:
>> +                    try:
>> +                        pool_name = pool.lstrip().split()[0]
>> +                        self.get_cmd_output_now("virsh -r vol-list --pool \
>> +                                                %s --details" % pool_name)
> 
> You only need to use get_cmd_output_now() when you both want to store
> the result in the report and have it immediately available (e.g. to
> drive further collection). Since that's not the case here you can just
> use add_cmd_output().
> 
> More importantly though I don't really think that these virsh operations
> belong in the kimchi plugin. There's nothing kimchi-specific that I can see
> here and we already have a libvirt plugin - it seems to make more sense to
> add this collections there.
> 
> Regards,
> Bryn.
>