-
Notifications
You must be signed in to change notification settings - Fork 263
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
feat(core): add support for direct Dockerfile builds #455
base: main
Are you sure you want to change the base?
Changes from 1 commit
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 |
---|---|---|
@@ -0,0 +1,115 @@ | ||
import functools as ft | ||
from typing import Optional | ||
|
||
import docker | ||
from docker.client import DockerClient | ||
from docker.models.images import Image, ImageCollection | ||
|
||
from .utils import setup_logger | ||
|
||
LOGGER = setup_logger(__name__) | ||
|
||
|
||
class DockerImage: | ||
""" | ||
Basic class to manage docker images. | ||
|
||
.. doctest:: | ||
>>> from testcontainers.core.image import DockerImage | ||
|
||
>>> image = DockerImage().from_dockerfile(path="core/tests/", tag="testcontainers/test-image") | ||
>>> image.exists("testcontainers/test-image") | ||
True | ||
>>> image.get("testcontainers/test-image").id | ||
'sha256:...' | ||
>>> image.remove(force=True) | ||
>>> image.exists("testcontainers/test-image") | ||
False | ||
""" | ||
|
||
def __init__(self, docker_client_kw: Optional[dict] = None, **kwargs) -> None: | ||
self._docker = DockerClient().from_env(**(docker_client_kw or {})) | ||
|
||
def from_dockerfile(self, path: str, tag: str = "local/image") -> "DockerImage": | ||
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. lets not make it dependent on an image, it should be possible to just use the image id, i think, both based on the java implementation link and also based on experience running |
||
""" | ||
Build an image from a Dockerfile. | ||
|
||
Args: | ||
path (str): Path to the Dockerfile | ||
tag (str): Tag for the image | ||
|
||
Returns: | ||
DockerImage: The current instance | ||
""" | ||
self.build(path=path, tag=tag) | ||
return self | ||
|
||
def from_image(self, repository: str, tag: str = "latest") -> "DockerImage": | ||
""" | ||
Pull an image from the registry. | ||
|
||
Args: | ||
repository (str): Image repository | ||
tag (str): Image tag | ||
|
||
Returns: | ||
DockerImage: The current instance | ||
""" | ||
self.pull(repository=repository, tag=tag) | ||
return self | ||
Comment on lines
+47
to
+59
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. it probably doesn't need to pull every time - can we make that configurable? |
||
|
||
@ft.wraps(ImageCollection.build) | ||
def build(self, **kwargs) -> "DockerImage": | ||
LOGGER.info("Building image from Dockerfile") | ||
self._image, _ = self._docker.images.build(**kwargs) | ||
return self | ||
Comment on lines
+61
to
+65
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 should probably be a private method to abstract away the detail that sometimes it is a dockerfile and other times it is a docker image that the container runtime would pull. |
||
|
||
@ft.wraps(ImageCollection.pull) | ||
def pull(self, **kwargs) -> Image: | ||
LOGGER.info("Pulling image") | ||
self._image = self._docker.images.pull(**kwargs) | ||
return self | ||
|
||
@ft.wraps(ImageCollection.get) | ||
def get(self, image: str) -> Image: | ||
LOGGER.info(f"Getting image {image}") | ||
image_obj = self._docker.images.get(image) | ||
return image_obj | ||
|
||
@ft.wraps(ImageCollection.remove) | ||
def remove(self, **kwargs) -> None: | ||
LOGGER.info(f"Removing image {self._image}") | ||
self._image.remove(**kwargs) | ||
Comment on lines
+73
to
+82
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 seems like a lot of stuff that is neat but may be painful to maintain - if this were java i would say lets annotate it with "incubating" or some other variant of message that this is a use at your own risk API. can we add a doc string comment here instead saying that this is an "incubating" API? i think this makes maintenance of this code much less intimidating 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. same for exists... if its used anywhere, make it private, if not used - should come with big hazard warning that says it may go away |
||
|
||
@property | ||
def id(self) -> str: | ||
return self._image.id | ||
|
||
@property | ||
def short_id(self) -> str: | ||
return self._image.short_id | ||
|
||
@property | ||
def tags(self) -> dict: | ||
return self._image.tags | ||
|
||
def get_wrapped_image(self) -> Image: | ||
return self._image | ||
|
||
def exists(self, image: str) -> bool: | ||
""" | ||
Check if the image exists in the local registry. | ||
|
||
Args: | ||
image (str): Image name | ||
|
||
Returns: | ||
bool: True if the image exists, False otherwise | ||
Raises: | ||
docker.errors.ImageNotFound: If the image does not exist | ||
""" | ||
try: | ||
self.get(image) | ||
return True | ||
except docker.errors.ImageNotFound: | ||
return False |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import pytest | ||
from testcontainers.core.image import DockerImage | ||
|
||
|
||
def test_docker_image_from_dockerfile(): | ||
image = DockerImage().from_dockerfile(path="core/tests/", tag="testcontainers/test-image") | ||
|
||
assert image.exists("testcontainers/test-image") == True | ||
|
||
retrieved_image = image.get("testcontainers/test-image") | ||
|
||
assert retrieved_image.id == image.id | ||
assert retrieved_image.short_id == image.short_id | ||
assert retrieved_image.tags == image.tags | ||
|
||
image.remove(force=True) | ||
|
||
assert image.exists("testcontainers/test-image") == False | ||
|
||
|
||
def test_docker_image_from_image(): | ||
image = DockerImage().from_image(repository="alpine") | ||
|
||
assert image.exists("alpine") == True | ||
|
||
retrieved_image = image.get("alpine") | ||
|
||
assert retrieved_image.id == image.id | ||
assert retrieved_image.short_id == image.short_id | ||
assert retrieved_image.tags == image.tags | ||
|
||
image.remove(force=True) | ||
|
||
assert image.exists("alpine") == False |
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.
can we simplify this logic to be:
DockerImage
docker run
and try to contain the conditionals inside
DockerImage