From 7779c4bc40016dfc481027ec5a288705db11fc0e Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Thu, 17 Sep 2020 13:19:35 -0600 Subject: [PATCH] [ROCKY-Only] Trigger container update on image id update NOTE: Rocky is missing a few backports so this patch is rocky specific. If you reuse a container tag for a new image, paunch does not currently rebuild the container to get the new image. This change adds logic to the updated check to also check the image id defined in the container instance against the image id for the image to see if the container image has changed. Closes-Bug: #1895974 Change-Id: I2710497282eb4d6fa8f68dc6d50044c1ce68eb35 (cherry picked from commit 4ae7d886db604f1ac00102803a5ea1fa2b3a479b) --- paunch/builder/compose1.py | 28 ++- paunch/tests/test_builder_compose1.py | 246 +++++++++++++++++++++++++- 2 files changed, 264 insertions(+), 10 deletions(-) diff --git a/paunch/builder/compose1.py b/paunch/builder/compose1.py index 450ef4d..1496129 100644 --- a/paunch/builder/compose1.py +++ b/paunch/builder/compose1.py @@ -94,24 +94,40 @@ class ComposeV1Builder(object): self.runner.remove_container(container) continue - ex_data_str = self.runner.inspect( - container, '{{index .Config.Labels "config_data"}}') - if not ex_data_str: - LOG.debug("Deleting container (no config_data): %s" - % container) + inspect_info = self.runner.inspect(container) + if not inspect_info: + # we shouldn't get here but you never know + LOG.debug("Deleting container (no inspect data): " + "%s" % container) self.runner.remove_container(container) continue + container_config = inspect_info.get('Config', {}) + config_data = container_config.get('Labels', {}).get('config_data') try: - ex_data = yaml.safe_load(str(ex_data_str)) + ex_data = yaml.safe_load(str(config_data)) except Exception: ex_data = None + # check if config_data has changed new_data = self.config.get(cn[-1]) if new_data != ex_data: LOG.debug("Deleting container (changed config_data): %s" % container) self.runner.remove_container(container) + continue + + # check if the container image has changed (but tag as not) + # e.g. if you use :latest, the name doesn't change but the ID does + container_image = container_config.get('Image') + + if container_image: + image_id_str = self.runner.inspect( + container_image, "{{index .Id}}", type='image') + if str(image_id_str).strip() != inspect_info.get('Image'): + LOG.debug("Deleting container (image updated): " + "%s" % container) + self.runner.remove_container(container) # deleting containers is an opportunity for renames to their # preferred name diff --git a/paunch/tests/test_builder_compose1.py b/paunch/tests/test_builder_compose1.py index c5dc401..d5aad85 100644 --- a/paunch/tests/test_builder_compose1.py +++ b/paunch/tests/test_builder_compose1.py @@ -238,11 +238,15 @@ three-12345678 three''', '', 0), # rm six ('', '', 0), # inspect two - ('{"start_order": 1, "image": "centos:6"}', '', 0), + ('[{"Config": {"Labels": {"config_data": ' + '"{\\\"start_order\\\": 1, \\\"image\\\": \\\"centos:6\\\"}"' + '}}}]', '', 0), # rm two, changed config data ('', '', 0), # inspect three - ('{"start_order": 2, "image": "centos:7"}', '', 0), + ('[{"Config": {"Labels": {"config_data": ' + '"{\\\"start_order\\\": 2, \\\"image\\\": \\\"centos:7\\\"}"' + '}}}]', '', 0), # ps for after delete_missing_and_updated renames ('', '', 0), # ps2 for after delete_missing_and_updated renames @@ -287,12 +291,10 @@ three-12345678 three''', '', 0), mock.call(['docker', 'rm', 'six']), # rm two, changed config mock.call(['docker', 'inspect', '--type', 'container', - '--format', '{{index .Config.Labels "config_data"}}', 'two-12345678']), mock.call(['docker', 'rm', 'two-12345678']), # check three, config hasn't changed mock.call(['docker', 'inspect', '--type', 'container', - '--format', '{{index .Config.Labels "config_data"}}', 'three-12345678']), # ps for after delete_missing_and_updated renames mock.call( @@ -602,3 +604,239 @@ three-12345678 three''', '', 0), ['ls', '-l', '"/foo', 'bar"'], b.command_argument('ls -l "/foo bar"') ) + + @mock.patch('paunch.runner.DockerRunner', autospec=True) + def test_delete_updated_no_change(self, runner): + mock_names = mock.MagicMock() + mock_names.return_value = [['one']] + runner.container_names = mock_names + mock_inspect = mock.MagicMock() + mock_inspect.side_effect = [ + { + "Id": ("d038dccebdb0996ed36ab4ff06e7c424b3816d67664aa11e00642" + "be5e00cec55"), + "Config": { + "Labels": { + "config_data": """{ + \"start_order\": 0, + \"image": \"centos:7\" + }""" + }, + "Image": "127.0.0.1:8787/centos:7" + }, + "Image": "sha256:1" + }, + "sha256:1" + ] + runner.inspect = mock_inspect + mock_remove = mock.MagicMock() + runner.remove_container = mock_remove + mock_rename = mock.MagicMock() + runner.rename_containers = mock_rename + + config = { + 'one': { + 'start_order': 0, + 'image': 'centos:7', + } + } + + self.builder = compose1.ComposeV1Builder( + 'one', config, runner.return_value) + + self.builder.runner = runner + self.builder.delete_missing_and_updated() + mock_names.assert_called_once_with('one') + + calls = [ + mock.call('one'), + mock.call('127.0.0.1:8787/centos:7', + '{{index .Id}}', + type='image') + ] + mock_inspect.has_calls(calls) + mock_remove.assert_not_called() + mock_rename.assert_called_once_with() + + @mock.patch('paunch.runner.DockerRunner', autospec=True) + def test_delete_updated_inspect_empty(self, runner): + mock_names = mock.MagicMock() + mock_names.return_value = [['one']] + runner.container_names = mock_names + mock_inspect = mock.MagicMock() + mock_inspect.return_value = None + + runner.inspect = mock_inspect + mock_remove = mock.MagicMock() + runner.remove_container = mock_remove + mock_rename = mock.MagicMock() + runner.rename_containers = mock_rename + + config = { + 'one': { + 'start_order': 0, + 'image': 'centos:7', + } + } + + self.builder = compose1.ComposeV1Builder( + 'one', config, runner.return_value) + + self.builder.runner = runner + self.builder.delete_missing_and_updated() + mock_names.assert_called_once_with('one') + + calls = [ + mock.call('one'), + ] + mock_inspect.has_calls(calls) + mock_remove.assert_called_once_with('one') + mock_rename.assert_called_once_with() + + @mock.patch('paunch.runner.DockerRunner', autospec=True) + def test_delete_updated_no_config_data(self, runner): + mock_names = mock.MagicMock() + mock_names.return_value = [['one']] + runner.container_names = mock_names + mock_inspect = mock.MagicMock() + mock_inspect.side_effect = [ + { + "Id": ("d038dccebdb0996ed36ab4ff06e7c424b3816d67664aa11e00642" + "be5e00cec55"), + "Config": { + "Labels": {}, + "Image": "127.0.0.1:8787/centos:7" + }, + "Image": "sha256:1" + }, + "sha256:1" + ] + runner.inspect = mock_inspect + mock_remove = mock.MagicMock() + runner.remove_container = mock_remove + mock_rename = mock.MagicMock() + runner.rename_containers = mock_rename + + config = { + 'one': { + 'start_order': 0, + 'image': 'centos:7', + } + } + + self.builder = compose1.ComposeV1Builder( + 'one', config, runner.return_value) + + self.builder.runner = runner + self.builder.delete_missing_and_updated() + mock_names.assert_called_once_with('one') + + calls = [ + mock.call('one'), + ] + mock_inspect.has_calls(calls) + mock_remove.assert_called_once_with('one') + mock_rename.assert_called_once_with() + + @mock.patch('paunch.runner.DockerRunner', autospec=True) + def test_delete_updated_update_config(self, runner): + mock_names = mock.MagicMock() + mock_names.return_value = [['one']] + runner.container_names = mock_names + mock_inspect = mock.MagicMock() + mock_inspect.side_effect = [ + { + "Id": ("d038dccebdb0996ed36ab4ff06e7c424b3816d67664aa11e00642" + "be5e00cec55"), + "Config": { + "Labels": { + "config_data": """{ + \"start_order\": 1, + \"image": \"centos:7\" + }""" + }, + "Image": "127.0.0.1:8787/centos:7" + }, + "Image": "sha256:1" + }, + "sha256:1" + ] + runner.inspect = mock_inspect + mock_remove = mock.MagicMock() + runner.remove_container = mock_remove + mock_rename = mock.MagicMock() + runner.rename_containers = mock_rename + + config = { + 'one': { + 'start_order': 0, + 'image': 'centos:7', + } + } + + self.builder = compose1.ComposeV1Builder( + 'one', config, runner.return_value) + + self.builder.runner = runner + self.builder.delete_missing_and_updated() + mock_names.assert_called_once_with('one') + + calls = [ + mock.call('one'), + ] + mock_inspect.has_calls(calls) + mock_remove.assert_called_once_with('one') + mock_rename.assert_called_once_with() + + @mock.patch('paunch.runner.DockerRunner', autospec=True) + def test_delete_updated_update_image(self, runner): + mock_names = mock.MagicMock() + mock_names.return_value = [['one']] + runner.container_names = mock_names + mock_inspect = mock.MagicMock() + mock_inspect.side_effect = [ + { + "Id": ("d038dccebdb0996ed36ab4ff06e7c424b3816d67664aa11e00642" + "be5e00cec55"), + "Config": { + "Labels": { + "config_data": """{ + \"start_order\": 0, + \"image": \"centos:7\" + }""" + }, + "Image": "127.0.0.1:8787/centos:7" + }, + "Image": "sha256:1" + }, + "sha256:2" + ] + runner.inspect = mock_inspect + mock_remove = mock.MagicMock() + runner.remove_container = mock_remove + mock_rename = mock.MagicMock() + runner.rename_containers = mock_rename + + config = { + 'one': { + 'start_order': 0, + 'image': 'centos:7', + } + } + + self.builder = compose1.ComposeV1Builder( + 'one', config, runner.return_value) + + self.builder.runner = runner + self.builder.delete_missing_and_updated() + mock_names.assert_called_once_with('one') + + calls = [ + mock.call('one'), + mock.call('127.0.0.1:8787/centos:7', + '{{index .Id}}', + type='image') + ] + mock_inspect.has_calls(calls) + mock_remove.assert_called_once_with('one') + mock_rename.assert_called_once_with()