Merge "Use metadata IP as DHCP service IP for subnet without gateway IP"

networking_ovn/common/ovn_client.py

@@ -1207,18 +1207,27 @@ class OVNClient(object):
         return dhcp_options
 
     def _get_ovn_dhcpv4_opts(self, subnet, network, server_mac=None):
-        if not subnet['gateway_ip']:
+        metadata_port_ip = self._find_metadata_port_ip(
+            n_context.get_admin_context(), subnet)
+        # TODO(dongj): Currently the metadata port is created only when
+        # ovn_metadata_enabled is true, therefore this is a restriction for
+        # supporting DHCP of subnet without gateway IP.
+        # We will remove this restriction later.
+        service_id = subnet['gateway_ip'] or metadata_port_ip
+        if not service_id:
             return {}
 
         default_lease_time = str(config.get_ovn_dhcp_default_lease_time())
         mtu = network['mtu']
         options = {
-            'server_id': subnet['gateway_ip'],
+            'server_id': service_id,
             'lease_time': default_lease_time,
             'mtu': str(mtu),
-            'router': subnet['gateway_ip']
         }
 
+        if subnet['gateway_ip']:
+            options['router'] = subnet['gateway_ip']
+
         if server_mac:
             options['server_mac'] = server_mac
         else:
@@ -1229,26 +1238,24 @@ class OVNClient(object):
             dns_servers = '{%s}' % ', '.join(subnet['dns_nameservers'])
             options['dns_server'] = dns_servers
 
-        # If subnet hostroutes are defined, add them in the
-        # 'classless_static_route' dhcp option
-        classless_static_routes = "{"
-        metadata_port_ip = self._find_metadata_port_ip(
-            n_context.get_admin_context(), subnet)
+        routes = []
         if metadata_port_ip:
-            classless_static_routes += ("%s/32,%s, ") % (
-                metadata_agent.METADATA_DEFAULT_IP, metadata_port_ip)
+            routes.append('%s/32,%s' % (
+                metadata_agent.METADATA_DEFAULT_IP, metadata_port_ip))
 
-        for route in subnet['host_routes']:
-            classless_static_routes += ("%s,%s, ") % (
-                route['destination'], route['nexthop'])
+        # Add subnet host_routes to 'classless_static_route' dhcp option
+        routes.extend(['%s,%s' % (route['destination'], route['nexthop'])
+                      for route in subnet['host_routes']])
 
-        if classless_static_routes != "{":
+        if routes:
             # if there are static routes, then we need to add the
             # default route in this option. As per RFC 3442 dhcp clients
             # should ignore 'router' dhcp option (option 3)
             # if option 121 is present.
-            classless_static_routes += "0.0.0.0/0,%s}" % (subnet['gateway_ip'])
-            options['classless_static_route'] = classless_static_routes
+            if subnet['gateway_ip']:
+                routes.append('0.0.0.0/0,%s' % subnet['gateway_ip'])
+
+            options['classless_static_route'] = '{' + ', '.join(routes) + '}'
 
         return options
 

networking_ovn/tests/unit/ml2/test_mech_driver.py

@@ -1594,6 +1594,33 @@ class TestOVNMechansimDriverDHCPOptions(OVNMechanismDriverTestCase):
         self._test_get_ovn_dhcp_options_helper(subnet, network,
                                                expected_dhcp_options)
 
+    def test_get_ovn_dhcp_options_no_gw_ip_but_metadata_ip(self):
+        subnet = {'id': 'foo-subnet', 'network_id': 'network-id',
+                  'cidr': '10.0.0.0/24',
+                  'ip_version': 4,
+                  'enable_dhcp': True,
+                  'dns_nameservers': [],
+                  'host_routes': [],
+                  'gateway_ip': None}
+        network = {'id': 'network-id', 'mtu': 1400}
+
+        expected_dhcp_options = {
+            'cidr': '10.0.0.0/24',
+            'external_ids': {'subnet_id': 'foo-subnet',
+                             ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1'},
+            'options': {'server_id': '10.0.0.2',
+                        'server_mac': '01:02:03:04:05:06',
+                        'lease_time': str(12 * 60 * 60),
+                        'mtu': '1400',
+                        'classless_static_route':
+                            '{169.254.169.254/32,10.0.0.2}'}}
+
+        with mock.patch.object(self.mech_driver._ovn_client,
+                               '_find_metadata_port_ip',
+                               return_value='10.0.0.2'):
+            self._test_get_ovn_dhcp_options_helper(subnet, network,
+                                                   expected_dhcp_options)
+
     def test_get_ovn_dhcp_options_ipv6_subnet(self):
         subnet = {'id': 'foo-subnet', 'network_id': 'network-id',
                   'cidr': 'ae70::/24',