-
Notifications
You must be signed in to change notification settings - Fork 178
Docker implementation #1219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Docker implementation #1219
Changes from 59 commits
81dfce0
749abd4
d60e4b3
c3524ba
df67edf
b71c8f9
ec455fa
1fd5f98
5028672
487d802
c46044a
ae648a5
539500c
43ca213
0bf8de5
9f20cbe
65f9fd8
4fd0868
8b10804
01a9982
d86ee52
5784073
b2ff9d5
f97e163
15a8b1f
a5704bb
8db9e96
6d1d118
f65ecf3
989fb30
7338b72
120d135
be48dd1
18b7001
583191d
b047963
80ee0a7
2dbe4eb
c1876f5
c0ab7cb
963126c
40475f9
9fea3d4
1720895
8735a9a
56957c2
ceb9a3c
511ade6
c10fe4d
f50394f
9ba611f
c0b7fd3
3fec340
15413f2
1b534dc
6998d3d
c0e7735
2a6c4cc
0b5ce84
e75758d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -42,6 +42,7 @@ class InstallerTypes(StrEnum): | |||||
| EXE = "exe" | ||||||
| PKG = "pkg" | ||||||
| SH = "sh" | ||||||
| DOCKER = "docker" | ||||||
|
|
||||||
|
|
||||||
| class PkgDomains(StrEnum): | ||||||
|
|
@@ -403,6 +404,7 @@ class ConstructorConfiguration(BaseModel): | |||||
| - `sh`: shell-based installer for Linux or macOS | ||||||
| - `pkg`: macOS GUI installer built with Apple's `pkgbuild` | ||||||
| - `exe`: Windows GUI installer built with NSIS | ||||||
| - `docker`: generates a `Dockerfile` or image of the installation environment. | ||||||
|
|
||||||
| The default type is `sh` on Linux and macOS, and `exe` on Windows. A special | ||||||
| value of `all` builds _both_ `sh` and `pkg` installers on macOS, as well | ||||||
|
|
@@ -588,7 +590,7 @@ class ConstructorConfiguration(BaseModel): | |||||
| this is used only for "Just Me" installations; for "All Users" installations, | ||||||
| use the `default_prefix_all_users` key. If not provided, the default prefix | ||||||
| is `%USERPROFILE%\\<NAME>`. Environment variables will be expanded at | ||||||
| install time. | ||||||
| install time. If creating a Docker output, the default is `/opt/<NAME>` and can be overridden during the Docker build process. | ||||||
| """ | ||||||
| default_prefix_domain_user: NonEmptyStr | None = None | ||||||
| """ | ||||||
|
|
@@ -853,6 +855,29 @@ class ConstructorConfiguration(BaseModel): | |||||
| message: "This base environment is frozen and cannot be modified." | ||||||
| ``` | ||||||
| """ | ||||||
| docker_base_image: Annotated[str, Field(min_length=1)] | None = None | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of prefixing all fields with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, please volunteer me if/when we can do a schema refactor. (:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||||||
| """ | ||||||
| Required to use docker-related features. | ||||||
| Base image to use for docker builds and Dockerfiles. This can be any valid docker image reference, including a tag and/or digest. | ||||||
| For example: `debian:13.4-slim@sha256:abc123...`. | ||||||
| """ | ||||||
| docker_tag: NonEmptyStr | None = None | ||||||
| """ | ||||||
| Tag to use for the docker image. | ||||||
| If not provided, it will default to `<name>:<version>`. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to provide a default here? Will this require
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should. It's handled in the script, but I could add something like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually wrote the opposite of what I meant to write:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify, it is highly suggested that a tag be used with docker build. The only thing is that it is not required for the user to come up with that tag because it will default to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if there's a limit on characters for the tag name, but the latter is definitely more descriptive. 😄 |
||||||
| Has no effect if not using the `docker_image` feature. | ||||||
| """ | ||||||
| docker_labels: dict[NonEmptyStr, NonEmptyStr] = {} | ||||||
| """ | ||||||
| Additional labels to add to the built docker image. | ||||||
| The labels `org.opencontainers.image.title` and `org.opencontainers.image.version` | ||||||
| are set automatically from `name` and `version`. | ||||||
| """ | ||||||
| docker_image: Literal["tar", "gz", "zst"] | None = None | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add explicit
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't allow inputs we don't support. It's okay to only allow
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, and we don't allow inputs we don't support and As far as the |
||||||
| """ | ||||||
| If set, builds a docker image using the Dockerfile generated by constructor and saves it as a portable tarball either uncompressed or compressed. | ||||||
| ``<name>-<version>-<platform>-<arch>-docker.tar`` will be created in the output docker directory. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| """ | ||||||
|
|
||||||
|
|
||||||
| def fix_descriptions(obj): | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,7 +245,8 @@ | |
| "all", | ||
| "exe", | ||
| "pkg", | ||
| "sh" | ||
| "sh", | ||
| "docker" | ||
| ], | ||
| "title": "InstallerTypes", | ||
| "type": "string" | ||
|
|
@@ -607,7 +608,7 @@ | |
| } | ||
| ], | ||
| "default": null, | ||
| "description": "Set default install prefix. On Linux, if not provided, the default prefix is `${HOME}/<NAME>` (or, if `HOME` is not set, `/opt/<NAME>`). On Windows, this is used only for \"Just Me\" installations; for \"All Users\" installations, use the `default_prefix_all_users` key. If not provided, the default prefix is `%USERPROFILE%\\<NAME>`. Environment variables will be expanded at install time.", | ||
| "description": "Set default install prefix. On Linux, if not provided, the default prefix is `${HOME}/<NAME>` (or, if `HOME` is not set, `/opt/<NAME>`). On Windows, this is used only for \"Just Me\" installations; for \"All Users\" installations, use the `default_prefix_all_users` key. If not provided, the default prefix is `%USERPROFILE%\\<NAME>`. Environment variables will be expanded at install time. If creating a Docker output, the default is `/opt/<NAME>` and can be overridden during the Docker build process.", | ||
| "title": "Default Prefix" | ||
| }, | ||
| "default_prefix_all_users": { | ||
|
|
@@ -638,6 +639,65 @@ | |
| "description": "Set default installation prefix for domain users. If not provided, the installation prefix for domain users will be `%LOCALAPPDATA%\\<NAME>`. By default, it is different from the `default_prefix` value to avoid installing the distribution into the roaming profile. Environment variables will be expanded at install time. Windows only.", | ||
| "title": "Default Prefix Domain User" | ||
| }, | ||
| "docker_base_image": { | ||
| "anyOf": [ | ||
| { | ||
| "minLength": 1, | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "null" | ||
| } | ||
| ], | ||
| "default": null, | ||
| "description": "Required to use docker-related features. Base image to use for docker builds and Dockerfiles. This can be any valid docker image reference, including a tag and/or digest. For example: `debian:13.4-slim@sha256:abc123...`.", | ||
| "title": "Docker Base Image" | ||
| }, | ||
| "docker_image": { | ||
| "anyOf": [ | ||
| { | ||
| "enum": [ | ||
| "tar", | ||
| "gz", | ||
| "zst" | ||
| ], | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "null" | ||
| } | ||
| ], | ||
| "default": null, | ||
| "description": "If set, builds a docker image using the Dockerfile generated by constructor and saves it as a portable tarball either uncompressed or compressed. ``<name>-<version>-<platform>-<arch>-docker.tar`` will be created in the output docker directory.", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the format |
||
| "title": "Docker Image" | ||
| }, | ||
| "docker_labels": { | ||
| "additionalProperties": { | ||
| "minLength": 1, | ||
| "type": "string" | ||
| }, | ||
| "default": {}, | ||
| "description": "Additional labels to add to the built docker image. The labels `org.opencontainers.image.title` and `org.opencontainers.image.version` are set automatically from `name` and `version`.", | ||
| "propertyNames": { | ||
| "minLength": 1 | ||
| }, | ||
| "title": "Docker Labels", | ||
| "type": "object" | ||
| }, | ||
| "docker_tag": { | ||
| "anyOf": [ | ||
| { | ||
| "minLength": 1, | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "type": "null" | ||
| } | ||
| ], | ||
| "default": null, | ||
| "description": "Tag to use for the docker image. If not provided, it will default to `<name>:<version>`. Has no effect if not using the `docker_image` feature.", | ||
| "title": "Docker Tag" | ||
| }, | ||
| "environment": { | ||
| "anyOf": [ | ||
| { | ||
|
|
@@ -864,7 +924,7 @@ | |
| } | ||
| ], | ||
| "default": null, | ||
| "description": "The type of the installer being created. Possible values are:\n- `sh`: shell-based installer for Linux or macOS\n- `pkg`: macOS GUI installer built with Apple's `pkgbuild`\n- `exe`: Windows GUI installer built with NSIS\nThe default type is `sh` on Linux and macOS, and `exe` on Windows. A special value of `all` builds _both_ `sh` and `pkg` installers on macOS, as well as `sh` on Linux and `exe` on Windows.", | ||
| "description": "The type of the installer being created. Possible values are:\n- `sh`: shell-based installer for Linux or macOS\n- `pkg`: macOS GUI installer built with Apple's `pkgbuild`\n- `exe`: Windows GUI installer built with NSIS\n- `docker`: generates a `Dockerfile` or image of the installation environment.\nThe default type is `sh` on Linux and macOS, and `exe` on Windows. A special value of `all` builds _both_ `sh` and `pkg` installers on macOS, as well as `sh` on Linux and `exe` on Windows.", | ||
| "title": "Installer Type" | ||
| }, | ||
| "keep_pkgs": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,166 @@ | ||||||||
| """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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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()}"), | ||||||||
|
Jrice1317 marked this conversation as resolved.
|
||||||||
| 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", | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still assuming that either
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. Moving this to the template and adding a v1 fallback |
||||||||
| 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())}." | ||||||||
| ) | ||||||||
|
|
||||||||
| 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, | ||||||||
|
Jrice1317 marked this conversation as resolved.
|
||||||||
| "--load", | ||||||||
| ] | ||||||||
|
|
||||||||
| logger.info("Building Docker image: '%s'", tag) | ||||||||
| try: | ||||||||
| subprocess.run(cmd, check=True) | ||||||||
| except subprocess.CalledProcessError as e: | ||||||||
| # Gather diagnostics on failure | ||||||||
| docker_version = subprocess.run(["docker", "--version"], capture_output=True, text=True) | ||||||||
| buildx_version = subprocess.run( | ||||||||
| ["docker", "buildx", "version"], capture_output=True, text=True | ||||||||
| ) | ||||||||
| buildx_ls = subprocess.run(["docker", "buildx", "ls"], capture_output=True, text=True) | ||||||||
| raise RuntimeError( | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have such detailed error reporting for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can provide some context here, I suggested to add that to debug I'm OK removing/keeping it - hopefully we dont need to debug issues like that again in the future.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||
| f"Docker build failed.\n" | ||||||||
| f"Command: {cmd}\n" | ||||||||
| f"Docker version: {docker_version.stdout.strip()}\n" | ||||||||
| f"Buildx version: {buildx_version.stdout.strip() or buildx_version.stderr.strip()}\n" | ||||||||
| f"Buildx builders: {buildx_ls.stdout.strip()}" | ||||||||
| ) from e | ||||||||
|
|
||||||||
| logger.info("Saving Docker image to tarball: '%s'", tarball_dest) | ||||||||
| subprocess.run(["docker", "save", tag, "-o", str(tarball_dest)], check=True) | ||||||||
| subprocess.run(["docker", "rmi", tag], check=False) | ||||||||
| return tarball_dest | ||||||||
|
|
||||||||
|
|
||||||||
| def create(info: dict, verbose: bool = False) -> None: | ||||||||
| """Build a Docker output | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add something to the doc string describing where the SH installer is coming from?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! |
||||||||
|
|
||||||||
| 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) | ||||||||
| 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"]) | ||||||||
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.
This should mention which variable to use to override it.
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.
Ok