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

Changed behavior with ec2_ami - version 2.5.0 #38482

Closed
maishsk opened this issue Apr 9, 2018 · 10 comments
Closed

Changed behavior with ec2_ami - version 2.5.0 #38482

maishsk opened this issue Apr 9, 2018 · 10 comments
Labels
affects_2.5 This issue/PR affects Ansible v2.5 aws bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. support:certified This issue/PR relates to certified code.

Comments

@maishsk
Copy link
Contributor

maishsk commented Apr 9, 2018

ISSUE TYPE
  • Bug Report
COMPONENT NAME

ec2_ami

ANSIBLE VERSION
ansible 2.5.0
  config file = None
  configured module search path = [u'/Users/msaidelk/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/msaidelk/venv/lib/python2.7/site-packages/ansible
  executable location = /Users/msaidelk/venv/bin/ansible
  python version = 2.7.10 (default, Jul 15 2017, 17:16:57) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)]
CONFIGURATION
OS / ENVIRONMENT

MacOS High Sierra

SUMMARY

name parameter that is now mandatory on all ec2_ami module actions - this was not the case before

STEPS TO REPRODUCE

ansible-playbook playbook.yml

---
- name: Connect to AWS
  hosts: localhost
  connection: local
  gather_facts: False

  tasks:
  - name: Grant permissions on all images
    ec2_ami:
      region: "eu-west-2"
      image_id: ami-44061XYZ
      state: present
      launch_permissions:
        user_ids: 09990XYZ4763
EXPECTED RESULTS

In version 2.4.2.0 the result was successful

TASK [Grant permissions on all images] *****************************************************************************************************************************************************************************************************
task path: /Users/msaidelk/git/hercules/ansible/aws/infra/image_permissions/update_image_permissions_maish.yml:8
Using module file /Users/msaidelk/venv/lib/python2.7/site-packages/ansible/modules/cloud/amazon/ec2_ami.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: msaidelk
<127.0.0.1> EXEC /bin/sh -c 'echo ~ && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501 `" && echo ansible-tmp-1523278035.5-197716510641501="` echo /Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501 `" ) && sleep 0'
<127.0.0.1> PUT /var/folders/2k/tnw1h1js25d9mpwdsfl_zdym0000gn/T/tmpvRLdYS TO /Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501/ec2_ami.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501/ /Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501/ec2_ami.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/Users/msaidelk/venv/bin/python /Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501/ec2_ami.py; rm -rf "/Users/msaidelk/.ansible/tmp/ansible-tmp-1523278035.5-197716510641501/" > /dev/null 2>&1 && sleep 0'
changed: [localhost] => {
    "architecture": "x86_64",
    "block_device_mapping": {
        "/dev/xvda": {
            "delete_on_termination": true,
            "encrypted": false,
            "size": 8,
            "snapshot_id": "snap-5aXYZe3d",
            "volume_type": "gp2"
        }
    },
    "changed": true,
    "creationDate": "2017-06-26T11:50:31.000Z",
    "description": "my.ami",
    "hypervisor": "xen",
    "image_id": "ami-44061XYZ",
    "invocation": {
        "module_args": {
            "architecture": "x86_64",
            "aws_access_key": null,
            "aws_secret_key": null,
            "delete_snapshot": false,
            "description": "",
            "device_mapping": null,
            "ec2_url": null,
            "image_id": "ami-44061XYZ",
            "instance_id": null,
            "kernel_id": null,
            "launch_permissions": {
                "user_ids": [
                    "09990XYZ4763"
                ]
            },
            "name": null,
            "no_reboot": false,
            "profile": null,
            "region": "eu-west-2",
            "root_device_name": null,
            "security_token": null,
            "state": "present",
            "tags": null,
            "validate_certs": true,
            "virtualization_type": "hvm",
            "wait": false,
            "wait_timeout": "900"
        }
    },
    "is_public": false,
    "launch_permissions": {
        "user_ids": [
            "09990XYZ4763"
        ]
    },
    "location": "9884XYZ02534/my.ami",
    "msg": "AMI launch permissions updated",
    "name": "my.ami",
    "ownerId": "9884XYZ02534",
    "platform": null,
    "root_device_name": "/dev/xvda",
    "root_device_type": "ebs",
    "set_perms": {},
    "state": "available",
    "tags": {
        "Name": "my.ami",
        "VPCname": "eu-west-2"
    },
    "virtualization_type": "hvm"
}
META: ran handlers
META: ran handlers

PLAY RECAP *********************************************************************************************************************************************************************************************************************************
localhost                  : ok=1    changed=1    unreachable=0    failed=0
ACTUAL RESULTS
TASK [Grant permissions on all images] *****************************************************************************************************************************************************************************************************
task path: /Users/msaidelk/git/hercules/ansible/aws/infra/image_permissions/update_image_permissions_maish.yml:8
Using module file /Users/msaidelk/venv/lib/python2.7/site-packages/ansible/modules/cloud/amazon/ec2_ami.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: msaidelk
<127.0.0.1> EXEC /bin/sh -c 'echo ~ && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809 `" && echo ansible-tmp-1523277826.0-59969560047809="` echo /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809 `" ) && sleep 0'
<127.0.0.1> PUT /Users/msaidelk/.ansible/tmp/ansible-local-18615a5653N/tmpgJxFW3 TO /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809/ec2_ami.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809/ /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809/ec2_ami.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/Users/msaidelk/venv/bin/python /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809/ec2_ami.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /Users/msaidelk/.ansible/tmp/ansible-tmp-1523277826.0-59969560047809/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
  File "/var/folders/2k/tnw1h1js25d9mpwdsfl_zdym0000gn/T/ansible_Vp0Z6c/ansible_modlib.zip/ansible/module_utils/aws/core.py", line 80, in __init__
    local_settings[key] = kwargs.pop(key)

fatal: [localhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "architecture": "x86_64",
            "delete_snapshot": false,
            "description": "",
            "image_id": "ami-44061XYZ",
            "launch_permissions": {
                "user_ids": "09990XYZ4763"
            },
            "no_reboot": false,
            "purge_tags": false,
            "region": "eu-west-2",
            "state": "present",
            "validate_certs": true,
            "virtualization_type": "hvm",
            "wait": false,
            "wait_timeout": 900
        }
    },
    "msg": "state is present but all of the following are missing: name"
}
	to retry, use: --limit @/Users/msaidelk/git/hercules/ansible/aws/infra/image_permissions/update_image_permissions_maish.retry

PLAY RECAP *********************************************************************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1

It seems that the name is now mandatory for all plays - this was not the case before.

What changed???

@ansibot
Copy link
Contributor

ansibot commented Apr 9, 2018

Files identified in the description:

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

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Apr 9, 2018

@ansibot ansibot added affects_2.5 This issue/PR affects Ansible v2.5 aws bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:certified This issue/PR relates to certified code. labels Apr 9, 2018
@maishsk
Copy link
Contributor Author

maishsk commented Apr 9, 2018

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 9, 2018
@willthames
Copy link
Contributor

@maishsk - that change is irrelevant - required: False is the default anyway.

The more important difference is https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/ec2_ami.py#L680

The underlying reason is that name is a required parameter when creating an image (https://boto3.readthedocs.io/en/latest/reference/services/ec2.html#EC2.Client.create_image) - that may have been a side effect of moving from boto to boto3.

Does the name just default to my.ami with 2.4.2 ? We can always make it the default, even if I hate the default, might be better than accidentally breaking backwards compatibility, which we have worked reasonably hard to maintain.

@maishsk
Copy link
Contributor Author

maishsk commented Apr 10, 2018

@willthames Thanks for the clarification - the my.ami - is NOT the default - the problem is that there is no default - and the is a backward compatible breaking change.

Documentation does not reflect this.. :)

@willthames
Copy link
Contributor

You say it works in 2.4.2 though - how did it work? Or did the AMI not get a name at all with 2.4.2?

@maishsk
Copy link
Contributor Author

maishsk commented Apr 10, 2018

With 2.4.2 I was not required to add a name parameter when changing permissions - I assume it because of the change - you mentioned above.

I checked the following which works both on 2.4.2.0 and on 2.5.0

---
- name: Connect to AWS
  hosts: localhost
  connection: local
  gather_facts: False

  tasks:
  - name: Grant permissions on all images
    ec2_ami:
      region: eu-west-2
      name: ''
      image_id: ami-44061XYZ
      state: present
      launch_permissions:
        user_ids: ['09990XYZ4763']

As soon as the name field is empty - on both versions the code that I used in 2.4.2.0 works also with 2.5.0.

Is it possible to put a default as empty in the code - unless specifically set?

When setting to '' it does not change the AMI name

@maishsk
Copy link
Contributor Author

maishsk commented Apr 10, 2018

Just submitted #38514 to fix this...

willthames pushed a commit that referenced this issue Apr 11, 2018
* Added empty default
Fix for issue #38482
@johnpetersjr
Copy link

johnpetersjr commented Apr 20, 2018

I think I'm seeing a similiar issue... because the wait: yes doesn't seem to work anymore, I'm trying to call ec2_ami twice, once to create an ami, then waiting to see it become available using ec2_ami_facts, and once it is ready, I then try to share the AMI, but I get this error:

"msg": "state is present but all of the following are missing: name"
- name: Share AMI
  ec2_ami:
    name: "{{ ami_created.image_id }}"
    image_id: "{{ ami_created.image_id }}"
    region: us-east-1
    launch_permissions:
      user_ids: ['MYOTHERAWSACCOUNT]

ryansb pushed a commit that referenced this issue May 3, 2018
* removed additional check for name parameter

(cherry picked from commit 91357d0)

* Added empty default
Fix for issue #38482

(cherry picked from commit 60a3277)

* [ec2_ami] Ensure name or image_id is provided for state=present (#38972)

Add integration tests for backward compatibility and ensuring name or image_id is provided
(cherry picked from commit e2aa115)
@s-hertel
Copy link
Contributor

Fixed in e2aa115, backported fix in 389db4c

Thanks @maishsk

ilicmilan pushed a commit to ilicmilan/ansible that referenced this issue Nov 7, 2018
@ansible ansible locked and limited conversation to collaborators May 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.5 This issue/PR affects Ansible v2.5 aws bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. support:certified This issue/PR relates to certified code.
Projects
None yet
Development

No branches or pull requests

6 participants