ec2_asg: fix #28087 and #35993

[msven]

Fixes #35993 - Changes to update_size in commit eb4cc31 made it so
the group dict passed into update_size was not modified. As a result,
the 'replace' call does not see an updated min_size like it previously
did and doesn't pause to wait for any new instances to spin up. Instead,
it moves straight into terminating old instances. Fix is to add batch_size
to min_size when calling wait_for_new_inst.

Fixes #28087 - Make replace_all_instances and replace_instances behave
exactly the same by setting replace_instances = current list of instances
when replace_all_instances used. Root cause of issue was that without lc_check
terminate_batch will terminate all instances passed to it and after updating
the asg size we were querying the asg again for the list of instances - so terminate batch
saw the list including new ones just spun up.

SUMMARY

Fixed issue "Have the ec2_asg module stop overshooting instances. #28087"
This was caused by the combination of lc_check: false and replace_all_instances: true.
Since lc_check is false terminate_batch will consider all instances passed into it as available
for termination (expected behavior). However, in the replace method just prior to the following loop
for i in get_chunks(instances, batch_size): we fetch the as_group again and update the list
of instances. This list of instances now includes additional instances that we just spun up earlier
in the replace call. This list is passed to terminate_batch and with lc_check false we end up thinking we need to terminate all of them. There are a couple of routes that could be taken to address this. I
chose to set replace_instances equal to the current set of instances attached to the asg at the start
of the replace call. This makes replace_instances and replace_all_instances logic behave the same
from that point on.

Fixed issue "ec2_asg terminates an instance before creating a replacement. #35993"
Commit eb4cc31 made changes to the update_size method which introduced this bug.
eb4cc31 made it so update_size didn't modify the group dict passed to it, but the replace
method expected group to be updated. The fix was to simply add the batch_size to
wait_for_new_inst so we wait for the new instances to spin up. Otherwise, we see
the original min which is already met and we proceed to immediately terminate instances.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

module ec2_asg

ANSIBLE VERSION

ansible 2.4.0.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/share/ansible']
  ansible python module location = /home/msven/github/test/env/local/lib/python2.7/site-packages/ansible
  executable location = /home/msven/github/test/env/bin/ansible
  python version = 2.7.6 (default, Nov 23 2017, 15:49:48) [GCC 4.8.4]

ADDITIONAL INFORMATION

The following playbook can be used to reproduce both issues (assuming you have the launch cfg setup)

---

- hosts: localhost
  gather_facts: false
  tasks:
    - ec2_asg:
        name: test
        launch_config_name: test-lc
        replace_all_instances: true
        wait_for_instances: true
        lc_check: false
        min_size: 2
        max_size: 2
        desired_capacity: 2
        replace_batch_size: 2

[ansibot]

cc @garethr @ryansb @s-hertel @willthames
click here for bot help

[s-hertel]

@msven Since you've been able to reproduce the problems do you mind adding a test case to https://github.com/ansible/ansible/blob/devel/test/integration/targets/ec2_asg/tasks/main.yml so these don't get broken again in the future?

[msven]

@s-hertel - Sure, I'll take a look

[ansibot]

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/cloud/amazon/ec2_asg.py:1421:1: E302 expected 2 blank lines, found 1
lib/ansible/modules/cloud/amazon/ec2_asg.py:1430:1: E302 expected 2 blank lines, found 1

click here for bot help

[msven]

@s-hertel Added some tests cases, along with another small change to the module to fix #28087 for the case where the asg is just being created.

[s-hertel]

Backported to 2.5 in ae4c246. Thanks @msven (and @willthames for bringing it up)!

[willthames]

Thanks for backporting @s-hertel