From 8b2bf09c4934deb7af07add343cc32ff69792d49 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Mon, 30 Oct 2017 01:03:54 +0100 Subject: [PATCH] service: PEP8 compliancy and doc fixes (#30890) This PR includes: - PEP8 compliancy fixes - Documentation fixes --- lib/ansible/modules/system/service.py | 190 +++++++++++--------------- test/sanity/pep8/legacy-files.txt | 1 - 2 files changed, 83 insertions(+), 108 deletions(-) diff --git a/lib/ansible/modules/system/service.py b/lib/ansible/modules/system/service.py index e0ac781616..ffbd34ddf0 100644 --- a/lib/ansible/modules/system/service.py +++ b/lib/ansible/modules/system/service.py @@ -1,38 +1,34 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# (c) 2012, Michael DeHaan +# Copyright: (c) 2012, Michael DeHaan # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type - ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['stableinterface'], 'supported_by': 'core'} - DOCUMENTATION = ''' --- module: service author: - - "Ansible Core Team" - - "Michael DeHaan" + - Ansible Core Team + - Michael DeHaan version_added: "0.1" -short_description: Manage services. +short_description: Manage services description: - Controls services on remote hosts. Supported init systems include BSD init, OpenRC, SysV, Solaris SMF, systemd, upstart. - For Windows targets, use the M(win_service) module instead. options: name: - required: true description: - Name of the service. + required: true state: - required: false - choices: [ started, stopped, restarted, reloaded ] description: - C(started)/C(stopped) are idempotent actions that will not run commands unless necessary. C(restarted) will always bounce the @@ -40,86 +36,81 @@ options: and enabled are required.) Note that reloaded will start the service if it is not already started, even if your chosen init system wouldn't normally. + choices: [ reloaded, restarted, running, started, stopped ] sleep: - required: false - version_added: "1.3" description: - If the service is being C(restarted) then sleep this many seconds between the stop and start command. This helps to workaround badly behaving init scripts that exit immediately after signaling a process to stop. + version_added: "1.3" pattern: - required: false - version_added: "0.7" description: - If the service does not respond to the status command, name a substring to look for as would be found in the output of the I(ps) command as a stand-in for a status result. If the string is found, the service will be assumed to be running. + version_added: "0.7" enabled: - required: false - choices: [ "yes", "no" ] description: - Whether the service should start on boot. B(At least one of state and enabled are required.) - + type: bool runlevel: - required: false - default: 'default' description: - "For OpenRC init scripts (ex: Gentoo) only. The runlevel that this service belongs to." + default: default arguments: description: - Additional arguments provided on the command line - aliases: [ 'args' ] + aliases: [ args ] use: description: - The service module actually uses system specific modules, normally through auto detection, this setting can force a specific module. - Normally it uses the value of the 'ansible_service_mgr' fact and falls back to the old 'service' module when none matching is found. - default: 'auto' + default: auto version_added: 2.2 notes: - For Windows targets, use the M(win_service) module instead. ''' EXAMPLES = ''' -# Example action to start service httpd, if not running -- service: +- name: Start service httpd, if not running + service: name: httpd state: started -# Example action to stop service httpd, if running -- service: +- name: Stop service httpd, if running + service: name: httpd state: stopped -# Example action to restart service httpd, in all cases -- service: +- name: Restart service httpd, in all cases + service: name: httpd state: restarted -# Example action to reload service httpd, in all cases -- service: +- name: Reload service httpd, in all cases + service: name: httpd state: reloaded -# Example action to enable service httpd, and not touch the running state -- service: +- name: Enable service httpd, and not touch the running state + service: name: httpd enabled: yes -# Example action to start service foo, based on running process /usr/bin/foo -- service: +- name: Start service foo, based on running process /usr/bin/foo + service: name: foo pattern: /usr/bin/foo state: started -# Example action to restart network service for interface eth0 -- service: +- name: Restart network service for interface eth0 + service: name: network state: restarted args: eth0 - ''' import glob @@ -172,26 +163,26 @@ class Service(object): return load_platform_subclass(Service, args, kwargs) def __init__(self, module): - self.module = module - self.name = module.params['name'] - self.state = module.params['state'] - self.sleep = module.params['sleep'] - self.pattern = module.params['pattern'] - self.enable = module.params['enabled'] - self.runlevel = module.params['runlevel'] - self.changed = False - self.running = None - self.crashed = None - self.action = None - self.svc_cmd = None + self.module = module + self.name = module.params['name'] + self.state = module.params['state'] + self.sleep = module.params['sleep'] + self.pattern = module.params['pattern'] + self.enable = module.params['enabled'] + self.runlevel = module.params['runlevel'] + self.changed = False + self.running = None + self.crashed = None + self.action = None + self.svc_cmd = None self.svc_initscript = None - self.svc_initctl = None - self.enable_cmd = None - self.arguments = module.params.get('arguments', '') - self.rcconf_file = None - self.rcconf_key = None - self.rcconf_value = None - self.svc_change = False + self.svc_initctl = None + self.enable_cmd = None + self.arguments = module.params.get('arguments', '') + self.rcconf_file = None + self.rcconf_key = None + self.rcconf_value = None + self.svc_change = False # =========================================== # Platform specific methods (must be replaced by subclass). @@ -314,7 +305,7 @@ class Service(object): self.running = False lines = psout.split("\n") for line in lines: - if self.pattern in line and not "pattern=" in line: + if self.pattern in line and "pattern=" not in line: # so as to not confuse ./hacking/test-module self.running = True break @@ -323,9 +314,9 @@ class Service(object): if self.state and self.running is None: self.module.fail_json(msg="failed determining service state, possible typo of service name?") # Find out if state has changed - if not self.running and self.state in ["started", "running", "reloaded"]: + if not self.running and self.state in ["reloaded", "running", "started"]: self.svc_change = True - elif self.running and self.state in ["stopped","reloaded"]: + elif self.running and self.state in ["reloaded", "stopped"]: self.svc_change = True elif self.state == "restarted": self.svc_change = True @@ -337,7 +328,7 @@ class Service(object): # Only do something if state will change if self.svc_change: # Control service - if self.state in ['started', 'running']: + if self.state in ['running', 'started']: self.action = "start" elif not self.running and self.state == 'reloaded': self.action = "start" @@ -417,8 +408,6 @@ class Service(object): # Replace previous rc.conf. self.module.atomic_move(tmp_rcconf_file, self.rcconf_file) -# =========================================== -# Subclass: Linux class LinuxService(Service): """ @@ -432,23 +421,23 @@ class LinuxService(Service): def get_service_tools(self): - paths = [ '/sbin', '/usr/sbin', '/bin', '/usr/bin' ] - binaries = [ 'service', 'chkconfig', 'update-rc.d', 'rc-service', 'rc-update', 'initctl', 'systemctl', 'start', 'stop', 'restart', 'insserv' ] - initpaths = [ '/etc/init.d' ] + paths = ['/sbin', '/usr/sbin', '/bin', '/usr/bin'] + binaries = ['service', 'chkconfig', 'update-rc.d', 'rc-service', 'rc-update', 'initctl', 'systemctl', 'start', 'stop', 'restart', 'insserv'] + initpaths = ['/etc/init.d'] location = dict() for binary in binaries: location[binary] = self.module.get_bin_path(binary, opt_dirs=paths) for initdir in initpaths: - initscript = "%s/%s" % (initdir,self.name) + initscript = "%s/%s" % (initdir, self.name) if os.path.isfile(initscript): self.svc_initscript = initscript def check_systemd(): # tools must be installed - if location.get('systemctl',False): + if location.get('systemctl', False): # this should show if systemd is the boot init system # these mirror systemd's own sd_boot test http://www.freedesktop.org/software/systemd/man/sd_booted.html @@ -483,7 +472,7 @@ class LinuxService(Service): self.upstart_version = LooseVersion('0.0.0') try: version_re = re.compile(r'\(upstart (.*)\)') - rc,stdout,stderr = self.module.run_command('%s version' % location['initctl']) + rc, stdout, stderr = self.module.run_command('%s version' % location['initctl']) if rc == 0: res = version_re.search(stdout) if res: @@ -497,7 +486,7 @@ class LinuxService(Service): # service is managed by OpenRC self.svc_cmd = location['rc-service'] self.enable_cmd = location['rc-update'] - return # already have service start/stop tool too! + return # already have service start/stop tool too! elif self.svc_initscript: # service is managed by with SysV init scripts @@ -612,7 +601,7 @@ class LinuxService(Service): # if we have decided the service is managed by upstart, we check for some additional output... if self.svc_initctl and self.running is None: # check the job status by upstart response - initctl_rc, initctl_status_stdout, initctl_status_stderr = self.execute_command("%s status %s %s" % (self.svc_initctl, self.name, self.arguments )) + initctl_rc, initctl_status_stdout, initctl_status_stderr = self.execute_command("%s status %s %s" % (self.svc_initctl, self.name, self.arguments)) if "stop/waiting" in initctl_status_stdout: self.running = False elif "start/running" in initctl_status_stdout: @@ -664,7 +653,6 @@ class LinuxService(Service): return self.running - def service_enable(self): if self.enable_cmd is None: @@ -746,10 +734,10 @@ class LinuxService(Service): if 'chkconfig --add %s' % self.name in err: self.execute_command("%s --add %s" % (self.enable_cmd, self.name)) (rc, out, err) = self.execute_command("%s --list %s" % (self.enable_cmd, self.name)) - if not self.name in out: + if self.name not in out: self.module.fail_json(msg="service %s does not support chkconfig" % self.name) - #TODO: look back on why this is here - #state = out.split()[-1] + # TODO: look back on why this is here + # state = out.split()[-1] # Check if we're already in the correct state if "3:%s" % action in out and "5:%s" % action in out: @@ -822,7 +810,7 @@ class LinuxService(Service): klinks = glob.glob('/etc/rc?.d/K??' + self.name) if not klinks: if not self.module.check_mode: - (rc, out, err) = self.execute_command("%s %s defaults" % (self.enable_cmd, self.name)) + (rc, out, err) = self.execute_command("%s %s defaults" % (self.enable_cmd, self.name)) if rc != 0: if err: self.module.fail_json(msg=err) @@ -832,7 +820,7 @@ class LinuxService(Service): action = 'disable' if not self.module.check_mode: - (rc, out, err) = self.execute_command("%s %s %s" % (self.enable_cmd, self.name, action)) + (rc, out, err) = self.execute_command("%s %s %s" % (self.enable_cmd, self.name, action)) if rc != 0: if err: self.module.fail_json(msg=err) @@ -905,7 +893,6 @@ class LinuxService(Service): return (rc, out, err) - def service_control(self): # Decide what command to run @@ -974,8 +961,6 @@ class LinuxService(Service): return(rc_state, stdout, stderr) -# =========================================== -# Subclass: FreeBSD class FreeBsdService(Service): """ @@ -1010,7 +995,7 @@ class FreeBsdService(Service): else: self.rcconf_value = "NO" - rcfiles = [ '/etc/rc.conf','/etc/rc.conf.local', '/usr/local/etc/rc.conf' ] + rcfiles = ['/etc/rc.conf', '/etc/rc.conf.local', '/usr/local/etc/rc.conf'] for rcfile in rcfiles: if os.path.isfile(rcfile): self.rcconf_file = rcfile @@ -1019,7 +1004,7 @@ class FreeBsdService(Service): try: rcvars = shlex.split(stdout, comments=True) except: - #TODO: add a warning to the output with the failure + # TODO: add a warning to the output with the failure pass if not rcvars: @@ -1038,7 +1023,7 @@ class FreeBsdService(Service): if self.rcconf_key is None: self.module.fail_json(msg="unable to determine rcvar", stdout=stdout, stderr=stderr) - if self.sysrc_cmd: # FreeBSD >= 9.2 + if self.sysrc_cmd: # FreeBSD >= 9.2 rc, current_rcconf_value, stderr = self.execute_command("%s -n %s" % (self.sysrc_cmd, self.rcconf_key)) # it can happen that rcvar is not set (case of a system coming from the ports collection) @@ -1053,25 +1038,24 @@ class FreeBsdService(Service): if self.module.check_mode: self.module.exit_json(changed=True, msg="changing service enablement") - rc, change_stdout, change_stderr = self.execute_command("%s %s=\"%s\"" % (self.sysrc_cmd, self.rcconf_key, self.rcconf_value ) ) + rc, change_stdout, change_stderr = self.execute_command("%s %s=\"%s\"" % (self.sysrc_cmd, self.rcconf_key, self.rcconf_value)) if rc != 0: self.module.fail_json(msg="unable to set rcvar using sysrc", stdout=change_stdout, stderr=change_stderr) # sysrc does not exit with code 1 on permission error => validate successful change using service(8) rc, check_stdout, check_stderr = self.execute_command("%s %s %s" % (self.svc_cmd, self.name, "enabled")) - if self.enable != (rc == 0): # rc = 0 indicates enabled service, rc = 1 indicates disabled service + if self.enable != (rc == 0): # rc = 0 indicates enabled service, rc = 1 indicates disabled service self.module.fail_json(msg="unable to set rcvar: sysrc did not change value", stdout=change_stdout, stderr=change_stderr) else: self.changed = False - else: # Legacy (FreeBSD < 9.2) + else: # Legacy (FreeBSD < 9.2) try: return self.service_enable_rcconf() except Exception: self.module.fail_json(msg='unable to set rcvar') - def service_control(self): if self.action == "start": @@ -1088,8 +1072,6 @@ class FreeBsdService(Service): return ret -# =========================================== -# Subclass: OpenBSD class OpenBsdService(Service): """ @@ -1239,8 +1221,6 @@ class OpenBsdService(Service): self.changed = True -# =========================================== -# Subclass: NetBSD class NetBsdService(Service): """ @@ -1255,10 +1235,10 @@ class NetBsdService(Service): distribution = None def get_service_tools(self): - initpaths = [ '/etc/rc.d' ] # better: $rc_directories - how to get in here? Run: sh -c '. /etc/rc.conf ; echo $rc_directories' + initpaths = ['/etc/rc.d'] # better: $rc_directories - how to get in here? Run: sh -c '. /etc/rc.conf ; echo $rc_directories' for initdir in initpaths: - initscript = "%s/%s" % (initdir,self.name) + initscript = "%s/%s" % (initdir, self.name) if os.path.isfile(initscript): self.svc_initscript = initscript @@ -1271,12 +1251,12 @@ class NetBsdService(Service): else: self.rcconf_value = "NO" - rcfiles = [ '/etc/rc.conf' ] # Overkill? + rcfiles = ['/etc/rc.conf'] # Overkill? for rcfile in rcfiles: if os.path.isfile(rcfile): self.rcconf_file = rcfile - self.rcconf_key = "%s" % string.replace(self.name,"-","_") + self.rcconf_key = "%s" % string.replace(self.name, "-", "_") return self.service_enable_rcconf() @@ -1297,8 +1277,7 @@ class NetBsdService(Service): self.svc_cmd = "%s" % self.svc_initscript return self.execute_command("%s %s" % (self.svc_cmd, self.action), daemonize=True) -# =========================================== -# Subclass: SunOS + class SunOSService(Service): """ This is the SunOS Service manipulation class - it uses the svcadm @@ -1406,7 +1385,6 @@ class SunOSService(Service): self.changed = True - def service_control(self): status = self.get_sunos_svcs_status() @@ -1433,8 +1411,6 @@ class SunOSService(Service): return self.execute_command("%s %s %s" % (self.svcadm_cmd, subcmd, self.name)) -# =========================================== -# Subclass: AIX class AIX(Service): """ @@ -1468,7 +1444,6 @@ class AIX(Service): if not self.refresh_cmd: self.module.fail_json(msg='unable to find refresh binary') - def get_service_status(self): status = self.get_aix_src_status() # Only 'active' is considered properly running. Everything else is off @@ -1513,14 +1488,14 @@ class AIX(Service): def main(): module = AnsibleModule( - argument_spec = dict( - name = dict(required=True), - state = dict(choices=['running', 'started', 'stopped', 'restarted', 'reloaded']), - sleep = dict(required=False, type='int', default=None), - pattern = dict(required=False, default=None), - enabled = dict(type='bool'), - runlevel = dict(required=False, default='default'), - arguments = dict(aliases=['args'], default=''), + argument_spec=dict( + name=dict(type='str', required=True), + state=dict(type='str', choices=['running', 'started', 'stopped', 'reloaded', 'restarted']), + sleep=dict(type='int'), + pattern=dict(type='str'), + enabled=dict(type='bool'), + runlevel=dict(type='str', default='default'), + arguments=dict(type='str', default='', aliases=['args']), ), supports_check_mode=True, required_one_of=[['state', 'enabled']], @@ -1594,12 +1569,13 @@ def main(): else: # as we may have just bounced the service the service command may not # report accurate state at this moment so just show what we ran - if service.module.params['state'] in ['started','restarted','running','reloaded']: + if service.module.params['state'] in ['reloaded', 'restarted', 'running', 'started']: result['state'] = 'started' else: result['state'] = 'stopped' module.exit_json(**result) + if __name__ == '__main__': main() diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 5c80781876..ae2fc08cee 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -358,7 +358,6 @@ lib/ansible/modules/system/pam_limits.py lib/ansible/modules/system/puppet.py lib/ansible/modules/system/runit.py lib/ansible/modules/system/seport.py -lib/ansible/modules/system/service.py lib/ansible/modules/system/solaris_zone.py lib/ansible/modules/system/svc.py lib/ansible/modules/system/systemd.py