diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index fd5d26b22c..4f0cda5245 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -24,11 +24,6 @@ import wsmeext.pecan as wsme_pecan from ironic.api.controllers.v1 import base from ironic.openstack.common import log -from oslo.config import cfg - -CONF = cfg.CONF -CONF.import_opt('heartbeat_timeout', 'ironic.conductor.manager', - group='conductor') LOG = log.getLogger(__name__) @@ -69,6 +64,5 @@ class DriversController(rest.RestController): # will break from a single-line doc string. # This is a result of a bug in sphinxcontrib-pecanwsme # https://github.com/dreamhost/sphinxcontrib-pecanwsme/issues/8 - drivers = pecan.request.dbapi.list_active_conductor_drivers( - interval=CONF.conductor.heartbeat_timeout) - return DriverList.convert(drivers) + d2c = pecan.request.dbapi.get_active_driver_dict() + return DriverList.convert(d2c.keys()) diff --git a/ironic/common/hash_ring.py b/ironic/common/hash_ring.py index 6cc475ef20..0fc932279d 100644 --- a/ironic/common/hash_ring.py +++ b/ironic/common/hash_ring.py @@ -47,13 +47,13 @@ class HashRing(object): def __init__(self, hosts, replicas=CONF.hash_distribution_replicas): """Create a new hash ring across the specified hosts. - :param hosts: list of hosts which will be mapped + :param hosts: an iterable of hosts which will be mapped. :param replicas: number of hosts to map to each hash partition, or len(hosts), which ever is lesser. Default: CONF.hash_distribution_replicas """ - self.hosts = hosts + self.hosts = list(hosts) self.replicas = replicas if replicas <= len(hosts) else len(hosts) self.partition_shift = 32 - CONF.hash_partition_exponent self.part2host = array.array('H') diff --git a/ironic/db/api.py b/ironic/db/api.py index a35531e052..40722971d7 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -336,8 +336,12 @@ class Connection(object): """ @abc.abstractmethod - def list_active_conductor_drivers(self, interval): - """Retrieve a list of drivers supported by the registered conductors. + def get_active_driver_dict(self, interval): + """Retrieve drivers for the registered and active conductors. - :param interval: Time since last check-in of a conductor. + :param interval: Seconds since last check-in of a conductor. + :returns: A dict which maps driver names to the set of hosts + which support them. For example: + {driverA: set([host1, host2]), + driverB: set([host2, host3])} """ diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index c487e84b3a..312ca9c5b2 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -17,10 +17,10 @@ """SQLAlchemy storage backend.""" +import collections import datetime from oslo.config import cfg - from sqlalchemy.orm.exc import NoResultFound from ironic.common import exception @@ -39,6 +39,9 @@ CONF = cfg.CONF CONF.import_opt('connection', 'ironic.openstack.common.db.sqlalchemy.session', group='database') +CONF.import_opt('heartbeat_timeout', + 'ironic.conductor.manager', + group='conductor') LOG = log.getLogger(__name__) @@ -544,15 +547,18 @@ class Connection(api.Connection): if count == 0: raise exception.ConductorNotFound(conductor=hostname) - def list_active_conductor_drivers(self, interval): - # TODO(deva): add configurable default 'interval', somewhere higher - # up the code. This isn't a db-specific option. + def get_active_driver_dict(self, interval=None): + if interval is None: + interval = CONF.conductor.heartbeat_timeout + limit = timeutils.utcnow() - datetime.timedelta(seconds=interval) result = model_query(models.Conductor).\ filter(models.Conductor.updated_at >= limit).\ all() - driver_set = set() + # build mapping of drivers to the set of hosts which support them + d2c = collections.defaultdict(set) for row in result: - driver_set.update(set(row['drivers'])) - return list(driver_set) + for driver in row['drivers']: + d2c[driver].add(row['hostname']) + return d2c diff --git a/ironic/tests/db/test_conductor.py b/ironic/tests/db/test_conductor.py index 49d9ca3cb5..bf92244234 100644 --- a/ironic/tests/db/test_conductor.py +++ b/ironic/tests/db/test_conductor.py @@ -85,27 +85,87 @@ class DbConductorTestCase(base.DbTestCase): self.dbapi.touch_conductor, 'bad-hostname') - def test_list_active_conductor_drivers(self): - # create some conductors with different timestamps - now = datetime.datetime(2000, 1, 1, 0, 0) - then = now + datetime.timedelta(hours=1) + def test_get_active_driver_dict_one_host_no_driver(self): + h = 'fake-host' + expected = {} - d1 = [u'not-this-one'] - timeutils.set_time_override(override_time=now) - self._create_test_cdr(id=1, hostname='d1', drivers=d1) + timeutils.set_time_override() + self._create_test_cdr(hostname=h, drivers=[]) + result = self.dbapi.get_active_driver_dict(interval=1) + self.assertEqual(expected, result) - d2 = [u'foo', u'bar'] - d3 = [u'another'] - timeutils.set_time_override(override_time=then) - self._create_test_cdr(id=2, hostname='d2', drivers=d2) - self._create_test_cdr(id=3, hostname='d3', drivers=d3) + def test_get_active_driver_dict_one_host_one_driver(self): + h = 'fake-host' + d = 'fake-driver' + expected = {d: set([h])} - # verify that res contains d2 and d3, but not the old d1 - res = self.dbapi.list_active_conductor_drivers(interval=60) - drivers = d2 + d3 - self.assertEqual(sorted(res), sorted(drivers)) + timeutils.set_time_override() + self._create_test_cdr(hostname=h, drivers=[d]) + result = self.dbapi.get_active_driver_dict(interval=1) + self.assertEqual(expected, result) - # change the interval, and verify that d1 appears - res = self.dbapi.list_active_conductor_drivers(interval=7200) - drivers = d1 + d2 + d3 - self.assertEqual(sorted(res), sorted(drivers)) + def test_get_active_driver_dict_one_host_many_drivers(self): + h = 'fake-host' + d1 = 'driver-one' + d2 = 'driver-two' + expected = {d1: set([h]), d2: set([h])} + + timeutils.set_time_override() + self._create_test_cdr(hostname=h, drivers=[d1, d2]) + result = self.dbapi.get_active_driver_dict(interval=1) + self.assertEqual(expected, result) + + def test_get_active_driver_dict_many_hosts_one_driver(self): + h1 = 'host-one' + h2 = 'host-two' + d = 'fake-driver' + expected = {d: set([h1, h2])} + + timeutils.set_time_override() + self._create_test_cdr(id=1, hostname=h1, drivers=[d]) + self._create_test_cdr(id=2, hostname=h2, drivers=[d]) + result = self.dbapi.get_active_driver_dict(interval=1) + self.assertEqual(expected, result) + + def test_get_active_driver_dict_many_hosts_and_drivers(self): + h1 = 'host-one' + h2 = 'host-two' + h3 = 'host-three' + d1 = 'driver-one' + d2 = 'driver-two' + expected = {d1: set([h1, h2]), d2: set([h2, h3])} + + timeutils.set_time_override() + self._create_test_cdr(id=1, hostname=h1, drivers=[d1]) + self._create_test_cdr(id=2, hostname=h2, drivers=[d1, d2]) + self._create_test_cdr(id=3, hostname=h3, drivers=[d2]) + result = self.dbapi.get_active_driver_dict(interval=1) + self.assertEqual(expected, result) + + def test_get_active_driver_dict_with_old_conductor(self): + past = datetime.datetime(2000, 1, 1, 0, 0) + present = past + datetime.timedelta(minutes=2) + + d = 'common-driver' + + h1 = 'old-host' + d1 = 'old-driver' + timeutils.set_time_override(override_time=past) + self._create_test_cdr(id=1, hostname=h1, drivers=[d, d1]) + + h2 = 'new-host' + d2 = 'new-driver' + timeutils.set_time_override(override_time=present) + self._create_test_cdr(id=2, hostname=h2, drivers=[d, d2]) + + # verify that old-host does not show up in current list + one_minute = 60 + expected = {d: set([h2]), d2: set([h2])} + result = self.dbapi.get_active_driver_dict(interval=one_minute) + self.assertEqual(expected, result) + + # change the interval, and verify that old-host appears + two_minute = one_minute * 2 + expected = {d: set([h1, h2]), d1: set([h1]), d2: set([h2])} + result = self.dbapi.get_active_driver_dict(interval=two_minute) + self.assertEqual(expected, result)