From 420fe0c5abdf904d84ce40e7126ba13f39b15736 Mon Sep 17 00:00:00 2001
From: Lucas Alvares Gomes <lucasagomes@gmail.com>
Date: Thu, 24 Mar 2016 14:48:12 +0000
Subject: [PATCH] Fix: Duplicated driver causes conductor to fail

This patch is fixing a problem which causes the conductor to get confused
and error out on startup in case there's a duplicated entry in the
enabled_drivers configuration option.

Closes-Bug: #1561564
Change-Id: Ib72bcde9b32d3c0b4b237068aa53146f58ea10a2
---
 ironic/common/driver_factory.py               | 21 +++++++++++++++----
 .../tests/unit/common/test_driver_factory.py  |  8 +++++++
 ...licated-driver-entry-775370ad84736206.yaml |  5 +++++
 3 files changed, 30 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/duplicated-driver-entry-775370ad84736206.yaml

diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py
index 191ced70a0..6e834d4f71 100644
--- a/ironic/common/driver_factory.py
+++ b/ironic/common/driver_factory.py
@@ -23,6 +23,7 @@ from stevedore import dispatch
 from ironic.common import exception
 from ironic.common.i18n import _
 from ironic.common.i18n import _LI
+from ironic.common.i18n import _LW
 from ironic.drivers import base as driver_base
 
 
@@ -136,6 +137,18 @@ class DriverFactory(object):
         if cls._extension_manager:
             return
 
+        # Check for duplicated driver entries and warn the operator
+        # about them
+        counter = collections.Counter(CONF.enabled_drivers).items()
+        duplicated_drivers = list(dup for (dup, i) in counter if i > 1)
+        if duplicated_drivers:
+            LOG.warning(_LW('The driver(s) "%s" is/are duplicated in the '
+                            'list of enabled_drivers. Please check your '
+                            'configuration file.'),
+                        ', '.join(duplicated_drivers))
+
+        enabled_drivers = set(CONF.enabled_drivers)
+
         # NOTE(deva): Drivers raise "DriverLoadError" if they are unable to be
         #             loaded, eg. due to missing external dependencies.
         #             We capture that exception, and, only if it is for an
@@ -147,13 +160,13 @@ class DriverFactory(object):
         def _catch_driver_not_found(mgr, ep, exc):
             # NOTE(deva): stevedore loads plugins *before* evaluating
             #             _check_func, so we need to check here, too.
-            if ep.name in CONF.enabled_drivers:
+            if ep.name in enabled_drivers:
                 if not isinstance(exc, exception.DriverLoadError):
                     raise exception.DriverLoadError(driver=ep.name, reason=exc)
                 raise exc
 
         def _check_func(ext):
-            return ext.name in CONF.enabled_drivers
+            return ext.name in enabled_drivers
 
         cls._extension_manager = (
             dispatch.NameDispatchExtensionManager(
@@ -164,10 +177,10 @@ class DriverFactory(object):
 
         # NOTE(deva): if we were unable to load any configured driver, perhaps
         #             because it is not present on the system, raise an error.
-        if (sorted(CONF.enabled_drivers) !=
+        if (sorted(enabled_drivers) !=
                 sorted(cls._extension_manager.names())):
             found = cls._extension_manager.names()
-            names = [n for n in CONF.enabled_drivers if n not in found]
+            names = [n for n in enabled_drivers if n not in found]
             # just in case more than one could not be found ...
             names = ', '.join(names)
             raise exception.DriverNotFound(driver_name=names)
diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py
index fae28f46a9..e42fe15410 100644
--- a/ironic/tests/unit/common/test_driver_factory.py
+++ b/ironic/tests/unit/common/test_driver_factory.py
@@ -64,6 +64,14 @@ class DriverLoadTestCase(base.TestCase):
             driver_factory.DriverFactory._init_extension_manager()
             self.assertEqual(2, mock_em.call_count)
 
+    @mock.patch.object(driver_factory.LOG, 'warning', autospec=True)
+    def test_driver_duplicated_entry(self, mock_log):
+        self.config(enabled_drivers=['fake', 'fake'])
+        driver_factory.DriverFactory._init_extension_manager()
+        self.assertEqual(
+            ['fake'], driver_factory.DriverFactory._extension_manager.names())
+        self.assertTrue(mock_log.called)
+
 
 class GetDriverTestCase(base.TestCase):
     def setUp(self):
diff --git a/releasenotes/notes/duplicated-driver-entry-775370ad84736206.yaml b/releasenotes/notes/duplicated-driver-entry-775370ad84736206.yaml
new file mode 100644
index 0000000000..bb7be7e557
--- /dev/null
+++ b/releasenotes/notes/duplicated-driver-entry-775370ad84736206.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - Fixes a problem which causes the conductor to error out on startup
+    in case there's a duplicated entry in the enabled_drivers configuration
+    option.