Convention for "reverting" code


#1

Currently in ansible we have to upgrade existing confs. It is mostly due to fixing bug in production, but it could also happen for peculiar update of non-containerized services or whatever. This might suppose to remove old conf before deploying the new one. This can be some code like:

- name: cleanup old nameservers config
  lineinfile:
    path: /etc/dhcp/dhclient.conf
    line: "prepend domain-name-servers {{ hostvars['bind-host']['ansible_host'] }};"
    state: absent

Since it is intended to occurs only once in production, this kind of code has some special/ephemeral status. So I wonder how to consider it:

  • we should maintain it because it is part of project history;

  • we should remove it as soon it made its job because it’s accumulation will be buggy and confusing.

I think it is better for long term to remove it. But if we choose to remove it we have to organize ourself to do so. I would propose to:

  • mark the considered code using comments;
  • define a removal condition: for avoiding ambiguousness we should prefer some machine readable code like grep -q 'prepend domain-name-server' /etc/dhcp/dhclient.conf && echo Do not remove it || echo Ok, you can remove it rather than some complex discussion;
  • open a bug tagged “Revert cleanup” referring precisely the code bloc and the removal condition;
  • add the proposal in documented conventions.

What do you think?


#2

Great question, @fpoulain! We’ve wrestled with this problem on long-running servers, as well. There are a couple workable solutions, with varying degrees of effort to get them implemented. The trick is to strike a balance between up-front investment in the config declaration with the maintenance burden over time.

Short answer: use templates, not lineinfile. That way, every playbook run will ensure that the entirety of a file is what you expect, and you don’t need to micromanage. You’ll have to write a template for the file, of course, which can be a bit irritating, but often there are community-maintained roles that will work for your use case, as long as you override a few vars at the site/group_vars level.

Another option while you’re still using lineinfile is to set the substitutions in a list of dicts, and update the task to assume state=present, but support overrides to state=absent. Example:

---
# group_vars/all.yml
dns_nameservers_prepend:
  - regexp: "^prepend domain-name-servers"
    line: "prepend domain-name-servers {{ hostvars['bind-host']['ansible_host'] }};"
    state: absent
# tasks/main.yml (whatever source file you posted the excerpt from)
- name: cleanup old nameservers config
  lineinfile:
    path: /etc/dhcp/dhclient.conf
    regexp: "{{ item.regexp }}"
    line: "{{ item.line }}"
    state: "{{ item.state|default(omit) }}"
  with_items: "{{ dns_nameservers_prepend }}"

Then you can make a one-line change to the vars to toggle present/absent. The template solution is what I’d prefer for long-term maintenance, but perhaps the stopgap will be useful to you, as well.

Does that work for you?


#3

I think so. I used lineinfile because at this time I wanted to not overload my influence on system’s conf, but I agree, for this specific example, it was a bad choice.