Docker implementation #1219
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,156 @@ | ||||||||
| """Logic for creating a Dockerfile and/or building portable Docker images from Constructor installers.""" | ||||||||
|
|
||||||||
| import logging | ||||||||
| import shutil | ||||||||
| import subprocess | ||||||||
| import tempfile | ||||||||
| from pathlib import Path | ||||||||
|
|
||||||||
| from jinja2 import Template | ||||||||
|
|
||||||||
| from . import __version__ | ||||||||
|
|
||||||||
| logger = logging.getLogger(__name__) | ||||||||
|
|
||||||||
| TEMPLATE_PATH = Path(__file__).parent / "dockerfile_template.tmpl" | ||||||||
|
|
||||||||
| DOCKER_PLATFORM_MAP = { | ||||||||
| "linux-64": "linux/amd64", | ||||||||
| "linux-aarch64": "linux/arm64", | ||||||||
| "linux-armv7l": "linux/arm/v7", | ||||||||
| "linux-32": "linux/386", | ||||||||
| "linux-ppc64le": "linux/ppc64le", | ||||||||
| "linux-s390x": "linux/s390x", | ||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| def generate_dockerfile(info: dict, docker_dir: Path) -> Path: | ||||||||
| """ | ||||||||
| Render the Dockerfile template and write it to the Docker build directory. | ||||||||
|
|
||||||||
| Parameters | ||||||||
| ---------- | ||||||||
| info: dict | ||||||||
| Constructor installer info dict. | ||||||||
| docker_dir: Path | ||||||||
| Path to the Docker build directory returned by prepare_docker_context(). | ||||||||
|
|
||||||||
| Returns | ||||||||
| ------- | ||||||||
| Path | ||||||||
| Path to the generated Dockerfile. | ||||||||
| """ | ||||||||
| from .conda_interface import MatchSpec | ||||||||
|
|
||||||||
| specs = {MatchSpec(spec).name for spec in info.get("specs", ())} | ||||||||
|
Contributor
There was a problem hiding this comment.
Suggested change
That will make things more clear later. Should probably also be done with |
||||||||
| has_mamba = "mamba" in specs | ||||||||
|
|
||||||||
| docker_template = Template(TEMPLATE_PATH.read_text()) | ||||||||
|
|
||||||||
| rendered_dockerfile = docker_template.render( | ||||||||
| constructor_version=__version__, | ||||||||
| base_image=info.get("docker_base_image"), | ||||||||
| default_prefix=info.get("default_prefix", f"/opt/{info['name'].lower()}"), | ||||||||
|
Contributor
There was a problem hiding this comment. I think this is the actual prefix, is it not?
Contributor
Author
There was a problem hiding this comment. I'm intentionally being explicit here since in the schema we have:
Contributor
There was a problem hiding this comment. I would still use the template variable name |
||||||||
| installer_filename=Path(info["_outpath"]).name, | ||||||||
| name=info["name"], | ||||||||
| version=info["version"], | ||||||||
| labels=info.get("docker_labels", {}), | ||||||||
| init_cmd="$PREFIX/bin/mamba shell" if has_mamba else "$PREFIX/bin/python -m conda", | ||||||||
| register_envs=info.get("register_envs"), | ||||||||
| keep_pkgs=info.get("keep_pkgs"), | ||||||||
| ) | ||||||||
|
|
||||||||
| logger.info("Writing Dockerfile...") | ||||||||
| dockerfile_path = docker_dir / "Dockerfile" | ||||||||
| dockerfile_path.write_text(rendered_dockerfile) | ||||||||
| return dockerfile_path | ||||||||
|
|
||||||||
|
|
||||||||
| def build_image(info: dict, docker_dir: Path) -> Path: | ||||||||
| """Optionally build the docker image from the generated Dockerfile. | ||||||||
| Currently supported on linux and macOS platforms. | ||||||||
|
|
||||||||
| Parameters | ||||||||
| ---------- | ||||||||
| info: dict | ||||||||
| Constructor installer info dict. | ||||||||
| docker_dir: Path | ||||||||
| Path to the Docker directory containing the Docker outputs. | ||||||||
|
|
||||||||
| Returns | ||||||||
| ------- | ||||||||
| Path | ||||||||
| Path to the saved Docker image tarball. | ||||||||
|
|
||||||||
| """ | ||||||||
| if not (docker_platform := DOCKER_PLATFORM_MAP.get(info["_platform"])): | ||||||||
| raise RuntimeError( | ||||||||
| f"Unsupported platform for Docker build: {info['_platform']}" | ||||||||
| f"Supported platforms are: {', '.join(DOCKER_PLATFORM_MAP.keys())}" | ||||||||
| ) | ||||||||
|
|
||||||||
| if info.get("docker_tag") and ":" not in info.get("docker_tag"): | ||||||||
| raise ValueError( | ||||||||
| "Invalid docker_tag: '{info['docker_tag']}'. Must be in format 'name:tag'." | ||||||||
| ) | ||||||||
|
|
||||||||
| tag = info.get("docker_tag", f"{info['name'].lower()}:{info['version']}") | ||||||||
| tarball_dest = docker_dir / f"{Path(info['_outpath']).stem}-docker.tar" | ||||||||
|
|
||||||||
| cmd = [ | ||||||||
| "docker", | ||||||||
| "buildx", | ||||||||
| "build", | ||||||||
| str(docker_dir), | ||||||||
| "--platform", | ||||||||
| docker_platform, | ||||||||
| "-t", | ||||||||
| tag, | ||||||||
|
Comment on lines
+107
to
+108
Contributor
There was a problem hiding this comment. Here is where I'm wondering if we need the tag - can we also retrieve the Docker image based on the emitted SHA?
Contributor
Author
There was a problem hiding this comment. I'm sorry I'm not understanding the ask here. If we do not provide a tag, the image would show up like
Contributor
Author
There was a problem hiding this comment. Are you thinking we use the SHA to pass into docker save instead of the tag?
Contributor
There was a problem hiding this comment. The tag right now is mandatory
Contributor
Author
There was a problem hiding this comment. No, it is not required to work, but we most definitely should use a tag. When someone tries to run the image, one of the easiest ways to do that is to use the tag. If we don't provide a tag, there would be a
Contributor
There was a problem hiding this comment.
I'm not sure I understand where the The tag provides some predictability to the code though and an untagged image requires On a very different note: as I was building my own images, |
||||||||
| "--load", | ||||||||
| ] | ||||||||
|
|
||||||||
| logger.info("Building Docker image: '%s'", tag) | ||||||||
| subprocess.run(cmd, check=True) | ||||||||
|
|
||||||||
| logger.info("Saving Docker image to tarball: '%s'", tarball_dest) | ||||||||
| subprocess.run(["docker", "save", tag, "-o", str(tarball_dest)], check=True) | ||||||||
| return tarball_dest | ||||||||
|
|
||||||||
|
|
||||||||
| def create(info: dict, verbose: bool = False) -> None: | ||||||||
| """Build a Docker output | ||||||||
|
|
||||||||
| Parameters | ||||||||
| ---------- | ||||||||
| info: dict | ||||||||
| Constructor installer info dict. | ||||||||
| verbose: bool, optional | ||||||||
| If ``True``, enables verbose logging. | ||||||||
| Defaults to ``False``. | ||||||||
|
|
||||||||
| """ | ||||||||
| with tempfile.TemporaryDirectory() as temp_dir: | ||||||||
| docker_tmp_dir = Path(temp_dir) | ||||||||
|
|
||||||||
| installer_path = Path(info["_outpath"]) | ||||||||
| if not installer_path.exists(): | ||||||||
| raise RuntimeError(f"Expected .sh installer not found: {installer_path}") | ||||||||
| shutil.copy(installer_path, docker_tmp_dir / installer_path.name) | ||||||||
| logger.info("Copied installer to build directory.") | ||||||||
|
|
||||||||
| generate_dockerfile(info, docker_tmp_dir) | ||||||||
|
|
||||||||
| if info.get("docker_image") == "tar": | ||||||||
| tarball = build_image(info, docker_tmp_dir) | ||||||||
| if tarball: | ||||||||
| shutil.copy(tarball, Path(info["_output_dir"]) / tarball.name) | ||||||||
| else: | ||||||||
| output_dir = Path(info["_output_dir"]) / installer_path.stem | ||||||||
| output_dir.mkdir(parents=True, exist_ok=True) | ||||||||
| shutil.copy(docker_tmp_dir / "Dockerfile", output_dir / "Dockerfile") | ||||||||
| shutil.copy( | ||||||||
| docker_tmp_dir / Path(info["_outpath"]).name, | ||||||||
| output_dir / Path(info["_outpath"]).name, | ||||||||
| ) | ||||||||
|
|
||||||||
| logger.info("Docker output complete. Docker directory: '%s'", info["_output_dir"]) | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Dockerfile generated by constructor {{ constructor_version }} | ||
|
|
||
| ########################################################## | ||
| # Stage 1: Run .sh installer | ||
| ########################################################## | ||
|
|
||
| FROM {{ base_image }} AS builder | ||
|
|
||
| ARG PREFIX={{ default_prefix }} | ||
|
|
||
| COPY {{ installer_filename }} /tmp/installer.sh | ||
|
|
||
| RUN sh /tmp/installer.sh -b -p "${PREFIX}" && \ | ||
| rm -f "${PREFIX}/uninstall.sh" && \ | ||
| rm -f "${PREFIX}/_conda" && \ | ||
| find "${PREFIX}" -follow -type f -name '*.js.map' -delete && \ | ||
| find "${PREFIX}" -name '__pycache__' -type d -exec rm -rf {} + 2>/dev/null || true && \ | ||
| {%- if not keep_pkgs %} | ||
| rm -rf "${PREFIX}/pkgs" && \ | ||
| {%- endif %} | ||
| find "${PREFIX}" -follow -type f -name '*.a' -delete | ||
|
|
||
| ########################################################## | ||
| # Stage 2: Final image | ||
| ########################################################## | ||
|
|
||
| FROM {{ base_image }} | ||
|
|
||
| ARG PREFIX={{ default_prefix }} | ||
|
|
||
| LABEL org.opencontainers.image.title="{{ name }}" | ||
| LABEL org.opencontainers.image.version="{{ version }}" | ||
| {%- for k, v in labels.items() %} | ||
| LABEL {{ k }}="{{ v }}" | ||
| {%- endfor %} | ||
|
|
||
| ENV LANG=C.UTF-8 | ||
| ENV LC_ALL=C.UTF-8 | ||
| ENV PATH="${PREFIX}/bin:${PATH}" | ||
|
|
||
| COPY --from=builder ${PREFIX} ${PREFIX} | ||
| {% if register_envs %} | ||
| COPY --from=builder /root/.conda /root/.conda | ||
| {% endif %} | ||
|
|
||
| RUN echo 'export PATH=$(sed -e "s,:\?{{ default_prefix }}/bin:,," <<< "${PATH}")' >> ~/.bashrc && \ | ||
| / {{ init_cmd }} init --all | ||
|
|
||
| CMD [ "/bin/bash" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of prefixing all fields with
docker_*, would it make more sense to makedockerits own key with all other properties being a subkey? I know that's not our current convention, so I'm not 100% sure about that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it could be cleaner, but unless we can do a broader refactor, I think docker_* keeps things consistent with how it's handled today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, please volunteer me if/when we can do a schema refactor. (:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sticking with the current schema is a fair choice. A schema refactor would constitute a major breaking change that I don't foresee ourselves doing unless we absolutely have to.