Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 23 additions & 4 deletions internal/boxcli/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ type generateCmdFlags struct {
rootUser bool
}

type generateDockerfileCmdFlags struct {
generateCmdFlags
forType string
}

type GenerateReadmeCmdFlags struct {
generateCmdFlags
saveTemplate bool
Expand Down Expand Up @@ -94,17 +99,33 @@ func devcontainerCmd() *cobra.Command {
}

func dockerfileCmd() *cobra.Command {
flags := &generateCmdFlags{}
flags := &generateDockerfileCmdFlags{}
command := &cobra.Command{
Use: "dockerfile",
Short: "Generate a Dockerfile that replicates devbox shell",
Long: "Generate a Dockerfile that replicates devbox shell. " +
"Can be used to run devbox shell environment in an OCI container.",
Args: cobra.MaximumNArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
return runGenerateCmd(cmd, flags)
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Environment: flags.config.environment,
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
}
return box.GenerateDockerfile(cmd.Context(), devopt.GenerateOpts{
ForType: flags.forType,
Force: flags.force,
RootUser: flags.rootUser,
})
},
}
command.Flags().StringVar(
&flags.forType, "for", "dev",
"Generate Dockerfile for a specific type of container (dev, prod)")
command.Flag("for").Hidden = true
command.Flags().BoolVarP(
&flags.force, "force", "f", false, "force overwrite existing files")
command.Flags().BoolVar(
Expand Down Expand Up @@ -264,8 +285,6 @@ func runGenerateCmd(cmd *cobra.Command, flags *generateCmdFlags) error {
return box.Generate(cmd.Context())
case "devcontainer":
return box.GenerateDevcontainer(cmd.Context(), generateOpts)
case "dockerfile":
return box.GenerateDockerfile(cmd.Context(), generateOpts)
}
return nil
}
Expand Down
11 changes: 9 additions & 2 deletions internal/devbox/devbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ func (d *Devbox) GenerateDevcontainer(ctx context.Context, generateOpts devopt.G
}

// generate dockerfile
err = gen.CreateDockerfile(ctx)
err = gen.CreateDockerfile(ctx, generate.CreateDockerfileOptions{})
if err != nil {
return redact.Errorf("error generating dev container Dockerfile in <project>/%s: %w",
redact.Safe(filepath.Base(devContainerPath)), err)
Expand Down Expand Up @@ -489,8 +489,15 @@ func (d *Devbox) GenerateDockerfile(ctx context.Context, generateOpts devopt.Gen
LocalFlakeDirs: d.getLocalFlakesDirs(),
}

scripts := d.cfg.Scripts()

// generate dockerfile
return errors.WithStack(gen.CreateDockerfile(ctx))
return errors.WithStack(gen.CreateDockerfile(ctx, generate.CreateDockerfileOptions{
ForType: generateOpts.ForType,
HasBuild: scripts["build"] != nil,
HasInstall: scripts["install"] != nil,
HasStart: scripts["start"] != nil,
}))
}

func PrintEnvrcContent(w io.Writer, envFlags devopt.EnvFlags) error {
Expand Down
1 change: 1 addition & 0 deletions internal/devbox/devopt/devboxopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ type Opts struct {
}

type GenerateOpts struct {
ForType string
Force bool
RootUser bool
}
Expand Down
66 changes: 52 additions & 14 deletions internal/devbox/generate/devcontainer_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,21 @@ package generate
// package generate has functionality to implement the `devbox generate` command

import (
"cmp"
"context"
"embed"
"encoding/json"
"fmt"
"html/template"
"io"
"os"
"path/filepath"
"regexp"
"runtime/trace"
"strings"
"text/template"

"github.com/samber/lo"
"go.jetpack.io/devbox/internal/boxcli/usererr"
"go.jetpack.io/devbox/internal/debug"
"go.jetpack.io/devbox/internal/devbox/devopt"
)
Expand Down Expand Up @@ -54,30 +57,65 @@ type vscode struct {
Extensions []string `json:"extensions"`
}

type dockerfileData struct {
IsDevcontainer bool
RootUser bool
LocalFlakeDirs []string
type CreateDockerfileOptions struct {
ForType string
HasInstall bool
HasBuild bool
HasStart bool
// Ideally we also support process-compose services as the dockerfile
// CMD, but I'm currently having trouble getting that to work. Will revisit.
// HasServices bool
}

func (opts CreateDockerfileOptions) Type() string {
return cmp.Or(opts.ForType, "dev")
}

func (opts CreateDockerfileOptions) validate() error {
if opts.Type() == "dev" {
return nil
} else if opts.Type() == "prod" {
if opts.HasStart {
return nil
}
return usererr.New(
"To generate a prod Dockerfile you must have either 'start' script in " +
"devbox.json",
)
}
return usererr.New(
"invalid Dockerfile type. Only 'dev' and 'prod' are supported")
}

// CreateDockerfile creates a Dockerfile in path and writes devcontainerDockerfile.tmpl's content into it
func (g *Options) CreateDockerfile(ctx context.Context) error {
// CreateDockerfile creates a Dockerfile in path.
func (g *Options) CreateDockerfile(
ctx context.Context,
opts CreateDockerfileOptions,
) error {
defer trace.StartRegion(ctx, "createDockerfile").End()

if err := opts.validate(); err != nil {
return err
}

// create dockerfile
file, err := os.Create(filepath.Join(g.Path, "Dockerfile"))
if err != nil {
return err
}
defer file.Close()
// get dockerfile content
tmplName := "devcontainerDockerfile.tmpl"
t := template.Must(template.ParseFS(tmplFS, "tmpl/"+tmplName))
path := fmt.Sprintf("tmpl/%s.Dockerfile.tmpl", opts.Type())
t := template.Must(template.ParseFS(tmplFS, path))
// write content into file
return t.Execute(file, &dockerfileData{
IsDevcontainer: g.IsDevcontainer,
RootUser: g.RootUser,
LocalFlakeDirs: g.LocalFlakeDirs,
return t.Execute(file, map[string]any{
"IsDevcontainer": g.IsDevcontainer,
"RootUser": g.RootUser,
"LocalFlakeDirs": g.LocalFlakeDirs,

// The following are only used for prod Dockerfile
"DevboxRunInstall": lo.Ternary(opts.HasInstall, "devbox run install", "echo 'No install script found, skipping'"),
"DevboxRunBuild": lo.Ternary(opts.HasBuild, "devbox run build", "echo 'No build script found, skipping'"),
"Cmd": fmt.Sprintf("%q, %q, %q", "devbox", "run", "start"),
})
}

Expand Down
28 changes: 28 additions & 0 deletions internal/devbox/generate/tmpl/prod.Dockerfile.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
FROM jetpackio/devbox:latest

WORKDIR /code
USER root:root
RUN mkdir -p /code && chown ${DEVBOX_USER}:${DEVBOX_USER} /code
USER ${DEVBOX_USER}:${DEVBOX_USER}

{{- /*
Ideally, we first copy over devbox.json and devbox.lock and run `devbox install`
to create a cache layer for the dependencies. This is complicated because
devbox.json may include local dependencies (flakes and plugins). We could try
to copy those in (the way the dev Dockerfile does) but that's brittle because
those dependencies may also pull in other local dependencies and so on. Another
sulution would be to add a new flag `devbox install --skip-errors` that would
just try to install what it can, and ignore the rest.

A hack to make this simpler is to install from the lockfile instead of the json.
*/}}

COPY --chown=${DEVBOX_USER}:${DEVBOX_USER} . .

RUN devbox install
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an issue with using devbox install command in dockerfile. IIRC it didn't activate the python plugin so the subsequent python commands were not running in a virtual environment.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the issue I'm talking about:
#1122

can you test the steps in the issue to make sure we don't regress?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by that issue. I understand that install doesn't run init hooks, but I would assume the line below

RUN devbox shellenv --init-hook >> ~/.profile

Would do the initialization.

Furthermore, actually activating the virtual environment is a manual process so neither install/run would do it.

Do you know why shellenv is not working?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, since the prod dockerfile always ends in devbox run start it will always run the init hook which creates the virtenv. To activate it, the user would need to add that to their start command. Maybe we can add an env variable that automatically activates it.


RUN {{ .DevboxRunInstall }}

RUN {{ .DevboxRunBuild }}

CMD [{{ .Cmd }}]
Copy link
Collaborator

@LucilleH LucilleH Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we can optimize this image further, especially it is supposedly for prod. Right now it builds for a long time and the image ends up super big. Something like:

  • gc nix store (clean up)
  • devbox shellenv (so cmd has the right env)
  • separate install layer

Although I don't know how much those steps could help.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LucilleH for sure! I think some of those might be doable without devbox changes (but not all).

gc nix store (clean up)

I can try this, but in theory the nix store should be pretty clean no? We haven't installed anything we don't need.

devbox shellenv

This can help a bit but not without changes. Ideally, we do shellenv and then call the raw start command (without using devbox run). A small concern is the init hook not running multiple times (I think that's probably fine). So in practice it would look like:

RUN eval $(devbox shellenv --init-hook)
RUN devbox run install --raw
RUN devbox run build --raw
CMD ["devbox", "run", "start", "--raw"] 

Where --raw would be a new flag that indicates run the raw script without starting a devbox env (which would be wasteful)

separate install layer

This is the biggest win. But see my comment about local dependencies. I think there's 2 huge optimizations here:

  • Do devbox install before copying source code. This makes a very cachable base layer.
  • Split stages so that we copy over only nix stuff we need into final image.

(1) is easy, but requires a new flag/command so we can ignore errors of missing local dependencies.
(2) is harder, but we did something similar in early devbox days.

My preference is to merge this and optimize in follow up. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently RUN eval $(devbox shellenv --init-hook) is not a thing you can do because each RUN runs in own shell. So maybe something like

RUN devbox shellenv --init-hook >> ~/.profile