From 04b41081243926160b76a7df41aedf9ae5943187 Mon Sep 17 00:00:00 2001 From: Riccardo Coccioli Date: Wed, 22 Sep 2021 23:31:01 +0200 Subject: [PATCH] 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 | 21 +++++++++++---------- spicerack/tests/unit/test_remote.py | 10 +++++----- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/spicerack/remote.py b/spicerack/remote.py index 24c94ac..e0a4b3f 100644 --- a/spicerack/remote.py +++ b/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) diff --git a/spicerack/tests/unit/test_remote.py b/spicerack/tests/unit/test_remote.py index cf587d1..6f68bca 100644 --- a/spicerack/tests/unit/test_remote.py +++ b/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)