Merge branch 'juerg/cache-quota' into 'master'

Cache quota configuration fixes

See merge request BuildStream/buildstream!1566

2 files changed, 13 insertions, 48 deletions

diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py
index c6cde4003..0d250eb56 100644
--- a/src/buildstream/_context.py
+++ b/src/buildstream/_context.py
@@ -242,6 +242,12 @@ class Context():
             path = os.path.normpath(path)
             setattr(self, directory, path)
 
+            # Relative paths don't make sense in user configuration. The exception is
+            # workspacedir where `.` is useful as it will be combined with the name
+            # specified on the command line.
+            if not os.path.isabs(path) and not (directory == 'workspacedir' and path == '.'):
+                raise LoadError("{} must be an absolute path".format(directory), LoadErrorReason.INVALID_DATA)
+
         # add directories not set by users
         self.tmpdir = os.path.join(self.cachedir, 'tmp')
         self.casdir = os.path.join(self.cachedir, 'cas')
@@ -264,14 +270,18 @@ class Context():
 
         # Load quota configuration
         # We need to find the first existing directory in the path of our
-        # cachedir - the cachedir may not have been created yet.
+        # casdir - the casdir may not have been created yet.
         cache = defaults.get_mapping('cache')
         cache.validate_keys(['quota', 'pull-buildtrees', 'cache-buildtrees'])
 
+        cas_volume = self.casdir
+        while not os.path.exists(cas_volume):
+            cas_volume = os.path.dirname(cas_volume)
+
         self.config_cache_quota_string = cache.get_str('quota')
         try:
             self.config_cache_quota = utils._parse_size(self.config_cache_quota_string,
-                                                        self.casdir)
+                                                        cas_volume)
         except utils.UtilError as e:
             raise LoadError("{}\nPlease specify the value in bytes or as a % of full disk space.\n"
                             "\nValid values are, for example: 800M 10G 1T 50%\n"
diff --git a/tests/artifactcache/expiry.py b/tests/artifactcache/expiry.py
index e0bbb4007..3e252fe2d 100644
--- a/tests/artifactcache/expiry.py
+++ b/tests/artifactcache/expiry.py
@@ -21,7 +21,6 @@
 # pylint: disable=redefined-outer-name
 
 import os
-from unittest import mock
 
 import pytest
 
@@ -304,10 +303,6 @@ def test_never_delete_required_track(cli, datafiles):
 # Ensure that only valid cache quotas make it through the loading
 # process.
 #
-# This test virtualizes the condition to assume a storage volume
-# has 10K total disk space, and 6K of it is already in use (not
-# including any space used by the artifact cache).
-#
 # Parameters:
 #    quota (str): A quota size configuration for the config file
 #    err_domain (str): An ErrorDomain, or 'success' or 'warning'
@@ -315,9 +310,6 @@ def test_never_delete_required_track(cli, datafiles):
 #
 # If err_domain is 'success', then err_reason is unused.
 #
-# If err_domain is 'warning', then err_reason is asserted to
-# be in the stderr.
-#
 @pytest.mark.parametrize("quota,err_domain,err_reason", [
     # Valid configurations
     ("1", 'success', None),
@@ -329,16 +321,8 @@ def test_never_delete_required_track(cli, datafiles):
     ("-1", ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA),
     ("pony", ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA),
     ("200%", ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA),
-
-    # Not enough space on disk even if you cleaned up
-    ("11K", ErrorDomain.CAS, 'insufficient-storage-for-quota'),
-
-    # Not enough space for these caches
-    ("7K", 'warning', 'Your system does not have enough available'),
-    ("70%", 'warning', 'Your system does not have enough available')
 ])
 @pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.xfail()
 def test_invalid_cache_quota(cli, datafiles, quota, err_domain, err_reason):
     project = str(datafiles)
     os.makedirs(os.path.join(project, 'elements'))
@@ -349,39 +333,10 @@ def test_invalid_cache_quota(cli, datafiles, quota, err_domain, err_reason):
         },
     })
 
-    # We patch how we get space information
-    # Ideally we would instead create a FUSE device on which we control
-    # everything.
-    # If the value is a percentage, we fix the current values to take into
-    # account the block size, since this is important in how we compute the size
-
-    if quota.endswith("%"):  # We set the used space at 60% of total space
-        stats = os.statvfs(".")
-        free_space = 0.6 * stats.f_bsize * stats.f_blocks
-        total_space = stats.f_bsize * stats.f_blocks
-    else:
-        free_space = 6000
-        total_space = 10000
-
-    volume_space_patch = mock.patch(
-        "buildstream.utils._get_volume_size",
-        autospec=True,
-        return_value=(total_space, free_space),
-    )
-
-    cache_size_patch = mock.patch(
-        "buildstream._cas.CASQuota.get_cache_size",
-        autospec=True,
-        return_value=0,
-    )
-
-    with volume_space_patch, cache_size_patch:
-        res = cli.run(project=project, args=['workspace', 'list'])
+    res = cli.run(project=project, args=['workspace', 'list'])
 
     if err_domain == 'success':
         res.assert_success()
-    elif err_domain == 'warning':
-        assert err_reason in res.stderr
     else:
         res.assert_main_error(err_domain, err_reason)