Fix issues with using the database
* Fix issue with 'database locked' in tests * Fix issue with database thread safety Change-Id: Ieffbc42a7767d0790e57461d564fccf68c0274db
This commit is contained in:
Dmitry Tantsur
parent
d4c5f5050a
commit
7894c3d3e7
|
|
@ -32,7 +32,8 @@
|
|||
; Whether to authenticate with Keystone on discovery initialization endpoint.
|
||||
; Note that discovery postback endpoint is never authenticated.
|
||||
;authenticate = true
|
||||
; SQLite3 database to store nodes under discovery, defaults to in-memory.
|
||||
; SQLite3 database to store nodes under discovery, defaults to a temporary
|
||||
; file. Do not use :memory: here, it won't work.
|
||||
;database =
|
||||
|
||||
; Debug mode enabled/disabled.
|
||||
|
|
|
|||
|
|
@ -13,8 +13,11 @@
|
|||
|
||||
"""Cache for nodes currently under discovery."""
|
||||
|
||||
import atexit
|
||||
import logging
|
||||
import os
|
||||
import sqlite3
|
||||
import tempfile
|
||||
import time
|
||||
|
||||
from ironic_discoverd import conf
|
||||
|
|
@ -22,7 +25,7 @@ from ironic_discoverd import utils
|
|||
|
||||
|
||||
LOG = logging.getLogger("discoverd")
|
||||
_DB = None
|
||||
_DB_NAME = None
|
||||
_SCHEMA = """
|
||||
create table if not exists nodes
|
||||
(uuid text primary key, started_at real);
|
||||
|
|
@ -36,16 +39,25 @@ create table if not exists attributes
|
|||
|
||||
def init():
|
||||
"""Initialize the database."""
|
||||
global _DB
|
||||
conn = conf.get('discoverd', 'database') or ':memory:'
|
||||
_DB = sqlite3.connect(conn)
|
||||
_DB.executescript(_SCHEMA)
|
||||
global _DB_NAME
|
||||
_DB_NAME = conf.get('discoverd', 'database').strip()
|
||||
if not _DB_NAME:
|
||||
# We can't use in-memory, so we create a temporary file
|
||||
fd, _DB_NAME = tempfile.mkstemp(prefix='discoverd-')
|
||||
os.close(fd)
|
||||
|
||||
def cleanup():
|
||||
if os.path.exists(_DB_NAME):
|
||||
os.unlink(_DB_NAME)
|
||||
|
||||
atexit.register(cleanup)
|
||||
sqlite3.connect(_DB_NAME).executescript(_SCHEMA)
|
||||
|
||||
|
||||
def _db():
|
||||
if _DB is None:
|
||||
if _DB_NAME is None:
|
||||
init()
|
||||
return _DB
|
||||
return sqlite3.connect(_DB_NAME)
|
||||
|
||||
|
||||
def add_node(uuid, **attributes):
|
||||
|
|
@ -58,9 +70,9 @@ def add_node(uuid, **attributes):
|
|||
:param attributes: attributes known about this node (like macs, BMC etc)
|
||||
"""
|
||||
drop_node(uuid)
|
||||
with _db():
|
||||
_db().execute("insert into nodes(uuid, started_at) "
|
||||
"values(?, ?)", (uuid, time.time()))
|
||||
with _db() as db:
|
||||
db.execute("insert into nodes(uuid, started_at) "
|
||||
"values(?, ?)", (uuid, time.time()))
|
||||
for (name, value) in attributes.items():
|
||||
if not value:
|
||||
continue
|
||||
|
|
@ -68,9 +80,9 @@ def add_node(uuid, **attributes):
|
|||
value = [value]
|
||||
|
||||
try:
|
||||
_db().executemany("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
[(name, v, uuid) for v in value])
|
||||
db.executemany("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
[(name, v, uuid) for v in value])
|
||||
except sqlite3.IntegrityError:
|
||||
raise utils.DiscoveryFailed(
|
||||
'Some or all of %(name)s\'s %(value)s are already '
|
||||
|
|
@ -86,9 +98,9 @@ def macs_on_discovery():
|
|||
|
||||
def drop_node(uuid):
|
||||
"""Forget information about node with given uuid."""
|
||||
with _db():
|
||||
_db().execute("delete from nodes where uuid=?", (uuid,))
|
||||
_db().execute("delete from attributes where uuid=?", (uuid,))
|
||||
with _db() as db:
|
||||
db.execute("delete from nodes where uuid=?", (uuid,))
|
||||
db.execute("delete from attributes where uuid=?", (uuid,))
|
||||
|
||||
|
||||
def pop_node(**attributes):
|
||||
|
|
@ -101,6 +113,7 @@ def pop_node(**attributes):
|
|||
"""
|
||||
# NOTE(dtantsur): sorting is not required, but gives us predictability
|
||||
found = set()
|
||||
db = _db()
|
||||
for (name, value) in sorted(attributes.items()):
|
||||
if not value:
|
||||
LOG.warning('Empty value for attribute %s', name)
|
||||
|
|
@ -109,9 +122,9 @@ def pop_node(**attributes):
|
|||
value = [value]
|
||||
|
||||
LOG.debug('Trying to use %s %s for discovery' % (name, value))
|
||||
rows = _db().execute('select distinct uuid from attributes where ' +
|
||||
' OR '.join('name=? AND value=?' for _ in value),
|
||||
sum(([name, v] for v in value), [])).fetchall()
|
||||
rows = db.execute('select distinct uuid from attributes where ' +
|
||||
' OR '.join('name=? AND value=?' for _ in value),
|
||||
sum(([name, v] for v in value), [])).fetchall()
|
||||
if rows:
|
||||
found.update(item[0] for item in rows)
|
||||
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@
|
|||
import eventlet
|
||||
eventlet.monkey_patch(thread=False)
|
||||
|
||||
import os
|
||||
import time
|
||||
import unittest
|
||||
|
||||
|
|
@ -30,18 +31,22 @@ from ironic_discoverd import node_cache
|
|||
from ironic_discoverd import utils
|
||||
|
||||
|
||||
def init_conf():
|
||||
conf.init_conf()
|
||||
conf.CONF.add_section('discoverd')
|
||||
conf.CONF.set('discoverd', 'database', '')
|
||||
node_cache._DB = None
|
||||
class BaseTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
super(BaseTest, self).setUp()
|
||||
conf.init_conf()
|
||||
conf.CONF.add_section('discoverd')
|
||||
conf.CONF.set('discoverd', 'database', '')
|
||||
node_cache._DB_NAME = None
|
||||
self.db = node_cache._db()
|
||||
self.addCleanup(lambda: os.unlink(node_cache._DB_NAME))
|
||||
|
||||
|
||||
# FIXME(dtantsur): this test suite is far from being complete
|
||||
@patch.object(firewall, 'update_filters', autospec=True)
|
||||
@patch.object(node_cache, 'pop_node', autospec=True)
|
||||
@patch.object(utils, 'get_client', autospec=True)
|
||||
class TestProcess(unittest.TestCase):
|
||||
class TestProcess(BaseTest):
|
||||
def setUp(self):
|
||||
self.node = Mock(driver_info={'ipmi_address': '1.2.3.4'},
|
||||
properties={'cpu_arch': 'i386', 'local_gb': 40},
|
||||
|
|
@ -68,7 +73,6 @@ class TestProcess(unittest.TestCase):
|
|||
}
|
||||
}
|
||||
self.macs = ['11:22:33:44:55:66', 'broken', '', '66:55:44:33:22:11']
|
||||
init_conf()
|
||||
|
||||
def _do_test(self, client_mock, pop_mock, filters_mock):
|
||||
cli = client_mock.return_value
|
||||
|
|
@ -135,7 +139,7 @@ class TestProcess(unittest.TestCase):
|
|||
@patch.object(firewall, 'update_filters', autospec=True)
|
||||
@patch.object(node_cache, 'add_node', autospec=True)
|
||||
@patch.object(utils, 'get_client', autospec=True)
|
||||
class TestDiscover(unittest.TestCase):
|
||||
class TestDiscover(BaseTest):
|
||||
def setUp(self):
|
||||
super(TestDiscover, self).setUp()
|
||||
self.node1 = Mock(driver='pxe_ssh',
|
||||
|
|
@ -159,7 +163,6 @@ class TestDiscover(unittest.TestCase):
|
|||
instance_uuid=None,
|
||||
power_state='power off',
|
||||
extra={'on_discovery': True})
|
||||
init_conf()
|
||||
|
||||
@patch.object(time, 'time', lambda: 42.0)
|
||||
def test_ok(self, client_mock, add_mock, filters_mock, spawn_mock):
|
||||
|
|
@ -311,9 +314,9 @@ class TestDiscover(unittest.TestCase):
|
|||
self.assertFalse(add_mock.called)
|
||||
|
||||
|
||||
class TestApi(unittest.TestCase):
|
||||
class TestApi(BaseTest):
|
||||
def setUp(self):
|
||||
init_conf()
|
||||
super(TestApi, self).setUp()
|
||||
main.app.config['TESTING'] = True
|
||||
self.app = main.app.test_client()
|
||||
|
||||
|
|
@ -394,11 +397,7 @@ class TestClient(unittest.TestCase):
|
|||
|
||||
@patch.object(eventlet.greenthread, 'sleep', autospec=True)
|
||||
@patch.object(utils, 'get_client')
|
||||
class TestCheckIronicAvailable(unittest.TestCase):
|
||||
def setUp(self):
|
||||
super(TestCheckIronicAvailable, self).setUp()
|
||||
init_conf()
|
||||
|
||||
class TestCheckIronicAvailable(BaseTest):
|
||||
def test_ok(self, client_mock, sleep_mock):
|
||||
utils.check_ironic_available()
|
||||
client_mock.return_value.driver.list.assert_called_once_with()
|
||||
|
|
@ -421,22 +420,22 @@ class TestCheckIronicAvailable(unittest.TestCase):
|
|||
self.assertEqual(attempts, sleep_mock.call_count)
|
||||
|
||||
|
||||
class TestNodeCache(unittest.TestCase):
|
||||
class TestNodeCache(BaseTest):
|
||||
def setUp(self):
|
||||
super(TestNodeCache, self).setUp()
|
||||
init_conf()
|
||||
self.db = node_cache._db()
|
||||
self.node = Mock(driver_info={'ipmi_address': '1.2.3.4'},
|
||||
uuid='uuid')
|
||||
self.macs = ['11:22:33:44:55:66', '66:55:44:33:22:11']
|
||||
|
||||
def test_add_node(self):
|
||||
# Ensure previous node information is cleared
|
||||
self.db.execute("insert into nodes(uuid) values(?)", (self.node.uuid,))
|
||||
self.db.execute("insert into nodes(uuid) values('uuid2')")
|
||||
self.db.execute("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
('mac', '11:22:11:22:11:22', self.node.uuid))
|
||||
with self.db:
|
||||
self.db.execute("insert into nodes(uuid) values(?)",
|
||||
(self.node.uuid,))
|
||||
self.db.execute("insert into nodes(uuid) values('uuid2')")
|
||||
self.db.execute("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
('mac', '11:22:11:22:11:22', self.node.uuid))
|
||||
|
||||
node_cache.add_node(self.node.uuid, mac=self.macs,
|
||||
bmc_address='1.2.3.4', foo=None)
|
||||
|
|
@ -454,21 +453,25 @@ class TestNodeCache(unittest.TestCase):
|
|||
res)
|
||||
|
||||
def test_add_node_duplicate_mac(self):
|
||||
self.db.execute("insert into nodes(uuid) values(?)", ('another-uuid',))
|
||||
self.db.execute("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
('mac', '11:22:11:22:11:22', 'another-uuid'))
|
||||
with self.db:
|
||||
self.db.execute("insert into nodes(uuid) values(?)",
|
||||
('another-uuid',))
|
||||
self.db.execute("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
('mac', '11:22:11:22:11:22', 'another-uuid'))
|
||||
|
||||
self.assertRaises(utils.DiscoveryFailed,
|
||||
node_cache.add_node,
|
||||
self.node.uuid, mac=['11:22:11:22:11:22'])
|
||||
|
||||
def test_drop_node(self):
|
||||
self.db.execute("insert into nodes(uuid) values(?)", (self.node.uuid,))
|
||||
self.db.execute("insert into nodes(uuid) values('uuid2')")
|
||||
self.db.execute("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
('mac', '11:22:11:22:11:22', self.node.uuid))
|
||||
with self.db:
|
||||
self.db.execute("insert into nodes(uuid) values(?)",
|
||||
(self.node.uuid,))
|
||||
self.db.execute("insert into nodes(uuid) values('uuid2')")
|
||||
self.db.execute("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
('mac', '11:22:11:22:11:22', self.node.uuid))
|
||||
|
||||
node_cache.drop_node(self.node.uuid)
|
||||
|
||||
|
|
@ -478,20 +481,20 @@ class TestNodeCache(unittest.TestCase):
|
|||
"select * from attributes").fetchall())
|
||||
|
||||
def test_macs_on_discovery(self):
|
||||
self.db.execute("insert into nodes(uuid) values(?)", (self.node.uuid,))
|
||||
self.db.executemany("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
[('mac', '11:22:11:22:11:22', self.node.uuid),
|
||||
('mac', '22:11:22:11:22:11', self.node.uuid)])
|
||||
with self.db:
|
||||
self.db.execute("insert into nodes(uuid) values(?)",
|
||||
(self.node.uuid,))
|
||||
self.db.executemany("insert into attributes(name, value, uuid) "
|
||||
"values(?, ?, ?)",
|
||||
[('mac', '11:22:11:22:11:22', self.node.uuid),
|
||||
('mac', '22:11:22:11:22:11', self.node.uuid)])
|
||||
self.assertEqual({'11:22:11:22:11:22', '22:11:22:11:22:11'},
|
||||
node_cache.macs_on_discovery())
|
||||
|
||||
|
||||
class TestNodeCachePop(unittest.TestCase):
|
||||
class TestNodeCachePop(BaseTest):
|
||||
def setUp(self):
|
||||
super(TestNodeCachePop, self).setUp()
|
||||
init_conf()
|
||||
self.db = node_cache._db()
|
||||
self.uuid = 'uuid'
|
||||
self.macs = ['11:22:33:44:55:66', '66:55:44:33:22:11']
|
||||
self.macs2 = ['00:00:00:00:00:00']
|
||||
|
|
|
|||
Loading…
Reference in New Issue