Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[juju] Add plugin option for Juju state reporting #3803

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[juju] Add plugin option for Juju state reporting #3803

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 97 additions & 2 deletions sos/report/plugins/juju.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,49 @@
#
# See the LICENSE file in the source distribution for further information.

from sos.report.plugins import Plugin, UbuntuPlugin
import pwd
import json
from sos.report.plugins import Plugin, UbuntuPlugin, PluginOpt


class Juju(Plugin, UbuntuPlugin):

short_desc = 'Juju orchestration tool'

plugin_name = 'juju'
profiles = ('virt', 'sysmgmt')
profiles = ('virt', 'sysmgmt',)

# Using files instead of packages here because there is no identifying
# package on a juju machine.
files = ('/var/log/juju',)

option_list = [
PluginOpt(
"juju-state",
default=False,
val_type=bool,
desc="Include Juju state in the report",
),
PluginOpt(
"juju-user",
default="ubuntu",
val_type=str,
desc="Juju client user",
),
PluginOpt(
"controllers",
default="",
val_type=str,
desc="collect for specified Juju controllers",
),
PluginOpt(
"models",
default="",
val_type=str,
desc="collect for specified Juju models",
),
]

def setup(self):
# Juju service names are not consistent through deployments,
# so we need to use a wildcard to get the correct service names.
Expand Down Expand Up @@ -53,6 +82,71 @@ def setup(self):
# logs in the directory.
self.add_copy_spec("/var/log/juju/*.log")

# Only include the Juju state report if this plugin option is set
if not self.get_option("juju-state"):
return

juju_user = self.get_option("juju-user")
try:
pwd.getpwnam(juju_user)
except KeyError:
self._log_warn(
f'User "{juju_user}" does not exist, '
"will not collect Juju information."
)
return

if self.get_option("controllers") and self.get_option("models"):
self._log_warn(
"Options: controllers, models are mutually exclusive. "
"Will not collect Juju information."
)
return
Comment on lines +99 to +104
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming these are mutually exclusive within the Juju stack, but would we not anticipate some (admittedly niche) use case where an sos end user isn't sure if a particular host is using one or the other, and wants to pass e.g. -k juju.controllers=foo,juju.models=bar to get either without needing to know the specific host configuration? I'm mainly thinking of some sort of automation-driven sos collection here, but I could be way off with that idea.


controllers_json = self.collect_cmd_output(
"juju controllers --format=json", runas=juju_user
)
if controllers_json["status"] == 0:
desired_controllers = set(
self.get_option("controllers").split(" ")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally have plugins use : as a delimiter in plugin options that can take a list (as the use of commas gets murky with argparse and how plugin options are....parsed.

Copy link
Author

@MichaelThamm MichaelThamm Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only problem with this is for the Juju model synatx it can be:

  • The current controller's model -> model_a
  • Or it can be a specific controller and model -> controller_x:model_a

For this reason I chose to split via empty space since models and controllers are not allowed to have spaces in their names according to Juju.

i.e. controller one -> not valid name

Would this be a reasonable exception to the Sos plugin standard?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's an expectation that users would pass something like -k juju.controllers="foo:bar", then yeah it's totally reasonable to use something else. We should just make note of that, ideally in both the plugin option description (for sos report -l output) and the docstring for the plugin (as the docstring gets used in sos help output, e.g. sos help report.plugins.juju - see the kernel plugin docstring for a good example).

)
# If a controller option is supplied, use it. Otherwise, get all
# controllers
if desired_controllers and desired_controllers != {""}:
controllers = desired_controllers
else:
controllers = set(
json.loads(controllers_json["output"])[
"controllers"
].keys()
)
else:
controllers = {}

# Specific models
if self.get_option("models"):
for model in self.get_option("models").split(" "):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, we prefer plugin options to use : as a delimiter. We may need to add this to the plugin guide, I'll check this shortly.

command = f"juju status -m {model} --format=json"
self.collect_cmd_output(command, runas=juju_user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collect_cmd_output() is designed to return the output for further processing, if you don't need to do anything with the output, use add_cmd_output() instead.


# All controllers and all models OR specific controllers and all
# models for each
else:
for controller in controllers:
models_json = self.exec_cmd(
f"juju models --all -c {controller} --format=json",
runas=juju_user,
)
if models_json["status"] == 0:
models = json.loads(models_json["output"])["models"]
for model in models:
short_name = model["short-name"]
command = (
f"juju status -m {controller}:{short_name} "
f"--format=json"
)
self.collect_cmd_output(command, runas=juju_user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, use add_cmd_output() if you don't need to do anything further with the output.


def postproc(self):
agents_path = "/var/lib/juju/agents/*"
protect_keys = [
Expand All @@ -68,5 +162,6 @@ def postproc(self):
self.do_path_regex_sub(agents_path, keys_regex, sub_regex)
# Redact certificates
self.do_file_private_sub(agents_path)
self.do_cmd_private_sub('juju controllers')

MichaelThamm marked this conversation as resolved.
Show resolved Hide resolved
# vim: set et ts=4 sw=4 :
MichaelThamm marked this conversation as resolved.
Show resolved Hide resolved
Loading