From 19c3b07cbf4ca9a646c935e3f2c408dfada60beb Mon Sep 17 00:00:00 2001
From: Ryan Brown <sb@ryansb.com>
Date: Mon, 3 Oct 2016 13:23:42 -0400
Subject: 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
```
---
 cloud/amazon/ec2.py | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/cloud/amazon/ec2.py b/cloud/amazon/ec2.py
index b6f30b25..bb0e9008 100644
--- a/cloud/amazon/ec2.py
+++ b/cloud/amazon/ec2.py
@@ -1369,7 +1369,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
 
@@ -1436,8 +1437,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
@@ -1455,17 +1454,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
 
@@ -1510,7 +1522,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),
-- 
cgit v1.2.1