Merge pull request #260 from cloudify-cosmo/allow-multiple-subnets-port

handle multiple subnets and ports on subnets

Author: EarthmanT

CHANGELOG.txt

@@ -1,3 +1,6 @@
+2.9.8:
+    - Add handling for bootable volume attribute.
+    - Change port fixed ip handling code to permit ports on more than one subnet.
 2.9.7:
     - Add IPv6 Example
     - Base internal volume bootable logic on API bootable flag.

neutron_plugin/port.py

@@ -13,6 +13,8 @@
 #  * See the License for the specific language governing permissions and
 #  * limitations under the License.
 
+import netaddr
+
 from cloudify import ctx
 from cloudify.decorators import operation
 from cloudify.exceptions import NonRecoverableError
@@ -91,7 +93,7 @@ def create(neutron_client, args, **kwargs):
                               PORT_OPENSTACK_TYPE,
                               args,
                               {'network_id': net_id})
-    _handle_fixed_ips(port)
+    _handle_fixed_ips(port, neutron_client)
     _handle_security_groups(port)
 
     p = neutron_client.create_port(
@@ -273,29 +275,101 @@ def _get_fixed_ip(port):
     return port['fixed_ips'][0]['ip_address'] if port['fixed_ips'] else None
 
 
-def _handle_fixed_ips(port):
-    fixed_ips_element = {}
+def _valid_subnet_ip(ip_address, subnet_dict):
+    """Check if ip_address is valid for subnet_dict['cidr']
+
+    :param ip_address: string
+    :param subnet_dict: dict with 'cidr' string
+    :return: bool
+    """
 
-    # checking for fixed ip property
-    if ctx.node.properties['fixed_ip']:
-        fixed_ips_element['ip_address'] = ctx.node.properties['fixed_ip']
+    try:
+        if netaddr.IPAddress(ip_address) in \
+                netaddr.IPNetwork(subnet_dict.get('cidr')):
+            return True
+    except TypeError:
+        pass
+    return False
+
+
+def _handle_fixed_ips(port, neutron_client):
+    """Combine IPs and Subnets for the Port fixed IPs list.
+
+    The Port object looks something this:
+    {
+      'port': {
+        'id': 'some-id',
+        'fixed_ips': [
+          {'subnet_id': 'subnet1', 'ip_address': '1.2.3.4'},
+          {'ip_address': '1.2.3.5'},
+          {'subnet_id': 'subnet3'},
+        ]
+      ...snip...
+    }
 
-    # checking for a connected subnet
-    subnet_id = get_openstack_id_of_single_connected_node_by_openstack_type(
-        ctx, SUBNET_OPENSTACK_TYPE, if_exists=True)
-    if subnet_id:
-        fixed_ips_element['subnet_id'] = subnet_id
+    We need to combine subnets and ips from three sources:
+    1) Fixed IPs and Subnets from the Port object.
+    2) Subnets from relationships to subnets.
+    3) A Fixed IP from node properties.
+
+    There are some issues:
+    1) Users can provide both subnets and relationships to subnets.
+    2) Recurrences of the subnet_id indicate a desire
+       for multiple IPs on that subnet.
+    3) If we provide a fixed_ip, we don't also know the
+       target subnet because of how the node properties are.
+       We should change that.
+       Have not yet changed that.
+       But will need to support both paths anyway.
+
+    :param port: An Openstack API Port Object.
+    :param neutron_client: Openstack Neutron Client.
+    :return: None
+    """
+
+    fixed_ips = port.get('fixed_ips', [])
+    subnet_ids_from_port = [net.get('subnet_id') for net in fixed_ips]
+    subnet_ids_from_rels = \
+        get_openstack_ids_of_connected_nodes_by_openstack_type(
+            ctx, SUBNET_OPENSTACK_TYPE)
+
+    # Add the subnets from relationships to the port subnets.
+    for subnet_from_rel in subnet_ids_from_rels:
+        if subnet_from_rel not in subnet_ids_from_port:
+            fixed_ips.append({'subnet_id': subnet_from_rel})
 
-    fixed_ips = port.get(
-        'fixed_ips',
-        [] if not fixed_ips_element else [fixed_ips_element])
     addresses = [ip.get('ip_address') for ip in fixed_ips]
-    subnets = [net.get('subnet_id') for net in fixed_ips]
-    # applying fixed ip parameter, if available
-    if fixed_ips_element and not \
-            (fixed_ips_element.get('ip_address') in addresses or
-             fixed_ips_element.get('subnet_id') in subnets):
-        fixed_ips.append(fixed_ips_element)
+    fixed_ip_from_props = ctx.node.properties['fixed_ip']
+
+    # If we have a fixed_ip from node props, we need to add it,
+    # but first try to match it with one of our subnets.
+    # The fixed_ip_element should be one of:
+    # 1) {'ip_address': 'x.x.x.x'}
+    # 2) {'subnet_id': '....'}
+    # 3) {'ip_address': 'x.x.x.x', 'subnet_id': '....'}
+    # show_subnet returns something like this:
+    # subnet = {
+    #   'subnet': {
+    #     'id': 'subnet1',
+    #     'cidr': '1.2.3.4/24',
+    #     'allocation_pools': [],
+    #     ...snip...
+    #   }
+    # }
+    if fixed_ip_from_props and not (fixed_ip_from_props in addresses):
+        fixed_ip_element = {'ip_address': fixed_ip_from_props}
+        for fixed_ip in fixed_ips:
+            subnet_id = fixed_ip.get('subnet_id')
+            if not _valid_subnet_ip(
+                    fixed_ip_from_props,
+                    neutron_client.show_subnet(subnet_id)):
+                continue
+            fixed_ip_element['subnet_id'] = subnet_id
+            del fixed_ips[fixed_ips.index(fixed_ip)]
+            break
+        fixed_ips.append(fixed_ip_element)
+
+    # Finally update the object.
     if fixed_ips:
         port['fixed_ips'] = fixed_ips
 

neutron_plugin/tests/test_port.py

@@ -14,7 +14,6 @@
 #    * limitations under the License.
 
 import unittest
-
 import mock
 
 import neutron_plugin.port
@@ -32,75 +31,79 @@ class TestPort(unittest.TestCase):
 
     def test_fixed_ips_no_fixed_ips(self):
         node_props = {'fixed_ip': ''}
+        mock_neutron = MockNeutronClient(update=True)
 
         with mock.patch(
                 'neutron_plugin.port.'
                 'get_openstack_id_of_single_connected_node_by_openstack_type',
-                self._get_connected_subnet_mock(return_empty=True)):
+                self._get_connected_subnets_mock(return_empty=True)):
             with mock.patch(
                     'neutron_plugin.port.ctx',
                     self._get_mock_ctx_with_node_properties(node_props)):
 
                 port = {}
-                neutron_plugin.port._handle_fixed_ips(port)
+                neutron_plugin.port._handle_fixed_ips(port, mock_neutron)
 
         self.assertNotIn('fixed_ips', port)
 
     def test_fixed_ips_subnet_only(self):
         node_props = {'fixed_ip': ''}
+        mock_neutron = MockNeutronClient(update=True)
 
         with mock.patch(
                 'neutron_plugin.port.'
-                'get_openstack_id_of_single_connected_node_by_openstack_type',
-                self._get_connected_subnet_mock(return_empty=False)):
+                'get_openstack_ids_of_connected_nodes_by_openstack_type',
+                self._get_connected_subnets_mock(return_empty=False)):
             with mock.patch(
                     'neutron_plugin.port.ctx',
                     self._get_mock_ctx_with_node_properties(node_props)):
 
                 port = {}
-                neutron_plugin.port._handle_fixed_ips(port)
+                neutron_plugin.port._handle_fixed_ips(port, mock_neutron)
 
         self.assertEquals([{'subnet_id': 'some-subnet-id'}],
                           port.get('fixed_ips'))
 
     def test_fixed_ips_ip_address_only(self):
         node_props = {'fixed_ip': '1.2.3.4'}
+        mock_neutron = MockNeutronClient(update=True)
 
         with mock.patch(
                 'neutron_plugin.port.'
                 'get_openstack_id_of_single_connected_node_by_openstack_type',
-                self._get_connected_subnet_mock(return_empty=True)):
+                self._get_connected_subnets_mock(return_empty=True)):
             with mock.patch(
                     'neutron_plugin.port.ctx',
                     self._get_mock_ctx_with_node_properties(node_props)):
 
                 port = {}
-                neutron_plugin.port._handle_fixed_ips(port)
+                neutron_plugin.port._handle_fixed_ips(port, mock_neutron)
 
         self.assertEquals([{'ip_address': '1.2.3.4'}],
                           port.get('fixed_ips'))
 
     def test_fixed_ips_subnet_and_ip_address(self):
         node_props = {'fixed_ip': '1.2.3.4'}
+        mock_neutron = MockNeutronClient(update=True)
 
         with mock.patch(
                 'neutron_plugin.port.'
-                'get_openstack_id_of_single_connected_node_by_openstack_type',
-                self._get_connected_subnet_mock(return_empty=False)):
+                'get_openstack_ids_of_connected_nodes_by_openstack_type',
+                self._get_connected_subnets_mock(return_empty=False)):
             with mock.patch(
                     'neutron_plugin.port.ctx',
                     self._get_mock_ctx_with_node_properties(node_props)):
 
                 port = {}
-                neutron_plugin.port._handle_fixed_ips(port)
+                neutron_plugin.port._handle_fixed_ips(port, mock_neutron)
 
         self.assertEquals([{'ip_address': '1.2.3.4',
                             'subnet_id': 'some-subnet-id'}],
                           port.get('fixed_ips'))
 
     @staticmethod
-    def _get_connected_subnet_mock(return_empty=True):
-        return lambda *args, **kw: None if return_empty else 'some-subnet-id'
+    def _get_connected_subnets_mock(return_empty=True):
+        return lambda *args, **kw: None if return_empty else ['some-subnet-id']
 
     @staticmethod
     def _get_mock_ctx_with_node_properties(properties):
@@ -125,6 +128,18 @@ def update_port(self, _, b, **__):
     def cosmo_get(self, *_, **__):
         return self.body['port']
 
+    def show_subnet(self, subnet_id=None):
+        subnet = {
+            'subnet': {
+                'id': subnet_id,
+            }
+        }
+        if subnet_id == 'some-subnet-id':
+            subnet['cidr'] = '1.2.3.0/24'
+        else:
+            subnet['cidr'] = '2.3.4.0/24'
+        return subnet
+
 
 class TestPortSG(unittest.TestCase):
     @mock.patch('openstack_plugin_common._handle_kw')

plugin.yaml

@@ -5,9 +5,9 @@
 plugins:
   openstack:
     executor: central_deployment_agent
-    source: https://github.com/cloudify-cosmo/cloudify-openstack-plugin/archive/2.9.7.zip
+    source: https://github.com/cloudify-cosmo/cloudify-openstack-plugin/archive/2.9.8.zip
     package_name: cloudify-openstack-plugin
-    package_version: '2.9.7'
+    package_version: '2.9.8'
 
 data_types:
   cloudify.openstack.types.custom_configuration:

setup.py

@@ -19,7 +19,7 @@
 setup(
     zip_safe=True,
     name='cloudify-openstack-plugin',
-    version='2.9.7',
+    version='2.9.8',
     author='Cloudify',
     author_email='hello@cloudify.co',
     packages=[