diff --git a/repo2docker/__main__.py b/repo2docker/__main__.py index fb4e7846..62d67f02 100644 --- a/repo2docker/__main__.py +++ b/repo2docker/__main__.py @@ -140,7 +140,8 @@ def get_argparser(): argparser.add_argument( "--build-memory-limit", - help="Total Memory that can be used by the docker build process", + # Removed argument, but we still want to support printing an error message if this is passed + help=argparse.SUPPRESS, ) argparser.add_argument( @@ -434,12 +435,13 @@ def make_r2d(argv=None): sys.exit(1) if args.build_memory_limit: - # if the string only contains numerals we assume it should be an int - # and specifies a size in bytes - if args.build_memory_limit.isnumeric(): - r2d.build_memory_limit = int(args.build_memory_limit) - else: - r2d.build_memory_limit = args.build_memory_limit + # We no longer support build_memory_limit, it must be set in the builder instance + print("--build-memory-limit is no longer supported", file=sys.stderr) + print( + "Check your build engine documentation to set memory limits. Use `docker buildx create` if using the default builder to create a custom builder with appropriate memory limits", + file=sys.stderr, + ) + sys.exit(-1) if args.environment and not r2d.run: print("To specify environment variables, you also need to run " "the container") diff --git a/repo2docker/app.py b/repo2docker/app.py index a2a9a92d..0d817452 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -165,13 +165,23 @@ def _default_log_level(self): build_memory_limit = ByteSpecification( 0, help=""" - Total memory that can be used by the docker image building process. + Unsupported. - Set to 0 for no limits. + When using docker, please use `docker buildx create` to create a new buildkit + builder with appropriate limits instead. """, config=True, ) + @observe("build_memory_limit") + def build_memory_limit_changed(self, change): + print("Setting build_memory_limit is not supported", file=sys.stderr) + print( + "Check your build engine documentation to set memory limits. Use `docker buildx create` if using the default builder to create a custom builder with appropriate memory limits", + file=sys.stderr, + ) + sys.exit(-1) + volumes = Dict( {}, help=""" @@ -856,6 +866,7 @@ def build(self): for l in picked_buildpack.build( docker_client, self.output_image_spec, + # This is deprecated, but passing it anyway to not break backwards compatibility self.build_memory_limit, build_args, self.cache_from, diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 830e5c78..7fcbc31e 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -2,12 +2,17 @@ Docker container engine for repo2docker """ +import shutil +import tarfile +import tempfile + from iso8601 import parse_date -from traitlets import Dict +from traitlets import Dict, List, Unicode import docker from .engine import Container, ContainerEngine, ContainerEngineException, Image +from .utils import execute_cmd class DockerContainer(Container): @@ -53,7 +58,7 @@ class DockerEngine(ContainerEngine): https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build """ - string_output = False + string_output = True extra_init_args = Dict( {}, @@ -69,6 +74,14 @@ class DockerEngine(ContainerEngine): config=True, ) + extra_buildx_build_args = List( + Unicode(), + help=""" + Extra commandline arguments to pass to `docker buildx build` when building the image. + """, + config=True, + ) + def __init__(self, *, parent): super().__init__(parent=parent) try: @@ -94,22 +107,46 @@ def build( platform=None, **kwargs, ): - return self._apiclient.build( - buildargs=buildargs, - cache_from=cache_from, - container_limits=container_limits, - forcerm=True, - rm=True, - tag=tag, - custom_context=custom_context, - decode=True, - dockerfile=dockerfile, - fileobj=fileobj, - path=path, - labels=labels, - platform=platform, - **kwargs, - ) + if not shutil.which("docker"): + raise RuntimeError("The docker commandline client must be installed") + args = ["docker", "buildx", "build", "--progress", "plain", "--load"] + if buildargs: + for k, v in buildargs.items(): + args += ["--build-arg", f"{k}={v}"] + + if cache_from: + for cf in cache_from: + args += ["--cache-from", cf] + + if dockerfile: + args += ["--file", dockerfile] + + if tag: + args += ["--tag", tag] + + if labels: + for k, v in labels.items(): + args += ["--label", f"{k}={v}"] + + if platform: + args += ["--platform", platform] + + # place extra args right *before* the path + args += self.extra_buildx_build_args + + if fileobj: + with tempfile.TemporaryDirectory() as d: + tarf = tarfile.open(fileobj=fileobj) + tarf.extractall(d) + + args += [d] + + yield from execute_cmd(args, True) + else: + # Assume 'path' is passed in + args += [path] + + yield from execute_cmd(args, True) def images(self): images = self._apiclient.images() diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index d37103e6..0ee4d0b3 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -78,19 +78,18 @@ def test_local_dir_image_name(repo_with_content): ) -def test_build_kwargs(repo_with_content): +def test_extra_buildx_build_args(repo_with_content): upstream, sha1 = repo_with_content - argv = [upstream] + argv = ["--DockerEngine.extra_buildx_build_args=--check", upstream] app = make_r2d(argv) - app.extra_build_kwargs = {"somekey": "somevalue"} - - with patch.object(docker.APIClient, "build") as builds: - builds.return_value = [] + with patch("repo2docker.docker.execute_cmd") as execute_cmd: app.build() - builds.assert_called_once() - args, kwargs = builds.call_args - assert "somekey" in kwargs - assert kwargs["somekey"] == "somevalue" + + args, kwargs = execute_cmd.call_args + cmd = args[0] + assert cmd[:3] == ["docker", "buildx", "build"] + # make sure it's inserted before the end + assert "--check" in cmd[:-1] def test_run_kwargs(repo_with_content): @@ -107,26 +106,6 @@ def test_run_kwargs(repo_with_content): assert kwargs["somekey"] == "somevalue" -def test_root_not_allowed(): - with TemporaryDirectory() as src, patch("os.geteuid") as geteuid: - geteuid.return_value = 0 - argv = [src] - with pytest.raises(SystemExit) as exc: - app = make_r2d(argv) - assert exc.code == 1 - - with pytest.raises(ValueError): - app = Repo2Docker(repo=src, run=False) - app.build() - - app = Repo2Docker(repo=src, user_id=1000, user_name="jovyan", run=False) - app.initialize() - with patch.object(docker.APIClient, "build") as builds: - builds.return_value = [] - app.build() - builds.assert_called_once() - - def test_dryrun_works_without_docker(tmpdir, capsys): with chdir(tmpdir): with patch.object(docker, "APIClient") as client: diff --git a/tests/unit/test_args.py b/tests/unit/test_args.py index d3e304f0..a116a988 100644 --- a/tests/unit/test_args.py +++ b/tests/unit/test_args.py @@ -50,11 +50,11 @@ def test_mem_limit(): """ Test various ways of passing --build-memory-limit """ - r2d = make_r2d(["--build-memory-limit", "1024", "."]) - assert int(r2d.build_memory_limit) == 1024 + with pytest.raises(SystemExit): + r2d = make_r2d(["--build-memory-limit", "1024", "."]) - r2d = make_r2d(["--build-memory-limit", "3K", "."]) - assert int(r2d.build_memory_limit) == 1024 * 3 + with pytest.raises(SystemExit): + r2d = make_r2d(["--Repo2Docker.build_memory_limit", "1024", "."]) def test_run_required():