From 5f7b253fa6db2a8a40d3cc0dd34f16e4281b5937 Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Thu, 23 Aug 2012 11:22:59 +0100 Subject: Re-work the handling of firewall_driver default Fixes bug #1040430 We have a different default firewall driver for libvirt and xenapi, yet the sample config file currently contains: firewall_driver=nova.virt.firewall.IptablesFirewallDriver In the case of libvirt, it should actually be: firewall_driver=nova.virt.firewall.libvirt.IptablesFirewallDriver This is really easy for users to get confused about. Since we don't have a different sample config file for each hypervisor, the best we can do is to just not include the default in the sample config and have each hypervisor supply its default at runtime. DocImpact: update nova.conf docs Change-Id: Ie78371bcceac5a65978d695934e0246022f748a3 --- etc/nova/nova.conf.sample | 7 ++++--- nova/flags.py | 3 --- nova/tests/test_virt_drivers.py | 3 +-- nova/virt/firewall.py | 20 ++++++++++++++++---- nova/virt/libvirt/driver.py | 14 +++++++++----- nova/virt/libvirt/firewall.py | 3 --- nova/virt/xenapi/firewall.py | 5 ----- nova/virt/xenapi/vmops.py | 12 +++++++----- 8 files changed, 37 insertions(+), 30 deletions(-) diff --git a/etc/nova/nova.conf.sample b/etc/nova/nova.conf.sample index ede3003cb..9772c0262 100644 --- a/etc/nova/nova.conf.sample +++ b/etc/nova/nova.conf.sample @@ -250,9 +250,6 @@ # scheduler_manager=nova.scheduler.manager.SchedulerManager #### (StrOpt) full class name for the Manager for scheduler -# firewall_driver=nova.virt.firewall.IptablesFirewallDriver -#### (StrOpt) Firewall driver (defaults to iptables) - # host=nova #### (StrOpt) Name of this node. This can be an opaque identifier. It is #### not necessarily a hostname, FQDN, or IP address. However, @@ -1325,6 +1322,10 @@ ######## defined in nova.virt.firewall ######## +# firewall_driver= +#### (StrOpt) Firewall driver (defaults to hypervisor specific iptables +#### driver) + # allow_same_net_traffic=true #### (BoolOpt) Whether to allow network traffic from same network diff --git a/nova/flags.py b/nova/flags.py index 316e35e0d..08a136963 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -303,9 +303,6 @@ global_opts = [ cfg.StrOpt('scheduler_manager', default='nova.scheduler.manager.SchedulerManager', help='full class name for the Manager for scheduler'), - cfg.StrOpt('firewall_driver', - default='nova.virt.firewall.IptablesFirewallDriver', - help='Firewall driver (defaults to iptables)'), cfg.StrOpt('host', default=socket.gethostname(), help='Name of this node. This can be an opaque identifier. ' diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py index aaae975a4..f64270b1a 100644 --- a/nova/tests/test_virt_drivers.py +++ b/nova/tests/test_virt_drivers.py @@ -77,8 +77,7 @@ class _FakeDriverBackendTestCase(test.TestCase): nova.virt.libvirt.driver.libvirt_utils = fake_libvirt_utils nova.virt.libvirt.firewall.libvirt = fakelibvirt - self.flags(firewall_driver=nova.virt.libvirt.firewall.drivers[0], - rescue_image_id="2", + self.flags(rescue_image_id="2", rescue_kernel_id="3", rescue_ramdisk_id=None, libvirt_snapshots_directory='./') diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py index 762d1dc38..77f7b3054 100644 --- a/nova/virt/firewall.py +++ b/nova/virt/firewall.py @@ -21,6 +21,7 @@ from nova import context from nova import db from nova import flags from nova.openstack.common import cfg +from nova.openstack.common import importutils from nova.openstack.common import log as logging from nova import utils from nova.virt import netutils @@ -28,12 +29,23 @@ from nova.virt import netutils LOG = logging.getLogger(__name__) -allow_same_net_traffic_opt = cfg.BoolOpt('allow_same_net_traffic', - default=True, - help='Whether to allow network traffic from same network') +firewall_opts = [ + cfg.StrOpt('firewall_driver', + default=None, + help='Firewall driver ' + '(defaults to hypervisor specific iptables driver)'), + cfg.BoolOpt('allow_same_net_traffic', + default=True, + help='Whether to allow network traffic from same network'), +] FLAGS = flags.FLAGS -FLAGS.register_opt(allow_same_net_traffic_opt) +FLAGS.register_opts(firewall_opts) + + +def load_driver(default, *args, **kwargs): + fw_class = importutils.import_class(FLAGS.firewall_driver or default) + return fw_class(*args, **kwargs) class FirewallDriver(object): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 8cd6cfe42..c4ebcf931 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -74,8 +74,9 @@ from nova import utils from nova.virt import configdrive from nova.virt.disk import api as disk from nova.virt import driver +from nova.virt import firewall from nova.virt.libvirt import config -from nova.virt.libvirt import firewall +from nova.virt.libvirt import firewall as libvirt_firewall from nova.virt.libvirt import imagebackend from nova.virt.libvirt import imagecache from nova.virt.libvirt import utils as libvirt_utils @@ -195,6 +196,10 @@ FLAGS.register_opts(libvirt_opts) flags.DECLARE('live_migration_retry_count', 'nova.compute.manager') flags.DECLARE('vncserver_proxyclient_address', 'nova.vnc') +DEFAULT_FIREWALL_DRIVER = "%s.%s" % ( + libvirt_firewall.__name__, + libvirt_firewall.IptablesFirewallDriver.__name__) + def patch_tpool_proxy(): """eventlet.tpool.Proxy doesn't work with old-style class in __str__() @@ -264,10 +269,9 @@ class LibvirtDriver(driver.ComputeDriver): self._initiator = None self._wrapped_conn = None self.read_only = read_only - if FLAGS.firewall_driver not in firewall.drivers: - FLAGS.set_default('firewall_driver', firewall.drivers[0]) - fw_class = importutils.import_class(FLAGS.firewall_driver) - self.firewall_driver = fw_class(get_connection=self._get_connection) + self.firewall_driver = firewall.load_driver( + default=DEFAULT_FIREWALL_DRIVER, + get_connection=self._get_connection) self.vif_driver = importutils.import_object(FLAGS.libvirt_vif_driver) self.volume_drivers = {} for driver_str in FLAGS.libvirt_volume_drivers: diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py index 4591bdd13..b3c6106ff 100644 --- a/nova/virt/libvirt/firewall.py +++ b/nova/virt/libvirt/firewall.py @@ -28,9 +28,6 @@ import nova.virt.firewall as base_firewall LOG = logging.getLogger(__name__) FLAGS = flags.FLAGS -# The default Firewall driver must be listed at position 0 -drivers = ['nova.virt.libvirt.firewall.IptablesFirewallDriver', ] - try: import libvirt except ImportError: diff --git a/nova/virt/xenapi/firewall.py b/nova/virt/xenapi/firewall.py index 3c974fc0f..f2b90c74b 100644 --- a/nova/virt/xenapi/firewall.py +++ b/nova/virt/xenapi/firewall.py @@ -29,11 +29,6 @@ from nova.virt import netutils LOG = logging.getLogger(__name__) FLAGS = flags.FLAGS -# The default Firewall driver must be listed at position 0 -drivers = ['nova.virt.firewall.IptablesFirewallDriver', - 'nova.virt.firewall.NoopFirewallDriver', - 'nova.virt.xenapi.firewall.Dom0IptablesFirewallDriver', ] - class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): """ Dom0IptablesFirewallDriver class diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 0b49bff39..d233244b2 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -42,8 +42,8 @@ from nova.openstack.common import jsonutils from nova.openstack.common import log as logging from nova.openstack.common import timeutils from nova import utils +from nova.virt import firewall from nova.virt.xenapi import agent -from nova.virt.xenapi import firewall from nova.virt.xenapi import pool_states from nova.virt.xenapi import vm_utils from nova.virt.xenapi import volume_utils @@ -70,6 +70,9 @@ FLAGS.register_opts(xenapi_vmops_opts) flags.DECLARE('vncserver_proxyclient_address', 'nova.vnc') +DEFAULT_FIREWALL_DRIVER = "%s.%s" % ( + firewall.__name__, + firewall.IptablesFirewallDriver.__name__) RESIZE_TOTAL_STEPS = 5 @@ -151,10 +154,9 @@ class VMOps(object): self.compute_api = compute.API() self._session = session self.poll_rescue_last_ran = None - if FLAGS.firewall_driver not in firewall.drivers: - FLAGS.set_default('firewall_driver', firewall.drivers[0]) - fw_class = importutils.import_class(FLAGS.firewall_driver) - self.firewall_driver = fw_class(xenapi_session=self._session) + self.firewall_driver = firewall.load_driver( + default=DEFAULT_FIREWALL_DRIVER, + xenapi_session=self._session) vif_impl = importutils.import_class(FLAGS.xenapi_vif_driver) self.vif_driver = vif_impl(xenapi_session=self._session) self.default_root_dev = '/dev/sda' -- cgit