Browse Source

Fix race in test_handler_poll_session_expired

Just waiting for a node request state change does not guarantee
that we have actually entered the poll() portion of request handling.
This was causing the mock.call_count assert to fail on occasion.
Change this to simply wait on the call_count to increment, eliminating
that race.

There was also another potential race in checking that the request
handler was removed from the request_handlers structure. It would
be possible for the same request to re-enter active handling before
we had a chance to check that it was removed when the session
exception was thrown. We handle that race by setting the request state
to FAILED on the first exception.

Change-Id: I646b82243eb7f8c4e83d6678c2c0d265d99e51e0
tags/3.4.0
David Shrewsbury 6 months ago
parent
commit
b75f463280
1 changed files with 35 additions and 12 deletions
  1. 35
    12
      nodepool/tests/unit/test_launcher.py

+ 35
- 12
nodepool/tests/unit/test_launcher.py View File

@@ -1653,31 +1653,54 @@ class TestLauncher(tests.DBTestCase):
1653 1653
         self.assertEqual(req.state, zk.FULFILLED)
1654 1654
         self.assertEqual(len(req.nodes), 1)
1655 1655
 
1656
-    @mock.patch(
1657
-        'nodepool.driver.openstack.handler.OpenStackNodeRequestHandler.poll')
1656
+    @mock.patch('nodepool.driver.NodeRequestHandler.poll')
1658 1657
     def test_handler_poll_session_expired(self, mock_poll):
1659 1658
         '''
1660
-        Test ZK session lost during handler poll().
1659
+        Test ZK session lost during handler poll() removes handler.
1661 1660
         '''
1662
-        mock_poll.side_effect = kze.SessionExpiredError()
1661
+        req = zk.NodeRequest()
1662
+        req.state = zk.REQUESTED
1663
+        req.node_types.append('fake-label')
1664
+        self.zk.storeNodeRequest(req)
1665
+
1666
+        # We need to stop processing of this request so that it does not
1667
+        # re-enter request handling, so we can then verify that it was
1668
+        # actually removed from request_handlers in the final assert of
1669
+        # this test.
1670
+        def side_effect():
1671
+            req.state = zk.FAILED
1672
+            # Intentionally ignore that it is already locked.
1673
+            self.zk.storeNodeRequest(req)
1674
+            raise kze.SessionExpiredError()
1675
+
1676
+        mock_poll.side_effect = side_effect
1663 1677
 
1664 1678
         # use a config with min-ready of 0
1665 1679
         configfile = self.setup_config('node_launch_retry.yaml')
1666 1680
         self.useBuilder(configfile)
1681
+
1682
+        # Wait for the image to exist before starting the launcher, else
1683
+        # we'll decline the request.
1684
+        self.waitForImage('fake-provider', 'fake-image')
1685
+
1667 1686
         pool = self.useNodepool(configfile, watermark_sleep=1)
1668 1687
         pool.cleanup_interval = 60
1669 1688
         pool.start()
1670
-        self.waitForImage('fake-provider', 'fake-image')
1671 1689
 
1672
-        req = zk.NodeRequest()
1673
-        req.state = zk.REQUESTED
1674
-        req.node_types.append('fake-label')
1675
-        self.zk.storeNodeRequest(req)
1690
+        # Wait for request handling to occur
1691
+        while not mock_poll.call_count:
1692
+            time.sleep(.1)
1693
+
1694
+        # Note: The launcher is not setting FAILED state here, but our mock
1695
+        # side effect should be doing so. Just verify that.
1696
+        req = self.waitForNodeRequest(req)
1697
+        self.assertEqual(zk.FAILED, req.state)
1676 1698
 
1677 1699
         # A session loss during handler poll should at least remove the
1678
-        # request from active handlers
1679
-        req = self.waitForNodeRequest(req, states=(zk.PENDING,))
1680
-        self.assertEqual(1, mock_poll.call_count)
1700
+        # request from active handlers. The session exception from our first
1701
+        # time through poll() should handle removing the request handler.
1702
+        # And our mock side effect should ensure it does not re-enter
1703
+        # request handling before we check it.
1681 1704
         self.assertEqual(0, len(
1682 1705
             pool._pool_threads["fake-provider-main"].request_handlers))
1683 1706
 

Loading…
Cancel
Save