You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by tv...@apache.org on 2021/01/25 08:19:52 UTC
[buildstream] 01/04: Remote execution can only be configured in
user configuration
This is an automated email from the ASF dual-hosted git repository.
tvb pushed a commit to branch tristan/change-remote-config
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 7898680d6a38341ed580ee3b2e302df0bf756b93
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Sun Jan 17 20:19:00 2021 +0900
Remote execution can only be configured in user configuration
As outlined on the mailing list[0], remote execution requires write access
and certificates, as such there is no point for the project to make
recommendations on this user configuration.
Summary of changes:
* _project.py: Removed RemoteExecutionSpec parsing
* _context.py: Adjusted parsing of RemoteExecutionSpec
First parse it initially, and then search for an override once
the toplevel project is loaded, only the toplevel project override
is significant, remote execution is only supported on a single
build cluster for a given session.
* element.py: Check context for whether remote execution is enabled
at build time, instead of the element's project.
* _stream.py: Check context for whether remote execution is enabled
at initialization time, instead of checking all projects.
* sandbox/_sandboxremote.py: Removed one keyword argument from the
constructor, no need to specify the RemoteExecutionSpec in the
constructor since the sandbox can just get that from the context object.
* tests/sandboxes/remote-exec-config.py: Test with buildstream.conf
This test was testing invalid configurations of remote-execution
on project.conf files, which is no longer supported with this commit.
This also removes one test which tests for an error with an empty
configuration, this failure does not trigger when tested via
project.conf due to the allowance of "pull-artifact-files" to sit
alone within that configuration block.
[0]: https://lists.apache.org/thread.html/rf2da9830e2fa918357f99a6021e55fc43df876f0b19d43f68802f083%40%3Cdev.buildstream.apache.org%3E
---
src/buildstream/_context.py | 43 ++++++++++++++++------
src/buildstream/_project.py | 23 +-----------
src/buildstream/_stream.py | 4 +-
src/buildstream/element.py | 3 +-
src/buildstream/sandbox/_sandboxremote.py | 3 +-
tests/sandboxes/remote-exec-config.py | 35 ++++--------------
.../remote-exec-config/missing-certs/project.conf | 2 +
7 files changed, 47 insertions(+), 66 deletions(-)
diff --git a/src/buildstream/_context.py b/src/buildstream/_context.py
index 7a81d10..bae97ab 100644
--- a/src/buildstream/_context.py
+++ b/src/buildstream/_context.py
@@ -150,7 +150,7 @@ class Context:
self.pull_buildtrees = None
# Whether to pull the files of an artifact when doing remote execution
- self.pull_artifact_files = None
+ self.pull_artifact_files = True
# Whether or not to cache build trees on artifact creation
self.cache_buildtrees = None
@@ -333,19 +333,10 @@ class Context:
# Load source cache config
self.source_cache_specs = SourceCache.specs_from_config_node(defaults)
- # Load remote execution config getting pull-artifact-files from it
+ # Load the global remote execution config including pull-artifact-files setting
remote_execution = defaults.get_mapping("remote-execution", default=None)
if remote_execution:
- self.pull_artifact_files = remote_execution.get_bool("pull-artifact-files", default=True)
- # This stops it being used in the remote service set up
- remote_execution.safe_del("pull-artifact-files")
-
- # Don't pass the remote execution settings if that was the only option
- if remote_execution.keys():
- self.remote_execution_specs = RemoteExecutionSpec.new_from_node(remote_execution)
- else:
- self.pull_artifact_files = True
- self.remote_execution_specs = None
+ self.pull_artifact_files, self.remote_execution_specs = self._load_remote_execution(remote_execution)
# Load pull build trees configuration
self.pull_buildtrees = cache.get_bool("pull-buildtrees")
@@ -448,6 +439,19 @@ class Context:
def add_project(self, project):
if not self._projects:
self._workspaces = Workspaces(project, self._workspace_project_cache)
+
+ #
+ # While loading the first, toplevel project, we can adjust some
+ # global settings which can be overridden on a per toplevel project basis.
+ #
+ override_node = self.get_overrides(project.name)
+ if override_node:
+ remote_execution = override_node.get_mapping("remote-execution", default=None)
+ if remote_execution:
+ self.pull_artifact_files, self.remote_execution_specs = self._load_remote_execution(
+ remote_execution
+ )
+
self._projects.append(project)
# get_projects():
@@ -563,3 +567,18 @@ class Context:
log_directory=self.logdir,
)
return self._cascache
+
+ def _load_remote_execution(self, node):
+ # The pull_artifact_files attribute is special, it is allowed to
+ # be set to False even if there is no remote execution service configured.
+ #
+ pull_artifact_files = node.get_bool("pull-artifact-files", default=True)
+ node.safe_del("pull-artifact-files")
+
+ # Don't pass the remote execution settings if that was the only option
+ if node.keys():
+ remote_execution_specs = RemoteExecutionSpec.new_from_node(node)
+ else:
+ remote_execution_specs = None
+
+ return pull_artifact_files, remote_execution_specs
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 8e9c9ed..f1d1fa6 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -43,7 +43,7 @@ from ._loader import Loader, LoadContext
from .element import Element
from ._includes import Includes
from ._workspaces import WORKSPACE_PROJECT_FILE
-from ._remotespec import RemoteSpec, RemoteExecutionSpec
+from ._remotespec import RemoteSpec
if TYPE_CHECKING:
@@ -141,7 +141,6 @@ class Project:
# Remote specs for communicating with remote services
self.artifact_cache_specs: List[RemoteSpec] = [] # Artifact caches
self.source_cache_specs: List[RemoteSpec] = [] # Source caches
- self.remote_execution_specs: Optional[RemoteExecutionSpec] = None # Remote execution services
self.element_factory: Optional[ElementFactory] = None # ElementFactory for loading elements
self.source_factory: Optional[SourceFactory] = None # SourceFactory for loading sources
@@ -881,26 +880,6 @@ class Project:
# Load source caches with pull/push config
self.source_cache_specs = SourceCache.specs_from_config_node(config, self.directory)
- # Load remote-execution configuration for this project
- project_re_specs = None
- project_re_node = config.get_mapping("remote-execution", default=None)
- if project_re_node:
- project_re_specs = RemoteExecutionSpec.new_from_node(project_re_node, self.directory)
-
- override_re_specs = None
- override_node = self._context.get_overrides(self.name)
- if override_node:
- override_re_node = override_node.get_mapping("remote-execution", default=None)
- if override_re_node:
- override_re_specs = RemoteExecutionSpec.new_from_node(override_re_node, self.directory)
-
- if override_re_specs is not None:
- self.remote_execution_specs = override_re_specs
- elif project_re_specs is not None:
- self.remote_execution_specs = project_re_specs
- else:
- self.remote_execution_specs = self._context.remote_execution_specs
-
# Load sandbox environment variables
self.base_environment = config.get_mapping("environment")
self.base_env_nocache = config.get_str_list("environment-nocache")
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 6eb25e8..fa2421e 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -286,8 +286,8 @@ class Stream:
# Assert that the elements are consistent
_pipeline.assert_consistent(self._context, elements)
- if all(project.remote_execution_specs for project in self._context.get_projects()):
- # Remote execution is configured for all projects.
+ if self._context.remote_execution_specs:
+ # Remote execution is configured.
# Require artifact files only for target elements and their runtime dependencies.
self._context.set_artifact_files_optional()
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 92f9e88..503361e 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -2772,7 +2772,7 @@ class Element(Plugin):
else:
output_node_properties = None
- if directory is not None and allow_remote and project.remote_execution_specs:
+ if directory is not None and allow_remote and context.remote_execution_specs:
self.info("Using a remote sandbox for artifact {} with directory '{}'".format(self.name, directory))
@@ -2786,7 +2786,6 @@ class Element(Plugin):
stdout=stdout,
stderr=stderr,
config=config,
- specs=project.remote_execution_specs,
output_files_required=output_files_required,
output_node_properties=output_node_properties,
)
diff --git a/src/buildstream/sandbox/_sandboxremote.py b/src/buildstream/sandbox/_sandboxremote.py
index 6ffe56b..af50051 100644
--- a/src/buildstream/sandbox/_sandboxremote.py
+++ b/src/buildstream/sandbox/_sandboxremote.py
@@ -43,7 +43,8 @@ class SandboxRemote(SandboxREAPI):
self._output_files_required = kwargs.get("output_files_required", True)
- specs = kwargs["specs"] # This should be a RemoteExecutionSpec
+ context = self._get_context()
+ specs = context.remote_execution_specs
if specs is None:
return
diff --git a/tests/sandboxes/remote-exec-config.py b/tests/sandboxes/remote-exec-config.py
index 2f5a2cf..b840b51 100644
--- a/tests/sandboxes/remote-exec-config.py
+++ b/tests/sandboxes/remote-exec-config.py
@@ -5,7 +5,6 @@ import os
import pytest
-from buildstream import _yaml
from buildstream.exceptions import ErrorDomain, LoadErrorReason
from buildstream.testing.runcli import cli # pylint: disable=unused-import
@@ -20,35 +19,17 @@ DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "remote-exe
def test_missing_certs(cli, datafiles, config_key, config_value):
project = os.path.join(datafiles.dirname, datafiles.basename, "missing-certs")
- project_conf = {
- "name": "test",
- "min-version": "2.0",
- "remote-execution": {
- "execution-service": {"url": "http://localhost:8088"},
- "storage-service": {"url": "http://charactron:11001", config_key: config_value,},
- },
- }
- project_conf_file = os.path.join(project, "project.conf")
- _yaml.roundtrip_dump(project_conf, project_conf_file)
+ cli.configure(
+ {
+ "remote-execution": {
+ "execution-service": {"url": "http://localhost:8088"},
+ "storage-service": {"url": "http://charactron:11001", config_key: config_value,},
+ },
+ }
+ )
# Use `pull` here to ensure we try to initialize the remotes, triggering the error
#
# This does not happen for a simple `bst show`.
result = cli.run(project=project, args=["show", "element.bst"])
result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA, "Your config is missing")
-
-
-# Assert that if incomplete information is supplied we get a sensible error message.
-@pytest.mark.datafiles(DATA_DIR)
-def test_empty_config(cli, datafiles):
- project = os.path.join(datafiles.dirname, datafiles.basename, "missing-certs")
-
- project_conf = {"name": "test", "min-version": "2.0", "remote-execution": {}}
- project_conf_file = os.path.join(project, "project.conf")
- _yaml.roundtrip_dump(project_conf, project_conf_file)
-
- # Use `pull` here to ensure we try to initialize the remotes, triggering the error
- #
- # This does not happen for a simple `bst show`.
- result = cli.run(project=project, args=["artifact", "pull", "element.bst"])
- result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA, "specify one")
diff --git a/tests/sandboxes/remote-exec-config/missing-certs/project.conf b/tests/sandboxes/remote-exec-config/missing-certs/project.conf
new file mode 100644
index 0000000..20636c4
--- /dev/null
+++ b/tests/sandboxes/remote-exec-config/missing-certs/project.conf
@@ -0,0 +1,2 @@
+name: test
+min-version: 2.0