Merge ~mpontillo/maas:fix-random-choice-no-choices--2.2 into maas:2.2

[Mike Pontillo]

Prevent RegionService from using empty lists.

LP: #1705254
Backports: 3dd8eedfba921760f98ea54805d05c86e63266ec

[Mike Pontillo (mpontillo)]

Self-approve backport.

review: Approve

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 7eaaeb7..abea670 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -28,7 +28,9 @@ LP: #1703845    Set the re-check interval for rack to region RPC connections to 
 
 LP: #1703403    Lower the maximum number of connections per worker to 9 connections is allowed as 1 is reserved.
 
-LP: #1704489    [2.2] Duplicate communication is occurring to the same rack controller
+LP: #1704489    Duplicate communication is occurring to the same rack controller
+
+LP: #1705254    RegionService can raise unexpected exceptions for an empty connection list
 
 
 2.2.1
diff --git a/requirements.txt b/requirements.txt
index ae87b46..098e026 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,3 @@
-bson==0.4.3
 convoy==0.4.4
 crochet==1.5.0
 django==1.8
@@ -13,7 +12,8 @@ paramiko==1.16.3
 petname==2.0
 pexpect==4.2.0
 psycopg2==2.6.2
-pyopenssl==16.0.0
+pymongo==3.2
+pyopenssl==16.2.0
 pyparsing==2.0.7
 pyvmomi==6.0.0.2016.6
 requests==2.11.0
diff --git a/src/maasserver/rpc/regionservice.py b/src/maasserver/rpc/regionservice.py
index a68b038..4d962e8 100644
--- a/src/maasserver/rpc/regionservice.py
+++ b/src/maasserver/rpc/regionservice.py
@@ -967,21 +967,25 @@ class RegionService(service.Service, object):
     def getAllClients(self):
         """Return a list with one connection per rack controller."""
         return [
-            common.Client(random.choice(list(conns)))
-            for conns in self.connections.values()
+            common.Client(random.choice(list(connections)))
+            for connections in self.connections.values()
+            if len(connections) > 0
         ]
 
     @asynchronous(timeout=FOREVER)
     def getRandomClient(self):
-        """Return a list of all connected :class:`common.Client`s."""
-        clients = list(self.connections.values())
-        if len(clients) == 0:
+        """Return a random connected :class:`common.Client`."""
+        connections = list(self.connections.values())
+        if len(connections) == 0:
             raise exceptions.NoConnectionsAvailable(
                 "Unable to connect to any rack controller; no connections "
                 "available.")
         else:
-            conns = list(random.choice(clients))
-            return common.Client(random.choice(conns))
+            connection = random.choice(connections)
+            # The connection object is a set of RegionServer objects.
+            # Make sure a sane set was returned.
+            assert len(connection) > 0, "Connection set empty."
+            return common.Client(random.choice(list(connection)))
 
 
 def ignoreCancellation(failure):
diff --git a/src/maasserver/rpc/tests/test_regionservice.py b/src/maasserver/rpc/tests/test_regionservice.py
index e845b30..972a3d7 100644
--- a/src/maasserver/rpc/tests/test_regionservice.py
+++ b/src/maasserver/rpc/tests/test_regionservice.py
@@ -80,7 +80,10 @@ from provisioningserver.rpc import (
     common,
     exceptions,
 )
-from provisioningserver.rpc.exceptions import CannotRegisterRackController
+from provisioningserver.rpc.exceptions import (
+    CannotRegisterRackController,
+    NoConnectionsAvailable,
+)
 from provisioningserver.rpc.interfaces import IConnection
 from provisioningserver.rpc.region import RegisterRackController
 from provisioningserver.rpc.testing import call_responder
@@ -91,6 +94,7 @@ from provisioningserver.utils.twisted import (
     callInReactorWithTimeout,
     DeferredValue,
 )
+from testtools import ExpectedException
 from testtools.deferredruntest import assert_fails_with
 from testtools.matchers import (
     AfterPreprocessing,
@@ -875,6 +879,21 @@ class TestRegionService(MAASTestCase):
         self.assertThat(service.getAllClients(), Equals([]))
 
     @wait_for_reactor
+    def test_getAllClients_empty_connections(self):
+        service = RegionService(sentinel.advertiser)
+        service.connections.clear()
+        uuid1 = factory.make_UUID()
+        service.connections[uuid1] = set()
+        self.assertThat(service.getAllClients(), Equals([]))
+
+    @wait_for_reactor
+    def test_getRandomClient_empty_raises_NoConnectionsAvailable(self):
+        service = RegionService(sentinel.advertiser)
+        service.connections.clear()
+        with ExpectedException(NoConnectionsAvailable):
+            service.getRandomClient()
+
+    @wait_for_reactor
     def test_getAllClients(self):
         service = RegionService(sentinel.advertiser)
         uuid1 = factory.make_UUID()