remote: refactor wait_reboot_since()

* As the check for uptime is currently either returning a value for all
  hosts or raising an exception, remove the existing logic to check for
  a partial result as that can't happen.
* Catch instead the error and re-raise a check exception with a clear
  message.
* Also round the printed value of the uptime and the time against which
  it's checked to 2 decimal values for more readability.

Change-Id: I87497ef53beb54dac97f6fbea73e58b59a71e6d8

spicerack/remote.py

@@ -3,7 +3,7 @@
 import math
 import time
 from datetime import datetime, timedelta
-from typing import Any, Callable, Iterator, List, Optional, Sequence, Tuple, Union, cast
+from typing import Any, Callable, Iterator, List, Optional, Sequence, Tuple, Union
 
 from ClusterShell.MsgTree import MsgTreeElem
 from cumin import Config, CuminError, NodeSet, query, transport, transports
@@ -557,7 +557,7 @@ def reboot(self, batch_size: int = 1, batch_sleep: Optional[float] = 180.0) -> N
         tries=360,
         delay=timedelta(seconds=10),
         backoff_mode="constant",
-        exceptions=(RemoteExecutionError, RemoteCheckError),
+        exceptions=(RemoteCheckError,),
     )
     def wait_reboot_since(self, since: datetime, print_progress_bars: bool = True) -> None:
         """Poll the host until is reachable and has an uptime lower than the provided datetime.
@@ -570,16 +570,17 @@ def wait_reboot_since(self, since: datetime, print_progress_bars: bool = True) -
             spicerack.remote.RemoteCheckError: if unable to connect to the host or the uptime is higher than expected.
 
         """
-        remaining = cast(NodeSet, self.hosts)
         delta = (datetime.utcnow() - since).total_seconds()
-        for nodeset, uptime in self.uptime(print_progress_bars=print_progress_bars):
-            if uptime >= delta:
-                raise RemoteCheckError(f"Uptime for {nodeset} higher than threshold: {uptime} > {delta}")
-
-            remaining.difference_update(nodeset)
+        try:
+            uptimes = self.uptime(print_progress_bars=print_progress_bars)
+        except RemoteExecutionError as e:
+            raise RemoteCheckError(f"Unable to get uptime for {self._hosts}") from e
 
-        if remaining:
-            raise RemoteCheckError(f"Unable to check uptime from {len(remaining)} hosts: {remaining}")
+        for nodeset, uptime in uptimes:
+            if uptime >= delta:
+                raise RemoteCheckError(
+                    f"Uptime for {nodeset} higher than threshold: {round(uptime, 2)} > {round(delta, 2)}"
+                )
 
         logger.info("Found reboot since %s for hosts %s", since, self._hosts)
 

spicerack/tests/unit/test_remote.py

@@ -368,13 +368,13 @@ def test_wait_reboot_since_ok(self, mocked_uptime):
 
     @mock.patch("wmflib.decorators.time.sleep", return_value=None)
     @mock.patch("spicerack.remote.RemoteHosts.uptime")
-    def test_wait_reboot_since_remaining_hosts(self, mocked_uptime, mocked_sleep):
-        """It should raise RemoteCheckError if unable to get the uptime from all hosts."""
-        since = datetime.utcnow() - timedelta(minutes=5)
-        mocked_uptime.return_value = [(NodeSet("host1"), 30.0)]
+    def test_wait_reboot_since_uptime_fails(self, mocked_uptime, mocked_sleep):
+        """It should raise RemoteCheckError if unable to check the uptime on any host."""
+        since = datetime.utcnow()
+        mocked_uptime.side_effect = remote.RemoteExecutionError(message="unable to connect", retcode=1)
         with pytest.raises(
             remote.RemoteCheckError,
-            match=r"Unable to check uptime from 8 hosts: host\[2-9\]",
+            match=r"Unable to get uptime for host\[1-9\]",
         ):
             self.remote_hosts.wait_reboot_since(since)