Skip to content

Commit 49bb5c3

Browse files
committed
feat(build): Introducing External build strategy.
1 parent 9df3cc6 commit 49bb5c3

File tree

4 files changed

+282
-13
lines changed

4 files changed

+282
-13
lines changed
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
package external
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"os"
7+
"os/exec"
8+
"path"
9+
"strings"
10+
"syscall"
11+
"text/template"
12+
13+
"github.com/openshift/source-to-image/pkg/api"
14+
"github.com/openshift/source-to-image/pkg/build/strategies/dockerfile"
15+
"github.com/openshift/source-to-image/pkg/util/fs"
16+
utillog "github.com/openshift/source-to-image/pkg/util/log"
17+
)
18+
19+
// External represents the shell out for external build commands, therefore s2i based build can
20+
// execute the generation of
21+
type External struct {
22+
dockerfile *dockerfile.Dockerfile
23+
}
24+
25+
// s2iDockerfile Dockerfile default filename.
26+
const s2iDockerfile = "Dockerfile.s2i"
27+
28+
var (
29+
// local logger
30+
log = utillog.StderrLog
31+
// supported external commands, template is based on api.Config instance
32+
commands = map[string]string{
33+
"buildah": `buildah bud --tag {{ .Tag }} --file {{ .AsDockerfile }} {{ or .WorkingDir "." }}`,
34+
"podman": `podman build --tag {{ .Tag }} --file {{ .AsDockerfile }} {{ or .WorkingDir "." }}`,
35+
}
36+
)
37+
38+
// GetBuilders returns a list of command names, based global commands map.
39+
func GetBuilders() []string {
40+
builders := []string{}
41+
for k := range commands {
42+
builders = append(builders, k)
43+
}
44+
return builders
45+
}
46+
47+
// ValidBuilderName returns a boolean based in keys of global commands map.
48+
func ValidBuilderName(name string) bool {
49+
_, exists := commands[name]
50+
return exists
51+
}
52+
53+
// renderCommand render a shell command based in api.Config instance. Attribute WithBuilder
54+
// wll determine external builder name, and api.Config feeds command's template variables. It can
55+
// return error in case of template parsing or evaluation issues.
56+
func (e *External) renderCommand(config *api.Config) (string, error) {
57+
commandTemplate, exists := commands[config.WithBuilder]
58+
if !exists {
59+
return "", fmt.Errorf("cannot find command '%s' in dictionary: '%#v'",
60+
config.WithBuilder, commands)
61+
}
62+
63+
t, err := template.New("external-command").Parse(commandTemplate)
64+
if err != nil {
65+
return "", err
66+
}
67+
var output bytes.Buffer
68+
if err = t.Execute(&output, config); err != nil {
69+
return "", err
70+
}
71+
return output.String(), nil
72+
}
73+
74+
// execute the given external command using "os/exec". Returns the outcomes as api.Result, making
75+
// sure it only marks result as success when exit-code is zero. Therefore, it returns errors based
76+
// in external command errors, so "s2i build" also fails.
77+
func (e *External) execute(externalCommand string) (*api.Result, error) {
78+
log.V(0).Infof("Executing external build command: '%s'", externalCommand)
79+
80+
externalCommandSlice := strings.Split(externalCommand, " ")
81+
cmd := exec.Command(externalCommandSlice[0], externalCommandSlice[1:]...)
82+
cmd.Stdout = os.Stdout
83+
cmd.Stderr = os.Stderr
84+
85+
res := &api.Result{Success: false}
86+
res.Messages = append(res.Messages, fmt.Sprintf("Running command: '%s'", externalCommand))
87+
err := cmd.Start()
88+
if err != nil {
89+
res.Messages = append(res.Messages, err.Error())
90+
return res, err
91+
}
92+
93+
if err := cmd.Wait(); err != nil {
94+
if exitErr, okay := err.(*exec.ExitError); okay {
95+
if status, okay := exitErr.Sys().(syscall.WaitStatus); okay {
96+
exitCode := status.ExitStatus()
97+
log.V(0).Infof("External command return-code: %d", exitCode)
98+
res.Messages = append(res.Messages, fmt.Sprintf("exit-code: %d", exitCode))
99+
if exitCode == 0 {
100+
res.Success = true
101+
} else {
102+
return res, exitErr
103+
}
104+
}
105+
} else {
106+
return res, err
107+
}
108+
}
109+
return res, nil
110+
}
111+
112+
// asDockerfile inspect config, if user has already informed `--as-dockerfile` option, that's simply
113+
// returned, otherwise, considering `--working-dir` option first before using artificial name.
114+
func (e *External) asDockerfile(config *api.Config) string {
115+
if len(config.AsDockerfile) > 0 {
116+
return config.AsDockerfile
117+
}
118+
119+
if len(config.WorkingDir) > 0 {
120+
return path.Join(config.WorkingDir, s2iDockerfile)
121+
}
122+
return s2iDockerfile
123+
}
124+
125+
// Build triggers the build of a "strategy/dockerfile" to obtain "AsDockerfile" first, and then
126+
// proceed to execute the external command.
127+
func (e *External) Build(config *api.Config) (*api.Result, error) {
128+
config.AsDockerfile = e.asDockerfile(config)
129+
130+
externalCommand, err := e.renderCommand(config)
131+
if err != nil {
132+
return nil, err
133+
}
134+
135+
// generating dockerfile following AsDockerfile directive
136+
err = e.dockerfile.CreateDockerfile(config)
137+
if err != nil {
138+
return nil, err
139+
}
140+
141+
return e.execute(externalCommand)
142+
}
143+
144+
// New instance of External command strategy.
145+
func New(config *api.Config, fs fs.FileSystem) (*External, error) {
146+
df, err := dockerfile.New(config, fs)
147+
if err != nil {
148+
return nil, err
149+
}
150+
return &External{dockerfile: df}, nil
151+
}
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
package external
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
"github.com/openshift/source-to-image/pkg/api"
8+
)
9+
10+
func TestExternalGetBuilders(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
want []string
14+
}{
15+
{"builders", []string{"buildah", "podman"}},
16+
}
17+
for _, tt := range tests {
18+
t.Run(tt.name, func(t *testing.T) {
19+
if got := GetBuilders(); !reflect.DeepEqual(got, tt.want) {
20+
t.Errorf("GetBuilders() = %v, want %v", got, tt.want)
21+
}
22+
})
23+
}
24+
}
25+
26+
func TestExternalValidBuilderName(t *testing.T) {
27+
tests := []struct {
28+
name string
29+
args string
30+
want bool
31+
}{
32+
{"valid-builder-name", "buildah", true},
33+
{"valid-builder-name", "podman", true},
34+
{"invalid-builder-name", "other", false},
35+
}
36+
for _, tt := range tests {
37+
t.Run(tt.name, func(t *testing.T) {
38+
if got := ValidBuilderName(tt.args); got != tt.want {
39+
t.Errorf("ValidBuilderName() = %v, want %v", got, tt.want)
40+
}
41+
})
42+
}
43+
}
44+
45+
func TestExternalRenderCommand(t *testing.T) {
46+
e := &External{}
47+
tests := []struct {
48+
name string
49+
config *api.Config
50+
want string
51+
wantErr bool
52+
}{
53+
{
54+
"render-buildah",
55+
&api.Config{WithBuilder: "buildah", Tag: "tag", AsDockerfile: "dockerfile"},
56+
"buildah bud --tag tag --file dockerfile .",
57+
false,
58+
},
59+
{
60+
"render-podman",
61+
&api.Config{WithBuilder: "podman", Tag: "tag", AsDockerfile: "dockerfile"},
62+
"podman build --tag tag --file dockerfile .",
63+
false,
64+
},
65+
}
66+
for _, tt := range tests {
67+
t.Run(tt.name, func(t *testing.T) {
68+
got, err := e.renderCommand(tt.config)
69+
if (err != nil) != tt.wantErr {
70+
t.Errorf("External.renderCommand() error = %v, wantErr %v", err, tt.wantErr)
71+
return
72+
}
73+
if got != tt.want {
74+
t.Errorf("External.renderCommand() = %v, want %v", got, tt.want)
75+
}
76+
})
77+
}
78+
}
79+
80+
func TestExternalAsDockerfile(t *testing.T) {
81+
e := &External{}
82+
tests := []struct {
83+
name string
84+
config *api.Config
85+
want string
86+
}{
87+
{
88+
"without-as-dockerfile-without-working-dir",
89+
&api.Config{},
90+
"Dockerfile.s2i",
91+
},
92+
{
93+
"without-as-dockerfile-with-working-dir",
94+
&api.Config{WorkingDir: "dir"},
95+
"dir/Dockerfile.s2i",
96+
},
97+
{
98+
"with-as-dockerfile-with-working-dir",
99+
&api.Config{AsDockerfile: "Dockerfile", WorkingDir: "dir"},
100+
"Dockerfile",
101+
},
102+
}
103+
for _, tt := range tests {
104+
t.Run(tt.name, func(t *testing.T) {
105+
if got := e.asDockerfile(tt.config); got != tt.want {
106+
t.Errorf("External.asDockerfile() = %v, want %v", got, tt.want)
107+
}
108+
})
109+
}
110+
}

pkg/build/strategies/strategies.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/openshift/source-to-image/pkg/api"
77
"github.com/openshift/source-to-image/pkg/build"
88
"github.com/openshift/source-to-image/pkg/build/strategies/dockerfile"
9+
"github.com/openshift/source-to-image/pkg/build/strategies/external"
910
"github.com/openshift/source-to-image/pkg/build/strategies/onbuild"
1011
"github.com/openshift/source-to-image/pkg/build/strategies/sti"
1112
"github.com/openshift/source-to-image/pkg/docker"
@@ -24,6 +25,18 @@ func Strategy(client docker.Client, config *api.Config, overrides build.Override
2425

2526
startTime := time.Now()
2627

28+
if len(config.WithBuilder) != 0 {
29+
builder, err = external.New(config, fileSystem)
30+
if err != nil {
31+
buildInfo.FailureReason = utilstatus.NewFailureReason(
32+
utilstatus.ReasonGenericS2IBuildFailed,
33+
utilstatus.ReasonMessageGenericS2iBuildFailed,
34+
)
35+
return nil, buildInfo, err
36+
}
37+
return builder, buildInfo, nil
38+
}
39+
2740
if len(config.AsDockerfile) != 0 {
2841
builder, err = dockerfile.New(config, fileSystem)
2942
if err != nil {

pkg/cmd/cli/cmd/build.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/openshift/source-to-image/pkg/api"
1111
"github.com/openshift/source-to-image/pkg/api/describe"
1212
"github.com/openshift/source-to-image/pkg/api/validation"
13+
"github.com/openshift/source-to-image/pkg/build"
1314
"github.com/openshift/source-to-image/pkg/build/strategies"
1415
"github.com/openshift/source-to-image/pkg/build/strategies/external"
1516
cmdutil "github.com/openshift/source-to-image/pkg/cmd/cli/util"
@@ -82,16 +83,10 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
8283
}
8384
}
8485

85-
if len(cfg.WithBuilder) > 0 {
86-
if len(cfg.AsDockerfile) == 0 {
87-
fmt.Fprintln(os.Stderr, "ERROR: --with-builder can only be used with --as-dockerfile")
88-
return
89-
}
90-
if !external.ValidBuilderName(cfg.WithBuilder) {
91-
fmt.Fprintf(os.Stderr, "ERROR: invalid --with-builder='%s', allowed options are: '[%v]'\n",
92-
cfg.WithBuilder, strings.Join(external.GetBuilders(), ", "))
93-
return
94-
}
86+
if len(cfg.WithBuilder) > 0 && !external.ValidBuilderName(cfg.WithBuilder) {
87+
fmt.Fprintf(os.Stderr, "ERROR: invalid --with-builder='%s', allowed options are: '[%v]'\n",
88+
cfg.WithBuilder, strings.Join(external.GetBuilders(), ", "))
89+
return
9590
}
9691

9792
if cfg.Incremental && len(cfg.RuntimeImage) > 0 {
@@ -166,7 +161,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
166161
log.Fatal(err)
167162
}
168163

169-
if len(cfg.AsDockerfile) == 0 {
164+
if len(cfg.AsDockerfile) == 0 && len(cfg.WithBuilder) == 0 {
170165
d := docker.New(client, cfg.PullAuthentication)
171166
err := d.CheckReachable()
172167
if err != nil {
@@ -176,7 +171,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
176171

177172
log.V(2).Infof("\n%s\n", describe.Config(client, cfg))
178173

179-
builder, _, err := strategies.GetStrategy(client, cfg)
174+
builder, _, err := strategies.Strategy(client, cfg, build.Overrides{})
180175
s2ierr.CheckError(err)
181176
result, err := builder.Build(cfg)
182177
if err != nil {
@@ -230,7 +225,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
230225
buildCmd.Flags().VarP(&(cfg.RuntimeArtifacts), "runtime-artifact", "a", "Specify a file or directory to be copied from the builder to the runtime image")
231226
buildCmd.Flags().StringVar(&(networkMode), "network", "", "Specify the default Docker Network name to be used in build process")
232227
buildCmd.Flags().StringVarP(&(cfg.AsDockerfile), "as-dockerfile", "", "", "EXPERIMENTAL: Output a Dockerfile to this path instead of building a new image")
233-
buildCmd.Flags().StringVarP(&(cfg.WithBuilder), "with-builder", "", "", "EXPERIMENTAL: Use a external builder against Dockerfile generated by --as-dockerfile. Supported builders: ["+strings.Join(external.GetBuilders(), ", ")+"]")
228+
buildCmd.Flags().StringVarP(&(cfg.WithBuilder), "with-builder", "", "", "EXPERIMENTAL: Use a external builder against Dockerfile generated by s2i. Supported builders: ["+strings.Join(external.GetBuilders(), ", ")+"]")
234229
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")
235230
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")
236231
return buildCmd

0 commit comments

Comments
 (0)