From 8f0ef26a734df37ea830956f25aee7ab9b464a04 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 6 Jul 2017 17:25:41 -0700 Subject: [PATCH] Improved version comparisons throughout the codebase Signed-off-by: Joffrey F --- compose/config/config.py | 17 ++++++----- compose/config/errors.py | 2 +- compose/config/interpolation.py | 3 +- compose/config/serialize.py | 9 +++--- compose/const.py | 18 +++++++----- compose/version.py | 10 +++++++ tests/acceptance/cli_test.py | 9 ++++-- tests/integration/project_test.py | 22 +++++++------- tests/integration/testcases.py | 39 +++++++++++-------------- tests/unit/bundle_test.py | 3 +- tests/unit/config/config_test.py | 8 ++--- tests/unit/config/interpolation_test.py | 8 +++-- tests/unit/project_test.py | 28 +++++++++--------- 13 files changed, 97 insertions(+), 79 deletions(-) create mode 100644 compose/version.py diff --git a/compose/config/config.py b/compose/config/config.py index 2b1b99104..f5053af8a 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -18,6 +18,7 @@ from ..const import COMPOSEFILE_V1 as V1 from ..utils import build_string_dict from ..utils import parse_nanoseconds_int from ..utils import splitdrive +from ..version import ComposeVersion from .environment import env_vars_from_file from .environment import Environment from .environment import split_env @@ -188,15 +189,16 @@ class ConfigFile(namedtuple('_ConfigFile', 'filename config')): if version == '1': raise ConfigurationError( 'Version in "{}" is invalid. {}' - .format(self.filename, VERSION_EXPLANATION)) + .format(self.filename, VERSION_EXPLANATION) + ) if version == '2': - version = const.COMPOSEFILE_V2_0 + return const.COMPOSEFILE_V2_0 if version == '3': - version = const.COMPOSEFILE_V3_0 + return const.COMPOSEFILE_V3_0 - return version + return ComposeVersion(version) def get_service(self, name): return self.get_service_dicts()[name] @@ -496,7 +498,7 @@ def process_config_file(config_file, environment, service_name=None): 'service', environment) - if config_file.version != V1: + if config_file.version > V1: processed_config = dict(config_file.config) processed_config['services'] = services processed_config['volumes'] = interpolate_config_section( @@ -509,14 +511,13 @@ def process_config_file(config_file, environment, service_name=None): config_file.get_networks(), 'network', environment) - if config_file.version in (const.COMPOSEFILE_V3_1, const.COMPOSEFILE_V3_2, - const.COMPOSEFILE_V3_3): + if config_file.version >= const.COMPOSEFILE_V3_1: processed_config['secrets'] = interpolate_config_section( config_file, config_file.get_secrets(), 'secrets', environment) - if config_file.version in (const.COMPOSEFILE_V3_3): + if config_file.version >= const.COMPOSEFILE_V3_3: processed_config['configs'] = interpolate_config_section( config_file, config_file.get_configs(), diff --git a/compose/config/errors.py b/compose/config/errors.py index ac1d3ac19..f5c038088 100644 --- a/compose/config/errors.py +++ b/compose/config/errors.py @@ -4,7 +4,7 @@ from __future__ import unicode_literals VERSION_EXPLANATION = ( 'You might be seeing this error because you\'re using the wrong Compose file version. ' - 'Either specify a supported version ("2.0", "2.1", "3.0", "3.1", "3.2") and place ' + 'Either specify a supported version (e.g "2.2" or "3.3") and place ' 'your service definitions under the `services` key, or omit the `version` key ' 'and place your service definitions at the root of the file to use ' 'version 1.\nFor more on the Compose file format versions, see ' diff --git a/compose/config/interpolation.py b/compose/config/interpolation.py index 1b270b9ea..b13ac591a 100644 --- a/compose/config/interpolation.py +++ b/compose/config/interpolation.py @@ -7,7 +7,6 @@ from string import Template import six from .errors import ConfigurationError -from compose.const import COMPOSEFILE_V1 as V1 from compose.const import COMPOSEFILE_V2_0 as V2_0 @@ -28,7 +27,7 @@ class Interpolator(object): def interpolate_environment_variables(version, config, section, environment): - if version in (V2_0, V1): + if version <= V2_0: interpolator = Interpolator(Template, environment) else: interpolator = Interpolator(TemplateWithDefaults, environment) diff --git a/compose/config/serialize.py b/compose/config/serialize.py index 306f86969..84521848d 100644 --- a/compose/config/serialize.py +++ b/compose/config/serialize.py @@ -7,9 +7,8 @@ import yaml from compose.config import types from compose.const import COMPOSEFILE_V1 as V1 from compose.const import COMPOSEFILE_V2_1 as V2_1 -from compose.const import COMPOSEFILE_V2_2 as V2_2 +from compose.const import COMPOSEFILE_V3_0 as V3_0 from compose.const import COMPOSEFILE_V3_2 as V3_2 -from compose.const import COMPOSEFILE_V3_3 as V3_3 def serialize_config_type(dumper, data): @@ -41,7 +40,7 @@ yaml.SafeDumper.add_representer(six.text_type, serialize_string) def denormalize_config(config, image_digests=None): - result = {'version': V2_1 if config.version == V1 else config.version} + result = {'version': str(V2_1) if config.version == V1 else str(config.version)} denormalized_services = [ denormalize_service_dict( service_dict, @@ -107,7 +106,7 @@ def denormalize_service_dict(service_dict, version, image_digest=None): if version == V1 and 'network_mode' not in service_dict: service_dict['network_mode'] = 'bridge' - if 'depends_on' in service_dict and version not in (V2_1, V2_2): + if 'depends_on' in service_dict and (version < V2_1 or version >= V3_0): service_dict['depends_on'] = sorted([ svc for svc in service_dict['depends_on'].keys() ]) @@ -122,7 +121,7 @@ def denormalize_service_dict(service_dict, version, image_digest=None): service_dict['healthcheck']['timeout'] ) - if 'ports' in service_dict and version not in (V3_2, V3_3): + if 'ports' in service_dict and version < V3_2: service_dict['ports'] = [ p.legacy_repr() if isinstance(p, types.ServicePort) else p for p in service_dict['ports'] diff --git a/compose/const.py b/compose/const.py index 36f213897..e46de8a73 100644 --- a/compose/const.py +++ b/compose/const.py @@ -3,6 +3,8 @@ from __future__ import unicode_literals import sys +from .version import ComposeVersion + DEFAULT_TIMEOUT = 10 HTTP_TIMEOUT = 60 IMAGE_EVENTS = ['delete', 'import', 'load', 'pull', 'push', 'save', 'tag', 'untag'] @@ -19,15 +21,15 @@ NANOCPUS_SCALE = 1000000000 SECRETS_PATH = '/run/secrets' -COMPOSEFILE_V1 = '1' -COMPOSEFILE_V2_0 = '2.0' -COMPOSEFILE_V2_1 = '2.1' -COMPOSEFILE_V2_2 = '2.2' +COMPOSEFILE_V1 = ComposeVersion('1') +COMPOSEFILE_V2_0 = ComposeVersion('2.0') +COMPOSEFILE_V2_1 = ComposeVersion('2.1') +COMPOSEFILE_V2_2 = ComposeVersion('2.2') -COMPOSEFILE_V3_0 = '3.0' -COMPOSEFILE_V3_1 = '3.1' -COMPOSEFILE_V3_2 = '3.2' -COMPOSEFILE_V3_3 = '3.3' +COMPOSEFILE_V3_0 = ComposeVersion('3.0') +COMPOSEFILE_V3_1 = ComposeVersion('3.1') +COMPOSEFILE_V3_2 = ComposeVersion('3.2') +COMPOSEFILE_V3_3 = ComposeVersion('3.3') API_VERSIONS = { COMPOSEFILE_V1: '1.21', diff --git a/compose/version.py b/compose/version.py new file mode 100644 index 000000000..0532e16c7 --- /dev/null +++ b/compose/version.py @@ -0,0 +1,10 @@ +from __future__ import absolute_import +from __future__ import unicode_literals + +from distutils.version import LooseVersion + + +class ComposeVersion(LooseVersion): + """ A hashable version object """ + def __hash__(self): + return hash(self.vstring) diff --git a/tests/acceptance/cli_test.py b/tests/acceptance/cli_test.py index 9d2de622d..343e4974f 100644 --- a/tests/acceptance/cli_test.py +++ b/tests/acceptance/cli_test.py @@ -850,8 +850,13 @@ class CLITestCase(DockerClientTestCase): # Two networks were created: back and front assert sorted(n['Name'].split('/')[-1] for n in networks) == [back_name, front_name] - back_network = [n for n in networks if n['Name'] == back_name][0] - front_network = [n for n in networks if n['Name'] == front_name][0] + # lookup by ID instead of name in case of duplicates + back_network = self.client.inspect_network( + [n for n in networks if n['Name'] == back_name][0]['Id'] + ) + front_network = self.client.inspect_network( + [n for n in networks if n['Name'] == front_name][0]['Id'] + ) web_container = self.project.get_service('web').containers()[0] app_container = self.project.get_service('app').containers()[0] diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 6731f25dd..ce95c5f21 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -34,6 +34,7 @@ from compose.project import Project from compose.project import ProjectError from compose.service import ConvergenceStrategy from tests.integration.testcases import v2_1_only +from tests.integration.testcases import v2_2_only from tests.integration.testcases import v2_only from tests.integration.testcases import v3_only @@ -150,7 +151,7 @@ class ProjectTest(DockerClientTestCase): name='composetest', client=self.client, config_data=load_config({ - 'version': V2_0, + 'version': str(V2_0), 'services': { 'net': { 'image': 'busybox:latest', @@ -178,7 +179,7 @@ class ProjectTest(DockerClientTestCase): return Project.from_config( name='composetest', config_data=load_config({ - 'version': V2_0, + 'version': str(V2_0), 'services': { 'web': { 'image': 'busybox:latest', @@ -820,7 +821,7 @@ class ProjectTest(DockerClientTestCase): def test_up_with_enable_ipv6(self): self.require_api_version('1.23') config_data = build_config( - version=V2_0, + version=V2_1, services=[{ 'name': 'web', 'image': 'busybox:latest', @@ -1003,7 +1004,7 @@ class ProjectTest(DockerClientTestCase): network_name = 'network_with_label' config_data = build_config( - version=V2_0, + version=V2_1, services=[{ 'name': 'web', 'image': 'busybox:latest', @@ -1063,7 +1064,7 @@ class ProjectTest(DockerClientTestCase): volume_name = 'volume_with_label' config_data = build_config( - version=V2_0, + version=V2_1, services=[{ 'name': 'web', 'image': 'busybox:latest', @@ -1103,7 +1104,7 @@ class ProjectTest(DockerClientTestCase): base_file = config.ConfigFile( 'base.yml', { - 'version': V2_0, + 'version': str(V2_0), 'services': { 'simple': {'image': 'busybox:latest', 'command': 'top'}, 'another': { @@ -1122,7 +1123,7 @@ class ProjectTest(DockerClientTestCase): override_file = config.ConfigFile( 'override.yml', { - 'version': V2_0, + 'version': str(V2_0), 'services': { 'another': { 'logging': { @@ -1155,7 +1156,7 @@ class ProjectTest(DockerClientTestCase): base_file = config.ConfigFile( 'base.yml', { - 'version': V2_0, + 'version': str(V2_0), 'services': { 'simple': { 'image': 'busybox:latest', @@ -1168,7 +1169,7 @@ class ProjectTest(DockerClientTestCase): override_file = config.ConfigFile( 'override.yml', { - 'version': V2_0, + 'version': str(V2_0), 'services': { 'simple': { 'ports': ['1234:1234'] @@ -1186,6 +1187,7 @@ class ProjectTest(DockerClientTestCase): containers = project.containers() self.assertEqual(len(containers), 1) + @v2_2_only() def test_project_up_config_scale(self): config_data = build_config( version=V2_2, @@ -1454,7 +1456,7 @@ class ProjectTest(DockerClientTestCase): base_file = config.ConfigFile( 'base.yml', { - 'version': V2_0, + 'version': str(V2_0), 'services': { 'simple': { 'image': 'busybox:latest', diff --git a/tests/integration/testcases.py b/tests/integration/testcases.py index 1e0d63215..7d600f323 100644 --- a/tests/integration/testcases.py +++ b/tests/integration/testcases.py @@ -1,11 +1,10 @@ from __future__ import absolute_import from __future__ import unicode_literals -import functools import os +import pytest from docker.utils import version_lt -from pytest import skip from .. import unittest from ..helpers import is_cluster @@ -17,7 +16,8 @@ from compose.const import COMPOSEFILE_V1 as V1 from compose.const import COMPOSEFILE_V2_0 as V2_0 from compose.const import COMPOSEFILE_V2_0 as V2_1 from compose.const import COMPOSEFILE_V2_2 as V2_2 -from compose.const import COMPOSEFILE_V3_2 as V3_2 +from compose.const import COMPOSEFILE_V3_0 as V3_0 +from compose.const import COMPOSEFILE_V3_3 as V3_3 from compose.const import LABEL_PROJECT from compose.progress_stream import stream_output from compose.service import Service @@ -43,7 +43,7 @@ def get_links(container): def engine_max_version(): if 'DOCKER_VERSION' not in os.environ: - return V3_2 + return V3_3 version = os.environ['DOCKER_VERSION'].partition('-')[0] if version_lt(version, '1.10'): return V1 @@ -51,37 +51,32 @@ def engine_max_version(): return V2_0 if version_lt(version, '1.13'): return V2_1 - return V3_2 + if version_lt(version, '17.06'): + return V2_2 + return V3_3 -def build_version_required_decorator(ignored_versions): - def decorator(f): - @functools.wraps(f) - def wrapper(self, *args, **kwargs): - max_version = engine_max_version() - if max_version in ignored_versions: - skip("Engine version %s is too low" % max_version) - return - return f(self, *args, **kwargs) - return wrapper - - return decorator +def min_version_skip(version): + return pytest.mark.skipif( + engine_max_version() < version, + reason="Engine version %s is too low" % version + ) def v2_only(): - return build_version_required_decorator((V1,)) + return min_version_skip(V2_0) def v2_1_only(): - return build_version_required_decorator((V1, V2_0)) + return min_version_skip(V2_1) def v2_2_only(): - return build_version_required_decorator((V1, V2_0, V2_1)) + return min_version_skip(V2_0) def v3_only(): - return build_version_required_decorator((V1, V2_0, V2_1, V2_2)) + return min_version_skip(V3_0) class DockerClientTestCase(unittest.TestCase): @@ -137,7 +132,7 @@ class DockerClientTestCase(unittest.TestCase): def require_api_version(self, minimum): api_version = self.client.version()['ApiVersion'] if version_lt(api_version, minimum): - skip("API version is too low ({} < {})".format(api_version, minimum)) + pytest.skip("API version is too low ({} < {})".format(api_version, minimum)) def get_volume_data(self, volume_name): if not is_cluster(self.client): diff --git a/tests/unit/bundle_test.py b/tests/unit/bundle_test.py index 3c6e9ec53..847795202 100644 --- a/tests/unit/bundle_test.py +++ b/tests/unit/bundle_test.py @@ -9,6 +9,7 @@ from compose import bundle from compose import service from compose.cli.errors import UserError from compose.config.config import Config +from compose.const import COMPOSEFILE_V2_0 as V2_0 @pytest.fixture @@ -74,7 +75,7 @@ def test_to_bundle(): {'name': 'b', 'build': './b'}, ] config = Config( - version=2, + version=V2_0, services=services, volumes={'special': {}}, networks={'extra': {}}, diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 6731a6bbc..ac742a199 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -378,7 +378,7 @@ class ConfigTest(unittest.TestCase): base_file = config.ConfigFile( 'base.yaml', { - 'version': V2_1, + 'version': str(V2_1), 'services': { 'web': { 'image': 'example/web', @@ -830,7 +830,7 @@ class ConfigTest(unittest.TestCase): service = config.load( build_config_details( { - 'version': V3_3, + 'version': str(V3_3), 'services': { 'web': { 'build': { @@ -1523,7 +1523,7 @@ class ConfigTest(unittest.TestCase): def test_isolation_option(self): actual = config.load(build_config_details({ - 'version': V2_1, + 'version': str(V2_1), 'services': { 'web': { 'image': 'win10', @@ -4122,7 +4122,7 @@ class SerializeTest(unittest.TestCase): assert serialized_config['secrets']['two'] == secrets_dict['two'] def test_serialize_ports(self): - config_dict = config.Config(version='2.0', services=[ + config_dict = config.Config(version=V2_0, services=[ { 'ports': [types.ServicePort('80', '8080', None, None, None)], 'image': 'alpine', diff --git a/tests/unit/config/interpolation_test.py b/tests/unit/config/interpolation_test.py index 256c74d9b..018a5621a 100644 --- a/tests/unit/config/interpolation_test.py +++ b/tests/unit/config/interpolation_test.py @@ -8,6 +8,8 @@ from compose.config.interpolation import interpolate_environment_variables from compose.config.interpolation import Interpolator from compose.config.interpolation import InvalidInterpolation from compose.config.interpolation import TemplateWithDefaults +from compose.const import COMPOSEFILE_V2_0 as V2_0 +from compose.const import COMPOSEFILE_V3_1 as V3_1 @pytest.fixture @@ -50,7 +52,7 @@ def test_interpolate_environment_variables_in_services(mock_env): } } } - value = interpolate_environment_variables("2.0", services, 'service', mock_env) + value = interpolate_environment_variables(V2_0, services, 'service', mock_env) assert value == expected @@ -75,7 +77,7 @@ def test_interpolate_environment_variables_in_volumes(mock_env): }, 'other': {}, } - value = interpolate_environment_variables("2.0", volumes, 'volume', mock_env) + value = interpolate_environment_variables(V2_0, volumes, 'volume', mock_env) assert value == expected @@ -100,7 +102,7 @@ def test_interpolate_environment_variables_in_secrets(mock_env): }, 'other': {}, } - value = interpolate_environment_variables("3.1", secrets, 'volume', mock_env) + value = interpolate_environment_variables(V3_1, secrets, 'volume', mock_env) assert value == expected diff --git a/tests/unit/project_test.py b/tests/unit/project_test.py index c5366c395..e5f1a175f 100644 --- a/tests/unit/project_test.py +++ b/tests/unit/project_test.py @@ -10,6 +10,8 @@ from .. import mock from .. import unittest from compose.config.config import Config from compose.config.types import VolumeFromSpec +from compose.const import COMPOSEFILE_V1 as V1 +from compose.const import COMPOSEFILE_V2_0 as V2_0 from compose.const import LABEL_SERVICE from compose.container import Container from compose.project import Project @@ -21,9 +23,9 @@ class ProjectTest(unittest.TestCase): def setUp(self): self.mock_client = mock.create_autospec(docker.APIClient) - def test_from_config(self): + def test_from_config_v1(self): config = Config( - version=None, + version=V1, services=[ { 'name': 'web', @@ -53,7 +55,7 @@ class ProjectTest(unittest.TestCase): def test_from_config_v2(self): config = Config( - version=2, + version=V2_0, services=[ { 'name': 'web', @@ -166,7 +168,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=None, + version=V2_0, services=[{ 'name': 'test', 'image': 'busybox:latest', @@ -194,7 +196,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=None, + version=V2_0, services=[ { 'name': 'vol', @@ -221,7 +223,7 @@ class ProjectTest(unittest.TestCase): name='test', client=None, config_data=Config( - version=None, + version=V2_0, services=[ { 'name': 'vol', @@ -361,7 +363,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=None, + version=V1, services=[ { 'name': 'test', @@ -386,7 +388,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=None, + version=V2_0, services=[ { 'name': 'test', @@ -417,7 +419,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=None, + version=V2_0, services=[ { 'name': 'aaa', @@ -444,7 +446,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=2, + version=V2_0, services=[ { 'name': 'foo', @@ -465,7 +467,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=2, + version=V2_0, services=[ { 'name': 'foo', @@ -500,7 +502,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version=None, + version=V2_0, services=[{ 'name': 'web', 'image': 'busybox:latest', @@ -518,7 +520,7 @@ class ProjectTest(unittest.TestCase): name='test', client=self.mock_client, config_data=Config( - version='2', + version=V2_0, services=[{ 'name': 'web', 'image': 'busybox:latest',