Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

playbook using sysctl module does not remove sysctl entries when state set to absent #29920

Closed
ansibot opened this issue Sep 12, 2017 · 8 comments · Fixed by #31486
Closed

playbook using sysctl module does not remove sysctl entries when state set to absent #29920

ansibot opened this issue Sep 12, 2017 · 8 comments · Fixed by #31486
Assignees
Labels
affects_2.3 This issue/PR affects Ansible v2.3 bug This issue/PR relates to a bug. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@ansibot
Copy link
Contributor

ansibot commented Sep 12, 2017

From @blutgens on 2016-01-27T14:24:49Z

ISSUE TYPE

Bug Report

COMPONENT NAME

sysctl module

ANSIBLE VERSION

N/A

SUMMARY

I'm using the following playbook to add/remove sysctl
-------- apply-sysctl.yml -----

  • name: Apply sysctl settings
    hosts: all
    tasks:
    • name: Set/Apply sysctl settings in hosts_vars/.yml
      sysctl:
      name: "{{ item.value.name }}"
      value: "{{ item.value.value }}"
      state: "{{ item.value.state }}"
      with_dict: sysctl | default({})

      With a dict in a host_vars file that looks like so:
      ---- host_vars dict ----
      sysctl:
      swappiness:
      name: vm.swappiness
      value: 50
      state: absent
      ------- host_vars dict-----

This all works great when state: present, it adds and applies and does everything it should. But when I set state: absent it seems like it works, but it just doesn't do any actual removal.

Both of these entries are in the sysctl.conf at the the start of this command as well as after:
myusername@myansiblehost: ~/ansible-plays ▸ ansible-playbook -l myhostname apply-sysctl.yml -u root

PLAY [Apply sysctl settings] ***************************************************

TASK [setup] *******************************************************************
ok: [myhostname]

TASK [Set/Apply sysctl settings in hosts_vars/.yml] ******************
ok: [myhostname] => (item={'value': {u'state': u'absent', u'name': u'vm.swappiness', u'value': 50}, 'key': u'swappiness'})
ok: [myhostname] => (item={'value': {u'state': u'absent', u'name': u'vm.nr_hugepages', u'value': 484}, 'key': u'hugepages'})

PLAY RECAP *********************************************************************
myhostname : ok=2 changed=0 unreachable=0 failed=0
******************************************** command output *********************************************************
Now, if i use the sysctl module in an ad-hoc "state=absent" command, it works perfectly!

Copied from original issue: ansible/ansible-modules-core#2899

@ansibot
Copy link
Contributor Author

ansibot commented Sep 12, 2017

From @blutgens on 2016-01-27T14:24:49Z

Oh I should mention that I'm using this version:
ansible --version
ansible 2.1.0 (devel 1b82de2) last updated 2016/01/15 14:42:19 (GMT +000)
lib/ansible/modules/core: (detached HEAD 0020287) last updated 2015/12/28 20:08:17 (GMT +000)
lib/ansible/modules/extras: (detached HEAD f6a7b6d) last updated 2015/12/28 20:08:19 (GMT +000)
config file = /home/lut0023/ansible-plays/ansible.cfg
configured module search path = Default w/o overrides

@ansibot ansibot added the affects_2.3 This issue/PR affects Ansible v2.3 label Sep 12, 2017
@ansibot
Copy link
Contributor Author

ansibot commented Sep 12, 2017

From @blutgens on 2016-01-27T14:24:49Z

It seems if I unset the value in the dict, it will remove the entries! So this is probably NOT a bug! But I'll leave it for you smart people to decide!

@ansibot
Copy link
Contributor Author

ansibot commented Sep 12, 2017

From @jimi-c on 2016-01-27T14:24:49Z

Hi @blutgens, I do believe this is a bug, as the module should most likely ignore the value when state=absent and remove the value (or at least check to see if the value matches and then remove it), so we'll keep this open.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 12, 2017

From @r1k0 on 2016-01-27T14:24:49Z

problem is that the sysctl module can only remove entries if NO value is specified as parameters. Why has this been changed?

This is by design:

- sysctl: 
    name: vm.swappiness 
    value: 5
    state: present

# Remove kernel.panic entry from /etc/sysctl.conf
- sysctl:
    name: kernel.panic
    state: absent 
    sysctl_file: /etc/sysctl.conf

This is inconvenient because if I change my vars from:

{ key: 'net.ipv6.conf.all.disable_ipv6',     value: '1', state: 'present' } 

to

{ key: 'net.ipv6.conf.all.disable_ipv6',     value: '1', state: 'absent' } 

The entry will NOT be removed.
So now I have to edit the task instead of playing with variables.

Originally this module had the support! Look at https://github.com/ansible/ansible/pull/1810/files

+def sysctl_args_collapse(**sysctl_args):
+    # go ahead
+    if sysctl_args.get('key_path') is not None:
+        del sysctl_args['key_path']
+    if sysctl_args['state'] == 'absent' and 'value' in sysctl_args:
+        del sysctl_args['value']
+    

But this code has vanished most likely from an update. @davixx is not reponsible for that change.

When state is absent the entry should be removed regardless of value or not.
Preventing a removal when state=absent and value is not None makes no sense.

WORKAROUND

For those who will hit this bad change of design, using ansible-2.2.0 you can change in /usr/lib/python2.7/dist-packages/ansible/modules/core/system/sysctl.py

root@xx:~# diff   /usr/lib/python2.7/dist-packages/ansible/modules/core/system/sysctl.py /tmp/sysctl.py 
137a138,139
>         if self.args['state'] == "absent" and self.args['value'] != None: self.args['value'] = None
> 
root@xx:~# 

Put the updated file into your library/ folder so it's used instead of /usr/lib/python2.7/dist-packages/ansible/modules/core/system/sysctl.py and good to go.

thank you

ps: also sysctl.py has a lot of trailing spaces, it's not nice ^^

@ansibot
Copy link
Contributor Author

ansibot commented Sep 12, 2017

@ansibot ansibot added bug_report module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 12, 2017
@samdoran samdoran self-assigned this Oct 4, 2017
@samdoran
Copy link
Contributor

samdoran commented Oct 4, 2017

Possibly related to #23905

@blutgens
Copy link

blutgens commented Oct 4, 2017 via email

@samdoran
Copy link
Contributor

samdoran commented Oct 9, 2017

@blutgens Can you test with PR #31486 ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 bug This issue/PR relates to a bug. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants