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

[Mike Pontillo]

Prevent RegionService from using empty lists.

LP: #1705254

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b fix-random-choice-no-choices lp:~mpontillo/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/120/console

d12f855... by Mike Pontillo on 2017-07-19

Fix incorrect assumptions.

diff --git a/requirements.txt b/requirements.txt
index e08077a..160c720 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -13,7 +13,7 @@ petname==2.0
 pexpect==4.2.0
 psycopg2==2.6.2
 pymongo==3.2
-pyopenssl==16.0.0
+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 a5790e8..02d987d 100644
--- a/src/maasserver/rpc/regionservice.py
+++ b/src/maasserver/rpc/regionservice.py
@@ -992,21 +992,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()