Support subnet and IP for instance creation

Support ``subnet_id`` and ``ip_address`` for creating instance. When
creating instance, trove will check the network conflicts between user's
network and the management network, additionally, the cloud admin is
able to define other reserved networks by configuring
``reserved_network_cidrs``.

Change-Id: Icc4eece2f265cb5a5c48c4f1024a9189d11b4687

api-ref/source/parameters.yaml

@@ -624,11 +624,10 @@ name:
   type: string
 nics:
   description: |
-     Network interfaces for database service inside Nova instances.
-     ``NOTE:`` For backward compatibility, this parameter uses the same schema
-     as novaclient creating servers, but only ``net-id`` is supported and can
-     only be specified once. This parameter is required in service tenant
-     deployment model.
+    Network interface definition for database instance. This is a list of
+    mappings for backward compatibility, but only one item is allowed. The
+    allowed keys in the mapping are: network_id, subnet_id, ip_address and
+    net-id (for backward compatibility, deprecated).
   in: body
   required: false
   type: array

releasenotes/notes/victoria-support-subnet-and-ip-address.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - Support ``subnet_id`` and ``ip_address`` for creating instance. When
+    creating instance, trove will check the network conflicts between user's
+    network and the management network, additionally, the cloud admin is able
+    to define other reserved networks by configuring
+    ``reserved_network_cidrs``.

trove/common/apischema.py

@@ -106,6 +106,13 @@ uuid = {
                "-([0-9a-fA-F]){4}-([0-9a-fA-F]){12}$"
 }
 
+ip_address_v4 = {
+    "type": "string",
+    "minLength": 7,
+    "maxLength": 15,
+    "pattern": r"^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$"
+}
+
 volume = {
     "type": "object",
     "required": ["size"],
@@ -116,7 +123,6 @@ volume = {
                 non_empty_string,
                 {"type": "null"}
             ]
-
         }
     }
 }
@@ -128,7 +134,10 @@ nics = {
         "type": "object",
         "additionalProperties": False,
         "properties": {
-            "net-id": uuid
+            "net-id": uuid,
+            "network_id": uuid,
+            "subnet_id": uuid,
+            "ip_address": ip_address_v4
         }
     }
 }

trove/common/cfg.py

@@ -473,6 +473,9 @@ common_opts = [
                help='The UID(GID) of database service user.'),
     cfg.StrOpt('backup_docker_image', default='openstacktrove/db-backup:1.0.0',
                help='The docker image used for backup and restore.'),
+    cfg.ListOpt('reserved_network_cidrs', default=[],
+                help='Network CIDRs reserved for Trove guest instance '
+                     'management.')
 ]
 
 

trove/common/clients_admin.py

@@ -19,13 +19,18 @@ from keystoneauth1 import loading
 from keystoneauth1 import session
 from neutronclient.v2_0 import client as NeutronClient
 from novaclient.client import Client as NovaClient
+from oslo_log import log as logging
 import swiftclient
 
 from trove.common import cfg
 from trove.common.clients import normalize_url
 
+LOG = logging.getLogger(__name__)
 CONF = cfg.CONF
 _SESSION = None
+ADMIN_NEUTRON_CLIENT = None
+ADMIN_NOVA_CLIENT = None
+ADMIN_CINDER_CLIENT = None
 
 
 def get_keystone_session():
@@ -54,9 +59,14 @@ def nova_client_trove_admin(context, region_name=None, password=None):
     :return novaclient: novaclient with trove admin credentials
     :rtype: novaclient.client.Client
     """
+    global ADMIN_NOVA_CLIENT
+
+    if ADMIN_NOVA_CLIENT:
+        LOG.debug('Re-use admin nova client')
+        return ADMIN_NOVA_CLIENT
 
     ks_session = get_keystone_session()
-    client = NovaClient(
+    ADMIN_NOVA_CLIENT = NovaClient(
         CONF.nova_client_version,
         session=ks_session,
         service_type=CONF.nova_compute_service_type,
@@ -65,11 +75,11 @@ def nova_client_trove_admin(context, region_name=None, password=None):
         endpoint_type=CONF.nova_compute_endpoint_type)
 
     if CONF.nova_compute_url and CONF.service_credentials.project_id:
-        client.client.endpoint_override = "%s/%s/" % (
+        ADMIN_NOVA_CLIENT.client.endpoint_override = "%s/%s/" % (
             normalize_url(CONF.nova_compute_url),
             CONF.service_credentials.project_id)
 
-    return client
+    return ADMIN_NOVA_CLIENT
 
 
 def cinder_client_trove_admin(context, region_name=None):
@@ -79,8 +89,14 @@ def cinder_client_trove_admin(context, region_name=None):
     :type context: trove.common.context.TroveContext
     :return cinderclient: cinderclient with trove admin credentials
     """
+    global ADMIN_CINDER_CLIENT
+
+    if ADMIN_CINDER_CLIENT:
+        LOG.debug('Re-use admin cinder client')
+        return ADMIN_CINDER_CLIENT
+
     ks_session = get_keystone_session()
-    client = CinderClient.Client(
+    ADMIN_CINDER_CLIENT = CinderClient.Client(
         session=ks_session,
         service_type=CONF.cinder_service_type,
         region_name=region_name or CONF.service_credentials.region_name,
@@ -88,11 +104,11 @@ def cinder_client_trove_admin(context, region_name=None):
         endpoint_type=CONF.cinder_endpoint_type)
 
     if CONF.cinder_url and CONF.service_credentials.project_id:
-        client.client.management_url = "%s/%s/" % (
+        ADMIN_CINDER_CLIENT.client.management_url = "%s/%s/" % (
             normalize_url(CONF.cinder_url),
             CONF.service_credentials.project_id)
 
-    return client
+    return ADMIN_CINDER_CLIENT
 
 
 def neutron_client_trove_admin(context, region_name=None):
@@ -102,8 +118,14 @@ def neutron_client_trove_admin(context, region_name=None):
     :type context: trove.common.context.TroveContext
     :return neutronclient: neutronclient with trove admin credentials
     """
+    global ADMIN_NEUTRON_CLIENT
+
+    if ADMIN_NEUTRON_CLIENT:
+        LOG.debug('Re-use admin neutron client')
+        return ADMIN_NEUTRON_CLIENT
+
     ks_session = get_keystone_session()
-    client = NeutronClient.Client(
+    ADMIN_NEUTRON_CLIENT = NeutronClient.Client(
         session=ks_session,
         service_type=CONF.neutron_service_type,
         region_name=region_name or CONF.service_credentials.region_name,
@@ -111,9 +133,9 @@ def neutron_client_trove_admin(context, region_name=None):
         endpoint_type=CONF.neutron_endpoint_type)
 
     if CONF.neutron_url:
-        client.management_url = CONF.neutron_url
+        ADMIN_NEUTRON_CLIENT.management_url = CONF.neutron_url
 
-    return client
+    return ADMIN_NEUTRON_CLIENT
 
 
 def swift_client_trove_admin(context, region_name=None):

trove/common/exception.py

@@ -622,7 +622,16 @@ class PublicNetworkNotFound(TroveError):
 
 
 class NetworkConflict(BadRequest):
-    message = _("User network conflicts with the management network.")
+    message = _("User network conflicts with the reserved network.")
+
+
+class NetworkNotProvided(BadRequest):
+    message = _("Instance %(resource)s needs to be specified.")
+
+
+class SubnetNotFound(BadRequest):
+    message = _("Subnet %(subnet_id)s not found in the network "
+                "%(network_id)s.")
 
 
 class ClusterVolumeSizeRequired(TroveError):

trove/common/neutron.py

@@ -55,7 +55,7 @@ def reset_management_networks():
 
 
 def create_port(client, name, description, network_id, security_groups,
-                is_public=False):
+                is_public=False, subnet_id=None, ip=None):
     port_body = {
         "port": {
             "name": name,
@@ -64,6 +64,15 @@ def create_port(client, name, description, network_id, security_groups,
             "security_groups": security_groups
         }
     }
+
+    if subnet_id:
+        fixed_ips = {
+            "fixed_ips": [{"subnet_id": subnet_id}]
+        }
+        if ip:
+            fixed_ips['fixed_ips'][0].update({'ip_address': ip})
+        port_body['port'].update(fixed_ips)
+
     port = client.create_port(body=port_body)
     port_id = port['port']['id']
 
@@ -150,9 +159,13 @@ def create_security_group_rule(client, sg_id, protocol, ports, remote_ips):
             client.create_security_group_rule(body)
 
 
-def get_subnet_cidrs(client, network_id):
+def get_subnet_cidrs(client, network_id=None, subnet_id=None):
     cidrs = []
 
+    # Check subnet first.
+    if subnet_id:
+        cidrs.append(client.show_subnet(subnet_id)['subnet']['cidr'])
+    elif network_id:
         subnets = client.list_subnets(network_id=network_id)['subnets']
         for subnet in subnets:
             cidrs.append(subnet.get('cidr'))

trove/instance/models.py

@@ -1207,7 +1207,7 @@ class Instance(BuiltInstance):
         if CONF.management_networks:
             # Make sure management network interface is always configured after
             # user defined instance.
-            nics = nics + [{"net-id": net_id}
+            nics = nics + [{"network_id": net_id}
                            for net_id in CONF.management_networks]
         if nics:
             call_args['nics'] = nics

trove/instance/service.py

@@ -282,17 +282,62 @@ class InstanceController(wsgi.Controller):
             instance.delete()
         return wsgi.Result(None, 202)
 
-    def _check_network_overlap(self, context, user_network):
+    def _check_nic(self, context, nic):
+        """Check user provided nic.
+
+        :param context: User context.
+        :param nic: A dict may contain network_id(net-id), subnet_id or
+            ip_address.
+        """
         neutron_client = clients.create_neutron_client(context)
-        user_cidrs = neutron.get_subnet_cidrs(neutron_client, user_network)
+        network_id = nic.get('network_id', nic.get('net-id'))
+        subnet_id = nic.get('subnet_id')
+        ip_address = nic.get('ip_address')
+
+        if not network_id and not subnet_id:
+            raise exception.NetworkNotProvided(resource='network or subnet')
+
+        if not subnet_id and ip_address:
+            raise exception.NetworkNotProvided(resource='subnet')
+
+        if subnet_id:
+            actual_network = neutron_client.show_subnet(
+                subnet_id)['subnet']['network_id']
+            if network_id and actual_network != network_id:
+                raise exception.SubnetNotFound(subnet_id=subnet_id,
+                                               network_id=network_id)
+            network_id = actual_network
+
+        nic['network_id'] = network_id
+        nic.pop('net-id', None)
+
+        self._check_network_overlap(context, network_id, subnet_id)
+
+    def _check_network_overlap(self, context, user_network=None,
+                               user_subnet=None):
+        """Check if the network contains IP address belongs to reserved
+        network.
+
+        :param context: User context.
+        :param user_network: Network ID.
+        :param user_subnet: Subnet ID.
+        """
+        neutron_client = clients.create_neutron_client(context)
+        user_cidrs = neutron.get_subnet_cidrs(neutron_client, user_network,
+                                              user_subnet)
+
+        reserved_cidrs = CONF.reserved_network_cidrs
         mgmt_cidrs = neutron.get_mamangement_subnet_cidrs(neutron_client)
-        LOG.debug("Cidrs of the user network: %s, cidrs of the management "
-                  "network: %s", user_cidrs, mgmt_cidrs)
+        reserved_cidrs.extend(mgmt_cidrs)
+
+        LOG.debug("Cidrs of the user network: %s, cidrs of the reserved "
+                  "network: %s", user_cidrs, reserved_cidrs)
+
         for user_cidr in user_cidrs:
             user_net = ipaddress.ip_network(user_cidr)
-            for mgmt_cidr in mgmt_cidrs:
-                mgmt_net = ipaddress.ip_network(mgmt_cidr)
-                if user_net.overlaps(mgmt_net):
+            for reserved_cidr in reserved_cidrs:
+                res_net = ipaddress.ip_network(reserved_cidr)
+                if user_net.overlaps(res_net):
                     raise exception.NetworkConflict()
 
     def create(self, req, body, tenant_id):
@@ -359,9 +404,12 @@ class InstanceController(wsgi.Controller):
 
         availability_zone = body['instance'].get('availability_zone')
 
+        # Only 1 nic is allowed as defined in API jsonschema.
+        # Use list here just for backward compatibility.
         nics = body['instance'].get('nics', [])
         if len(nics) > 0:
-            self._check_network_overlap(context, nics[0].get('net-id'))
+            LOG.info('Checking user provided instance network %s', nics[0])
+            self._check_nic(context, nics[0])
 
         slave_of_id = body['instance'].get('replica_of',
                                            # also check for older name

trove/taskmanager/models.py

@@ -461,7 +461,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
                     skip_delta=CONF.usage_sleep_time + 1
                 )
 
-    def _create_port(self, network, security_groups, is_mgmt=False,
+    def _create_port(self, network_info, security_groups, is_mgmt=False,
                      is_public=False):
         name = 'trove-%s' % self.id
         type = 'Management' if is_mgmt else 'User'
@@ -470,9 +470,11 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
         try:
             port_id = neutron.create_port(
                 self.neutron_client, name,
-                description, network,
+                description, network_info.get('network_id'),
                 security_groups,
-                is_public=is_public
+                is_public=is_public,
+                subnet_id=network_info.get('subnet_id'),
+                ip=network_info.get('ip_address')
             )
         except Exception:
             error = ("Failed to create %s port for instance %s"
@@ -491,6 +493,8 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
 
         'nics' contains the networks that management network always comes at
         last.
+
+        returns a list of dicts which only contains port-id.
         """
         LOG.info("Preparing networks for the instance %s", self.id)
         security_group = None
@@ -515,7 +519,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
             # The management network is always the last one
             networks.pop(-1)
             port_id = self._create_port(
-                CONF.management_networks[-1],
+                {'network_id': CONF.management_networks[-1]},
                 port_sgs,
                 is_mgmt=True
             )
@@ -526,10 +530,10 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin):
         # Create port in the user defined network, associate floating IP if
         # needed
         if len(networks) > 1 or not CONF.management_networks:
-            network = networks.pop(0).get("net-id")
+            network_info = networks.pop(0)
             port_sgs = [security_group] if security_group else []
             port_id = self._create_port(
-                network,
+                network_info,
                 port_sgs,
                 is_mgmt=False,
                 is_public=access.get('is_public', False)

trove/tests/unittests/instance/test_instance_controller.py

@@ -13,9 +13,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 #
+import copy
+import uuid
+
 import jsonschema
 from mock import Mock
-from testtools.matchers import Is, Equals
+from testtools.matchers import Equals
+from testtools.matchers import Is
 from testtools.testcase import skip
 
 from trove.common import apischema
@@ -165,6 +169,20 @@ class TestInstanceController(trove_testtools.TestCase):
                       error_messages)
         self.assertIn("locality", error_paths)
 
+    def test_validate_create_valid_nics(self):
+        body = copy.copy(self.instance)
+        body['instance']['nics'] = [
+            {
+                'network_id': str(uuid.uuid4()),
+                'subnet_id': str(uuid.uuid4()),
+                'ip_address': '192.168.1.11'
+            }
+        ]
+
+        schema = self.controller.get_schema('create', body)
+        validator = jsonschema.Draft4Validator(schema)
+        self.assertTrue(validator.is_valid(body))
+
     def test_validate_restart(self):
         body = {"restart": {}}
         schema = self.controller.get_schema('action', body)

trove/tests/unittests/taskmanager/test_models.py

@@ -372,7 +372,7 @@ class FreshInstanceTasksTest(BaseFreshInstanceTasksTest):
 
         mock_create_secgroup.assert_called_with('mysql', [])
         mock_create_port.assert_called_once_with(
-            'fake-net-id',
+            {'net-id': 'fake-net-id'},
             ['fake_security_group_id'],
             is_mgmt=False,
             is_public=False