From cb4cbd1f5bcee818cd55b08f48f35a4202f1cd7f Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Mon, 2 May 2022 15:00:06 +0000 Subject: [PATCH] EC2 - Spot Requests improvements (#5087) --- moto/ec2/_models/instances.py | 4 +- moto/ec2/_models/spot_requests.py | 26 ++++++- moto/ec2/responses/spot_fleets.py | 4 + moto/ec2/responses/spot_instances.py | 17 +++-- .../etc/0002-EC2-reduce-wait-times.patch | 4 +- .../terraform-tests.success.txt | 2 + tests/test_ec2/test_spot_instances.py | 76 +++++++++++-------- 7 files changed, 90 insertions(+), 43 deletions(-) diff --git a/moto/ec2/_models/instances.py b/moto/ec2/_models/instances.py index d99a47316..634b64673 100644 --- a/moto/ec2/_models/instances.py +++ b/moto/ec2/_models/instances.py @@ -99,8 +99,8 @@ class Instance(TaggedEC2Resource, BotoInstance, CloudFormationModel): self.launch_time = utc_date_and_time() self.ami_launch_index = kwargs.get("ami_launch_index", 0) self.disable_api_termination = kwargs.get("disable_api_termination", False) - self.instance_initiated_shutdown_behavior = kwargs.get( - "instance_initiated_shutdown_behavior", "stop" + self.instance_initiated_shutdown_behavior = ( + kwargs.get("instance_initiated_shutdown_behavior") or "stop" ) self.sriov_net_support = "simple" self._spot_fleet_id = kwargs.get("spot_fleet_id", None) diff --git a/moto/ec2/_models/spot_requests.py b/moto/ec2/_models/spot_requests.py index 3a753e316..7dc8d59cd 100644 --- a/moto/ec2/_models/spot_requests.py +++ b/moto/ec2/_models/spot_requests.py @@ -38,6 +38,7 @@ class SpotInstanceRequest(BotoSpotRequest, TaggedEC2Resource): subnet_id, tags, spot_fleet_id, + instance_interruption_behaviour, **kwargs, ): super().__init__(**kwargs) @@ -46,12 +47,20 @@ class SpotInstanceRequest(BotoSpotRequest, TaggedEC2Resource): self.launch_specification = ls self.id = spot_request_id self.state = "open" + self.status = "pending-evaluation" + self.status_message = "Your Spot request has been submitted for review, and is pending evaluation." + if price: + price = float(price) + price = "{0:.6f}".format(price) # round up/down to 6 decimals self.price = price self.type = spot_instance_type self.valid_from = valid_from self.valid_until = valid_until self.launch_group = launch_group self.availability_zone_group = availability_zone_group + self.instance_interruption_behaviour = ( + instance_interruption_behaviour or "terminate" + ) self.user_data = user_data # NOT ls.kernel = kernel_id ls.ramdisk = ramdisk_id @@ -62,7 +71,9 @@ class SpotInstanceRequest(BotoSpotRequest, TaggedEC2Resource): ls.monitored = monitoring_enabled ls.subnet_id = subnet_id self.spot_fleet_id = spot_fleet_id - self.tags = tags + tag_map = tags.get("spot-instances-request", {}) + self.add_tags(tag_map) + self.all_tags = tags if security_groups: for group_name in security_groups: @@ -75,6 +86,9 @@ class SpotInstanceRequest(BotoSpotRequest, TaggedEC2Resource): ls.groups.append(default_group) self.instance = self.launch_instance() + self.state = "active" + self.status = "fulfilled" + self.status_message = "" def get_filter_value(self, filter_name): if filter_name == "state": @@ -95,7 +109,7 @@ class SpotInstanceRequest(BotoSpotRequest, TaggedEC2Resource): security_group_names=[], security_group_ids=self.launch_specification.groups, spot_fleet_id=self.spot_fleet_id, - tags=self.tags, + tags=self.all_tags, lifecycle="spot", ) instance = reservation.instances[0] @@ -128,6 +142,7 @@ class SpotRequestBackend(object, metaclass=Model): subnet_id, tags=None, spot_fleet_id=None, + instance_interruption_behaviour=None, ): requests = [] tags = tags or {} @@ -154,6 +169,7 @@ class SpotRequestBackend(object, metaclass=Model): subnet_id, tags, spot_fleet_id, + instance_interruption_behaviour, ) self.spot_instance_requests[spot_request_id] = request requests.append(request) @@ -216,6 +232,7 @@ class SpotFleetRequest(TaggedEC2Resource, CloudFormationModel): allocation_strategy, launch_specs, launch_template_config, + instance_interruption_behaviour, ): self.ec2_backend = ec2_backend @@ -224,6 +241,9 @@ class SpotFleetRequest(TaggedEC2Resource, CloudFormationModel): self.target_capacity = int(target_capacity) self.iam_fleet_role = iam_fleet_role self.allocation_strategy = allocation_strategy + self.instance_interruption_behaviour = ( + instance_interruption_behaviour or "terminate" + ) self.state = "active" self.fulfilled_capacity = 0.0 @@ -404,6 +424,7 @@ class SpotFleetBackend(object): allocation_strategy, launch_specs, launch_template_config=None, + instance_interruption_behaviour=None, ): spot_fleet_request_id = random_spot_fleet_request_id() @@ -416,6 +437,7 @@ class SpotFleetBackend(object): allocation_strategy, launch_specs, launch_template_config, + instance_interruption_behaviour, ) self.spot_fleet_requests[spot_fleet_request_id] = request return request diff --git a/moto/ec2/responses/spot_fleets.py b/moto/ec2/responses/spot_fleets.py index c38c08dee..c32ad9e19 100644 --- a/moto/ec2/responses/spot_fleets.py +++ b/moto/ec2/responses/spot_fleets.py @@ -47,6 +47,9 @@ class SpotFleets(BaseResponse): target_capacity = spot_config["TargetCapacity"] iam_fleet_role = spot_config["IamFleetRole"] allocation_strategy = spot_config["AllocationStrategy"] + instance_interruption_behaviour = spot_config.get( + "InstanceInterruptionBehavior" + ) launch_specs = spot_config.get("LaunchSpecifications") launch_template_config = list( @@ -63,6 +66,7 @@ class SpotFleets(BaseResponse): allocation_strategy=allocation_strategy, launch_specs=launch_specs, launch_template_config=launch_template_config, + instance_interruption_behaviour=instance_interruption_behaviour, ) template = self.response_template(REQUEST_SPOT_FLEET_TEMPLATE) diff --git a/moto/ec2/responses/spot_instances.py b/moto/ec2/responses/spot_instances.py index 56286b82f..05de98eb9 100644 --- a/moto/ec2/responses/spot_instances.py +++ b/moto/ec2/responses/spot_instances.py @@ -62,6 +62,10 @@ class SpotInstances(EC2BaseResponse): ramdisk_id = self._get_param("LaunchSpecification.RamdiskId") monitoring_enabled = self._get_param("LaunchSpecification.Monitoring.Enabled") subnet_id = self._get_param("LaunchSpecification.SubnetId") + instance_interruption_behaviour = self._get_param( + "InstanceInterruptionBehavior" + ) + tags = self._parse_tag_specification() if self.is_not_dryrun("RequestSpotInstance"): requests = self.ec2_backend.request_spot_instances( @@ -82,6 +86,8 @@ class SpotInstances(EC2BaseResponse): ramdisk_id=ramdisk_id, monitoring_enabled=monitoring_enabled, subnet_id=subnet_id, + instance_interruption_behaviour=instance_interruption_behaviour, + tags=tags, ) template = self.response_template(REQUEST_SPOT_INSTANCES_TEMPLATE) @@ -98,9 +104,9 @@ REQUEST_SPOT_INSTANCES_TEMPLATE = """{{ request.type }} {{ request.state }} - pending-evaluation + {{ request.status }} 2015-01-01T00:00:00.000Z - Your Spot request has been submitted for review, and is pending evaluation. + {{ request.status_message }} - {{ request.instance_id }} + {{ request.instance.id }} {% if request.availability_zone_group %} {{ request.availability_zone_group }} {% endif %} @@ -217,6 +223,7 @@ DESCRIBE_SPOT_INSTANCES_TEMPLATE = """{{ request.valid_until }} {% endif %} Linux/UNIX + {{ request.instance_interruption_behaviour }} {% endfor %} diff --git a/tests/terraformtests/etc/0002-EC2-reduce-wait-times.patch b/tests/terraformtests/etc/0002-EC2-reduce-wait-times.patch index 4b53a9cd2..ef599e580 100644 --- a/tests/terraformtests/etc/0002-EC2-reduce-wait-times.patch +++ b/tests/terraformtests/etc/0002-EC2-reduce-wait-times.patch @@ -342,7 +342,7 @@ index 49e4909b3a..731a37f253 100644 Timeouts: &schema.ResourceTimeout{ - Create: schema.DefaultTimeout(10 * time.Minute), - Delete: schema.DefaultTimeout(15 * time.Minute), -+ Create: schema.DefaultTimeout(10 * time.Second), ++ Create: schema.DefaultTimeout(20 * time.Second), + Delete: schema.DefaultTimeout(15 * time.Second), }, @@ -366,7 +366,7 @@ index e054f82987..08aeb6cf70 100644 Timeouts: &schema.ResourceTimeout{ - Create: schema.DefaultTimeout(10 * time.Minute), - Delete: schema.DefaultTimeout(20 * time.Minute), -+ Create: schema.DefaultTimeout(10 * time.Second), ++ Create: schema.DefaultTimeout(20 * time.Second), + Delete: schema.DefaultTimeout(20 * time.Second), }, diff --git a/tests/terraformtests/terraform-tests.success.txt b/tests/terraformtests/terraform-tests.success.txt index e77012a3c..2afba913a 100644 --- a/tests/terraformtests/terraform-tests.success.txt +++ b/tests/terraformtests/terraform-tests.success.txt @@ -40,6 +40,8 @@ ec2: - TestAccEC2InternetGateway_ - TestAccEC2NATGateway_ - TestAccEC2RouteTableAssociation_ + - TestAccEC2SpotInstanceRequest_disappears + - TestAccEC2SpotInstanceRequest_interruptUpdate - TestAccEC2VPCEndpointService_ - TestAccEC2VPNGateway_ - TestAccEC2VPNGatewayAttachment_ diff --git a/tests/test_ec2/test_spot_instances.py b/tests/test_ec2/test_spot_instances.py index ee145a58c..4e7f5bc03 100644 --- a/tests/test_ec2/test_spot_instances.py +++ b/tests/test_ec2/test_spot_instances.py @@ -7,7 +7,6 @@ import pytz import sure # noqa # pylint: disable=unused-import from moto import mock_ec2, settings -from moto.ec2.models import ec2_backends from moto.core.utils import iso_8601_datetime_with_milliseconds from tests import EXAMPLE_AMI_ID from uuid import uuid4 @@ -90,8 +89,8 @@ def test_request_spot_instances(): requests.should.have.length_of(1) request = requests[0] - request["State"].should.equal("open") - request["SpotPrice"].should.equal("0.5") + request["State"].should.equal("active") + request["SpotPrice"].should.equal("0.500000") request["Type"].should.equal("one-time") request["ValidFrom"].should.equal(start_dt) request["ValidUntil"].should.equal(end_dt) @@ -129,13 +128,14 @@ def test_request_spot_instances_default_arguments(): requests.should.have.length_of(1) request = requests[0] - request["State"].should.equal("open") - request["SpotPrice"].should.equal("0.5") + request["State"].should.equal("active") + request["SpotPrice"].should.equal("0.500000") request["Type"].should.equal("one-time") request.shouldnt.contain("ValidFrom") request.shouldnt.contain("ValidUntil") request.shouldnt.contain("LaunchGroup") request.shouldnt.contain("AvailabilityZoneGroup") + request.should.have.key("InstanceInterruptionBehavior").equals("terminate") launch_spec = request["LaunchSpecification"] @@ -153,7 +153,7 @@ def test_request_spot_instances_default_arguments(): @mock_ec2 -def test_cancel_spot_instance_request_boto3(): +def test_cancel_spot_instance_request(): client = boto3.client("ec2", region_name="us-west-1") rsi = client.request_spot_instances( @@ -169,7 +169,7 @@ def test_cancel_spot_instance_request_boto3(): request.should.have.key("CreateTime") request.should.have.key("Type").equal("one-time") request.should.have.key("SpotInstanceRequestId") - request.should.have.key("SpotPrice").equal("0.5") + request.should.have.key("SpotPrice").equal("0.500000") request["LaunchSpecification"]["ImageId"].should.equal(EXAMPLE_AMI_ID) with pytest.raises(ClientError) as ex: @@ -193,7 +193,7 @@ def test_cancel_spot_instance_request_boto3(): @mock_ec2 -def test_request_spot_instances_fulfilled_boto3(): +def test_request_spot_instances_fulfilled(): """ Test that moto correctly fullfills a spot instance request """ @@ -210,22 +210,11 @@ def test_request_spot_instances_fulfilled_boto3(): requests.should.have.length_of(1) request = requests[0] - request["State"].should.equal("open") - - if not settings.TEST_SERVER_MODE: - ec2_backends["us-east-1"].spot_instance_requests[request_id].state = "active" - - requests = client.describe_spot_instance_requests( - SpotInstanceRequestIds=[request_id] - )["SpotInstanceRequests"] - requests.should.have.length_of(1) - request = requests[0] - - request["State"].should.equal("active") + request["State"].should.equal("active") @mock_ec2 -def test_tag_spot_instance_request_boto3(): +def test_tag_spot_instance_request(): """ Test that moto correctly tags a spot instance request """ @@ -252,7 +241,7 @@ def test_tag_spot_instance_request_boto3(): @mock_ec2 -def test_get_all_spot_instance_requests_filtering_boto3(): +def test_get_all_spot_instance_requests_filtering(): """ Test that moto correctly filters spot instance requests """ @@ -280,14 +269,14 @@ def test_get_all_spot_instance_requests_filtering_boto3(): ) requests = client.describe_spot_instance_requests( - Filters=[{"Name": "state", "Values": ["active"]}] + Filters=[{"Name": "state", "Values": ["failed"]}] )["SpotInstanceRequests"] r_ids = [r["SpotInstanceRequestId"] for r in requests] r_ids.shouldnt.contain(request1_id) r_ids.shouldnt.contain(request2_id) requests = client.describe_spot_instance_requests( - Filters=[{"Name": "state", "Values": ["open"]}] + Filters=[{"Name": "state", "Values": ["active"]}] )["SpotInstanceRequests"] r_ids = [r["SpotInstanceRequestId"] for r in requests] r_ids.should.contain(request1_id) @@ -322,6 +311,27 @@ def test_request_spot_instances_instance_lifecycle(): instance["InstanceLifecycle"].should.equal("spot") +@mock_ec2 +def test_request_spot_instances_with_tags(): + client = boto3.client("ec2", region_name="us-east-1") + request = client.request_spot_instances( + SpotPrice="0.5", + TagSpecifications=[ + { + "ResourceType": "spot-instances-request", + "Tags": [{"Key": "k", "Value": "v"}], + } + ], + ) + + request_id = request["SpotInstanceRequests"][0]["SpotInstanceRequestId"] + + request = client.describe_spot_instance_requests( + SpotInstanceRequestIds=[request_id] + )["SpotInstanceRequests"][0] + request.should.have.key("Tags").equals([{"Key": "k", "Value": "v"}]) + + @mock_ec2 def test_launch_spot_instance_instance_lifecycle(): client = boto3.client("ec2", region_name="us-east-1") @@ -391,18 +401,20 @@ def test_spot_price_history(): @mock_ec2 -def test_request_spot_instances_setting_instance_id_boto3(): +def test_request_spot_instances__instance_should_exist(): client = boto3.client("ec2", region_name="us-east-1") request = client.request_spot_instances( SpotPrice="0.5", LaunchSpecification={"ImageId": EXAMPLE_AMI_ID} ) request_id = request["SpotInstanceRequests"][0]["SpotInstanceRequestId"] - if not settings.TEST_SERVER_MODE: - req = ec2_backends["us-east-1"].spot_instance_requests[request_id] - req.state = "active" - req.instance_id = "i-12345678" + request = client.describe_spot_instance_requests( + SpotInstanceRequestIds=[request_id] + )["SpotInstanceRequests"][0] + request.should.have.key("InstanceId") + instance_id = request["InstanceId"] - request = client.describe_spot_instance_requests()["SpotInstanceRequests"][0] - assert request["State"] == "active" - assert request["InstanceId"] == "i-12345678" + response = client.describe_instances(InstanceIds=[instance_id]) + instance = response["Reservations"][0]["Instances"][0] + instance.should.have.key("InstanceId").equals(instance_id) + instance.should.have.key("ImageId").equals(EXAMPLE_AMI_ID)