From 1b973907658374d146f7a4126a6b9979171db326 Mon Sep 17 00:00:00 2001 From: Jonathan Armani Date: Wed, 27 Aug 2014 22:26:47 +0200 Subject: [PATCH 01/10] Add enable / disable of services for OpenBSD if rcctl is present --- system/service.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/system/service.py b/system/service.py index b235ee25c57..d708a848bd3 100644 --- a/system/service.py +++ b/system/service.py @@ -945,9 +945,7 @@ class FreeBsdService(Service): class OpenBsdService(Service): """ This is the OpenBSD Service manipulation class - it uses /etc/rc.d for - service control. Enabling a service is currently not supported because the - _flags variable is not boolean, you should supply a rc.conf.local - file in some other way. + service control. Enabling a service is currently supported if rcctl is present """ platform = 'OpenBSD' @@ -963,6 +961,8 @@ class OpenBsdService(Service): if not self.svc_cmd: self.module.fail_json(msg='unable to find rc.d script') + self.enable_cmd = self.module.get_bin_path('rcctl') + def get_service_status(self): rc, stdout, stderr = self.execute_command("%s %s" % (self.svc_cmd, 'check')) if rc == 1: @@ -971,7 +971,27 @@ class OpenBsdService(Service): self.running = True def service_control(self): - return self.execute_command("%s %s" % (self.svc_cmd, self.action)) + return self.execute_command("%s -f %s" % (self.svc_cmd, self.action)) + + def service_enable(self): + if not self.enable_cmd: + return super(OpenBsdService, self).service_enable() + + rc, stdout, stderr = self.execute_command("%s %s %s" % (self.enable_cmd, 'status', self.name)) + + if self.enable: + action = "enable %s flags %s" % (self.name, self.arguments) + args = self.arguments + if rc == 0: + return + else: + action = "disable %s" % self.name + if rc == 1: + return + + # XXX check rc ? + rc, stdout, stderr = self.execute_command("%s %s" % (self.enable_cmd, action)) + self.changed = True # =========================================== # Subclass: NetBSD From c46e030100dbde9f8a8d71263a3a9a11b45880df Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Thu, 28 Aug 2014 14:44:43 +0200 Subject: [PATCH 02/10] Make "enabled" code aware of --check mode. --- system/service.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/system/service.py b/system/service.py index d708a848bd3..e17eb610872 100644 --- a/system/service.py +++ b/system/service.py @@ -989,6 +989,9 @@ class OpenBsdService(Service): if rc == 1: return + if self.module.check_mode: + self.module.exit_json(changed=True, msg="changing service enablement") + # XXX check rc ? rc, stdout, stderr = self.execute_command("%s %s" % (self.enable_cmd, action)) self.changed = True From 1a8cdb5e3e68b710cec1c0b2d7d94f48d7f1586b Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Thu, 28 Aug 2014 16:33:39 +0200 Subject: [PATCH 03/10] Check rc and print error message if any. It is probably good to use stdout before printing a generic error message as well. --- system/service.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/system/service.py b/system/service.py index e17eb610872..238612ea30a 100644 --- a/system/service.py +++ b/system/service.py @@ -992,8 +992,16 @@ class OpenBsdService(Service): if self.module.check_mode: self.module.exit_json(changed=True, msg="changing service enablement") - # XXX check rc ? rc, stdout, stderr = self.execute_command("%s %s" % (self.enable_cmd, action)) + + if rc != 0: + if stderr: + self.module.fail_json(msg=stderr) + elif stdout: + self.module.fail_json(msg=stdout) + else: + self.module.fail_json(msg="rcctl failed to modify service enablement") + self.changed = True # =========================================== From c6dd88c1d162e1a16749577e5d4df9693583389e Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Thu, 28 Aug 2014 16:50:37 +0200 Subject: [PATCH 04/10] Fail if "rcctl status" writes to stderr. --- system/service.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/system/service.py b/system/service.py index 238612ea30a..75450aa4c13 100644 --- a/system/service.py +++ b/system/service.py @@ -987,6 +987,8 @@ class OpenBsdService(Service): else: action = "disable %s" % self.name if rc == 1: + if stderr: + self.module.fail_json(msg=stderr) return if self.module.check_mode: From 5f37624eb4debca831a3b44a39b2b3b96f1872aa Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Sun, 31 Aug 2014 11:58:37 +0200 Subject: [PATCH 05/10] Tweak error checking for "enabled" code. Based on input from @jarmani: * A return value of 2 now means a service does not exist. Instead of trying to handle the different meanings of rc after running "status", just look at stderr to know if something failed. * Skip looking at stdout to make the code cleaner. Any errors should turn up on stderr. --- system/service.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/system/service.py b/system/service.py index 75450aa4c13..b5c6da9dee8 100644 --- a/system/service.py +++ b/system/service.py @@ -979,6 +979,9 @@ class OpenBsdService(Service): rc, stdout, stderr = self.execute_command("%s %s %s" % (self.enable_cmd, 'status', self.name)) + if stderr: + self.module.fail_json(msg=stderr) + if self.enable: action = "enable %s flags %s" % (self.name, self.arguments) args = self.arguments @@ -987,8 +990,6 @@ class OpenBsdService(Service): else: action = "disable %s" % self.name if rc == 1: - if stderr: - self.module.fail_json(msg=stderr) return if self.module.check_mode: @@ -999,8 +1000,6 @@ class OpenBsdService(Service): if rc != 0: if stderr: self.module.fail_json(msg=stderr) - elif stdout: - self.module.fail_json(msg=stdout) else: self.module.fail_json(msg="rcctl failed to modify service enablement") From 924cf20cf8ad955af8a187cbdbe508852eac0d3c Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Sat, 6 Sep 2014 19:48:14 +0200 Subject: [PATCH 06/10] Depend more on rcctl if it is present. * Make the module support enable/disable of special services like pf via rcctl. Idea and method from @jarmani. * Make the module handle when the user supplied 'arguments' variable does not match the current flags in rc.conf.local. * Update description now that the code tries to use rcctl for everything if it is available. --- system/service.py | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/system/service.py b/system/service.py index b5c6da9dee8..ac266087e85 100644 --- a/system/service.py +++ b/system/service.py @@ -944,34 +944,48 @@ class FreeBsdService(Service): class OpenBsdService(Service): """ - This is the OpenBSD Service manipulation class - it uses /etc/rc.d for - service control. Enabling a service is currently supported if rcctl is present + This is the OpenBSD Service manipulation class - it uses rcctl(8) or + /etc/rc.d scripts for service control. Enabling a service is + only supported if rcctl is present. """ platform = 'OpenBSD' distribution = None def get_service_tools(self): - rcdir = '/etc/rc.d' + self.enable_cmd = self.module.get_bin_path('rcctl') - rc_script = "%s/%s" % (rcdir, self.name) - if os.path.isfile(rc_script): - self.svc_cmd = rc_script + if self.enable_cmd: + self.svc_cmd = self.enable_cmd + else: + rcdir = '/etc/rc.d' - if not self.svc_cmd: - self.module.fail_json(msg='unable to find rc.d script') + rc_script = "%s/%s" % (rcdir, self.name) + if os.path.isfile(rc_script): + self.svc_cmd = rc_script - self.enable_cmd = self.module.get_bin_path('rcctl') + if not self.svc_cmd: + self.module.fail_json(msg='unable to find svc_cmd') def get_service_status(self): - rc, stdout, stderr = self.execute_command("%s %s" % (self.svc_cmd, 'check')) + if self.enable_cmd: + rc, stdout, stderr = self.execute_command("%s %s %s" % (self.svc_cmd, 'check', self.name)) + else: + rc, stdout, stderr = self.execute_command("%s %s" % (self.svc_cmd, 'check')) + + if stderr: + self.module.fail_json(msg=stderr) + if rc == 1: self.running = False elif rc == 0: self.running = True def service_control(self): - return self.execute_command("%s -f %s" % (self.svc_cmd, self.action)) + if self.enable_cmd: + return self.execute_command("%s -f %s %s" % (self.svc_cmd, self.action, self.name)) + else: + return self.execute_command("%s -f %s" % (self.svc_cmd, self.action)) def service_enable(self): if not self.enable_cmd: @@ -982,10 +996,13 @@ class OpenBsdService(Service): if stderr: self.module.fail_json(msg=stderr) + current_flags = stdout.rstrip() + if self.enable: - action = "enable %s flags %s" % (self.name, self.arguments) - args = self.arguments - if rc == 0: + action = "enable %s" % (self.name) + if self.arguments or self.arguments != current_flags: + action = action + " flags %s" % (self.arguments) + if rc == 0 and self.arguments == current_flags: return else: action = "disable %s" % self.name From e463400412d30b0c26c474af625f799241a7c4cf Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Tue, 9 Sep 2014 16:12:47 +0200 Subject: [PATCH 07/10] Simplify self.arguments logic. Strange logic pointed out by @jarmani, thanks! --- system/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/service.py b/system/service.py index ac266087e85..484f007cac9 100644 --- a/system/service.py +++ b/system/service.py @@ -1000,7 +1000,7 @@ class OpenBsdService(Service): if self.enable: action = "enable %s" % (self.name) - if self.arguments or self.arguments != current_flags: + if self.arguments or current_flags: action = action + " flags %s" % (self.arguments) if rc == 0 and self.arguments == current_flags: return From f9d9c1b6d7d8326142ec1c4e6db46fb4e9e808ab Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Sun, 12 Oct 2014 18:32:41 +0200 Subject: [PATCH 08/10] Multiple fixes for OpenBSD rcctl handling. * Use the newly added 'default' argument to know if the default flags are set or not. * Handle that 'status' may either return flags or YES/NO. * Centralize flag handling logic. * Set action variable after check if we need to keep going. Big thanks to @ajacoutot for implementing the rcctl 'default' argument. --- system/service.py | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/system/service.py b/system/service.py index 484f007cac9..e8708682eb8 100644 --- a/system/service.py +++ b/system/service.py @@ -991,24 +991,52 @@ class OpenBsdService(Service): if not self.enable_cmd: return super(OpenBsdService, self).service_enable() + rc, stdout, stderr = self.execute_command("%s %s %s" % (self.enable_cmd, 'default', self.name)) + + if stderr: + self.module.fail_json(msg=stderr) + + default_flags = stdout.rstrip() + rc, stdout, stderr = self.execute_command("%s %s %s" % (self.enable_cmd, 'status', self.name)) if stderr: self.module.fail_json(msg=stderr) - current_flags = stdout.rstrip() + status_string = stdout.rstrip() + + # Depending on the service the string returned from 'status' may be + # either a set of flags or the boolean YES/NO + if status_string == "YES" or status_string == "N0": + current_flags = '' + else: + current_flags = status_string + + # If there are arguments from the user we use these as flags unless + # they are already set. + if self.arguments and self.arguments != current_flags: + changed_flags = self.arguments + # If the user has not supplied any arguments and the current flags + # differ from the default we reset them. + elif not self.arguments and current_flags != default_flags: + changed_flags = ' ' + # Otherwise there is no need to modify flags. + else: + changed_flags = '' if self.enable: - action = "enable %s" % (self.name) - if self.arguments or current_flags: - action = action + " flags %s" % (self.arguments) - if rc == 0 and self.arguments == current_flags: + if rc == 0 and not changed_flags: return + + action = "enable %s" % (self.name) + if changed_flags: + action = action + " flags %s" % (changed_flags) else: - action = "disable %s" % self.name if rc == 1: return + action = "disable %s" % self.name + if self.module.check_mode: self.module.exit_json(changed=True, msg="changing service enablement") From eea4d068482a9d056e84a259ca712cea8ea69302 Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Thu, 13 Nov 2014 12:39:29 +0100 Subject: [PATCH 09/10] Fix typo: Replace "N0" with "NO". --- system/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/service.py b/system/service.py index e8708682eb8..2d62edc67ac 100644 --- a/system/service.py +++ b/system/service.py @@ -1007,7 +1007,7 @@ class OpenBsdService(Service): # Depending on the service the string returned from 'status' may be # either a set of flags or the boolean YES/NO - if status_string == "YES" or status_string == "N0": + if status_string == "YES" or status_string == "NO": current_flags = '' else: current_flags = status_string From 2acfbf016d8626df445839caf522debc393f0d31 Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Thu, 11 Dec 2014 23:01:23 +0100 Subject: [PATCH 10/10] Handle string returned by 'default' correctly. We need to handle the string returned by 'default' in the same way we handle the string returned by 'status' since the resulting flags are compared later. --- system/service.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/system/service.py b/system/service.py index 2d62edc67ac..c9ce55d1a37 100644 --- a/system/service.py +++ b/system/service.py @@ -996,7 +996,14 @@ class OpenBsdService(Service): if stderr: self.module.fail_json(msg=stderr) - default_flags = stdout.rstrip() + default_string = stdout.rstrip() + + # Depending on the service the string returned from 'default' may be + # either a set of flags or the boolean YES/NO + if default_string == "YES" or default_string == "NO": + default_flags = '' + else: + default_flags = default_string rc, stdout, stderr = self.execute_command("%s %s %s" % (self.enable_cmd, 'status', self.name))