Merge "Cleanup containers in the same loop as they are created" into stable/queens

This commit is contained in:
Zuul 2020-03-23 18:34:09 +00:00 committed by Gerrit Code Review
commit 30f6cbf666
3 changed files with 144 additions and 128 deletions

View File

@ -11,6 +11,7 @@
# under the License. # under the License.
# #
import itertools
import json import json
import tenacity import tenacity
import yaml import yaml
@ -36,15 +37,29 @@ class ComposeV1Builder(object):
if pull_returncode != 0: if pull_returncode != 0:
return stdout, stderr, pull_returncode return stdout, stderr, pull_returncode
self.delete_missing_and_updated()
deploy_status_code = 0 deploy_status_code = 0
key_fltr = lambda k: self.config[k].get('start_order', 0) key_fltr = lambda k: self.config[k].get('start_order', 0)
container_names = self.runner.container_names(self.config_id) container_names = self.runner.container_names(self.config_id)
# Cleanup containers missing from the config.
# Also applying new containers configs is an opportunity for
# renames to their preferred names.
changed = self.delete_missing(container_names)
changed |= self.runner.rename_containers()
if changed:
# If anything has been changed, refresh the container_names
container_names = self.runner.container_names(self.config_id)
desired_names = set([cn[-1] for cn in container_names]) desired_names = set([cn[-1] for cn in container_names])
for container in sorted(self.config, key=key_fltr): for container in sorted(self.config, key=key_fltr):
# Before creating the container, figure out if it needs to be
# removed because of its configuration has changed.
# If anything has been deleted, refresh the container_names/desired
if self.delete_updated(container, container_names):
container_names = self.runner.container_names(self.config_id)
desired_names = set([cn[-1] for cn in container_names])
self.log.debug("Running container: %s" % container) self.log.debug("Running container: %s" % container)
action = self.config[container].get('action', 'run') action = self.config[container].get('action', 'run')
exit_codes = self.config[container].get('exit_codes', [0]) exit_codes = self.config[container].get('exit_codes', [0])
@ -87,39 +102,44 @@ class ComposeV1Builder(object):
self.log.info("stderr: %s" % cmd_stderr) self.log.info("stderr: %s" % cmd_stderr)
return stdout, stderr, deploy_status_code return stdout, stderr, deploy_status_code
def delete_missing_and_updated(self): def delete_missing(self, container_names):
container_names = self.runner.container_names(self.config_id) deleted = False
for cn in container_names: for cn in container_names:
container = cn[0] container = cn[0]
# if the desired name is not in the config, delete it # if the desired name is not in the config, delete it
if cn[-1] not in self.config: if cn[-1] not in self.config:
self.log.debug("Deleting container (removed): %s" % container) self.log.debug("Deleting container (removed): "
"%s" % container)
self.runner.remove_container(container) self.runner.remove_container(container)
continue deleted = True
return deleted
ex_data_str = self.runner.inspect( def delete_updated(self, container, container_names):
container, '{{index .Config.Labels "config_data"}}') # If a container is not deployed, there is nothing to delete
if not ex_data_str: if (container not in
self.log.debug("Deleting container (no config_data): %s" % list(itertools.chain.from_iterable(container_names))):
container) return False
self.runner.remove_container(container)
continue
try: ex_data_str = self.runner.inspect(
ex_data = yaml.safe_load(str(ex_data_str)) container, '{{index .Config.Labels "config_data"}}')
except Exception: if not ex_data_str:
ex_data = None self.log.debug("Deleting container (no_config_data): "
"%s" % container)
self.runner.remove_container(container)
return True
new_data = self.config.get(cn[-1]) try:
if new_data != ex_data: ex_data = yaml.safe_load(str(ex_data_str))
self.log.debug("Deleting container (changed config_data): %s" % except Exception:
container) ex_data = None
self.runner.remove_container(container)
# deleting containers is an opportunity for renames to their new_data = self.config[container]
# preferred name if new_data != ex_data:
self.runner.rename_containers() self.log.debug("Deleting container (changed config_data): %s"
% container)
self.runner.remove_container(container)
return True
return False
def label_arguments(self, cmd, container): def label_arguments(self, cmd, container):
if self.labels: if self.labels:

View File

@ -101,13 +101,16 @@ class DockerRunner(object):
cmd_stdout, cmd_stderr, returncode = self.execute(cmd, self.log) cmd_stdout, cmd_stderr, returncode = self.execute(cmd, self.log)
if returncode != 0: if returncode != 0:
return return
result = []
for line in cmd_stdout.split("\n"): for line in cmd_stdout.split("\n"):
if line: if line:
yield line.split() result.append(line.split())
return result
def rename_containers(self): def rename_containers(self):
current_containers = [] current_containers = []
need_renaming = {} need_renaming = {}
renamed = False
for entry in self.container_names(): for entry in self.container_names():
current_containers.append(entry[0]) current_containers.append(entry[0])
@ -129,7 +132,9 @@ class DockerRunner(object):
self.log.info('Renaming "%s" to "%s"' % ( self.log.info('Renaming "%s" to "%s"' % (
current, desired)) current, desired))
self.rename_container(current, desired) self.rename_container(current, desired)
renamed = True
current_containers.append(desired) current_containers.append(desired)
return renamed
def rename_container(self, container, name): def rename_container(self, container, name):
cmd = [self.docker_cmd, 'rename', container, name] cmd = [self.docker_cmd, 'rename', container, name]

View File

@ -26,7 +26,9 @@ from paunch.tests import base
class TestComposeV1Builder(base.TestCase): class TestComposeV1Builder(base.TestCase):
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3]) @mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
def test_apply(self, mock_cpu): @mock.patch("paunch.builder.compose1.ComposeV1Builder.delete_updated",
return_value=False)
def test_apply(self, mock_delete_updated, mock_cpu):
orig_call = tenacity.wait.wait_random_exponential.__call__ orig_call = tenacity.wait.wait_random_exponential.__call__
orig_argspec = inspect.getargspec(orig_call) orig_argspec = inspect.getargspec(orig_call)
config = { config = {
@ -60,14 +62,24 @@ class TestComposeV1Builder(base.TestCase):
('', '', 1), # inspect for missing image centos:7 ('', '', 1), # inspect for missing image centos:7
('Pulled centos:7', 'ouch', 1), # pull centos:6 fails ('Pulled centos:7', 'ouch', 1), # pull centos:6 fails
('Pulled centos:7', '', 0), # pull centos:6 succeeds ('Pulled centos:7', '', 0), # pull centos:6 succeeds
('', '', 0), # ps for delete_missing_and_updated container_names # container_names for delete_missing
('', '', 0), # ps for after delete_missing_and_updated renames ('''five five
('', '', 0), # ps to only create containers which don't exist six six
two two
three-12345678 three''', '', 0),
('', '', 0), # stop five
('', '', 0), # rm five
('', '', 0), # stop six
('', '', 0), # rm six
# container_names for rename_containers
('three-12345678 three', '', 0),
('', '', 0), # rename three
# desired/container_names to be refreshed after delete/rename
('three three', '', 0), # renamed three already exists
('Created one-12345678', '', 0), ('Created one-12345678', '', 0),
('Created two-12345678', '', 0), ('Created two-12345678', '', 0),
('Created three-12345678', '', 0),
('Created four-12345678', '', 0), ('Created four-12345678', '', 0),
('a\nb\nc', '', 0) ('a\nb\nc', '', 0) # exec four
] ]
r.discover_container_name = lambda n, c: '%s-12345678' % n r.discover_container_name = lambda n, c: '%s-12345678' % n
r.unique_container_name = lambda n: '%s-12345678' % n r.unique_container_name = lambda n: '%s-12345678' % n
@ -85,7 +97,6 @@ class TestComposeV1Builder(base.TestCase):
'Pulled centos:7', 'Pulled centos:7',
'Created one-12345678', 'Created one-12345678',
'Created two-12345678', 'Created two-12345678',
'Created three-12345678',
'Created four-12345678', 'Created four-12345678',
'a\nb\nc' 'a\nb\nc'
], stdout) ], stdout)
@ -110,7 +121,7 @@ class TestComposeV1Builder(base.TestCase):
mock.call( mock.call(
['docker', 'pull', 'centos:7'], mock.ANY ['docker', 'pull', 'centos:7'], mock.ANY
), ),
# ps for delete_missing_and_updated container_names # container_names for delete_missing
mock.call( mock.call(
['docker', 'ps', '-a', ['docker', 'ps', '-a',
'--filter', 'label=managed_by=tester', '--filter', 'label=managed_by=tester',
@ -118,14 +129,22 @@ class TestComposeV1Builder(base.TestCase):
'--format', '{{.Names}} {{.Label "container_name"}}'], '--format', '{{.Names}} {{.Label "container_name"}}'],
mock.ANY mock.ANY
), ),
# ps for after delete_missing_and_updated renames # rm containers missing in config
mock.call(['docker', 'stop', 'five'], mock.ANY),
mock.call(['docker', 'rm', 'five'], mock.ANY),
mock.call(['docker', 'stop', 'six'], mock.ANY),
mock.call(['docker', 'rm', 'six'], mock.ANY),
# container_names for rename
mock.call( mock.call(
['docker', 'ps', '-a', ['docker', 'ps', '-a',
'--filter', 'label=managed_by=tester', '--filter', 'label=managed_by=tester',
'--format', '{{.Names}} {{.Label "container_name"}}'], '--format', '{{.Names}} {{.Label "container_name"}}'],
mock.ANY mock.ANY
), ),
# ps to only create containers which don't exist # rename three from an ephemeral to the static name
mock.call(['docker', 'rename', 'three-12345678', 'three'],
mock.ANY),
# container_names to be refreshed after delete/rename
mock.call( mock.call(
['docker', 'ps', '-a', ['docker', 'ps', '-a',
'--filter', 'label=managed_by=tester', '--filter', 'label=managed_by=tester',
@ -153,16 +172,6 @@ class TestComposeV1Builder(base.TestCase):
'--detach=true', '--cpuset-cpus=0,1,2,3', 'centos:7'], '--detach=true', '--cpuset-cpus=0,1,2,3', 'centos:7'],
mock.ANY mock.ANY
), ),
# run three
mock.call(
['docker', 'run', '--name', 'three-12345678',
'--label', 'config_id=foo',
'--label', 'container_name=three',
'--label', 'managed_by=tester',
'--label', 'config_data=%s' % json.dumps(config['three']),
'--detach=true', '--cpuset-cpus=0,1,2,3', 'centos:6'],
mock.ANY
),
# run four # run four
mock.call( mock.call(
['docker', 'run', '--name', 'four-12345678', ['docker', 'run', '--name', 'four-12345678',
@ -180,19 +189,22 @@ class TestComposeV1Builder(base.TestCase):
]) ])
@mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3]) @mock.patch("psutil.Process.cpu_affinity", return_value=[0, 1, 2, 3])
def test_apply_idempotency(self, mock_cpu): @mock.patch("paunch.runner.DockerRunner.container_names")
@mock.patch("paunch.runner.DockerRunner.discover_container_name",
return_value='one')
def test_apply_idempotency(self, mock_dname, mock_cnames, mock_cpu):
config = { config = {
# not running yet # running with the same config and given an ephemeral name
'one': { 'one': {
'start_order': 0, 'start_order': 0,
'image': 'centos:7', 'image': 'centos:7',
}, },
# running, but with a different config # not running yet
'two': { 'two': {
'start_order': 1, 'start_order': 1,
'image': 'centos:7', 'image': 'centos:7',
}, },
# running with the same config # running, but with a different config
'three': { 'three': {
'start_order': 2, 'start_order': 2,
'image': 'centos:7', 'image': 'centos:7',
@ -202,47 +214,46 @@ class TestComposeV1Builder(base.TestCase):
'start_order': 10, 'start_order': 10,
'image': 'centos:7', 'image': 'centos:7',
}, },
'four_ls': { 'one_ls': {
'action': 'exec', 'action': 'exec',
'start_order': 20, 'start_order': 20,
'command': ['four', 'ls', '-l', '/'] 'command': ['one', 'ls', '-l', '/']
} }
# five is running but is not managed by us
} }
# represents the state before and after renaming/removing things
mock_cnames.side_effect = (
# delete_missing
[['five', 'five'], ['one-12345678', 'one'], ['three', 'three']],
# rename_containers
[['one-12345678', 'one']],
# refresh container_names/desired after del/rename
[['one', 'one'], ['three', 'three']],
# refresh container_names/desired after delete_updated
[['one', 'one']]
)
r = runner.DockerRunner(managed_by='tester', docker_cmd='docker') r = runner.DockerRunner(managed_by='tester', docker_cmd='docker')
exe = mock.Mock() exe = mock.Mock()
exe.side_effect = [ exe.side_effect = [
# inspect for image centos:7 # inspect for image centos:7
('exists', '', 0), ('exists', '', 0),
# ps for delete_missing_and_updated container_names
('''five five
six six
two-12345678 two
three-12345678 three''', '', 0),
# stop five # stop five
('', '', 0), ('', '', 0),
# rm five # rm five
('', '', 0), ('', '', 0),
# stop six ('', '', 0), # ps for rename one
('', '', 0), # inspect one
# rm six ('{"start_order": 0, "image": "centos:7"}', '', 0),
('', '', 0),
# inspect two
('{"start_order": 1, "image": "centos:6"}', '', 0),
# stop two, changed config data
('', '', 0),
# rm two, changed config data
('', '', 0),
# inspect three
('{"start_order": 2, "image": "centos:7"}', '', 0),
# ps for after delete_missing_and_updated renames
('', '', 0),
# ps to only create containers which don't exist
('three-12345678 three', '', 0),
('Created one-12345678', '', 0),
('Created two-12345678', '', 0), ('Created two-12345678', '', 0),
# inspect three
('{"start_order": 42, "image": "centos:7"}', '', 0),
# stop three, changed config data
('', '', 0),
# rm three, changed config data
('', '', 0),
('Created three-12345678', '', 0),
('Created four-12345678', '', 0), ('Created four-12345678', '', 0),
('a\nb\nc', '', 0) ('a\nb\nc', '', 0) # exec one
] ]
r.discover_container_name = lambda n, c: '%s-12345678' % n r.discover_container_name = lambda n, c: '%s-12345678' % n
r.unique_container_name = lambda n: '%s-12345678' % n r.unique_container_name = lambda n: '%s-12345678' % n
@ -252,8 +263,8 @@ three-12345678 three''', '', 0),
stdout, stderr, deploy_status_code = builder.apply() stdout, stderr, deploy_status_code = builder.apply()
self.assertEqual(0, deploy_status_code) self.assertEqual(0, deploy_status_code)
self.assertEqual([ self.assertEqual([
'Created one-12345678',
'Created two-12345678', 'Created two-12345678',
'Created three-12345678',
'Created four-12345678', 'Created four-12345678',
'a\nb\nc' 'a\nb\nc'
], stdout) ], stdout)
@ -265,54 +276,17 @@ three-12345678 three''', '', 0),
['docker', 'inspect', '--type', 'image', ['docker', 'inspect', '--type', 'image',
'--format', 'exists', 'centos:7'], mock.ANY '--format', 'exists', 'centos:7'], mock.ANY
), ),
# ps for delete_missing_and_updated container_names
mock.call(
['docker', 'ps', '-a',
'--filter', 'label=managed_by=tester',
'--filter', 'label=config_id=foo',
'--format', '{{.Names}} {{.Label "container_name"}}'],
mock.ANY
),
# rm containers not in config # rm containers not in config
mock.call(['docker', 'stop', 'five'], mock.ANY), mock.call(['docker', 'stop', 'five'], mock.ANY),
mock.call(['docker', 'rm', 'five'], mock.ANY), mock.call(['docker', 'rm', 'five'], mock.ANY),
mock.call(['docker', 'stop', 'six'], mock.ANY), # rename one from an ephemeral to the static name
mock.call(['docker', 'rm', 'six'], mock.ANY), mock.call(['docker', 'rename', 'one-12345678', 'one'],
# rm two, changed config mock.ANY),
# check the renamed one, config hasn't changed
mock.call(['docker', 'inspect', '--type', 'container', mock.call(['docker', 'inspect', '--type', 'container',
'--format', '{{index .Config.Labels "config_data"}}', '--format', '{{index .Config.Labels "config_data"}}',
'two-12345678'], mock.ANY), 'one'], mock.ANY),
mock.call(['docker', 'stop', 'two-12345678'], mock.ANY), # don't run one, its already running
mock.call(['docker', 'rm', 'two-12345678'], mock.ANY),
# check three, config hasn't changed
mock.call(['docker', 'inspect', '--type', 'container',
'--format', '{{index .Config.Labels "config_data"}}',
'three-12345678'], mock.ANY),
# ps for after delete_missing_and_updated renames
mock.call(
['docker', 'ps', '-a',
'--filter', 'label=managed_by=tester',
'--format', '{{.Names}} {{.Label "container_name"}}'],
mock.ANY
),
# ps to only create containers which don't exist
mock.call(
['docker', 'ps', '-a',
'--filter', 'label=managed_by=tester',
'--filter', 'label=config_id=foo',
'--format', '{{.Names}} {{.Label "container_name"}}'],
mock.ANY
),
# run one
mock.call(
['docker', 'run', '--name', 'one-12345678',
'--label', 'config_id=foo',
'--label', 'container_name=one',
'--label', 'managed_by=tester',
'--label', 'config_data=%s' % json.dumps(config['one']),
'--detach=true', '--cpuset-cpus=0,1,2,3', 'centos:7'],
mock.ANY
),
# run two # run two
mock.call( mock.call(
['docker', 'run', '--name', 'two-12345678', ['docker', 'run', '--name', 'two-12345678',
@ -320,10 +294,25 @@ three-12345678 three''', '', 0),
'--label', 'container_name=two', '--label', 'container_name=two',
'--label', 'managed_by=tester', '--label', 'managed_by=tester',
'--label', 'config_data=%s' % json.dumps(config['two']), '--label', 'config_data=%s' % json.dumps(config['two']),
'--detach=true', '--cpuset-cpus=0,1,2,3', 'centos:7'], '--detach=true', '--cpuset-cpus=0,1,2,3',
mock.ANY 'centos:7'], mock.ANY
),
# rm three, changed config
mock.call(['docker', 'inspect', '--type', 'container',
'--format', '{{index .Config.Labels "config_data"}}',
'three'], mock.ANY),
mock.call(['docker', 'stop', 'three'], mock.ANY),
mock.call(['docker', 'rm', 'three'], mock.ANY),
# run three
mock.call(
['docker', 'run', '--name', 'three-12345678',
'--label', 'config_id=foo',
'--label', 'container_name=three',
'--label', 'managed_by=tester',
'--label', 'config_data=%s' % json.dumps(config['three']),
'--detach=true', '--cpuset-cpus=0,1,2,3',
'centos:7'], mock.ANY
), ),
# don't run three, its already running
# run four # run four
mock.call( mock.call(
['docker', 'run', '--name', 'four-12345678', ['docker', 'run', '--name', 'four-12345678',
@ -331,12 +320,14 @@ three-12345678 three''', '', 0),
'--label', 'container_name=four', '--label', 'container_name=four',
'--label', 'managed_by=tester', '--label', 'managed_by=tester',
'--label', 'config_data=%s' % json.dumps(config['four']), '--label', 'config_data=%s' % json.dumps(config['four']),
'--detach=true', '--cpuset-cpus=0,1,2,3', 'centos:7'], '--detach=true', '--cpuset-cpus=0,1,2,3',
mock.ANY 'centos:7'], mock.ANY
), ),
# execute within four # FIXME(bogdando): shall exec ls in the renamed one!
# Why discover_container_name is never called to get it as c_name?
mock.call( mock.call(
['docker', 'exec', 'four-12345678', 'ls', '-l', '/'], mock.ANY ['docker', 'exec', 'one-12345678', 'ls', '-l',
'/'], mock.ANY
), ),
]) ])