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

template module doesn't return a backup file #83167

Closed
1 task done
crazzy opened this issue Apr 30, 2024 · 7 comments
Closed
1 task done

template module doesn't return a backup file #83167

crazzy opened this issue Apr 30, 2024 · 7 comments
Labels
affects_2.11 bug This issue/PR relates to a bug. module This issue/PR relates to a module.

Comments

@crazzy
Copy link

crazzy commented Apr 30, 2024

Summary

Using the backup option in the template module to get a backup_file property on the registered return value from the task doesn't work. It's indicated here that it should be working:
https://docs.ansible.com/ansible/latest/reference_appendices/faq.html#complex-configuration-validation
https://docs.ansible.com/ansible/latest/collections/ansible/builtin/template_module.html

From what reading of the code I've done this option doesn't seem to be implemented but I may be wrong in this.

Doing a debug on the registered variable from my template task clearly shows there's no backup file being returned:

ok: [target_hostname] => {
    "proxmox_cluster_firewall_updated": {
        "changed": false,
        "checksum": "a3ea09435174ebf7702ca2ac3e9c5b23c162dc12",
        "dest": "/etc/pve/firewall/cluster.fw",
        "diff": {
            "after": {
                "path": "/etc/pve/firewall/cluster.fw"
            },
            "before": {
                "path": "/etc/pve/firewall/cluster.fw"
            }
        },
        "failed": false,
        "gid": 33,
        "group": "www-data",
        "mode": "0640",
        "owner": "root",
        "path": "/etc/pve/firewall/cluster.fw",
        "size": 459,
        "state": "file",
        "uid": 0
    }
}

Issue Type

Bug Report

Component Name

template

Ansible Version

$ ansible --version
ansible [core 2.11.12]
  config file = /local/home/username/git/xxxxx/ansible/ansible.cfg
  configured module search path = ['/local/home/username/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /local/home/username/venv/lib64/python3.6/site-packages/ansible
  ansible collection location = /local/home/username/.ansible/collections:/usr/share/ansible/collections
  executable location = /local/home/username/venv/bin/ansible
  python version = 3.6.8 (default, Nov 14 2023, 16:29:52) [GCC 4.8.5 20150623 (Red Hat 4.8.5-44)]
  jinja version = 3.0.3
  libyaml = True

Configuration

# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all
CACHE_PLUGIN(/local/home/username/git/xxxxx/ansible/ansible.cfg) = jsonfile
CACHE_PLUGIN_CONNECTION(/local/home/username/git/xxxxx/ansible/ansible.cfg) = ./cache/
CACHE_PLUGIN_PREFIX(/local/home/username/git/xxxxx/ansible/ansible.cfg) = facts_
CALLBACKS_ENABLED(/local/home/username/git/xxxxx/ansible/ansible.cfg) = ['profile_tasks', 'timer']
DEFAULT_FORKS(/local/home/username/git/xxxxx/ansible/ansible.cfg) = 20
DEFAULT_GATHERING(/local/home/username/git/xxxxx/ansible/ansible.cfg) = smart
DEFAULT_HOST_LIST(/local/home/username/git/xxxxx/ansible/ansible.cfg) = ['/local/home/username/git/xxxxx/ansible/etc/hosts', '/local/home/username/git/xxxxx/ansible/inventory']
DEFAULT_ROLES_PATH(/local/home/username/git/xxxxx/ansible/ansible.cfg) = ['/local/home/username/git/xxxxx/ansible/roles']
DEFAULT_STDOUT_CALLBACK(/local/home/username/git/xxxxx/ansible/ansible.cfg) = debug
DEFAULT_TIMEOUT(/local/home/username/git/xxxxx/ansible/ansible.cfg) = 240
DEPRECATION_WARNINGS(/local/home/username/git/xxxxx/ansible/ansible.cfg) = False
RETRY_FILES_ENABLED(/local/home/username/git/xxxxx/ansible/ansible.cfg) = True

OS / Environment

ansible host: CentOS Linux release 7.9.2009 (Core)
target host: Debian 12.5 / Proxmox VE 8.2.2

Steps to Reproduce

- name: Configure cluster firewalling
  block:
    - name: Configure proxmox cluster firewall
      ansible.builtin.template:
        src: "{{ role_path }}/templates/etc/pve/firewall/cluster.fw.j2"
        dest: "/etc/pve/firewall/cluster.fw"
        mode: 0640
        owner: root
        group: www-data
        backup: yes
      register: proxmox_cluster_firewall_updated

    # This command returns 0 regardless of if it fails or not
    - name: Validate cluster firewall config
      ansible.builtin.shell: |
        #!/bin/bash
        pve-firewall compile
      args:
        executable: /bin/bash
      check_mode: true
      changed_when: false
      register: proxmox_cluster_firewall_validate
      failed_when: '"errors" in proxmox_cluster_firewall_validate.stdout'

  rescue:
    - name: Restore backup of proxmox cluster firewall configuration
      ansible.builtin.copy:
        remote_src: true
        src: "{{ proxmox_cluster_firewall_updated.backup_file }}"
        dest: "/etc/pve/firewall/cluster.fw"

  always:
    - name: debug
      debug:
        var: proxmox_cluster_firewall_updated

    - name: Delete cluster firewall backup file
      ansible.builtin.file:
        path: "{{ proxmox_cluster_firewall_updated.backup_file }}"
        state: absent

Expected Results

I expected the registered variable from the template task to contain the property "backup_file" as is indicated in the documentation that it would:
https://docs.ansible.com/ansible/latest/reference_appendices/faq.html#complex-configuration-validation
https://docs.ansible.com/ansible/latest/collections/ansible/builtin/template_module.html

Actual Results

MSG:

The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'backup_file'

The error appears to be in '/local/home/username/git/xxxxx/ansible/roles/proxmox/tasks/main.yml': line 61, column 7, but may be elsewhere in the file depending on the exact syntax problem.

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. affects_2.11 module This issue/PR relates to a module. labels Apr 30, 2024
@ansibot
Copy link
Contributor

ansibot commented Apr 30, 2024

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the component bot command.

@ansibot
Copy link
Contributor

ansibot commented Apr 30, 2024

@crazzy ansible-core 2.11 is not supported and no longer receives bug fixes. Please test against one of the supported versions of ansible-core, preferably the most recent one, to see whether the bug has been fixed.

click here for bot help

@sivel
Copy link
Member

sivel commented Apr 30, 2024

I believe this is ultimately a duplicate of #33907

@bcoca
Copy link
Member

bcoca commented May 1, 2024

"changed": false, should not return a backup_file, it should only appear when the file content actually changed.

@flowerysong
Copy link
Contributor

Even so, the documentation should be fixed. It's an illustrative sketch of the process which is enough to go on if you're familiar with Ansible, but tweaking it slightly would make it more usable for a broad audience.

- name: update config and back out if validation fails
  block:
    # This approach can be used with other actions that support `backup`,
    # such as `copy` and `lineinfile`.
    - name: do the actual update
      template:
        src: template.j2
        dest: /foo/bar/baz
        backup: yes
      register: config_update

    # The assumption is that the validation command will return non-zero for
    # failure, make sure to use `failed_when` if that's not the case.
    - name: validate the updated state
      shell: my_validation_commmand
      become: true
      become_user: requiredbyapp
      environment:
        WEIRD_REQUIREMENT: 1
      when: config_update is changed
  rescue:
    # Hopefully the previous configuration was working
    - name: restore original file
      copy:
        remote_src: true
        src: "{{ config_update['backup_file'] }}"
        dest: /foo/bar/baz
  always:
    # We choose to always delete the backup, but it could also be left in place,
    # moved somewhere for auditing, or only deleted in `rescue`.
    - name: delete backup file
      file:
        path: "{{ config_update['backup_file'] }}"
        state: absent
      when: config_update is changed

@bcoca
Copy link
Member

bcoca commented May 1, 2024

Agreed, docs should match behavior, I was just clarifying the currently expected behavior in this case.

I'm not against always including it, but if the file is not created you could still have issues with the user assuming so in their play or the key returning a null value.

Right now the above would also work but just checking "backup_file" in config_update so it is not that much of an impactful change.

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label May 2, 2024
@bcoca
Copy link
Member

bcoca commented May 2, 2024

closing as this was not a documented feature, just unclear from examples. docs PR above should help clarify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.11 bug This issue/PR relates to a bug. module This issue/PR relates to a module.
Projects
None yet
Development

No branches or pull requests

5 participants