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.

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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
124 changes: 122 additions & 2 deletions sos/report/plugins/juju.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,74 @@
#
# 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):
"""The Juju plugin is aimed at collecting Juju-related logs,
configurations, and controller/model state(s).

Logs and agent configuration information (/var/log/juju and /var/lib/juju)
is collected by default since these are useful for troubleshooting.

The Juju state collection is disabled by default and can be enabled with
the 'juju-state' option. Collecting Juju state is safe in theory, but it
does act on the live controller(s)/model(s) and is therefore optional.

The default Juju state collection collects all controllers and models that
the 'juju-user' (default=ubuntu) has access to.

Specific controllers or models can be collected using the 'controllers'
and 'models' options.

Important: the string list is whitespace delimited, not colon delimited
(sos plugin standard). This is due to the underlying Juju CLI accepting
specific models in the format 'controller:model' and whitespaces are not
allowed in either controller and model names.

Example: models="controller_a:model_x controller_b:model_y"
"""

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 Juju state for specified controllers. Uses a \
whitespace delimited list.",
),
PluginOpt(
"models",
default="",
val_type=str,
desc="Collect Juju state for specified models. Uses a whitespace \
delimited list.",
),
]

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 +107,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 +124 to +129
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.

Copy link
Author

@MichaelThamm MichaelThamm Nov 18, 2024

Choose a reason for hiding this comment

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

According to the the Juju CLI:

juju status --help

-m, --model (= "")
    Model to operate in. Accepts [<controller name>:]<model name>|<model UUID>

There is no controller option elsewhere so I think it is reasonable to be opinionated and keep the 2 options mutually exclusive. As an added bonus, this simplifies the logic in the plugin.

Note: I tested this with model UUID and model name. Both work fine.


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

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).

Copy link
Author

Choose a reason for hiding this comment

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

I guess there is no reason why we couldn't use a colon delimiter for the controllers option. However, it is required for the models option (as I mentioned above) so I think it makes sense to be consistent within the scope of this plugin to keep the white-space delimiter rather than colon. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

I added a docstring and the PluginOpts have description args. Is this enough to show up in the sos report -l output? I tested this locally and I could not get my Juju plugin options to show up in this output and I have no idea why.

Reference

Copy link
Member

Choose a reason for hiding this comment

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

This is what I get when I run your changes:

# bin/sos report -l | grep juju
 juju                 Juju orchestration tool
 juju.juju-state           off             Include Juju state in the report
 juju.juju-user            ubuntu          Juju client user.
 juju.controllers                          Collect Juju state for specified controllers. Uses a             whitespace delimited list.
 juju.models                               Collect Juju state for specified models. Uses a whitespace             delimited list.

And this is what I get when running sos help:

Plugin Options
These options may be toggled or changed using '-k juju.option_name=$value'

    Option Name         Default                       Description         
    juju-state          False/Off                     Include Juju state in the report
    juju-user           ubuntu                        Juju client user.
    controllers         None/Unset                    Collect Juju state for specified controllers. Uses a
                                                      whitespace delimited list.
    models              None/Unset                    Collect Juju state for specified models. Uses a whitespace
                                                      delimited list.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding why the options are not showing up in the output locally, perhaps it's because the machine where you are running doesn't have the /var/log/juju file present? Since that's the activation condition for this plugin:

files = ('/var/log/juju',)

As far as I understand it, if it's not present then the plugin should show as inactive and the options won't be printed.

)
# 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.add_cmd_output(command, runas=juju_user)

# 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.add_cmd_output(command, runas=juju_user)

def postproc(self):
agents_path = "/var/lib/juju/agents/*"
protect_keys = [
Expand All @@ -68,5 +187,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