From 2807ea5464e610b4ac77d95f9216b86b5f36bde0 Mon Sep 17 00:00:00 2001 From: Ben Nemec Date: Mon, 13 May 2013 23:06:11 +0000 Subject: [PATCH] Don't inject settings for dynamic network When using Quantum for networking and setting flat_injected to inject network settings into the guest VM, Nova injects a static network configuration for networks using DHCP. This change checks for the existence of a dhcp server and, if found, does not inject static settings for that network. Due to limitations of the legacy network info, the fix only applies when new-style network info is passed to the injection template function. The only code still passing in legacy network info is in libvirt, and there is work ongoing to remove that dependency. Fixes bug 1163985 Fixes bug 1112659 Change-Id: I8d3f16bde22e1c6dc0d2432aa263e2b15ae5c93a --- nova/network/model.py | 12 ++- nova/tests/network/test_network_info.py | 76 ++++++++++++++++ nova/virt/netutils.py | 114 +++++++++++++++++++++--- 3 files changed, 187 insertions(+), 15 deletions(-) diff --git a/nova/network/model.py b/nova/network/model.py index 28651737bc56..cf01d98cfe1a 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -293,6 +293,13 @@ class VIF(Model): return vif +def get_netmask(ip, subnet): + """Returns the netmask appropriate for injection into a guest.""" + if ip['version'] == 4: + return str(subnet.as_netaddr().netmask) + return subnet.as_netaddr()._prefixlen + + class NetworkInfo(list): """Stores and manipulates network information for a Nova instance.""" @@ -325,10 +332,7 @@ class NetworkInfo(list): return ip['address'] def fixed_ip_dict(ip, subnet): - if ip['version'] == 4: - netmask = str(subnet.as_netaddr().netmask) - else: - netmask = subnet.as_netaddr()._prefixlen + netmask = get_netmask(ip, subnet) return {'ip': ip['address'], 'enabled': '1', diff --git a/nova/tests/network/test_network_info.py b/nova/tests/network/test_network_info.py index 960f3583a9d1..56522e6a5f42 100644 --- a/nova/tests/network/test_network_info.py +++ b/nova/tests/network/test_network_info.py @@ -2,6 +2,7 @@ # Copyright 2011 OpenStack Foundation # All Rights Reserved. +# Copyright 2013 IBM Corp. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -19,6 +20,7 @@ from nova import exception from nova.network import model from nova import test from nova.tests import fake_network_cache_model +from nova.virt import netutils class RouteTests(test.TestCase): @@ -347,3 +349,77 @@ class NetworkInfoTests(test.TestCase): [fake_network_cache_model.new_ip({'address': '10.10.0.2'}), fake_network_cache_model.new_ip( {'address': '10.10.0.3'})] * 4) + + def _test_injected_network_template(self, should_inject, use_ipv6=False, + legacy=False): + """Check that netutils properly decides whether to inject based on + whether the supplied subnet is static or dynamic. + """ + network = fake_network_cache_model.new_network({'subnets': []}) + if should_inject: + network.add_subnet(fake_network_cache_model.new_subnet()) + if use_ipv6: + gateway_ip = fake_network_cache_model.new_ip(dict( + address='1234:567::1')) + ip = fake_network_cache_model.new_ip(dict( + address='1234:567::2')) + subnet_dict = dict( + cidr='1234:567::/48', + gateway=gateway_ip, + ips=[ip]) + network.add_subnet(fake_network_cache_model.new_subnet( + subnet_dict)) + else: + subnet_dict = dict(dhcp_server='10.10.0.1') + network.add_subnet(fake_network_cache_model.new_subnet( + subnet_dict)) + # Behave as though CONF.flat_injected is True + network['meta']['injected'] = True + vif = fake_network_cache_model.new_vif({'network': network}) + ninfo = model.NetworkInfo([vif]) + if legacy: + ninfo = ninfo.legacy() + + template = netutils.get_injected_network_template(ninfo, + use_ipv6=use_ipv6) + + # NOTE(bnemec): There is a bug with legacy network info that causes + # it to inject regardless of whether the network is static or dynamic. + # This can't be fixed without changes that would potentially break + # existing code, so until legacy network info goes away this test + # will just ignore the improper behavior. + if not should_inject and not legacy: + self.assertTrue(template is None) + else: + self.assertTrue('auto eth0' in template) + self.assertTrue('iface eth0 inet static' in template) + self.assertTrue('address 10.10.0.2' in template) + self.assertTrue('netmask 255.255.255.0' in template) + self.assertTrue('broadcast 10.10.0.255' in template) + self.assertTrue('gateway 10.10.0.1' in template) + self.assertTrue('dns-nameservers 1.2.3.4 2.3.4.5' in template) + if use_ipv6: + self.assertTrue('iface eth0 inet6 static' in template) + self.assertTrue('address 1234:567::2' in template) + self.assertTrue('netmask 48' in template) + self.assertTrue('gateway 1234:567::1' in template) + + def test_injection_static(self): + self._test_injected_network_template(should_inject=True) + + def test_injection_static_ipv6(self): + self._test_injected_network_template(should_inject=True, use_ipv6=True) + + def test_injection_dynamic(self): + self._test_injected_network_template(should_inject=False) + + def test_injection_static_legacy(self): + self._test_injected_network_template(should_inject=True, legacy=True) + + def test_injection_static_ipv6_legacy(self): + self._test_injected_network_template(should_inject=True, + use_ipv6=True, + legacy=True) + + def test_injection_dynamic_legacy(self): + self._test_injected_network_template(should_inject=False, legacy=True) diff --git a/nova/virt/netutils.py b/nova/virt/netutils.py index d38197948136..301fa3b09ce5 100644 --- a/nova/virt/netutils.py +++ b/nova/virt/netutils.py @@ -4,6 +4,7 @@ # Administrator of the National Aeronautics and Space Administration. # All Rights Reserved. # Copyright (c) 2010 Citrix Systems, Inc. +# Copyright 2013 IBM Corp. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -25,6 +26,8 @@ import netaddr from oslo.config import cfg +from nova.network import model + CONF = cfg.CONF CONF.import_opt('use_ipv6', 'nova.netconf') CONF.import_opt('injected_network_template', 'nova.virt.disk.api') @@ -55,6 +58,91 @@ def get_ip_version(cidr): return int(net.version) +def get_non_legacy_network_template(network_info, use_ipv6=CONF.use_ipv6, + template=CONF.injected_network_template): + """A new version of get_injected_network_template that does not rely on + legacy network info. + + Returns a rendered network template for the given network_info. When + libvirt's dependency on using legacy network info for network config + injection goes away, this function can replace + get_injected_network_template entirely. + + :param network_info: + :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info` + :param use_ipv6: If False, do not return IPv6 template information + even if an IPv6 subnet is present in network_info. + :param template: Path to the interfaces template file. + """ + if not (network_info and template): + return + + nets = [] + ifc_num = -1 + ipv6_is_available = False + + for vif in network_info: + if not vif['network'] or not vif['network']['subnets']: + continue + + network = vif['network'] + # NOTE(bnemec): The template only supports a single subnet per + # interface and I'm not sure how/if that can be fixed, so this + # code only takes the first subnet of the appropriate type. + subnet_v4 = [i for i in network['subnets'] if i['version'] == 4][0] + subnet_v6 = [i for i in network['subnets'] if i['version'] == 6] + if subnet_v6: + subnet_v6 = subnet_v6[0] + + ifc_num += 1 + + if (not network.get_meta('injected') or not subnet_v4['ips'] or + subnet_v4.get_meta('dhcp_server') is not None): + continue + + ip = subnet_v4['ips'][0] + address = ip['address'] + netmask = model.get_netmask(ip, subnet_v4) + gateway = '' + if subnet_v4['gateway']: + gateway = subnet_v4['gateway']['address'] + broadcast = str(subnet_v4.as_netaddr().broadcast) + dns = ' '.join([i['address'] for i in subnet_v4['dns']]) + # NOTE(bnemec): I don't think this code would handle a pure IPv6 + # environment properly, but I don't have such an environment in + # which to test/fix that. + address_v6 = None + gateway_v6 = None + netmask_v6 = None + have_ipv6 = (use_ipv6 and subnet_v6) + if have_ipv6: + if subnet_v6['ips']: + ipv6_is_available = True + ip_v6 = subnet_v6['ips'][0] + address_v6 = ip_v6['address'] + netmask_v6 = model.get_netmask(ip_v6, subnet_v6) + gateway_v6 = '' + if subnet_v6['gateway']: + gateway_v6 = subnet_v6['gateway']['address'] + + net_info = {'name': 'eth%d' % ifc_num, + 'address': address, + 'netmask': netmask, + 'gateway': gateway, + 'broadcast': broadcast, + 'dns': dns, + 'address_v6': address_v6, + 'gateway_v6': gateway_v6, + 'netmask_v6': netmask_v6, + } + nets.append(net_info) + + if not nets: + return + + return build_template(template, nets, ipv6_is_available) + + def get_injected_network_template(network_info, use_ipv6=CONF.use_ipv6, template=CONF.injected_network_template): """ @@ -62,17 +150,20 @@ def get_injected_network_template(network_info, use_ipv6=CONF.use_ipv6, :param network_info: :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info` - - Note: this code actually depends on the legacy network_info, but will - convert the type itself if necessary. + :param use_ipv6: If False, do not return IPv6 template information + even if an IPv6 subnet is present in network_info. + :param template: Path to the interfaces template file. """ - if network_info is None: - return None + if not (network_info and template): + return - # the code below depends on the legacy 'network_info' - if hasattr(network_info, 'legacy'): - network_info = network_info.legacy() + # If we're passed new network_info, make use of it instead of forcing + # it to the legacy format required below. + if isinstance(network_info, model.NetworkInfo): + return get_non_legacy_network_template(network_info, + use_ipv6, + template) nets = [] ifc_num = -1 @@ -107,11 +198,12 @@ def get_injected_network_template(network_info, use_ipv6=CONF.use_ipv6, nets.append(net_info) if have_injected_networks is False: - return None + return - if not template: - return None + return build_template(template, nets, ipv6_is_available) + +def build_template(template, nets, ipv6_is_available): _late_load_cheetah() ifc_template = open(template).read()