Skip to content

Commit

Permalink
feat: Refactor with Builder.Options (#615)
Browse files Browse the repository at this point in the history 
* feat: Refactor with Builder.Options

Signed-off-by: Ce Gao <cegao@tensorchord.ai>

* fix: Fix bug

Signed-off-by: Ce Gao <cegao@tensorchord.ai>
  • Loading branch information
@gaocegege
gaocegege committed Jul 17, 2022
1 parent fead6ae commit ef8a90d
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 77 deletions.
25 changes: 20 additions & 5 deletions pkg/app/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ func build(clicontext *cli.Context) error {
return errors.Wrap(err, "failed to get absolute path of the build context")
}

filename, funcname, err := builder.ParseFromStr(clicontext.String("from"))
fileName, funcName, err := builder.ParseFromStr(clicontext.String("from"))
if err != nil {
return err
}
manifest, err := filepath.Abs(filepath.Join(buildContext, filename))
manifest, err := filepath.Abs(filepath.Join(buildContext, fileName))
if err != nil {
return errors.Wrap(err, "failed to get absolute path of the build file")
}
Expand Down Expand Up @@ -116,11 +116,26 @@ func build(clicontext *cli.Context) error {
})
debug := clicontext.Bool("debug")
output := clicontext.String("output")

opt := builder.Options{
ManifestFilePath: manifest,
ConfigFilePath: cfg,
BuildFuncName: funcName,
BuildContextDir: buildContext,
Tag: tag,
OutputOpts: output,
PubKeyPath: clicontext.Path("public-key"),
ProgressMode: "auto",
}
if debug {
opt.ProgressMode = "plain"
}

logger.WithFields(logrus.Fields{
"output": output,
"builder-options": opt,
}).Debug("starting build command")
builder, err := builder.New(clicontext.Context, cfg,
manifest, funcname, buildContext, tag, output, debug, clicontext.Path("public-key"))

builder, err := builder.New(clicontext.Context, opt)
if err != nil {
return errors.Wrap(err, "failed to create the builder")
}
Expand Down
37 changes: 22 additions & 15 deletions pkg/app/up.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,10 @@ import (

"github.com/cockroachdb/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/viper"
"github.com/urfave/cli/v2"

"github.com/tensorchord/envd/pkg/builder"
"github.com/tensorchord/envd/pkg/docker"
"github.com/tensorchord/envd/pkg/flag"
"github.com/tensorchord/envd/pkg/home"
"github.com/tensorchord/envd/pkg/lang/ir"
"github.com/tensorchord/envd/pkg/ssh"
Expand Down Expand Up @@ -111,12 +109,12 @@ func up(clicontext *cli.Context) error {
if err != nil {
return errors.Wrap(err, "failed to get absolute path of the build context")
}
filename, funcname, err := builder.ParseFromStr(clicontext.String("from"))
fileName, funcName, err := builder.ParseFromStr(clicontext.String("from"))
if err != nil {
return err
}

manifest, err := filepath.Abs(filepath.Join(buildContext, filename))
manifest, err := filepath.Abs(filepath.Join(buildContext, fileName))
if err != nil {
return errors.Wrap(err, "failed to get absolute path of the build file")
}
Expand All @@ -139,21 +137,30 @@ func up(clicontext *cli.Context) error {
ctr := fileutil.Base(buildContext)

detach := clicontext.Bool("detach")
debug := clicontext.Bool("debug")
output := ""

opt := builder.Options{
ManifestFilePath: manifest,
ConfigFilePath: config,
BuildFuncName: funcName,
BuildContextDir: buildContext,
Tag: tag,
OutputOpts: output,
PubKeyPath: clicontext.Path("public-key"),
ProgressMode: "auto",
}
if debug {
opt.ProgressMode = "plain"
}

logger := logrus.WithFields(logrus.Fields{
"build-context": buildContext,
"build-file": manifest,
"config": config,
"tag": tag,
"container-name": ctr,
"detach": detach,
flag.FlagBuildkitdImage: viper.GetString(flag.FlagBuildkitdImage),
"builder-options": opt,
"container-name": ctr,
"detach": detach,
})
logger.Debug("starting up command")
debug := clicontext.Bool("debug")
output := ""
builder, err := builder.New(clicontext.Context, config, manifest, funcname,
buildContext, tag, output, debug, clicontext.Path("public-key"))
builder, err := builder.New(clicontext.Context, opt)
if err != nil {
return errors.Wrap(err, "failed to create the builder")
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/builder/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (
func (b generalBuilder) BuildFunc() func(ctx context.Context, c client.Client) (*client.Result, error) {
return func(ctx context.Context, c client.Client) (*client.Result, error) {
depsFiles := []string{
b.pubKeyPath,
b.configFilePath,
b.manifestFilePath,
b.PubKeyPath,
b.ConfigFilePath,
b.ManifestFilePath,
}
isUpdated, err := b.CheckDepsFileUpdate(ctx, b.tag, depsFiles)
isUpdated, err := b.CheckDepsFileUpdate(ctx, b.Tag, depsFiles)
if err != nil {
b.logger.Debugf("failed to check manifest update: %s", err)
}
Expand Down
67 changes: 33 additions & 34 deletions pkg/builder/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,36 @@ type Builder interface {
NumGPUs() int
}

type generalBuilder struct {
manifestFilePath string
configFilePath string
progressMode string
tag string
buildContextDir string
buildfuncname string
pubKeyPath string
type Options struct {
// ManifestFilePath is the path to the manifest file `build.envd`.
ManifestFilePath string
// ConfigFilePath is the path to the config file `config.envd`.
ConfigFilePath string
// ProgressMode is the output mode (auto, plain).
ProgressMode string
// Tag is the name of the image.
Tag string
// BuildContextDir is the directory of the build context.
BuildContextDir string
// BuildFuncName is the name of the build func.
BuildFuncName string
// PubKeyPath is the path to the ssh public key.
PubKeyPath string
// OutputOpts is the output options.
OutputOpts string
}

type generalBuilder struct {
Options
entries []client.ExportEntry

logger *logrus.Entry
starlark.Interpreter
buildkitd.Client
}

func New(ctx context.Context, configFilePath, manifestFilePath, funcname,
buildContextDir, tag string, output string, debug bool,
pubKeyPath string) (Builder, error) {
entries, err := parseOutput(output)
func New(ctx context.Context, opt Options) (Builder, error) {
entries, err := parseOutput(opt.OutputOpts)
if err != nil {
return nil, errors.Wrap(err, "failed to parse output")
}
Expand All @@ -80,22 +90,11 @@ func New(ctx context.Context, configFilePath, manifestFilePath, funcname,
return nil, errors.New("only one output type is supported")
}

var mode string = "auto"
if debug {
mode = "plain"
}

b := &generalBuilder{
manifestFilePath: manifestFilePath,
buildfuncname: funcname,
configFilePath: configFilePath,
buildContextDir: buildContextDir,
entries: entries,
pubKeyPath: pubKeyPath,
progressMode: mode,
tag: tag,
Options: opt,
entries: entries,
logger: logrus.WithFields(logrus.Fields{
"tag": tag,
"tag": opt.Tag,
}),
}

Expand All @@ -109,7 +108,7 @@ func New(ctx context.Context, configFilePath, manifestFilePath, funcname,
}
b.Client = cli

b.Interpreter = starlark.NewInterpreter(buildContextDir)
b.Interpreter = starlark.NewInterpreter(opt.BuildContextDir)
return b, nil
}

Expand Down Expand Up @@ -154,7 +153,7 @@ func (b generalBuilder) CheckDepsFileUpdate(ctx context.Context, tag string, dep
}

func (b generalBuilder) Build(ctx context.Context) error {
pw, err := progresswriter.NewPrinter(ctx, os.Stdout, b.progressMode)
pw, err := progresswriter.NewPrinter(ctx, os.Stdout, b.ProgressMode)
if err != nil {
return errors.Wrap(err, "failed to create progress writer")
}
Expand All @@ -167,11 +166,11 @@ func (b generalBuilder) Build(ctx context.Context) error {

func (b generalBuilder) interpret() error {
// Evaluate config first.
if _, err := b.ExecFile(b.configFilePath, ""); err != nil {
if _, err := b.ExecFile(b.ConfigFilePath, ""); err != nil {
return errors.Wrap(err, "failed to exec starlark file")
}

if _, err := b.ExecFile(b.manifestFilePath, b.buildfuncname); err != nil {
if _, err := b.ExecFile(b.ManifestFilePath, b.BuildFuncName); err != nil {
return errors.Wrap(err, "failed to exec starlark file")
}
return nil
Expand All @@ -181,7 +180,7 @@ func (b generalBuilder) compile(ctx context.Context) (*llb.Definition, error) {
if err := b.interpret(); err != nil {
return nil, errors.Wrap(err, "failed to interpret")
}
def, err := ir.Compile(ctx, fileutil.Base(b.buildContextDir), b.pubKeyPath)
def, err := ir.Compile(ctx, fileutil.Base(b.BuildContextDir), b.PubKeyPath)
if err != nil {
return nil, errors.Wrap(err, "failed to compile build.envd")
}
Expand All @@ -198,9 +197,9 @@ func (b generalBuilder) imageConfig(ctx context.Context) (string, error) {
if err != nil {
return "", errors.Wrap(err, "failed to get expose ports")
}
labels[types.ImageLabelContext] = b.buildContextDir
labels[types.ImageLabelContext] = b.BuildContextDir

ep, err := ir.Entrypoint(b.buildContextDir)
ep, err := ir.Entrypoint(b.BuildContextDir)
if err != nil {
return "", errors.Wrap(err, "failed to get entrypoint")
}
Expand Down Expand Up @@ -235,7 +234,7 @@ func (b generalBuilder) build(ctx context.Context, pw progresswriter.Writer) err
entry = client.ExportEntry{
Type: client.ExporterDocker,
Attrs: map[string]string{
"name": b.tag,
"name": b.Tag,
// Ref https://github.com/r2d4/mockerfile/blob/140c6a912bbfdae220febe59ab535ef0acba0e1f/pkg/build/build.go#L65
"containerimage.config": imageConfig,
},
Expand Down
18 changes: 10 additions & 8 deletions pkg/builder/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ var _ = Describe("Builder", func() {
ctrlStarlark := gomock.NewController(GinkgoT())
pub := sshconfig.GetPublicKey()
b = &generalBuilder{
manifestFilePath: manifestFilePath,
configFilePath: configFilePath,
progressMode: "plain",
tag: tag,
buildfuncname: "build",
pubKeyPath: pub,
Options: Options{
ManifestFilePath: manifestFilePath,
ConfigFilePath: configFilePath,
ProgressMode: "plain",
Tag: tag,
BuildFuncName: "build",
PubKeyPath: pub,
},
logger: logrus.WithFields(logrus.Fields{
"tag": tag,
}),
Expand All @@ -82,7 +84,7 @@ var _ = Describe("Builder", func() {
gomock.Any(), gomock.Eq("envd"), gomock.Any(), gomock.Any()).
Return(nil, errors.New("build error"))

pw, err := progresswriter.NewPrinter(context.TODO(), os.Stdout, b.progressMode)
pw, err := progresswriter.NewPrinter(context.TODO(), os.Stdout, b.ProgressMode)
Expect(err).NotTo(HaveOccurred())

close(pw.Status())
Expand All @@ -105,7 +107,7 @@ var _ = Describe("Builder", func() {
gomock.Any(), gomock.Eq("envd"), gomock.Any(), gomock.Any()).
Return(nil, nil)

pw, err := progresswriter.NewPrinter(context.TODO(), os.Stdout, b.progressMode)
pw, err := progresswriter.NewPrinter(context.TODO(), os.Stdout, b.ProgressMode)
Expect(err).NotTo(HaveOccurred())

close(pw.Status())
Expand Down
11 changes: 0 additions & 11 deletions pkg/builder/testdata/build.MIDI
View file

This file was deleted.

Empty file removed pkg/builder/testdata/config.MIDI
View file
Empty file.

0 comments on commit ef8a90d

Please sign in to comment.