From b091d39bafe31152c908c7e7017ba2acbe661ba6 Mon Sep 17 00:00:00 2001 From: Ryan Brown Date: Mon, 3 Oct 2016 13:23:42 -0400 Subject: [PATCH] Handle termination_protection parameter when restarting instances (#5076) * Restart EC2 instances with multiple network interfaces A previous bug, #3234, caused instances with multiple ENI's to fail when being started or stopped because sourceDestCheck is a per-interface attribute, but we use the boto global access to it (which only works when there's a single ENI). This patch handles a variant of that bug that only surfaced when restarting an instance, and catches the same type of exception. * Default termination_protection to None instead of False AWS defaults the value of termination_protection to False, so we don't need to explicitly send `False` when the user hasn't specified a termination protection level. Before this patch, the below pair of tasks would: 1. Create an instance (enabling termination_protection) 2. Restart that instance (disabling termination_protection) Now, the default None value would prevent the restart task from disabling termination_protection. ``` - name: make an EC2 instance ec2: vpc_subnet_id: {{ subnet }} instance_type: t2.micro termination_protection: yes exact_count: 1 count_tag: Name: TestInstance instance_tags: Name: TestInstance group_id: {{ group }} image: ami-7172b611 wait: yes - name: restart a protected EC2 instance ec2: vpc_subnet_id: {{ subnet }} state: restarted instance_tags: Name: TestInstance group_id: {{ group }} image: ami-7172b611 wait: yes ``` --- lib/ansible/modules/cloud/amazon/ec2.py | 30 +++++++++++++++++-------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/ec2.py b/lib/ansible/modules/cloud/amazon/ec2.py index eb56684b044..31186393c13 100755 --- a/lib/ansible/modules/cloud/amazon/ec2.py +++ b/lib/ansible/modules/cloud/amazon/ec2.py @@ -1366,7 +1366,8 @@ def startstop_instances(module, ec2, instance_ids, state, instance_tags): exception=traceback.format_exc(exc)) # Check "termination_protection" attribute - if inst.get_attribute('disableApiTermination')['disableApiTermination'] != termination_protection: + if (inst.get_attribute('disableApiTermination')['disableApiTermination'] != termination_protection + and termination_protection is not None): inst.modify_attribute('disableApiTermination', termination_protection) changed = True @@ -1433,8 +1434,6 @@ def restart_instances(module, ec2, instance_ids, state, instance_tags): termination_protection = module.params.get('termination_protection') changed = False instance_dict_array = [] - source_dest_check = module.params.get('source_dest_check') - termination_protection = module.params.get('termination_protection') if not isinstance(instance_ids, list) or len(instance_ids) < 1: # Fail unless the user defined instance tags @@ -1452,17 +1451,30 @@ def restart_instances(module, ec2, instance_ids, state, instance_tags): # Check that our instances are not in the state we want to take # Check (and eventually change) instances attributes and instances state - running_instances_array = [] for res in ec2.get_all_instances(instance_ids, filters=filters): for inst in res.instances: # Check "source_dest_check" attribute - if inst.get_attribute('sourceDestCheck')['sourceDestCheck'] != source_dest_check: - inst.modify_attribute('sourceDestCheck', source_dest_check) - changed = True + try: + if inst.vpc_id is not None and inst.get_attribute('sourceDestCheck')['sourceDestCheck'] != source_dest_check: + inst.modify_attribute('sourceDestCheck', source_dest_check) + changed = True + except boto.exception.EC2ResponseError as exc: + # instances with more than one Elastic Network Interface will + # fail, because they have the sourceDestCheck attribute defined + # per-interface + if exc.code == 'InvalidInstanceID': + for interface in inst.interfaces: + if interface.source_dest_check != source_dest_check: + ec2.modify_network_interface_attribute(interface.id, "sourceDestCheck", source_dest_check) + changed = True + else: + module.fail_json(msg='Failed to handle source_dest_check state for instance {0}, error: {1}'.format(inst.id, exc), + exception=traceback.format_exc(exc)) # Check "termination_protection" attribute - if inst.get_attribute('disableApiTermination')['disableApiTermination'] != termination_protection: + if (inst.get_attribute('disableApiTermination')['disableApiTermination'] != termination_protection + and termination_protection is not None): inst.modify_attribute('disableApiTermination', termination_protection) changed = True @@ -1507,7 +1519,7 @@ def main(): instance_profile_name = dict(), instance_ids = dict(type='list', aliases=['instance_id']), source_dest_check = dict(type='bool', default=True), - termination_protection = dict(type='bool', default=False), + termination_protection = dict(type='bool', default=None), state = dict(default='present', choices=['present', 'absent', 'running', 'restarted', 'stopped']), instance_initiated_shutdown_behavior=dict(default=None, choices=['stop', 'terminate']), exact_count = dict(type='int', default=None),