search

sosreport / sos

A unified tool for collecting system logs and other debug information
http://sos.rtfd.org
GNU General Public License v2.0
508 stars 542 forks source link

Add crictl plugin without crio data #2850

Open samkunz opened 2 years ago

samkunz commented 2 years ago

Example Use Case: User wants to collect crictl data, but does not want to use crio plugin to do so, because does not want to clutter results with "empty" crio related information (e.g. add_copy_spec(["/etc/crio/crio.conf"]), e.g. add_cmd_output(["crio config"])) and/or does not want to introduce risk of crio plugin failing if crio is not installed on system.

Google COS has already implemented a workaround by patching in a separate crictl plugin.

I don't mind opening a pull request to get this functionality into upstream sosreport, but I wanted to check in with the maintainers about the preferred way to do this, since I'm not very familiar with the sosreport design philosophies.

One way to accomplish this is to transition the existing crio plugin into crio, crictl, containerd plugins. E.g. add_cmd_output(["crio config"]) goes into crio plugin, add_cmd_output(["crictl info"]) goes into crictl plugin, self.add_journal(units='containerd') goes into containerd plugin. -> Pro of this approach: minimize duplicate information collected by multiple plugins. -> Con of this approach: existing users of crio plugin would also need to make a change to use crio and crictl plugins to collect the same info. -> Note that this approach doesn't require creation of a containerd plugin -- but to me it seemed like one way to reduce duplication of data collection (e.g. crio plugin and crictl plugin users both might want data on containerd and may not want to commit to crictl and/or crio plugins to get it.)

Another way to accomplish this is to simply add a crictl plugin, similar to the patch linked above, without modifying the crio plugin. -> Pro of this approach: introduces crictl specific plugin to satisfy use case while not changing crio plugin behavior. -> Con of this approach: duplicate data collected between crio plugin and crictl plugin.

Please advise on the preferred approach, thank you!

TurboTurtle commented 2 years ago

First, let me preface all of this with saying we'd welcome and appreciate any contributions from Google/COS coming back into upstream. And looking at the other patches on that branch there are at least a couple that are very good candidates for upstream contribution (such as the COS package manager).

I think there may be some misunderstanding of how sos plugins work, or I may be reading this issue incorrectly:

because does not want to clutter results with "empty" crio related information

If by "empty" you mean a literally empty file within the sos archive - that would only happen if the file on the host is also literally empty, but still exists. If a file specified by add_copy_spec() doesn't exist, we just continue on to the next file in the spec, we don't create an empty file in the archive.

As for commands - if the command doesn't exist on the system, we don't record anything in the archive. We detect the specific return code from a command not found error, and then move on to the next command the plugin marked for collection.

So, in those cases there shouldn't be anything "polluting" the archive. If you're seeing this, then that's a bug we should look into.

does not want to introduce risk of crio plugin failing if crio is not installed on system.

This should not happen. The crio plugin has packages = ('cri-o', 'cri-tools') defined as part of the enablement triggers. If any of the elements in these trigger tuples are present, the plugin activates. So if you have cri-tools installed but not cri-o, the plugin will still activate.

On that note, I should point out that if you're adding crictl.py and not removing crio.py - both plugins will activate on the presence of cri-tools, and you'd be duplicating the crictl collections while still collecting the crio files unless you specifically run sos report with -n crio.

All that said, let me better understand the use case here. For COS, are the items like crio.conf not present on hosts, or are they present but effectively unused by anything? How is sos report typically executed on COS hosts? Do your users typically just run a stock sos report command, or does COS support have a standard command recommended for end users (or a stock sos.conf that's in use)?

On the surface I don't have any objections to splitting out these collections into more specific plugins (for containerd we'd need to pull some stuff out of docker as well, but that's also fine), but I want to better grasp the environments those changes would be made for. The crio plugin as it is today was written from a RH perspective of crio and crictl going hand-in-hand with each other, but that doesn't mean it has to stay that way.

TurboTurtle commented 2 years ago

@samkunz just wanted to check in with you on this one. Does it sound feasible to split out these plugins while contributing some of the COS-specific downstream patches to upstream?

TurboTurtle commented 2 years ago

Ping @samkunz - just wanted to cycle back around on this with you and see what we can work out.

TurboTurtle commented 1 year ago

One last check in. @samkunz - is there any interest in splitting these plugins and/or pulling in some of the COS patches to upstream?

samkunz commented 1 year ago

@TurboTurtle thanks for checking in! I'm no longer involved with the COS project, so I cannot comment on whether there is still interest in splitting these plugins and/or pulling in some of the COS patches to upstream.

That said, my guess is there would be interest in at least pulling COS patches into upstream. @vteratipally can probably provide some opinions from the COS project.