Merge branch 'bschubert/stricter-cas-soc' into 'master'

Don't create Buildstream-related directories in the cas - owned directory

See merge request BuildStream/buildstream!1832

7 files changed, 36 insertions, 21 deletions

diff --git a/src/buildstream/_cas/cascache.py b/src/buildstream/_cas/cascache.py
index cfdd4af09..48b700c68 100644
--- a/src/buildstream/_cas/cascache.py
+++ b/src/buildstream/_cas/cascache.py
@@ -62,14 +62,21 @@ class CASLogLevel(FastEnum):
 #     cache_quota (int): User configured cache quota
 #     protect_session_blobs (bool): Disable expiry for blobs used in the current session
 #     log_level (LogLevel): Log level to give to buildbox-casd for logging
+#     log_directory (str): the root of the directory in which to store logs
 #
 class CASCache:
     def __init__(
-        self, path, *, casd=True, cache_quota=None, protect_session_blobs=True, log_level=CASLogLevel.WARNING
+        self,
+        path,
+        *,
+        casd=True,
+        cache_quota=None,
+        protect_session_blobs=True,
+        log_level=CASLogLevel.WARNING,
+        log_directory=None
     ):
         self.casdir = os.path.join(path, "cas")
         self.tmpdir = os.path.join(path, "tmp")
-        os.makedirs(os.path.join(self.casdir, "objects"), exist_ok=True)
         os.makedirs(self.tmpdir, exist_ok=True)
 
         self._cache_usage_monitor = None
@@ -78,7 +85,8 @@ class CASCache:
         self._casd_process_manager = None
         self._casd_channel = None
         if casd:
-            log_dir = os.path.join(self.casdir, "logs")
+            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, protect_session_blobs
             )
diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py
index c3ea52f0e..090b3e0bc 100644
--- a/src/buildstream/_context.py
+++ b/src/buildstream/_context.py
@@ -534,7 +534,11 @@ class Context:
                 log_level = CASLogLevel.WARNING
 
             self._cascache = CASCache(
-                self.cachedir, casd=self.use_casd, cache_quota=self.config_cache_quota, log_level=log_level
+                self.cachedir,
+                casd=self.use_casd,
+                cache_quota=self.config_cache_quota,
+                log_level=log_level,
+                log_directory=self.logdir,
             )
         return self._cascache
 
diff --git a/tests/artifactcache/expiry.py b/tests/artifactcache/expiry.py
index 1474cecf6..f2be797c3 100644
--- a/tests/artifactcache/expiry.py
+++ b/tests/artifactcache/expiry.py
@@ -36,7 +36,7 @@ DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "expiry")
 
 
 def get_cache_usage(directory):
-    cas_cache = CASCache(directory)
+    cas_cache = CASCache(directory, log_directory=os.path.dirname(directory))
     try:
         wait = 0.1
         for _ in range(0, int(5 / wait)):
diff --git a/tests/internals/cascache.py b/tests/internals/cascache.py
index d669f2a2c..f095663a3 100644
--- a/tests/internals/cascache.py
+++ b/tests/internals/cascache.py
@@ -14,7 +14,7 @@ def test_report_when_cascache_dies_before_asked_to(tmp_path, monkeypatch):
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
 
     messenger = MagicMock(spec_set=Messenger)
-    cache = CASCache(str(tmp_path.joinpath("casd")), casd=True)
+    cache = CASCache(str(tmp_path.joinpath("casd")), casd=True, log_directory=str(tmp_path.joinpath("logs")))
     time.sleep(1)
     cache.release_resources(messenger)
 
@@ -26,14 +26,14 @@ def test_report_when_cascache_dies_before_asked_to(tmp_path, monkeypatch):
     assert "died" in message.message
 
 
-def test_report_when_cascache_exist_not_cleanly(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.chmod(0o777)
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
 
     messenger = MagicMock(spec_set=Messenger)
-    cache = CASCache(str(tmp_path.joinpath("casd")), casd=True)
+    cache = CASCache(str(tmp_path.joinpath("casd")), casd=True, log_directory=str(tmp_path.joinpath("logs")))
     time.sleep(1)
     cache.release_resources(messenger)
 
@@ -52,7 +52,7 @@ def test_report_when_cascache_is_forcefully_killed(tmp_path, monkeypatch):
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
 
     messenger = MagicMock(spec_set=Messenger)
-    cache = CASCache(str(tmp_path.joinpath("casd")), casd=True)
+    cache = CASCache(str(tmp_path.joinpath("casd")), casd=True, log_directory=str(tmp_path.joinpath("logs")))
     time.sleep(1)
     cache.release_resources(messenger)
 
@@ -72,7 +72,8 @@ def test_casd_redirects_stderr_to_file_and_rotate(tmp_path, monkeypatch):
     monkeypatch.setenv("PATH", str(tmp_path), prepend=os.pathsep)
 
     casd_files_path = tmp_path.joinpath("casd")
-    casd_logs_path = casd_files_path.joinpath("cas", "logs")
+    casd_parent_logs_path = tmp_path.joinpath("logs")
+    casd_logs_path = casd_parent_logs_path.joinpath("_casd")
 
     # Ensure we don't have any files in the log directory
     assert not casd_logs_path.exists()
@@ -80,7 +81,7 @@ def test_casd_redirects_stderr_to_file_and_rotate(tmp_path, monkeypatch):
 
     # Let's create the first `n_max_log_files` log files
     for i in range(1, n_max_log_files + 1):
-        cache = CASCache(str(casd_files_path), casd=True)
+        cache = CASCache(str(casd_files_path), casd=True, log_directory=str(casd_parent_logs_path))
         time.sleep(0.05)
         cache.release_resources()
 
@@ -92,7 +93,7 @@ def test_casd_redirects_stderr_to_file_and_rotate(tmp_path, monkeypatch):
     for _ in range(3):
         evicted_file = existing_log_files.pop(0)
 
-        cache = CASCache(str(casd_files_path), casd=True)
+        cache = CASCache(str(casd_files_path), casd=True, log_directory=str(casd_parent_logs_path))
         time.sleep(0.05)
         cache.release_resources()
 
diff --git a/tests/internals/storage.py b/tests/internals/storage.py
index 89a198d98..2e636cc53 100644
--- a/tests/internals/storage.py
+++ b/tests/internals/storage.py
@@ -21,7 +21,7 @@ def setup_backend(backend_class, tmpdir):
     if backend_class == FileBasedDirectory:
         yield backend_class(os.path.join(tmpdir, "vdir"))
     else:
-        cas_cache = CASCache(tmpdir)
+        cas_cache = CASCache(os.path.join(tmpdir, "cas"), log_directory=os.path.join(tmpdir, "logs"))
         try:
             yield backend_class(cas_cache)
         finally:
diff --git a/tests/internals/storage_vdir_import.py b/tests/internals/storage_vdir_import.py
index fe3012712..459cb48dd 100644
--- a/tests/internals/storage_vdir_import.py
+++ b/tests/internals/storage_vdir_import.py
@@ -185,7 +185,7 @@ def directory_not_empty(path):
 
 
 def _import_test(tmpdir, original, overlay, generator_function, verify_contents=False):
-    cas_cache = CASCache(tmpdir)
+    cas_cache = CASCache(tmpdir, log_directory=os.path.join(tmpdir, "logs"))
     try:
         # Create some fake content
         generator_function(original, tmpdir)
@@ -258,7 +258,7 @@ def test_random_cas_import(tmpdir, original, overlay):
 
 
 def _listing_test(tmpdir, root, generator_function):
-    cas_cache = CASCache(tmpdir)
+    cas_cache = CASCache(tmpdir, log_directory=os.path.join(tmpdir, "logs"))
     try:
         # Create some fake content
         generator_function(root, tmpdir)
@@ -287,7 +287,7 @@ def test_fixed_directory_listing(tmpdir, root):
 # Check that the vdir is decending and readable
 def test_descend(tmpdir):
     cas_dir = os.path.join(str(tmpdir), "cas")
-    cas_cache = CASCache(cas_dir)
+    cas_cache = CASCache(cas_dir, log_directory=os.path.join(str(tmpdir), "logs"))
     try:
         d = CasBasedDirectory(cas_cache)
 
@@ -309,7 +309,7 @@ def test_descend(tmpdir):
 # to decend in to files or links to files
 def test_bad_symlinks(tmpdir):
     cas_dir = os.path.join(str(tmpdir), "cas")
-    cas_cache = CASCache(cas_dir)
+    cas_cache = CASCache(cas_dir, log_directory=os.path.join(str(tmpdir), "logs"))
     try:
         d = CasBasedDirectory(cas_cache)
 
@@ -338,7 +338,7 @@ def test_bad_symlinks(tmpdir):
 # Check decend accross relitive link
 def test_relative_symlink(tmpdir):
     cas_dir = os.path.join(str(tmpdir), "cas")
-    cas_cache = CASCache(cas_dir)
+    cas_cache = CASCache(cas_dir, log_directory=os.path.join(str(tmpdir), "logs"))
     try:
         d = CasBasedDirectory(cas_cache)
 
@@ -363,7 +363,7 @@ def test_relative_symlink(tmpdir):
 # Check deccend accross abs link
 def test_abs_symlink(tmpdir):
     cas_dir = os.path.join(str(tmpdir), "cas")
-    cas_cache = CASCache(cas_dir)
+    cas_cache = CASCache(cas_dir, log_directory=os.path.join(str(tmpdir), "logs"))
     try:
         d = CasBasedDirectory(cas_cache)
 
@@ -389,7 +389,7 @@ def test_abs_symlink(tmpdir):
 # Check symlink can not escape root
 def test_bad_sym_escape(tmpdir):
     cas_dir = os.path.join(str(tmpdir), "cas")
-    cas_cache = CASCache(cas_dir)
+    cas_cache = CASCache(cas_dir, log_directory=os.path.join(str(tmpdir), "logs"))
     try:
         d = CasBasedDirectory(cas_cache)
 
diff --git a/tests/testutils/artifactshare.py b/tests/testutils/artifactshare.py
index 19c19131a..a15a7c27e 100644
--- a/tests/testutils/artifactshare.py
+++ b/tests/testutils/artifactshare.py
@@ -123,7 +123,9 @@ class ArtifactShare(BaseArtifactShare):
         self.sourcedir = os.path.join(self.repodir, "source_protos", "refs")
         os.makedirs(self.sourcedir)
 
-        self.cas = CASCache(self.repodir, casd=casd)
+        logdir = os.path.join(self.directory, "logs") if casd else None
+
+        self.cas = CASCache(self.repodir, casd=casd, log_directory=logdir)
 
         self.quota = quota
         self.index_only = index_only