Merge ~cgrabowski/maas:fix_wrong_rack_controller_selection_for_bmc into maas:master

[Christian Grabowski]

check to see the rack controller is connected via rpc to consider it accessible to a bmc
fix test

add test asserting routable rack controllers are updated

only consider a BMC accessible if a rack controller is usable AND has a connection

test get_bmc_client_connection_info
add tests for _get_bmc_client_connection_info

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_wrong_rack_controller_selection_for_bmc lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12590/console
COMMIT: f080a982b64c9ffd4b3d2c9a357898c88e090ad5

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_wrong_rack_controller_selection_for_bmc lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/12591/console
COMMIT: 8f15383ae1646688a1fec3257b3f9ad8df166f8b

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_wrong_rack_controller_selection_for_bmc lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 2315c4ca66d8307efd38b4a3da32686ee7373346

review: Approve

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index c697c49..5fda784 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -543,7 +543,7 @@ class BMC(CleanSave, TimestampedModel):
 
     def is_accessible(self):
         """If the BMC is accessible by at least one rack controller."""
-        racks = self.get_usable_rack_controllers(with_connection=False)
+        racks = self.get_usable_rack_controllers(with_connection=True)
         return len(racks) > 0
 
     def update_routable_racks(
@@ -564,11 +564,12 @@ class BMC(CleanSave, TimestampedModel):
         from maasserver.models.node import RackController
 
         for rack_id in rack_ids:
+            rack = None
             try:
                 rack = RackController.objects.get(system_id=rack_id)
             except RackController.DoesNotExist:
                 # Possible it was delete before this call, but very very rare.
-                pass
+                continue
             BMCRoutableRackControllerRelationship(
                 bmc=self, rack_controller=rack, routable=routable
             ).save()
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index 5d46f76..7af9b8c 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -559,7 +559,7 @@ class TestBMC(MAASServerTestCase):
         bmc.is_accessible()
         self.assertThat(
             mock_get_usable_rack_controllers,
-            MockCalledOnceWith(with_connection=False),
+            MockCalledOnceWith(with_connection=True),
         )
 
     def test_is_accessible_returns_true(self):
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index aa968c8..33752b8 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -6099,6 +6099,77 @@ class TestNode(MAASServerTestCase):
             interface, list(node.current_config.interface_set.all())
         )
 
+    def test_get_bmc_client_connection_info_layer2(self):
+        admin = factory.make_admin()
+        subnet = factory.make_Subnet()
+
+        rack_controllers = [
+            factory.make_RackController(owner=admin) for _ in range(2)
+        ]
+        for rack_controller in rack_controllers:
+            ip = subnet.get_next_ip_for_allocation()
+            factory.make_Interface(node=rack_controller, ip=ip)
+
+        clients = [Mock() for _ in rack_controllers]
+        for i, rack_controller in enumerate(rack_controllers):
+            clients[i].ident = rack_controller.system_id
+
+        mock_getAllClients = self.patch(node_module, "getAllClients")
+        mock_getAllClients.return_value = clients
+
+        ip = subnet.get_next_ip_for_allocation()
+        ip_address = factory.make_StaticIPAddress(ip=ip)
+        bmc = factory.make_BMC(ip_address=ip_address)
+        node = factory.make_Node(bmc=bmc)
+        clients, _ = node._get_bmc_client_connection_info()
+        self.assertCountEqual(
+            [
+                rack_controller.system_id
+                for rack_controller in rack_controllers
+            ],
+            clients,
+        )
+
+    def test_get_bmc_client_connection_info_routable(self):
+        admin = factory.make_admin()
+        subnet1 = factory.make_Subnet()
+        subnet2 = factory.make_Subnet()
+
+        rack_controllers = [
+            factory.make_RackController(owner=admin) for _ in range(2)
+        ]
+        for rack_controller in rack_controllers:
+            ip = subnet1.get_next_ip_for_allocation()
+            factory.make_Interface(node=rack_controller, ip=ip)
+
+        clients = [Mock() for _ in rack_controllers]
+        for i, rack_controller in enumerate(rack_controllers):
+            clients[i].ident = rack_controller.system_id
+
+        mock_getAllClients_node = self.patch(node_module, "getAllClients")
+        mock_getAllClients_node.return_value = clients
+        mock_getAllClients_bmc = self.patch(bmc_module, "getAllClients")
+        mock_getAllClients_bmc.return_value = clients
+
+        ip = subnet2.get_next_ip_for_allocation()
+        ip_address = factory.make_StaticIPAddress(ip=ip, subnet=subnet2)
+        bmc = factory.make_BMC(ip_address=ip_address)
+        node = factory.make_Node(bmc=bmc)
+        [
+            BMCRoutableRackControllerRelationship(
+                bmc=bmc, rack_controller=rack_controller, routable=True
+            ).save()
+            for rack_controller in rack_controllers
+        ]
+        clients, _ = node._get_bmc_client_connection_info()
+        self.assertCountEqual(
+            [
+                rack_controller.system_id
+                for rack_controller in rack_controllers
+            ],
+            clients,
+        )
+
 
 class TestNodePowerParameters(MAASServerTestCase):
     def setUp(self):
@@ -6374,6 +6445,119 @@ class TestNodePowerParameters(MAASServerTestCase):
         self.assertFalse(node.is_sync_healthy)
 
 
+class TestPowerControlNode(MAASTransactionServerTestCase):
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test_power_control_node_updates_routable_rack_controllers(self):
+        bmc = yield deferToDatabase(factory.make_BMC)
+        node = yield deferToDatabase(
+            factory.make_Node_with_Interface_on_Subnet, bmc=bmc
+        )
+        rack_ip = yield deferToDatabase(factory.make_ip_address)
+        rack_controller = yield deferToDatabase(node.get_boot_rack_controller)
+        yield deferToDatabase(
+            factory.make_Interface, node=rack_controller, ip=rack_ip
+        )
+
+        def _create_initial_relationship():
+            b = BMCRoutableRackControllerRelationship(
+                bmc=bmc, rack_controller=rack_controller, routable=True
+            )
+            b.save()
+
+        yield deferToDatabase(_create_initial_relationship)
+
+        other_rack_controller = yield deferToDatabase(
+            factory.make_RackController
+        )
+
+        def _assert_no_routable():
+            self.assertRaises(
+                BMCRoutableRackControllerRelationship.DoesNotExist,
+                BMCRoutableRackControllerRelationship.objects.get,
+                bmc=node.bmc,
+                rack_controller=other_rack_controller,
+            )
+
+        yield deferToDatabase(_assert_no_routable)
+
+        client = Mock()
+        client.ident = other_rack_controller.system_id
+        self.patch(bmc_module, "getAllClients").return_value = [client]
+        self.patch(node_module, "getAllClients").return_value = [client]
+        client2 = Mock()
+        client2.return_value = defer.succeed({"missing_packages": []})
+        d1 = defer.succeed(client2)
+        self.patch(bmc_module, "getClientFromIdentifiers").return_value = d1
+        self.patch(node_module, "getClientFromIdentifiers").return_value = d1
+        self.patch(
+            node_module, "power_query_all"
+        ).return_value = defer.succeed(
+            (POWER_STATE.ON, set([other_rack_controller.system_id]), set())
+        )
+
+        power_info = yield deferToDatabase(node.get_effective_power_info)
+        yield node._power_control_node(
+            defer.succeed(None), power_query, power_info
+        )
+
+        def _assert_routable():
+            self.assertTrue(
+                BMCRoutableRackControllerRelationship.objects.get(
+                    bmc=node.bmc, rack_controller=other_rack_controller
+                )
+            )
+
+        yield deferToDatabase(_assert_routable)
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test_power_control_node_removes_non_routable_rack_controllers(self):
+        bmc = yield deferToDatabase(factory.make_BMC)
+        node = yield deferToDatabase(
+            factory.make_Node_with_Interface_on_Subnet, bmc=bmc
+        )
+        rack_ip = yield deferToDatabase(factory.make_ip_address)
+        rack_controller = yield deferToDatabase(node.get_boot_rack_controller)
+        yield deferToDatabase(
+            factory.make_Interface, node=rack_controller, ip=rack_ip
+        )
+
+        def _create_initial_relationship():
+            b = BMCRoutableRackControllerRelationship(
+                bmc=bmc, rack_controller=rack_controller, routable=True
+            )
+            b.save()
+
+        yield deferToDatabase(_create_initial_relationship)
+
+        self.patch(bmc_module, "getAllClients").return_value = []
+        self.patch(node_module, "getAllClients").return_value = []
+        client2 = Mock()
+        client2.return_value = defer.succeed({"missing_packages": []})
+        d1 = defer.succeed(client2)
+        self.patch(bmc_module, "getClientFromIdentifiers").return_value = d1
+        self.patch(node_module, "getClientFromIdentifiers").return_value = d1
+        self.patch(
+            node_module, "power_query_all"
+        ).return_value = defer.succeed((POWER_STATE.ON, set(), set()))
+
+        power_info = yield deferToDatabase(node.get_effective_power_info)
+        yield node._power_control_node(
+            defer.succeed(None), power_query, power_info
+        )
+
+        def _assert_no_routable():
+            self.assertRaises(
+                BMCRoutableRackControllerRelationship.DoesNotExist,
+                BMCRoutableRackControllerRelationship.objects.get,
+                bmc=node.bmc,
+                rack_controller=rack_controller,
+            )
+
+        yield deferToDatabase(_assert_no_routable)
+
+
 class TestDecomposeMachineMixin:
     """Mixin to help `TestDecomposeMachine` and
     `TestDecomposeMachineTransactional`."""