Feat: Allow users to bring their own Dockerfile #773

dleviminzi · 2024-12-10T20:05:31Z

Resolve BE-2084

This feature adds a new operator to the Image abstraction that allows users to specify a path to a Dockerfile that they wish to use as the base of their Image. We use buildah and umoci to build their Dockerfile and gather the bundle needed to run it.

It can be used as follows:

from beta9 import Image, endpoint

image = Image().from_dockerfile("./Dockerfile").add_python_packages(["numpy"])


@endpoint(image=image, name="test_dockerfile")
def handler():
    return "pass"

Where ./Dockerfile is:

FROM ubuntu:22.04
RUN echo hello
ENV FOO=bar

Note: when testing locally, you might need to build gateway again to get make start to work because the go version is bumped.

pkg/abstractions/image/build.go

dleviminzi · 2024-12-11T02:50:30Z

pkg/abstractions/image/build.go

+	if opts.Dockerfile != "" {
+		fmt.Fprint(h, opts.Dockerfile)
+	}
+	if opts.BuildCtxObject != "" {
+		fmt.Fprint(h, opts.BuildCtxObject)
+	}


This feels pretty hacky to me, but I don't think we can change hash body.

docker/Dockerfile.worker

pkg/worker/image.go

luke-lombardi · 2024-12-11T22:16:21Z

docker/Dockerfile.gateway

@@ -25,7 +25,7 @@ FROM base AS build

 WORKDIR /workspace

-RUN apt-get install -y libfuse3-dev && \
+RUN apt-get install -y libfuse3-dev libbtrfs-dev libgpgme-dev && \


I think there may have been an issue with libgpg + cedana, we should look into if it causes any issues with C/R

luke-lombardi · 2024-12-11T22:16:30Z

docker/Dockerfile.worker

@@ -93,6 +115,11 @@ RUN apt-get install -y --no-install-recommends nvidia-container-toolkit-base nvi

 RUN apt-get update && apt-get install -y fuse3 libfuse2 libfuse3-dev libfuse-dev bash-completion

+RUN apt-get install -y libbtrfs-dev libgpgme11-dev


same comment as above

luke-lombardi · 2024-12-11T22:17:02Z

pkg/abstractions/image/build.go

+	fmt.Fprintf(&b, "  \"ExistingImageUri\": %q,", o.ExistingImageUri)
+	fmt.Fprintf(&b, "  \"ExistingImageCreds\": %#v,", o.ExistingImageCreds)
+	fmt.Fprintf(&b, "  \"Dockerfile\": %q,", o.Dockerfile)
+	fmt.Fprintf(&b, "  \"BuildCtxObject\": %q,", o.BuildCtxObject)


What exactly is in this var?

The object ID of the synced build context directory

pkg/abstractions/image/build.go

luke-lombardi · 2024-12-11T22:18:19Z

pkg/abstractions/image/build.go

@@ -201,6 +263,8 @@ func (b *Builder) Build(ctx context.Context, opts *BuildOpts, outputChan chan co
 		ImageId:          baseImageId,
 		SourceImage:      &sourceImage,
 		SourceImageCreds: opts.BaseImageCreds,
+		Dockerfile:       dockerfile,
+		BuildCtxObject:   &opts.BuildCtxObject,


I really don't like that this is top level on the ContainerRequest. It might seem out of scope but I think we should nest these under a BuildOptions struct on the container request

Bumped it out

luke-lombardi · 2024-12-11T22:19:15Z

pkg/types/scheduler.go

@@ -101,6 +111,16 @@ func (c *ContainerRequest) RequiresGPU() bool {
 	return len(c.GpuRequest) > 0 || c.Gpu != ""
 }

+func (c *ContainerRequest) ImageSourceType() ImageSourceType {
+	if c.Dockerfile != nil {
+		return ImageSourceTypeBuild


probably would call this just ImageSource

Also I'd rather it be a method on a BuildOptions struct

We also already have an isBuildRequest thing so it's more fitting under that struct IMO

I just removed this entirely as I don't think it was really needed

luke-lombardi · 2024-12-11T22:21:31Z

pkg/worker/lifecycle.go

 	if err != nil {
-		return err
+		switch request.ImageSourceType() {


I think after moving this to the build options struct, it would be cleaner to refactor this to first check if it's a build request using the container ID, etc. And then handle these clauses in a separate function so it's logically separate from the standard container execution flow.

dleviminzi added 16 commits December 5, 2024 15:56

buildah pkg installed and test set up

87423e4

compiles with upgraded major version

ac308bd

add deps to final image

f67fd99

not working pkg use to build

3dbc626

add cni tag

952bef4

starts container but final archival is broken

513b238

working

a4456b3

broken run spec

6f48138

sdk stopped compiling w/o upgrade

d56a324

builds and launches

490e393

add base requirements

7799128

Merge branch 'main' into dlm/from-dockerfile

296ffc9

mod tidy

af68314

prune deps

ebf3cd5

clean up

07e3f42

update test

654af01

dleviminzi requested review from luke-lombardi, nickpetrovic and jsun-m and removed request for nickpetrovic December 10, 2024 20:06

support for docker copy commands

0c62909

dleviminzi marked this pull request as ready for review December 10, 2024 22:57

dleviminzi commented Dec 10, 2024

View reviewed changes

pkg/abstractions/image/build.go Outdated Show resolved Hide resolved

dleviminzi added 4 commits December 10, 2024 21:34

add docstring

ee9ade3

fix bug where syncing nested build context dir stopped workspace sync

76e83db

add python req filling to verify image build

011bada

simplify image id generation scheme

f557ba8

dleviminzi commented Dec 11, 2024

View reviewed changes

nickpetrovic reviewed Dec 11, 2024

View reviewed changes

docker/Dockerfile.worker Outdated Show resolved Hide resolved

pkg/worker/image.go Outdated Show resolved Hide resolved

dleviminzi added 2 commits December 11, 2024 11:38

move defer

228fc0f

static build

75d007a

dleviminzi requested a review from nickpetrovic December 11, 2024 16:44

nickpetrovic approved these changes Dec 11, 2024

View reviewed changes

dleviminzi added 2 commits December 11, 2024 14:13

add old dependencies back

19a9c9c

no more podman dep

b43b086

luke-lombardi requested changes Dec 11, 2024

View reviewed changes

dleviminzi added 5 commits December 11, 2024 17:38

remove redundant message

aed9672

remove image source type method

0705834

bump out build or pull method

9302b4f

Add nested struct for build options in container request

ef525d1

Merge branch 'main' into dlm/from-dockerfile

2d9518e

dleviminzi requested a review from luke-lombardi December 11, 2024 22:57

pipe output with container id so users can see it

b45e75f

dleviminzi temporarily deployed to Release December 11, 2024 23:31 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Allow users to bring their own Dockerfile #773

Feat: Allow users to bring their own Dockerfile #773

dleviminzi commented Dec 10, 2024 •

edited

Loading

dleviminzi Dec 11, 2024

luke-lombardi Dec 11, 2024

luke-lombardi Dec 11, 2024

luke-lombardi Dec 11, 2024

dleviminzi Dec 11, 2024

luke-lombardi Dec 11, 2024

dleviminzi Dec 11, 2024

luke-lombardi Dec 11, 2024

luke-lombardi Dec 11, 2024

luke-lombardi Dec 11, 2024

dleviminzi Dec 11, 2024

luke-lombardi Dec 11, 2024

dleviminzi Dec 11, 2024

		@@ -93,6 +115,11 @@ RUN apt-get install -y --no-install-recommends nvidia-container-toolkit-base nvi

		RUN apt-get update && apt-get install -y fuse3 libfuse2 libfuse3-dev libfuse-dev bash-completion

		RUN apt-get install -y libbtrfs-dev libgpgme11-dev

Feat: Allow users to bring their own Dockerfile #773

Are you sure you want to change the base?

Feat: Allow users to bring their own Dockerfile #773

Conversation

dleviminzi commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dleviminzi commented Dec 10, 2024 •

edited

Loading