Skip to content
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

[Flytekit] Support extra copy commands in ImageSpec #2715

Merged
merged 30 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2f0d516
feat: support building images with extra copy commands
mao3267 Aug 27, 2024
f675359
feat: add `copy` function in ImageSpec
mao3267 Aug 27, 2024
4808d32
test: build image with extra copy cmds
mao3267 Aug 27, 2024
53b859c
test: ImageSpec with extra copy cmds
mao3267 Aug 27, 2024
4f9dafe
fix: f-string syntax error in copy command
mao3267 Aug 27, 2024
76520a0
fix: rename copy function and related arguments
mao3267 Sep 4, 2024
9a9707d
Merge branch 'master' of https://github.com/mao3267/flytekit into add…
mao3267 Sep 4, 2024
e390c51
fix: update image name hash
mao3267 Sep 4, 2024
d1adcee
Merge branch 'master' of https://github.com/mao3267/flytekit into add…
mao3267 Sep 12, 2024
a8bbb1a
fix: update digest with copy files
mao3267 Sep 12, 2024
e2ec68f
feat: support computing digest from list of files
mao3267 Sep 12, 2024
02611de
fix: update image tag
mao3267 Sep 12, 2024
948447e
Merge branch 'master' of https://github.com/mao3267/flytekit into add…
mao3267 Sep 12, 2024
a619773
fix: remove ignore logic
mao3267 Sep 13, 2024
e8545f3
fix: lint
mao3267 Sep 13, 2024
1c46800
fix: add source_copy_node
mao3267 Sep 14, 2024
eca2063
fix: copy local files to tmp_dir for docker copy
mao3267 Sep 17, 2024
dea9233
test: update default builder test and files for copy
mao3267 Sep 17, 2024
954b27b
fix: f-string related error
mao3267 Sep 17, 2024
4d5215c
fix: lint
mao3267 Sep 17, 2024
8f0e3af
feat: support list of strings and strings in `with_copy`
mao3267 Sep 18, 2024
3b4070d
fix: change ImageSpec.copy to list[str]
mao3267 Sep 18, 2024
b2f5659
Merge branch 'master' of https://github.com/mao3267/flytekit into add…
mao3267 Sep 20, 2024
b7e37dd
fix: remain source tree structure
mao3267 Sep 21, 2024
53ddf88
fix: path separator diff on windows
mao3267 Sep 21, 2024
e9b1c96
fix: use update_attribute instead
mao3267 Sep 25, 2024
ff7699f
rm: remove test files
mao3267 Sep 25, 2024
b1ee46a
fix: test with tmp dirs/files
mao3267 Sep 25, 2024
4e347b9
fix: path separator error
mao3267 Sep 25, 2024
a15940d
nit
pingsutw Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions flytekit/image_spec/default_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@

$COPY_COMMAND_RUNTIME
RUN $RUN_COMMANDS
$EXTRA_COPY_CMDS

WORKDIR /root
SHELL ["/bin/bash", "-c"]
Expand Down Expand Up @@ -207,6 +208,14 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path):
else:
run_commands = ""

extra_copy_cmds = ""
if image_spec.copy_src_dest:
for src, dest in image_spec.copy_src_dest:
src_files_and_dirs = " ".join(src)
extra_copy_cmds += f"COPY --chown=flytekit {src_files_and_dirs} {dest}\n"
else:
extra_copy_cmds = ""

docker_content = DOCKER_FILE_TEMPLATE.substitute(
PYTHON_VERSION=python_version,
UV_PYTHON_INSTALL_COMMAND=uv_python_install_command,
Expand All @@ -218,6 +227,7 @@ def create_docker_context(image_spec: ImageSpec, tmp_dir: Path):
COPY_COMMAND_RUNTIME=copy_command_runtime,
ENTRYPOINT=entrypoint,
RUN_COMMANDS=run_commands,
EXTRA_COPY_CMDS=extra_copy_cmds,
)

dockerfile_path = tmp_dir / "Dockerfile"
Expand Down Expand Up @@ -247,6 +257,7 @@ class DefaultImageBuilder(ImageSpecBuilder):
"pip_extra_index_url",
# "registry_config",
"commands",
"copy_src_dest",
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
}

def build_image(self, image_spec: ImageSpec) -> str:
Expand Down
14 changes: 14 additions & 0 deletions flytekit/image_spec/image_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ImageSpec:
commands: Command to run during the building process
tag_format: Custom string format for image tag. The ImageSpec hash passed in as `spec_hash`. For example,
to add a "dev" suffix to the image tag, set `tag_format="{spec_hash}-dev"`
copy_src_dest: List of tuples containing source files/directories and their corresponding destination directory for copying.
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
pingsutw marked this conversation as resolved.
Show resolved Hide resolved
"""

name: str = "flytekit"
Expand All @@ -73,6 +74,7 @@ class ImageSpec:
entrypoint: Optional[List[str]] = None
commands: Optional[List[str]] = None
tag_format: Optional[str] = None
copy_src_dest: Optional[List[Tuple[List[str], str]]] = None

def __post_init__(self):
self.name = self.name.lower()
Expand Down Expand Up @@ -232,6 +234,18 @@ def with_apt_packages(self, apt_packages: Union[str, List[str]]) -> "ImageSpec":

return new_image_spec

def copy(self, src: List[str], dest: str = ".") -> "ImageSpec":
"""
Builder that returns a new image spec with the source files copied to the destination directory.
"""
new_image_spec = copy.deepcopy(self)
if new_image_spec.copy_src_dest is None:
new_image_spec.copy_src_dest = []

new_image_spec.copy_src_dest.append((src, dest))

return new_image_spec

def force_push(self) -> "ImageSpec":
"""
Builder that returns a new image spec with force push enabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def test_create_docker_context(tmp_path):
source_root=os.fspath(source_root),
commands=["mkdir my_dir"],
entrypoint=["/bin/bash"],
pip_extra_index_url=["https://extra-url.com"]
pip_extra_index_url=["https://extra-url.com"],
copy_src_dest=[(["file1.txt", "/dir1"], "/root")]
)

create_docker_context(image_spec, docker_context_path)
Expand All @@ -49,6 +50,7 @@ def test_create_docker_context(tmp_path):
assert "RUN mkdir my_dir" in dockerfile_content
assert "ENTRYPOINT [\"/bin/bash\"]" in dockerfile_content
assert "mkdir -p $HOME" in dockerfile_content
assert "COPY --chown=flytekit file1.txt /dir1 /root" in dockerfile_content

requirements_path = docker_context_path / "requirements_uv.txt"
assert requirements_path.exists()
Expand Down Expand Up @@ -177,6 +179,7 @@ def test_build(tmp_path):
requirements=os.fspath(other_requirements_path),
source_root=os.fspath(source_root),
commands=["mkdir my_dir"],
copy_src_dest=[([f"{tmp_path}/hello_world.txt", f"{tmp_path}/requirements.txt"], "/my_dir")]
)

builder = DefaultImageBuilder()
Expand Down
3 changes: 3 additions & 0 deletions tests/flytekit/unit/core/image_spec/test_image_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ def test_image_spec(mock_image_spec_builder):
requirements=REQUIREMENT_FILE,
registry_config=REGISTRY_CONFIG_FILE,
entrypoint=["/bin/bash"],
copy_src_dest=[(["/src/file1.txt"], "/dest")]
)
assert image_spec._is_force_push is False

image_spec = image_spec.with_commands("echo hello")
image_spec = image_spec.with_packages("numpy")
image_spec = image_spec.with_apt_packages("wget")
image_spec = image_spec.copy(["/src", "/src/file2.txt"], "/dest")
image_spec = image_spec.force_push()

assert image_spec.python_version == "3.8"
Expand All @@ -52,6 +54,7 @@ def test_image_spec(mock_image_spec_builder):
assert image_spec.commands == ["echo hello"]
assert image_spec._is_force_push is True
assert image_spec.entrypoint == ["/bin/bash"]
assert image_spec.copy_src_dest == [(["/src/file1.txt"], "/dest"), (["/src", "/src/file2.txt"], "/dest")]

tag = calculate_hash_from_image_spec(image_spec)
assert "=" != tag[-1]
Expand Down
Loading