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

[Bryn M. Reeves]

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.