From 6fc20457561dd5bb7124c9446e368e83496e14f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ota=CC=81vio=20Fernandes?= Date: Wed, 25 Sep 2019 18:55:41 +0200 Subject: [PATCH] feat(build): Introducing External build strategy. --- pkg/build/strategies/external/external.go | 154 ++++++++++++++++++ .../strategies/external/external_test.go | 110 +++++++++++++ pkg/build/strategies/strategies.go | 13 ++ pkg/cmd/cli/cmd/build.go | 19 +-- 4 files changed, 284 insertions(+), 12 deletions(-) create mode 100644 pkg/build/strategies/external/external.go create mode 100644 pkg/build/strategies/external/external_test.go diff --git a/pkg/build/strategies/external/external.go b/pkg/build/strategies/external/external.go new file mode 100644 index 000000000..40c78bae7 --- /dev/null +++ b/pkg/build/strategies/external/external.go @@ -0,0 +1,154 @@ +package external + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path" + "strings" + "syscall" + "text/template" + + "github.com/openshift/source-to-image/pkg/api" + "github.com/openshift/source-to-image/pkg/build/strategies/dockerfile" + "github.com/openshift/source-to-image/pkg/util/fs" + utillog "github.com/openshift/source-to-image/pkg/util/log" +) + +// External represents the shell out for external build commands, therefore s2i based build can +// execute the generation of +type External struct { + dockerfile *dockerfile.Dockerfile +} + +// s2iDockerfile Dockerfile default filename. +const s2iDockerfile = "Dockerfile.s2i" + +var ( + // local logger + log = utillog.StderrLog + // supported external commands, template is based on api.Config instance + commands = map[string]string{ + "buildah": `buildah bud --tag {{ .Tag }} --file {{ .AsDockerfile }} {{ or .WorkingDir "." }}`, + "podman": `podman build --tag {{ .Tag }} --file {{ .AsDockerfile }} {{ or .WorkingDir "." }}`, + } +) + +// GetBuilders returns a list of command names, based global commands map. +func GetBuilders() []string { + builders := []string{} + for k := range commands { + builders = append(builders, k) + } + return builders +} + +// ValidBuilderName returns a boolean based in keys of global commands map. +func ValidBuilderName(name string) bool { + _, exists := commands[name] + return exists +} + +// renderCommand render a shell command based in api.Config instance. Attribute WithBuilder +// wll determine external builder name, and api.Config feeds command's template variables. It can +// return error in case of template parsing or evaluation issues. +func (e *External) renderCommand(config *api.Config) (string, error) { + commandTemplate, exists := commands[config.WithBuilder] + if !exists { + return "", fmt.Errorf("cannot find command '%s' in dictionary: '%#v'", + config.WithBuilder, commands) + } + + t, err := template.New("external-command").Parse(commandTemplate) + if err != nil { + return "", err + } + var output bytes.Buffer + if err = t.Execute(&output, config); err != nil { + return "", err + } + return output.String(), nil +} + +// execute the given external command using "os/exec". Returns the outcomes as api.Result, making +// sure it only marks result as success when exit-code is zero. Therefore, it returns errors based +// in external command errors, so "s2i build" also fails. +func (e *External) execute(externalCommand string) (*api.Result, error) { + log.V(0).Infof("Executing external build command: '%s'", externalCommand) + + externalCommandSlice := strings.Split(externalCommand, " ") + cmd := exec.Command(externalCommandSlice[0], externalCommandSlice[1:]...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + res := &api.Result{Success: false} + res.Messages = append(res.Messages, fmt.Sprintf("Running command: '%s'", externalCommand)) + err := cmd.Start() + if err != nil { + res.Messages = append(res.Messages, err.Error()) + return res, err + } + + if err := cmd.Wait(); err != nil { + if exitErr, okay := err.(*exec.ExitError); okay { + if status, okay := exitErr.Sys().(syscall.WaitStatus); okay { + exitCode := status.ExitStatus() + log.V(0).Infof("External command return-code: %d", exitCode) + res.Messages = append(res.Messages, fmt.Sprintf("exit-code: %d", exitCode)) + if exitCode == 0 { + res.Success = true + } else { + return res, exitErr + } + } + } else { + return res, err + } + } + return res, nil +} + +// asDockerfile inspect config, if user has already informed `--as-dockerfile` option, that's simply +// returned, otherwise, considering `--working-dir` option first before using artificial name. +func (e *External) asDockerfile(config *api.Config) string { + if len(config.AsDockerfile) > 0 { + return config.AsDockerfile + } + + if len(config.WorkingDir) > 0 { + return path.Join(config.WorkingDir, s2iDockerfile) + } + return s2iDockerfile +} + +// Build triggers the build of a "strategy/dockerfile" to obtain "AsDockerfile" first, and then +// proceed to execute the external command. +func (e *External) Build(config *api.Config) (*api.Result, error) { + config.AsDockerfile = e.asDockerfile(config) + + externalCommand, err := e.renderCommand(config) + if err != nil { + return nil, err + } + + // generating dockerfile following AsDockerfile directive + res, err := e.dockerfile.Build(config) + if err != nil { + return nil, err + } + if !res.Success { + return nil, fmt.Errorf("unable to genereate a Dockerfile") + } + + return e.execute(externalCommand) +} + +// New instance of External command strategy. +func New(config *api.Config, fs fs.FileSystem) (*External, error) { + df, err := dockerfile.New(config, fs) + if err != nil { + return nil, err + } + return &External{dockerfile: df}, nil +} diff --git a/pkg/build/strategies/external/external_test.go b/pkg/build/strategies/external/external_test.go new file mode 100644 index 000000000..0c16c61b6 --- /dev/null +++ b/pkg/build/strategies/external/external_test.go @@ -0,0 +1,110 @@ +package external + +import ( + "reflect" + "testing" + + "github.com/openshift/source-to-image/pkg/api" +) + +func TestExternalGetBuilders(t *testing.T) { + tests := []struct { + name string + want []string + }{ + {"builders", []string{"buildah", "podman"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetBuilders(); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetBuilders() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestExternalValidBuilderName(t *testing.T) { + tests := []struct { + name string + args string + want bool + }{ + {"valid-builder-name", "buildah", true}, + {"valid-builder-name", "podman", true}, + {"invalid-builder-name", "other", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ValidBuilderName(tt.args); got != tt.want { + t.Errorf("ValidBuilderName() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestExternalRenderCommand(t *testing.T) { + e := &External{} + tests := []struct { + name string + config *api.Config + want string + wantErr bool + }{ + { + "render-buildah", + &api.Config{WithBuilder: "buildah", Tag: "tag", AsDockerfile: "dockerfile"}, + "buildah bud --tag tag --file dockerfile .", + false, + }, + { + "render-podman", + &api.Config{WithBuilder: "podman", Tag: "tag", AsDockerfile: "dockerfile"}, + "podman build --tag tag --file dockerfile .", + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := e.renderCommand(tt.config) + if (err != nil) != tt.wantErr { + t.Errorf("External.renderCommand() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("External.renderCommand() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestExternalAsDockerfile(t *testing.T) { + e := &External{} + tests := []struct { + name string + config *api.Config + want string + }{ + { + "without-as-dockerfile-without-working-dir", + &api.Config{}, + "Dockerfile.s2i", + }, + { + "without-as-dockerfile-with-working-dir", + &api.Config{WorkingDir: "dir"}, + "dir/Dockerfile.s2i", + }, + { + "with-as-dockerfile-with-working-dir", + &api.Config{AsDockerfile: "Dockerfile", WorkingDir: "dir"}, + "Dockerfile", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := e.asDockerfile(tt.config); got != tt.want { + t.Errorf("External.asDockerfile() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/build/strategies/strategies.go b/pkg/build/strategies/strategies.go index 9738dbc39..ea2cf790e 100644 --- a/pkg/build/strategies/strategies.go +++ b/pkg/build/strategies/strategies.go @@ -6,6 +6,7 @@ import ( "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/build" "github.com/openshift/source-to-image/pkg/build/strategies/dockerfile" + "github.com/openshift/source-to-image/pkg/build/strategies/external" "github.com/openshift/source-to-image/pkg/build/strategies/onbuild" "github.com/openshift/source-to-image/pkg/build/strategies/sti" "github.com/openshift/source-to-image/pkg/docker" @@ -24,6 +25,18 @@ func Strategy(client docker.Client, config *api.Config, overrides build.Override startTime := time.Now() + if len(config.WithBuilder) != 0 { + builder, err = external.New(config, fileSystem) + if err != nil { + buildInfo.FailureReason = utilstatus.NewFailureReason( + utilstatus.ReasonGenericS2IBuildFailed, + utilstatus.ReasonMessageGenericS2iBuildFailed, + ) + return nil, buildInfo, err + } + return builder, buildInfo, nil + } + if len(config.AsDockerfile) != 0 { builder, err = dockerfile.New(config, fileSystem) if err != nil { diff --git a/pkg/cmd/cli/cmd/build.go b/pkg/cmd/cli/cmd/build.go index a6c90d2ff..c87236689 100644 --- a/pkg/cmd/cli/cmd/build.go +++ b/pkg/cmd/cli/cmd/build.go @@ -10,6 +10,7 @@ import ( "github.com/openshift/source-to-image/pkg/api" "github.com/openshift/source-to-image/pkg/api/describe" "github.com/openshift/source-to-image/pkg/api/validation" + "github.com/openshift/source-to-image/pkg/build" "github.com/openshift/source-to-image/pkg/build/strategies" "github.com/openshift/source-to-image/pkg/build/strategies/external" cmdutil "github.com/openshift/source-to-image/pkg/cmd/cli/util" @@ -82,16 +83,10 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app } } - if len(cfg.WithBuilder) > 0 { - if len(cfg.AsDockerfile) == 0 { - fmt.Fprintln(os.Stderr, "ERROR: --with-builder can only be used with --as-dockerfile") - return - } - if !external.ValidBuilderName(cfg.WithBuilder) { - fmt.Fprintf(os.Stderr, "ERROR: invalid --with-builder='%s', allowed options are: '[%v]'\n", - cfg.WithBuilder, strings.Join(external.GetBuilders(), ", ")) - return - } + if len(cfg.WithBuilder) > 0 && !external.ValidBuilderName(cfg.WithBuilder) { + fmt.Fprintf(os.Stderr, "ERROR: invalid --with-builder='%s', allowed options are: '[%v]'\n", + cfg.WithBuilder, strings.Join(external.GetBuilders(), ", ")) + return } if cfg.Incremental && len(cfg.RuntimeImage) > 0 { @@ -176,7 +171,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app log.V(2).Infof("\n%s\n", describe.Config(client, cfg)) - builder, _, err := strategies.GetStrategy(client, cfg) + builder, _, err := strategies.Strategy(client, cfg, build.Overrides{}) s2ierr.CheckError(err) result, err := builder.Build(cfg) if err != nil { @@ -230,7 +225,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app buildCmd.Flags().VarP(&(cfg.RuntimeArtifacts), "runtime-artifact", "a", "Specify a file or directory to be copied from the builder to the runtime image") buildCmd.Flags().StringVar(&(networkMode), "network", "", "Specify the default Docker Network name to be used in build process") buildCmd.Flags().StringVarP(&(cfg.AsDockerfile), "as-dockerfile", "", "", "EXPERIMENTAL: Output a Dockerfile to this path instead of building a new image") - buildCmd.Flags().StringVarP(&(cfg.WithBuilder), "with-builder", "", "", "EXPERIMENTAL: Use a external builder against Dockerfile generated by --as-dockerfile. Supported builders: ["+strings.Join(external.GetBuilders(), ", ")+"]") + buildCmd.Flags().StringVarP(&(cfg.WithBuilder), "with-builder", "", "", "EXPERIMENTAL: Use a external builder against Dockerfile generated by s2i. Supported builders: ["+strings.Join(external.GetBuilders(), ", ")+"]") buildCmd.Flags().BoolVarP(&(cfg.KeepSymlinks), "keep-symlinks", "", false, "When using '--copy', copy symlinks as symlinks. Default behavior is to follow symlinks and copy files by content") buildCmd.Flags().StringArrayVar(&cfg.AddHost, "add-host", []string{}, "Specify additional entries to add to the /etc/hosts in the assemble container, multiple --add-host can be used to add multiple entries") return buildCmd