Skip to content

Commit

Permalink
Send use_cache env to build container
Browse files Browse the repository at this point in the history
  • Loading branch information
Ravid Brown committed Dec 13, 2017
1 parent b39c448 commit 0e789ed
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 31 deletions.
9 changes: 6 additions & 3 deletions skipper/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion skipper/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
11 changes: 8 additions & 3 deletions skipper/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down Expand Up @@ -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([
Expand Down
4 changes: 2 additions & 2 deletions skipper/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 != ''


Expand All @@ -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()]
Expand Down
45 changes: 23 additions & 22 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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'))
Expand All @@ -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'))
Expand All @@ -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)
Expand All @@ -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)

Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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))
Expand All @@ -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)
Expand All @@ -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)

Expand All @@ -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)
Expand Down

0 comments on commit 0e789ed

Please sign in to comment.