[FEATURE REQUEST] Provide file.absent with removedirs option

[jgraichen]

Is your feature request related to a problem? Please describe.

file.managed does support makedirs: True for creating the directories need to write a file. When that file is no longer needed, file.absent can remove that file, but all directories are kept.

Describe the solution you'd like

I'd like to have a removedirs: True option on file.absent that works like Python's os.removedirs() function and removes all empty parent directories after removing the file.

/path/to/file.txt:
  file.absent:
    - removedirs: True

This also looks very symmetrical to file.managed with makedirs: True.

Describe alternatives you've considered

The most common solution mentioned is using file.absent on the parent directory with unless: and some glob check, but that has many edge cases. Just running a /path/to/* glob often will not detect .hidden files, and it only works for a single directory level. A custom state could be added, but needs to be maintained separately:

# vim: ft=python:sw=4

import os

def _each_parent(name):
    head, tail = os.path.split(name)
    if not tail:
        head, tail = os.path.split(head)

    while head and tail:
        yield head
        head, tail = os.path.split(head)

def absent(name, removedirs=True):
    """
    Ensures the given file path is removed, and all empty parent directories are
    removed too.
    """

    ret = __states__["file.absent"](name)

    if "removed" in ret["changes"]:
        ret["changes"]["removed"] = [ret["changes"]["removed"]]

    if not removedirs or ret["result"] == False:
        return ret

    # Clean up empty parent directories
    for path in _each_parent(name):
        try:
            if not os.path.isdir(path) or len(os.listdir(path)) > 0:
                break

            if "removed" not in ret["changes"]:
                ret["changes"]["removed"] = []

            ret["changes"]["removed"].append(path)

            if __opts__["test"]:
                ret["result"] = None
            else:
                os.rmdir(path)
        except OSError:
            break

    return ret

(This one does not change the comment, but extends the state diff with deleted parent directories.)

[dmurphy18]

@jgraichen May I direct you to file.remove which already performs the operation you desire, see
https://docs.saltproject.io/en/latest/ref/modules/all/salt.modules.file.html#salt.modules.file.remove

Also https://docs.saltproject.io/en/latest/ref/states/all/salt.states.file.html#salt.states.file.absent for file.absent states

Make sure that the named file or directory is absent. If it exists, it will be deleted. This will work to reverse any of the functions in the file state module. If a directory is supplied, it will be recursively deleted.

Hence file.absent deletes a provided directory path, just tried it and it works.

Closing this issue since functionality already exists, but please re-open if need to bring some other information to attention.

[jgraichen]

This is more about deleting empty parent directories, not nested content. Image the following state, which write a configuration file, or removes it, if the pillar value is not set:

/etc/systemd/system/name.service.d/override.conf:
{% if pillar['config'] %}
  file.managed:
    - contents_pillar: config
    - makedirs: True # This will create the `name.service.d` directory
{% else %}
  file.absent: # This will *only* remove the file, but leave the empty `name.service.d` directory
{% endif %}

This is a basic scenario, but with loops, and deeper directories, a removedirs: True option to the file.absent state would be very helpful. The only alternative is to keep empty directories, which not all application will accept, or more complex states, checking if there are any other files (maybe a different state).

For example, in the above example, when pillar['config'] is empty/null, salt will remove the file override.conf, and also check if name.service.d is empty. If it is, it will be removed too.

[dmurphy18]

@jgraichen I actually tried with empty sub-directories and files and they were all deleted, including the specified parent directory. Please give a specific test case where the command is not working, and file it as a bug report.

Note file.absent will only remove a file if the specified input is a file, if the specified input is a directory then it removes the directory recursively.

[jgraichen]

Please note that this is not about deleting a directory and its content! It's the contrary, deleting only all empty parent directories. Please see the example state code and the SLS example. Please also see the difference between os.removedirs() and shutil.rmtree. file.absent on a directory is doing the latter one, not what os.removedirs() is doing.

What I am looking for is not a bug fix, but a feature, for cases such as [1] in nested scenarios, an inverse to makedirs: True.

See this example:

/etc/systemd/system/keepalived.service.d/override.conf:
{% if pillar['config'] %}
  file.managed:
    - contents_pillar: config
    - makedirs: True # This will create the `name.service.d` directory if needed
{% else %}
  file.absent: []
{% endif %}

/etc/systemd/system/keepalived.service.d/ubuntu-1804-patch.conf:
{% if pillar['patch-needed'] %}
  file.managed:
    - contents_pillar: config
    - makedirs: True # This will create the `name.service.d` directory if needed
{% else %}
  file.absent: []
{% endif %}

This will keep the empty directory keepalived.service.d, when pillar['config'] and pillar['patch-needed'] are changed to null/false. Please note that this is a simplified example, with two static states, one can construct a file.absent state one that single directory, which also uses a unless: ls name.service.d or similar.

See this example too:

{% for module, config in pillar['modules_configs'].items() %}
/etc/application/modules/{{ module }}/config/module.ini:
{% if config %}
  file.managed:
    - makedirs: True # By default, the `/etc/application` directory is empty
{% else %}
  file.absent: [] # Config files *and directories* that are not needed anymore must be removed other the app does not start
{% endif %}
{% endfor %}

This is an example application that requires a configuration file for each loaded module, but also does not start if any directory or config files for non-loaded module is present. Therefore, config can be set to False to remove that file. Unfortunately, the application also does write some configuration files itself, in some directories, that must not be removed. Right now, one needs many conditional statements for that:

{% for module, config in pillar['modules_configs'].items() %}
{% if not config %}
/etc/application/modules/{{ module }}/config/module.ini:
  file.absent: [] # remove config file
  
/etc/application/modules/{{ module }}/config:
  file.absent: # remove directory *IF EMPTY*
    - unless: ls /etc/application/modules/{{ module }}/config/*
    
/etc/application/modules/{{ module }}:
  file.absent: # remove directory *IF EMPTY*
    - unless: ls /etc/application/modules/{{ module }}/*
    
/etc/application/modules:
  file.absent: # remove directory *IF EMPTY*
    - unless: ls /etc/application/modules/*
{% endif %}
{% endfor %}

But be aware, the application just wrote a file with leading dot (e.g. .important-state) into one directory. That will not be returned by just ls, so we deleted the directory, and it contents, something we didn't want. We had a data loss because of complex states and wrong assumptions.

All we did want is os.removedirs() after file.absent of the config file. That will not remove any file, and only empty parent directories in any depth. Not a single one, but any depth.

Assume we have two entirely different states (or even some Debian package), that need to put files in the same location, e.g. a system path:

# states/systemd/networkd.sls

/etc/systemd/networkd.d/salt.conf:
{% if pillar['networkd']['config'] %}
  file.managed:
    - contents_pillar: networkd:config
    - makedirs: True
{% else %}
  file.absent: [] # Will not remove `networkd.d` directory
  
/etc/systemd/networkd.d:
  file.absent: []
{% end %}

# states/keepalived.sls

/etc/systemd/networkd.d/keepalived-dependency.conf:
{% if pillar['...'] %}
  file.managed:
    - contents: salt://keepalived/files/systemd-dependency.conf
    - makedirs: True
{% else %}
  file.absent: [] # Will not remove `networkd.d` directory
  
/etc/systemd/networkd.d:
  file.absent: []
{% end %}

These states will just delete each other config files. This would be much easier if both could just have removedirs: True in their file.absent states.

Please let me repeat, this is NOT about deleting a single directory and its content. It is the contrary!

[dmurphy18]

re-opening as Feature Request

[dmurphy18]

@jgraichen A PR with tests would help make the Feature Request proceed quicker, not saying it would be accepted, just working code always helps, otherwise it could be quite a while before it is implemented if the Feature Request is accepted.