[Notes] [Git][BuildStream/buildstream][tpollard/566] WIP: Make uploading

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

Commits:

54b50486

by Tom Pollard at 2018-12-19T16:53:08Z

WIP: Make uploading of build trees configurable

5 changed files:

buildstream/_artifactcache/artifactcache.py
buildstream/_artifactcache/cascache.py
buildstream/element.py
+ tests/integration/pushbuildtrees.py
tests/testutils/runcli.py

Changes:

buildstream/_artifactcache/artifactcache.py

@@ -74,6 +74,7 @@ class ArtifactCache():
          self._has_fetch_remotes = False
          self._has_push_remotes = False
 +        self._has_partial_push_remotes = False
          os.makedirs(self.extractdir, exist_ok=True)
@@ -398,6 +399,8 @@ class ArtifactCache():
                  self._has_fetch_remotes = True
                  if remote_spec.push:
                      self._has_push_remotes = True
 +                    if remote_spec.partial_push:
 +                        self._has_partial_push_remotes = True
                  remotes[remote_spec.url] = CASRemote(remote_spec)
@@ -596,6 +599,31 @@ class ArtifactCache():
              remotes_for_project = self._remotes[element._get_project()]
              return any(remote.spec.push for remote in remotes_for_project)
 +    # has_partial_push_remotes():
 +    #
 +    # Check whether any remote repositories are available for pushing
 +    # non-complete artifacts
 +    #
 +    # Args:
 +    #     element (Element): The Element to check
 +    #
 +    # Returns:
 +    #   (bool): True if any remote repository is configured for optional
 +    #            partial pushes, False otherwise
 +    #
 +    def has_partial_push_remotes(self, *, element=None):
 +        # If there's no partial push remotes available, we can't partial push at all
 +        if not self._has_partial_push_remotes:
 +            return False
 +        elif element is None:
 +            # At least one remote is set to allow partial pushes
 +            return True
 +        else:
 +            # Check whether the specified element's project has push remotes configured
 +            # to not accept partial artifact pushes
 +            remotes_for_project = self._remotes[element._get_project()]
 +            return any(remote.spec.partial_push for remote in remotes_for_project)
++
      # push():
+     #
      # Push committed artifact to remote repository.
@@ -603,6 +631,8 @@ class ArtifactCache():
      # Args:
      #     element (Element): The Element whose artifact is to be pushed
      #     keys (list): The cache keys to use
 +    #     partial(bool): If the artifact is cached in a partial state
 +    #     subdir(string): Optional subdir to not push
+     #
      # Returns:
      #   (bool): True if any remote was updated, False if no pushes were required
@@ -610,12 +640,23 @@ class ArtifactCache():
      # Raises:
      #   (ArtifactError): if there was an error
+     #
 -    def push(self, element, keys):
 +    def push(self, element, keys, partial=False, subdir=None):
          refs = [self.get_artifact_fullname(element, key) for key in list(keys)]
          project = element._get_project()
 -        push_remotes = [r for r in self._remotes[project] if r.spec.push]
 +        push_remotes = []
 +        partial_remotes = []
++
 +        # Create list of remotes to push to, given current element and partial push config
 +        if not partial:
 +            push_remotes = [r for r in self._remotes[project] if (r.spec.push and not r.spec.partial_push)]
++
 +        if self._has_partial_push_remotes:
 +            # Create a specific list of the remotes expecting the artifact to be push in a partial
 +            # state. This list needs to be pushed in a partial state, without the optional subdir if
 +            # exists locally
 +            partial_remotes = [r for r in self._remotes[project] if (r.spec.partial_push and r.spec.push)]
          pushed = False
@@ -632,6 +673,19 @@ class ArtifactCache():
                      remote.spec.url, element._get_brief_display_key()
                  ))
 +        for remote in partial_remotes:
 +            remote.init()
 +            display_key = element._get_brief_display_key()
 +            element.status("Pushing partial artifact {} -> {}".format(display_key, remote.spec.url))
++
 +            if self.cas.push(refs, remote, subdir=subdir):
 +                element.info("Pushed partial artifact {} -> {}".format(display_key, remote.spec.url))
 +                pushed = True
 +            else:
 +                element.info("Remote ({}) already has {} partial cached".format(
 +                    remote.spec.url, element._get_brief_display_key()
 +                ))
++
          return pushed
      # pull():

buildstream/_artifactcache/cascache.py

@@ -45,7 +45,7 @@ from .. import _yaml
  _MAX_PAYLOAD_BYTES = 1024 * 1024
 -class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key client_cert')):
 +class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push partial_push server_cert client_key client_cert')):
      # _new_from_config_node
+     #
@@ -53,9 +53,12 @@ class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key
+     #
      @staticmethod
      def _new_from_config_node(spec_node, basedir=None):
 -        _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert'])
 +        _yaml.node_validate(spec_node,
 +                            ['url', 'push', 'allow-partial-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)
 +        partial_push = _yaml.node_get(spec_node, bool, 'allow-partial-push', default_value=False)
++
          if not url:
              provenance = _yaml.node_get_provenance(spec_node, 'url')
              raise LoadError(LoadErrorReason.INVALID_DATA,
@@ -83,10 +86,10 @@ class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key
              raise LoadError(LoadErrorReason.INVALID_DATA,
                              "{}: 'client-cert' was specified without 'client-key'".format(provenance))
 -        return CASRemoteSpec(url, push, server_cert, client_key, client_cert)
 +        return CASRemoteSpec(url, push, partial_push, server_cert, client_key, client_cert)
 -CASRemoteSpec.__new__.__defaults__ = (None, None, None)
 +CASRemoteSpec.__new__.__defaults__ = (False, None, None, None)
  class BlobNotFound(CASError):
@@ -308,6 +311,7 @@ class CASCache():
              else:
                  return False
          except BlobNotFound as e:
 +            raise ValueError("Got to here")
              return False
      # pull_tree():
@@ -353,6 +357,7 @@ class CASCache():
      # Args:
      #     refs (list): The refs to push
      #     remote (CASRemote): The remote to push to
 +    #     subdir (string): Optional specific subdir to exempt from the push
+     #
      # Returns:
      #   (bool): True if any remote was updated, False if no pushes were required
@@ -360,7 +365,7 @@ class CASCache():
      # Raises:
      #   (CASError): if there was an error
+     #
 -    def push(self, refs, remote):
 +    def push(self, refs, remote, subdir=None):
          skipped_remote = True
          try:
              for ref in refs:
@@ -382,7 +387,7 @@ class CASCache():
                          # Intentionally re-raise RpcError for outer except block.
                          raise
 -                self._send_directory(remote, tree)
 +                self._send_directory(remote, tree, excluded_dir=subdir)
                  request = buildstream_pb2.UpdateReferenceRequest()
                  request.keys.append(ref)
@@ -864,10 +869,17 @@ class CASCache():
                  a += 1
                  b += 1
 -    def _reachable_refs_dir(self, reachable, tree, update_mtime=False):
 +    def _reachable_refs_dir(self, reachable, tree, update_mtime=False, subdir=False):
          if tree.hash in reachable:
              return
 +        # If looping through subdir digests, skip processing if
 +        # ref path does not exist, allowing for partial objects
 +        if subdir and not os.path.exists(self.objpath(tree)):
 +            return
++
 +        # Raises FileNotFound exception is path does not exist,
 +        # which should only be entered on the top level digest
          if update_mtime:
              os.utime(self.objpath(tree))
@@ -884,9 +896,9 @@ class CASCache():
              reachable.add(filenode.digest.hash)
          for dirnode in directory.directories:
 -            self._reachable_refs_dir(reachable, dirnode.digest, update_mtime=update_mtime)
 +            self._reachable_refs_dir(reachable, dirnode.digest, update_mtime=update_mtime, subdir=True)
 -    def _required_blobs(self, directory_digest):
 +    def _required_blobs(self, directory_digest, excluded_dir=None):
          # parse directory, and recursively add blobs
          d = remote_execution_pb2.Digest()
          d.hash = directory_digest.hash
@@ -905,7 +917,8 @@ class CASCache():
              yield d
          for dirnode in directory.directories:
 -            yield from self._required_blobs(dirnode.digest)
 +            if dirnode.name != excluded_dir:
 +                yield from self._required_blobs(dirnode.digest)
      def _fetch_blob(self, remote, digest, stream):
          resource_name = '/'.join(['blobs', digest.hash, str(digest.size_bytes)])
@@ -1021,6 +1034,7 @@ class CASCache():
              objpath = self._ensure_blob(remote, dir_digest)
              directory = remote_execution_pb2.Directory()
++
              with open(objpath, 'rb') as f:
                  directory.ParseFromString(f.read())
@@ -1091,9 +1105,8 @@ class CASCache():
          assert response.committed_size == digest.size_bytes
 -    def _send_directory(self, remote, digest, u_uid=uuid.uuid4()):
 -        required_blobs = self._required_blobs(digest)
+-
 +    def _send_directory(self, remote, digest, u_uid=uuid.uuid4(), excluded_dir=None):
 +        required_blobs = self._required_blobs(digest, excluded_dir=excluded_dir)
          missing_blobs = dict()
          # Limit size of FindMissingBlobs request
          for required_blobs_group in _grouper(required_blobs, 512):

buildstream/element.py

@@ -1800,13 +1800,19 @@ class Element(Plugin):
      #   (bool): True if this element does not need a push job to be created
+     #
      def _skip_push(self):
++
          if not self.__artifacts.has_push_remotes(element=self):
              # No push remotes for this element's project
              return True
          # Do not push elements that aren't cached, or that are cached with a dangling buildtree
 -        # artifact unless element type is expected to have an an empty buildtree directory
 -        if not self._cached_buildtree():
 +        # artifact unless element type is expected to have an an empty buildtree directory. Check
 +        # that this default behaviour is not overriden via a remote configured to allow pushing
 +        # artifacts without their corresponding buildtree.
 +        if not self._cached():
 +            return True
++
 +        if not self._cached_buildtree() and not self.__artifacts.has_partial_push_remotes(element=self):
              return True
          # Do not push tainted artifact
@@ -1817,7 +1823,8 @@ class Element(Plugin):
      # _push():
+     #
 -    # Push locally cached artifact to remote artifact repository.
 +    # Push locally cached artifact to remote artifact repository. An attempt
 +    # will be made to push partial artifacts given current config
+     #
      # Returns:
      #   (bool): True if the remote was updated, False if it already existed
@@ -1830,8 +1837,19 @@ class Element(Plugin):
              self.warn("Not pushing tainted artifact.")
              return False
 -        # Push all keys used for local commit
 -        pushed = self.__artifacts.push(self, self.__get_cache_keys_for_commit())
 +        # Push all keys used for local commit, this could be full or partial,
 +        # given previous _skip_push() logic. If buildtree isn't cached, then
 +        # set partial push
++
 +        partial = False
 +        subdir = 'buildtree'
 +        if not self._cached_buildtree():
 +            partial = True
++
 +        pushed = self.__artifacts.push(self, self.__get_cache_keys_for_commit(), partial=partial, subdir=subdir)
++
 +        # Artifact might be cached in the server partially with the top level ref existing.
 +        # Check if we need to attempt a push of a locally cached buildtree given current config
          if not pushed:
              return False

tests/integration/pushbuildtrees.py

 +import os
 +import shutil
 +import pytest
++
 +from tests.testutils import cli_integration as cli, create_artifact_share
 +from tests.testutils.integration import assert_contains
 +from tests.testutils.site import HAVE_BWRAP, IS_LINUX
 +from buildstream._exceptions import ErrorDomain, LoadErrorReason
++
++
 +DATA_DIR = os.path.join(
 +    os.path.dirname(os.path.realpath(__file__)),
 +    "project"
 +)
++
++
 +# Remove artifact cache & set cli.config value of pull-buildtrees
 +# to false, which is the default user context. The cache has to be
 +# cleared as just forcefully removing the refpath leaves dangling objects.
 +def default_state(cli, tmpdir, share):
 +    shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
 +    cli.configure({
 +        'artifacts': {'url': share.repo, 'push': False},
 +        'artifactdir': os.path.join(str(tmpdir), 'artifacts'),
 +        'cache': {'pull-buildtrees': False},
 +    })
++
++
 +# A test to capture the integration of the optionl push of buildtrees.
 +# The behaviour should encompass pushing artifacts that are already cached
 +# without a buildtree as well as artifacts that are cached with their buildtree.
 +# This option is handled via 'allow-partial-push' on a per artifact remote config
 +# node basis. Multiple remote config nodes can point to the same url and as such can
 +# have different 'allow-partial-push' options, tests need to cover this
 +@pytest.mark.integration
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 +def test_pushbuildtrees(cli, tmpdir, datafiles, integration_cache):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'autotools/amhello.bst'
++
 +    # 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), 'share3')) as share3:
++
 +        cli.configure({
 +            'artifacts': {'url': share1.repo, 'push': True},
 +            'artifactdir': os.path.join(str(tmpdir), 'artifacts')
 +        })
++
 +        cli.configure({'artifacts': [{'url': share1.repo, 'push': True},
 +                                     {'url': share2.repo, 'push': True, 'allow-partial-push': True}]})
++
 +        # Build autotools element, checked pushed, delete local.
 +        # As share 2 has push & allow-partial-push set a true, it
 +        # should have pushed the artifacts, without the cached buildtrees,
 +        # to it.
 +        result = cli.run(project=project, args=['build', element_name])
 +        assert result.exit_code == 0
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        elementdigest = share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        buildtreedir = os.path.join(str(tmpdir), 'artifacts', 'extract', 'test', 'autotools-amhello',
 +                                    elementdigest.hash, 'buildtree')
 +        assert os.path.isdir(buildtreedir)
 +        assert element_name in result.get_pushed_elements()
 +        assert "Pushed partial artifact" in result.stderr
 +        assert share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        assert share2.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        default_state(cli, tmpdir, share1)
++
 +        # Check that after explictly pulling an artifact without it's buildtree,
 +        # we can push it to another remote that is configured to accept the partial
 +        # artifact
 +        result = cli.run(project=project, args=['pull', element_name])
 +        cli.configure({'artifacts': {'url': share3.repo, 'push': True, 'allow-partial-push': True}})
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        assert not os.path.isdir(buildtreedir)
 +        result = cli.run(project=project, args=['push', element_name])
 +        assert result.exit_code == 0
 +        assert "Pushed partial artifact" in result.stderr
 +        assert "Pushed artifact" not in result.stderr
 +        assert element_name in result.get_pushed_elements()
 +        assert share3.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        default_state(cli, tmpdir, share3)
++
 +        # Delete the local cache and pull the partial artifact from share 3,
 +        # this should not include the buildtree when extracted locally, even when
 +        # pull-buildtrees is given as a cli parameter
 +        assert not os.path.isdir(buildtreedir)
 +        assert cli.get_element_state(project, element_name) != 'cached'
 +        print("share3 repo value is {}".format(share3.repo))
 +        with open('/home/tom/buildstream/tmp/test_pushbuildtrees0/cache/buildstream.conf', 'r') as f:
 +            print(f.read())
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
 +        assert element_name in result.get_pulled_elements()
 +        assert not os.path.isdir(buildtreedir)

tests/testutils/runcli.py

@@ -171,7 +171,7 @@ class Result():
          return list(tracked)
      def get_pushed_elements(self):
 -        pushed = re.findall(r'\[\s*push:(\S+)\s*\]\s*INFO\s*Pushed artifact', self.stderr)
 +        pushed = re.findall(r'\[\s*push:(\S+)\s*\]\s*INFO\s*Pushed', self.stderr)
          if pushed is None:
              return []

[Notes] [Git][BuildStream/buildstream][tpollard/566] WIP: Make uploading of build trees configurable

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

Commits:

5 changed files:

Changes: