Failure to enable a service now fails the task

Fixes: 8855

library/system/service

@@ -647,7 +647,11 @@ class LinuxService(Service):
             # committing the change is done in this conditional and then we
             # skip the boilerplate at the bottom of the method
             if self.changed:
-                write_to_override_file(override_file_name, override_state)
+                try:
+                    write_to_override_file(override_file_name, override_state)
+                except:
+                    self.module.fail_json(msg='Could not modify override file')
+
             return
 
         #
@@ -724,6 +728,9 @@ class LinuxService(Service):
             if not self.changed:
                 return
 
+        #
+        # update-rc.d style
+        #
         if self.enable_cmd.endswith("update-rc.d"):
             if self.enable:
                 action = 'enable'
@@ -736,7 +743,10 @@ class LinuxService(Service):
                 (rc, out, err) = self.execute_command("%s %s defaults" \
                                                       % (self.enable_cmd, self.name))
                 if rc != 0:
-                    return (rc, out, err)
+                    if err:
+                        self.module.fail_json(msg=err)
+                    else:
+                        self.module.fail_json(msg=out)
 
             (rc, out, err) = self.execute_command("%s -n %s %s" \
                                                   % (self.enable_cmd, self.name, action))
@@ -761,7 +771,9 @@ class LinuxService(Service):
             if not self.changed:
                 return
 
+        #
         # If we've gotten to the end, the service needs to be updated
+        #
         self.changed = True
 
         # we change argument order depending on real binary used:
@@ -777,7 +789,14 @@ class LinuxService(Service):
         if self.module.check_mode:
             self.module.exit_json(changed=self.changed)
 
-        return self.execute_command("%s %s %s" % args)
+        (rc, out, err) = self.execute_command("%s %s %s" % args)
+        if rc != 0:
+            if err:
+                self.module.fail_json(msg=err)
+            else:
+                self.module.fail_json(msg=out)
+
+        return (rc, out, err)
 
 
     def service_control(self):
@@ -900,7 +919,10 @@ class FreeBsdService(Service):
         if self.rcconf_key is None:
             self.module.fail_json(msg="unable to determine rcvar", stdout=stdout, stderr=stderr)
 
-        return self.service_enable_rcconf()
+        try:
+            return self.service_enable_rcconf()
+        except:
+            self.module.fail_json(msg='unable to set rcvar')
 
     def service_control(self):
 

test/integration/roles/test_service/files/ansible-broken.upstart

@@ -0,0 +1,10 @@
+description     "ansible test daemon"
+
+start on runlevel [345]
+stop on runlevel [!345]
+
+expect daemon
+
+exec ansible_test_service
+
+manual

test/integration/roles/test_service/tasks/main.yml

@@ -90,6 +90,16 @@
     that:
     - "disable_result.enabled == false"
 
+- name: try to enable a broken service
+  service: name=ansible_test_broken enabled=yes
+  register: broken_enable_result
+  ignore_errors: True
+
+- name: assert that the broken test failed
+  assert:
+    that:
+    - "broken_enable_result.failed == True"
+
 - name: remove the test daemon script
   file: path=/usr/sbin/ansible_test_service state=absent
   register: remove_result

test/integration/roles/test_service/tasks/systemd_cleanup.yml

@@ -2,11 +2,18 @@
   file: path=/usr/lib/systemd/system/ansible_test.service state=absent
   register: remove_systemd_result
 
+- name: remove the systemd unit file
+  file: path=/usr/lib/systemd/system/ansible_test_broken.service state=absent
+  register: remove_systemd_broken_result
+
+- debug: var=remove_systemd_broken_result
 - name: assert that the systemd unit file was removed
   assert:
     that:
     - "remove_systemd_result.path == '/usr/lib/systemd/system/ansible_test.service'"
     - "remove_systemd_result.state == 'absent'"
+    - "remove_systemd_broken_result.path == '/usr/lib/systemd/system/ansible_test_broken.service'"
+    - "remove_systemd_broken_result.state == 'absent'"
 
 - name: make sure systemd is reloaded
   shell: systemctl daemon-reload

test/integration/roles/test_service/tasks/systemd_setup.yml

@@ -2,6 +2,10 @@
   copy: src=ansible.systemd dest=/usr/lib/systemd/system/ansible_test.service
   register: install_systemd_result
 
+- name: install a broken systemd unit file
+  file: src=ansible_test.service path=/usr/lib/systemd/system/ansible_test_broken.service state=link
+  register: install_broken_systemd_result
+
 - name: assert that the systemd unit file was installed
   assert:
     that:
@@ -9,4 +13,6 @@
     - "install_systemd_result.state == 'file'"
     - "install_systemd_result.mode == '0644'"
     - "install_systemd_result.md5sum == 'f634df77d9160ab05bad4ed49d82a0d0'"
+    - "install_broken_systemd_result.dest == '/usr/lib/systemd/system/ansible_test_broken.service'"
+    - "install_broken_systemd_result.state == 'link'"
 

test/integration/roles/test_service/tasks/upstart_cleanup.yml

@@ -2,9 +2,14 @@
   file: path=/etc/init/ansible_test.conf state=absent
   register: remove_upstart_result
 
+- name: remove the upstart init file
+  file: path=/etc/init/ansible_test_broken.conf state=absent
+  register: remove_upstart_broken_result
+
 - name: assert that the upstart init file was removed
   assert:
     that:
     - "remove_upstart_result.path == '/etc/init/ansible_test.conf'"
     - "remove_upstart_result.state == 'absent'"
-
+    - "remove_upstart_broken_result.path == '/etc/init/ansible_test_broken.conf'"
+    - "remove_upstart_broken_result.state == 'absent'"

test/integration/roles/test_service/tasks/upstart_setup.yml

@@ -2,6 +2,10 @@
   copy: src=ansible.upstart dest=/etc/init/ansible_test.conf mode=0644
   register: install_upstart_result
 
+- name: install an upstart init file that will fail (manual in .conf)
+  copy: src=ansible-broken.upstart dest=/etc/init/ansible_broken_test.conf mode=0644
+  register: install_upstart_broken_result
+
 - name: assert that the upstart init file was installed
   assert:
     that:
@@ -9,4 +13,7 @@
     - "install_upstart_result.state == 'file'"
     - "install_upstart_result.mode == '0644'"
     - "install_upstart_result.md5sum == 'ab3900ea4de8423add764c12aeb90c01'"
-
+    - "install_upstart_result.dest == '/etc/init/ansible_broken_test.conf'"
+    - "install_upstart_result.state == 'file'"
+    - "install_upstart_result.mode == '0644'"
+    - "install_upstart_result.md5sum == '015e183d10c311276c3e269cbeb309b7'"