openstack
/
paunch
Code Issues Proposed changes

Explicitly pull images before docker run 

By pulling any missing required images, a pull failure will result in
an early failure with a clear cause. For detached containers, this is
improved error handling than having a container which fails
to start.

Change-Id: Ifa0257cbeadd3da9be756edf6a729c90141c238f
Closes-Bug: #1733941
This commit is contained in:
Steve Baker 2017-11-24 10:38:22 +13:00
parent b9cb9a7129
commit 9b2ae8bd96
5 changed files with 138 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
paunch/builder/compose1.py
View File

@ -27,10 +27,14 @@ class ComposeV1Builder(object):
def apply(self):
self.delete_missing_and_updated()
stdout = []
stderr = []
pull_returncode = self.pull_missing_images(stdout, stderr)
if pull_returncode != 0:
return stdout, stderr, pull_returncode
self.delete_missing_and_updated()
deploy_status_code = 0
key_fltr = lambda k: self.config[k].get('start_order', 0)
@ -195,6 +199,40 @@ class ComposeV1Builder(object):
command[0], self.config_id)
cmd.extend(command)
def pull_missing_images(self, stdout, stderr):
images = set()
for container in self.config:
cconfig = self.config[container]
image = cconfig.get('image')
if image:
images.add(image)
returncode = 0
for image in sorted(images):
# only pull if the image does not exist locally
if self.runner.inspect(image, format='exists', type='image'):
continue
cmd = [self.runner.docker_cmd, 'pull', image]
(cmd_stdout, cmd_stderr, rc) = self.runner.execute(cmd)
if cmd_stdout:
stdout.append(cmd_stdout)
if cmd_stderr:
stderr.append(cmd_stderr)
if rc != 0:
returncode = rc
LOG.error("Error running %s. [%s]\n" % (cmd, returncode))
LOG.error("stdout: %s" % cmd_stdout)
LOG.error("stderr: %s" % cmd_stderr)
else:
LOG.debug('Completed $ %s' % ' '.join(cmd))
LOG.info("stdout: %s" % cmd_stdout)
LOG.info("stderr: %s" % cmd_stderr)
return returncode
@staticmethod
def command_argument(command):
if not command:

6
paunch/runner.py
View File

@ -132,12 +132,12 @@ class DockerRunner(object):
LOG.error('Error renaming container: %s' % container)
LOG.error(cmd_stderr)
def inspect(self, container, format=None):
cmd = [self.docker_cmd, 'inspect']
def inspect(self, name, format=None, type='container'):
cmd = [self.docker_cmd, 'inspect', '--type', type]
if format:
cmd.append('--format')
cmd.append(format)
cmd.append(container)
cmd.append(name)
(cmd_stdout, cmd_stderr, returncode) = self.execute(cmd)
if returncode != 0:
return

92
paunch/tests/test_builder_compose1.py
View File

@ -35,7 +35,7 @@ class TestComposeV1Builder(base.TestCase):
},
'three': {
'start_order': 2,
'image': 'centos:7',
'image': 'centos:6',
},
'four': {
'start_order': 10,
@ -51,6 +51,9 @@ class TestComposeV1Builder(base.TestCase):
r = runner.DockerRunner(managed_by='tester', docker_cmd='docker')
exe = mock.Mock()
exe.side_effect = [
('exists', '', 0), # inspect for image centos:6
('', '', 1), # inspect for missing image centos:7
('Pulled centos:7', '', 0), # pull centos:6
('', '', 0), # ps for delete_missing_and_updated container_names
('', '', 0), # ps for after delete_missing_and_updated renames
('', '', 0), # ps to only create containers which don't exist
@ -68,6 +71,7 @@ class TestComposeV1Builder(base.TestCase):
stdout, stderr, deploy_status_code = builder.apply()
self.assertEqual(0, deploy_status_code)
self.assertEqual([
'Pulled centos:7',
'Created one-12345678',
'Created two-12345678',
'Created three-12345678',
@ -77,6 +81,19 @@ class TestComposeV1Builder(base.TestCase):
self.assertEqual([], stderr)
exe.assert_has_calls([
# inspect existing image centos:6
mock.call(
['docker', 'inspect', '--type', 'image',
'--format', 'exists', 'centos:6']
),
# inspect and pull missing image centos:7
mock.call(
['docker', 'inspect', '--type', 'image',
'--format', 'exists', 'centos:7']
),
mock.call(
['docker', 'pull', 'centos:7']
),
# ps for delete_missing_and_updated container_names
mock.call(
['docker', 'ps', '-a',
@ -122,7 +139,7 @@ class TestComposeV1Builder(base.TestCase):
'--label', 'container_name=three',
'--label', 'managed_by=tester',
'--label', 'config_data=%s' % json.dumps(config['three']),
'--detach=true', 'centos:7']
'--detach=true', 'centos:6']
),
# run four
mock.call(
@ -171,6 +188,8 @@ class TestComposeV1Builder(base.TestCase):
r = runner.DockerRunner(managed_by='tester', docker_cmd='docker')
exe = mock.Mock()
exe.side_effect = [
# inspect for image centos:7
('exists', '', 0),
# ps for delete_missing_and_updated container_names
('''five five
six six
@ -211,6 +230,11 @@ three-12345678 three''', '', 0),
self.assertEqual([], stderr)
exe.assert_has_calls([
# inspect image centos:7
mock.call(
['docker', 'inspect', '--type', 'image',
'--format', 'exists', 'centos:7']
),
# ps for delete_missing_and_updated container_names
mock.call(
['docker', 'ps', '-a',
@ -222,13 +246,13 @@ three-12345678 three''', '', 0),
mock.call(['docker', 'rm', '-f', 'five']),
mock.call(['docker', 'rm', '-f', 'six']),
# rm two, changed config
mock.call(['docker', 'inspect', '--format',
'{{index .Config.Labels "config_data"}}',
mock.call(['docker', 'inspect', '--type', 'container',
'--format', '{{index .Config.Labels "config_data"}}',
'two-12345678']),
mock.call(['docker', 'rm', '-f', 'two-12345678']),
# check three, config hasn't changed
mock.call(['docker', 'inspect', '--format',
'{{index .Config.Labels "config_data"}}',
mock.call(['docker', 'inspect', '--type', 'container',
'--format', '{{index .Config.Labels "config_data"}}',
'three-12345678']),
# ps for after delete_missing_and_updated renames
mock.call(
@ -277,6 +301,62 @@ three-12345678 three''', '', 0),
),
])
def test_apply_failed_pull(self):
config = {
'one': {
'start_order': 0,
'image': 'centos:7',
},
'two': {
'start_order': 1,
'image': 'centos:7',
},
'three': {
'start_order': 2,
'image': 'centos:6',
},
'four': {
'start_order': 10,
'image': 'centos:7',
},
'four_ls': {
'action': 'exec',
'start_order': 20,
'command': ['four', 'ls', '-l', '/']
}
}
r = runner.DockerRunner(managed_by='tester', docker_cmd='docker')
exe = mock.Mock()
exe.side_effect = [
('exists', '', 0), # inspect for image centos:6
('', '', 1), # inspect for missing image centos:7
('Pulling centos:7', 'ouch', 1), # pull centos:7 failure
]
r.execute = exe
builder = compose1.ComposeV1Builder('foo', config, r)
stdout, stderr, deploy_status_code = builder.apply()
self.assertEqual(1, deploy_status_code)
self.assertEqual(['Pulling centos:7'], stdout)
self.assertEqual(['ouch'], stderr)
exe.assert_has_calls([
# inspect existing image centos:6
mock.call(
['docker', 'inspect', '--type', 'image',
'--format', 'exists', 'centos:6']
),
# inspect and pull missing image centos:7
mock.call(
['docker', 'inspect', '--type', 'image',
'--format', 'exists', 'centos:7']
),
mock.call(
['docker', 'pull', 'centos:7']
),
])
@mock.patch('paunch.runner.DockerRunner', autospec=True)
def test_label_arguments(self, runner):
r = runner.return_value

5
paunch/tests/test_runner.py
View File

@ -179,7 +179,7 @@ four-12345678 four
self.runner.inspect('one')
)
self.assert_execute(
popen, ['docker', 'inspect', 'one']
popen, ['docker', 'inspect', '--type', 'container', 'one']
)
@mock.patch('subprocess.Popen')
@ -191,7 +191,8 @@ four-12345678 four
self.runner.inspect('one', format='{{foo}}')
)
self.assert_execute(
popen, ['docker', 'inspect', '--format', '{{foo}}', 'one']
popen, ['docker', 'inspect', '--type', 'container',
'--format', '{{foo}}', 'one']
)
def test_unique_container_name(self):

6
releasenotes/notes/pre-pull-aa780b6a3a519adc.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- All referenced images are now pulled as the first step of applying a
config. Any pull failures will result in failing with an error before any of
the config is applied. This is especially helpful for detached containers,
where pull failures will not be captured during the apply.