[Notes] [Git][BuildStream/buildstream][jmac/remote_execution_split] 4 commits: tests/sandboxes/remote-exec-config.py: New test.

[Jim MacArthur <gitlab mg gitlab com>]

Title: GitLab

Jim MacArthur pushed to branch jmac/remote_execution_split at BuildStream / buildstream

Commits:

• cdcfc0d2
  by Jim MacArthur at 2018-11-23T17:53:44Z

  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.
  

• 46aa0237
  by Jim MacArthur at 2018-11-23T17:53:44Z

  artifactcache.py: Remove unused verify_digest_pushed
  

• 20cfc39b
  by Jim MacArthur at 2018-11-23T17:53:44Z

  _sandboxremote.py: Remove print statements and correct some mistakes
  

• d980a5c3
  by Jim MacArthur at 2018-11-23T17:53:44Z

  _sandboxremote: pep8
  

6 changed files:

• buildstream/_artifactcache/artifactcache.py
• buildstream/sandbox/_sandboxremote.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

---

...	...	@@ -751,34 +751,6 @@ class ArtifactCache():
751	751
752	752	return message_digest
753	753
754		- # verify_digest_pushed():
755		- #
756		- # Check whether the object is already on the server in which case
757		- # there is no need to upload it.
758		- #
759		- # Args:
760		- # project (Project): The current project
761		- # digest (Digest): The object digest.
762		- #
763		- def verify_digest_pushed(self, project, digest):
764		-
765		- if self._has_push_remotes:
766		- push_remotes = [r for r in self._remotes[project] if r.spec.push]
767		- else:
768		- push_remotes = []
769		-
770		- if not push_remotes:
771		- raise ArtifactError("verify_digest_pushed was called, but no remote artifact " +
772		- "servers are configured as push remotes.")
773		-
774		- pushed = False
775		-
776		- for remote in push_remotes:
777		- if self.cas.verify_digest_on_remote(remote, digest):
778		- pushed = True
779		-
780		- return pushed
781		-
782	754	# link_key():
783	755	#
784	756	# Add a key for an existing artifact.

buildstream/sandbox/_sandboxremote.py

---

...	...	@@ -36,8 +36,6 @@ from .. import _yaml
36	36	from .._protos.google.longrunning import operations_pb2, operations_pb2_grpc
37	37	from .._artifactcache.cascache import CASRemote, CASRemoteSpec
38	38
39		-from .._exceptions import SandboxError
40		-
41	39
42	40	class RemoteExecutionSpec(namedtuple('RemoteExecutionSpec', 'exec_service storage_service')):
43	41	pass
...	...	@@ -64,6 +62,7 @@ class SandboxRemote(Sandbox):
64	62	server_cert=config.storage_service['server-cert'],
65	63	client_key=config.storage_service['client-key'],
66	64	client_cert=config.storage_service['client-cert'])
	65	+ self.operation_name = None
67	66
68	67	@staticmethod
69	68	def specs_from_config_node(config_node, basedir):
...	...	@@ -73,9 +72,9 @@ class SandboxRemote(Sandbox):
73	72	if val is None:
74	73	provenance = _yaml.node_get_provenance(remote_config, key=keyname)
75	74	raise _yaml.LoadError(_yaml.LoadErrorReason.INVALID_DATA,
76		- "'{}' was not present in the remote "
	75	+ "{}: '{}' was not present in the remote "
77	76	"execution configuration (remote-execution). "
78		- .format(keyname))
	77	+ .format(str(provenance), keyname))
79	78	return val
80	79
81	80	remote_config = config_node.get("remote-execution", None)
...	...	@@ -100,9 +99,10 @@ class SandboxRemote(Sandbox):
100	99	else:
101	100	provenance = _yaml.node_get_provenance(remote_config, key='url')
102	101	raise _yaml.LoadError(_yaml.LoadErrorReason.INVALID_DATA,
103		- "'url' and 'execution-service' keys were found in the remote "
	102	+ "{}: 'url' and 'execution-service' keys were found in the remote "
104	103	"execution configuration (remote-execution). "
105		- "You can only specify one of these.")
	104	+ "You can only specify one of these."
	105	+ .format(str(provenance)))
106	106
107	107	for key in tls_keys:
108	108	if key not in remote_exec_storage_config:
...	...	@@ -139,7 +139,6 @@ class SandboxRemote(Sandbox):
139	139	command_digest = cascache.push_message(casremote, remote_command)
140	140	if not command_digest or not cascache.verify_digest_on_remote(casremote, command_digest):
141	141	raise SandboxError("Failed pushing build command to remote CAS.")
142		- print("Created command digest: {}".format(command_digest.hash))
143	142	# Create and send the action.
144	143	action = remote_execution_pb2.Action(command_digest=command_digest,
145	144	input_root_digest=input_root_digest,
...	...	@@ -148,7 +147,6 @@ class SandboxRemote(Sandbox):
148	147
149	148	# Upload the Action message to the remote CAS server
150	149	action_digest = cascache.push_message(casremote, action)
151		- print("Created action digest: {}".format(action_digest.hash))
152	150	if not action_digest or not cascache.verify_digest_on_remote(casremote, action_digest):
153	151	raise SandboxError("Failed pushing build action to remote CAS.")
154	152
...	...	@@ -289,14 +287,14 @@ class SandboxRemote(Sandbox):
289	287	# Upload sources
290	288	upload_vdir = self.get_virtual_directory()
291	289
	290	+ cascache = self._get_context().get_cascache()
292	291	if isinstance(upload_vdir, FileBasedDirectory):
293	292	# Make a new temporary directory to put source in
294		- upload_vdir = CasBasedDirectory(self._get_context().artifactcache.cas, ref=None)
	293	+ upload_vdir = CasBasedDirectory(cascache, ref=None)
295	294	upload_vdir.import_files(self.get_virtual_directory()._get_underlying_directory())
296	295
297	296	upload_vdir.recalculate_hash()
298	297
299		- cascache = context.get_cascache()
300	298	casremote = CASRemote(self.storage_remote_spec)
301	299	# Now, push that key (without necessarily needing a ref) to the remote.
302	300

tests/sandboxes/remote-exec-config.py

---

	1	+import pytest
	2	+
	3	+import itertools
	4	+import os
	5	+
	6	+from buildstream import _yaml
	7	+from buildstream._exceptions import ErrorDomain, LoadErrorReason
	8	+
	9	+from tests.testutils.runcli import cli
	10	+
	11	+DATA_DIR = os.path.join(
	12	+ os.path.dirname(os.path.realpath(__file__)),
	13	+ "remote-exec-config"
	14	+)
	15	+
	16	+# Tests that we get a useful error message when supplying invalid
	17	+# remote execution configurations.
	18	+
	19	+
	20	+# Assert that if both 'url' (the old style) and 'execution-service' (the new style)
	21	+# are used at once, a LoadError results.
	22	+@pytest.mark.datafiles(DATA_DIR)
	23	+def test_old_and_new_configs(cli, datafiles):
	24	+ project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs')
	25	+
	26	+ project_conf = {
	27	+ 'name': 'test',
	28	+
	29	+ 'remote-execution': {
	30	+ 'url': 'https://cache.example.com:12345',
	31	+ 'execution-service': {
	32	+ 'url': 'http://localhost:8088'
	33	+ },
	34	+ 'storage-service': {
	35	+ 'url': 'http://charactron:11001',
	36	+ }
	37	+ }
	38	+ }
	39	+ project_conf_file = os.path.join(project, 'project.conf')
	40	+ _yaml.dump(project_conf, project_conf_file)
	41	+
	42	+ # Use `pull` here to ensure we try to initialize the remotes, triggering the error
	43	+ #
	44	+ # This does not happen for a simple `bst show`.
	45	+ result = cli.run(project=project, args=['pull', 'element.bst'])
	46	+ result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA, "specify one")
	47	+
	48	+
	49	+# Assert that if either the client key or client cert is specified
	50	+# without specifying its counterpart, we get a comprehensive LoadError
	51	+# instead of an unhandled exception.
	52	+@pytest.mark.datafiles(DATA_DIR)
	53	+@pytest.mark.parametrize('config_key, config_value', [
	54	+ ('client-cert', 'client.crt'),
	55	+ ('client-key', 'client.key')
	56	+])
	57	+def test_missing_certs(cli, datafiles, config_key, config_value):
	58	+ project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs')
	59	+
	60	+ project_conf = {
	61	+ 'name': 'test',
	62	+
	63	+ 'remote-execution': {
	64	+ 'execution-service': {
	65	+ 'url': 'http://localhost:8088'
	66	+ },
	67	+ 'storage-service': {
	68	+ 'url': 'http://charactron:11001',
	69	+ config_key: config_value,
	70	+ }
	71	+ }
	72	+ }
	73	+ project_conf_file = os.path.join(project, 'project.conf')
	74	+ _yaml.dump(project_conf, project_conf_file)
	75	+
	76	+ # Use `pull` here to ensure we try to initialize the remotes, triggering the error
	77	+ #
	78	+ # This does not happen for a simple `bst show`.
	79	+ result = cli.run(project=project, args=['show', 'element.bst'])
	80	+ result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA, "Your config is missing")
	81	+
	82	+
	83	+# Assert that if incomplete information is supplied we get a sensible error message.
	84	+@pytest.mark.datafiles(DATA_DIR)
	85	+def test_empty_config(cli, datafiles):
	86	+ project = os.path.join(datafiles.dirname, datafiles.basename, 'missing-certs')
	87	+
	88	+ project_conf = {
	89	+ 'name': 'test',
	90	+
	91	+ 'remote-execution': {
	92	+ }
	93	+ }
	94	+ project_conf_file = os.path.join(project, 'project.conf')
	95	+ _yaml.dump(project_conf, project_conf_file)
	96	+
	97	+ # Use `pull` here to ensure we try to initialize the remotes, triggering the error
	98	+ #
	99	+ # This does not happen for a simple `bst show`.
	100	+ result = cli.run(project=project, args=['pull', 'element.bst'])
	101	+ 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

---

1

+kind: autotools

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.