Repository: chaostoolkit-incubator/chaostoolkit-aws

Issue:
Can't stop instances with explicit lifecycle
When an instance has no explicit lifecycle, the extension fails to stop it. This is usually the case of On-Demand or Reserved instances.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1cf8254..3e93e48 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,15 @@
 
 [Unreleased]: https://github.com/chaostoolkit-incubator/chaostoolkit-aws/compare/0.7.1...HEAD
 
+### Changed
+
+-   make sure instances with no explicit lifecycle can be stopped. They are
+    assumed to be in the `normal` lifecycle as per the last
+    [line of the AWS documentation][]. [#25][25]
+
+[25]: https://github.com/chaostoolkit-incubator/chaostoolkit-aws/issues/25
+[instlifecycledocs]: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-purchasing-options.html
+
 ## [0.7.1][]
 
 [0.7.1]: https://github.com/chaostoolkit-incubator/chaostoolkit-aws/compare/0.7.0...0.7.1
diff --git a/chaosaws/ec2/actions.py b/chaosaws/ec2/actions.py
index 230a1a3..c60a05d 100644
--- a/chaosaws/ec2/actions.py
+++ b/chaosaws/ec2/actions.py
@@ -46,7 +46,7 @@ def stop_instance(instance_id: str = None, az: str = None, force: bool = False,
             raise FailedActivity(
                 "No instances in availability zone: {}".format(az))
     else:
-        instance_types = get_instance_type_by_id(instance_id, client)
+        instance_types = get_instance_type_by_id([instance_id], client)
 
     logger.debug(
         "Picked EC2 instance '{}' from AZ '{}' to be stopped".format(
@@ -140,10 +140,17 @@ def get_instance_type_from_response(response: Dict) -> Dict:
 
     for reservation in response['Reservations']:
         for inst in reservation['Instances']:
-            if inst['InstanceLifecycle'] not in instances_type.keys():
+            # when this field is missing, we assume "normal"
+            # which means On-Demand or Reserved
+            # this seems what the last line of the docs imply at
+            # https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-purchasing-options.html
+            lifecycle = inst.get('InstanceLifecycle', 'normal')
+
+            if lifecycle not in instances_type.keys():
                 # adding empty list (value) for new instance type (key)
-                instances_type[inst['InstanceLifecycle']] = []
-            instances_type[inst['InstanceLifecycle']].append(
+                instances_type[lifecycle] = []
+
+            instances_type[lifecycle].append(
                 inst['InstanceId'])
 
     return instances_type
@@ -158,7 +165,10 @@ def get_spot_request_ids_from_response(response: Dict) -> List[str]:
 
     for reservation in response['Reservations']:
         for inst in reservation['Instances']:
-            if inst['InstanceLifecycle'] == 'spot':
+            # when this field is missing, we assume "normal"
+            # which means On-Demand or Reserved
+            lifecycle = inst.get('InstanceLifecycle', 'normal')
+            if lifecycle == 'spot':
                 spot_request_ids.append(inst['SpotInstanceRequestId'])
 
     return spot_request_ids


Test patch:
diff --git a/tests/ec2/test_ec2_actions.py b/tests/ec2/test_ec2_actions.py
index 1611aba..37baa7a 100644
--- a/tests/ec2/test_ec2_actions.py
+++ b/tests/ec2/test_ec2_actions.py
@@ -180,3 +180,34 @@ def test_stop_instance_by_specific_filters(aws_client):
     client.describe_instances.assert_called_with(Filters=called_filters)
     client.stop_instances.assert_called_with(
         InstanceIds=[inst_1_id], Force=False)
+
+
+@patch('chaosaws.ec2.actions.aws_client', autospec=True)
+def test_stop_instance_with_no_lifecycle(aws_client):
+    client = MagicMock()
+    aws_client.return_value = client
+    inst_id = "i-1234567890abcdef0"
+    client.describe_instances.return_value = {
+        'Reservations':
+        [{'Instances': [{'InstanceId': inst_id}]}]
+    }
+    stop_instance(inst_id)
+    client.stop_instances.assert_called_with(
+        InstanceIds=[inst_id], Force=False)
+
+
+@patch('chaosaws.ec2.actions.aws_client', autospec=True)
+def test_stop_normal_instance(aws_client):
+    client = MagicMock()
+    aws_client.return_value = client
+    inst_id = "i-1234567890abcdef0"
+    client.describe_instances.return_value = {
+        'Reservations':
+        [{'Instances': [{
+            'InstanceId': inst_id,
+            'InstanceLifecycle': 'normal'
+        }]}]
+    }
+    stop_instance(inst_id)
+    client.stop_instances.assert_called_with(
+        InstanceIds=[inst_id], Force=False)