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

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=<None>
+#### (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'