diff --git a/changelogs/fragments/sysctl-check-file-and-system.yaml b/changelogs/fragments/sysctl-check-file-and-system.yaml new file mode 100644 index 0000000000..8bf33ed493 --- /dev/null +++ b/changelogs/fragments/sysctl-check-file-and-system.yaml @@ -0,0 +1,2 @@ +bugfixes: + - sysctl - check system values, not just sysctl.conf file, when determing state (https://github.com/ansible/ansible/pull/56153#issuecomment-514384922) diff --git a/lib/ansible/modules/system/sysctl.py b/lib/ansible/modules/system/sysctl.py index 521d7ab908..ad7f349ec7 100644 --- a/lib/ansible/modules/system/sysctl.py +++ b/lib/ansible/modules/system/sysctl.py @@ -169,9 +169,16 @@ class SysctlModule(object): elif self.file_values[thisname] != self.args['value']: self.changed = True self.write_file = True + # with reload=yes we should check if the current system values are + # correct, so that we know if we should reload + elif self.args['reload']: + if self.proc_value is None: + self.changed = True + elif not self._values_is_equal(self.proc_value, self.args['value']): + self.changed = True # use the sysctl command or not? - if self.args['sysctl_set']: + if self.args['sysctl_set'] and self.args['state'] == "present": if self.proc_value is None: self.changed = True elif not self._values_is_equal(self.proc_value, self.args['value']): @@ -182,7 +189,7 @@ class SysctlModule(object): if not self.module.check_mode: if self.write_file: self.write_sysctl() - if self.write_file and self.args['reload']: + if self.changed and self.args['reload']: self.reload_sysctl() if self.set_proc: self.set_token_value(self.args['name'], self.args['value']) diff --git a/test/integration/targets/sysctl/tasks/main.yml b/test/integration/targets/sysctl/tasks/main.yml index e23b2ccee6..d72fd52448 100644 --- a/test/integration/targets/sysctl/tasks/main.yml +++ b/test/integration/targets/sysctl/tasks/main.yml @@ -20,191 +20,272 @@ # apply sysctl, or it will always fail, because of that in most cases (except # those when it should fail) we have to use `reload=no`. -- set_fact: - output_dir_test: "{{ output_dir }}/test_sysctl" +- name: Test inside Docker + when: + - ansible_facts.virtualization_type == 'docker' + block: + - 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 + - name: make sure our testing sub-directory does not exist + file: + path: "{{ output_dir_test }}" + state: absent -- name: create our testing sub-directory - file: - path: "{{ output_dir_test }}" - state: directory + - name: create our testing sub-directory + file: + path: "{{ output_dir_test }}" + state: directory -## -## sysctl - file manipulation -## + ## + ## sysctl - file manipulation + ## -- name: copy the example conf to the test dir - copy: - src: sysctl.conf - dest: "{{ output_dir_test }}" + - name: copy the example conf to the test dir + copy: + src: sysctl.conf + dest: "{{ output_dir_test }}" -- name: Set vm.swappiness to 5 - sysctl: - name: vm.swappiness - value: 5 - state: present - reload: no - sysctl_file: "{{ output_dir_test }}/sysctl.conf" - register: sysctl_test0 + - name: Set vm.swappiness to 5 + sysctl: + name: vm.swappiness + value: 5 + state: present + reload: no + sysctl_file: "{{ output_dir_test }}/sysctl.conf" + register: sysctl_test0 -- debug: - var: sysctl_test0 - verbosity: 1 + - debug: + var: sysctl_test0 + verbosity: 1 -- name: get file content - shell: "cat {{ output_dir_test }}/sysctl.conf | egrep -v ^\\#" - register: sysctl_content0 + - name: get file content + shell: "cat {{ output_dir_test }}/sysctl.conf | egrep -v ^\\#" + register: sysctl_content0 -- debug: - var: sysctl_content0 - verbosity: 1 + - debug: + var: sysctl_content0 + verbosity: 1 -- name: Set vm.swappiness to 5 again - sysctl: - name: vm.swappiness - value: 5 - state: present - reload: no - sysctl_file: "{{ output_dir_test }}/sysctl.conf" - register: sysctl_test1 + - name: Set vm.swappiness to 5 again + sysctl: + name: vm.swappiness + value: 5 + state: present + reload: no + sysctl_file: "{{ output_dir_test }}/sysctl.conf" + register: sysctl_test1 -- name: validate results - assert: - that: - - sysctl_test0 is changed - - sysctl_test1 is not changed - - 'sysctl_content0.stdout_lines[sysctl_content0.stdout_lines.index("vm.swappiness=5")] == "vm.swappiness=5"' + - name: validate results + assert: + that: + - sysctl_test0 is changed + - sysctl_test1 is not changed + - 'sysctl_content0.stdout_lines[sysctl_content0.stdout_lines.index("vm.swappiness=5")] == "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: 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 + - 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 }}" + - debug: + var: item + verbosity: 1 + with_items: + - "{{ sysctl_test2 }}" + - "{{ sysctl_content2 }}" -- name: Validate results for key removal - assert: - that: - - sysctl_test2 is changed - - "'kernel.panic' not in sysctl_content2.stdout_lines" + - name: Validate results for key removal + assert: + that: + - sysctl_test2 is 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: 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: - - sysctl_test2_change_test is not changed + - name: Assert that no change was made + assert: + that: + - sysctl_test2_change_test is not changed -- name: Try sysctl with an invalid value - sysctl: - name: net.ipv4.ip_forward - value: foo - register: sysctl_test3 - ignore_errors: yes + - name: Try sysctl with an invalid value + sysctl: + name: net.ipv4.ip_forward + value: foo + register: sysctl_test3 + ignore_errors: yes -- debug: - var: sysctl_test3 - verbosity: 1 + - debug: + var: sysctl_test3 + verbosity: 1 -- name: validate results for test 3 - assert: - that: - - sysctl_test3 is failed + - name: validate results for test 3 + assert: + that: + - sysctl_test3 is failed -## -## sysctl - sysctl_set -## + ## + ## sysctl - sysctl_set + ## -- name: set net.ipv4.ip_forward - sysctl: - name: net.ipv4.ip_forward - value: 1 - sysctl_set: yes - reload: no - register: sysctl_test3 + - name: set net.ipv4.ip_forward + sysctl: + name: net.ipv4.ip_forward + value: 1 + sysctl_set: yes + reload: no + register: sysctl_test3 -- name: check with sysctl command - shell: sysctl net.ipv4.ip_forward - register: sysctl_check3 + - name: check with sysctl command + shell: sysctl net.ipv4.ip_forward + register: sysctl_check3 -- debug: - var: item - verbosity: 1 - with_items: - - "{{ sysctl_test3 }}" - - "{{ sysctl_check3 }}" + - debug: + var: item + verbosity: 1 + with_items: + - "{{ sysctl_test3 }}" + - "{{ sysctl_check3 }}" -- name: validate results for test 3 - assert: - that: - - sysctl_test3 is changed - - 'sysctl_check3.stdout_lines == ["net.ipv4.ip_forward = 1"]' + - name: validate results for test 3 + assert: + that: + - sysctl_test3 is changed + - 'sysctl_check3.stdout_lines == ["net.ipv4.ip_forward = 1"]' -- name: Try sysctl with no name - sysctl: - name: - value: 1 - sysctl_set: yes - ignore_errors: True - register: sysctl_no_name + - name: Try sysctl with no name + sysctl: + name: + value: 1 + sysctl_set: yes + ignore_errors: True + register: sysctl_no_name -- name: validate nameless results - assert: - that: - - sysctl_no_name is failed - - "sysctl_no_name.msg == 'name cannot be None'" + - name: validate nameless results + assert: + that: + - sysctl_no_name is failed + - "sysctl_no_name.msg == 'name cannot be None'" -- name: Try sysctl with no value - sysctl: - name: Foo - value: - sysctl_set: yes - ignore_errors: True - register: sysctl_no_value + - name: Try sysctl with no value + sysctl: + name: Foo + value: + sysctl_set: yes + ignore_errors: True + register: sysctl_no_value -- name: validate nameless results - assert: - that: - - sysctl_no_value is failed - - "sysctl_no_value.msg == 'value cannot be None'" + - name: validate nameless results + assert: + that: + - sysctl_no_value is failed + - "sysctl_no_value.msg == 'value cannot be None'" -- name: Try sysctl with an invalid value - sysctl: - name: net.ipv4.ip_forward - value: foo - sysctl_set: yes - register: sysctl_test4 - ignore_errors: yes + - name: Try sysctl with an invalid value + sysctl: + name: net.ipv4.ip_forward + value: foo + sysctl_set: yes + register: sysctl_test4 + ignore_errors: yes -- debug: - var: sysctl_test4 - verbosity: 1 + - debug: + var: sysctl_test4 + verbosity: 1 -- name: validate results for test 4 - assert: - that: - - sysctl_test4 is failed + - name: validate results for test 4 + assert: + that: + - sysctl_test4 is failed + + +- name: Test on RHEL VMs + when: + - ansible_facts.virtualization_type != 'docker' + - ansible_facts.distribution == 'RedHat' + block: + # Test reload: yes + - name: Set sysctl property using module + sysctl: + name: vm.swappiness + value: '22' + state: present + reload: yes + register: sysctl_set1 + + - name: Change sysctl property using command + command: sysctl vm.swappiness=33 + + - name: Set sysctl property using module + sysctl: + name: vm.swappiness + value: '22' + state: present + reload: yes + register: sysctl_set2 + + - name: Read /etc/sysctl.conf + command: 'egrep -v ^# /etc/sysctl.conf' + register: sysctl_conf_content + + - name: Get current value of vm.swappiness + command: sysctl -n vm.swappiness + register: sysctl_current_vm_swappiness + + - name: Ensure changes were made appropriately + assert: + that: + - sysctl_set1 is changed + - sysctl_set2 is changed + - "'vm.swappiness=22' in sysctl_conf_content.stdout_lines" + - sysctl_current_vm_swappiness.stdout == '22' + + # Test reload: yes in check mode + - name: Set the same value using module in check mode + sysctl: + name: vm.swappiness + value: '22' + state: present + reload: yes + check_mode: yes + register: sysctl_check_mode1 + + - name: Set a different value using module in check mode + sysctl: + name: vm.swappiness + value: '44' + state: present + reload: yes + check_mode: yes + register: sysctl_check_mode2 + + - name: Read /etc/sysctl.conf + command: 'egrep -v ^# /etc/sysctl.conf' + register: sysctl_check_mode_conf_content + + - name: Get current value of vm.swappiness + command: sysctl -n vm.swappiness + register: sysctl_check_mode_current_vm_swappiness + + - name: Ensure no changes were made in check mode + assert: + that: + - sysctl_check_mode1 is success + - sysctl_check_mode2 is changed + - "'vm.swappiness=22' in sysctl_check_mode_conf_content.stdout_lines" + - sysctl_check_mode_current_vm_swappiness.stdout == '22'