SCLPlugin doesn't work on EL8 and makes plugins using it not auto-load

[evgeni]

Ohai,

we (@theforeman) recently noticed that the foreman plugin is not automatically enabled on EL8 (CentOS8). A bit of digging revealed that this is due to the fact that the plugin does subclass SCLPlugin

sos/sos/plugins/foreman.py

Lines 309 to 316 in 00a25de

	class RedHatForeman(Foreman, SCLPlugin, RedHatPlugin):
	apachepkg = 'httpd'
	def setup(self):
	super(RedHatForeman, self).setup()
	self.add_cmd_output_scl('tfm', 'gem list',
	suggest_filename='scl enable tfm gem list')

And that seems to confuse sos on EL8 as there are no SCL there.

Dropping the SCLPlugin (and the add_cmd_output_scl call) makes sos (sos-3.9.1-6.el8.noarch) auto-enable the plugin as desired when one of the listed packages is installed.

We see the same behavior with the postgresql plugin, which also uses SCLPlugin.

What's the correct solution here? Fixing SCLPlugin not to fail on EL8? Making two foreman plugins (well, three, we already have RedHatForeman and DebianForeman)?

[TurboTurtle]

I wonder if this is due to the fact that we have:

class RedHatForeman(Foreman, SCLPlugin, RedHatPlugin):

and

class SCLPlugin(RedHatPlugin)

This might be causing an MRO exception during instantiation. If you instead remove RedHatPlugin and leave SCLPlugin, does the plugin load properly on CentOS for you?

[evgeni]

Nope, that's not it. The PostgreSQL plugin doesn't do this double-inheritance from RedHatPlugin:

sos/sos/plugins/postgresql.py

Line 80 in 00a25de

class RedHatPostgreSQL(PostgreSQL, SCLPlugin):

FWIW, this is rather easily reproducible with a CentOS 8 container:

% podman run -ti --rm centos:8 bash
# yum install -y sos postgresql
# sosreport --profile services --list-plugins | grep PostgreSQL
WARNING: unable to set option for disabled or non-existing plugin (boot)

 postgresql           inactive       PostgreSQL RDBMS

(the --profile services is required, as it's not loaded by default in a container that has no init)

Making the plugin definition be

class RedHatPostgreSQL(PostgreSQL, RedHatPlugin)

makes it work (after importing RedHatPlugin)

[TurboTurtle]

Right, so here's the logic for enabling plugins based on if an SCL is present:

            if isinstance(self, SCLPlugin):
                # save SCLs that match files or packages
                type(self)._scls_matched = []
                for scl in self._get_scls():
                    files = [f % {"scl_name": scl} for f in self.files]
                    packages = [p % {"scl_name": scl} for p in self.packages]
                    commands = [c % {"scl_name": scl} for c in self.commands]
                    services = [s % {"scl_name": scl} for s in self.services]
                    if self._check_plugin_triggers(files,
                                                   packages,
                                                   commands,
                                                   services):
                        type(self)._scls_matched.append(scl)
                return len(type(self)._scls_matched) > 0

            return self._check_plugin_triggers(self.files,
                                               self.packages,
                                               self.commands,
                                               self.services)

So we don't even attempt to do the plugin verification based on the normal triggers if a plugin subclasses SCLPlugin at all. This may be a hold over of sorts as in the past the SCLPlugin plugins would usually be a separate class definition by themselves.

So, we have two options I believe:

1. Change the logic above to only return IF we match SCLs, and otherwise let the regular triggers make the enablement decisions.
2. Break out separate subclasses for SCLPlugin (again?).

The problem with option 1 would be controlling the SCLPlugin methods for when the plugin gets enabled by the normal, non-scl, triggers.

The problem with option 2 is we'd have to come up with a way to determine which class to run if both a RedHatPlugin and SCLPlugin are determined to be enabled.

[TurboTurtle]

A quick test for postgresql shows this solves the enablement issue, and for this particular plugin the scl methods don't seem to come into play - or at least they don't generate any errors or unexpected collections.

diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py
index 5ad7e369..24948ed5 100644
--- a/sos/report/plugins/__init__.py
+++ b/sos/report/plugins/__init__.py
@@ -2568,7 +2568,9 @@ class Plugin(object):
                                                    commands,
                                                    services):
                         type(self)._scls_matched.append(scl)
-                return len(type(self)._scls_matched) > 0
+                    if type(self)._scls_matched:
+                        return True
 
             return self._check_plugin_triggers(self.files,
                                                self.packages,

I believe this should solve the issue for Foreman-on-Centos for plugin enablement, but I don't have such a test system available to me to validate if we need any additional protections for the tfm call.

[evgeni]

The patch fixes it for Foreman too, yeah.

It does yield two warnings when executing the plugin:

[plugin:foreman] Failed to find prefix for SCL tfm, using /opt/rh
[plugin:foreman] Failed to find prefix for SCL tfm, using /opt/rh

But that's not bad as such -- we just don't have an SCL on EL8 and use system-ruby.

We could guard the tfm gem list command with a if type(self)._scls_matched:, then the warnings are gone too and nothing is missed (the ruby plugin collects the gem list for the system ruby already).