[sos-devel] [PATCH 2/2] [ovirt_hosted_engine] new plugin for oVirt

[Sandro Bonazzola]

Hi,
I think I have addressed all your comments below.
You may find attached a sample of the files involved and 2 sosreport generated by the attached plugin with and without --all-logs.
I've dropped the reverse sorting patch for now.




Il 19/08/2014 12:23, Bryn M. Reeves ha scritto:
> On Tue, Aug 19, 2014 at 11:05:37AM +0200, Sandro Bonazzola wrote:
>> The oVirt Hosted Engine packages allow to run
>> ovirt-engine inside a VM. This plugin provides
>> info about the setup and the high availability daemons
>> running such VM.
> 
> This comment seems to have been copied from an old version of the example
> plugin (example_plugins/example.py):
> 
>> +# Class name must be the same as file name and method names must not change
>> +class OvirtHostedEngine(Plugin, RedHatPlugin):
> 
> There's no need for it in this file.
> 
>> +        # Add configuration files
>> +        self.add_copy_specs([
>> +            '/etc/ovirt-hosted-engine/',
>> +            '/etc/ovirt-hosted-engine-ha/',
>> +            '/etc/ovirt-hosted-engine-setup.env.d/'
>> +        ])
> 
> See the comments in Issue #364: "It'd also be good to have some example
> tarballs representing the raw content of /etc/ovirt-hosted-engine/ and
> /etc/ovirt-hosted-engine-ha/ for review and testing (esp. if these are similar
> to the other ovirt config directories in terms of having secrets scattered
> around multiple files)."
> 
> I'd rather we did not collect whole directories at all and if we're going to
> then we need to have good justification for it and record that.
> 
>> +        # Add ovirt-hosted-engine-setup log files
>> +        self.add_copy_specs([
>> +            '/var/log/ovirt-hosted-engine-setup/'
>> +        ])
> 
> Again, we're trying to avoid whole directory copying, especially in /var.
> 
> There is nothing to prevent something dumping GiBs of data under this
> path and the above code will faithfully attempt to copy it all. This
> needs a more restrictive path and also ideally size limiting (an
> alternative would be to move the whole thing under and 'if
> get_option("all_logs")' branch).
> 
>> +        # Add ovirt-hosted-engine-ha logs, limiting the size
>> +        self.add_copy_spec_limit(
>> +            '/var/log/ovirt-hosted-engine-ha/*.log',
>> +            sizelimit=self.limit,
>> +            reverse_order=True
>> +        )
> 
> The trend with recent plugin changes has been to only collect a single
> (current) log file by default and to make all the rotated copies subject
> to a non-default 'all_logs' option. I'm fine with keeping this as-is if
> the rotated versions are really crucial for oVirt's needs but if not I'd
> like to keep things as consistent as possible.
> 
> Regards,
> Bryn.
> 


-- 
Sandro Bonazzola
Better technology. Faster innovation. Powered by community collaboration.
See how it works at redhat.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ovirt_hosted_engine_sample.tar.xz
Type: application/x-xz
Size: 46360 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/sos-devel/attachments/20140826/4909a2a9/attachment.xz>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sosreport-mididell.home-20140826114946.tar.xz
Type: application/x-xz
Size: 41084 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/sos-devel/attachments/20140826/4909a2a9/attachment-0001.xz>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sosreport-mididell.home-20140826115131.tar.xz
Type: application/x-xz
Size: 49188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/sos-devel/attachments/20140826/4909a2a9/attachment-0002.xz>