Repository: getmoto/moto
Pull request: Add support for BlockDeviceMappings argument
This PR adds a parser for BlockDeviceMappings argument that passed to `ec2_client.run_instances`

Fixes: https://github.com/spulec/moto/issues/418

Problem statement:
Add support for 'block_device-map' on instance creation
Currently on instance creation, only a "ghost" default block map is added, even if the 'block_device_map' argument is provided to 'run_instances'.

It would be good to have this default block device added to the EBSBackend instance, and better to respect the 'block_device_map' argument (adding to the instance block_device_mapping, and adding to the EBSBackend volumes member) on instance creation.

Hints from issue discussion:
Thanks for opening. This would be a great addition. Unfortunately, that is not a feature I currently use so it won't likely be high on my personal todo list. If someone else gets some time, a pull request would gladly be accepted.

Hi @spulec, just before starting to work on this one I want to make sure that duplicate issue wasn't already solved. Can you please confirm?
@sf-mkirilov correct; it looks like this hasn't been implemented yet.

Commit message: Add support for BlockDeviceMappings argument upon run_instances execution

Patch to review:
diff --git a/moto/ec2/models.py b/moto/ec2/models.py
index e94d2877c990..6bc0e70329c7 100644
--- a/moto/ec2/models.py
+++ b/moto/ec2/models.py
@@ -560,8 +560,10 @@ def __del__(self):
             # worst case we'll get IP address exaustion... rarely
             pass
 
-    def add_block_device(self, size, device_path):
-        volume = self.ec2_backend.create_volume(size, self.region_name)
+    def add_block_device(self, size, device_path, snapshot_id=None, encrypted=False):
+        volume = self.ec2_backend.create_volume(
+            size, self.region_name, snapshot_id, encrypted
+        )
         self.ec2_backend.attach_volume(volume.id, self.id, device_path)
 
     def setup_defaults(self):
@@ -891,8 +893,24 @@ def add_instances(self, image_id, count, user_data, security_group_names, **kwar
             new_instance.add_tags(instance_tags)
             if "block_device_mappings" in kwargs:
                 for block_device in kwargs["block_device_mappings"]:
+                    device_name = block_device["DeviceName"]
+                    volume_size = (
+                        block_device["Ebs"].get("VolumeSize")
+                        if "Ebs" in block_device
+                        else None
+                    )
+                    snapshot_id = (
+                        block_device["Ebs"].get("SnapshotId")
+                        if "Ebs" in block_device
+                        else None
+                    )
+                    encrypted = (
+                        block_device["Ebs"].get("Encrypted")
+                        if "Ebs" in block_device
+                        else False
+                    )
                     new_instance.add_block_device(
-                        block_device["Ebs"]["VolumeSize"], block_device["DeviceName"]
+                        volume_size, device_name, snapshot_id, encrypted
                     )
             else:
                 new_instance.setup_defaults()
diff --git a/moto/ec2/responses/instances.py b/moto/ec2/responses/instances.py
index de17f060931c..adcbfa741738 100644
--- a/moto/ec2/responses/instances.py
+++ b/moto/ec2/responses/instances.py
@@ -4,10 +4,16 @@
 from moto.autoscaling import autoscaling_backends
 from moto.core.responses import BaseResponse
 from moto.core.utils import camelcase_to_underscores
-from moto.ec2.utils import filters_from_querystring, dict_from_querystring
+from moto.ec2.exceptions import MissingParameterError
+from moto.ec2.utils import (
+    filters_from_querystring,
+    dict_from_querystring,
+)
 from moto.elbv2 import elbv2_backends
 from moto.core import ACCOUNT_ID
 
+from copy import deepcopy
+
 
 class InstanceResponse(BaseResponse):
     def describe_instances(self):
@@ -44,40 +50,31 @@ def run_instances(self):
         owner_id = self._get_param("OwnerId")
         user_data = self._get_param("UserData")
         security_group_names = self._get_multi_param("SecurityGroup")
-        security_group_ids = self._get_multi_param("SecurityGroupId")
-        nics = dict_from_querystring("NetworkInterface", self.querystring)
-        instance_type = self._get_param("InstanceType", if_none="m1.small")
-        placement = self._get_param("Placement.AvailabilityZone")
-        subnet_id = self._get_param("SubnetId")
-        private_ip = self._get_param("PrivateIpAddress")
-        associate_public_ip = self._get_param("AssociatePublicIpAddress")
-        key_name = self._get_param("KeyName")
-        ebs_optimized = self._get_param("EbsOptimized") or False
-        instance_initiated_shutdown_behavior = self._get_param(
-            "InstanceInitiatedShutdownBehavior"
-        )
-        tags = self._parse_tag_specification("TagSpecification")
-        region_name = self.region
+        kwargs = {
+            "instance_type": self._get_param("InstanceType", if_none="m1.small"),
+            "placement": self._get_param("Placement.AvailabilityZone"),
+            "region_name": self.region,
+            "subnet_id": self._get_param("SubnetId"),
+            "owner_id": owner_id,
+            "key_name": self._get_param("KeyName"),
+            "security_group_ids": self._get_multi_param("SecurityGroupId"),
+            "nics": dict_from_querystring("NetworkInterface", self.querystring),
+            "private_ip": self._get_param("PrivateIpAddress"),
+            "associate_public_ip": self._get_param("AssociatePublicIpAddress"),
+            "tags": self._parse_tag_specification("TagSpecification"),
+            "ebs_optimized": self._get_param("EbsOptimized") or False,
+            "instance_initiated_shutdown_behavior": self._get_param(
+                "InstanceInitiatedShutdownBehavior"
+            ),
+        }
+
+        mappings = self._parse_block_device_mapping()
+        if mappings:
+            kwargs["block_device_mappings"] = mappings
 
         if self.is_not_dryrun("RunInstance"):
             new_reservation = self.ec2_backend.add_instances(
-                image_id,
-                min_count,
-                user_data,
-                security_group_names,
-                instance_type=instance_type,
-                placement=placement,
-                region_name=region_name,
-                subnet_id=subnet_id,
-                owner_id=owner_id,
-                key_name=key_name,
-                security_group_ids=security_group_ids,
-                nics=nics,
-                private_ip=private_ip,
-                associate_public_ip=associate_public_ip,
-                tags=tags,
-                ebs_optimized=ebs_optimized,
-                instance_initiated_shutdown_behavior=instance_initiated_shutdown_behavior,
+                image_id, min_count, user_data, security_group_names, **kwargs
             )
 
             template = self.response_template(EC2_RUN_INSTANCES)
@@ -272,6 +269,58 @@ def _security_grp_instance_attribute_handler(self):
             )
             return EC2_MODIFY_INSTANCE_ATTRIBUTE
 
+    def _parse_block_device_mapping(self):
+        device_mappings = self._get_list_prefix("BlockDeviceMapping")
+        mappings = []
+        for device_mapping in device_mappings:
+            self._validate_block_device_mapping(device_mapping)
+            device_template = deepcopy(BLOCK_DEVICE_MAPPING_TEMPLATE)
+            device_template["VirtualName"] = device_mapping.get("virtual_name")
+            device_template["DeviceName"] = device_mapping.get("device_name")
+            device_template["Ebs"]["SnapshotId"] = device_mapping.get(
+                "ebs._snapshot_id"
+            )
+            device_template["Ebs"]["VolumeSize"] = device_mapping.get(
+                "ebs._volume_size"
+            )
+            device_template["Ebs"]["DeleteOnTermination"] = device_mapping.get(
+                "ebs._delete_on_termination", False
+            )
+            device_template["Ebs"]["VolumeType"] = device_mapping.get(
+                "ebs._volume_type"
+            )
+            device_template["Ebs"]["Iops"] = device_mapping.get("ebs._iops")
+            device_template["Ebs"]["Encrypted"] = device_mapping.get(
+                "ebs._encrypted", False
+            )
+            mappings.append(device_template)
+
+        return mappings
+
+    @staticmethod
+    def _validate_block_device_mapping(device_mapping):
+
+        if not any(mapping for mapping in device_mapping if mapping.startswith("ebs.")):
+            raise MissingParameterError("ebs")
+        if (
+            "ebs._volume_size" not in device_mapping
+            and "ebs._snapshot_id" not in device_mapping
+        ):
+            raise MissingParameterError("size or snapshotId")
+
+
+BLOCK_DEVICE_MAPPING_TEMPLATE = {
+    "VirtualName": None,
+    "DeviceName": None,
+    "Ebs": {
+        "SnapshotId": None,
+        "VolumeSize": None,
+        "DeleteOnTermination": None,
+        "VolumeType": None,
+        "Iops": None,
+        "Encrypted": None,
+    },
+}
 
 EC2_RUN_INSTANCES = (
     """<RunInstancesResponse xmlns="http://ec2.amazonaws.com/doc/2013-10-15/">
diff --git a/tests/test_ec2/test_instances.py b/tests/test_ec2/test_instances.py
index 0509e1a45115..d53bd14aaa3d 100644
--- a/tests/test_ec2/test_instances.py
+++ b/tests/test_ec2/test_instances.py
@@ -1126,6 +1126,111 @@ def test_run_instance_with_keypair():
     instance.key_name.should.equal("keypair_name")
 
 
+@mock_ec2
+def test_run_instance_with_block_device_mappings():
+    ec2_client = boto3.client("ec2", region_name="us-east-1")
+
+    kwargs = {
+        "MinCount": 1,
+        "MaxCount": 1,
+        "ImageId": "ami-d3adb33f",
+        "KeyName": "the_key",
+        "InstanceType": "t1.micro",
+        "BlockDeviceMappings": [{"DeviceName": "/dev/sda2", "Ebs": {"VolumeSize": 50}}],
+    }
+
+    ec2_client.run_instances(**kwargs)
+
+    instances = ec2_client.describe_instances()
+    volume = instances["Reservations"][0]["Instances"][0]["BlockDeviceMappings"][0][
+        "Ebs"
+    ]
+
+    volumes = ec2_client.describe_volumes(VolumeIds=[volume["VolumeId"]])
+    volumes["Volumes"][0]["Size"].should.equal(50)
+
+
+@mock_ec2
+def test_run_instance_with_block_device_mappings_missing_ebs():
+    ec2_client = boto3.client("ec2", region_name="us-east-1")
+
+    kwargs = {
+        "MinCount": 1,
+        "MaxCount": 1,
+        "ImageId": "ami-d3adb33f",
+        "KeyName": "the_key",
+        "InstanceType": "t1.micro",
+        "BlockDeviceMappings": [{"DeviceName": "/dev/sda2"}],
+    }
+    with assert_raises(ClientError) as ex:
+        ec2_client.run_instances(**kwargs)
+
+    ex.exception.response["Error"]["Code"].should.equal("MissingParameter")
+    ex.exception.response["ResponseMetadata"]["HTTPStatusCode"].should.equal(400)
+    ex.exception.response["Error"]["Message"].should.equal(
+        "The request must contain the parameter ebs"
+    )
+
+
+@mock_ec2
+def test_run_instance_with_block_device_mappings_missing_size():
+    ec2_client = boto3.client("ec2", region_name="us-east-1")
+
+    kwargs = {
+        "MinCount": 1,
+        "MaxCount": 1,
+        "ImageId": "ami-d3adb33f",
+        "KeyName": "the_key",
+        "InstanceType": "t1.micro",
+        "BlockDeviceMappings": [
+            {"DeviceName": "/dev/sda2", "Ebs": {"VolumeType": "standard"}}
+        ],
+    }
+    with assert_raises(ClientError) as ex:
+        ec2_client.run_instances(**kwargs)
+
+    ex.exception.response["Error"]["Code"].should.equal("MissingParameter")
+    ex.exception.response["ResponseMetadata"]["HTTPStatusCode"].should.equal(400)
+    ex.exception.response["Error"]["Message"].should.equal(
+        "The request must contain the parameter size or snapshotId"
+    )
+
+
+@mock_ec2
+def test_run_instance_with_block_device_mappings_from_snapshot():
+    ec2_client = boto3.client("ec2", region_name="us-east-1")
+    ec2_resource = boto3.resource("ec2", region_name="us-east-1")
+    volume_details = {
+        "AvailabilityZone": "1a",
+        "Size": 30,
+    }
+
+    volume = ec2_resource.create_volume(**volume_details)
+    snapshot = volume.create_snapshot()
+    kwargs = {
+        "MinCount": 1,
+        "MaxCount": 1,
+        "ImageId": "ami-d3adb33f",
+        "KeyName": "the_key",
+        "InstanceType": "t1.micro",
+        "BlockDeviceMappings": [
+            {"DeviceName": "/dev/sda2", "Ebs": {"SnapshotId": snapshot.snapshot_id}}
+        ],
+    }
+
+    ec2_client.run_instances(**kwargs)
+
+    instances = ec2_client.describe_instances()
+    volume = instances["Reservations"][0]["Instances"][0]["BlockDeviceMappings"][0][
+        "Ebs"
+    ]
+
+    volumes = ec2_client.describe_volumes(VolumeIds=[volume["VolumeId"]])
+
+    volumes["Volumes"][0]["Size"].should.equal(30)
+    volumes["Volumes"][0]["SnapshotId"].should.equal(snapshot.snapshot_id)
+
+
 @mock_ec2_deprecated
 def test_describe_instance_status_no_instances():
     conn = boto.connect_ec2("the_key", "the_secret")


Code review comments:
diff_hunk: @@ -891,8 +893,24 @@ def add_instances(self, image_id, count, user_data, security_group_names, **kwar
             new_instance.add_tags(instance_tags)
             if "block_device_mappings" in kwargs:
                 for block_device in kwargs["block_device_mappings"]:
+                    device_name = block_device["DeviceName"]
+                    volume_size = (
path: moto/ec2/models.py
text: @user2:
Thanks for adding the test cases, @user1-mkirilov. I guess we can remove the if-statements here, now that validation is in place that EBS will also be present? 
Happy to merge after that

@author:
@user2, just did.