Fixed bug with regards to subnet lookups.

Neutron returns all subnets regardless of visibility which cause problems
within routers if there is another subnet with the same name attached to
a different network.

JIRA: SNAPS-304

In addition, this patch contains two other minor fixes.
  launch_utils.py - raise an exception when the creator is not properly instantiated
  network.py - allow fixed IPs to be none.

Change-Id: Ib343074d925be4592a713727a03d5b531890eada
Signed-off-by: spisarski <s.pisarski@cablelabs.com>

3 files changed, 84 insertions, 18 deletions

diff --git a/snaps/config/network.py b/snaps/config/network.py
index 8148c61..eca8734 100644
--- a/snaps/config/network.py
+++ b/snaps/config/network.py
@@ -412,22 +412,23 @@ class PortConfig(object):
             raise PortConfigError(
                 'The attribute network_name is required')
 
-    def __get_fixed_ips(self, neutron):
+    def __get_fixed_ips(self, neutron, network):
         """
         Sets the self.fixed_ips value
         :param neutron: the Neutron client
+        :param network: the SNAPS-OO network domain object
         :return: None
         """
-
         fixed_ips = list()
         if self.ip_addrs:
 
             for ip_addr_dict in self.ip_addrs:
                 subnet = neutron_utils.get_subnet(
-                    neutron, subnet_name=ip_addr_dict['subnet_name'])
-                if subnet and 'ip' in ip_addr_dict:
-                    fixed_ips.append({'ip_address': ip_addr_dict['ip'],
-                                      'subnet_id': subnet.id})
+                    neutron, network, subnet_name=ip_addr_dict['subnet_name'])
+                if subnet:
+                    if 'ip' in ip_addr_dict:
+                        fixed_ips.append({'ip_address': ip_addr_dict['ip'],
+                                          'subnet_id': subnet.id})
                 else:
                     raise PortConfigError(
                         'Invalid port configuration, subnet does not exist '
@@ -443,25 +444,29 @@ class PortConfig(object):
 
         TODO - expand automated testing to exercise all parameters
         :param neutron: the Neutron client
+        :param keystone: the Keystone client
         :param os_creds: the OpenStack credentials
         :return: the dictionary object
         """
-
         out = dict()
-
         session = keystone_utils.keystone_session(os_creds)
         keystone = keystone_utils.keystone_client(os_creds, session)
+
+        project_name = os_creds.project_name
+        if self.project_name:
+            project_name = project_name
         try:
             network = neutron_utils.get_network(
                 neutron, keystone, network_name=self.network_name,
-                project_name=self.project_name)
+                project_name=project_name)
         finally:
-            keystone_utils.close_session(session)
+            if session:
+                keystone_utils.close_session(session)
 
         if not network:
             raise PortConfigError(
                 'Cannot locate network with name - ' + self.network_name
-                + ' in project - ' + str(self.project_name))
+                + ' in project - ' + str(project_name))
 
         out['network_id'] = network.id
 
@@ -484,7 +489,7 @@ class PortConfig(object):
         if self.mac_address:
             out['mac_address'] = self.mac_address
 
-        fixed_ips = self.__get_fixed_ips(neutron)
+        fixed_ips = self.__get_fixed_ips(neutron, network)
         if fixed_ips and len(fixed_ips) > 0:
             out['fixed_ips'] = fixed_ips
 
diff --git a/snaps/config/router.py b/snaps/config/router.py
index 6b03ad2..2a0b6a4 100644
--- a/snaps/config/router.py
+++ b/snaps/config/router.py
@@ -33,7 +33,11 @@ class RouterConfig(object):
         :param admin_state_up: The administrative status of the router.
                                True = up / False = down (default True)
         :param internal_subnets: List of subnet names to which to connect this
-                                 router for Floating IP purposes
+                                 router (this way is deprecated).
+                                 *** NEW WAY ***
+                                 List of dict where the key is 'subnet' that
+                                 contains members with the following keys:
+                                 project_name, network_name, and subnet_name
         :param port_settings: List of PortConfig objects
         :return:
         """
@@ -45,6 +49,19 @@ class RouterConfig(object):
         self.enable_snat = kwargs.get('enable_snat')
         if kwargs.get('internal_subnets'):
             self.internal_subnets = kwargs['internal_subnets']
+            if isinstance(self.internal_subnets, dict):
+                if 'subnet' not in self.internal_subnets:
+                    raise RouterConfigError(
+                        'subnet is a required key to internal_subnets')
+                if 'project_name' not in self.internal_subnets['subnet']:
+                    raise RouterConfigError(
+                        'subnet.project is a required key to subnet')
+                if 'network_name' not in self.internal_subnets['subnet']:
+                    raise RouterConfigError(
+                        'network_name is a required key to subnet')
+                if 'subnet_name' not in self.internal_subnets['subnet']:
+                    raise RouterConfigError(
+                        'subnet_name is a required key to subnet')
         else:
             self.internal_subnets = list()
 
diff --git a/snaps/config/tests/router_tests.py b/snaps/config/tests/router_tests.py
index 2c8f91f..1397f23 100644
--- a/snaps/config/tests/router_tests.py
+++ b/snaps/config/tests/router_tests.py
@@ -31,6 +31,25 @@ class RouterConfigUnitTests(unittest.TestCase):
         with self.assertRaises(RouterConfigError):
             RouterConfig(**dict())
 
+    def test_bad_internal_subnets_bad_key(self):
+        with self.assertRaises(RouterConfigError):
+            RouterConfig(name='foo', internal_subnets={'foo': 'bar'})
+
+    def test_bad_internal_subnets_no_project(self):
+        with self.assertRaises(RouterConfigError):
+            RouterConfig(name='foo', internal_subnets={
+                'subnet': {'subnet_name': 'bar', 'network_name': 'foo'}})
+
+    def test_bad_internal_subnets_no_network(self):
+        with self.assertRaises(RouterConfigError):
+            RouterConfig(name='foo', internal_subnets={
+                'subnet': {'subnet_name': 'bar', 'project_name': 'foo'}})
+
+    def test_bad_internal_subnets_no_subnet(self):
+        with self.assertRaises(RouterConfigError):
+            RouterConfig(name='foo', internal_subnets={
+                'subnet': {'project_name': 'bar', 'network_name': 'foo'}})
+
     def test_name_only(self):
         settings = RouterConfig(name='foo')
         self.assertEqual('foo', settings.name)
@@ -59,7 +78,7 @@ class RouterConfigUnitTests(unittest.TestCase):
         self.assertTrue(isinstance(settings.port_settings, list))
         self.assertEqual(0, len(settings.port_settings))
 
-    def test_all(self):
+    def test_all_internal_subnets_str(self):
         port_settings = PortConfig(name='foo', network_name='bar')
         settings = RouterConfig(
             name='foo', project_name='bar', external_gateway='foo_gateway',
@@ -76,11 +95,36 @@ class RouterConfigUnitTests(unittest.TestCase):
         self.assertEqual(['10.0.0.1/24'], settings.internal_subnets)
         self.assertEqual([port_settings], settings.port_settings)
 
-    def test_config_all(self):
+    def test_all_internal_subnets_dict(self):
+        port_settings = PortConfig(name='foo', network_name='bar')
+        int_subs = {'subnet': {
+            'project_name': 'proj_a', 'network_name': 'net_name',
+            'subnet_name': 'sub_name'}}
+        settings = RouterConfig(
+            name='foo', project_name='bar', external_gateway='foo_gateway',
+            admin_state_up=True, enable_snat=False,
+            internal_subnets=int_subs,
+            interfaces=[port_settings])
+        self.assertEqual('foo', settings.name)
+        self.assertEqual('bar', settings.project_name)
+        self.assertEqual('foo_gateway', settings.external_gateway)
+        self.assertTrue(settings.admin_state_up)
+        self.assertFalse(settings.enable_snat)
+        self.assertIsNotNone(settings.internal_subnets)
+        self.assertTrue(isinstance(settings.internal_subnets, dict))
+        self.assertEqual(1, len(settings.internal_subnets))
+        self.assertEqual(int_subs, settings.internal_subnets)
+        self.assertEqual([port_settings], settings.port_settings)
+
+    def test_config_all_internal_subnets_str(self):
+        int_subs = {'subnet': {
+            'project_name': 'proj_a', 'network_name': 'net_name',
+            'subnet_name': 'sub_name'}}
         settings = RouterConfig(
             **{'name': 'foo', 'project_name': 'bar',
                'external_gateway': 'foo_gateway', 'admin_state_up': True,
-               'enable_snat': False, 'internal_subnets': ['10.0.0.1/24'],
+               'enable_snat': False,
+               'internal_subnets': int_subs,
                'interfaces':
                    [{'port': {'name': 'foo-port',
                               'network_name': 'bar-net'}}]})
@@ -90,9 +134,9 @@ class RouterConfigUnitTests(unittest.TestCase):
         self.assertTrue(settings.admin_state_up)
         self.assertFalse(settings.enable_snat)
         self.assertIsNotNone(settings.internal_subnets)
-        self.assertTrue(isinstance(settings.internal_subnets, list))
+        self.assertTrue(isinstance(settings.internal_subnets, dict))
         self.assertEqual(1, len(settings.internal_subnets))
-        self.assertEqual(['10.0.0.1/24'], settings.internal_subnets)
+        self.assertEqual(int_subs, settings.internal_subnets)
         self.assertEqual([PortConfig(**{'name': 'foo-port',
                                         'network_name': 'bar-net'})],
                          settings.port_settings)