fix(robot-server): get versions from VERSION.json (#12361)

sfoster1 · SyntaxColoring · web-flow · commit 5dcc0fbf67b8 · 2023-03-27T18:07:29.000-04:00
In d7a9200 we switched to generating the version of the opentrons package from git, including feeding its __version__ by using importlib metadata instead of ingesting package.json. The problem is that __version__ now has a python-style semver string (e.g. 0.1.0a0) instead of a rest-of-the-world semver string (e.g. 0.1.0-alpha.0), which node semver doesn't like, which means that the app was rejecting the version the robot server reported in /health and falling back to the update server. This means things continued to work perfectly fine on OT-2, but on the flex a separate issue meant the update server wasn't reporting a version for the opentrons package, so versions broke. This change uses the VERSION.json file to get the "real" semver string for the opentrons package instead of the importlib metadata one. It also uses that file to load the system version rather than a config flag. There are fallbacks to the original versions of this implementation for both, which will be used in dev servers where VERSION.json is not present. Closes RQA-577 Co-authored-by: Max Marrone <max@opentrons.com>
diff --git a/robot-server/robot_server/health/router.py b/robot-server/robot_server/health/router.py
@@ -1,16 +1,92 @@
 """HTTP routes and handlers for /health endpoints."""
+from dataclasses import dataclass
 from fastapi import APIRouter, Depends, status
+from typing import Dict, cast
+import logging
+import json
 
 from opentrons import __version__, config, protocol_api
 from opentrons.hardware_control import HardwareControlAPI
 
 from robot_server.hardware import get_hardware
 from robot_server.persistence import get_sql_engine as ensure_sql_engine_is_ready
 from robot_server.service.legacy.models import V1BasicResponse
+from robot_server.util import call_once
 from .models import Health, HealthLinks
 
+_log = logging.getLogger(__name__)
 
 LOG_PATHS = ["/logs/serial.log", "/logs/api.log", "/logs/server.log"]
+VERSION_PATH = "/etc/VERSION.json"
+
+
+@dataclass
+class ComponentVersions:
+    """Holds the versions of system components."""
+
+    api_version: str
+    system_version: str
+
+
+@call_once
+async def _get_version() -> Dict[str, str]:
+    try:
+        with open(VERSION_PATH, "r") as version_file:
+            return cast(Dict[str, str], json.load(version_file))
+    except FileNotFoundError:
+        _log.warning(f"{VERSION_PATH} does not exist - is this a dev server?")
+        return {}
+    except OSError as ose:
+        _log.warning(
+            f"Could not open {VERSION_PATH}: {ose.errno}: {ose.strerror} - is this a dev server?"
+        )
+        return {}
+    except json.JSONDecodeError as jde:
+        _log.error(
+            f"Could not parse {VERSION_PATH}: {jde.msg} at line {jde.lineno} col {jde.colno}"
+        )
+        return {}
+    except Exception:
+        _log.exception(f"Failed to read version from {VERSION_PATH}")
+        return {}
+
+
+def _get_config_system_version() -> str:
+    return config.OT_SYSTEM_VERSION
+
+
+def _get_api_version_dunder() -> str:
+    return __version__
+
+
+async def get_versions() -> ComponentVersions:
+    """Dependency function for the versions of system components."""
+    version_file = await _get_version()
+
+    def _api_version_or_fallback() -> str:
+        if "opentrons_api_version" in version_file:
+            return version_file["opentrons_api_version"]
+        version_dunder = _get_api_version_dunder()
+        _log.warning(
+            f"Could not find api version in VERSION, falling back to {version_dunder}"
+        )
+        return version_dunder
+
+    def _system_version_or_fallback() -> str:
+        if "buildroot_version" in version_file:
+            return version_file["buildroot_version"]
+        if "openembedded_version" in version_file:
+            return version_file["openembedded_version"]
+        config_version = _get_config_system_version()
+        _log.warning(
+            f"Could not find system version in VERSION, falling back to {config_version}"
+        )
+        return config_version
+
+    return ComponentVersions(
+        api_version=_api_version_or_fallback(),
+        system_version=_system_version_or_fallback(),
+    )
 
 
 health_router = APIRouter()
@@ -37,6 +113,7 @@ async def get_health(
     # like viewing runs and uploading protocols, which would hit "database not ready"
     # errors that would present in a confusing way.
     sql_engine: object = Depends(ensure_sql_engine_is_ready),
+    versions: ComponentVersions = Depends(get_versions),
 ) -> Health:
     """Get information about the health of the robot server.
 
@@ -46,11 +123,11 @@ async def get_health(
     """
     return Health(
         name=config.name(),
-        api_version=__version__,
+        api_version=versions.api_version,
         fw_version=hardware.fw_version,
         board_revision=hardware.board_revision,
         logs=LOG_PATHS,
-        system_version=config.OT_SYSTEM_VERSION,
+        system_version=versions.system_version,
         maximum_protocol_api_version=list(protocol_api.MAX_SUPPORTED_VERSION),
         minimum_protocol_api_version=list(protocol_api.MIN_SUPPORTED_VERSION),
         robot_model=(
diff --git a/robot-server/tests/conftest.py b/robot-server/tests/conftest.py
@@ -39,6 +39,7 @@
 from robot_server.service.session.manager import SessionManager
 from robot_server.persistence import get_sql_engine, create_sql_engine
 from .integration.robot_client import RobotClient
+from robot_server.health.router import ComponentVersions, get_versions
 
 test_router = routing.APIRouter()
 
@@ -92,6 +93,16 @@ def hardware() -> MagicMock:
     return MagicMock(spec=API)
 
 
+@pytest.fixture
+def versions() -> MagicMock:
+    m = MagicMock(spec=get_versions)
+    m.return_value = ComponentVersions(
+        api_version="someTestApiVersion",
+        system_version="someTestSystemVersion",
+    )
+    return m
+
+
 @pytest.fixture
 def _override_hardware_with_mock(hardware: MagicMock) -> Iterator[None]:
     async def get_hardware_override() -> HardwareControlAPI:
@@ -114,10 +125,22 @@ async def get_sql_engine_override() -> SQLEngine:
     del app.dependency_overrides[get_sql_engine]
 
 
+@pytest.fixture
+def _override_version_with_mock(versions: MagicMock) -> Iterator[None]:
+    async def get_version_override() -> ComponentVersions:
+        """Override for the get_versions() FastAPI dependency."""
+        return cast(ComponentVersions, await versions())
+
+    app.dependency_overrides[get_versions] = get_version_override
+    yield
+    del app.dependency_overrides[get_versions]
+
+
 @pytest.fixture
 def api_client(
     _override_hardware_with_mock: None,
     _override_sql_engine_with_mock: None,
+    _override_version_with_mock: None,
 ) -> TestClient:
     client = TestClient(app)
     client.headers.update({API_VERSION_HEADER: LATEST_API_VERSION_HEADER_VALUE})
diff --git a/robot-server/tests/health/test_health_router.py b/robot-server/tests/health/test_health_router.py
@@ -1,23 +1,31 @@
 """Tests for the /health router."""
-from mock import MagicMock
+import pytest
+from typing import Dict, Iterator
+from mock import MagicMock, patch
 from starlette.testclient import TestClient
 
-from opentrons import __version__
 from opentrons.protocol_api import MAX_SUPPORTED_VERSION, MIN_SUPPORTED_VERSION
 
+from robot_server.health.router import ComponentVersions, get_versions, _get_version
 
-def test_get_health(api_client: TestClient, hardware: MagicMock) -> None:
+
+def test_get_health(
+    api_client: TestClient, hardware: MagicMock, versions: MagicMock
+) -> None:
     """Test GET /health."""
     hardware.fw_version = "FW111"
     hardware.board_revision = "BR2.1"
+    versions.return_value = ComponentVersions(
+        api_version="mytestapiversion", system_version="mytestsystemversion"
+    )
 
     expected = {
         "name": "opentrons-dev",
-        "api_version": __version__,
+        "api_version": "mytestapiversion",
         "fw_version": "FW111",
         "board_revision": "BR2.1",
         "logs": ["/logs/serial.log", "/logs/api.log", "/logs/server.log"],
-        "system_version": "0.0.0",
+        "system_version": "mytestsystemversion",
         "minimum_protocol_api_version": list(MIN_SUPPORTED_VERSION),
         "maximum_protocol_api_version": list(MAX_SUPPORTED_VERSION),
         "robot_model": "OT-2 Standard",
@@ -34,3 +42,90 @@ def test_get_health(api_client: TestClient, hardware: MagicMock) -> None:
     text = resp.json()
     assert resp.status_code == 200
     assert text == expected
+
+
+@pytest.fixture
+def mock_version_file_contents() -> Iterator[MagicMock]:
+    """Returns a mock for version file contents."""
+    with patch("robot_server.health.router._get_version", spec=_get_version) as p:
+        p.return_value = {}
+        yield p
+
+
+@pytest.fixture
+def mock_config_version() -> Iterator[MagicMock]:
+    """Returns a mock for the config version."""
+    with patch("robot_server.health.router._get_config_system_version") as p:
+        p.return_value = "mysystemversion"
+        yield p
+
+
+@pytest.fixture
+def mock_api_version() -> Iterator[MagicMock]:
+    """Returns a mock for the api version."""
+    with patch("robot_server.health.router._get_api_version_dunder") as p:
+        p.return_value = "myapiversion"
+        yield p
+
+
+@pytest.mark.parametrize(
+    "file_contents,config_system_version,api_version,computed_version",
+    [
+        (
+            {},
+            "rightsystemversion",
+            "rightapiversion",
+            ComponentVersions("rightapiversion", "rightsystemversion"),
+        ),
+        (
+            {"opentrons_api_version": "fileapiversion"},
+            "rightsystemversion",
+            "wrongapiversion",
+            ComponentVersions("fileapiversion", "rightsystemversion"),
+        ),
+        (
+            {"buildroot_version": "filesystemversion"},
+            "wrongsystemversion",
+            "rightapiversion",
+            ComponentVersions("rightapiversion", "filesystemversion"),
+        ),
+        (
+            {"openembedded_version": "filesystemversion"},
+            "wrongsystemversion",
+            "rightapiversion",
+            ComponentVersions("rightapiversion", "filesystemversion"),
+        ),
+        (
+            {
+                "opentrons_api_version": "fileapiversion",
+                "buildroot_version": "filesystemversion",
+            },
+            "wrongsystemversion",
+            "wrongapiversion",
+            ComponentVersions("fileapiversion", "filesystemversion"),
+        ),
+        (
+            {
+                "opentrons_api_version": "fileapiversion",
+                "openembedded_version": "filesystemversion",
+            },
+            "wrongsystemversion",
+            "wrongapiversion",
+            ComponentVersions("fileapiversion", "filesystemversion"),
+        ),
+    ],
+)
+async def test_version_dependency(
+    file_contents: Dict[str, str],
+    config_system_version: str,
+    api_version: str,
+    computed_version: ComponentVersions,
+    mock_version_file_contents: MagicMock,
+    mock_config_version: MagicMock,
+    mock_api_version: MagicMock,
+) -> None:
+    """Tests whether the version dependency function works."""
+    mock_version_file_contents.return_value = file_contents
+    mock_config_version.return_value = config_system_version
+    mock_api_version.return_value = api_version
+    assert (await get_versions()) == computed_version
