diff --git a/README.md b/README.md index a4534ec..b372da7 100644 --- a/README.md +++ b/README.md @@ -50,9 +50,27 @@ Skipper can serve as your primary tool for your daily development tasks: --build-container-tag Tag of the build container --build-container-net Network to connect the build container (default: net=host) --env-file Environment variables file/s to pass to the container + --build-arg Set build-time variables for the container + --build-context Additional build contexts when running the build command, give them a name, and then access them inside a Dockerfile --help Show this message and exit. ``` +### [Build context explained](https://www.docker.com/blog/dockerfiles-now-support-multiple-build-contexts/) +Skipper allows you to add additional build contexts when running the build command, give them a name, and then access them inside a Dockerfile. +The build context can be one of the following: +* Local directory – e.g. `--build-context project2=../path/to/project2/src` +* Git repository – e.g. `--build-context qemu-src=https://github.com/qemu/qemu.git` +* HTTP URL to a tarball – e.g. `--build-context src=https://example.org/releases/src.tar` +* Docker image – Define with a `docker-image://` prefix, e.g. `--build-context alpine=docker-image://alpine:3.15` + +On the Dockerfile side, you can reference the build context on all commands that accept the “from” parameter. Here’s how that might look: + +```dockerfile +FROM [name] +COPY --from=[name] ... +RUN --mount=from=[name] … +``` + ### Build As a convention, skipper infers the docker images from the Dockerfiles in the top directory of your repository. For example, assuming that there are 3 Dockerfile in the top directory of the repository: ``` @@ -154,6 +172,16 @@ build-container-image: development build-container-tag: latest container-context: /path/to/context/dir +build-arg: + - VAR1=value1 + - VAR2=value2 + +build-context: + - context1=/path/to/context/dir # Local directory + - qemu-src=https://github.com/qemu/qemu.git # Remote git repository + - src=https://example.org/releases/src.tar # Remote tar file + - alpine=docker-image://alpine:3.15 # Remote docker image + make: makefile: Makefile.arm32 containers: @@ -164,6 +192,14 @@ env: env_file: path/to/env_file.env ``` +```yaml +# Use the git revision as the build container tag +# Allows to use the same build container unless the git revision changes +# This is useful when using a CI system that caches the build container +# Remember to commit if you changing the build container +build-container-tag: 'git:revision' +``` + Using the above configuration file, we now can run a simplified version of the make command described above: ```bash skipper make tests diff --git a/requirements.txt b/requirements.txt index 037601e..25d34b4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ PyYAML>=3.11 -click==6.7 +click>=6.7 requests>=2.6.0 tabulate>=0.7.5 six>=1.10.0 diff --git a/setup.cfg b/setup.cfg index 945107b..4bdc832 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,15 +1,15 @@ [metadata] name = strato-skipper author = Adir Gabai -author-email = adir@stratoscale.com -home-page = http://github.com/Stratoscale/skipper +author_email = adir@stratoscale.com +home_page = http://github.com/Stratoscale/skipper summary = Easily dockerize your Git repository license = Apache-2 long_description = file: README.md long_description_content_type = text/markdown -requires-dist = setuptools -requires-python = >=3 +requires_dist = setuptools +requires_python = >=3 [files] @@ -21,4 +21,4 @@ console_scripts = skipper = skipper.main:main [pep8] -max-line-length=145 +max_line_length=145 diff --git a/skipper.yaml b/skipper.yaml index a93b1a2..ba7bc9a 100644 --- a/skipper.yaml +++ b/skipper.yaml @@ -1 +1 @@ -build-container-image: skipper-build +build-container-image: skipper-build \ No newline at end of file diff --git a/skipper/builder.py b/skipper/builder.py new file mode 100644 index 0000000..0773287 --- /dev/null +++ b/skipper/builder.py @@ -0,0 +1,192 @@ +from dataclasses import dataclass +from logging import Logger +from typing import Callable + +from skipper import utils + +DOCKER_TAG_FOR_CACHE = "cache" + + +class Image: + """ + A class to represent an image with attributes like registry, name, tag, namespace. + """ + + def __init__(self, name, tag, dockerfile=None, registry=None, namespace=None): + """ + Constructs all the necessary attributes for the Image object. + + :param registry: Registry for the image + :param name: Name of the image + :param tag: Tag for the image + :param namespace: Namespace for the registry + """ + if not name: + raise ValueError("Image name is required") + + self.name = name + self.tag = tag + self.registry = registry + self.namespace = namespace + self.__dockerfile = dockerfile + self.__cache_fqdn = None + self.__fqdn = None + + def __str__(self): + return self.fqdn + + @property + def local(self): + """ + Creates a string that represents the local image. + + :return: Image in name:tag format + """ + if self.tag: + return self.name + ":" + self.tag + return self.name + + @property + def cache_fqdn(self): + """ + Generates a Fully Qualified Domain Name for the cached image. + + :return: Cached image Fully Qualified Domain Name + """ + if not self.__cache_fqdn: + self.__cache_fqdn = utils.generate_fqdn_image( + self.registry, self.namespace, self.name, DOCKER_TAG_FOR_CACHE + ) + return self.__cache_fqdn + + @property + def fqdn(self): + """ + Generates a Fully Qualified Domain Name for the image. + + :return: image Fully Qualified Domain Name + """ + if not self.__fqdn: + self.__fqdn = utils.generate_fqdn_image( + self.registry, self.namespace, self.name, self.tag + ) + return self.__fqdn + + @property + def dockerfile(self): + """ + Returns the Dockerfile for the image. + + :return: Dockerfile for the image + """ + if not self.__dockerfile: + self.__dockerfile = utils.image_to_dockerfile(self.name) + return self.__dockerfile + + @classmethod + def from_context_obj(cls, ctx_obj): + """ + Creates an instance of Image from a given context. + + :param ctx_obj: Click context object + :return: An instance of Image + """ + if ctx_obj is None: + return None + + return cls( + name=ctx_obj.get("build_container_image"), + tag=ctx_obj.get("build_container_tag"), + registry=ctx_obj.get("registry"), + ) + + +@dataclass +class BuildOptions: + """ + A class to encapsulate all the build options needed to create Docker image. + """ + + def __init__( + self, + image: Image, + container_context, + build_contexts=None, + build_args=None, + use_cache=False, + ): + """ + Constructs all the necessary attributes for the build options. + + :param image: image details as an instance of Image + :param container_context: Context for the build + :param build_contexts: Build contexts to add to build + :param build_args: Arguments to pass to build + :param use_cache: Boolean indicating if cache should be used + """ + self.image = image + self.container_context = container_context + self.build_contexts = [ctx for ctx in build_contexts if ctx] if build_contexts else [] + self.build_args = [arg for arg in build_args if arg] if build_args else [] + self.use_cache = use_cache + + @classmethod + def from_context_obj(cls, ctx_obj): + """ + Creates an instance of BuildOptions from a given context. + + :param ctx_obj: Click context object + :return: An instance of BuildOptions + """ + if ctx_obj is None: + return None + + return cls( + image=Image.from_context_obj(ctx_obj), + container_context=ctx_obj.get("container_context"), + build_contexts=ctx_obj.get("build_contexts"), + build_args=ctx_obj.get("build_args"), + use_cache=ctx_obj.get("use_cache"), + ) + + +def build(options: BuildOptions, runner: Callable, logger: Logger) -> int: + """ + Builds a image based on given build options and runner function. + + :param options: Build options as an instance of BuildOptions + :param runner: Callable that runs the Docker commands + :param logger: Logger instance + :return: A return code representing the success or failure of the build + """ + cmd = ["build", "--network=host"] + + for arg in options.build_args: + cmd += ["--build-arg", arg] + + for build_ctx in options.build_contexts: + cmd += ["--build-context", build_ctx] + + cmd += [ + "-f", + options.image.dockerfile, + "-t", + options.image.local, + options.container_context or ".", + ] + + if options.use_cache: + runner(["pull", options.image.cache_fqdn]) + cmd.extend(["--cache-from", options.image.cache_fqdn]) + + ret = runner(cmd) + + if ret != 0: + logger.error("Failed to build image: %s", options.image) + return ret + + if options.use_cache: + runner(["tag", options.image.name, options.image.cache_fqdn]) + runner(["push", options.image.cache_fqdn]) + + return 0 diff --git a/skipper/cli.py b/skipper/cli.py index c74a350..cd0ce1f 100644 --- a/skipper/cli.py +++ b/skipper/cli.py @@ -12,11 +12,10 @@ from pkg_resources import get_distribution from pbr import packaging -from skipper import git +from skipper import git, builder from skipper import runner from skipper import utils - -DOCKER_TAG_FOR_CACHE = "cache" +from skipper.builder import BuildOptions, Image def _validate_publish(ctx, param, value): @@ -54,6 +53,7 @@ def _validate_port_out_of_range(port): raise click.BadParameter(f"Invalid port number: port {port} is out of range") +# pylint: disable=too-many-arguments @click.group() @click.option('-v', '--verbose', help='Increase verbosity', is_flag=True, default=False) @click.option('--registry', help='URL of the docker registry') @@ -61,8 +61,20 @@ def _validate_port_out_of_range(port): @click.option('--build-container-tag', help='Tag of the build container') @click.option('--build-container-net', help='Network to connect the build container') @click.option('--env-file', multiple=True, help='Environment variable file(s) to load') +@click.option('--build-arg', multiple=True, help='Build arguments to pass to the container build', envvar='SKIPPER_BUILD_ARGS') +@click.option('--build-context', multiple=True, help='Build contexts to pass to the container build') @click.pass_context -def cli(ctx, registry, build_container_image, build_container_tag, build_container_net, verbose, env_file): +def cli( + ctx, + registry, + build_container_image, + build_container_tag, + build_container_net, + verbose, + env_file, + build_arg, + build_context, +): """ Easily dockerize your Git repository """ @@ -73,13 +85,15 @@ def cli(ctx, registry, build_container_image, build_container_tag, build_contain ctx.obj['build_container_image'] = build_container_image ctx.obj['build_container_net'] = build_container_net ctx.obj['git_revision'] = build_container_tag == 'git:revision' - ctx.obj['build_container_tag'] = git.get_hash() if ctx.obj['git_revision'] else build_container_tag + ctx.obj['build_container_tag'] = (git.get_hash() if ctx.obj['git_revision'] else build_container_tag) ctx.obj['env'] = ctx.default_map.get('env', {}) ctx.obj['containers'] = ctx.default_map.get('containers') ctx.obj['volumes'] = ctx.default_map.get('volumes') ctx.obj['workdir'] = ctx.default_map.get('workdir') ctx.obj['workspace'] = ctx.default_map.get('workspace', None) ctx.obj['container_context'] = ctx.default_map.get('container_context') + ctx.obj['build_args'] = build_arg + ctx.obj['build_contexts'] = build_context utils.set_remote_registry_login_info(registry, ctx.obj) @@ -94,50 +108,29 @@ def build(ctx, images_to_build, container_context, cache): """ utils.logger.debug("Executing build command") - valid_images = ctx.obj.get('containers') or utils.get_images_from_dockerfiles() - valid_images = {image: os.path.abspath(dockerfile) for image, dockerfile in six.iteritems(valid_images)} - valid_images_to_build = {} - if not images_to_build: - valid_images_to_build = valid_images - else: - for image in images_to_build: - if image not in valid_images: - utils.logger.warning('Image %s is not valid for this project! Skipping...', image) - continue - valid_images_to_build[image] = valid_images[image] + valid_images_to_build = _get_images_to_build(ctx, images_to_build) tag = git.get_hash() - for image, dockerfile in six.iteritems(valid_images_to_build): - utils.logger.info('Building image: %s', image) - - if not os.path.exists(dockerfile): - utils.logger.warning('File %s does not exist! Skipping...', dockerfile) - continue - - fqdn_image = image + ':' + tag - if container_context is not None: - build_context = container_context - elif ctx.obj['container_context']: - build_context = ctx.obj['container_context'] - else: - build_context = os.path.dirname(dockerfile) - command = ['build', '--network=host', '--build-arg', f'TAG={tag}', - '-f', dockerfile, '-t', fqdn_image, build_context] - if cache: - cache_image = utils.generate_fqdn_image(ctx.obj['registry'], namespace=None, image=image, tag=DOCKER_TAG_FOR_CACHE) - runner.run(['pull', cache_image]) - command.extend(['--cache-from', cache_image]) - ret = runner.run(command) - + build_args = (ctx.obj.get('build_args', ()) + (f'TAG={tag}',)) + build_contexts = ctx.obj.get('build_contexts', ()) + main_context = container_context or ctx.obj.get('container_context') + + for image, dockerfile in valid_images_to_build.items(): + utils.logger.info("Building image: %s", image) + + options = BuildOptions( + Image(name=image, tag=tag, dockerfile=dockerfile), + main_context or os.path.dirname(dockerfile), + build_contexts, + build_args, + cache, + ) + + ret = builder.build(options, runner.run, utils.logger) if ret != 0: - utils.logger.error('Failed to build image: %s', image) + utils.logger.error("Failed to build image: %s", options.image) return ret - if cache: - cache_image = utils.generate_fqdn_image(ctx.obj['registry'], namespace=None, image=image, tag=DOCKER_TAG_FOR_CACHE) - runner.run(['tag', fqdn_image, cache_image]) - runner.run(['push', cache_image]) - return 0 @@ -254,26 +247,29 @@ def run(ctx, interactive, name, env, publish, cache, command): """ utils.logger.debug("Executing run command") _validate_global_params(ctx, 'build_container_image') - build_container = _prepare_build_container(ctx.obj['registry'], - ctx.obj['build_container_image'], - ctx.obj['build_container_tag'], - ctx.obj['git_revision'], - ctx.obj['container_context'], - ctx.obj.get('username'), - ctx.obj.get('password'), - cache) - return runner.run(list(command), - fqdn_image=build_container, - environment=_expend_env(ctx, env), - interactive=interactive, - name=name, - net=ctx.obj['build_container_net'], - publish=publish, - volumes=ctx.obj.get('volumes'), - workdir=ctx.obj.get('workdir'), - use_cache=cache, - workspace=ctx.obj.get('workspace'), - env_file=ctx.obj.get('env_file')) + ctx.obj['use_cache'] = cache + + build_container = _prepare_build_container( + BuildOptions.from_context_obj(ctx.obj), + ctx.obj.get('git_revision'), + ctx.obj.get('username'), + ctx.obj.get('password'), + ) + + return runner.run( + list(command), + fqdn_image=build_container, + environment=_expend_env(ctx, env), + interactive=interactive, + name=name, + net=ctx.obj['build_container_net'], + publish=publish, + volumes=ctx.obj.get('volumes'), + workdir=ctx.obj.get('workdir'), + use_cache=cache, + workspace=ctx.obj.get('workspace'), + env_file=ctx.obj.get('env_file'), + ) @cli.command(context_settings={"ignore_unknown_options": True}) @@ -291,27 +287,30 @@ def make(ctx, interactive, name, env, makefile, cache, publish, make_params): """ utils.logger.debug("Executing make command") _validate_global_params(ctx, 'build_container_image') - build_container = _prepare_build_container(ctx.obj['registry'], - ctx.obj['build_container_image'], - ctx.obj['build_container_tag'], - ctx.obj['git_revision'], - ctx.obj['container_context'], - ctx.obj.get('username'), - ctx.obj.get('password'), - cache) + ctx.obj['use_cache'] = cache + + build_container = _prepare_build_container( + BuildOptions.from_context_obj(ctx.obj), + ctx.obj.get('git_revision'), + ctx.obj.get('username'), + ctx.obj.get('password'), + ) + command = ['make', '-f', makefile] + list(make_params) - return runner.run(command, - fqdn_image=build_container, - environment=_expend_env(ctx, env), - interactive=interactive, - name=name, - net=ctx.obj['build_container_net'], - publish=publish, - volumes=ctx.obj.get('volumes'), - workdir=ctx.obj.get('workdir'), - use_cache=cache, - workspace=ctx.obj.get('workspace'), - env_file=ctx.obj.get('env_file')) + return runner.run( + command, + fqdn_image=build_container, + environment=_expend_env(ctx, env), + interactive=interactive, + name=name, + net=ctx.obj['build_container_net'], + publish=publish, + volumes=ctx.obj.get('volumes'), + workdir=ctx.obj.get('workdir'), + use_cache=cache, + workspace=ctx.obj.get('workspace'), + env_file=ctx.obj.get('env_file'), + ) @cli.command() @@ -326,26 +325,29 @@ def shell(ctx, env, name, cache, publish): """ utils.logger.debug("Starting a shell") _validate_global_params(ctx, 'build_container_image') - build_container = _prepare_build_container(ctx.obj['registry'], - ctx.obj['build_container_image'], - ctx.obj['build_container_tag'], - ctx.obj['git_revision'], - ctx.obj['container_context'], - ctx.obj.get('username'), - ctx.obj.get('password'), - cache) - return runner.run(['bash'], - fqdn_image=build_container, - environment=_expend_env(ctx, env), - interactive=True, - name=name, - net=ctx.obj['build_container_net'], - publish=publish, - volumes=ctx.obj.get('volumes'), - workdir=ctx.obj.get('workdir'), - use_cache=cache, - workspace=ctx.obj.get('workspace'), - env_file=ctx.obj.get('env_file')) + ctx.obj['use_cache'] = cache + + build_container = _prepare_build_container( + BuildOptions.from_context_obj(ctx.obj), + ctx.obj.get('git_revision'), + ctx.obj.get('username'), + ctx.obj.get('password'), + ) + + return runner.run( + ['bash'], + fqdn_image=build_container, + environment=_expend_env(ctx, env), + interactive=True, + name=name, + net=ctx.obj['build_container_net'], + publish=publish, + volumes=ctx.obj.get('volumes'), + workdir=ctx.obj.get('workdir'), + use_cache=cache, + workspace=ctx.obj.get('workspace'), + env_file=ctx.obj.get('env_file'), + ) @cli.command() @@ -377,7 +379,12 @@ def _push_to_registry(registry, fqdn_image): sys.exit(ret) -def _prepare_build_container(registry, image, tag, git_revision, container_context, username, password, use_cache): +def _prepare_build_container( + options: BuildOptions, + git_revision: bool, + username: str, + password: str, +): def runner_run(command): """ All output generated by the container runtime during this stage should @@ -392,61 +399,34 @@ def runner_run(command): without having the build process output be included in their VERSION env var. """ + utils.logger.debug("Running command: %s", command) return runner.run(command, stdout_to_stderr=True) - if tag is not None: - - tagged_image_name = image + ':' + tag + image = options.image - if utils.local_image_exist(image, tag): - utils.logger.info("Using build container: %s", tagged_image_name) - return tagged_image_name + if image.tag: + if utils.local_image_exist(image.name, image.tag): + utils.logger.info('Using build container: %s', image.name) + return image.local - if utils.remote_image_exist(registry, image, tag, username, password): - fqdn_image = utils.generate_fqdn_image(registry, None, image, tag) - utils.logger.info("Using build container: %s", fqdn_image) - return fqdn_image + if image.registry and utils.remote_image_exist(image.registry, image.name, image.tag, username, password): + utils.logger.info('Using build container: %s', image.fqdn) + return image.fqdn if not git_revision: - raise click.exceptions.ClickException(f"Couldn't find build image {image} with tag {tag}") - + raise click.exceptions.ClickException(f"Couldn't find build image {image.name} with tag {image.tag}") else: - tagged_image_name = image - utils.logger.info("No build container tag was provided") + utils.logger.info('No build container tag was provided') - docker_file = utils.image_to_dockerfile(image) - if docker_file is None: - sys.exit(f'Could not find any dockerfile for {image}') + if not image.dockerfile: + sys.exit(f'Could not find any dockerfile for {image.name}') - utils.logger.info("Building image using docker file: %s", docker_file) - if container_context is not None: - build_context = container_context - else: - build_context = '.' + utils.logger.info('Building image using docker file: %s', image.dockerfile) - command = ['build', '--network=host', '-t', tagged_image_name, '-f', docker_file, build_context] - - for cmd_limit in utils.SKIPPER_ULIMIT: - command += cmd_limit - - if use_cache: - cache_image = utils.generate_fqdn_image(registry, namespace=None, image=image, tag=DOCKER_TAG_FOR_CACHE) - runner_run(['pull', cache_image]) - command.extend(['--cache-from', cache_image]) - - if runner_run(command) != 0: + if builder.build(options, runner_run, utils.logger) != 0: sys.exit(f'Failed to build image: {image}') - if git_revision and not git.uncommitted_changes(): - utils.logger.info("Tagging image with git revision: %s", tag) - runner_run(['tag', image, tagged_image_name]) - - if use_cache: - cache_image = utils.generate_fqdn_image(registry, namespace=None, image=image, tag=DOCKER_TAG_FOR_CACHE) - runner_run(['tag', image, cache_image]) - runner_run(['push', cache_image]) - - return image + return image.local def _validate_global_params(ctx, *params): @@ -483,3 +463,23 @@ def _expend_env(ctx, extra_env): raise TypeError(f'Type {type(env)} not supported for key env, use dict or list instead') return environment + list(extra_env) + + +def _get_images_to_build(ctx, images_to_build): + valid_images = ctx.obj.get('containers') or utils.get_images_from_dockerfiles() + valid_images = { + image: os.path.abspath(dockerfile) for image, dockerfile in valid_images.items() + } + valid_images_to_build = {} + if not images_to_build: + valid_images_to_build = valid_images + else: + for image in images_to_build: + if image not in valid_images: + utils.logger.warning("Image %s is not valid for this project! Skipping...", image) + continue + if not os.path.exists(valid_images[image]): + utils.logger.warning("Dockerfile %s does not exist! Skipping...", valid_images[image]) + continue + valid_images_to_build[image] = valid_images[image] + return valid_images_to_build diff --git a/skipper/config.py b/skipper/config.py index 8836afa..9470532 100644 --- a/skipper/config.py +++ b/skipper/config.py @@ -17,7 +17,7 @@ def load_defaults(): config = yaml.safe_load(confile) containers = config.pop('containers', None) _normalize_config(config, defaults) - if containers is not None: + if containers: defaults['containers'] = containers return defaults diff --git a/tests/consts.py b/tests/consts.py new file mode 100644 index 0000000..b0fecec --- /dev/null +++ b/tests/consts.py @@ -0,0 +1,7 @@ +REGISTRY = 'registry.io:5000' +IMAGE = 'image' +TAG = '1234567' +FQDN_IMAGE = REGISTRY + '/' + IMAGE + ':' + TAG +SKIPPER_CONF_BUILD_CONTAINER_IMAGE = 'skipper-conf-build-container-image' +SKIPPER_CONF_BUILD_CONTAINER_TAG = 'skipper-conf-build-container-tag' +SKIPPER_CONF_MAKEFILE = 'Makefile.skipper' diff --git a/tests/test_builder.py b/tests/test_builder.py new file mode 100644 index 0000000..849a3e7 --- /dev/null +++ b/tests/test_builder.py @@ -0,0 +1,216 @@ +import logging +from unittest import TestCase, mock + +from skipper import builder +from skipper.builder import BuildOptions, Image + + +class TestBuilder(TestCase): + def test_build_basic_usage(self): + """Testing the basic usage of the 'build' function.""" + + runner = mock.MagicMock() + runner.run.return_value = 0 + + options = BuildOptions( + image=Image( + name="test", + tag="test", + dockerfile="test", + ), + container_context=(), + ) + expected_cmd = [ + "build", + "--network=host", + "-f", + options.image.dockerfile, + "-t", + options.image.local, + ".", + ] + + result = builder.build(options, runner.run, logging.getLogger()) + self.assertEqual(0, result) + runner.run.assert_called_once_with(expected_cmd) + + def test_build_with_build_args(self): + """Testing the 'build' function with build args.""" + + runner = mock.MagicMock() + runner.run.return_value = 0 + + options = BuildOptions( + image=Image( + name="test", + tag="test", + dockerfile="test", + ), + container_context=(), + build_args=["test1", "test2"], + ) + expected_cmd = [ + "build", + "--network=host", + "--build-arg", + "test1", + "--build-arg", + "test2", + "-f", + options.image.dockerfile, + "-t", + options.image.local, + ".", + ] + + result = builder.build(options, runner.run, logging.getLogger()) + self.assertEqual(0, result) + runner.run.assert_called_once_with(expected_cmd) + + def test_build_with_build_contexts(self): + """Testing the 'build' function with build contexts.""" + + runner = mock.MagicMock() + runner.run.return_value = 0 + + options = BuildOptions( + image=Image( + name="test", + tag="test", + dockerfile="test", + ), + container_context=(), + build_contexts=["test1", "test2"], + ) + expected_cmd = [ + "build", + "--network=host", + "--build-context", + "test1", + "--build-context", + "test2", + "-f", + options.image.dockerfile, + "-t", + options.image.local, + ".", + ] + + result = builder.build(options, runner.run, logging.getLogger()) + self.assertEqual(0, result) + runner.run.assert_called_once_with(expected_cmd) + + def test_build_with_use_cache(self): + """Testing the 'build' function with use cache.""" + + runner = mock.MagicMock() + runner.run.return_value = 0 + + options = BuildOptions( + image=Image( + name="test", + tag="test", + dockerfile="test", + ), + container_context=(), + use_cache=True, + ) + expected_cmds = [ + mock.call( + [ + "build", + "--network=host", + "--cache-from", + options.image.cache_fqdn, + "-f", + options.image.dockerfile, + "-t", + options.image.local, + ".", + ] + ), + mock.call(["pull", options.image.cache_fqdn]), + mock.call(["tag", options.image.name, options.image.cache_fqdn]), + ] + + result = builder.build(options, runner.run, logging.getLogger()) + self.assertEqual(0, result) + runner.run.has_calls(expected_cmds) + + def test_build_with_options_from_context(self): + """Testing the 'build' function with options from context.""" + + runner = mock.MagicMock() + runner.run.return_value = 0 + + ctx_obj = { + "build_container_image": "test", + "build_container_tag": "test", + "container_context": (), + "build_contexts": ["test1", "test2"], + "build_args": ["test1", "test2"], + "use_cache": True, + } + expected_cmds = [ + mock.call( + [ + "build", + "--network=host", + "--cache-from", + "test:test", + "--build-arg", + "test1", + "--build-arg", + "test2", + "--build-context", + "test1", + "--build-context", + "test2", + "-f", + "test", + "-t", + "test:test", + ".", + ] + ), + mock.call(["pull", "test:test"]), + mock.call(["tag", "test", "test:test"]), + ] + + result = builder.build( + BuildOptions.from_context_obj(ctx_obj), runner.run, logging.getLogger() + ) + self.assertEqual(0, result) + runner.run.has_calls(expected_cmds) + + def test_build_fail(self): + """Testing the 'build' function when the build fails.""" + + runner = mock.MagicMock() + runner.run.return_value = 1 + + options = BuildOptions( + image=Image( + name="test", + tag="test", + dockerfile="test", + ), + container_context=(), + ) + + result = builder.build(options, runner.run, logging.getLogger()) + self.assertEqual(1, result) + runner.run.assert_called_once() + + def test_build_fail_without_image_name(self): + """Testing the 'build' function when the image name is not specified.""" + + ctx_obj = { + "build_container_tag": "test", + "container_context": (), + "build_contexts": ["test1", "test2"], + "build_args": ["test1", "test2"], + "use_cache": True, + } + + self.assertRaises(ValueError, BuildOptions.from_context_obj, ctx_obj) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9ac3b6b..b416c3a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5,16 +5,11 @@ import click import six from click import testing -from shutil import which from requests import HTTPError from skipper import cli from skipper import config, utils - -REGISTRY = 'registry.io:5000' -IMAGE = 'image' -TAG = '1234567' -FQDN_IMAGE = REGISTRY + '/' + IMAGE + ':' + TAG +from tests.consts import REGISTRY, SKIPPER_CONF_BUILD_CONTAINER_TAG, SKIPPER_CONF_MAKEFILE, SKIPPER_CONF_BUILD_CONTAINER_IMAGE, IMAGE, TAG BUILD_CONTAINER_IMAGE = 'build-container-image' BUILD_CONTAINER_TAG = 'build-container-tag' @@ -25,52 +20,42 @@ ENV_FILES = ['/home/envfile1.env', '/home/envfile2.env'] SKIPPER_CONF_CONTAINER_CONTEXT = '/some/context' -SKIPPER_CONF_BUILD_CONTAINER_IMAGE = 'skipper-conf-build-container-image' -SKIPPER_CONF_BUILD_CONTAINER_TAG = 'skipper-conf-build-container-tag' SKIPPER_CONF_BUILD_CONTAINER_FQDN_IMAGE = REGISTRY + '/' + SKIPPER_CONF_BUILD_CONTAINER_IMAGE + ':' + SKIPPER_CONF_BUILD_CONTAINER_TAG -SKIPPER_CONF_MAKEFILE = 'Makefile.skipper' SKIPPER_CONF = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, } } -CONFIG_ENV = { - "KEY2": "NOT_VAL2", - "KEY3": "VAL3", - "KEY4": "$VAL4", - "KEY5": "$$VAL5" -} -CONFIG_ENV_EVALUATION = { - "KEY2": "NOT_VAL2", - "KEY3": "VAL3", - "KEY4": "val4-evaluation", - "KEY5": "$VAL5" -} -SKIPPER_CONF_WITH_ENV = { + +SKIPPER_CONF_WITH_ENV_FILE = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, - 'env': CONFIG_ENV + 'env_file': [ENV_FILE_PATH] } -SKIPPER_CONF_WITH_ENV_FILE = { +SKIPPER_CONF_ENV = { + "KEY2": "NOT_VAL2", + "KEY3": "VAL3", +} +SKIPPER_CONF_WITH_ENV = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, - 'env_file': [ENV_FILE_PATH] + 'env': SKIPPER_CONF_ENV } SKIPPER_CONF_WITH_MULTIPLE_ENV_FILES = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, @@ -96,8 +81,8 @@ } SKIPPER_CONF_WITH_CONTAINERS = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, @@ -108,8 +93,8 @@ } SKIPPER_CONF_WITH_VOLUMES = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, @@ -121,8 +106,8 @@ SKIPPER_CONF_WITH_WORKDIR = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, @@ -130,8 +115,8 @@ } SKIPPER_CONF_WITH_WORKSPACE = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, @@ -140,8 +125,8 @@ SKIPPER_CONF_WITH_GIT_REV = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': 'git:revision', + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': 'git:revision', 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, @@ -149,48 +134,21 @@ SKIPPER_CONF_WITH_CONTEXT = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + 'build_container_tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, - 'container-context': SKIPPER_CONF_CONTAINER_CONTEXT + 'container_context': SKIPPER_CONF_CONTAINER_CONTEXT } SKIPPER_CONF_WITH_CONTEXT_NO_TAG = { 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'make': { - 'makefile': SKIPPER_CONF_MAKEFILE, - }, - 'container-context': SKIPPER_CONF_CONTAINER_CONTEXT -} - -SKIPPER_CONF_WITH_SHELL_INTERPOLATION = { - 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, - 'make': { - 'makefile': SKIPPER_CONF_MAKEFILE, - }, - 'volumes': [ - '$(which cat):/cat', - ], - 'env': [ - 'KEY=$(expr ${MY_NUMBER:-5} + 5)' - ] -} - -SKIPPER_CONF_WITH_INVALID_SHELL_INTERPOLATION = { - 'registry': REGISTRY, - 'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, - 'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG, + 'build_container_image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE, 'make': { 'makefile': SKIPPER_CONF_MAKEFILE, }, - 'volumes': [ - '$(bla bla):/cat', - ] + 'container_context': SKIPPER_CONF_CONTAINER_CONTEXT } @@ -236,7 +194,9 @@ def test_subcommand_without_global_params(self): subcmd_params=subcmd_params, ) self.assertIsInstance(result.exception, click.BadParameter) - self.assertEqual(result.exit_code, -1) + # since click testing module messes up exit code + # we just verify if the exit code is not 0 + self.assertNotEqual(0, result.exit_code) @mock.patch('skipper.runner.run', autospec=True) def test_subcommand_without_subcommand_params(self, skipper_runner_run_mock): @@ -298,9 +258,8 @@ def test_build_existing_image_with_context(self, skipper_runner_run_mock): return_value={ 'image1': '/home/user/work/project/Dockerfile.image1', 'image2': '/home/user/work/project/Dockerfile.image2'})) - @mock.patch('builtins.open', mock.MagicMock(create=True)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_CONTEXT)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_CONTEXT)) @mock.patch('skipper.git.get_hash', mock.MagicMock(autospec=True, return_value='1234567')) @mock.patch('skipper.runner.run', autospec=True, return_value=0) def test_build_with_context_from_config_file(self, skipper_runner_run_mock): @@ -320,9 +279,8 @@ def test_build_with_context_from_config_file(self, skipper_runner_run_mock): ] skipper_runner_run_mock.assert_called_once_with(expected_command) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_CONTEXT_NO_TAG)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_CONTEXT_NO_TAG)) @mock.patch('skipper.git.get_hash', mock.MagicMock(autospec=True, return_value='1234567')) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='')) @mock.patch('skipper.runner.run', autospec=True, return_value=0) @@ -339,9 +297,9 @@ def test_make_without_build_container_tag_with_context(self, skipper_runner_run_ ) expected_commands = [ mock.call(['build', '--network=host', - '-t', 'build-container-image', '-f', - 'Dockerfile.build-container-image', - SKIPPER_CONF_CONTAINER_CONTEXT, '--ulimit', 'nofile=65536:65536'], + '-f', 'Dockerfile.build-container-image', + '-t', 'build-container-image', + SKIPPER_CONF_CONTAINER_CONTEXT], stdout_to_stderr=True), mock.call(['make'] + make_params, fqdn_image='build-container-image', environment=[], interactive=False, name=None, net=None, publish=(), volumes=None, workdir=None, @@ -464,9 +422,8 @@ def test_build_all_images(self, skipper_runner_run_mock): return_value={ 'image1': '/home/user/work/project/Dockerfile.image1', 'image2': '/home/user/work/project/Dockerfile.image2'})) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) @mock.patch('skipper.git.get_hash', mock.MagicMock(autospec=True, return_value='1234567')) @mock.patch('skipper.runner.run', autospec=True, return_value=0) def test_build_with_defaults_from_config_file(self, skipper_runner_run_mock): @@ -485,11 +442,10 @@ def test_build_with_defaults_from_config_file(self, skipper_runner_run_mock): ] skipper_runner_run_mock.assert_called_once_with(expected_command) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.abspath', mock.MagicMock(autospec=True, return_value='/home/user/work/project/app1/Dockerfile')) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_CONTAINERS)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_CONTAINERS)) @mock.patch('skipper.git.get_hash', mock.MagicMock(autospec=True, return_value='1234567')) @mock.patch('skipper.runner.run', autospec=True, return_value=0) def test_build_with_defaults_from_config_file_including_containers(self, skipper_runner_run_mock): @@ -508,6 +464,72 @@ def test_build_with_defaults_from_config_file_including_containers(self, skipper ] skipper_runner_run_mock.assert_called_once_with(expected_command) + @mock.patch( + "skipper.utils.get_images_from_dockerfiles", + mock.MagicMock( + autospec=True, + return_value={"image1": "/home/user/work/project/Dockerfile.image1"}, + ), + ) + @mock.patch("os.path.exists", mock.MagicMock(autospec=True, return_value=True)) + @mock.patch( + "skipper.git.get_hash", mock.MagicMock(autospec=True, return_value="1234567") + ) + @mock.patch("skipper.runner.run", autospec=True, return_value=0) + def test_build_with_build_args(self, skipper_runner_run_mock): + build_params = ["image1"] + self._invoke_cli( + global_params=self.global_params + ["--build-arg", "key1=value1", "--build-arg", "key2=value2"], + subcmd="build", + subcmd_params=build_params, + ) + expected_commands = [ + "build", + "--network=host", + "--build-arg", + "key1=value1", + "--build-arg", + "key2=value2", + "--build-arg", + "TAG=1234567", + "-f", + "/home/user/work/project/Dockerfile.image1", + "-t", + "image1:1234567", + "/home/user/work/project", + ] + skipper_runner_run_mock.assert_called_once_with(expected_commands) + + @mock.patch( + "skipper.utils.get_images_from_dockerfiles", + mock.MagicMock( + autospec=True, + return_value={"image1": "/home/user/work/project/Dockerfile.image1"}, + ), + ) + @mock.patch("os.path.exists", mock.MagicMock(autospec=True, return_value=True)) + @mock.patch( + "skipper.git.get_hash", mock.MagicMock(autospec=True, return_value="1234567") + ) + @mock.patch("skipper.runner.run", autospec=True, return_value=0) + def test_build_with_build_contexts(self, skipper_runner_run_mock): + build_params = ["image1"] + self._invoke_cli( + global_params=self.global_params + ["--build-context", "context1=/path/to/context"], + subcmd="build", + subcmd_params=build_params, + ) + expected_commands = [ + "build", + "--network=host", + "--build-arg", "TAG=1234567", + "--build-context", "context1=/path/to/context", + "-f", "/home/user/work/project/Dockerfile.image1", + "-t", "image1:1234567", + "/home/user/work/project", + ] + skipper_runner_run_mock.assert_called_once_with(expected_commands) + @mock.patch('skipper.git.get_hash', mock.MagicMock(autospec=True, return_value='1234567')) @mock.patch('requests.get', autospec=True) @mock.patch('skipper.runner.run', autospec=True) @@ -680,9 +702,8 @@ def test_push_to_namespace(self, skipper_runner_run_mock, requests_get_mock): ] skipper_runner_run_mock.assert_has_calls(expected_commands) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) @mock.patch('skipper.git.get_hash', mock.MagicMock(autospec=True, return_value='1234567')) @mock.patch('requests.get', autospec=True) @mock.patch('skipper.runner.run', autospec=True) @@ -1133,9 +1154,8 @@ def test_run_with_non_existing_build_container(self, requests_get_mock): ) self.assertIsInstance(ret.exception, click.exceptions.ClickException) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) def test_run_with_defaults_from_config_file(self, skipper_runner_run_mock): @@ -1153,36 +1173,9 @@ def test_run_with_defaults_from_config_file(self, skipper_runner_run_mock): workdir=None, workspace=None, use_cache=False, env_file=()) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_ENV)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) - def test_run_with_defaults_and_env_from_config_file(self, skipper_runner_run_mock): - command = ['ls', '-l'] - run_params = command - os.environ['VAL4'] = "val4-evaluation" - self._invoke_cli( - defaults=config.load_defaults(), - subcmd='run', - subcmd_params=run_params - ) - 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=None, publish=(), - volumes=None, - workdir=None, workspace=None, use_cache=False, - env_file=()) - - @mock.patch('builtins.open', mock.MagicMock(create=True)) - @mock.patch('os.path.exists', - mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock( - autospec=True, return_value=SKIPPER_CONF_WITH_ENV_FILE)) - @mock.patch('subprocess.check_output', - mock.MagicMock(autospec=True, return_value='1234567\n')) - @mock.patch('skipper.runner.run', autospec=True) def test_run_with_defaults_and_env_from_env_file( self, skipper_runner_run_mock @@ -1190,7 +1183,7 @@ def test_run_with_defaults_and_env_from_env_file( command = ['ls', '-l'] run_params = command self._invoke_cli( - defaults=config.load_defaults(), + defaults=SKIPPER_CONF_WITH_ENV_FILE, subcmd='run', subcmd_params=run_params ) @@ -1206,11 +1199,8 @@ def test_run_with_defaults_and_env_from_env_file( use_cache=False, env_file=(ENV_FILE_PATH,)) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock( - autospec=True, return_value=SKIPPER_CONF_WITH_MULTIPLE_ENV_FILES)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) @@ -1221,7 +1211,7 @@ def test_run_with_defaults_and_env_from_multiple_env_file( command = ['ls', '-l'] run_params = command self._invoke_cli( - defaults=config.load_defaults(), + defaults=SKIPPER_CONF_WITH_MULTIPLE_ENV_FILES, subcmd='run', subcmd_params=run_params ) @@ -1237,21 +1227,18 @@ def test_run_with_defaults_and_env_from_multiple_env_file( use_cache=False, env_file=tuple(ENV_FILES)) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_ENV)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) def test_run_with_env_overriding_config_file(self, skipper_runner_run_mock): - os.environ['VAL4'] = "val4-evaluation" command = ['ls', '-l'] run_params = ['-e', ENV[0], '-e', ENV[1]] + command self._invoke_cli( - defaults=config.load_defaults(), + defaults=SKIPPER_CONF_WITH_ENV, subcmd='run', subcmd_params=run_params ) - env = ["%s=%s" % (key, value) for key, value in six.iteritems(CONFIG_ENV_EVALUATION)] + ENV + env = [f'{key}={value}' for key, value in six.iteritems(SKIPPER_CONF_ENV)] + 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=None, publish=(), @@ -1405,8 +1392,9 @@ def test_run_without_build_container_tag(self, skipper_runner_run_mock): subcmd_params=run_params ) expected_commands = [ - mock.call(['build', '--network=host', '-t', 'build-container-image', '-f', - 'Dockerfile.build-container-image', '.', '--ulimit', 'nofile=65536:65536'], + mock.call(['build', '--network=host', + '-f', 'Dockerfile.build-container-image', + '-t', 'build-container-image', '.'], stdout_to_stderr=True), mock.call(command, fqdn_image='build-container-image', environment=[], interactive=False, name=None, net=None, publish=(), volumes=None, workdir=None, workspace=None, @@ -1540,7 +1528,9 @@ def test_run_with_publish_textual_port(self): result = self._invoke_cli(global_params=global_params, subcmd='make', subcmd_params=make_params) self.assertIsInstance(result.exception, click.BadParameter) self.assertEqual("Publish need to be in format port:port or port-port:port-port", result.exception.message) - self.assertEqual(-1, result.exit_code) + # since click testing module messes up exit code + # we just verify if the exit code is not 0 + self.assertNotEqual(0, result.exit_code) def test_run_with_publish_textual_port_range(self): global_params = self.global_params @@ -1553,7 +1543,9 @@ def test_run_with_publish_textual_port_range(self): result = self._invoke_cli(global_params=global_params, subcmd='make', subcmd_params=make_params) self.assertIsInstance(result.exception, click.BadParameter) self.assertEqual("Publish need to be in format port:port or port-port:port-port", result.exception.message) - self.assertEqual(-1, result.exit_code) + # since click testing module messes up exit code + # we just verify if the exit code is not 0 + self.assertNotEqual(0, result.exit_code) def test_run_with_invalid_port_range(self): global_params = self.global_params @@ -1566,13 +1558,17 @@ def test_run_with_invalid_port_range(self): result = self._invoke_cli(global_params=global_params, subcmd='make', subcmd_params=make_params) self.assertIsInstance(result.exception, click.BadParameter) self.assertEqual("Invalid port range: 25 should be bigger than 15", result.exception.message) - self.assertEqual(-1, result.exit_code) + # since click testing module messes up exit code + # we just verify if the exit code is not 0 + self.assertNotEqual(0, result.exit_code) make_params = ['-p', '25-15:15-25', '-p', '12:12', '-f', makefile, target] result = self._invoke_cli(global_params=global_params, subcmd='make', subcmd_params=make_params) self.assertIsInstance(result.exception, click.BadParameter) self.assertEqual("Invalid port range: 25 should be bigger than 15", result.exception.message) - self.assertEqual(-1, result.exit_code) + # since click testing module messes up exit code + # we just verify if the exit code is not 0 + self.assertNotEqual(0, result.exit_code) def test_run_with_publish_out_of_range_port(self): global_params = self.global_params @@ -1585,11 +1581,12 @@ def test_run_with_publish_out_of_range_port(self): result = self._invoke_cli(global_params=global_params, subcmd='make', subcmd_params=make_params) self.assertIsInstance(result.exception, click.BadParameter) self.assertEqual("Invalid port number: port 121111111 is out of range", result.exception.message) - self.assertEqual(-1, result.exit_code) + # since click testing module messes up exit code + # we just verify if the exit code is not 0 + self.assertNotEqual(0, result.exit_code) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_VOLUMES)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_VOLUMES)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) def test_run_with_defaults_from_config_file_including_volumes(self, skipper_runner_run_mock): @@ -1606,40 +1603,8 @@ def test_run_with_defaults_from_config_file_including_volumes(self, skipper_runn volumes=['volume1', 'volume2'], workspace=None, workdir=None, use_cache=False, env_file=()) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_SHELL_INTERPOLATION)) - @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) - @mock.patch('skipper.runner.run', autospec=True) - def test_run_with_defaults_from_config_file_including_interpolated_volumes(self, skipper_runner_run_mock): - command = ['ls', '-l'] - run_params = command - self._invoke_cli( - defaults=config.load_defaults(), - subcmd='run', - subcmd_params=run_params - ) - 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=['KEY=10'], - interactive=False, name=None, net=None, publish=(), - volumes=[f'{which("cat")}:/cat'], workspace=None, - workdir=None, use_cache=False, env_file=()) - - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_INVALID_SHELL_INTERPOLATION)) - def test_run_with_defaults_from_config_file_including_invalid_interploated_volumes_interpolated(self): - command = ['ls', '-l'] - run_params = command - - with self.assertRaises(ValueError): - self._invoke_cli( - defaults=config.load_defaults(), - subcmd='run', - subcmd_params=run_params - ) - - @mock.patch('builtins.open', mock.MagicMock(create=True)) - @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_WORKDIR)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_WORKDIR)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) def test_run_with_defaults_from_config_file_including_workdir(self, skipper_runner_run_mock): @@ -1657,9 +1622,8 @@ def test_run_with_defaults_from_config_file_including_workdir(self, skipper_runn workdir='test-workdir', workspace=None, use_cache=False, env_file=()) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_WORKSPACE)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_WORKSPACE)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) def test_run_with_defaults_from_config_file_including_workspace(self, skipper_runner_run_mock): @@ -1677,9 +1641,8 @@ def test_run_with_defaults_from_config_file_including_workspace(self, skipper_ru workdir=None, workspace="/test/workspace", use_cache=False, env_file=()) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_GIT_REV)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_GIT_REV)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value=b'1234567\n')) @mock.patch('skipper.git.uncommitted_changes', mock.MagicMock(return_value=True)) @mock.patch('skipper.runner.run', autospec=True) @@ -1698,9 +1661,8 @@ def test_run_with_config_including_git_revision_with_uncommitted_changes(self, s workdir=None, use_cache=False, workspace=None, env_file=()) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_GIT_REV)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_GIT_REV)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value=b'1234567\n')) @mock.patch('skipper.git.uncommitted_changes', mock.MagicMock(return_value=False)) @mock.patch('skipper.runner.run', autospec=True) @@ -1774,9 +1736,8 @@ def test_make_with_additional_make_params(self, skipper_runner_run_mock): workdir=None, workspace=None, use_cache=False, env_file=()) - @mock.patch('builtins.open', mock.MagicMock(create=True)) @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True)) - @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) + @mock.patch('skipper.config.load_defaults', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF)) @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n')) @mock.patch('skipper.runner.run', autospec=True) def test_make_with_defaults_from_config_file(self, skipper_runner_run_mock): @@ -1811,8 +1772,8 @@ def test_make_without_build_container_tag(self, skipper_runner_run_mock): subcmd_params=make_params ) expected_commands = [ - mock.call(['build', '--network=host', '-t', 'build-container-image', '-f', - 'Dockerfile.build-container-image', '.', '--ulimit', 'nofile=65536:65536'], + mock.call(['build', '--network=host', '-f', 'Dockerfile.build-container-image', + '-t', 'build-container-image', '.'], stdout_to_stderr=True), mock.call(['make'] + make_params, fqdn_image='build-container-image', environment=[], interactive=False, name=None, net=None, publish=(), volumes=None, workdir=None, workspace=None, diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..a3520be --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,120 @@ +import json +import os +from shutil import which +from unittest import TestCase, mock + +from skipper import config +from tests.consts import ( + REGISTRY, + SKIPPER_CONF_BUILD_CONTAINER_TAG, + SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + SKIPPER_CONF_MAKEFILE, +) + +ENV_FILE_PATH = "/home/envfile.env" +CONFIG_ENV = {"KEY2": "NOT_VAL2", "KEY3": "VAL3", "KEY4": "$VAL4", "KEY5": "$$VAL5"} + +CONFIG_ENV_EVALUATION = { + "KEY2": "NOT_VAL2", + "KEY3": "VAL3", + "KEY4": "val4-evaluation", + "KEY5": "$VAL5", +} + +SKIPPER_CONF_WITH_ENV = json.dumps( + { + "registry": REGISTRY, + "build-container-image": SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + "build-container-tag": SKIPPER_CONF_BUILD_CONTAINER_TAG, + "make": { + "makefile": SKIPPER_CONF_MAKEFILE, + }, + "env": CONFIG_ENV, + } +) + +SKIPPER_CONF_WITH_ENV_FILE = json.dumps( + { + "registry": REGISTRY, + "build-container-image": SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + "build-container-tag": SKIPPER_CONF_BUILD_CONTAINER_TAG, + "make": { + "makefile": SKIPPER_CONF_MAKEFILE, + }, + "env_file": [ENV_FILE_PATH], + } +) + +SKIPPER_CONF_WITH_SHELL_INTERPOLATION = json.dumps( + { + "registry": REGISTRY, + "build-container-image": SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + "build-container-tag": SKIPPER_CONF_BUILD_CONTAINER_TAG, + "make": { + "makefile": SKIPPER_CONF_MAKEFILE, + }, + "volumes": [ + "$(which cat):/cat", + ], + "env": ["KEY=$(expr ${MY_NUMBER:-5} + 5)"], + } +) + +SKIPPER_CONF_WITH_INVALID_SHELL_INTERPOLATION = json.dumps( + { + "registry": REGISTRY, + "build_container_image": SKIPPER_CONF_BUILD_CONTAINER_IMAGE, + "build_container_tag": SKIPPER_CONF_BUILD_CONTAINER_TAG, + "make": { + "makefile": SKIPPER_CONF_MAKEFILE, + }, + "volumes": [ + "$(bla bla):/cat", + ], + } +) + + +class TestConfig(TestCase): + @mock.patch( + "builtins.open", + mock.MagicMock(side_effect=mock.mock_open(read_data=SKIPPER_CONF_WITH_ENV)), + ) + @mock.patch("os.path.exists", mock.MagicMock(autospec=True, return_value=True)) + def test_config_with_env_eval(self): + os.environ["VAL4"] = "val4-evaluation" + defaults = config.load_defaults() + + self.assertEqual(CONFIG_ENV_EVALUATION, defaults.get("env")) + self.assertEqual( + defaults.get("build_container_image"), SKIPPER_CONF_BUILD_CONTAINER_IMAGE + ) + self.assertEqual( + defaults.get("build_container_tag"), SKIPPER_CONF_BUILD_CONTAINER_TAG + ) + + @mock.patch( + "builtins.open", + mock.MagicMock( + side_effect=mock.mock_open(read_data=SKIPPER_CONF_WITH_SHELL_INTERPOLATION) + ), + ) + @mock.patch("os.path.exists", mock.MagicMock(autospec=True, return_value=True)) + def test_config_with_interpolation(self): + defaults = config.load_defaults() + + self.assertEqual(defaults.get("env"), ["KEY=10"]) + self.assertEqual(defaults.get("volumes"), [f'{which("cat")}:/cat']) + + @mock.patch( + "builtins.open", + mock.MagicMock( + side_effect=mock.mock_open( + read_data=SKIPPER_CONF_WITH_INVALID_SHELL_INTERPOLATION + ) + ), + ) + @mock.patch("os.path.exists", mock.MagicMock(autospec=True, return_value=True)) + def test_config_with_wrong_interpolation(self): + with self.assertRaises(ValueError): + config.load_defaults() diff --git a/tests/test_utils.py b/tests/test_utils.py index 6c8ed6b..c990d4b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -13,18 +13,18 @@ def test_get_runtime_command(self, find_executable_mock): utils.CONTAINER_RUNTIME_COMMAND = None find_executable_mock.side_effect = "done" res = utils.get_runtime_command() - self.assertEquals(res, utils.DOCKER) + self.assertEqual(res, utils.DOCKER) find_executable_mock.side_effect = [None, "done"] utils.CONTAINER_RUNTIME_COMMAND = None res = utils.get_runtime_command() - self.assertEquals(res, utils.PODMAN) + self.assertEqual(res, utils.PODMAN) with self.assertRaises(Exception): find_executable_mock.side_effect = [None, None] utils.CONTAINER_RUNTIME_COMMAND = None utils.get_runtime_command() utils.CONTAINER_RUNTIME_COMMAND = utils.DOCKER res = utils.get_runtime_command() - self.assertEquals(res, utils.DOCKER) + self.assertEqual(res, utils.DOCKER) @mock.patch('skipper.utils.open', autospec=False) @mock.patch('skipper.utils.os.makedirs', autospec=True)