Skip to content

Commit

Permalink
fix: retry setting the computer title
Browse files Browse the repository at this point in the history
  • Loading branch information
st3v3nmw committed Jul 23, 2024
1 parent f4c7f7e commit cd418ee
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 11 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,5 @@ landscape-client_ppc64el*.txt
landscape-client_s390x*.txt
landscape_client.egg-info
.pybuild
venv
.venv
28 changes: 20 additions & 8 deletions landscape/client/deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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", {})
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions landscape/client/tests/test_deployment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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 = {}
Expand All @@ -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 = {
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion landscape/client/tests/test_watchdog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion landscape/client/watchdog.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion snap-http
Submodule snap-http updated 2 files
+14 −0 CHANGELOG.md
+1 −1 pyproject.toml

0 comments on commit cd418ee

Please sign in to comment.