Remove sysctl entries when state=absent (#31486)

* Remove sysctl entry when state=absent
* Cleanup sysctl integration test syntax
* Correct grammar on error message
* Add sysctl integration test for state=absent

lib/ansible/modules/system/sysctl.py

@@ -165,6 +165,9 @@ class SysctlModule(object):
             self.write_file = True
         elif self.file_values[thisname] is None and self.args['state'] == "absent":
             self.changed = False
+        elif self.file_values[thisname] and self.args['state'] == "absent":
+            self.changed = True
+            self.write_file = True
         elif self.file_values[thisname] != self.args['value']:
             self.changed = True
             self.write_file = True
@@ -380,20 +383,20 @@ def main():
     )
 
     if module.params['name'] is None:
-        module.fail_json(msg="name can not be None")
+        module.fail_json(msg="name cannot be None")
     if module.params['state'] == 'present' and module.params['value'] is None:
-        module.fail_json(msg="value can not be None")
+        module.fail_json(msg="value cannot be None")
 
     # In case of in-line params
     if module.params['name'] == '':
-        module.fail_json(msg="name can not be blank")
+        module.fail_json(msg="name cannot be blank")
     if module.params['state'] == 'present' and module.params['value'] == '':
-        module.fail_json(msg="value can not be blank")
+        module.fail_json(msg="value cannot be blank")
 
     result = SysctlModule(module)
 
     module.exit_json(changed=result.changed)
 
-# import module snippets
+
 if __name__ == '__main__':
     main()

test/integration/targets/sysctl/files/sysctl.conf

@@ -9,3 +9,4 @@
 #
 # For more information, see sysctl.conf(5) and sysctl.d(5).
 vm.swappiness=1
+kernel.panic=2

test/integration/targets/sysctl/tasks/main.yml

@@ -16,13 +16,18 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
-- set_fact: output_dir_test={{output_dir}}/test_sysctl
+- set_fact:
+    output_dir_test: "{{ output_dir }}/test_sysctl"
 
 - name: make sure our testing sub-directory does not exist
-  file: path="{{ output_dir_test }}" state=absent
+  file:
+    path: "{{ output_dir_test }}"
+    state: absent
 
 - name: create our testing sub-directory
-  file: path="{{ output_dir_test }}" state=directory
+  file:
+    path: "{{ output_dir_test }}"
+    state: directory
 
 ##
 ## sysctl - file manipulation
@@ -30,41 +35,85 @@
 
 - name: copy the example conf to the test dir
   copy:
-      src: sysctl.conf
+    src: sysctl.conf
-      dest: "{{ output_dir_test }}"
+    dest: "{{ output_dir_test }}"
 
 - name: Set vm.swappiness to 5
   sysctl:
-      name: vm.swappiness
+    name: vm.swappiness
-      value: 5
+    value: 5
-      state: present
+    state: present
-      reload: False
+    reload: no
-      sysctl_file: "{{ output_dir_test }}/sysctl.conf"
+    sysctl_file: "{{ output_dir_test }}/sysctl.conf"
   register: sysctl_test0
-- debug: var=sysctl_test0
+
+- debug:
+    var: sysctl_test0
+    verbosity: 1
 
 - name: get file content
   shell: "cat {{ output_dir_test }}/sysctl.conf | egrep -v ^\\#"
   register: sysctl_content0
-- debug: var=sysctl_content0
+
+- debug:
+    var: sysctl_content0
+    verbosity: 1
 
 - name: Set vm.swappiness to 5 again
   sysctl:
       name: vm.swappiness
       value: 5
       state: present
-      reload: False
+      reload: no
       sysctl_file: "{{ output_dir_test }}/sysctl.conf"
   register: sysctl_test1
 
 - name: validate results
   assert:
       that:
-          - 'sysctl_test0.changed is defined'
+        - sysctl_test0 | changed
-          - 'sysctl_test1.changed is defined'
+        - not sysctl_test1 | changed
-          - 'sysctl_test0.changed'
+        - 'sysctl_content0.stdout_lines[sysctl_content0.stdout_lines.index("vm.swappiness=5")] == "vm.swappiness=5"'
-          - 'not sysctl_test1.changed'
+
-          - 'sysctl_content0.stdout_lines == ["vm.swappiness=5"]'
+- name: Remove kernel.panic
+  sysctl:
+    name: kernel.panic
+    value: 2
+    reload: no
+    state: absent
+    sysctl_file: "{{ output_dir_test }}/sysctl.conf"
+  register: sysctl_test2
+
+- name: get file content
+  shell: "cat {{ output_dir_test }}/sysctl.conf | egrep -v ^\\#"
+  register: sysctl_content2
+
+- debug:
+    var: item
+    verbosity: 1
+  with_items:
+    - "{{ sysctl_test2 }}"
+    - "{{ sysctl_content2 }}"
+
+- name: Validate results for key removal
+  assert:
+    that:
+      - sysctl_test2 | changed
+      - "'kernel.panic' not in sysctl_content2.stdout_lines"
+
+- name: Test remove kernel.panic again
+  sysctl:
+    name: kernel.panic
+    value: 2
+    state: absent
+    reload: no
+    sysctl_file: "{{ output_dir_test }}/sysctl.conf"
+  register: sysctl_test2_change_test
+
+- name: Assert that no change was made
+  assert:
+    that:
+      - not sysctl_test2_change_test | changed
 
 ##
 ## sysctl - sysctl_set
@@ -72,23 +121,28 @@
 
 - name: set net.ipv4.ip_forward
   sysctl:
-      name: net.ipv4.ip_forward
+    name: net.ipv4.ip_forward
-      value: 1
+    value: 1
-      sysctl_set: yes
+    sysctl_set: yes
-      reload: False
+    reload: no
-  register: sysctl_test2
+  register: sysctl_test3
-- debug: var=sysctl_test2
 
 - name: check with sysctl command
   shell: sysctl net.ipv4.ip_forward
-  register: sysctl_check2
+  register: sysctl_check3
 
-- name: validate results for test 2
+- debug:
+    var: item
+    verbosity: 1
+  with_items:
+    - "{{ sysctl_test3 }}"
+    - "{{ sysctl_check3 }}"
+
+- name: validate results for test 3
   assert:
-      that:
+    that:
-          - 'sysctl_test2.changed is defined'
+      - sysctl_test3 | changed
-          - 'sysctl_test2.changed'
+      - 'sysctl_check3.stdout_lines == ["net.ipv4.ip_forward = 1"]'
-          - 'sysctl_check2.stdout_lines == ["net.ipv4.ip_forward = 1"]'
 
 - name: Try sysctl with no name
   sysctl:
@@ -101,8 +155,8 @@
 - name: validate nameless results
   assert:
     that:
-    - "sysctl_no_name|failed"
+      - sysctl_no_name | failed
-    - "sysctl_no_name.msg == 'name can not be None'"
+      - "sysctl_no_name.msg == 'name cannot be None'"
 
 - name: Try sysctl with no value
   sysctl:
@@ -115,5 +169,5 @@
 - name: validate nameless results
   assert:
     that:
-    - "sysctl_no_value|failed"
+      - sysctl_no_value | failed
-    - "sysctl_no_value.msg == 'value can not be None'"
+      - "sysctl_no_value.msg == 'value cannot be None'"