ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
92 stars 19 forks source link

`template` and other modules break zero-assumption declarative configuration principle #206

Open jficz opened 1 year ago

jficz commented 1 year ago

Proposal: a consistent behavior for modules like template which act differently based on what state the dest file is

Author: Jakub Fišer <@jficz> IRC: jficz

Date: 2022-08-09

Motivation

In the declarative config management world one assumes that after an atomic, self-contained step is done, the target system is in the declared state. I'll use the builtin.temlplate module as an example which breaks that assumption.

Problem

With declarative configuration approach, the resulting state of the target system should always be the same regardless of what state the system was before. The template module breaks this principle.

The state the target system is in after the template module execution is directly dependent on the state the system was before the module was executed. This means that after the template module is applied, the admin cannot make any definite assumption as to what state the system is in.

Specifically, the template module result is directly dependent on what the dest file is. If it is a directory, the result is different from when it is a file or if it doesn't exist at all.

If dest doesn't exist or is a file, template module creates/replaces the dest file content with the content of the template source.

If dest, however, is a directory, the module assumes that the admin wants to puts the template file(s) inside that directory and does that.

There is no known way (at least to me) to control this behavior explicitly. Admin cannot assume either result until they make their own procedural tests (like stat, etc) which means that they cannot depend on the module's declarativeness and always have to double-check it's results.

The default behavior should not change to not break current workflows but the admin should have the option to make template module (and others which behave the same) zero-assumption and declarative. A flag or set of flags should exist which will make the result definite at the time of declaration (as opposed to the time of execution).

Example

- name: service config
  template:
    src: "service.conf.j2"
    dest: "/etc/service.conf"
    dest_type: file
    dest_force: true

^^^ In this case template should either delete the dest file if it is a directory or fail audibly if dest_force: false.

- name: service config
  template:
    src: "00-service.conf.j2"
    dest: "/etc/service.conf.d"
    dest_type: directory
    dest_force: true

^^^ In this case, similarly to the above, the module should delete dest if it is a regular file or fail audibly it dest_force: false.

Testing (optional)

Standard module tests I guess.

Documentation (optional)

Affected module's docs should be updated.

bcoca commented 1 year ago

You leave out mode/acls/selinux/etc that are also conditional on the state of the remote system. In most cases we only change what we need to match the information provided in the task itself.

When it comes to directory, we know template always outputs a file, this is an issue with the 'copy' action (which template uses) as it assumes that if the destination is a directory, you meant to copy into it, not over it.

I"m not sure that changing this inside the action at this point is a good idea, the play/role itself can take care of this with a simple 'stat' to the destination instead.

jficz commented 1 year ago

the play/role itself can take care of this with a simple 'stat' to the destination instead.

I'd argue it requires much more than a simple stat to ensure the consistency of the results. It requires a conditional and logical branching of the task sequence. Given how Ansible works, each such a sequence not only adds unnecessary tasks to the play, it also adds valuable time to the play execution, introduces more sources of possible human errors and introduces stronger incentive for the "not-invented-here syndrome" to kick in.

In current situation the admin is basically required to either write configuration checker logic to double-check the result of all the instances of the affected tasks whenever there is even a remote possibility the system is in an unknown state (and then fix the result and basically execute the task again which breaks idempotency), or always bring the system to a well-known state before the module is executed. Neither one of those options plays well with declarative configuration principles.

My proposal doesn't break anything while it allows the module(s) to be much more deterministic.

You correctly state that "it assumes that if the destination is a directory, you meant to copy into it" which is imho the worst possible thing a config management tool could ever do - to assume anything, let alone the admins will that is, on top of everything, based on something the admin might not even have control of.

bcoca commented 1 year ago

Ansible is not just CM, it is an automation tool, it allows you to do CM by creating plays/roles that handle these cases.

You can also solve this issue by creating a custom template (or copy) action that functions the way you want.

jficz commented 1 year ago

I'm well aware I can fix issues by creating custom patches, but thanks for the suggestion anyway. I guess I missed the point of these "proposals". Feel free to close this and sorry to have wasted your time.

bcoca commented 1 year ago

@jficz I'm not even talking about custom patches (though those are always possible) but a custom plugin. This is a fundamental part of how Ansible is designed, with the knowledge that we cannot support 100% of the use cases so we allow for simple override and additions via the plugin system.

This is not a waste of time, proposals are here to discuss major new features or redesigns, it also gives us a look into what people are thinking and how they are using Ansible, this is useful even when we reject a proposed change.