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

incompatible change in copy resultset in 2.4 #31477

Closed
philfry opened this issue Oct 9, 2017 · 5 comments · Fixed by #35702
Closed

incompatible change in copy resultset in 2.4 #31477

philfry opened this issue Oct 9, 2017 · 5 comments · Fixed by #35702
Labels
affects_2.4 This issue/PR affects Ansible v2.4 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

@philfry
Copy link
Contributor

philfry commented Oct 9, 2017

ISSUE TYPE
  • Bug Report
COMPONENT NAME

copy

ANSIBLE VERSION
ansible 2.4.0.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/phil/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.13 (default, Sep  5 2017, 08:53:59) [GCC 7.1.1 20170622 (Red Hat 7.1.1-3)]
CONFIGURATION

n/a

OS / ENVIRONMENT

n/a

SUMMARY

Between ansible 2.3.2 to 2.4.0 the results array of the copy module has changed in an incompatible way regarding path and dest.

STEPS TO REPRODUCE
  1. ansible-playbook copy.yml
---
- hosts: localhost
  gather_facts: no
  tasks:
    - name: "cleanup"
      command: "rm -rf /tmp/ansibleresults"
      args:
        warn: no

    - name: "create directory structure"
      file: path={{item}} state=directory
      with_items:
        - /tmp/ansibleresults
        - /tmp/ansibleresults/src
        - /tmp/ansibleresults/dest

    - name: "create some dummy files"
      copy: content="I am a dummy file {{item}}" dest=/tmp/ansibleresults/src/myfile{{item}}
      with_items: [ 1,2,3,4 ]

    - name: "pre-copy a file"
      copy: src=/tmp/ansibleresults/src/myfile3 dest=/tmp/ansibleresults/dest/

    - name: "copy all source files"
      copy: src={{item}} dest=/tmp/ansibleresults/dest/
      register: t_copyresults
      with_fileglob:
        - /tmp/ansibleresults/src/*

    - name: "show attribute 'dest'"
      debug: var=item
      with_items:
        - '{{t_copyresults.results|map(attribute="dest")|list}}'
EXPECTED RESULTS
TASK [show attribute 'dest'] ***
ok: [localhost] => (item=/tmp/ansibleresults/dest/myfile4) => {
    "item": "/tmp/ansibleresults/dest/myfile4"
}
ok: [localhost] => (item=/tmp/ansibleresults/dest/myfile3) => {
    "item": "/tmp/ansibleresults/dest/myfile3"
}
ok: [localhost] => (item=/tmp/ansibleresults/dest/myfile2) => {
    "item": "/tmp/ansibleresults/dest/myfile2"
}
ok: [localhost] => (item=/tmp/ansibleresults/dest/myfile1) => {
    "item": "/tmp/ansibleresults/dest/myfile1"
}
ACTUAL RESULTS
TASK [show attribute 'dest'] ***
ok: [localhost] => (item=[u'/tmp/ansibleresults/dest/myfile4', Undefined, u'/tmp/ansibleresults/dest/myfile2', u'/tmp/ansibleresults/dest/myfile1']) => {
    "item": "[u'/tmp/ansibleresults/dest/myfile4', Undefined, u'/tmp/ansibleresults/dest/myfile2', u'/tmp/ansibleresults/dest/myfile1']"
}
@philfry
Copy link
Contributor Author

philfry commented Oct 9, 2017

possible workaround:

    - name: "show attribute 'dest'"
      debug: var=item
      with_items:
        - '{{(t_copyresults.results|selectattr("path", "defined")|map(attribute="path")|list+t_copyresults.results|selectattr("dest", "defined")|map(attribute="dest")|list)|list|unique}}'

but... really?

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bug_report module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 9, 2017
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Oct 13, 2017
@ktosiek
Copy link
Contributor

ktosiek commented Nov 6, 2017

Bisected this to f86ce09, seems to work on stable-2.3 (contrary to #25431)

@jfbibeau
Copy link

This is pretty much making Ansible 2.4 unusable for us. Confirmed that the resultset contains dest when copy task is changed, but contains path when the copy task is unchanged (ie: file already exists).

Example when file was copied:

ok: [192.168.11.10] => {
    "rpm_files": {
        "changed": true,
        "msg": "All items completed",
        "results": [
            {
                "_ansible_ignore_errors": null,
                "_ansible_item_result": true,
                "_ansible_no_log": false,
                "_ansible_parsed": true,
                "changed": true,
                "checksum": "e54a942de3eef088b5db7374b236c4ae8a61f04b",

                "dest": "/tmp/my.rpm",   <- Note the dest attribute, which is what's in ansible 2.2
...

Example when running the same task a 2nd time, this time with the file:

ok: [192.168.11.10] => {
    "rpm_files": {
        "changed": false,
        "msg": "All items completed",
        "results": [
            {
                "_ansible_ignore_errors": null,
                "_ansible_item_result": true,
                "_ansible_no_log": false,
                "_ansible_parsed": true,
                "changed": false,
                "checksum": "e54a942de3eef088b5db7374b236c4ae8a61f04b",

                 <--dest is missing here!!!

                "diff": {
                    "after": {
                        "path": "/tmp/my.rpm"
                    },
                    "before": {
                        "path": "/tmp/my.rpm"
                    }
                },
                "failed": false,
                "gid": 0,
                "group": "root",
                "invocation": {
                   ...
                    }
                },
                "item": "/src/path/to/my.rpm",
                "mode": "0644",
                "owner": "root",

                "path": "my.rpm",           <- dest was replaced by path?!

                "secontext": "unconfined_u:object_r:admin_home_t:s0",
                "size": 41525950,
                "state": "file",
                "uid": 0
...

@jfbibeau
Copy link

Looks like when the file needs to be copied, the copy action is the one that is fired.

However when the file checksum is the same, the file action is the one that is fired (to do any permission changes that are needed).

The file module seems to accept/return only path (which it says is an alias to dest), whereas the copy module seems to accept/return dest only.

I still don't see exactly where in commit f86ce09 this got changed, however the copy action was pretty heavily refactored and the diff is hard to read, so seems plausible the change was introduced there.

@ktosiek
Copy link
Contributor

ktosiek commented Feb 4, 2018

I've prepared a failing test case: ktosiek@d324c09

abadger pushed a commit that referenced this issue Feb 9, 2018
abadger pushed a commit that referenced this issue Feb 9, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bug_report labels Mar 7, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 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.

5 participants