From c372051af2123ad289a83a5129b1dc70ff406388 Mon Sep 17 00:00:00 2001
From: John-Mark Gurney <jmg@funkthat.com>
Date: Mon, 21 Dec 2020 12:48:42 -0800
Subject: [PATCH] first stages of reworking so that BoardImpl implements all
 the logic for reserve/release, relocating this logic out of the API logic.

---
 bitelab/__init__.py | 66 ++++++++++++++++++++++++++++++++++++++-------
 bitelab/testing.py  |  2 +-
 2 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/bitelab/__init__.py b/bitelab/__init__.py
index 4adf5af..21e9c9b 100644
--- a/bitelab/__init__.py
+++ b/bitelab/__init__.py
@@ -235,6 +235,15 @@ class TimeOut(Attribute):
 	@_tbprinter
 	async def timeout_coro(self):
 		async with self._brd.lock:
+			# we don't want to cancel ourselves while
+			# releasing the board, otherwise it'll stop
+			# in the middle!
+			#
+			# Note: not sure if we need to await on the
+			# task to clean things up though.
+
+			self._task = None
+
 			await self._brd.release()
 
 	def timeout_callback(self):
@@ -280,7 +289,15 @@ class BoardImpl:
 	be destroyed before the operation completes.
 	'''
 
-	def __init__(self, name, brdclass, options):
+	def __init__(self, name: str, brdclass: str, options):
+		'''
+		name: name of the board.
+		brdclass: class that the board belongs to.
+		options: list of tuples, the first item of the tuple,
+		    being the factory to call, and the second item, the
+		    keyword args to use when calling the factory.
+		'''
+
 		self.name = name
 		self.brdclass = brdclass
 		self.options = options
@@ -302,42 +319,72 @@ class BoardImpl:
 		return repr(Board.from_orm(self))
 
 	async def reserve(self):
+		'''Reserve the board.'''
+
 		assert self.lock.locked() and not self.reserved
 
 		self.reserved = True
 
+		await self.activate()
+
 	async def release(self):
+		'''Release the board.'''
+
 		assert self.lock.locked() and self.reserved
 
+		await self.deactivate()
+
 		self.reserved = False
 
 	async def update_attrs(self, **attrs):
+		'''Set the various attrs that are specified.'''
+
 		assert self.lock.locked() and self.reserved
 
 		for i in attrs:
 			self.attrcache[i] = await self.attrmap[i].setvalue(attrs[i])
 
 	async def update(self):
+		'''Update the attr cache w/ current values.'''
+
 		for i in self.attrmap:
 			self.attrcache[i] = await self.attrmap[i].getvalue()
 
 	async def activate(self):
+		'''Activate the attributes.  This means that the board
+		has been reserved.  This is for things like adding an
+		ethernet interface to the jail's vnet, or starting the
+		timeout.'''
+
 		assert self.lock.locked() and self.reserved
 
 		for i in self.attrmap.values():
 			await i.activate(self)
 
 	async def deactivate(self):
+		'''Deactivate the attributes.  This should be called
+		just before release.  This is to clean up anything like
+		scheduled tasks.'''
+
 		assert self.lock.locked() and self.reserved
 
 		for i in self.attrmap.values():
 			await i.deactivate(self)
 
 	def add_info(self, d):
+		'''Add some additional data that will be returned for
+		the current reservation.  All the data added here will
+		be removed when the board is deactivated, by clean_info.
+		'''
+
 		self.attrcache.update(d)
 
 	def clean_info(self):
-		# clean up attributes
+		'''Remove any attributes that aren't in the attrmap.
+
+		This is to clean up after the add_info call.
+		'''
+
 		for i in set(self.attrcache) - set(self.attrmap):
 			del self.attrcache[i]
 
@@ -1007,7 +1054,7 @@ class TestWebSocket(TestCommon):
 # IsolatedAsyncioTestCase was added.  The tearDown has to happen
 # with the event loop running, otherwise the task and other things
 # do not get cleaned up properly.
-class TestBiteLab(TestCommon):
+class TestBiteLabAPI(TestCommon):
 	async def asyncSetUp(self):
 		await super().asyncSetUp()
 
@@ -1059,7 +1106,7 @@ class TestBiteLab(TestCommon):
 		# that when the setup script will fail
 		wrap_subprocess_exec(cse, stderr=b'error', retcode=1)
 
-		# that the cora-1 board is reserved
+		# and that the cora-1 board is reserved
 		data = self.data
 		brd = self.brdmgr.boards['cora-1']
 		attrs = dict(iface='a', ip='b', devfsrule='c')
@@ -1377,6 +1424,11 @@ class TestBiteLab(TestCommon):
 		self.assertEqual(res.json(), info)
 
 class TestBoardImpl(unittest.IsolatedAsyncioTestCase):
+	async def xtest_reserve(self):
+		# that when the board is reserved
+		async with brd.lock:
+			await brd.reserve()
+
 	async def test_activate(self):
 		# that a board impl
 		opttup = create_autospec, dict(spec=Attribute)
@@ -1385,7 +1437,6 @@ class TestBoardImpl(unittest.IsolatedAsyncioTestCase):
 
 		async with brd.lock:
 			await brd.reserve()
-			await brd.activate()
 
 		opt.activate.assert_called_with(brd)
 
@@ -1397,7 +1448,7 @@ class TestBoardImpl(unittest.IsolatedAsyncioTestCase):
 
 		async with brd.lock:
 			await brd.reserve()
-			await brd.deactivate()
+			await brd.release()
 
 		opt.deactivate.assert_called_with(brd)
 
@@ -1625,7 +1676,6 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase):
 		# that when reserved/activated
 		async with brd.lock:
 			await brd.reserve()
-			await brd.activate()
 
 		evt = asyncio.Event()
 		loop = asyncio.get_running_loop()
@@ -1638,9 +1688,7 @@ class TestAttrs(unittest.IsolatedAsyncioTestCase):
 		# that when reserved/activated/deactivated/released
 		async with brd.lock:
 			await brd.reserve()
-			await brd.activate()
 			exp = to._exp
-			await brd.deactivate()
 			await brd.release()
 
 		# that the expiration is no longer there
diff --git a/bitelab/testing.py b/bitelab/testing.py
index 3d8f00c..5b270f7 100644
--- a/bitelab/testing.py
+++ b/bitelab/testing.py
@@ -30,6 +30,6 @@
 
 from .snmp import TestSNMPPower, TestSNMPWrapper
 from .data import TestDatabase
-from . import TestBiteLab, TestUnhashLRU, TestAttrs, TestBoardImpl
+from . import TestBiteLabAPI, TestUnhashLRU, TestAttrs, TestBoardImpl
 from . import TestWebSocket, TestLogEvent
 from .__main__ import TestClient, TestExecClient