Merge pull request #1304 from terrycain/ec2_instance_ip_improvement

Fixed a few ip issues

moto/awslambda/models.py

@@ -298,7 +298,12 @@ class LambdaFunction(BaseModel):
                         volumes=["{}:/var/task".format(data_vol.name)], environment=env_vars, detach=True, **run_kwargs)
                 finally:
                     if container:
-                        exit_code = container.wait()
+                        try:
+                            exit_code = container.wait(timeout=300)
+                        except requests.exceptions.ReadTimeout:
+                            exit_code = -1
+                            container.stop()
+                            container.kill()
                         output = container.logs(stdout=False, stderr=True)
                         output += container.logs(stdout=True, stderr=False)
                         container.remove()

moto/ec2/models.py

@@ -2,6 +2,7 @@ from __future__ import unicode_literals
 
 import copy
 import itertools
+import ipaddress
 import json
 import os
 import re
@@ -402,6 +403,10 @@ class Instance(TaggedEC2Resource, BotoInstance):
             subnet = ec2_backend.get_subnet(self.subnet_id)
             self.vpc_id = subnet.vpc_id
             self._placement.zone = subnet.availability_zone
+
+            if associate_public_ip is None:
+                # Mapping public ip hasnt been explicitly enabled or disabled
+                associate_public_ip = subnet.map_public_ip_on_launch == 'true'
         elif placement:
             self._placement.zone = placement
         else:
@@ -409,10 +414,22 @@ class Instance(TaggedEC2Resource, BotoInstance):
 
         self.block_device_mapping = BlockDeviceMapping()
 
-        self.prep_nics(kwargs.get("nics", {}),
+        self._private_ips = set()
-                       subnet_id=self.subnet_id,
+        self.prep_nics(
-                       private_ip=kwargs.get("private_ip"),
+            kwargs.get("nics", {}),
-                       associate_public_ip=associate_public_ip)
+            private_ip=kwargs.get("private_ip"),
+            associate_public_ip=associate_public_ip
+        )
+
+    def __del__(self):
+        try:
+            subnet = self.ec2_backend.get_subnet(self.subnet_id)
+            for ip in self._private_ips:
+                subnet.del_subnet_ip(ip)
+        except Exception:
+            # Its not "super" critical we clean this up, as reset will do this
+            # worst case we'll get IP address exaustion... rarely
+            pass
 
     def setup_defaults(self):
         # Default have an instance with root volume should you not wish to
@@ -547,14 +564,23 @@ class Instance(TaggedEC2Resource, BotoInstance):
         else:
             return self.security_groups
 
-    def prep_nics(self, nic_spec, subnet_id=None, private_ip=None, associate_public_ip=None):
+    def prep_nics(self, nic_spec, private_ip=None, associate_public_ip=None):
         self.nics = {}
 
-        if not private_ip:
+        if self.subnet_id:
+            subnet = self.ec2_backend.get_subnet(self.subnet_id)
+            if not private_ip:
+                private_ip = subnet.get_available_subnet_ip(instance=self)
+            else:
+                subnet.request_ip(private_ip, instance=self)
+
+            self._private_ips.add(private_ip)
+        elif private_ip is None:
+            # Preserve old behaviour if in EC2-Classic mode
             private_ip = random_private_ip()
 
         # Primary NIC defaults
-        primary_nic = {'SubnetId': subnet_id,
+        primary_nic = {'SubnetId': self.subnet_id,
                        'PrivateIpAddress': private_ip,
                        'AssociatePublicIpAddress': associate_public_ip}
         primary_nic = dict((k, v) for k, v in primary_nic.items() if v)
@@ -2114,10 +2140,17 @@ class Subnet(TaggedEC2Resource):
         self.id = subnet_id
         self.vpc_id = vpc_id
         self.cidr_block = cidr_block
+        self.cidr = ipaddress.ip_network(six.text_type(self.cidr_block))
         self._availability_zone = availability_zone
         self.default_for_az = default_for_az
         self.map_public_ip_on_launch = map_public_ip_on_launch
 
+        # Theory is we assign ip's as we go (as 16,777,214 usable IPs in a /8)
+        self._subnet_ip_generator = self.cidr.hosts()
+        self.reserved_ips = [six.next(self._subnet_ip_generator) for _ in range(0, 3)]  # Reserved by AWS
+        self._unused_ips = set()  # if instance is destroyed hold IP here for reuse
+        self._subnet_ips = {}  # has IP: instance
+
     @classmethod
     def create_from_cloudformation_json(cls, resource_name, cloudformation_json, region_name):
         properties = cloudformation_json['Properties']
@@ -2184,6 +2217,46 @@ class Subnet(TaggedEC2Resource):
                 '"Fn::GetAtt" : [ "{0}" , "AvailabilityZone" ]"')
         raise UnformattedGetAttTemplateException()
 
+    def get_available_subnet_ip(self, instance):
+        try:
+            new_ip = self._unused_ips.pop()
+        except KeyError:
+            new_ip = six.next(self._subnet_ip_generator)
+
+            # Skips any IP's if they've been manually specified
+            while str(new_ip) in self._subnet_ips:
+                new_ip = six.next(self._subnet_ip_generator)
+
+            if new_ip == self.cidr.broadcast_address:
+                raise StopIteration()  # Broadcast address cant be used obviously
+        # TODO StopIteration will be raised if no ip's available, not sure how aws handles this.
+
+        new_ip = str(new_ip)
+        self._subnet_ips[new_ip] = instance
+
+        return new_ip
+
+    def request_ip(self, ip, instance):
+        if ipaddress.ip_address(ip) not in self.cidr:
+            raise Exception('IP does not fall in the subnet CIDR of {0}'.format(self.cidr))
+
+        if ip in self._subnet_ips:
+            raise Exception('IP already in use')
+        try:
+            self._unused_ips.remove(ip)
+        except KeyError:
+            pass
+
+        self._subnet_ips[ip] = instance
+        return ip
+
+    def del_subnet_ip(self, ip):
+        try:
+            del self._subnet_ips[ip]
+            self._unused_ips.add(ip)
+        except KeyError:
+            pass  # Unknown IP
+
 
 class SubnetBackend(object):
     def __init__(self):

moto/rds2/models.py

@@ -704,7 +704,8 @@ class RDS2Backend(BaseBackend):
         if self.arn_regex.match(source_database_id):
             db_kwargs['region'] = self.region
 
-        replica = copy.deepcopy(primary)
+        # Shouldn't really copy here as the instance is duplicated. RDS replicas have different instances.
+        replica = copy.copy(primary)
         replica.update(db_kwargs)
         replica.set_as_replica()
         self.databases[database_id] = replica

tests/test_awslambda/test_lambda.py

@@ -488,6 +488,7 @@ def lambda_handler(event, context):
     assert 'FunctionError' in result
     assert result['FunctionError'] == 'Handled'
 
+
 @mock_lambda
 @mock_s3
 def test_tags():
@@ -554,6 +555,7 @@ def test_tags():
         TagKeys=['spam']
     )['ResponseMetadata']['HTTPStatusCode'].should.equal(204)
 
+
 @mock_lambda
 def test_tags_not_found():
     """
@@ -574,6 +576,7 @@ def test_tags_not_found():
         TagKeys=['spam']
     ).should.throw(botocore.client.ClientError)
 
+
 @mock_lambda
 def test_invoke_async_function():
     conn = boto3.client('lambda', 'us-west-2')
@@ -581,10 +584,8 @@ def test_invoke_async_function():
         FunctionName='testFunction',
         Runtime='python2.7',
         Role='test-iam-role',
-        Handler='lambda_function.handler',
+        Handler='lambda_function.lambda_handler',
-        Code={
+        Code={'ZipFile': get_test_zip_file1()},
-            'ZipFile': get_test_zip_file1(),
-        },
         Description='test lambda function',
         Timeout=3,
         MemorySize=128,
@@ -593,11 +594,12 @@ def test_invoke_async_function():
 
     success_result = conn.invoke_async(
         FunctionName='testFunction', 
-        InvokeArgs=json.dumps({ 'test': 'event' })
+        InvokeArgs=json.dumps({'test': 'event'})
         )
 
     success_result['Status'].should.equal(202)
 
+
 @mock_lambda
 @freeze_time('2015-01-01 00:00:00')
 def test_get_function_created_with_zipfile():
@@ -646,6 +648,7 @@ def test_get_function_created_with_zipfile():
         },
     )
 
+
 @mock_lambda
 def add_function_permission():
     conn = boto3.client('lambda', 'us-west-2')

tests/test_ec2/test_instances.py

@@ -5,7 +5,9 @@ from nose.tools import assert_raises
 
 import base64
 import datetime
+import ipaddress
 
+import six
 import boto
 import boto3
 from boto.ec2.instance import Reservation, InstanceAttribute
@@ -413,6 +415,7 @@ def test_get_instances_filtering_by_image_id():
                                                        'Values': [image_id]}])['Reservations']
     reservations[0]['Instances'].should.have.length_of(1)
 
+
 @mock_ec2
 def test_get_instances_filtering_by_private_dns():
     image_id = 'ami-1234abcd'
@@ -427,6 +430,7 @@ def test_get_instances_filtering_by_private_dns():
     ])['Reservations']
     reservations[0]['Instances'].should.have.length_of(1)
 
+
 @mock_ec2
 def test_get_instances_filtering_by_ni_private_dns():
     image_id = 'ami-1234abcd'
@@ -441,6 +445,7 @@ def test_get_instances_filtering_by_ni_private_dns():
     ])['Reservations']
     reservations[0]['Instances'].should.have.length_of(1)
 
+
 @mock_ec2
 def test_get_instances_filtering_by_instance_group_name():
     image_id = 'ami-1234abcd'
@@ -458,6 +463,7 @@ def test_get_instances_filtering_by_instance_group_name():
     ])['Reservations']
     reservations[0]['Instances'].should.have.length_of(1)
 
+
 @mock_ec2
 def test_get_instances_filtering_by_instance_group_id():
     image_id = 'ami-1234abcd'
@@ -476,6 +482,7 @@ def test_get_instances_filtering_by_instance_group_id():
     ])['Reservations']
     reservations[0]['Instances'].should.have.length_of(1)
 
+
 @mock_ec2_deprecated
 def test_get_instances_filtering_by_tag():
     conn = boto.connect_ec2()
@@ -830,18 +837,113 @@ def test_run_instance_with_placement():
     instance.placement.should.equal("us-east-1b")
 
 
-@mock_ec2_deprecated
+@mock_ec2
-def test_run_instance_with_subnet():
+def test_run_instance_with_subnet_boto3():
-    conn = boto.connect_vpc('the_key', 'the_secret')
+    client = boto3.client('ec2', region_name='eu-central-1')
-    vpc = conn.create_vpc("10.0.0.0/16")
-    subnet = conn.create_subnet(vpc.id, "10.0.0.0/18")
-    reservation = conn.run_instances('ami-1234abcd', subnet_id=subnet.id)
-    instance = reservation.instances[0]
 
-    instance.subnet_id.should.equal(subnet.id)
+    ip_networks = [
+        (ipaddress.ip_network('10.0.0.0/16'), ipaddress.ip_network('10.0.99.0/24')),
+        (ipaddress.ip_network('192.168.42.0/24'), ipaddress.ip_network('192.168.42.0/25'))
+    ]
 
-    all_enis = conn.get_all_network_interfaces()
+    # Tests instances are created with the correct IPs
-    all_enis.should.have.length_of(1)
+    for vpc_cidr, subnet_cidr in ip_networks:
+        resp = client.create_vpc(
+            CidrBlock=str(vpc_cidr),
+            AmazonProvidedIpv6CidrBlock=False,
+            DryRun=False,
+            InstanceTenancy='default'
+        )
+        vpc_id = resp['Vpc']['VpcId']
+
+        resp = client.create_subnet(
+            CidrBlock=str(subnet_cidr),
+            VpcId=vpc_id
+        )
+        subnet_id = resp['Subnet']['SubnetId']
+
+        resp = client.run_instances(
+            ImageId='ami-1234abcd',
+            MaxCount=1,
+            MinCount=1,
+            SubnetId=subnet_id
+        )
+        instance = resp['Instances'][0]
+        instance['SubnetId'].should.equal(subnet_id)
+
+        priv_ipv4 = ipaddress.ip_address(six.text_type(instance['PrivateIpAddress']))
+        subnet_cidr.should.contain(priv_ipv4)
+
+
+@mock_ec2
+def test_run_instance_with_specified_private_ipv4():
+    client = boto3.client('ec2', region_name='eu-central-1')
+
+    vpc_cidr = ipaddress.ip_network('192.168.42.0/24')
+    subnet_cidr = ipaddress.ip_network('192.168.42.0/25')
+
+    resp = client.create_vpc(
+        CidrBlock=str(vpc_cidr),
+        AmazonProvidedIpv6CidrBlock=False,
+        DryRun=False,
+        InstanceTenancy='default'
+    )
+    vpc_id = resp['Vpc']['VpcId']
+
+    resp = client.create_subnet(
+        CidrBlock=str(subnet_cidr),
+        VpcId=vpc_id
+    )
+    subnet_id = resp['Subnet']['SubnetId']
+
+    resp = client.run_instances(
+        ImageId='ami-1234abcd',
+        MaxCount=1,
+        MinCount=1,
+        SubnetId=subnet_id,
+        PrivateIpAddress='192.168.42.5'
+    )
+    instance = resp['Instances'][0]
+    instance['SubnetId'].should.equal(subnet_id)
+    instance['PrivateIpAddress'].should.equal('192.168.42.5')
+
+
+@mock_ec2
+def test_run_instance_mapped_public_ipv4():
+    client = boto3.client('ec2', region_name='eu-central-1')
+
+    vpc_cidr = ipaddress.ip_network('192.168.42.0/24')
+    subnet_cidr = ipaddress.ip_network('192.168.42.0/25')
+
+    resp = client.create_vpc(
+        CidrBlock=str(vpc_cidr),
+        AmazonProvidedIpv6CidrBlock=False,
+        DryRun=False,
+        InstanceTenancy='default'
+    )
+    vpc_id = resp['Vpc']['VpcId']
+
+    resp = client.create_subnet(
+        CidrBlock=str(subnet_cidr),
+        VpcId=vpc_id
+    )
+    subnet_id = resp['Subnet']['SubnetId']
+    client.modify_subnet_attribute(
+        SubnetId=subnet_id,
+        MapPublicIpOnLaunch={'Value': True}
+    )
+
+    resp = client.run_instances(
+        ImageId='ami-1234abcd',
+        MaxCount=1,
+        MinCount=1,
+        SubnetId=subnet_id
+    )
+    instance = resp['Instances'][0]
+    instance.should.contain('PublicDnsName')
+    instance.should.contain('PublicIpAddress')
+    len(instance['PublicDnsName']).should.be.greater_than(0)
+    len(instance['PublicIpAddress']).should.be.greater_than(0)
 
 
 @mock_ec2_deprecated
@@ -853,7 +955,7 @@ def test_run_instance_with_nic_autocreated():
         'test security group #1', 'this is a test security group')
     security_group2 = conn.create_security_group(
         'test security group #2', 'this is a test security group')
-    private_ip = "54.0.0.1"
+    private_ip = "10.0.0.1"
 
     reservation = conn.run_instances('ami-1234abcd', subnet_id=subnet.id,
                                      security_groups=[security_group1.name],
@@ -880,6 +982,7 @@ def test_run_instance_with_nic_autocreated():
     eni.private_ip_addresses.should.have.length_of(1)
     eni.private_ip_addresses[0].private_ip_address.should.equal(private_ip)
 
+
 @mock_ec2_deprecated
 def test_run_instance_with_nic_preexisting():
     conn = boto.connect_vpc('the_key', 'the_secret')
@@ -1012,6 +1115,7 @@ def test_ec2_classic_has_public_ip_address():
     instance.private_ip_address.should_not.equal(None)
     instance.private_dns_name.should.contain(instance.private_ip_address.replace('.', '-'))
 
+
 @mock_ec2_deprecated
 def test_run_instance_with_keypair():
     conn = boto.connect_ec2('the_key', 'the_secret')

tests/test_ec2/test_route_tables.py

@@ -126,9 +126,9 @@ def test_route_tables_filters_associations():
     conn = boto.connect_vpc('the_key', 'the_secret')
 
     vpc = conn.create_vpc("10.0.0.0/16")
-    subnet1 = conn.create_subnet(vpc.id, "10.0.0.0/18")
+    subnet1 = conn.create_subnet(vpc.id, "10.0.0.0/24")
-    subnet2 = conn.create_subnet(vpc.id, "10.0.1.0/18")
+    subnet2 = conn.create_subnet(vpc.id, "10.0.1.0/24")
-    subnet3 = conn.create_subnet(vpc.id, "10.0.2.0/18")
+    subnet3 = conn.create_subnet(vpc.id, "10.0.2.0/24")
     route_table1 = conn.create_route_table(vpc.id)
     route_table2 = conn.create_route_table(vpc.id)
 

wait_for.py

@@ -24,7 +24,7 @@ while True:
         break
     except EXCEPTIONS:
         elapsed_s = time.time() - start_ts
-        if elapsed_s > 30:
+        if elapsed_s > 60:
             raise
 
         print('.')