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

…network (#10559)

Fixes #10126.

update-server/Pipfile

@@ -18,12 +18,14 @@ pytest-watch = "~=4.2.0"
 pytest-cov = "==2.10.1"
 pytest-aiohttp = "==0.3.0"
 coverage = "==5.1"
-# pytest dependencies on windows, spec'd here to force lockfile inclusion
+# atomicwrites and colorama are pytest dependencies on windows,
+# spec'd here to force lockfile inclusion
 # https://github.com/pypa/pipenv/issues/4408#issuecomment-668324177
 atomicwrites = {version="==1.4.0", sys_platform="== 'win32'"}
 colorama = {version="==0.4.4", sys_platform="== 'win32'"}
 mypy = "==0.940"
 black = "==22.3.0"
+decoy = "~=1.10"
 
 [requires]
 python_version = "3.7"

update-server/mypy.ini

@@ -1,6 +1,7 @@
 [mypy]
 strict = True
 show_error_codes = True
+plugins = decoy.mypy
 
 
 # The dbus and systemd packages will not be installed in non-Linux dev environments.

update-server/otupdate/buildroot/__init__.py

@@ -3,14 +3,15 @@
 import logging
 import json
 from typing import Any, Mapping, Optional
+
 from aiohttp import web
 
 from otupdate.common import (
     config,
+    constants,
     control,
-    ssh_key_management,
     name_management,
-    constants,
+    ssh_key_management,
     update,
 )
 from . import update_actions
@@ -38,11 +39,11 @@ def get_version(version_file: str) -> Mapping[str, str]:
 
 
 def get_app(
+    name_synchronizer: name_management.NameSynchronizer,
     system_version_file: Optional[str] = None,
     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 +53,17 @@ 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",
-            ]
-        )
-    )
-
-    if not loop:
-        loop = asyncio.get_event_loop()
-
     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)
+    name_management.install_name_synchronizer(name_synchronizer, app)
+
     app.router.add_routes(
         [
             web.get(
@@ -106,6 +84,28 @@ def get_app(
             web.get("/server/name", name_management.get_name_endpoint),
         ]
     )
+
+    LOG.info(
+        "Setup: "
+        + "\n\t".join(
+            [
+                f"Device name: {name_synchronizer.get_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",
+            ]
+        )
+    )
+
     return app
 
 

update-server/otupdate/buildroot/__main__.py

@@ -3,45 +3,40 @@
 """
 import asyncio
 import logging
+from typing import NoReturn
 
-from . import get_app, constants
+from . import get_app
 
 from otupdate.common import name_management, cli, systemd
-from aiohttp import web
+from otupdate.common.run_application import run_and_notify_up
+
 
 LOG = logging.getLogger(__name__)
 
 
-def main() -> None:
+async def main() -> NoReturn:
     parser = cli.build_root_parser()
     args = parser.parse_args()
-    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 NameSynchronizer 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}")
-
-    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."
+    static_hostname = await name_management.set_up_static_hostname()
+    LOG.info(f"Set static hostname to {static_hostname}")
+
+    async with name_management.NameSynchronizer.start() as name_synchronizer:
+        LOG.info("Building buildroot update server")
+        app = get_app(
+            system_version_file=args.version_file,
+            config_file_override=args.config_file,
+            name_synchronizer=name_synchronizer,
         )
 
-    LOG.info(f"Notifying {systemd.SOURCE} that service is up")
-    systemd.notify_up()
-
-    LOG.info(f"Starting buildroot update server on http://{args.host}:{args.port}")
-    web.run_app(app, host=args.host, port=args.port)  # type: ignore[no-untyped-call]
+        LOG.info(f"Starting buildroot update server on http://{args.host}:{args.port}")
+        await run_and_notify_up(app=app, host=args.host, port=args.port)
 
 
 if __name__ == "__main__":
-    main()
+    asyncio.run(main())

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 get_name_synchronizer
 
 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": get_name_synchronizer(request).get_name(),
                     "serialNumber": get_serial(),
                     "bootId": request.app[DEVICE_BOOT_ID_NAME],
                 },

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

@@ -41,30 +41,20 @@
   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_synchronizer import (
+    NameSynchronizer,
+    install_name_synchronizer,
+    get_name_synchronizer,
 )
 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 +94,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_synchronizer = get_name_synchronizer(request)
+    new_name = await name_synchronizer.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 +110,17 @@ async def get_name_endpoint(request: web.Request) -> web.Response:
 
     GET /server/name -> 200 OK, {'name': robot name}
     """
+    name_synchronizer = get_name_synchronizer(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_synchronizer.get_name()}, status=200
     )
 
 
 __all__ = [
-    "get_pretty_hostname",
+    "NameSynchronizer",
+    "install_name_synchronizer",
+    "get_name_synchronizer",
     "set_up_static_hostname",
-    "set_avahi_service_name",
     "get_name_endpoint",
     "set_name_endpoint",
 ]