Repository: apache/buildstream
Issue: Check versions of buildbox

[See original issue on GitLab](https://gitlab.com/BuildStream/buildstream/-/issues/1189)
In GitLab by [[Gitlab user @traveltissues]](https://gitlab.com/traveltissues) on Nov 1, 2019, 12:18

A number of issues have been reported with buildstream which were resolved by changing buildbox versions. This is not clearly suggested/indicated to the user. If possible buildstream should attempt to check the versions of buildbox that will be used before doing any work.

Changed files:
src/buildstream/_cas/cascache.py, tests/internals/cascache.py, src/buildstream/_cas/casdprocessmanager.py, src/buildstream/_cas/casserver.py, src/buildstream/_context.py

diff --git a/src/buildstream/_cas/cascache.py b/src/buildstream/_cas/cascache.py
index c6a58098f..eb3cc9161 100644
--- a/src/buildstream/_cas/cascache.py
+++ b/src/buildstream/_cas/cascache.py
@@ -71,7 +71,8 @@ class CASCache:
         remote_cache_spec=None,
         protect_session_blobs=True,
         log_level=CASLogLevel.WARNING,
-        log_directory=None
+        log_directory=None,
+        messenger=None
     ):
         self.casdir = os.path.join(path, "cas")
         self.tmpdir = os.path.join(path, "tmp")
@@ -88,7 +89,7 @@ class CASCache:
             assert log_directory is not None, "log_directory is required when casd is True"
             log_dir = os.path.join(log_directory, "_casd")
             self._casd_process_manager = CASDProcessManager(
-                path, log_dir, log_level, cache_quota, remote_cache_spec, protect_session_blobs
+                path, log_dir, log_level, cache_quota, remote_cache_spec, protect_session_blobs, messenger
             )
 
             self._casd_channel = self._casd_process_manager.create_channel()
diff --git a/src/buildstream/_cas/casdprocessmanager.py b/src/buildstream/_cas/casdprocessmanager.py
index 34f830483..e7a73c1b2 100644
--- a/src/buildstream/_cas/casdprocessmanager.py
+++ b/src/buildstream/_cas/casdprocessmanager.py
@@ -18,14 +18,16 @@
 import contextlib
 import threading
 import os
+import re
 import random
 import shutil
 import stat
 import subprocess
 import tempfile
 import time
-import psutil
+from subprocess import CalledProcessError
 
+import psutil
 import grpc
 
 from .._protos.build.bazel.remote.asset.v1 import remote_asset_pb2_grpc
@@ -40,6 +42,14 @@ _CASD_MAX_LOGFILES = 10
 _CASD_TIMEOUT = 300  # in seconds
 
 
+#
+# Minimum required version of buildbox-casd
+#
+_REQUIRED_CASD_MAJOR = 0
+_REQUIRED_CASD_MINOR = 0
+_REQUIRED_CASD_MICRO = 58
+
+
 # CASDProcessManager
 #
 # This manages the subprocess that runs buildbox-casd.
@@ -51,14 +61,18 @@ _CASD_TIMEOUT = 300  # in seconds
 #     cache_quota (int): User configured cache quota
 #     remote_cache_spec (RemoteSpec): Optional remote cache server
 #     protect_session_blobs (bool): Disable expiry for blobs used in the current session
+#     messenger (Messenger): The messenger to report warnings through the UI
 #
 class CASDProcessManager:
-    def __init__(self, path, log_dir, log_level, cache_quota, remote_cache_spec, protect_session_blobs):
+    def __init__(self, path, log_dir, log_level, cache_quota, remote_cache_spec, protect_session_blobs, messenger):
         self._log_dir = log_dir
 
         self._socket_path = self._make_socket_path(path)
         self._connection_string = "unix:" + self._socket_path
 
+        # Early version check
+        self._check_casd_version(messenger)
+
         casd_args = [utils.get_host_tool("buildbox-casd")]
         casd_args.append("--bind=" + self._connection_string)
         casd_args.append("--log-level=" + log_level.value)
@@ -92,6 +106,60 @@ class CASDProcessManager:
                 casd_args, cwd=path, stdout=logfile_fp, stderr=subprocess.STDOUT, preexec_fn=os.setpgrp
             )
 
+    # _check_casd_version()
+    #
+    # Check for minimal acceptable version of buildbox-casd.
+    #
+    # If the version is unacceptable, then an error is raised.
+    #
+    # If buildbox-casd was built without version information available (or has reported
+    # version information with a string which we are unprepared to parse), then
+    # a warning is produced to inform the user.
+    #
+    def _check_casd_version(self, messenger):
+        #
+        # We specify a trailing "path" argument because some versions of buildbox-casd
+        # require specifying the storage path even for invoking the --version option.
+        #
+        casd_args = [utils.get_host_tool("buildbox-casd")]
+        casd_args.append("--version")
+        casd_args.append("/")
+
+        try:
+            version_output = subprocess.check_output(casd_args)
+        except CalledProcessError as e:
+            raise CASCacheError("Error checking buildbox-casd version") from e
+
+        version_output = version_output.decode("utf-8")
+        version_match = re.match(r".*buildbox-casd (\\d+).(\\d+).(\\d+).*", version_output)
+
+        if version_match:
+            version_major = int(version_match.group(1))
+            version_minor = int(version_match.group(2))
+            version_micro = int(version_match.group(3))
+
+            acceptable_version = True
+            if version_major < _REQUIRED_CASD_MAJOR:
+                acceptable_version = False
+            elif version_major == _REQUIRED_CASD_MAJOR:
+                if version_minor < _REQUIRED_CASD_MINOR:
+                    acceptable_version = False
+                elif version_minor == _REQUIRED_CASD_MINOR:
+                    if version_micro < _REQUIRED_CASD_MICRO:
+                        acceptable_version = False
+
+            if not acceptable_version:
+                raise CASCacheError(
+                    "BuildStream requires buildbox-casd >= {}.{}.{}".format(
+                        _REQUIRED_CASD_MAJOR, _REQUIRED_CASD_MINOR, _REQUIRED_CASD_MICRO
+                    ),
+                    detail="Currently installed: {}".format(version_output),
+                )
+        elif messenger:
+            messenger.warn(
+                "Unable to determine buildbox-casd version", detail="buildbox-casd reported: {}".format(version_output)
+            )
+
     # _make_socket_path()
     #
     # Create a path to the CASD socket, ensuring that we don't exceed
diff --git a/src/buildstream/_cas/casserver.py b/src/buildstream/_cas/casserver.py
index 184ff438f..9736387e0 100644
--- a/src/buildstream/_cas/casserver.py
+++ b/src/buildstream/_cas/casserver.py
@@ -94,7 +94,7 @@ def create_server(repo, *, enable_push, quota, index_only, log_level=LogLevel.Le
     logger.addHandler(handler)
 
     casd_manager = CASDProcessManager(
-        os.path.abspath(repo), os.path.join(os.path.abspath(repo), "logs"), log_level, quota, None, False
+        os.path.abspath(repo), os.path.join(os.path.abspath(repo), "logs"), log_level, quota, None, False, None
     )
     casd_channel = casd_manager.create_channel()
 
diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py
index dfe369479..ecca27c10 100644
--- a/src/buildstream/_context.py
+++ b/src/buildstream/_context.py
@@ -688,6 +688,7 @@ class Context:
                 remote_cache_spec=self.remote_cache_spec,
                 log_level=log_level,
                 log_directory=self.logdir,
+                messenger=self.messenger,
             )
         return self._cascache
 
diff --git a/tests/internals/cascache.py b/tests/internals/cascache.py
index 046a14bbe..61d1b8312 100644
--- a/tests/internals/cascache.py
+++ b/tests/internals/cascache.py
@@ -7,9 +7,24 @@ from buildstream._cas import casdprocessmanager
 from buildstream._messenger import Messenger
 
 
+#
+# A dummy CASD script placeholder which supports the --version argument
+#
+DUMMY_CASD_SCRIPT_FMT = (
+    "#!/usr/bin/env sh\\n"
+    + "\\n"
+    + 'if test "$1" = "--version"; then\\n'
+    + '  echo "buildbox-casd 2.0.0"\\n'
+    + "  exit 0\\n"
+    + "fi\\n"
+    + "{}\\n"
+    + "exit 0\\n"
+)
+
+
 def test_report_when_cascache_dies_before_asked_to(tmp_path, monkeypatch):
     dummy_buildbox_casd = tmp_path.joinpath("buildbox-casd")
-    dummy_buildbox_casd.write_text("#!/usr/bin/env sh\\nexit 0")
+    dummy_buildbox_casd.write_text(DUMMY_CASD_SCRIPT_FMT.format(""))
     dummy_buildbox_casd.chmod(0o777)
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
 
@@ -27,7 +42,7 @@ def test_report_when_cascache_dies_before_asked_to(tmp_path, monkeypatch):
 
 def test_report_when_cascache_exits_not_cleanly(tmp_path, monkeypatch):
     dummy_buildbox_casd = tmp_path.joinpath("buildbox-casd")
-    dummy_buildbox_casd.write_text("#!/usr/bin/env sh\\nwhile :\\ndo\\nsleep 60\\ndone")
+    dummy_buildbox_casd.write_text(DUMMY_CASD_SCRIPT_FMT.format("while :\\ndo\\nsleep 60\\ndone"))
     dummy_buildbox_casd.chmod(0o777)
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
     # FIXME: this is a hack, we should instead have a socket be created nicely
@@ -49,7 +64,7 @@ def test_report_when_cascache_exits_not_cleanly(tmp_path, monkeypatch):
 
 def test_report_when_cascache_is_forcefully_killed(tmp_path, monkeypatch):
     dummy_buildbox_casd = tmp_path.joinpath("buildbox-casd")
-    dummy_buildbox_casd.write_text("#!/usr/bin/env sh\\ntrap 'echo hello' TERM\\nwhile :\\ndo\\nsleep 60\\ndone")
+    dummy_buildbox_casd.write_text(DUMMY_CASD_SCRIPT_FMT.format("trap 'echo hello' TERM\\nwhile :\\ndo\\nsleep 60\\ndone"))
     dummy_buildbox_casd.chmod(0o777)
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
     # FIXME: this is a hack, we should instead have a socket be created nicely
@@ -72,7 +87,7 @@ def test_casd_redirects_stderr_to_file_and_rotate(tmp_path, monkeypatch):
     n_max_log_files = 10
 
     dummy_buildbox_casd = tmp_path.joinpath("buildbox-casd")
-    dummy_buildbox_casd.write_text("#!/usr/bin/env sh\\nprintf '%s\\n' hello")
+    dummy_buildbox_casd.write_text(DUMMY_CASD_SCRIPT_FMT.format("printf '%s\\n' hello"))
     dummy_buildbox_casd.chmod(0o777)
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)