From 939bd1cd86ad62b4e33b937a8f21253664f085d0 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Mon, 24 Feb 2020 13:43:19 +0000 Subject: [PATCH 1/2] EC2 - Add some filters for describe_instance_status --- moto/ec2/models.py | 39 +++++++++++++++------ moto/ec2/responses/instances.py | 19 ++++++++-- tests/test_ec2/test_instances.py | 59 +++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 15 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 166d8e646..ef506e443 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -822,6 +822,21 @@ class Instance(TaggedEC2Resource, BotoInstance): return self.public_ip raise UnformattedGetAttTemplateException() + def applies(self, filters): + if filters: + applicable = False + for f in filters: + acceptable_values = f['values'] + if f['name'] == "instance-state-name": + if self._state.name in acceptable_values: + applicable = True + if f['name'] == "instance-state-code": + if str(self._state.code) in acceptable_values: + applicable = True + return applicable + # If there are no filters, all instances are valid + return True + class InstanceBackend(object): def __init__(self): @@ -921,22 +936,23 @@ class InstanceBackend(object): value = getattr(instance, key) return instance, value - def all_instances(self): + def all_instances(self, filters=None): instances = [] for reservation in self.all_reservations(): for instance in reservation.instances: - instances.append(instance) - return instances - - def all_running_instances(self): - instances = [] - for reservation in self.all_reservations(): - for instance in reservation.instances: - if instance.state_code == 16: + if instance.applies(filters): instances.append(instance) return instances - def get_multi_instances_by_id(self, instance_ids): + def all_running_instances(self, filters=None): + instances = [] + for reservation in self.all_reservations(): + for instance in reservation.instances: + if instance.state_code == 16 and instance.applies(filters): + instances.append(instance) + return instances + + def get_multi_instances_by_id(self, instance_ids, filters=None): """ :param instance_ids: A string list with instance ids :return: A list with instance objects @@ -946,7 +962,8 @@ class InstanceBackend(object): for reservation in self.all_reservations(): for instance in reservation.instances: if instance.id in instance_ids: - result.append(instance) + if instance.applies(filters): + result.append(instance) # TODO: Trim error message down to specific invalid id. if instance_ids and len(instance_ids) > len(result): diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index b9e572d29..9b1105291 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -113,16 +113,29 @@ class InstanceResponse(BaseResponse): template = self.response_template(EC2_START_INSTANCES) return template.render(instances=instances) + def _get_list_of_dict_params(self, param_prefix, _dct): + """ + Simplified version of _get_dict_param + Allows you to pass in a custom dict instead of using self.querystring by default + """ + params = [] + for key, value in _dct.items(): + if key.startswith(param_prefix): + params.append(value) + return params + def describe_instance_status(self): instance_ids = self._get_multi_param("InstanceId") include_all_instances = self._get_param("IncludeAllInstances") == "true" + filters = self._get_list_prefix("Filter") + filters = [{'name': f['name'], 'values': self._get_list_of_dict_params("value.", f)} for f in filters] if instance_ids: - instances = self.ec2_backend.get_multi_instances_by_id(instance_ids) + instances = self.ec2_backend.get_multi_instances_by_id(instance_ids, filters) elif include_all_instances: - instances = self.ec2_backend.all_instances() + instances = self.ec2_backend.all_instances(filters) else: - instances = self.ec2_backend.all_running_instances() + instances = self.ec2_backend.all_running_instances(filters) template = self.response_template(EC2_INSTANCE_STATUS) return template.render(instances=instances) diff --git a/tests/test_ec2/test_instances.py b/tests/test_ec2/test_instances.py index 041bc8c85..ac6a4f4ec 100644 --- a/tests/test_ec2/test_instances.py +++ b/tests/test_ec2/test_instances.py @@ -1144,7 +1144,7 @@ def test_describe_instance_status_with_instances(): @mock_ec2_deprecated -def test_describe_instance_status_with_instance_filter(): +def test_describe_instance_status_with_instance_filter_deprecated(): conn = boto.connect_ec2("the_key", "the_secret") # We want to filter based on this one @@ -1166,6 +1166,63 @@ def test_describe_instance_status_with_instance_filter(): cm.exception.request_id.should_not.be.none +@mock_ec2 +def test_describe_instance_status_with_instance_filter(): + conn = boto3.client("ec2", region_name="us-west-1") + + # We want to filter based on this one + reservation = conn.run_instances(ImageId="ami-1234abcd", MinCount=3, MaxCount=3) + instance1 = reservation['Instances'][0] + instance2 = reservation['Instances'][1] + instance3 = reservation['Instances'][2] + conn.stop_instances(InstanceIds=[instance1['InstanceId']]) + stopped_instance_ids = [instance1['InstanceId']] + running_instance_ids = sorted([instance2['InstanceId'], instance3['InstanceId']]) + all_instance_ids = sorted(stopped_instance_ids + running_instance_ids) + + # Filter instance using the state name + state_name_filter = { + "running_and_stopped": [ + {"Name": "instance-state-name", "Values": ["running", "stopped"]} + ], + "running": [{"Name": "instance-state-name", "Values": ["running"]}], + "stopped": [{"Name": "instance-state-name", "Values": ["stopped"]}], + } + + found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_name_filter["running_and_stopped"])['InstanceStatuses'] + found_instance_ids = [status['InstanceId'] for status in found_statuses] + sorted(found_instance_ids).should.equal(all_instance_ids) + + found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_name_filter["running"])['InstanceStatuses'] + found_instance_ids = [status['InstanceId'] for status in found_statuses] + sorted(found_instance_ids).should.equal(running_instance_ids) + + found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_name_filter["stopped"])['InstanceStatuses'] + found_instance_ids = [status['InstanceId'] for status in found_statuses] + sorted(found_instance_ids).should.equal(stopped_instance_ids) + + # Filter instance using the state code + state_code_filter = { + "running_and_stopped": [ + {"Name": "instance-state-code", "Values": ["16", "80"]} + ], + "running": [{"Name": "instance-state-code", "Values": ["16"]}], + "stopped": [{"Name": "instance-state-code", "Values": ["80"]}], + } + + found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_code_filter["running_and_stopped"])['InstanceStatuses'] + found_instance_ids = [status['InstanceId'] for status in found_statuses] + sorted(found_instance_ids).should.equal(all_instance_ids) + + found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_code_filter["running"])['InstanceStatuses'] + found_instance_ids = [status['InstanceId'] for status in found_statuses] + sorted(found_instance_ids).should.equal(running_instance_ids) + + found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_code_filter["stopped"])['InstanceStatuses'] + found_instance_ids = [status['InstanceId'] for status in found_statuses] + sorted(found_instance_ids).should.equal(stopped_instance_ids) + + @requires_boto_gte("2.32.0") @mock_ec2_deprecated def test_describe_instance_status_with_non_running_instances(): From 3aeb5f504319fd901482bc7f3428b3e27774c217 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Mon, 24 Feb 2020 13:43:58 +0000 Subject: [PATCH 2/2] Linting --- moto/ec2/models.py | 6 ++-- moto/ec2/responses/instances.py | 9 ++++-- tests/test_ec2/test_instances.py | 48 ++++++++++++++++++++------------ 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index ef506e443..9c720cda8 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -826,11 +826,11 @@ class Instance(TaggedEC2Resource, BotoInstance): if filters: applicable = False for f in filters: - acceptable_values = f['values'] - if f['name'] == "instance-state-name": + acceptable_values = f["values"] + if f["name"] == "instance-state-name": if self._state.name in acceptable_values: applicable = True - if f['name'] == "instance-state-code": + if f["name"] == "instance-state-code": if str(self._state.code) in acceptable_values: applicable = True return applicable diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py index 9b1105291..29c346f82 100644 --- a/moto/ec2/responses/instances.py +++ b/moto/ec2/responses/instances.py @@ -128,10 +128,15 @@ class InstanceResponse(BaseResponse): instance_ids = self._get_multi_param("InstanceId") include_all_instances = self._get_param("IncludeAllInstances") == "true" filters = self._get_list_prefix("Filter") - filters = [{'name': f['name'], 'values': self._get_list_of_dict_params("value.", f)} for f in filters] + filters = [ + {"name": f["name"], "values": self._get_list_of_dict_params("value.", f)} + for f in filters + ] if instance_ids: - instances = self.ec2_backend.get_multi_instances_by_id(instance_ids, filters) + instances = self.ec2_backend.get_multi_instances_by_id( + instance_ids, filters + ) elif include_all_instances: instances = self.ec2_backend.all_instances(filters) else: diff --git a/tests/test_ec2/test_instances.py b/tests/test_ec2/test_instances.py index ac6a4f4ec..85ba0fe01 100644 --- a/tests/test_ec2/test_instances.py +++ b/tests/test_ec2/test_instances.py @@ -1172,12 +1172,12 @@ def test_describe_instance_status_with_instance_filter(): # We want to filter based on this one reservation = conn.run_instances(ImageId="ami-1234abcd", MinCount=3, MaxCount=3) - instance1 = reservation['Instances'][0] - instance2 = reservation['Instances'][1] - instance3 = reservation['Instances'][2] - conn.stop_instances(InstanceIds=[instance1['InstanceId']]) - stopped_instance_ids = [instance1['InstanceId']] - running_instance_ids = sorted([instance2['InstanceId'], instance3['InstanceId']]) + instance1 = reservation["Instances"][0] + instance2 = reservation["Instances"][1] + instance3 = reservation["Instances"][2] + conn.stop_instances(InstanceIds=[instance1["InstanceId"]]) + stopped_instance_ids = [instance1["InstanceId"]] + running_instance_ids = sorted([instance2["InstanceId"], instance3["InstanceId"]]) all_instance_ids = sorted(stopped_instance_ids + running_instance_ids) # Filter instance using the state name @@ -1189,16 +1189,22 @@ def test_describe_instance_status_with_instance_filter(): "stopped": [{"Name": "instance-state-name", "Values": ["stopped"]}], } - found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_name_filter["running_and_stopped"])['InstanceStatuses'] - found_instance_ids = [status['InstanceId'] for status in found_statuses] + found_statuses = conn.describe_instance_status( + IncludeAllInstances=True, Filters=state_name_filter["running_and_stopped"] + )["InstanceStatuses"] + found_instance_ids = [status["InstanceId"] for status in found_statuses] sorted(found_instance_ids).should.equal(all_instance_ids) - found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_name_filter["running"])['InstanceStatuses'] - found_instance_ids = [status['InstanceId'] for status in found_statuses] + found_statuses = conn.describe_instance_status( + IncludeAllInstances=True, Filters=state_name_filter["running"] + )["InstanceStatuses"] + found_instance_ids = [status["InstanceId"] for status in found_statuses] sorted(found_instance_ids).should.equal(running_instance_ids) - found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_name_filter["stopped"])['InstanceStatuses'] - found_instance_ids = [status['InstanceId'] for status in found_statuses] + found_statuses = conn.describe_instance_status( + IncludeAllInstances=True, Filters=state_name_filter["stopped"] + )["InstanceStatuses"] + found_instance_ids = [status["InstanceId"] for status in found_statuses] sorted(found_instance_ids).should.equal(stopped_instance_ids) # Filter instance using the state code @@ -1210,16 +1216,22 @@ def test_describe_instance_status_with_instance_filter(): "stopped": [{"Name": "instance-state-code", "Values": ["80"]}], } - found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_code_filter["running_and_stopped"])['InstanceStatuses'] - found_instance_ids = [status['InstanceId'] for status in found_statuses] + found_statuses = conn.describe_instance_status( + IncludeAllInstances=True, Filters=state_code_filter["running_and_stopped"] + )["InstanceStatuses"] + found_instance_ids = [status["InstanceId"] for status in found_statuses] sorted(found_instance_ids).should.equal(all_instance_ids) - found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_code_filter["running"])['InstanceStatuses'] - found_instance_ids = [status['InstanceId'] for status in found_statuses] + found_statuses = conn.describe_instance_status( + IncludeAllInstances=True, Filters=state_code_filter["running"] + )["InstanceStatuses"] + found_instance_ids = [status["InstanceId"] for status in found_statuses] sorted(found_instance_ids).should.equal(running_instance_ids) - found_statuses = conn.describe_instance_status(IncludeAllInstances=True, Filters=state_code_filter["stopped"])['InstanceStatuses'] - found_instance_ids = [status['InstanceId'] for status in found_statuses] + found_statuses = conn.describe_instance_status( + IncludeAllInstances=True, Filters=state_code_filter["stopped"] + )["InstanceStatuses"] + found_instance_ids = [status["InstanceId"] for status in found_statuses] sorted(found_instance_ids).should.equal(stopped_instance_ids)