From cd418ee4cbc9aaacada277dd3ddf28de18129c05 Mon Sep 17 00:00:00 2001 From: Stephen Mwangi Date: Tue, 23 Jul 2024 10:00:29 +0300 Subject: [PATCH] fix: retry setting the computer title --- .gitignore | 2 ++ landscape/client/deployment.py | 28 ++++++++++++++------ landscape/client/tests/test_deployment.py | 31 +++++++++++++++++++++++ landscape/client/tests/test_watchdog.py | 2 +- landscape/client/watchdog.py | 2 +- snap-http | 2 +- 6 files changed, 56 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 21fb9f713..4d34aebf4 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,5 @@ landscape-client_ppc64el*.txt landscape-client_s390x*.txt landscape_client.egg-info .pybuild +venv +.venv diff --git a/landscape/client/deployment.py b/landscape/client/deployment.py index 28bc830cd..4e1b5c0ec 100644 --- a/landscape/client/deployment.py +++ b/landscape/client/deployment.py @@ -2,8 +2,10 @@ import os.path import subprocess import sys +import time from datetime import datetime from datetime import timezone +from logging import debug from optparse import SUPPRESS_HELP from twisted.logger import globalLogBeginner @@ -198,7 +200,7 @@ def juju_filename(self): backwards-compatibility.""" return os.path.join(self.data_path, "juju-info.json") - def auto_configure(self): + def auto_configure(self, retry=False, delay=120, max_retries=5): """Automatically configure the client snap.""" client_conf = snap_http.get_conf("landscape-client").result auto_enroll_conf = client_conf.get("auto-register", {}) @@ -208,14 +210,22 @@ def auto_configure(self): if not enabled or configured: return - title = generate_computer_title(auto_enroll_conf) - if title: - self.computer_title = title - self.write() + for _ in range(max_retries): + title = generate_computer_title(auto_enroll_conf) + if title: + self.computer_title = title + self.write() - auto_enroll_conf["configured"] = True - client_conf["auto-register"] = auto_enroll_conf - snap_http.set_conf("landscape-client", client_conf) + auto_enroll_conf["configured"] = True + client_conf["auto-register"] = auto_enroll_conf + snap_http.set_conf("landscape-client", client_conf) + + break + elif retry: + # retry until we get the computer title (exponential-backoff) + # no. of retries capped by `max_retries` + time.sleep(delay) + delay *= 2 def get_versioned_persist(service): @@ -243,11 +253,13 @@ def generate_computer_title(auto_enroll_config): snap_info = get_snap_info() wait_for_serial = auto_enroll_config.get("wait-for-serial-as", True) if "serial" not in snap_info and wait_for_serial: + debug(f"No serial assertion in snap info {snap_info}, waiting...") return hostname = get_fqdn() wait_for_hostname = auto_enroll_config.get("wait-for-hostname", False) if hostname == "localhost" and wait_for_hostname: + debug("Waiting for hostname...") return nics = get_active_device_info(default_only=True) diff --git a/landscape/client/tests/test_deployment.py b/landscape/client/tests/test_deployment.py index 83faa4bfc..4e4fbea6d 100644 --- a/landscape/client/tests/test_deployment.py +++ b/landscape/client/tests/test_deployment.py @@ -354,6 +354,29 @@ def test_auto_configuration_no_title_generated( self.assertIsNone(self.config.get("computer_title")) mock_snap_http.set_conf.assert_not_called() + @mock.patch("landscape.client.deployment.generate_computer_title") + @mock.patch("landscape.client.deployment.snap_http") + def test_auto_configuration_no_title_generated_retry( + self, + mock_snap_http, + mock_generate_title, + ): + """The client is not configured.""" + mock_snap_http.get_conf.return_value = SnapdResponse( + "sync", + 200, + "OK", + {"auto-register": {"enabled": True, "configured": False}}, + ) + mock_generate_title.return_value = None + + self.assertIsNone(self.config.get("computer_title")) + + self.config.auto_configure(retry=True, delay=0.01, max_retries=2) + assert mock_generate_title.call_count == 2 + self.assertIsNone(self.config.get("computer_title")) + mock_snap_http.set_conf.assert_not_called() + class GetVersionedPersistTest(LandscapeTest): def test_upgrade_service(self): @@ -424,10 +447,12 @@ def test_generate_computer_title( ) self.assertEqual(title, "classic-f315cab5") + @mock.patch("landscape.client.deployment.debug") @mock.patch("landscape.client.deployment.get_snap_info") def test_generate_computer_title_wait_for_serial_no_serial_assertion( self, mock_snap_info, + mock_debug, ): """Returns `None`.""" mock_snap_info.return_value = {} @@ -442,13 +467,18 @@ def test_generate_computer_title_wait_for_serial_no_serial_assertion( }, ) self.assertIsNone(title) + mock_debug.assert_called_once_with( + "No serial assertion in snap info {}, waiting..." + ) + @mock.patch("landscape.client.deployment.debug") @mock.patch("landscape.client.deployment.get_fqdn") @mock.patch("landscape.client.deployment.get_snap_info") def test_generate_computer_title_wait_for_hostname( self, mock_snap_info, mock_fqdn, + mock_debug, ): """Returns `None`.""" mock_snap_info.return_value = { @@ -468,6 +498,7 @@ def test_generate_computer_title_wait_for_hostname( }, ) self.assertIsNone(title) + mock_debug.assert_called_once_with("Waiting for hostname...") @mock.patch("landscape.client.deployment.subprocess") @mock.patch("landscape.client.deployment.get_active_device_info") diff --git a/landscape/client/tests/test_watchdog.py b/landscape/client/tests/test_watchdog.py index 4e8ad5a1c..a81695c88 100644 --- a/landscape/client/tests/test_watchdog.py +++ b/landscape/client/tests/test_watchdog.py @@ -1527,5 +1527,5 @@ def test_is_snap(self, mock_auto_configure): with mock.patch("landscape.client.watchdog.pwd", new=self.fake_pwd): run(["--log-dir", self.makeDir()], reactor=reactor) - mock_auto_configure.assert_called_once_with() + mock_auto_configure.assert_called_once_with(retry=True) self.assertTrue(reactor.running) diff --git a/landscape/client/watchdog.py b/landscape/client/watchdog.py index 2b5b469ed..ed2762a24 100644 --- a/landscape/client/watchdog.py +++ b/landscape/client/watchdog.py @@ -721,7 +721,7 @@ def run(args=sys.argv, reactor=None): init_logging(config, "watchdog") if IS_SNAP: - config.auto_configure() + config.auto_configure(retry=True) application = Application("landscape-client") watchdog_service = WatchDogService(config) diff --git a/snap-http b/snap-http index f3bb53973..dc1cdf396 160000 --- a/snap-http +++ b/snap-http @@ -1 +1 @@ -Subproject commit f3bb5397395296c345d85a0ea6e7665cb61d4321 +Subproject commit dc1cdf396daed1ce302cef6f614e7e088b83e877