From 4b04280db83b5d8c4b259586df8ae568eee5f3a0 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 22 Feb 2016 17:30:42 -0800 Subject: [PATCH 1/2] Revert "Change special case from '_', None to ()" This reverts commit 677c50650c86b4b6fabbc21e18165f2117022bbe. Revert "Modify service_test.py::ServiceTest::test_resolve_env to reflect new behavior" This reverts commit 001903771260069c475738efbbcb830dd9cf8227. Revert "Mangle the tests. They pass for better or worse!" This reverts commit 7ab9509ce65167dc81dd14f34cddfb5ecff1329d. Revert "If an env var is passthrough but not defined on the host don't set it." This reverts commit 6540efb3d380e7ae50dd94493a43382f31e1e004. Signed-off-by: Daniel Nephin --- compose/config/config.py | 6 +++--- tests/integration/service_test.py | 1 + tests/unit/config/config_test.py | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 055ae18a..98b825ec 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -512,12 +512,12 @@ def resolve_environment(service_dict): env.update(env_vars_from_file(env_file)) env.update(parse_environment(service_dict.get('environment'))) - return dict(filter(None, (resolve_env_var(k, v) for k, v in six.iteritems(env)))) + return dict(resolve_env_var(k, v) for k, v in six.iteritems(env)) def resolve_build_args(build): args = parse_build_arguments(build.get('args')) - return dict(filter(None, (resolve_env_var(k, v) for k, v in six.iteritems(args)))) + return dict(resolve_env_var(k, v) for k, v in six.iteritems(args)) def validate_extended_service_dict(service_dict, filename, service): @@ -827,7 +827,7 @@ def resolve_env_var(key, val): elif key in os.environ: return key, os.environ[key] else: - return () + return key, '' def env_vars_from_file(filename): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index bcb87335..129d996d 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -916,6 +916,7 @@ class ServiceTest(DockerClientTestCase): 'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', + 'NO_DEF': '' }.items(): self.assertEqual(env[k], v) diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 4d3bb7be..446fc560 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1975,7 +1975,7 @@ class EnvTest(unittest.TestCase): } self.assertEqual( resolve_environment(service_dict), - {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3'}, + {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': ''}, ) def test_resolve_environment_from_env_file(self): @@ -2016,6 +2016,7 @@ class EnvTest(unittest.TestCase): 'FILE_DEF': u'bär', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', + 'NO_DEF': '' }, ) @@ -2034,7 +2035,7 @@ class EnvTest(unittest.TestCase): } self.assertEqual( resolve_build_args(build), - {'arg1': 'value1', 'empty_arg': '', 'env_arg': 'value2'}, + {'arg1': 'value1', 'empty_arg': '', 'env_arg': 'value2', 'no_env': ''}, ) @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash') From d4515781525f57e0cf92e115379191cd1f3a1e9a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 22 Feb 2016 17:47:51 -0800 Subject: [PATCH 2/2] Make environment variables without a value the same as docker-cli. Signed-off-by: Daniel Nephin --- compose/config/config.py | 2 +- compose/container.py | 6 +++++- compose/service.py | 11 +++++++++++ tests/integration/service_test.py | 2 +- tests/unit/cli_test.py | 7 ++++--- tests/unit/config/config_test.py | 6 +++--- tests/unit/service_test.py | 6 +++--- 7 files changed, 28 insertions(+), 12 deletions(-) diff --git a/compose/config/config.py b/compose/config/config.py index 98b825ec..4e91a3af 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -827,7 +827,7 @@ def resolve_env_var(key, val): elif key in os.environ: return key, os.environ[key] else: - return key, '' + return key, None def env_vars_from_file(filename): diff --git a/compose/container.py b/compose/container.py index 3a1ce0b9..c96b63ef 100644 --- a/compose/container.py +++ b/compose/container.py @@ -134,7 +134,11 @@ class Container(object): @property def environment(self): - return dict(var.split("=", 1) for var in self.get('Config.Env') or []) + def parse_env(var): + if '=' in var: + return var.split("=", 1) + return var, None + return dict(parse_env(var) for var in self.get('Config.Env') or []) @property def exit_code(self): diff --git a/compose/service.py b/compose/service.py index 8b22b7d7..01f17a12 100644 --- a/compose/service.py +++ b/compose/service.py @@ -622,6 +622,8 @@ class Service(object): override_options, one_off=one_off) + container_options['environment'] = format_environment( + container_options['environment']) return container_options def _get_container_host_config(self, override_options, one_off=False): @@ -1020,3 +1022,12 @@ def get_log_config(logging_dict): type=log_driver, config=log_options ) + + +# TODO: remove once fix is available in docker-py +def format_environment(environment): + def format_env(key, value): + if value is None: + return key + return '{key}={value}'.format(key=key, value=value) + return [format_env(*item) for item in environment.items()] diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 129d996d..968c0947 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -916,7 +916,7 @@ class ServiceTest(DockerClientTestCase): 'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', - 'NO_DEF': '' + 'NO_DEF': None }.items(): self.assertEqual(env[k], v) diff --git a/tests/unit/cli_test.py b/tests/unit/cli_test.py index 69236e2e..26ae4e30 100644 --- a/tests/unit/cli_test.py +++ b/tests/unit/cli_test.py @@ -138,9 +138,10 @@ class CLITestCase(unittest.TestCase): }) _, _, call_kwargs = mock_client.create_container.mock_calls[0] - self.assertEqual( - call_kwargs['environment'], - {'FOO': 'ONE', 'BAR': 'NEW', 'OTHER': u'bär'}) + assert ( + sorted(call_kwargs['environment']) == + sorted(['FOO=ONE', 'BAR=NEW', 'OTHER=bär']) + ) def test_run_service_with_restart_always(self): command = TopLevelCommand() diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 446fc560..11bc7f0b 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -1975,7 +1975,7 @@ class EnvTest(unittest.TestCase): } self.assertEqual( resolve_environment(service_dict), - {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': ''}, + {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': None}, ) def test_resolve_environment_from_env_file(self): @@ -2016,7 +2016,7 @@ class EnvTest(unittest.TestCase): 'FILE_DEF': u'bär', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', - 'NO_DEF': '' + 'NO_DEF': None }, ) @@ -2035,7 +2035,7 @@ class EnvTest(unittest.TestCase): } self.assertEqual( resolve_build_args(build), - {'arg1': 'value1', 'empty_arg': '', 'env_arg': 'value2', 'no_env': ''}, + {'arg1': 'value1', 'empty_arg': '', 'env_arg': 'value2', 'no_env': None}, ) @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash') diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index ce28a9ca..321ebad0 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -267,7 +267,7 @@ class ServiceTest(unittest.TestCase): self.assertEqual( opts['labels'][LABEL_CONFIG_HASH], 'f8bfa1058ad1f4231372a0b1639f0dfdb574dafff4e8d7938049ae993f7cf1fc') - assert opts['environment'] == {'also': 'real'} + assert opts['environment'] == ['also=real'] def test_get_container_create_options_sets_affinity_with_binds(self): service = Service( @@ -298,7 +298,7 @@ class ServiceTest(unittest.TestCase): 1, previous_container=prev_container) - assert opts['environment'] == {'affinity:container': '=ababab'} + assert opts['environment'] == ['affinity:container==ababab'] def test_get_container_create_options_no_affinity_without_binds(self): service = Service('foo', image='foo', client=self.mock_client) @@ -312,7 +312,7 @@ class ServiceTest(unittest.TestCase): {}, 1, previous_container=prev_container) - assert opts['environment'] == {} + assert opts['environment'] == [] def test_get_container_not_found(self): self.mock_client.containers.return_value = []