From 0e789ed3f583851b3b6e8f37545e0170bb8ca90a Mon Sep 17 00:00:00 2001 From: Ravid Brown Date: Wed, 13 Dec 2017 11:58:28 +0200 Subject: [PATCH] Send use_cache env to build container --- skipper/cli.py | 9 ++++++--- skipper/git.py | 2 +- skipper/runner.py | 11 ++++++++--- skipper/utils.py | 4 ++-- tests/test_cli.py | 45 +++++++++++++++++++++++---------------------- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/skipper/cli.py b/skipper/cli.py index 0054533..be1fa27 100644 --- a/skipper/cli.py +++ b/skipper/cli.py @@ -209,7 +209,8 @@ def run(ctx, interactive, name, env, cache, command): name=name, net=ctx.obj['build_container_net'], volumes=ctx.obj.get('volumes'), - workdir=ctx.obj.get('workdir')) + workdir=ctx.obj.get('workdir'), + use_cache=cache) @cli.command(context_settings=dict(ignore_unknown_options=True)) @@ -240,7 +241,8 @@ def make(ctx, interactive, name, env, makefile, cache, make_params): name=name, net=ctx.obj['build_container_net'], volumes=ctx.obj.get('volumes'), - workdir=ctx.obj.get('workdir')) + workdir=ctx.obj.get('workdir'), + use_cache=cache) @cli.command() @@ -267,7 +269,8 @@ def shell(ctx, env, name, cache): name=name, net=ctx.obj['build_container_net'], volumes=ctx.obj.get('volumes'), - workdir=ctx.obj.get('workdir')) + workdir=ctx.obj.get('workdir'), + use_cache=cache) @cli.command() diff --git a/skipper/git.py b/skipper/git.py index f25726c..302bcaa 100644 --- a/skipper/git.py +++ b/skipper/git.py @@ -11,7 +11,7 @@ def get_hash(short=False): if uncommitted_changes(): logging.warning("*** Uncommitted changes present - Build container version might be outdated ***") - return subprocess.check_output(git_command).strip() + return subprocess.check_output(git_command).strip().decode() def uncommitted_changes(): diff --git a/skipper/runner.py b/skipper/runner.py index 6a01d60..6e5b191 100644 --- a/skipper/runner.py +++ b/skipper/runner.py @@ -6,9 +6,10 @@ from contextlib import contextmanager -def run(command, fqdn_image=None, environment=None, interactive=False, name=None, net='host', volumes=None, workdir=None): +# pylint: disable=too-many-arguments +def run(command, fqdn_image=None, environment=None, interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False): if fqdn_image is not None: - return _run_nested(fqdn_image, environment, command, interactive, name, net, volumes, workdir) + return _run_nested(fqdn_image, environment, command, interactive, name, net, volumes, workdir, use_cache) return _run(command) @@ -22,7 +23,8 @@ def _run(cmd): # pylint: disable=too-many-locals -def _run_nested(fqdn_image, environment, command, interactive, name, net='host', volumes=None, workdir=None): +# pylint: disable=too-many-arguments +def _run_nested(fqdn_image, environment, command, interactive, name, net, volumes, workdir, use_cache): cwd = os.getcwd() workspace = os.path.dirname(cwd) project = os.path.basename(cwd) @@ -51,6 +53,9 @@ def _run_nested(fqdn_image, environment, command, interactive, name, net='host', docker_gid = grp.getgrnam('docker').gr_gid docker_cmd += ['-e', 'SKIPPER_DOCKER_GID=%(docker_gid)s' % dict(docker_gid=docker_gid)] + if use_cache: + docker_cmd += ['-e', 'SKIPPER_USE_CACHE_IMAGE=True'] + volumes = volumes or [] volumes.extend([ diff --git a/skipper/utils.py b/skipper/utils.py index 9f9375b..21db037 100644 --- a/skipper/utils.py +++ b/skipper/utils.py @@ -42,7 +42,7 @@ def local_image_exist(image, tag): '--format', '{{.ID}}', name ] - output = subprocess.check_output(command) + output = subprocess.check_output(command).decode() return output != '' @@ -66,7 +66,7 @@ def get_local_images_info(images): ] images_info = [] for image in images: - output = subprocess.check_output(command + [image]) + output = subprocess.check_output(command + [image]).decode() if output == '': continue image_info = [json.loads(record) for record in output.splitlines()] diff --git a/tests/test_cli.py b/tests/test_cli.py index 3a57075..a8cb372 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -257,7 +257,7 @@ def test_make_without_build_container_tag_with_context(self, skipper_runner_run_ mock.call(['docker', 'build', '-t', 'build-container-image', '-f', 'Dockerfile.build-container-image', SKIPPER_CONF_CONTAINER_CONTEXT]), mock.call(['make'] + make_params, fqdn_image='build-container-image', environment=[], - interactive=False, name=None, net='host', volumes=None, workdir=None), + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False), ] skipper_runner_run_mock.assert_has_calls(expected_commands) @@ -943,7 +943,7 @@ def test_run_with_existing_local_build_container(self, skipper_runner_run_mock): expected_image_name = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_image_name, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='')) @mock.patch('requests.get', autospec=True) @@ -968,7 +968,7 @@ def test_run_with_existing_remote_build_container(self, skipper_runner_run_mock, expected_image_name = 'registry.io:5000/build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_image_name, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='')) @mock.patch('skipper.runner.run', mock.MagicMock(autospec=True)) @@ -1006,7 +1006,7 @@ def test_run_with_defaults_from_config_file(self, skipper_runner_run_mock): ) expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], - interactive=False, name=None, net='host', volumes=None, workdir=None) + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1025,7 +1025,7 @@ def test_run_with_defaults_and_env_from_config_file(self, skipper_runner_run_moc env = ["%s=%s" % (key, value) for key, value in six.iteritems(CONFIG_ENV_EVALUATION)] expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=env, - interactive=False, name=None, net='host', volumes=None, workdir=None) + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1044,7 +1044,7 @@ def test_run_with_env_overriding_config_file(self, skipper_runner_run_mock): env = ["%s=%s" % (key, value) for key, value in six.iteritems(CONFIG_ENV_EVALUATION)] + ENV expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=env, - interactive=False, name=None, net='host', volumes=None, workdir=None) + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) @@ -1059,7 +1059,7 @@ def test_run_with_env(self, skipper_runner_run_mock): ) expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=ENV, - interactive=False, name=None, net='host', volumes=None, workdir=None) + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) @@ -1074,7 +1074,7 @@ def test_run_interactive_from_environment(self, skipper_runner_run_mock): ) expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], - interactive=True, name=None, net='host', volumes=None, workdir=None) + interactive=True, name=None, net='host', volumes=None, workdir=None, use_cache=False) del os.environ['SKIPPER_INTERACTIVE'] @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @@ -1090,7 +1090,7 @@ def test_run_non_interactive_from_environment(self, skipper_runner_run_mock): ) expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], - interactive=False, name=None, net='host', volumes=None, workdir=None) + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False) del os.environ['SKIPPER_INTERACTIVE'] @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @@ -1105,7 +1105,7 @@ def test_run_non_interactive(self, skipper_runner_run_mock): ) expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], - interactive=True, name=None, net='host', volumes=None, workdir=None) + interactive=True, name=None, net='host', volumes=None, workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='')) @mock.patch('skipper.runner.run', autospec=True, return_value=0) @@ -1121,7 +1121,7 @@ def test_run_without_build_container_tag(self, skipper_runner_run_mock): expected_commands = [ mock.call(['docker', 'build', '-t', 'build-container-image', '-f', 'Dockerfile.build-container-image', '.']), mock.call(command, fqdn_image='build-container-image', environment=[], - interactive=False, name=None, net='host', volumes=None, workdir=None), + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False), ] skipper_runner_run_mock.assert_has_calls(expected_commands) @@ -1139,7 +1139,8 @@ def test_run_with_non_default_net(self, skipper_runner_run_mock): ) expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], - interactive=False, name=None, net='non-default-net', volumes=None, workdir=None) + interactive=False, name=None, net='non-default-net', volumes=None, workdir=None, + use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1157,7 +1158,7 @@ def test_run_with_defaults_from_config_file_including_volumes(self, skipper_runn expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', - volumes=['volume1', 'volume2'], workdir=None) + volumes=['volume1', 'volume2'], workdir=None, use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1175,7 +1176,7 @@ def test_run_with_defaults_from_config_file_including_workdir(self, skipper_runn expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir='test-workdir') + workdir='test-workdir', use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1194,7 +1195,7 @@ def test_run_with_config_including_git_revision_with_uncommitted_changes(self, s expected_fqdn_image = 'skipper-conf-build-container-image:1234567' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1212,7 +1213,7 @@ def test_run_with_config_including_git_revision_without_uncommitted_changes(self ) expected_fqdn_image = 'skipper-conf-build-container-image:1234567' skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=[], - interactive=False, name=None, net='host', volumes=None, workdir=None) + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) @@ -1229,7 +1230,7 @@ def test_make(self, skipper_runner_run_mock): expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(expected_command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) @@ -1242,7 +1243,7 @@ def test_make_with_default_params(self, skipper_runner_run_mock): expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(expected_command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) @@ -1258,7 +1259,7 @@ def test_make_with_additional_make_params(self, skipper_runner_run_mock): expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(expected_command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('__builtin__.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) @@ -1278,7 +1279,7 @@ def test_make_with_defaults_from_config_file(self, skipper_runner_run_mock): expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag' skipper_runner_run_mock.assert_called_once_with(expected_command, fqdn_image=expected_fqdn_image, environment=[], interactive=False, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='')) @mock.patch('skipper.runner.run', autospec=True, return_value=0) @@ -1295,7 +1296,7 @@ def test_make_without_build_container_tag(self, skipper_runner_run_mock): expected_commands = [ mock.call(['docker', 'build', '-t', 'build-container-image', '-f', 'Dockerfile.build-container-image', '.']), mock.call(['make'] + make_params, fqdn_image='build-container-image', environment=[], - interactive=False, name=None, net='host', volumes=None, workdir=None), + interactive=False, name=None, net='host', volumes=None, workdir=None, use_cache=False), ] skipper_runner_run_mock.assert_has_calls(expected_commands) @@ -1309,7 +1310,7 @@ def test_shell(self, skipper_runner_run_mock): expected_fqdn_image = 'build-container-image:build-container-tag' skipper_runner_run_mock.assert_called_once_with(['bash'], fqdn_image=expected_fqdn_image, environment=[], interactive=True, name=None, net='host', volumes=None, - workdir=None) + workdir=None, use_cache=False) @mock.patch('click.echo', autospec=True) @mock.patch('skipper.cli.get_distribution', autospec=True)