Skip to content

Commit b9aaae5

Browse files
author
Doug Greiman
committed
Merge branch 'master' into dgreiman/substitutions
2 parents 3500395 + e3f21c4 commit b9aaae5

4 files changed

Lines changed: 117 additions & 73 deletions

File tree

‎scripts/local_cloudbuild.py‎

Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,11 @@
7474
'TAG_NAME': '',
7575
}
7676

77+
# Use this image for cleanup actions
78+
DEBIAN_IMAGE='gcr.io/google-appengine/debian8'
79+
7780
# File template
78-
BUILD_SCRIPT_HEADER = """\
81+
BUILD_SCRIPT_TEMPLATE = """\
7982
#!/bin/bash
8083
# This is a generated file. Do not edit.
8184
@@ -84,24 +87,24 @@
8487
SOURCE_DIR=.
8588
8689
# Setup staging directory
87-
HOST_WORKSPACE=$(mktemp -d)
88-
function cleanup {
89-
if [ "${HOST_WORKSPACE}" != '/' -a -d "${HOST_WORKSPACE}" ]; then
90-
rm -rf "${HOST_WORKSPACE}"
90+
HOST_WORKSPACE=$(mktemp -d -t local_cloudbuild_XXXXXXXXXX)
91+
function cleanup {{
92+
if [ "${{HOST_WORKSPACE}}" != '/' -a -d "${{HOST_WORKSPACE}}" ]; then
93+
# Expect a single error message about /workspace busy
94+
{cleanup_str} 2>/dev/null || true
95+
# Do not expect error messages here. Display but ignore any that happen.
96+
rmdir "${{HOST_WORKSPACE}}" || true
9197
fi
92-
}
98+
}}
9399
trap cleanup EXIT
94100
95101
# Copy source to staging directory
96-
echo "Copying source to staging directory ${HOST_WORKSPACE}"
97-
rsync -avzq --exclude=.git "${SOURCE_DIR}" "${HOST_WORKSPACE}"
102+
echo "Copying source to staging directory ${{HOST_WORKSPACE}}"
103+
rsync -avzq --exclude=.git "${{SOURCE_DIR}}" "${{HOST_WORKSPACE}}"
98104
99105
# Build commands
100-
"""
101-
102-
BUILD_SCRIPT_FOOTER = """\
106+
{docker_str}
103107
# End of build commands
104-
105108
echo "Build completed successfully"
106109
"""
107110

@@ -296,17 +299,19 @@ def generate_script(cloudbuild):
296299
Returns:
297300
(str): Contents of shell script
298301
"""
299-
with io.StringIO() as outfile:
300-
outfile.write(BUILD_SCRIPT_HEADER)
301-
subs_used = set()
302-
docker_commands = [
303-
generate_command(step, cloudbuild.substitutions, subs_used)
304-
for step in cloudbuild.steps]
305-
for docker_command in docker_commands:
306-
line = ' '.join(docker_command) + '\n\n'
307-
outfile.write(line)
308-
outfile.write(BUILD_SCRIPT_FOOTER)
309-
s = outfile.getvalue()
302+
# This deletes everything in /workspace including hidden files,
303+
# but not /workspace itself
304+
cleanup_step = Step(
305+
args=['rm', '-rf', '/workspace'],
306+
dir_='',
307+
env=[],
308+
name=DEBIAN_IMAGE,
309+
)
310+
cleanup_command = generate_command(cleanup_step, {}, set())
311+
subs_used = set()
312+
docker_commands = [
313+
generate_command(step, cloudbuild.substitutions, subs_used)
314+
for step in cloudbuild.steps]
310315

311316
# Check that all user variables were referenced at least once
312317
user_subs_unused = [name for name in cloudbuild.substitutions.keys()
@@ -317,6 +322,15 @@ def generate_script(cloudbuild):
317322
'User substitution variables {} were defined in the --substitution '
318323
'flag but never used in the cloudbuild file.'.format(nice_list))
319324

325+
cleanup_str = ' '.join(cleanup_command)
326+
docker_lines = []
327+
for docker_command in docker_commands:
328+
line = ' '.join(docker_command) + '\n\n'
329+
docker_lines.append(line)
330+
docker_str = ''.join(docker_lines)
331+
332+
s = BUILD_SCRIPT_TEMPLATE.format(cleanup_str=cleanup_str,
333+
docker_str=docker_str)
320334
return s
321335

322336

‎scripts/local_cloudbuild_test.py‎

Lines changed: 71 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
import local_cloudbuild
3131

3232

33+
# Matches script boilerplate
34+
STAGING_DIR_REGEX = re.compile(
35+
b'(?m)Copying source to staging directory (.+)$')
36+
3337
class ValidationUtilsTest(unittest.TestCase):
3438

3539
def test_get_field_value(self):
@@ -163,6 +167,25 @@ def test_sub_and_quote(self):
163167
used = set()
164168
local_cloudbuild.sub_and_quote(s, subs, used)
165169

170+
def check_call_with_capture(self, *args, **kw_args):
171+
"""Act like subprocess.check_call but capture stdout"""
172+
try:
173+
self.check_call_output = subprocess.check_output(*args, **kw_args)
174+
print(self.check_call_output)
175+
except subprocess.CalledProcessError as e:
176+
self.check_call_output = e.output
177+
print(self.check_call_output)
178+
raise
179+
180+
def have_docker(self):
181+
"""Determine if the Docker daemon is present and usable"""
182+
if ((shutil.which('docker') is not None) and
183+
(subprocess.call(['docker', 'info'],
184+
stdout=subprocess.DEVNULL,
185+
stderr=subprocess.DEVNULL) == 0)):
186+
return True
187+
return False
188+
166189
def test_get_cloudbuild(self):
167190
args = argparse.Namespace(
168191
config='some_config_file',
@@ -339,7 +362,7 @@ def test_generate_script_unused_user_substitution(self):
339362
steps=[],
340363
substitutions={'_FOO':'_foo'},
341364
)
342-
with self.assertRaisesRegexp(ValueError, 'User substitution variables'):
365+
with self.assertRaisesRegex(ValueError, 'User substitution variables'):
343366
actual = local_cloudbuild.generate_script(cloudbuild)
344367

345368
def test_make_executable(self):
@@ -369,58 +392,59 @@ def test_write_script(self):
369392
self.assertEqual(actual, contents)
370393

371394
def test_local_cloudbuild(self):
372-
# Actually run it if we can find a docker command.
373-
should_run = False
374-
if ((shutil.which('docker') is not None) and
375-
(subprocess.call(['docker', 'info'],
376-
stdout=subprocess.DEVNULL,
377-
stderr=subprocess.DEVNULL) == 0)):
378-
should_run = True
395+
if not self.have_docker():
396+
self.fail('This test requires a working Docker daemon')
379397

380398
# Read cloudbuild.yaml from testdata file, write output to
381399
# tempdir, and maybe try to run it
382-
with tempfile.TemporaryDirectory(
383-
prefix='local_cloudbuild_test_') as tempdir:
384-
cases = (
385-
# Everything is ok
386-
('cloudbuild_ok.yaml', None, None),
387-
# Builtin substitutions like $PROJECT_ID work
388-
('cloudbuild_builtin_substitutions.yaml', None, None),
389-
# User substitutions like $_FOO work
390-
('cloudbuild_user_substitutions.yaml',
391-
{'_FOO':'this is foo value'},
392-
None
393-
),
394-
# User substitutions like $_FOO fails when undefined
395-
('cloudbuild_user_substitutions.yaml', None, ValueError),
396-
# Exit code 1 (failure)
397-
('cloudbuild_err_rc1.yaml', None, subprocess.CalledProcessError),
398-
# Command not found
399-
('cloudbuild_err_not_found.yaml', None, subprocess.CalledProcessError),
400+
cases = (
401+
# Everything is ok
402+
('cloudbuild_ok.yaml', None, None),
403+
# Builtin substitutions like $PROJECT_ID work
404+
('cloudbuild_builtin_substitutions.yaml', None, None),
405+
# User substitutions like $_FOO work
406+
('cloudbuild_user_substitutions.yaml',
407+
{'_FOO':'this is foo value'},
408+
None
409+
),
410+
# User substitutions like $_FOO fails when undefined
411+
('cloudbuild_user_substitutions.yaml', None, ValueError),
412+
# Exit code 1 (failure)
413+
('cloudbuild_err_rc1.yaml', None, subprocess.CalledProcessError),
414+
# Command not found
415+
('cloudbuild_err_not_found.yaml', None, subprocess.CalledProcessError),
416+
# Cleaning up files owned by root
417+
('cloudbuild_difficult_cleanup.yaml', None, None),
418+
)
419+
for case in cases:
420+
with self.subTest(case=case), \
421+
tempfile.TemporaryDirectory(prefix='local_cloudbuild_test_') as tempdir, \
422+
unittest.mock.patch('subprocess.check_call', self.check_call_with_capture):
423+
config_name, substitutions, exception = case
424+
if substitutions is None:
425+
substitutions = local_cloudbuild.DEFAULT_SUBSTITUTIONS
426+
should_succeed = (exception is None)
427+
config = os.path.join(self.testdata_dir, config_name)
428+
actual_output_script = os.path.join(
429+
tempdir, config_name + '_local.sh')
430+
args = argparse.Namespace(
431+
config=config,
432+
output_script=actual_output_script,
433+
run=True,
434+
substitutions=substitutions,
400435
)
401-
for case in cases:
402-
with self.subTest(case=case):
403-
config_name, substitutions, exception = case
404-
if substitutions is None:
405-
substitutions = local_cloudbuild.DEFAULT_SUBSTITUTIONS
406-
should_succeed = (exception is None)
407-
config = os.path.join(self.testdata_dir, config_name)
408-
actual_output_script = os.path.join(
409-
tempdir, config_name + '_local.sh')
410-
args = argparse.Namespace(
411-
config=config,
412-
output_script=actual_output_script,
413-
run=should_run,
414-
substitutions=substitutions,
415-
)
416-
if should_run:
417-
print("Executing docker commands in {}".format(actual_output_script))
418-
if should_succeed:
436+
437+
if should_succeed:
438+
local_cloudbuild.local_cloudbuild(args)
439+
else:
440+
with self.assertRaises(exception):
419441
local_cloudbuild.local_cloudbuild(args)
420-
else:
421-
with self.assertRaises(exception):
422-
local_cloudbuild.local_cloudbuild(args)
423442

443+
# Check that staging dir was cleaned up
444+
match = re.search(STAGING_DIR_REGEX, self.check_call_output)
445+
self.assertTrue(match)
446+
staging_dir = match.group(1)
447+
self.assertFalse(os.path.isdir(staging_dir), staging_dir)
424448

425449
def test_parse_args(self):
426450
# Test explicit output_script

‎scripts/testdata/cloudbuild_difficult_cleanup.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
steps:
2+
- name: debian
3+
args: ['/bin/sh', '-c', 'mkdir root; umask 0000; touch root/deny_all.txt']

‎scripts/testdata/cloudbuild_ok.yaml_golden.sh‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ set -euo pipefail
66
SOURCE_DIR=.
77

88
# Setup staging directory
9-
HOST_WORKSPACE=$(mktemp -d)
9+
HOST_WORKSPACE=$(mktemp -d -t local_cloudbuild_XXXXXXXXXX)
1010
function cleanup {
1111
if [ "${HOST_WORKSPACE}" != '/' -a -d "${HOST_WORKSPACE}" ]; then
12-
rm -rf "${HOST_WORKSPACE}"
12+
# Expect a single error message about /workspace busy
13+
docker run --volume /var/run/docker.sock:/var/run/docker.sock --volume /root/.docker:/root/.docker --volume ${HOST_WORKSPACE}:/workspace --workdir /workspace gcr.io/google-appengine/debian8 rm -rf /workspace 2>/dev/null || true
14+
# Do not expect error messages here. Display but ignore any that happen.
15+
rmdir "${HOST_WORKSPACE}" || true
1316
fi
1417
}
1518
trap cleanup EXIT
@@ -23,6 +26,6 @@ docker run --volume /var/run/docker.sock:/var/run/docker.sock --volume /root/.do
2326

2427
docker run --volume /var/run/docker.sock:/var/run/docker.sock --volume /root/.docker:/root/.docker --volume ${HOST_WORKSPACE}:/workspace --workdir /workspace --env 'MESSAGE=Goodbye\n And Farewell!' --env UNUSED=unused debian /bin/sh -c 'printenv MESSAGE'
2528

26-
# End of build commands
2729

30+
# End of build commands
2831
echo "Build completed successfully"

0 commit comments

Comments
 (0)