[Notes] [Git][BuildStream/buildstream][tpollard/774] 11 commits: artifac

Tom Pollard pushed to branch tpollard/774 at BuildStream / buildstream

Commits:

d5b5538d

by Jim MacArthur at 2018-11-27T15:32:19Z

artifactcache: Move ArtifactCacheSpec code into CASRemoteSpec.

There is nothing in ArtifactCacheSpec that's actually specific to
artifacts, so I've made it a CAS class so we can use the same
spec for remote execution.

f224d4cb

by Jim MacArthur at 2018-11-27T15:32:19Z

artifactcache.py, _context.py: Move CASCache object into context

Since the artifact cache and remote execution share the same
local CAS store, they should share the same CASCache object. Moving
this into context allows us to do this.

926c1446

by Jim MacArthur at 2018-11-27T15:33:34Z

_sandboxremote: Remove references to artifact cache.

Uses the new config options introduced earlier to specify remote
exec storage and execution service.

d7cdb03e

by Jim MacArthur at 2018-11-27T15:33:36Z

Documentation: Update to show new config options

95d8244f

by Jim MacArthur at 2018-11-27T15:33:36Z

tests/sandboxes/remote-exec-config.py: New test.

Tests that we get a sensible error message when we specify incomplete
or wrong information in the configuration for remote execution.

7f2f62eb

by Jim MacArthur at 2018-11-27T15:33:36Z

artifactcache.py: Remove unused verify_digest_pushed

c55ca3a5

by Jim MacArthur at 2018-11-27T15:33:36Z

Docs: Correct URLs in artifact cache example

The port number must come after the hostname and before the path.
The path is ignored by CASCache anyway, so including it is misleading.

2a8a3b19

by Jim MacArthur at 2018-11-27T18:17:03Z

Merge branch 'jmac/remote_execution_split' into 'master'

Split remote execution from artifact cache

Closes #750

See merge request BuildStream/buildstream!946

499c70fd

by Valentin David at 2018-11-28T09:11:21Z

setup.py: require ruamel.yaml >= 0.15.41 < 0.15.52

3513580c

by Valentin David at 2018-11-28T14:02:32Z

Merge branch 'valentindavid/ruamel_0_15_41' into 'master'

setup.py: require ruamel.yaml >= 0.15.41 < 0.15.52

See merge request BuildStream/buildstream!975

5d783289

by Tom Pollard at 2018-11-28T17:04:17Z

_stream.py: Ability to pull missing buildtrees outside of pull/build

Adds helper function _buildtree_required to determine if a
pullqueue should be constructed, for commands outside of bst pull
and build where it is determined that an element's buildtree
artifact is to be required given the respective semantics and
config. Utilised in push() to attempt to mitigate skipping the
push of partial elements without the user having to have preceded
it with an explicit pull.

tests/integration/pullbuildtrees.py also updated to cover this
use-case.

15 changed files:

buildstream/_artifactcache/artifactcache.py
buildstream/_artifactcache/cascache.py
buildstream/_context.py
buildstream/_project.py
buildstream/_stream.py
buildstream/data/projectconfig.yaml
buildstream/element.py
buildstream/sandbox/_sandboxremote.py
doc/source/format_project.rst
setup.py
tests/integration/pullbuildtrees.py
+ tests/sandboxes/remote-exec-config.py
+ tests/sandboxes/remote-exec-config/missing-certs/certificates/client.crt
+ tests/sandboxes/remote-exec-config/missing-certs/certificates/client.key
+ tests/sandboxes/remote-exec-config/missing-certs/element.bst

Changes:

buildstream/_artifactcache/artifactcache.py

@@ -21,7 +21,6 @@ import multiprocessing
  import os
  import signal
  import string
 -from collections import namedtuple
  from collections.abc import Mapping
  from ..types import _KeyStrength
@@ -31,7 +30,7 @@ from .. import _signals
  from .. import utils
  from .. import _yaml
 -from .cascache import CASCache, CASRemote
 +from .cascache import CASRemote, CASRemoteSpec
  CACHE_SIZE_FILE = "cache_size"
@@ -45,48 +44,8 @@ CACHE_SIZE_FILE = "cache_size"
  #     push (bool): Whether we should attempt to push artifacts to this cache,
  #                  in addition to pulling from it.
+ #
 -class ArtifactCacheSpec(namedtuple('ArtifactCacheSpec', 'url push server_cert client_key client_cert')):
+-
 -    # _new_from_config_node
 -    #
 -    # Creates an ArtifactCacheSpec() from a YAML loaded node
 -    #
 -    @staticmethod
 -    def _new_from_config_node(spec_node, basedir=None):
 -        _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert'])
 -        url = _yaml.node_get(spec_node, str, 'url')
 -        push = _yaml.node_get(spec_node, bool, 'push', default_value=False)
 -        if not url:
 -            provenance = _yaml.node_get_provenance(spec_node, 'url')
 -            raise LoadError(LoadErrorReason.INVALID_DATA,
 -                            "{}: empty artifact cache URL".format(provenance))
+-
 -        server_cert = _yaml.node_get(spec_node, str, 'server-cert', default_value=None)
 -        if server_cert and basedir:
 -            server_cert = os.path.join(basedir, server_cert)
+-
 -        client_key = _yaml.node_get(spec_node, str, 'client-key', default_value=None)
 -        if client_key and basedir:
 -            client_key = os.path.join(basedir, client_key)
+-
 -        client_cert = _yaml.node_get(spec_node, str, 'client-cert', default_value=None)
 -        if client_cert and basedir:
 -            client_cert = os.path.join(basedir, client_cert)
+-
 -        if client_key and not client_cert:
 -            provenance = _yaml.node_get_provenance(spec_node, 'client-key')
 -            raise LoadError(LoadErrorReason.INVALID_DATA,
 -                            "{}: 'client-key' was specified without 'client-cert'".format(provenance))
+-
 -        if client_cert and not client_key:
 -            provenance = _yaml.node_get_provenance(spec_node, 'client-cert')
 -            raise LoadError(LoadErrorReason.INVALID_DATA,
 -                            "{}: 'client-cert' was specified without 'client-key'".format(provenance))
+-
 -        return ArtifactCacheSpec(url, push, server_cert, client_key, client_cert)
+-
+-
 -ArtifactCacheSpec.__new__.__defaults__ = (None, None, None)
 +class ArtifactCacheSpec(CASRemoteSpec):
 +    pass
  # An ArtifactCache manages artifacts.
@@ -99,7 +58,7 @@ class ArtifactCache():
          self.context = context
          self.extractdir = os.path.join(context.artifactdir, 'extract')
 -        self.cas = CASCache(context.artifactdir)
 +        self.cas = context.get_cascache()
          self.global_remote_specs = []
          self.project_remote_specs = {}
@@ -792,34 +751,6 @@ class ArtifactCache():
          return message_digest
 -    # verify_digest_pushed():
 -    #
 -    # Check whether the object is already on the server in which case
 -    # there is no need to upload it.
 -    #
 -    # Args:
 -    #     project (Project): The current project
 -    #     digest (Digest): The object digest.
 -    #
 -    def verify_digest_pushed(self, project, digest):
+-
 -        if self._has_push_remotes:
 -            push_remotes = [r for r in self._remotes[project] if r.spec.push]
 -        else:
 -            push_remotes = []
+-
 -        if not push_remotes:
 -            raise ArtifactError("verify_digest_pushed was called, but no remote artifact " +
 -                                "servers are configured as push remotes.")
+-
 -        pushed = False
+-
 -        for remote in push_remotes:
 -            if self.cas.verify_digest_on_remote(remote, digest):
 -                pushed = True
+-
 -        return pushed
+-
      # link_key():
+     #
      # Add a key for an existing artifact.

buildstream/_artifactcache/cascache.py

@@ -17,6 +17,7 @@
  #  Authors:
  #        Jürg Billeter <juerg billeter codethink co uk>
 +from collections import namedtuple
  import hashlib
  import itertools
  import io
@@ -34,7 +35,8 @@ from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2, remo
  from .._protos.buildstream.v2 import buildstream_pb2, buildstream_pb2_grpc
  from .. import utils
 -from .._exceptions import CASError
 +from .._exceptions import CASError, LoadError, LoadErrorReason
 +from .. import _yaml
  # The default limit for gRPC messages is 4 MiB.
@@ -42,6 +44,50 @@ from .._exceptions import CASError
  _MAX_PAYLOAD_BYTES = 1024 * 1024
 +class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key client_cert')):
++
 +    # _new_from_config_node
 +    #
 +    # Creates an CASRemoteSpec() from a YAML loaded node
 +    #
 +    @staticmethod
 +    def _new_from_config_node(spec_node, basedir=None):
 +        _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert'])
 +        url = _yaml.node_get(spec_node, str, 'url')
 +        push = _yaml.node_get(spec_node, bool, 'push', default_value=False)
 +        if not url:
 +            provenance = _yaml.node_get_provenance(spec_node, 'url')
 +            raise LoadError(LoadErrorReason.INVALID_DATA,
 +                            "{}: empty artifact cache URL".format(provenance))
++
 +        server_cert = _yaml.node_get(spec_node, str, 'server-cert', default_value=None)
 +        if server_cert and basedir:
 +            server_cert = os.path.join(basedir, server_cert)
++
 +        client_key = _yaml.node_get(spec_node, str, 'client-key', default_value=None)
 +        if client_key and basedir:
 +            client_key = os.path.join(basedir, client_key)
++
 +        client_cert = _yaml.node_get(spec_node, str, 'client-cert', default_value=None)
 +        if client_cert and basedir:
 +            client_cert = os.path.join(basedir, client_cert)
++
 +        if client_key and not client_cert:
 +            provenance = _yaml.node_get_provenance(spec_node, 'client-key')
 +            raise LoadError(LoadErrorReason.INVALID_DATA,
 +                            "{}: 'client-key' was specified without 'client-cert'".format(provenance))
++
 +        if client_cert and not client_key:
 +            provenance = _yaml.node_get_provenance(spec_node, 'client-cert')
 +            raise LoadError(LoadErrorReason.INVALID_DATA,
 +                            "{}: 'client-cert' was specified without 'client-key'".format(provenance))
++
 +        return CASRemoteSpec(url, push, server_cert, client_key, client_cert)
++
++
 +CASRemoteSpec.__new__.__defaults__ = (None, None, None)
++
++
  # A CASCache manages a CAS repository as specified in the Remote Execution API.
+ #
  # Args:

buildstream/_context.py

@@ -31,6 +31,7 @@ from ._exceptions import LoadError, LoadErrorReason, BstError
  from ._message import Message, MessageType
  from ._profile import Topics, profile_start, profile_end
  from ._artifactcache import ArtifactCache
 +from ._artifactcache.cascache import CASCache
  from ._workspaces import Workspaces
  from .plugin import _plugin_lookup
@@ -141,6 +142,7 @@ class Context():
          self._workspaces = None
          self._log_handle = None
          self._log_filename = None
 +        self._cascache = None
      # load()
+     #
@@ -620,6 +622,11 @@ class Context():
          if not os.environ.get('XDG_DATA_HOME'):
              os.environ['XDG_DATA_HOME'] = os.path.expanduser('~/.local/share')
 +    def get_cascache(self):
 +        if self._cascache is None:
 +            self._cascache = CASCache(self.artifactdir)
 +        return self._cascache
++
  # _node_get_option_str()
+ #

buildstream/_project.py

@@ -30,6 +30,7 @@ from ._profile import Topics, profile_start, profile_end
  from ._exceptions import LoadError, LoadErrorReason
  from ._options import OptionPool
  from ._artifactcache import ArtifactCache
 +from .sandbox import SandboxRemote
  from ._elementfactory import ElementFactory
  from ._sourcefactory import SourceFactory
  from .plugin import CoreWarnings
@@ -130,7 +131,7 @@ class Project():
          self._shell_host_files = []   # A list of HostMount objects
          self.artifact_cache_specs = None
 -        self.remote_execution_url = None
 +        self.remote_execution_specs = None
          self._sandbox = None
          self._splits = None
@@ -493,9 +494,7 @@ class Project():
          self.artifact_cache_specs = ArtifactCache.specs_from_config_node(config, self.directory)
          # Load remote-execution configuration for this project
 -        remote_execution = _yaml.node_get(config, Mapping, 'remote-execution')
 -        _yaml.node_validate(remote_execution, ['url'])
 -        self.remote_execution_url = _yaml.node_get(remote_execution, str, 'url')
 +        self.remote_execution_specs = SandboxRemote.specs_from_config_node(config, self.directory)
          # Load sandbox environment variables
          self.base_environment = _yaml.node_get(config, Mapping, 'environment')

buildstream/_stream.py

@@ -327,6 +327,10 @@ class Stream():
      # If `remote` specified as None, then regular configuration will be used
      # to determine where to push artifacts to.
+     #
 +    # If any of the given targets are missing their expected buildtree artifact,
 +    # a pull queue will be created if user context and availavble remotes allow for
 +    # attempting to fetch them.
 +    #
      def push(self, targets, *,
               selection=PipelineSelection.NONE,
               remote=None):
@@ -345,8 +349,17 @@ class Stream():
              raise StreamError("No artifact caches available for pushing artifacts")
          self._pipeline.assert_consistent(elements)
 -        self._add_queue(PushQueue(self._scheduler))
 -        self._enqueue_plan(elements)
++
 +        # Check if we require a pull queue, with given artifact state and context
 +        require_buildtrees = self._buildtree_required(elements)
 +        if require_buildtrees:
 +            self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtrees")
 +            self._add_queue(PullQueue(self._scheduler))
 +            self._enqueue_plan(require_buildtrees)
++
 +        push_queue = PushQueue(self._scheduler)
 +        self._add_queue(push_queue)
 +        self._enqueue_plan(elements, queue=push_queue)
          self._run()
      # checkout()
@@ -1237,3 +1250,31 @@ class Stream():
              parts.append(element.normal_name)
          return os.path.join(directory, *reversed(parts))
++
 +    # _buildtree_required()
 +    #
 +    # Check if current task, given config, requires element buildtree artifact
 +    #
 +    # Args:
 +    #    elements (list): elements to check if buildtrees are required
 +    #
 +    # Returns:
 +    #    required _list (list): elements requiring buildtrees
 +    #
 +    def _buildtree_required(self, elements):
 +        required_list = []
++
 +        # If context is set to not pull buildtree or no fetch remotes, return empty list
 +        if not (self._context.pull_buildtrees or self._artifacts.has_fetch_remotes()):
 +            return required_list
++
 +        for element in elements:
 +            if element._cached():
 +                if self._context.get_strict():
 +                    if not self._artifacts.contains_subdir_artifact(element, element._get_cache_key(), 'buildtree'):
 +                        required_list.append(element)
 +                elif not self._artifacts.contains_subdir_artifact(element, element._get_cache_key(strength=None),
 +                                                                  'buildtree'):
 +                    required_list.append(element)
++
 +        return required_list

buildstream/data/projectconfig.yaml

@@ -196,7 +196,4 @@ shell:
    # Command to run when `bst shell` does not provide a command
+   #
 -  command: [ 'sh', '-i' ]
+-
 -remote-execution:
 -  url: ""
 \ No newline at end of file
 +  command: [ 'sh', '-i' ]
 \ No newline at end of file

buildstream/element.py

@@ -255,9 +255,9 @@ class Element(Plugin):
          # Extract remote execution URL
          if not self.__is_junction:
 -            self.__remote_execution_url = project.remote_execution_url
 +            self.__remote_execution_specs = project.remote_execution_specs
          else:
 -            self.__remote_execution_url = None
 +            self.__remote_execution_specs = None
          # Extract Sandbox config
          self.__sandbox_config = self.__extract_sandbox_config(meta)
@@ -2171,7 +2171,7 @@ class Element(Plugin):
      # supports it.
+     #
      def __use_remote_execution(self):
 -        return self.__remote_execution_url and self.BST_VIRTUAL_DIRECTORY
 +        return self.__remote_execution_specs and self.BST_VIRTUAL_DIRECTORY
      # __sandbox():
+     #
@@ -2207,13 +2207,13 @@ class Element(Plugin):
                                      stdout=stdout,
                                      stderr=stderr,
                                      config=config,
 -                                    server_url=self.__remote_execution_url,
 +                                    specs=self.__remote_execution_specs,
                                      bare_directory=bare_directory,
                                      allow_real_directory=False)
              yield sandbox
          elif directory is not None and os.path.exists(directory):
 -            if allow_remote and self.__remote_execution_url:
 +            if allow_remote and self.__remote_execution_specs:
                  self.warn("Artifact {} is configured to use remote execution but element plugin does not support it."
                            .format(self.name), detail="Element plugin '{kind}' does not support virtual directories."
                            .format(kind=self.get_kind()), warning_token="remote-failure")

buildstream/sandbox/_sandboxremote.py

@@ -20,6 +20,7 @@
  import os
  import shlex
 +from collections import namedtuple
  from urllib.parse import urlparse
  from functools import partial
@@ -33,7 +34,13 @@ from .. import _signals
  from .._protos.build.bazel.remote.execution.v2 import remote_execution_pb2, remote_execution_pb2_grpc
  from .._protos.google.rpc import code_pb2
  from .._exceptions import SandboxError
 +from .. import _yaml
  from .._protos.google.longrunning import operations_pb2, operations_pb2_grpc
 +from .._artifactcache.cascache import CASRemote, CASRemoteSpec
++
++
 +class RemoteExecutionSpec(namedtuple('RemoteExecutionSpec', 'exec_service storage_service')):
 +    pass
  # SandboxRemote()
@@ -46,18 +53,70 @@ class SandboxRemote(Sandbox):
      def __init__(self, *args, **kwargs):
          super().__init__(*args, **kwargs)
 -        url = urlparse(kwargs['server_url'])
 -        if not url.scheme or not url.hostname or not url.port:
 -            raise SandboxError("Configured remote URL '{}' does not match the expected layout. "
 -                               .format(kwargs['server_url']) +
 -                               "It should be of the form <protocol>://<domain name>:<port>.")
 -        elif url.scheme != 'http':
 -            raise SandboxError("Configured remote '{}' uses an unsupported protocol. "
 -                               "Only plain HTTP is currenlty supported (no HTTPS).")
 +        config = kwargs['specs']  # This should be a RemoteExecutionSpec
 +        if config is None:
 +            return
++
 +        self.storage_url = config.storage_service['url']
 +        self.exec_url = config.exec_service['url']
 -        self.server_url = '{}:{}'.format(url.hostname, url.port)
 +        self.storage_remote_spec = CASRemoteSpec(self.storage_url, push=True,
 +                                                 server_cert=config.storage_service['server-cert'],
 +                                                 client_key=config.storage_service['client-key'],
 +                                                 client_cert=config.storage_service['client-cert'])
          self.operation_name = None
 +    @staticmethod
 +    def specs_from_config_node(config_node, basedir):
++
 +        def require_node(config, keyname):
 +            val = config.get(keyname)
 +            if val is None:
 +                provenance = _yaml.node_get_provenance(remote_config, key=keyname)
 +                raise _yaml.LoadError(_yaml.LoadErrorReason.INVALID_DATA,
 +                                      "{}: '{}' was not present in the remote "
 +                                      "execution configuration (remote-execution). "
 +                                      .format(str(provenance), keyname))
 +            return val
++
 +        remote_config = config_node.get("remote-execution", None)
 +        if remote_config is None:
 +            return None
++
 +        # Maintain some backwards compatibility with older configs, in which 'url' was the only valid key for
 +        # remote-execution.
++
 +        tls_keys = ['client-key', 'client-cert', 'server-cert']
++
 +        _yaml.node_validate(remote_config, ['execution-service', 'storage-service', 'url'])
 +        remote_exec_service_config = require_node(remote_config, 'execution-service')
 +        remote_exec_storage_config = require_node(remote_config, 'storage-service')
++
 +        _yaml.node_validate(remote_exec_service_config, ['url'])
 +        _yaml.node_validate(remote_exec_storage_config, ['url'] + tls_keys)
++
 +        if 'url' in remote_config:
 +            if 'execution-service' not in remote_config:
 +                remote_config['execution-service'] = {'url': remote_config['url']}
 +            else:
 +                provenance = _yaml.node_get_provenance(remote_config, key='url')
 +                raise _yaml.LoadError(_yaml.LoadErrorReason.INVALID_DATA,
 +                                      "{}: 'url' and 'execution-service' keys were found in the remote "
 +                                      "execution configuration (remote-execution). "
 +                                      "You can only specify one of these."
 +                                      .format(str(provenance)))
++
 +        for key in tls_keys:
 +            if key not in remote_exec_storage_config:
 +                provenance = _yaml.node_get_provenance(remote_config, key='storage-service')
 +                raise _yaml.LoadError(_yaml.LoadErrorReason.INVALID_DATA,
 +                                      "{}: The keys {} are necessary for the storage-service section of "
 +                                      "remote-execution configuration. Your config is missing '{}'."
 +                                      .format(str(provenance), tls_keys, key))
++
 +        spec = RemoteExecutionSpec(remote_config['execution-service'], remote_config['storage-service'])
 +        return spec
++
      def run_remote_command(self, command, input_root_digest, working_directory, environment):
          # Sends an execution request to the remote execution server.
+         #
@@ -75,12 +134,13 @@ class SandboxRemote(Sandbox):
                                                        output_directories=[self._output_directory],
                                                        platform=None)
          context = self._get_context()
 -        cascache = context.artifactcache
 +        cascache = context.get_cascache()
 +        casremote = CASRemote(self.storage_remote_spec)
++
          # Upload the Command message to the remote CAS server
 -        command_digest = cascache.push_message(self._get_project(), remote_command)
 -        if not command_digest or not cascache.verify_digest_pushed(self._get_project(), command_digest):
 +        command_digest = cascache.push_message(casremote, remote_command)
 +        if not command_digest or not cascache.verify_digest_on_remote(casremote, command_digest):
              raise SandboxError("Failed pushing build command to remote CAS.")
+-
          # Create and send the action.
          action = remote_execution_pb2.Action(command_digest=command_digest,
                                               input_root_digest=input_root_digest,
@@ -88,12 +148,21 @@ class SandboxRemote(Sandbox):
                                               do_not_cache=False)
          # Upload the Action message to the remote CAS server
 -        action_digest = cascache.push_message(self._get_project(), action)
 -        if not action_digest or not cascache.verify_digest_pushed(self._get_project(), action_digest):
 +        action_digest = cascache.push_message(casremote, action)
 +        if not action_digest or not cascache.verify_digest_on_remote(casremote, action_digest):
              raise SandboxError("Failed pushing build action to remote CAS.")
          # Next, try to create a communication channel to the BuildGrid server.
 -        channel = grpc.insecure_channel(self.server_url)
 +        url = urlparse(self.exec_url)
 +        if not url.port:
 +            raise SandboxError("You must supply a protocol and port number in the execution-service url, "
 +                               "for example: http://buildservice:50051.")
 +        if url.scheme == 'http':
 +            channel = grpc.insecure_channel('{}:{}'.format(url.hostname, url.port))
 +        else:
 +            raise SandboxError("Remote execution currently only supports the 'http' protocol "
 +                               "and '{}' was supplied.".format(url.scheme))
++
          stub = remote_execution_pb2_grpc.ExecutionStub(channel)
          request = remote_execution_pb2.ExecuteRequest(action_digest=action_digest,
                                                        skip_cache_lookup=False)
@@ -119,7 +188,7 @@ class SandboxRemote(Sandbox):
                  status_code = e.code()
                  if status_code == grpc.StatusCode.UNAVAILABLE:
                      raise SandboxError("Failed contacting remote execution server at {}."
 -                                       .format(self.server_url))
 +                                       .format(self.exec_url))
                  elif status_code in (grpc.StatusCode.INVALID_ARGUMENT,
                                       grpc.StatusCode.FAILED_PRECONDITION,
@@ -190,9 +259,11 @@ class SandboxRemote(Sandbox):
              raise SandboxError("Output directory structure had no digest attached.")
          context = self._get_context()
 -        cascache = context.artifactcache
 +        cascache = context.get_cascache()
 +        casremote = CASRemote(self.storage_remote_spec)
++
          # Now do a pull to ensure we have the necessary parts.
 -        dir_digest = cascache.pull_tree(self._get_project(), tree_digest)
 +        dir_digest = cascache.pull_tree(casremote, tree_digest)
          if dir_digest is None or not dir_digest.hash or not dir_digest.size_bytes:
              raise SandboxError("Output directory structure pulling from remote failed.")
@@ -218,18 +289,23 @@ class SandboxRemote(Sandbox):
          # Upload sources
          upload_vdir = self.get_virtual_directory()
 +        cascache = self._get_context().get_cascache()
          if isinstance(upload_vdir, FileBasedDirectory):
              # Make a new temporary directory to put source in
 -            upload_vdir = CasBasedDirectory(self._get_context().artifactcache.cas, ref=None)
 +            upload_vdir = CasBasedDirectory(cascache, ref=None)
              upload_vdir.import_files(self.get_virtual_directory()._get_underlying_directory())
          upload_vdir.recalculate_hash()
 -        context = self._get_context()
 -        cascache = context.artifactcache
 +        casremote = CASRemote(self.storage_remote_spec)
          # Now, push that key (without necessarily needing a ref) to the remote.
 -        cascache.push_directory(self._get_project(), upload_vdir)
 -        if not cascache.verify_digest_pushed(self._get_project(), upload_vdir.ref):
++
 +        try:
 +            cascache.push_directory(casremote, upload_vdir)
 +        except grpc.RpcError as e:
 +            raise SandboxError("Failed to push source directory to remote: {}".format(e)) from e
++
 +        if not cascache.verify_digest_on_remote(casremote, upload_vdir.ref):
              raise SandboxError("Failed to verify that source has been pushed to the remote artifact cache.")
          # Now transmit the command to execute

doc/source/format_project.rst

@@ -201,10 +201,10 @@ with an artifact share.
+   #
    artifacts:
      # A remote cache from which to download prebuilt artifacts
 -    - url: https://foo.com/artifacts:11001
 +    - url: https://foo.com:11001
        server.cert: server.crt
      # A remote cache from which to upload/download built/prebuilt artifacts
 -    - url: https://foo.com/artifacts:11002
 +    - url: https://foo.com:11002
        server-cert: server.crt
        client-cert: client.crt
        client-key: client.key
@@ -231,10 +231,24 @@ using the `remote-execution` option:
    remote-execution:
      # A url defining a remote execution server
 -    url: http://buildserver.example.com:50051
 +    execution-service:
 +      url: http://buildserver.example.com:50051
 +    storage-service:
 +    - url: https://foo.com:11002/
 +      server-cert: server.crt
 +      client-cert: client.crt
 +      client-key: client.key
++
 +The execution-service part of remote execution does not support encrypted
 +connections yet, so the protocol must always be http.
++
 +storage-service specifies a remote CAS store and the parameters are the
 +same as those used to specify an :ref:`artifact server <artifacts>`.
 -The url should contain a hostname and port separated by ':'. Only plain HTTP is
 -currently suported (no HTTPS).
 +The storage service may be the same endpoint used for artifact
 +caching. Remote execution cannot work without push access to the
 +storage endpoint, so you must specify a client certificate and key,
 +and a server certificate.
  The Remote Execution API can be found via https://github.com/bazelbuild/remote-apis.

setup.py

@@ -337,7 +337,14 @@ setup(name='BuildStream',
        install_requires=[
            'setuptools',
            'psutil',
 -          'ruamel.yaml < 0.15.52',
 +          # According to ruamel.yaml's PyPI page, we are suppose to use
 +          # "<=0.15" in production until 0.15 becomes API stable.
 +          # However we need ruamel.yaml 0.15.41 or greater for Python 3.7.
 +          # We know that ruamel.yaml 0.15.52 breaks API in a way that
 +          # is incompatible with BuildStream.
 +          #
 +          # See issues #571 and #790.
 +          'ruamel.yaml >= 0.15.41, < 0.15.52',
            'pluginbase',
            'Click',
            'jinja2 >= 2.10',

tests/integration/pullbuildtrees.py

@@ -38,7 +38,8 @@ def test_pullbuildtrees(cli, tmpdir, datafiles, integration_cache):
      # Create artifact shares for pull & push testing
      with create_artifact_share(os.path.join(str(tmpdir), 'share1')) as share1,\
 -        create_artifact_share(os.path.join(str(tmpdir), 'share2')) as share2:
 +        create_artifact_share(os.path.join(str(tmpdir), 'share2')) as share2,\
 +        create_artifact_share(os.path.join(str(tmpdir), 'share3')) as share3:
          cli.configure({
              'artifacts': {'url': share1.repo, 'push': True},
              'artifactdir': os.path.join(str(tmpdir), 'artifacts')
@@ -123,6 +124,32 @@ def test_pullbuildtrees(cli, tmpdir, datafiles, integration_cache):
          assert share2.has_artifact('test', element_name, cli.get_element_key(project, element_name))
          default_state(cli, tmpdir, share1)
 +        # Assert that bst push will automatically attempt to pull a missing buildtree
 +        # if pull-buildtrees is set, however as share3 is the only defined remote and is empty,
 +        # assert that no element artifact buildtrees are pulled (no available remote buildtree) and thus the
 +        # artifact cannot be pushed.
 +        result = cli.run(project=project, args=['pull', element_name])
 +        assert element_name in result.get_pulled_elements()
 +        cli.configure({'artifacts': {'url': share3.repo, 'push': True}})
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'push', element_name])
 +        assert "Attempting to fetch missing artifact buildtrees" in result.stderr
 +        assert element_name not in result.get_pulled_elements()
 +        assert not os.path.isdir(buildtreedir)
 +        assert element_name not in result.get_pushed_elements()
 +        assert not share3.has_artifact('test', element_name, cli.get_element_key(project, element_name))
++
 +        # Assert that if we add an extra remote that has the buildtree artfact cached, bst push will
 +        # automatically attempt to pull it and will be successful, leading to the full artifact being pushed
 +        # to the empty share3. This gives the ability to attempt push currently partial artifacts to a remote,
 +        # without exlipictly requiring a bst pull.
 +        cli.configure({'artifacts': [{'url': share1.repo, 'push': False}, {'url': share3.repo, 'push': True}]})
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'push', element_name])
 +        assert "Attempting to fetch missing artifact buildtrees" in result.stderr
 +        assert element_name in result.get_pulled_elements()
 +        assert os.path.isdir(buildtreedir)
 +        assert element_name in result.get_pushed_elements()
 +        assert share3.has_artifact('test', element_name, cli.get_element_key(project, element_name))
++
  # Ensure that only valid pull-buildtrees boolean options make it through the loading
  # process.

tests/sandboxes/remote-exec-config.py

 +import pytest
++
 +import itertools
 +import os
++
 +from buildstream import _yaml
 +from buildstream._exceptions import ErrorDomain, LoadErrorReason
++
 +from tests.testutils.runcli import cli
++
 +DATA_DIR = os.path.join(
 +    os.path.dirname(os.path.realpath(__file__)),
 +    "remote-exec-config"
 +)
++
 +# Tests that we get a useful error message when supplying invalid
 +# remote execution configurations.
++
++
 +# Assert that if both 'url' (the old style) and 'execution-service' (the new style)
 +# are used at once, a LoadError results.
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_old_and_new_configs(cli, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs')
++
 +    project_conf = {
 +        'name': 'test',
++
 +        'remote-execution': {
 +            'url': 'https://cache.example.com:12345',
 +            'execution-service': {
 +                'url': 'http://localhost:8088'
 +            },
 +            'storage-service': {
 +                'url': 'http://charactron:11001',
 +            }
 +        }
 +    }
 +    project_conf_file = os.path.join(project, 'project.conf')
 +    _yaml.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=['pull', 'element.bst'])
 +    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA, "specify one")
++
++
 +# Assert that if either the client key or client cert is specified
 +# without specifying its counterpart, we get a comprehensive LoadError
 +# instead of an unhandled exception.
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.parametrize('config_key, config_value', [
 +    ('client-cert', 'client.crt'),
 +    ('client-key', 'client.key')
 +])
 +def test_missing_certs(cli, datafiles, config_key, config_value):
 +    project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs')
++
 +    project_conf = {
 +        'name': 'test',
++
 +        '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.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=['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',
++
 +        'remote-execution': {
 +        }
 +    }
 +    project_conf_file = os.path.join(project, 'project.conf')
 +    _yaml.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=['pull', 'element.bst'])
 +    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA, "specify one")

tests/sandboxes/remote-exec-config/missing-certs/certificates/client.crt

tests/sandboxes/remote-exec-config/missing-certs/certificates/client.key

tests/sandboxes/remote-exec-config/missing-certs/element.bst

+kind: autotools

[Notes] [Git][BuildStream/buildstream][tpollard/774] 11 commits: artifactcache: Move ArtifactCacheSpec code into CASRemoteSpec.

Tom Pollard pushed to branch tpollard/774 at BuildStream / buildstream

Commits:

15 changed files:

Changes: