fix(update-server): Keep name deconflicted with other devices on the network

update-server/otupdate/buildroot/__init__.py

@@ -2,15 +2,16 @@
 import asyncio
 import logging
 import json
-from typing import Any, Mapping, Optional
+from typing import Any, AsyncGenerator, Mapping, Optional
 from aiohttp import web
 
 from otupdate.common import (
     config,
+    constants,
     control,
-    ssh_key_management,
     name_management,
-    constants,
+    ssh_key_management,
+    systemd,
     update,
 )
 from . import update_actions
@@ -42,7 +43,6 @@ def get_app(
     config_file_override: Optional[str] = None,
     name_override: Optional[str] = None,
     boot_id_override: Optional[str] = None,
-    loop: Optional[asyncio.AbstractEventLoop] = None,
 ) -> web.Application:
     """Build and return the aiohttp.web.Application that runs the server
 
@@ -52,40 +52,57 @@ def get_app(
         system_version_file = BR_BUILTIN_VERSION_FILE
 
     version = get_version(system_version_file)
-    name = name_override or name_management.get_pretty_hostname()
     boot_id = boot_id_override or control.get_boot_id()
     config_obj = config.load(config_file_override)
 
-    LOG.info(
-        "Setup: "
-        + "\n\t".join(
-            [
-                f"Device name: {name}",
-                "Buildroot version:         "
-                f'{version.get("buildroot_version", "unknown")}',
-                "\t(from git sha      " f'{version.get("buildroot_sha", "unknown")}',
-                "API version:               "
-                f'{version.get("opentrons_api_version", "unknown")}',
-                "\t(from git sha      "
-                f'{version.get("opentrons_api_sha", "unknown")}',
-                "Update server version:     "
-                f'{version.get("update_server_version", "unknown")}',
-                "\t(from git sha      "
-                f'{version.get("update_server_sha", "unknown")}',
-                "Smoothie firmware version: TODO",
-            ]
-        )
-    )
+    app = web.Application(middlewares=[log_error_middleware])
 
-    if not loop:
-        loop = asyncio.get_event_loop()
+    async def set_up_and_tear_down(app: web.Application) -> AsyncGenerator[None, None]:
+        # Stuff everything inside here so that:
+        # - Getting the order right is more foolproof
+        # - We can log it all together
+
+        app[config.CONFIG_VARNAME] = config_obj
+        app[constants.RESTART_LOCK_NAME] = asyncio.Lock()
+        app[constants.DEVICE_BOOT_ID_NAME] = boot_id
+
+        update_actions.OT2UpdateActions.build_and_insert(app)
+
+        async with name_management.build_name_manager(
+            name_override=name_override
+        ) as name_manager:
+            name_manager.install_on_app(app)
+            initial_name = name_manager.get_name()
+
+            LOG.info(
+                "Setup: "
+                + "\n\t".join(
+                    [
+                        f"Device name: {initial_name}",
+                        "Buildroot version:         "
+                        f'{version.get("buildroot_version", "unknown")}',
+                        "\t(from git sha      "
+                        f'{version.get("buildroot_sha", "unknown")}',
+                        "API version:               "
+                        f'{version.get("opentrons_api_version", "unknown")}',
+                        "\t(from git sha      "
+                        f'{version.get("opentrons_api_sha", "unknown")}',
+                        "Update server version:     "
+                        f'{version.get("update_server_version", "unknown")}',
+                        "\t(from git sha      "
+                        f'{version.get("update_server_sha", "unknown")}',
+                        "Smoothie firmware version: TODO",
+                    ]
+                )
+            )
+
+            LOG.info(f"Notifying {systemd.SOURCE} that service is up.")
+            systemd.notify_up()
+
+            yield
+
+    app.cleanup_ctx.append(set_up_and_tear_down)
 
-    app = web.Application(middlewares=[log_error_middleware])
-    app[config.CONFIG_VARNAME] = config_obj
-    app[constants.RESTART_LOCK_NAME] = asyncio.Lock()
-    app[constants.DEVICE_BOOT_ID_NAME] = boot_id
-    app[constants.DEVICE_NAME_VARNAME] = name
-    update_actions.OT2UpdateActions.build_and_insert(app)
     app.router.add_routes(
         [
             web.get(
@@ -106,6 +123,7 @@ def get_app(
             web.get("/server/name", name_management.get_name_endpoint),
         ]
     )
+
     return app
 
 

update-server/otupdate/buildroot/__main__.py

@@ -4,7 +4,7 @@
 import asyncio
 import logging
 
-from . import get_app, constants
+from . import get_app
 
 from otupdate.common import name_management, cli, systemd
 from aiohttp import web
@@ -18,24 +18,15 @@ def main() -> None:
     loop = asyncio.get_event_loop()
     systemd.configure_logging(getattr(logging, args.log_level.upper()))
 
+    # Because this involves restarting Avahi, this must happen early,
+    # before the NameManager starts up and connects to Avahi.
     LOG.info("Setting static hostname")
-    hostname = loop.run_until_complete(name_management.set_up_static_hostname())
-    LOG.info(f"Set static hostname to {hostname}")
+    static_hostname = loop.run_until_complete(name_management.set_up_static_hostname())
+    LOG.info(f"Set static hostname to {static_hostname}")
 
     LOG.info("Building buildroot update server")
     app = get_app(args.version_file, args.config_file)
 
-    name = app[constants.DEVICE_NAME_VARNAME]  # Set by get_app().
-
-    LOG.info(f"Setting Avahi service name to {name}")
-    try:
-        loop.run_until_complete(name_management.set_avahi_service_name(name))
-    except Exception:
-        LOG.exception(
-            "Error setting the Avahi service name."
-            " mDNS + DNS-SD advertisement may not work."
-        )
-
     LOG.info(f"Notifying {systemd.SOURCE} that service is up")
     systemd.notify_up()
 

update-server/otupdate/buildroot/constants.py

@@ -6,9 +6,6 @@
 RESTART_LOCK_NAME = APP_VARIABLE_PREFIX + "restartlock"
 #: Name for the asyncio lock in the application dictlike
 
-DEVICE_NAME_VARNAME = APP_VARIABLE_PREFIX + "name"
-#: Name for the device
-
 DEVICE_BOOT_ID_NAME = APP_VARIABLE_PREFIX + "boot_id"
 #: A random string that changes every time the device boots.
 #:

update-server/otupdate/common/constants.py

@@ -6,9 +6,6 @@
 RESTART_LOCK_NAME = APP_VARIABLE_PREFIX + "restartlock"
 #: Name for the asyncio lock in the application dictlike
 
-DEVICE_NAME_VARNAME = APP_VARIABLE_PREFIX + "name"
-#: Name for the device
-
 DEVICE_BOOT_ID_NAME = APP_VARIABLE_PREFIX + "boot_id"
 #: A random string that changes every time the device boots.
 #:

update-server/otupdate/common/control.py

@@ -12,7 +12,8 @@
 
 from aiohttp import web
 
-from .constants import RESTART_LOCK_NAME, DEVICE_BOOT_ID_NAME, DEVICE_NAME_VARNAME
+from .constants import RESTART_LOCK_NAME, DEVICE_BOOT_ID_NAME
+from .name_management import NameManager
 
 LOG = logging.getLogger(__name__)
 
@@ -41,7 +42,7 @@ async def health(request: web.Request) -> web.Response:
             {
                 **health_response,
                 **{
-                    "name": request.app[DEVICE_NAME_VARNAME],
+                    "name": NameManager.from_request(request).get_name(),
                     "serialNumber": get_serial(),
                     "bootId": request.app[DEVICE_BOOT_ID_NAME],
                 },

update-server/otupdate/common/name_management/__init__.py

@@ -41,30 +41,16 @@
   See `set_name_endpoint()`.
 """
 
+from __future__ import annotations
 
 import json
 
 from aiohttp import web
 
-from ..constants import DEVICE_NAME_VARNAME
-
-from .avahi import set_avahi_service_name
-from .pretty_hostname import (
-    get_pretty_hostname,
-    persist_pretty_hostname,
-)
+from .name_manager import NameManager, build_name_manager
 from .static_hostname import set_up_static_hostname
 
 
-async def set_name(app: web.Application, new_name: str) -> str:
-    """See `set_name_endpoint()`."""
-    await set_avahi_service_name(new_name)
-    # Setting the Avahi service name can fail if Avahi doesn't like the new name.
-    # Persist only after it succeeds, so we don't persist something invalid.
-    persisted_pretty_hostname = await persist_pretty_hostname(new_name)
-    return persisted_pretty_hostname
-
-
 async def set_name_endpoint(request: web.Request) -> web.Response:
     """Set the robot's name.
 
@@ -104,9 +90,9 @@ def build_400(msg: str) -> web.Response:
     if not isinstance(name_to_set, str):
         return build_400('"name" key is not a string"')
 
-    new_name = await set_name(app=request.app, new_name=name_to_set)
+    name_manager = NameManager.from_request(request)
+    new_name = await name_manager.set_name(new_name=name_to_set)
 
-    request.app[DEVICE_NAME_VARNAME] = new_name
     return web.json_response(  # type: ignore[no-untyped-call,no-any-return]
         data={"name": new_name}, status=200
     )
@@ -120,15 +106,16 @@ async def get_name_endpoint(request: web.Request) -> web.Response:
 
     GET /server/name -> 200 OK, {'name': robot name}
     """
+    name_manager = NameManager.from_request(request)
     return web.json_response(  # type: ignore[no-untyped-call,no-any-return]
-        data={"name": request.app[DEVICE_NAME_VARNAME]}, status=200
+        data={"name": name_manager.get_name()}, status=200
     )
 
 
 __all__ = [
-    "get_pretty_hostname",
+    "NameManager",
+    "build_name_manager",
     "set_up_static_hostname",
-    "set_avahi_service_name",
     "get_name_endpoint",
     "set_name_endpoint",
 ]