[maas] improve/refactor MAAS plugin #3687

troyanov · 2024-06-25T13:06:51Z

Changes:

plugin_timeout = 1800 (because journal collection takes time)
collect journal for all MAAS related services (by default since yesterday)
mask passwords in (conf|yaml|yml|toml) files with do_path_regex_sub
collect only recent syslog data for machines (if --all-logs was not provided)

snap:

collect all .conf and .yaml files
exclude files containing secrets and keys under /var/snap/maas/**

deb:

add missing service maas-temporal-worker introduced in MAAS 3.5
collect /var/lib/maas/temporal
use glob pattern to collect .conf files
exclude secrets and keys under /var/lib/maas/** and /etc/maas/**
remove /var/log/upstart/maas-* as it was removed in xenial
remove maas-region dumpdata as it can expose sensitive information

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

Is the commit message split over multiple lines and hard-wrapped at 72 characters?
Is the subject and message clear and concise?
Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
Does the commit contain a Signed-off-by: First Lastname [email protected]?
Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
Are all passwords or private data gathered by this PR obfuscated?

packit-as-a-service · 2024-06-25T13:10:59Z

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

sudo yum install -y dnf-plugins-core on RHEL 8
sudo dnf install -y dnf-plugins-core on Fedora
dnf copr enable packit/sosreport-sos-3687
And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

arif-ali

some comments based on our discussions

sos/report/plugins/maas.py

arif-ali

CI caught the following issue, which we ought to fix.

Also, we prefer commits to be squashed for final merge, so once this is ready, that should be done.

sos/report/plugins/maas.py

pmoravec · 2024-06-28T06:02:13Z

sos/report/plugins/maas.py

+        if not os.path.exists(dir):
+            return []
+
+        # Machine messages are collected with syslog and are stored under:
+        #   $template "{{log_dir}}/rsyslog/%HOSTNAME%/%$YEAR%-%$MONTH%-%$DAY%"
+        # Collect only the most recent "%$YEAR%-%$MONTH%-%$DAY%"
+        # for each "%HOSTNAME%".
+        recent = []
+        for host_dir in os.listdir(dir):
+            host_path = os.path.join(dir, host_dir)
+            if not os.path.isdir(host_path):
+                continue
+
+            subdirs = [
+                os.path.join(host_path, d)
+                for d in os.listdir(host_path)
+                if os.path.isdir(host_path)
+            ]
+
+            if not subdirs:
+                continue
+
+            recent.append(sorted(
+                subdirs, key=lambda f: os.stat(f).st_mtime, reverse=True)[0]
+            )
+
+        return recent


I think using glob.glob can be shorter here but since you need to apply some logic (take most recent file from each subdir only), the code sounds good to me as well.

Thanks for the review!

I was looking for some built-in functions that I can use together with glob.glob but didn't find anything.
MAAS collects machine logs using rsyslog and stores them in the following way:

/var/snap/maas/common/log/rsyslog$ tree . ├── 32toledo │ └── 2021-01-21 │ └── messages ├── adm21 │ └── 2021-03-03 │ └── messages ├── amco │ ├── 2020-05-26 │ │ └── messages │ ├── 2020-05-28 │ │ └── messages │ └── 2020-08-06 │ └── messages └── witty-shrimp └── 2021-10-05 └── messages

I thought it would be great to collect just the most recent machine log (e.g. for amco that will be amco/2020-08-06) as it will speed up sos report -o maas in most of the cases.

If --all_logs provided, we want to collect all the data for all the dates

Hm, I think for a better UX I should also check self.get_option("since") here and return matching directories

troyanov · 2024-06-28T06:02:19Z

sos/report/plugins/maas.py

-                self._log_error(
-                    "Cannot login into MAAS remote API with provided creds.")
+    def setup(self):
+        for service in self._services:


@arif-ali I decided not to use services because I don't want default behaviour as defined in

sos/sos/report/plugins/__init__.py

Lines 3334 to 3347 in ca7300e

def add_default_collections(self):

"""Based on the class attrs defined for plugin enablement, add a

standardized set of collections before we call the plugin's own setup()

method.

"""

# For any service used for enablement checks, collect its current

# status if it exists

for service in self.services:

if self.is_service(service):

self.add_service_status(service)

self.add_journal(service)

for kmod in self.kernel_mods:

if self.is_module_loaded(kmod):

self.add_cmd_output(f"modinfo {kmod}")

I would like to call add_journal with since -1days by default

yeah, I think that's what was here before from the MAAS plugin

troyanov · 2024-06-28T07:01:51Z

I am going to try the new plugin on various MAAS deployments over the weekend and I will move PR into "Read for review" if there will be no issues.

arif-ali

lgtm

TurboTurtle

Looks good overall, just noting that we should prefer the path wrappers in Plugin so that we can account for sysroot changes.

TurboTurtle · 2024-06-28T14:04:37Z

sos/report/plugins/maas.py

+        for host_dir in os.listdir(dir):
+            host_path = os.path.join(dir, host_dir)
+            if not os.path.isdir(host_path):
+                continue
+
+            subdirs = [
+                os.path.join(host_path, d)
+                for d in os.listdir(host_path)
+                if os.path.isdir(host_path)
+            ]


Plugin includes wrappers for these path methods, e.g. Plugin.path_join(), that are preferred because they will account for any sysroot handed to sos.

Oh, TIL. Thanks!

- plugin_timeout = 1800 (because journal collection takes time) - collect journal for all MAAS related services (by default since yesterday) - mask passwords in (conf|yaml|yml|toml) files with `do_path_regex_sub` - collect only recent syslog data for machines (if `--all-logs` was not provided) snap: - collect all `.conf` and `.yaml` files - exclude files containing secrets and keys under `/var/snap/maas/**` deb: - add missing service `maas-temporal-worker` introduced in MAAS 3.5 - collect `/var/lib/maas/temporal` - use glob pattern to collect `.conf` files - exclude secrets and keys under `/var/lib/maas/**` and `/etc/maas/**` - remove `/var/log/upstart/maas-*` as it was removed in xenial - remove `maas-region dumpdata` as it can expose sensitive information Signed-off-by: Anton Troyanov <[email protected]>

arif-ali reviewed Jun 25, 2024

View reviewed changes

sos/report/plugins/maas.py Show resolved Hide resolved

sos/report/plugins/maas.py Outdated Show resolved Hide resolved

sos/report/plugins/maas.py Outdated Show resolved Hide resolved

sos/report/plugins/maas.py Outdated Show resolved Hide resolved

cpg1111 reviewed Jun 26, 2024

View reviewed changes

sos/report/plugins/maas.py Show resolved Hide resolved

troyanov force-pushed the refactor-maas branch 2 times, most recently from a0d3c43 to a52f4ec Compare June 27, 2024 14:15

arif-ali requested changes Jun 27, 2024

View reviewed changes

sos/report/plugins/maas.py Outdated Show resolved Hide resolved

troyanov force-pushed the refactor-maas branch 5 times, most recently from 6b12d58 to cffbe9c Compare June 27, 2024 23:01

pmoravec reviewed Jun 28, 2024

View reviewed changes

troyanov commented Jun 28, 2024

View reviewed changes

pmoravec approved these changes Jun 28, 2024

View reviewed changes

troyanov force-pushed the refactor-maas branch from cffbe9c to 88cf3f6 Compare June 28, 2024 06:52

arif-ali approved these changes Jun 28, 2024

View reviewed changes

troyanov force-pushed the refactor-maas branch from 88cf3f6 to b12c18d Compare June 28, 2024 10:01

pmoravec approved these changes Jun 28, 2024

View reviewed changes

TurboTurtle reviewed Jun 28, 2024

View reviewed changes

troyanov force-pushed the refactor-maas branch 3 times, most recently from 2274805 to 6e9664a Compare July 4, 2024 12:00

troyanov force-pushed the refactor-maas branch from 6e9664a to 3522e1f Compare July 4, 2024 14:08

troyanov marked this pull request as ready for review August 6, 2024 09:07

arif-ali approved these changes Aug 8, 2024

View reviewed changes

pmoravec approved these changes Aug 8, 2024

View reviewed changes

TurboTurtle approved these changes Aug 8, 2024

View reviewed changes

TurboTurtle merged commit 992fda9 into sosreport:main Aug 8, 2024
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[maas] improve/refactor MAAS plugin #3687

[maas] improve/refactor MAAS plugin #3687

troyanov commented Jun 25, 2024 •

edited

Loading

packit-as-a-service bot commented Jun 25, 2024

arif-ali left a comment

arif-ali left a comment

pmoravec Jun 28, 2024

troyanov Jun 28, 2024

troyanov Jun 28, 2024

troyanov Jun 28, 2024

arif-ali Jun 28, 2024

troyanov commented Jun 28, 2024

arif-ali left a comment

TurboTurtle left a comment

TurboTurtle Jun 28, 2024

troyanov Jun 28, 2024

	def add_default_collections(self):
	"""Based on the class attrs defined for plugin enablement, add a
	standardized set of collections before we call the plugin's own setup()
	method.
	"""
	# For any service used for enablement checks, collect its current
	# status if it exists
	for service in self.services:
	if self.is_service(service):
	self.add_service_status(service)
	self.add_journal(service)
	for kmod in self.kernel_mods:
	if self.is_module_loaded(kmod):
	self.add_cmd_output(f"modinfo {kmod}")

[maas] improve/refactor MAAS plugin #3687

[maas] improve/refactor MAAS plugin #3687

Conversation

troyanov commented Jun 25, 2024 • edited Loading

packit-as-a-service bot commented Jun 25, 2024

arif-ali left a comment

Choose a reason for hiding this comment

arif-ali left a comment

Choose a reason for hiding this comment

pmoravec Jun 28, 2024

Choose a reason for hiding this comment

troyanov Jun 28, 2024

Choose a reason for hiding this comment

troyanov Jun 28, 2024

Choose a reason for hiding this comment

troyanov Jun 28, 2024

Choose a reason for hiding this comment

arif-ali Jun 28, 2024

Choose a reason for hiding this comment

troyanov commented Jun 28, 2024

arif-ali left a comment

Choose a reason for hiding this comment

TurboTurtle left a comment

Choose a reason for hiding this comment

TurboTurtle Jun 28, 2024

Choose a reason for hiding this comment

troyanov Jun 28, 2024

Choose a reason for hiding this comment

troyanov commented Jun 25, 2024 •

edited

Loading