From d7927f11307aedb77cd941794b644d1d60595ac4 Mon Sep 17 00:00:00 2001 From: John-Mark Gurney Date: Mon, 21 Dec 2020 19:49:22 -0800 Subject: [PATCH] push down code for reserve/release into BoardImpl... This makes the separation between the implementation and the API more clean, as before the API was doing too much work.. --- bitelab/__init__.py | 345 ++++++++++++++++++++++++++++++---------- bitelab/config.py | 1 - fixtures/board_conf.ucl | 2 + 3 files changed, 259 insertions(+), 89 deletions(-) diff --git a/bitelab/__init__.py b/bitelab/__init__.py index 21e9c9b..9fcdaed 100644 --- a/bitelab/__init__.py +++ b/bitelab/__init__.py @@ -73,6 +73,11 @@ import unittest import urllib import websockets +# Silence warnings with a sledge hammer, since I'm tired of them, and +# I can't get warnings.filterwarnings to work. +import warnings +warnings.warn = lambda *args, **kwargs: None + epsilon = sys.float_info.epsilon # fix up parse_socket_addr for hypercorn @@ -206,7 +211,8 @@ class TimeOut(Attribute): self._exp = self._cb.when() async def activate(self, brd): - assert brd.lock.locked() + if not brd.lock.locked(): + raise RuntimeError('code error, board not locked') loop = asyncio.get_running_loop() self._brd = brd @@ -215,7 +221,8 @@ class TimeOut(Attribute): self._task = None async def deactivate(self, brd): - assert brd.lock.locked() + if not brd.lock.locked(): + raise RuntimeError('code error, board not locked') if self._cb is not None: self._cb.cancel() @@ -289,7 +296,7 @@ class BoardImpl: be destroyed before the operation completes. ''' - def __init__(self, name: str, brdclass: str, options): + def __init__(self, name: str, brdclass: str, setupscript: str, options): ''' name: name of the board. brdclass: class that the board belongs to. @@ -300,8 +307,10 @@ class BoardImpl: self.name = name self.brdclass = brdclass + self.setupscript = setupscript self.options = options self.reserved = False + self.user = None self.attrmap = {} self.lock = asyncio.Lock() for i in options: @@ -318,11 +327,28 @@ class BoardImpl: def __repr__(self): #pragma: no cover return repr(Board.from_orm(self)) - async def reserve(self): + async def reserve(self, user, sshpubkey=None): '''Reserve the board.''' - assert self.lock.locked() and not self.reserved + if not self.lock.locked(): + raise RuntimeError('code error, board not locked') + + if self.reserved: + raise RuntimeError('Board currently reserved.') + + args = ( self.setupscript, 'reserve', + self.name, user, ) + if sshpubkey is not None: + args += (sshpubkey, ) + sub = await asyncio.create_subprocess_exec(*args, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout, stderr = await sub.communicate() + if sub.returncode: + raise RuntimeError(sub.returncode, stderr) + + self.add_info(json.loads(stdout)) + self.user = user self.reserved = True await self.activate() @@ -330,16 +356,44 @@ class BoardImpl: async def release(self): '''Release the board.''' - assert self.lock.locked() and self.reserved + if not self.lock.locked(): + raise RuntimeError('code error, board not locked') + + if not self.reserved: + raise RuntimeError('Board not reserved.') await self.deactivate() + env = os.environ.copy() + addkeys = { 'iface', 'ip', 'devfsrule', 'devfspath' } + env.update((k, self.attrs[k]) for k in addkeys if k in self.attrs) + + sub = await asyncio.create_subprocess_exec( + self.setupscript, 'release', self.name, self.user, env=env, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout, stderr = await sub.communicate() + retcode = sub.returncode + if retcode: + logging.error('release script failure: ' + + 'board: %s, ret: %s, stderr: %s' % (repr(self.name), + retcode, repr(stderr))) + raise RuntimeError( + 'Failed to release board, ret: %d, stderr: %s' % + (retcode, repr(stderr)) + ) + + self.clean_info() + self.reserved = False async def update_attrs(self, **attrs): '''Set the various attrs that are specified.''' - assert self.lock.locked() and self.reserved + if not self.lock.locked(): + raise RuntimeError('code error, board not locked') + + if not self.reserved: + raise RuntimeError('code error, board not reserved') for i in attrs: self.attrcache[i] = await self.attrmap[i].setvalue(attrs[i]) @@ -356,7 +410,11 @@ class BoardImpl: ethernet interface to the jail's vnet, or starting the timeout.''' - assert self.lock.locked() and self.reserved + if not self.lock.locked(): + raise RuntimeError('code error, board not locked') + + if not self.reserved: + raise RuntimeError('code error, board not reserved') for i in self.attrmap.values(): await i.activate(self) @@ -366,7 +424,11 @@ class BoardImpl: just before release. This is to clean up anything like scheduled tasks.''' - assert self.lock.locked() and self.reserved + if not self.lock.locked(): + raise RuntimeError('code error, board not locked') + + if not self.reserved: + raise RuntimeError('code error, board not reserved') for i in self.attrmap.values(): await i.deactivate(self) @@ -404,13 +466,13 @@ class BoardManager(object): snmppower=SNMPPower, ) - def __init__(self, cls_info, boards): + def __init__(self, cls_info, boards, setup_script): # add the name to the classes classes = { k: dict(clsname=k, **cls_info[k]) for k in cls_info } self.board_class_info = classes self.boards = dict(**{ x.name: x for x in - (BoardImpl(**y) for y in boards)}) + (BoardImpl(setupscript=setup_script, **y) for y in boards)}) @classmethod def from_settings(cls, settings): @@ -429,7 +491,7 @@ class BoardManager(object): opt = i['options'] opt[:] = [ makeopt(x) for x in opt ] - return cls(classes, brds) + return cls(classes, brds, setup_script=conf['setup_script']) def classes(self): return self.board_class_info @@ -605,7 +667,6 @@ async def reserve_board(board_id_or_class, # obrdreq.user == user brdreq = await data.BoardStatus.objects.get(board=board_id, user=user) - await brd.reserve() # XXX - orm isn't doing it's job here except sqlite3.IntegrityError: raise BITEError( @@ -614,20 +675,11 @@ async def reserve_board(board_id_or_class, board=Board.from_orm(brd)), ) - # Initialize board + # Reserve board try: - args = ( settings.setup_script, 'reserve', - brd.name, user, ) - if sshpubkey is not None: - args += (sshpubkey, ) - sub = await asyncio.create_subprocess_exec(*args, - stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = await sub.communicate() - if sub.returncode: - raise RuntimeError(sub.returncode, stderr) + await brd.reserve(user, sshpubkey) except Exception as e: await brdreq.delete() - await brd.release() if isinstance(e, RuntimeError): retcode, stderr = e.args raise BITEError( @@ -639,10 +691,6 @@ async def reserve_board(board_id_or_class, ) raise - brd.add_info(json.loads(stdout)) - - await brd.activate() - await log_event('reserve', user=user, board=brd) await brd.update() @@ -789,33 +837,16 @@ async def release_board(board_id, user: str = Depends(lookup_user), board=Board.from_orm(brd)), ) - await brd.deactivate() - - env = os.environ.copy() - addkeys = { 'iface', 'ip', 'devfsrule', 'devfspath' } - env.update((k, brd.attrs[k]) for k in addkeys if k in brd.attrs) - - sub = await asyncio.create_subprocess_exec( - settings.setup_script, 'release', brd.name, user, env=env, - stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = await sub.communicate() - retcode = sub.returncode - if retcode: - logging.error('release script failure: ' + - 'board: %s, ret: %s, stderr: %s' % (repr(brd.name), - retcode, repr(stderr))) + try: + await brd.release() + except RuntimeError as e: raise BITEError( status_code=HTTP_500_INTERNAL_SERVER_ERROR, - errobj=Error(error= - 'Failed to release board, ret: %d, stderr: %s' % - (retcode, repr(stderr)), - board=Board.from_orm(brd)), + errobj=Error(error=e.args[0], + board=Board.from_orm(brd)), ) await data.BoardStatus.delete(brduser) - await brd.release() - - brd.clean_info() await brd.update() @@ -906,7 +937,6 @@ class TestCommon(unittest.IsolatedAsyncioTestCase): # setup settings self.settings = config.Settings(db_file=self.dbtempfile.name, - setup_script='somesetupscript', board_conf = os.path.join('fixtures', 'board_conf.ucl') ) @@ -991,11 +1021,12 @@ class TestWebSocket(TestCommon): stdout=2) # that when the board is reserved by the wrong user + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) brd = self.brdmgr.boards['cora-1'] obrdreq = await self.data.BoardStatus.objects.create( board='cora-1', user='bar') async with brd.lock: - await brd.reserve() + await brd.reserve('bar') # that it fails with self.assertRaisesRegex(RuntimeError, 'Board reserved by \'bar\'.'): @@ -1103,19 +1134,22 @@ class TestBiteLabAPI(TestCommon): # that when snmpget returns False sg.return_value = False - # that when the setup script will fail - wrap_subprocess_exec(cse, stderr=b'error', retcode=1) + # that when the script is successful + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) - # and that the cora-1 board is reserved + # that the cora-1 board is reserved data = self.data brd = self.brdmgr.boards['cora-1'] attrs = dict(iface='a', ip='b', devfsrule='c') async with brd.lock: - await brd.reserve() + await brd.reserve('foo') obrdreq = await data.BoardStatus.objects.create( board='cora-1', user='foo') brd.attrcache.update(attrs) + # that when the script fails + wrap_subprocess_exec(cse, stderr=b'error', retcode=1) + # that when the correct user releases the board res = await self.client.post('/board/cora-1/release', auth=BiteAuth('thisisanapikey')) @@ -1136,7 +1170,7 @@ class TestBiteLabAPI(TestCommon): # and that it called the release script env = os.environ.copy() env.update(attrs) - cse.assert_called_with(self.settings.setup_script, + cse.assert_called_with('somesetupscript', 'release', 'cora-1', 'foo', env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -1179,7 +1213,7 @@ class TestBiteLabAPI(TestCommon): self.assertEqual(res.json(), info) # and that it called the start script - cse.assert_called_with(self.settings.setup_script, 'reserve', + cse.assert_called_with('somesetupscript', 'reserve', 'cora-1', 'foo', stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -1215,7 +1249,7 @@ class TestBiteLabAPI(TestCommon): self.assertEqual(res.json(), brdinfo) # and that it called the start script - cse.assert_called_with(self.settings.setup_script, 'reserve', + cse.assert_called_with('somesetupscript', 'reserve', 'cora-1', 'foo', 'pubsshkey', stdout=subprocess.PIPE, stderr=subprocess.PIPE) # and that the board was activated @@ -1280,7 +1314,7 @@ class TestBiteLabAPI(TestCommon): env['devfspath'] = brdinfo['attrs']['devfspath'] # and that it called the release script - cse.assert_called_with(self.settings.setup_script, 'release', + cse.assert_called_with('somesetupscript', 'release', 'cora-1', 'foo', env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -1344,13 +1378,17 @@ class TestBiteLabAPI(TestCommon): } self.assertEqual(res.json(), info) + @patch('asyncio.create_subprocess_exec') @patch('bitelab.snmp.snmpset') - async def test_board_attrs(self, ss): + async def test_board_attrs(self, ss, cse): data = self.data # that when snmpset returns False ss.return_value = False + # that when the setup script passed + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) + attrs = dict(power=False) # that setting the board attributes requires auth @@ -1372,7 +1410,7 @@ class TestBiteLabAPI(TestCommon): # that the cora-1 board is reserved brd = self.brdmgr.boards['cora-1'] async with brd.lock: - await brd.reserve() + await brd.reserve('foo') obrdreq = await data.BoardStatus.objects.create( board='cora-1', user='foo') @@ -1424,34 +1462,154 @@ class TestBiteLabAPI(TestCommon): self.assertEqual(res.json(), info) class TestBoardImpl(unittest.IsolatedAsyncioTestCase): - async def xtest_reserve(self): + async def asyncSetUp(self): + opttup = create_autospec, dict(spec=Attribute) + brd = BoardImpl('aboard', 'aclass', 'ascript', [ opttup ]) + (opt,) = tuple(brd.attrmap.values()) + + self.brd = brd + self.opt = opt + + async def asyncTearDown(self): + await asyncio.sleep(0) + await asyncio.sleep(0) + + sys.stdout.flush() + sys.stderr.flush() + + async def test_errors(self): + # that reserve raises an error when not locked + with self.assertRaises(RuntimeError): + await self.brd.reserve(None) + + # that release raises an error when not locked + with self.assertRaises(RuntimeError): + await self.brd.release() + + # that update_attrs raises an error when not locked + with self.assertRaises(RuntimeError): + await self.brd.update_attrs() + + # that update_attrs raises an error when not reserved + async with self.brd.lock: + with self.assertRaises(RuntimeError): + await self.brd.update_attrs() + + # that activate raises an error when not reserved + async with self.brd.lock: + with self.assertRaises(RuntimeError): + await self.brd.activate() + + # that deactivate raises an error when not reserved + async with self.brd.lock: + with self.assertRaises(RuntimeError): + await self.brd.deactivate() + + @patch('bitelab.BoardImpl.deactivate') + @patch('bitelab.BoardImpl.activate') + @patch('logging.error') + @patch('asyncio.create_subprocess_exec') + async def test_reserve_release(self, cse, le, act, deact): + brd = self.brd + + # that when the setup script will fail + wrap_subprocess_exec(cse, stderr=b'error', retcode=1) + + # that attempting to reserve the board + async with brd.lock: + with self.assertRaises(RuntimeError): + await brd.reserve(None) + + # that the setup script will pass + attrs = dict(iface='a', ip='b', devfsrule='c') + wrap_subprocess_exec(cse, stdout=json.dumps(attrs).encode(), retcode=0) + # that when the board is reserved async with brd.lock: - await brd.reserve() + await brd.reserve('auser') - async def test_activate(self): - # that a board impl - opttup = create_autospec, dict(spec=Attribute) - brd = BoardImpl('foo', 'bar', [ opttup ]) - (opt,) = tuple(brd.attrmap.values()) + # that the script was called properly + cse.assert_called_with('ascript', + 'reserve', 'aboard', 'auser', + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + # that the returned attribute is there: + # XXX - I can't silenct the deprecation warning about + # assertDictContainsSubset + self.assertEqual(attrs, { k: v for k, v in + brd.attrs.items() if k in set(attrs.keys()) }) + + # activate is called + act.assert_called() + + # that if it tries to be reserved again + async with brd.lock: + + # that it raises a RuntimeError + with self.assertRaisesRegex(RuntimeError, + 'Board currently reserved.'): + await brd.reserve('auser') + # that it can be released: async with brd.lock: - await brd.reserve() + await brd.release() + + # that the script was called properly + env = os.environ.copy() + env.update(attrs) + self.assertEqual(env, cse.mock_calls[-2].kwargs['env']) + cse.assert_called_with('ascript', + 'release', 'aboard', 'auser', env=env, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + # deactivate is called + deact.assert_called() + + # that the attrs are no longer present + # (clear_info was called) + self.assertFalse(set(brd.attrs.keys()) & set(attrs.keys())) + + # that if released w/o being reserved + # that it raises a RuntimeError + with self.assertRaisesRegex(RuntimeError, + 'Board not reserved.'): + await brd.release() + + @patch('asyncio.create_subprocess_exec') + async def test_activate(self, cse): + brd, opt = self.brd, self.opt + + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) + + # that a board impl + async with brd.lock: + await brd.reserve(None) opt.activate.assert_called_with(brd) - async def test_deactivate(self): + # that raises runtime when not locked + with self.assertRaises(RuntimeError): + await brd.activate() + + @patch('asyncio.create_subprocess_exec') + async def test_deactivate(self, cse): # that a board impl opttup = create_autospec, dict(spec=Attribute) - brd = BoardImpl('foo', 'bar', [ opttup ]) + brd = BoardImpl('foo', 'bar', None, [ opttup ]) (opt,) = tuple(brd.attrmap.values()) + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) + async with brd.lock: - await brd.reserve() + await brd.reserve(None) await brd.release() opt.deactivate.assert_called_with(brd) + # that raises runtime when not locked + with self.assertRaises(RuntimeError): + await brd.deactivate() + class TestLogEvent(unittest.IsolatedAsyncioTestCase): @patch('time.time') @patch('logging.info') @@ -1460,7 +1618,7 @@ class TestLogEvent(unittest.IsolatedAsyncioTestCase): user = 'weoijsdfkj' brdname = 'woied' extra = dict(something=2323, someelse='asdlfkj') - brd = BoardImpl(brdname, {}, []) + brd = BoardImpl(brdname, None, None, []) tt.return_value = 1607650392.384 @@ -1502,7 +1660,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): sctup = (SerialConsole, dict(val=data)) - brd = BoardImpl('foo', 'bar', [ sctup ]) + brd = BoardImpl('foo', 'bar', None, [ sctup ]) sc = brd.attrmap['console'] self.assertEqual(sc.defattrname, 'console') @@ -1542,7 +1700,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): eitup = EtherIface, dict(val=eiface) - brd = BoardImpl('foo', 'bar', [ eitup ]) + brd = BoardImpl('foo', 'bar', None, [ eitup ]) ei = brd.attrmap['eiface'] @@ -1570,7 +1728,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): # That multiple attributes w/ same name raises ValueError with self.assertRaises(ValueError): - BoardImpl('foo', 'bar', attrs) + BoardImpl('foo', 'bar', None, attrs) # Enough of this code depends upon the event loop using the # code in BaseEventLoop wrt scheduling that this is not a @@ -1606,14 +1764,17 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): self.assertAlmostEqual(looptoutc(utctoloop(sometime)), sometime) @timeout(2) + @patch('asyncio.create_subprocess_exec') @patch('asyncio.BaseEventLoop.time') @patch('time.time') - async def test_timeout_vals(self, ttime, belt): + async def test_timeout_vals(self, ttime, belt, cse): # that a TimeOut with args totup = TimeOut, dict(val=10) + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) + # passed to a board w/ the totup - brd = BoardImpl('foo', 'bar', [ totup ]) + brd = BoardImpl('foo', 'bar', None, [ totup ]) to = brd.attrmap['timeout'] @@ -1638,8 +1799,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): # that when reserved/activated async with brd.lock: - await brd.reserve() - await brd.activate() + await brd.reserve(None) await brd.update() @@ -1664,18 +1824,29 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): self.assertEqual(brd.attrs, dict(timeout='2020-12-10T14:06:44.280Z')) @timeout(2) - async def test_timeout(self): + @patch('asyncio.create_subprocess_exec') + async def test_timeout(self, cse): # that a TimeOut with args totup = TimeOut, dict(val=.01) + wrap_subprocess_exec(cse, stdout=b'{}', retcode=0) + # passed to a board w/ the totup - brd = BoardImpl('foo', 'bar', [ totup ]) + brd = BoardImpl('foo', 'bar', None, [ totup ]) to = brd.attrmap['timeout'] + # that it raises a RuntimeError when not locked + with self.assertRaises(RuntimeError): + await to.activate(brd) + + # that it raises a RuntimeError when not locked + with self.assertRaises(RuntimeError): + await to.deactivate(brd) + # that when reserved/activated async with brd.lock: - await brd.reserve() + await brd.reserve(None) evt = asyncio.Event() loop = asyncio.get_running_loop() @@ -1687,7 +1858,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): # that when reserved/activated/deactivated/released async with brd.lock: - await brd.reserve() + await brd.reserve(None) exp = to._exp await brd.release() @@ -1702,8 +1873,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): # that when reserved/activated async with brd.lock: - await brd.reserve() - await brd.activate() + await brd.reserve(None) # but the board is locked for some reason await brd.lock.acquire() @@ -1722,4 +1892,3 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase): # that the board was not released self.assertTrue(brd.reserved) - diff --git a/bitelab/config.py b/bitelab/config.py index c5479d8..21e92ca 100644 --- a/bitelab/config.py +++ b/bitelab/config.py @@ -36,7 +36,6 @@ __all__ = [ 'Settings' ] # https://web.archive.org/web/20201113005838/https://github.com/samuelcolvin/pydantic/issues/655 class Settings(BaseSettings): db_file: str = Field(description='path to SQLite3 database file') - setup_script: str = Field(description='script that will initalize an environment') board_conf: str = Field(description='UCL configuration for the boards') class Config: diff --git a/fixtures/board_conf.ucl b/fixtures/board_conf.ucl index 24dd356..68ef139 100644 --- a/fixtures/board_conf.ucl +++ b/fixtures/board_conf.ucl @@ -1,3 +1,5 @@ +setup_script = somesetupscript; + classes { cora-z7s = { arch = arm-armv7;