From aa1fb6749542733dc36f101b84b4dd38498fa4a3 Mon Sep 17 00:00:00 2001 From: Guillermo Arribas Date: Mon, 23 Oct 2017 12:48:44 -0300 Subject: [PATCH] Wrong format in the healthcheck test does not issue a warning (fixes #4424) Signed-off-by: Guillermo Arribas --- compose/config/config.py | 33 ++++------ compose/config/validation.py | 24 +++++++ tests/unit/config/config_test.py | 110 ++++++++++++++++++++++--------- 3 files changed, 115 insertions(+), 52 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 31c6f7277..b65733573 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -47,6 +47,7 @@ from .validation import validate_config_section from .validation import validate_cpu from .validation import validate_depends_on from .validation import validate_extends_file_path +from .validation import validate_healthcheck from .validation import validate_links from .validation import validate_network_mode from .validation import validate_pid_mode @@ -686,6 +687,7 @@ def validate_service(service_config, service_names, config_file): validate_pid_mode(service_config, service_names) validate_depends_on(service_config, service_names) validate_links(service_config, service_names) + validate_healthcheck(service_config) if not service_dict.get('image') and has_uppercase(service_name): raise ConfigurationError( @@ -728,7 +730,7 @@ def process_service(service_config): service_dict[field] = to_list(service_dict[field]) service_dict = process_blkio_config(process_ports( - process_healthcheck(service_dict, service_config.name) + process_healthcheck(service_dict) )) return service_dict @@ -781,33 +783,20 @@ def process_blkio_config(service_dict): return service_dict -def process_healthcheck(service_dict, service_name): +def process_healthcheck(service_dict): if 'healthcheck' not in service_dict: return service_dict - hc = {} - raw = service_dict['healthcheck'] - - if raw.get('disable'): - if len(raw) > 1: - raise ConfigurationError( - 'Service "{}" defines an invalid healthcheck: ' - '"disable: true" cannot be combined with other options' - .format(service_name)) - hc['test'] = ['NONE'] - elif 'test' in raw: - hc['test'] = raw['test'] + if 'disable' in service_dict['healthcheck']: + del service_dict['healthcheck']['disable'] + service_dict['healthcheck']['test'] = ['NONE'] for field in ['interval', 'timeout', 'start_period']: - if field in raw: - if not isinstance(raw[field], six.integer_types): - hc[field] = parse_nanoseconds_int(raw[field]) - else: # Conversion has been done previously - hc[field] = raw[field] - if 'retries' in raw: - hc['retries'] = raw['retries'] + if field in service_dict['healthcheck']: + if not isinstance(service_dict['healthcheck'][field], six.integer_types): + service_dict['healthcheck'][field] = parse_nanoseconds_int( + service_dict['healthcheck'][field]) - service_dict['healthcheck'] = hc return service_dict diff --git a/compose/config/validation.py b/compose/config/validation.py index 940775a20..8247cf150 100644 --- a/compose/config/validation.py +++ b/compose/config/validation.py @@ -465,3 +465,27 @@ def handle_errors(errors, format_error_func, filename): "The Compose file{file_msg} is invalid because:\n{error_msg}".format( file_msg=" '{}'".format(filename) if filename else "", error_msg=error_msg)) + + +def validate_healthcheck(service_config): + healthcheck = service_config.config.get('healthcheck', {}) + + if 'test' in healthcheck and isinstance(healthcheck['test'], list): + if len(healthcheck['test']) == 0: + raise ConfigurationError( + 'Service "{}" defines an invalid healthcheck: ' + '"test" is an empty list' + .format(service_config.name)) + + # when disable is true config.py::process_healthcheck adds "test: ['NONE']" to service_config + elif healthcheck['test'][0] == 'NONE' and len(healthcheck) > 1: + raise ConfigurationError( + 'Service "{}" defines an invalid healthcheck: ' + '"disable: true" cannot be combined with other options' + .format(service_config.name)) + + elif healthcheck['test'][0] not in ('NONE', 'CMD', 'CMD-SHELL'): + raise ConfigurationError( + 'Service "{}" defines an invalid healthcheck: ' + 'when "test" is a list the first item must be either NONE, CMD or CMD-SHELL' + .format(service_config.name)) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 7a7cfacdd..3eaa97166 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -34,7 +34,6 @@ from compose.const import COMPOSEFILE_V3_1 as V3_1 from compose.const import COMPOSEFILE_V3_2 as V3_2 from compose.const import COMPOSEFILE_V3_3 as V3_3 from compose.const import IS_WINDOWS_PLATFORM -from compose.utils import nanoseconds_from_time_seconds from tests import mock from tests import unittest @@ -4191,52 +4190,103 @@ class BuildPathTest(unittest.TestCase): class HealthcheckTest(unittest.TestCase): def test_healthcheck(self): - service_dict = make_service_dict( - 'test', - {'healthcheck': { - 'test': ['CMD', 'true'], - 'interval': '1s', - 'timeout': '1m', - 'retries': 3, - 'start_period': '10s' - }}, - '.', + config_dict = config.load( + build_config_details({ + 'version': '2.3', + 'services': { + 'test': { + 'image': 'busybox', + 'healthcheck': { + 'test': ['CMD', 'true'], + 'interval': '1s', + 'timeout': '1m', + 'retries': 3, + 'start_period': '10s', + } + } + } + + }) ) - assert service_dict['healthcheck'] == { + serialized_config = yaml.load(serialize_config(config_dict)) + serialized_service = serialized_config['services']['test'] + + assert serialized_service['healthcheck'] == { 'test': ['CMD', 'true'], - 'interval': nanoseconds_from_time_seconds(1), - 'timeout': nanoseconds_from_time_seconds(60), + 'interval': '1s', + 'timeout': '1m', 'retries': 3, - 'start_period': nanoseconds_from_time_seconds(10) + 'start_period': '10s' } def test_disable(self): - service_dict = make_service_dict( - 'test', - {'healthcheck': { - 'disable': True, - }}, - '.', + config_dict = config.load( + build_config_details({ + 'version': '2.3', + 'services': { + 'test': { + 'image': 'busybox', + 'healthcheck': { + 'disable': True, + } + } + } + + }) ) - assert service_dict['healthcheck'] == { + serialized_config = yaml.load(serialize_config(config_dict)) + serialized_service = serialized_config['services']['test'] + + assert serialized_service['healthcheck'] == { 'test': ['NONE'], } def test_disable_with_other_config_is_invalid(self): with pytest.raises(ConfigurationError) as excinfo: - make_service_dict( - 'invalid-healthcheck', - {'healthcheck': { - 'disable': True, - 'interval': '1s', - }}, - '.', + config.load( + build_config_details({ + 'version': '2.3', + 'services': { + 'invalid-healthcheck': { + 'image': 'busybox', + 'healthcheck': { + 'disable': True, + 'interval': '1s', + } + } + } + + }) ) assert 'invalid-healthcheck' in excinfo.exconly() - assert 'disable' in excinfo.exconly() + assert '"disable: true" cannot be combined with other options' in excinfo.exconly() + + def test_healthcheck_with_invalid_test(self): + with pytest.raises(ConfigurationError) as excinfo: + config.load( + build_config_details({ + 'version': '2.3', + 'services': { + 'invalid-healthcheck': { + 'image': 'busybox', + 'healthcheck': { + 'test': ['true'], + 'interval': '1s', + 'timeout': '1m', + 'retries': 3, + 'start_period': '10s', + } + } + } + + }) + ) + + assert 'invalid-healthcheck' in excinfo.exconly() + assert 'the first item must be either NONE, CMD or CMD-SHELL' in excinfo.exconly() class GetDefaultConfigFilesTestCase(unittest.TestCase):