Page MenuHomePhabricator

cookbook: tracebacks instead of showing proper error message when running without proper perms
Open, LowestPublic

Description

aborrero@cumin2001:~ $ cookbook sre.hosts.reboot-single -h
Traceback (most recent call last):
  File "/usr/bin/cookbook", line 11, in <module>
    load_entry_point('wikimedia-spicerack==0.0.48', 'console_scripts', 'cookbook')()
  File "/usr/lib/python3/dist-packages/spicerack/_cookbook.py", line 420, in main
    return execute_cookbook(config, args, cookbooks)
  File "/usr/lib/python3/dist-packages/spicerack/_cookbook.py", line 385, in execute_cookbook
    host=config.get('tcpircbot_host', None), port=int(config.get('tcpircbot_port', 0)))
  File "/usr/lib/python3/dist-packages/spicerack/_log.py", line 59, in setup_logging
    handler = logging.FileHandler(os.path.join(base_path, '{name}.log'.format(name=name)))
  File "/usr/lib/python3.7/logging/__init__.py", line 1092, in __init__
    StreamHandler.__init__(self, self._open())
  File "/usr/lib/python3.7/logging/__init__.py", line 1121, in _open
    return open(self.baseFilename, self.mode, encoding=self.encoding)
PermissionError: [Errno 13] Permission denied: '/var/log/spicerack/sre/hosts/reboot-single.log'
aborrero@cumin2001:~ $ sudo cookbook sre.hosts.reboot-single -h
usage: cookbook [-h] [--depool] host

Downtime a single host and reboot it

    - Set Icinga downtime
    - Reboot
    - Wait for host to come back online
    - Remove the Icinga downtime after the host has been rebooted and the
      first Puppet run is complete

    This is meant for one off servers and doesn't support pooling/depooling
    clustered services (yet).

    Usage example:
        cookbook sre.hosts.reboot-single sretest1001.eqiad.wmnet

    

positional arguments:
  host        A single host to be rebooted (specified in Cumin query syntax)

optional arguments:
  -h, --help  show this help message and exit
  --depool    Whether to run depool/pool on the server around reboots.

Event Timeline

@aborrero
With the current efforts in T244840, there is no such concept of "proper" permissions, as spicerack will most likely be available also for non-root in the near future. So adding a message that says it requires sudo is pointless as it will need to be removed soon.
The only option I see is to put a message that says the setup logging phase failed, but I fail to see how that could help the user more than this very explicit one. Thoughts?
As for the traceback, having them has been proved to be much more useful than harmful in most cases.

I don't know the codebase, but perhaps showing the help message before configuring the log is an "easy" fix for this particular case.

No, because the logging is set by the framework before executing the cookbook. Asking for the help message it's just a particular option but is still executing the cookbook, so the logging must be already setup by then.

Again, I'm not familiar with the codebase, but what about simple try: when calling the cookbook backend, catch that particular error and print something more friendly.

What would be a more friendly message in this case?
What is not "friendly" or confusing in the current one?

PermissionError: [Errno 13] Permission denied: '/var/log/spicerack/sre/hosts/reboot-single.log'

My expectation would be something like:

aborrero@cumin2001:~ $ cookbook sre.hosts.reboot-single -h
ERROR: you don't have the required permissions to run this cookbook

aborrero@cumin2001:~ $ cookbook sre.hosts.reboot-single -h
FATAL: current user doesn't have proper groups/perms to run this

aborrero@cumin2001:~ $ cookbook sre.hosts.reboot-single -h
E: this cookbook requires root or be part of group XYZ

aborrero@cumin2001:~ $ cookbook sre.hosts.reboot-single -h
try as root!

What doesn't feel friendly about the current situation is:

  • requesting for help fails with a traceback. You explained above why that happens, but honestly this isn't friendly.
  • did I get the traceback because I did something wrong or because the software did something wrong?
  • I don't know what that file is or why it is important or what the right permissions should be or why should I care
  • to understand what's happening in the traceback I would need to read the source code of spicerack/cookbook and I bet users wouldn't want to.
aborrero triaged this task as Lowest priority.Feb 23 2021, 10:53 AM

BTW, this is just a cosmetic thing, so I'm changing priority to 'lowest' :-)