Stuff

colony/core/roles/foreman/role.py

@@ -36,21 +36,30 @@ class Foreman(Role):
         k = (self_node.ip, self_node.port)
         if k not in by_key:
             by_key[k] = self_node
-        return list(by_key.values())
+        return [by_key[key] for key in sorted(by_key)]
+
+    def _call(self, peer, command, *args):
+        try:
+            resp = peer.execute(command, *args)
+        except Exception as e:
+            self.logger.warning("%s failed for %s:%s: %s", command.value, peer.ip, peer.port, e)
+            return None
+
+        if not isinstance(resp, dict) or resp.get("status") != "ok":
+            self.logger.warning("%s failed for %s:%s", command.value, peer.ip, peer.port)
+            return None
+        return resp
 
     def _fetch_live_roles(self, peers):
         live = {}
         for peer in peers:
-            resp = peer.execute(Unit.COMMANDS.SHOW_STATS)
-            if resp.get("status") != "ok":
-                self.logger.warning(
-                    "SHOW_STATS failed for %s:%s, assuming no roles",
-                    peer.ip,
-                    peer.port,
-                )
-                live[peer] = []
+            resp = self._call(peer, Unit.COMMANDS.SHOW_ROLES)
+            if resp is None:
                 continue
-            live[peer] = list(resp["result"].get("roles", []))
+            if not isinstance(resp.get("result"), list):
+                self.logger.warning("Bad role list from %s:%s, leaving it out of this round", peer.ip, peer.port)
+                continue
+            live[peer] = list(resp["result"])
         return live
 
     def _assign_critical_holders(self, peers, critical_roles, live_roles):
@@ -68,15 +77,23 @@ class Foreman(Role):
                 holders[critical_role] = peer_list[i % n]
         return holders
 
-    def _best_peer_for_optional(self, peers, role_stat):
+    def _fetch_hardware_scores(self, peers):
+        scores = {}
+        for peer in peers:
+            resp = self._call(peer, Unit.COMMANDS.SHOW_HARDWARE)
+            if resp is None:
+                continue
+            hw_obj = HardwareStatistics.from_dict(resp["result"])
+            scores[peer] = HardwareScorer.from_object(hw_obj)
+        return scores
+
+    def _best_peer_for_optional(self, peers, role_stat, hardware_scores):
         best_score = float("-inf")
         best_peer = None
         for peer in peers:
-            hw_resp = peer.execute(Unit.COMMANDS.SHOW_HARDWARE)
-            if hw_resp.get("status") != "ok":
+            hs = hardware_scores.get(peer)
+            if hs is None:
                 continue
-            hw_obj = HardwareStatistics.from_dict(hw_resp["result"])
-            hs = HardwareScorer.from_object(hw_obj)
             score = 0.0
             if role_stat.hardware_preferences:
                 for hw_type, weight in role_stat.hardware_preferences.priorities.items():
@@ -119,6 +136,10 @@ class Foreman(Role):
             return
 
         live_roles = self._fetch_live_roles(peers)
+        peers = [peer for peer in peers if peer in live_roles]
+        if not peers:
+            return
+
         critical_holders = self._assign_critical_holders(
             peers, critical_roles, live_roles
         )
@@ -127,14 +148,16 @@ class Foreman(Role):
         for critical_role, holder in critical_holders.items():
             desired[holder].append(critical_role)
 
+        hardware_scores = self._fetch_hardware_scores(peers) if optional_roles else {}
         for role_name in optional_roles:
             role_stat = roles[role_name]
-            best_peer = self._best_peer_for_optional(peers, role_stat)
+            best_peer = self._best_peer_for_optional(peers, role_stat, hardware_scores)
             if best_peer is not None:
                 desired[best_peer].append(role_name)
 
         for peer in peers:
-            peer.execute(
+            self._call(
+                peer,
                 Unit.COMMANDS.SET_ROLES,
                 self._dedupe_preserve_order(desired[peer]),
             )