diff --git a/ec2api/api/subnet.py b/ec2api/api/subnet.py index b367f06a..ee768b05 100644 --- a/ec2api/api/subnet.py +++ b/ec2api/api/subnet.py @@ -34,6 +34,8 @@ LOG = logging.getLogger(__name__) """Subnet related API implementation """ +# TODO(ft): implement ModifySubnetAttribute method +# TODO(ft): implement 'availabilityZone' property Validator = common.Validator @@ -47,14 +49,7 @@ def create_subnet(context, vpc_id, cidr_block, if subnet_ipnet not in vpc_ipnet: raise exception.InvalidSubnetRange(cidr_block=cidr_block) - # TODO(ft): - # check availability zone - # choose default availability zone gateway_ip = str(netaddr.IPAddress(subnet_ipnet.first + 1)) - # NOTE(Alex): AWS takes 4 first addresses (.1 - .4) but for - # OpenStack we decided not to support this as compatibility. - # start_ip = str(netaddr.IPAddress(subnet_ipnet.first + 4)) - # end_ip = str(netaddr.IPAddress(subnet_ipnet.last - 1)) main_route_table = db_api.get_item_by_id(context, 'rtb', vpc['route_table_id']) host_routes = route_table_api._get_subnet_host_routes( @@ -62,23 +57,26 @@ def create_subnet(context, vpc_id, cidr_block, neutron = clients.neutron(context) with common.OnCrashCleaner() as cleaner: os_network_body = {'network': {}} - os_network = neutron.create_network(os_network_body)['network'] - cleaner.addCleanup(neutron.delete_network, os_network['id']) - os_subnet_body = {'subnet': {'network_id': os_network['id'], - 'ip_version': '4', - 'cidr': cidr_block, - # 'allocation_pools': [{'start': start_ip, - # 'end': end_ip}], - 'host_routes': host_routes}} - os_subnet = neutron.create_subnet(os_subnet_body)['subnet'] - cleaner.addCleanup(neutron.delete_subnet, os_subnet['id']) - neutron.add_interface_router(vpc['os_id'], - {'subnet_id': os_subnet['id']}) + try: + os_network = neutron.create_network(os_network_body)['network'] + cleaner.addCleanup(neutron.delete_network, os_network['id']) + # NOTE(Alex): AWS takes 4 first addresses (.1 - .4) but for + # OpenStack we decided not to support this as compatibility. + os_subnet_body = {'subnet': {'network_id': os_network['id'], + 'ip_version': '4', + 'cidr': cidr_block, + 'host_routes': host_routes}} + os_subnet = neutron.create_subnet(os_subnet_body)['subnet'] + cleaner.addCleanup(neutron.delete_subnet, os_subnet['id']) + except neutron_exception.OverQuotaClient: + raise exception.SubnetLimitExceeded() + try: + neutron.add_interface_router(vpc['os_id'], + {'subnet_id': os_subnet['id']}) + except neutron_exception.BadRequest: + raise exception.InvalidSubnetConflict(cidr_block=cidr_block) cleaner.addCleanup(neutron.remove_interface_router, vpc['os_id'], {'subnet_id': os_subnet['id']}) - # TODO(Alex): Handle errors like cidr conflict or overlimit - # TODO(ft): - # store availability_zone subnet = db_api.add_item(context, 'subnet', {'os_id': os_subnet['id'], 'vpc_id': vpc['id']}) @@ -89,7 +87,7 @@ def create_subnet(context, vpc_id, cidr_block, {'subnet': {'name': subnet['id']}}) os_ports = neutron.list_ports()['ports'] return {'subnet': _format_subnet(context, subnet, os_subnet, - os_network, os_ports)} + os_network, os_ports)} def delete_subnet(context, subnet_id): @@ -108,24 +106,26 @@ def delete_subnet(context, subnet_id): db_api.delete_item(context, subnet['id']) cleaner.addCleanup(db_api.restore_item, context, 'subnet', subnet) try: - if vpc is not None: - neutron.remove_interface_router(vpc['os_id'], - {'subnet_id': subnet['os_id']}) - cleaner.addCleanup(neutron.add_interface_router, - vpc['os_id'], - {'subnet_id': subnet['os_id']}) - os_subnet = neutron.show_subnet(subnet['os_id'])['subnet'] - neutron.delete_subnet(os_subnet['id']) - neutron.delete_network(os_subnet['network_id']) - except neutron_exception.NeutronClientException: - # TODO(ft): do log error - # TODO(ft): adjust catched exception classes to catch: - # the subnet is already unplugged from the router - # no such router - # the subnet doesn't exist - # some ports exist in the subnet - # the network has other not empty subnets + neutron.remove_interface_router(vpc['os_id'], + {'subnet_id': subnet['os_id']}) + except neutron_exception.NotFound: pass + cleaner.addCleanup(neutron.add_interface_router, + vpc['os_id'], + {'subnet_id': subnet['os_id']}) + try: + os_subnet = neutron.show_subnet(subnet['os_id'])['subnet'] + except neutron_exception.NotFound: + pass + else: + try: + neutron.delete_network(os_subnet['network_id']) + except neutron_exception.NetworkInUseClient as ex: + LOG.warning(_('Failed to delete network %(os_id)s during ' + 'deleting Subnet %(id)s. Reason: %(reason)s'), + {'id': subnet['id'], + 'os_id': os_subnet['network_id'], + 'reason': ex.message}) return True @@ -174,9 +174,9 @@ def _format_subnet(context, subnet, os_subnet, os_network, os_ports): 'BUILD': 'pending', 'DOWN': 'available', 'ERROR': 'available'} - range = int(os_subnet['cidr'].split('/')[1]) + cidr_range = int(os_subnet['cidr'].split('/')[1]) # NOTE(Alex) First and last IP addresses are system ones. - ip_count = pow(2, 32 - range) - 2 + ip_count = pow(2, 32 - cidr_range) - 2 # TODO(Alex): Probably performance-killer. Will have to optimize. dhcp_port_accounted = False for port in os_ports: diff --git a/ec2api/exception.py b/ec2api/exception.py index 74432db2..781f79bb 100644 --- a/ec2api/exception.py +++ b/ec2api/exception.py @@ -342,7 +342,12 @@ class RouteAlreadyExists(Invalid): class VpcLimitExceeded(Overlimit): - msg_fmt = _("The maximum number of VPCs has been reached.") + msg_fmt = _('The maximum number of VPCs has been reached.') + + +class SubnetLimitExceeded(Overlimit): + msg_fmt = _('You have reached the limit on the number of subnets that you ' + 'can create') class NetworkInterfaceLimitExceeded(Overlimit): diff --git a/ec2api/tests/unit/test_subnet.py b/ec2api/tests/unit/test_subnet.py index 2bcfb227..2949a3dc 100644 --- a/ec2api/tests/unit/test_subnet.py +++ b/ec2api/tests/unit/test_subnet.py @@ -50,7 +50,7 @@ class SubnetTestCase(base.ApiTestCase): {'network': {'name': fakes.ID_EC2_SUBNET_1}}) self.neutron.create_subnet.assert_called_once_with( {'subnet': tools.purge_dict(fakes.OS_SUBNET_1, - ('id', 'name'))}) + ('id', 'name'))}) self.neutron.update_subnet.assert_called_once_with( fakes.ID_OS_SUBNET_1, {'subnet': {'name': fakes.ID_EC2_SUBNET_1}}) @@ -105,9 +105,29 @@ class SubnetTestCase(base.ApiTestCase): fakes.ID_EC2_VPC_1) check_response(resp, 'InvalidSubnet.Range') - @base.skip_not_implemented + def test_create_subnet_overlapped(self): + self.db_api.get_item_by_id.side_effect = ( + fakes.get_db_api_get_item_by_id({ + fakes.ID_EC2_VPC_1: fakes.DB_VPC_1, + fakes.ID_EC2_ROUTE_TABLE_1: fakes.DB_ROUTE_TABLE_1})) + self.neutron.create_network.side_effect = ( + fakes.get_neutron_create('network', fakes.ID_OS_NETWORK_1, + {'status': 'available'})) + self.neutron.create_subnet.side_effect = ( + fakes.get_neutron_create('subnet', fakes.ID_OS_SUBNET_1)) + self.neutron.add_interface_router.side_effect = ( + neutron_exception.BadRequest()) + + resp = self.execute('CreateSubnet', {'VpcId': fakes.ID_EC2_VPC_1, + 'CidrBlock': fakes.CIDR_SUBNET_1}) + self.assertEqual(400, resp['http_status_code']) + self.assertEqual('InvalidSubnet.Conflict', resp['Error']['Code']) + def test_create_subnet_overlimit(self): - self.db_api.get_item_by_id.return_value = fakes.DB_VPC_1 + self.db_api.get_item_by_id.side_effect = ( + fakes.get_db_api_get_item_by_id({ + fakes.ID_EC2_VPC_1: fakes.DB_VPC_1, + fakes.ID_EC2_ROUTE_TABLE_1: fakes.DB_ROUTE_TABLE_1})) self.neutron.create_network.side_effect = ( fakes.get_neutron_create('network', fakes.ID_OS_NETWORK_1, {'status': 'available'})) @@ -117,7 +137,7 @@ class SubnetTestCase(base.ApiTestCase): def test_overlimit(func): self.neutron.reset_mock() saved_side_effect = func.side_effect - func.side_effect = neutron_exception.Conflict + func.side_effect = neutron_exception.OverQuotaClient resp = self.execute('CreateSubnet', {'VpcId': fakes.ID_EC2_VPC_1, @@ -129,7 +149,6 @@ class SubnetTestCase(base.ApiTestCase): test_overlimit(self.neutron.create_network) test_overlimit(self.neutron.create_subnet) - test_overlimit(self.neutron.add_interface_router) def test_create_subnet_rollback(self): self.db_api.get_item_by_id.side_effect = ( @@ -156,10 +175,6 @@ class SubnetTestCase(base.ApiTestCase): self.db_api.delete_item.assert_called_once_with( mock.ANY, fakes.ID_EC2_SUBNET_1) - @base.skip_not_implemented - def test_create_subnet_not_consistent_os_vpc(self): - pass - def test_delete_subnet(self): self.db_api.get_item_by_id.side_effect = ( fakes.get_db_api_get_item_by_id( @@ -174,22 +189,48 @@ class SubnetTestCase(base.ApiTestCase): self.assertEqual(200, resp['http_status_code']) self.assertEqual(True, resp['return']) - self.db_api.get_item_by_id.assert_has_call( - mock.ANY, - fakes.ID_EC2_SUBNET_1) - self.db_api.get_item_by_id.assert_has_call( - mock.ANY, - fakes.ID_EC2_VPC_1) self.db_api.delete_item.assert_called_once_with( mock.ANY, fakes.ID_EC2_SUBNET_1) - self.neutron.show_subnet.assert_called_once_with( - fakes.ID_OS_SUBNET_1) self.neutron.remove_interface_router.assert_called_once_with( fakes.ID_OS_ROUTER_1, {'subnet_id': fakes.ID_OS_SUBNET_1}) + self.neutron.delete_network.assert_called_once_with( + fakes.ID_OS_NETWORK_1) + self.assertTrue( + self.neutron.mock_calls.index( + mock.call.delete_network(fakes.ID_OS_NETWORK_1)) > + self.neutron.mock_calls.index( + mock.call.remove_interface_router( + fakes.ID_OS_ROUTER_1, + {'subnet_id': fakes.ID_OS_SUBNET_1}))) - def test_delete_subnet_no_subnet(self): + def test_delete_subnet_inconsistent_os(self): + self.db_api.get_item_by_id.side_effect = ( + fakes.get_db_api_get_item_by_id( + {fakes.ID_EC2_VPC_1: fakes.DB_VPC_1, + fakes.ID_EC2_SUBNET_1: fakes.DB_SUBNET_1})) + self.db_api.get_items.return_value = [] + self.neutron.remove_interface_router.side_effect = ( + neutron_exception.NotFound()) + self.neutron.show_subnet.return_value = ( + {'subnet': fakes.OS_SUBNET_1}) + self.neutron.delete_network.side_effect = ( + neutron_exception.NetworkInUseClient()) + + resp = self.execute('DeleteSubnet', + {'subnetId': fakes.ID_EC2_SUBNET_1}) + self.assertEqual(200, resp['http_status_code']) + self.assertEqual(True, resp['return']) + + self.neutron.show_subnet.side_effect = neutron_exception.NotFound() + + resp = self.execute('DeleteSubnet', + {'subnetId': fakes.ID_EC2_SUBNET_1}) + self.assertEqual(200, resp['http_status_code']) + self.assertEqual(True, resp['return']) + + def test_delete_subnet_invalid_parameters(self): self.db_api.get_item_by_id.return_value = None self.neutron.show_subnet.return_value = fakes.OS_SUBNET_1 self.neutron.show_network.return_value = fakes.OS_NETWORK_1 @@ -203,6 +244,19 @@ class SubnetTestCase(base.ApiTestCase): self.assertEqual(0, self.neutron.delete_subnet.call_count) self.assertEqual(0, self.neutron.remove_interface_router.call_count) + @mock.patch('ec2api.api.network_interface.describe_network_interfaces') + def test_delete_subnet_not_empty(self, describe_network_interfaces): + self.db_api.get_item_by_id.side_effect = ( + fakes.get_db_api_get_item_by_id( + {fakes.ID_EC2_VPC_1: fakes.DB_VPC_1, + fakes.ID_EC2_SUBNET_1: fakes.DB_SUBNET_1})) + describe_network_interfaces.return_value = ( + {'networkInterfaceSet': [fakes.EC2_NETWORK_INTERFACE_1]}) + resp = self.execute('DeleteSubnet', + {'subnetId': fakes.ID_EC2_SUBNET_1}) + self.assertEqual(400, resp['http_status_code']) + self.assertEqual('DependencyViolation', resp['Error']['Code']) + def test_delete_subnet_rollback(self): self.db_api.get_item_by_id.side_effect = ( fakes.get_db_api_get_item_by_id( @@ -217,28 +271,6 @@ class SubnetTestCase(base.ApiTestCase): self.neutron.add_interface_router.assert_called_once_with( fakes.ID_OS_ROUTER_1, {'subnet_id': fakes.ID_OS_SUBNET_1}) - def test_delete_subnet_has_ports(self): - self.db_api.get_item_by_id.side_effect = ( - fakes.get_db_api_get_item_by_id( - {fakes.ID_EC2_VPC_1: fakes.DB_VPC_1, - fakes.ID_EC2_SUBNET_1: fakes.DB_SUBNET_1})) - self.db_api.get_items.side_effect = ( - fakes.get_db_api_get_items({ - 'eni': [fakes.DB_NETWORK_INTERFACE_1, - fakes.DB_NETWORK_INTERFACE_2], - 'eipalloc': [fakes.DB_ADDRESS_1, fakes.DB_ADDRESS_2], - 'i': [fakes.DB_INSTANCE_1, fakes.DB_INSTANCE_2]})) - self.neutron.list_ports.return_value = ( - {'ports': [fakes.OS_PORT_1, fakes.OS_PORT_2]}) - self.neutron.list_floatingips.return_value = ( - {'floatingips': [fakes.OS_FLOATING_IP_1, - fakes.OS_FLOATING_IP_2]}) - - resp = self.execute('DeleteSubnet', - {'subnetId': fakes.ID_EC2_SUBNET_1}) - self.assertEqual(400, resp['http_status_code']) - self.assertEqual('DependencyViolation', resp['Error']['Code']) - def test_describe_subnets(self): self.db_api.get_items.return_value = ( [fakes.DB_SUBNET_1, fakes.DB_SUBNET_2]) @@ -266,9 +298,9 @@ class SubnetTestCase(base.ApiTestCase): self.check_filtering( 'DescribeSubnets', 'subnetSet', [ - # TODO(ft): support filtering by a number value + # TODO(ft): support filtering by a number value # noqa # NOTE(ft): declare a constant for the count in fakes -# ('available-ip-address-count', 253), + # ('available-ip-address-count', 253), ('cidr', fakes.CIDR_SUBNET_2), ('cidrBlock', fakes.CIDR_SUBNET_2), ('cidr-block', fakes.CIDR_SUBNET_2), @@ -279,14 +311,6 @@ class SubnetTestCase(base.ApiTestCase): 'DescribeSubnets', 'subnetSet', fakes.ID_EC2_SUBNET_2, 'subnetId') - @base.skip_not_implemented - def test_describe_subnets_no_vpc(self): - pass - - @base.skip_not_implemented - def test_describe_subnets_not_consistent_os_vpc(self): - pass - def test_describe_subnets_not_consistent_os_subnet(self): self.db_api.get_items.return_value = ( [fakes.DB_SUBNET_1, fakes.DB_SUBNET_2]) @@ -298,7 +322,3 @@ class SubnetTestCase(base.ApiTestCase): resp = self.execute('DescribeSubnets', {}) self.assertEqual(200, resp['http_status_code']) self.assertEqual([], resp['subnetSet']) - - @base.skip_not_implemented - def test_describe_subnets_is_not_attached_to_router(self): - pass