Skip to content

Commit 7ccdc44

Browse files
kcqkcq
committed
imagebuild cleanup and fixes
Signed-off-by: Kyle Quest <[email protected]>
1 parent 255f553 commit 7ccdc44

File tree

7 files changed

+66
-119
lines changed

7 files changed

+66
-119
lines changed

‎pkg/app/master/command/cliprompt.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
type InteractiveApp struct {
2727
appPrompt *prompt.Prompt
2828
fpCompleter completer.FilePathCompleter
29+
dpCompleter completer.FilePathCompleter
2930
app *cli.App
3031
dclient *dockerapi.Client
3132
crtConnection string
@@ -38,6 +39,10 @@ func NewInteractiveApp(app *cli.App, gparams *GenericParams) *InteractiveApp {
3839
fpCompleter: completer.FilePathCompleter{
3940
IgnoreCase: true,
4041
},
42+
dpCompleter: completer.FilePathCompleter{
43+
IgnoreCase: true,
44+
Filter: func(fi os.FileInfo) bool { return fi.IsDir() },
45+
},
4146
}
4247

4348
ia.appPrompt = prompt.New(
@@ -507,6 +512,10 @@ func CompleteFile(ia *InteractiveApp, token string, params prompt.Document) []pr
507512
return ia.fpCompleter.Complete(params)
508513
}
509514

515+
func CompleteDir(ia *InteractiveApp, token string, params prompt.Document) []prompt.Suggest {
516+
return ia.dpCompleter.Complete(params)
517+
}
518+
510519
// Runtime
511520

512521
var runtimeValues = []prompt.Suggest{

‎pkg/app/master/command/imagebuild/cli.go‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package imagebuild
33
import (
44
"fmt"
55
"os"
6+
"path/filepath"
67
"strings"
78

9+
log "github.com/sirupsen/logrus"
810
"github.com/urfave/cli/v2"
911

1012
"github.com/mintoolkit/mint/pkg/app"
@@ -43,8 +45,11 @@ var CLI = &cli.Command{
4345
Usage: Usage,
4446
Flags: ImageBuildFlags,
4547
Action: func(ctx *cli.Context) error {
48+
logger := log.WithFields(log.Fields{"app": command.AppName, "cmd": Name, "op": "cli.Action"})
49+
4650
gcvalues, ok := command.CLIContextGet(ctx.Context, command.GlobalParams).(*command.GenericParams)
4751
if !ok || gcvalues == nil {
52+
logger.Error("no gcvalues")
4853
return command.ErrNoGlobalParams
4954
}
5055

@@ -64,6 +69,7 @@ var CLI = &cli.Command{
6469
ContextDir: ctx.String(FlagContextDir),
6570
Runtime: ctx.String(FlagRuntimeLoad),
6671
Architecture: ctx.String(FlagArchitecture),
72+
Labels: map[string]string{},
6773
}
6874

6975
cboBuildArgs := command.ParseKVParams(ctx.StringSlice(FlagBuildArg))
@@ -79,26 +85,39 @@ var CLI = &cli.Command{
7985

8086
engineProps, found := BuildEngines[cparams.Engine]
8187
if !found {
88+
logger.Errorf("engine not found - %s", cparams.Engine)
8289
return command.ErrBadParamValue
8390
}
8491

8592
if cparams.Dockerfile == "" {
8693
cparams.Dockerfile = DefaultDockerfilePath
8794
}
8895

89-
if !fsutil.Exists(cparams.Dockerfile) {
96+
if !fsutil.DirExists(cparams.ContextDir) {
97+
logger.Errorf("context dir not found - %s", cparams.ContextDir)
9098
return command.ErrBadParamValue
9199
}
92100

93-
if !fsutil.DirExists(cparams.ContextDir) {
94-
return command.ErrBadParamValue
101+
switch cparams.Engine {
102+
case BuildkitBuildEngine, DepotBuildEngine:
103+
if !fsutil.Exists(cparams.Dockerfile) {
104+
logger.Errorf("Dockerfile not found - '%s'", cparams.Dockerfile)
105+
return command.ErrBadParamValue
106+
}
107+
default:
108+
fullDockerfilePath := filepath.Join(cparams.ContextDir, cparams.Dockerfile)
109+
if !fsutil.Exists(fullDockerfilePath) {
110+
logger.Errorf("Dockerfile not found - '%s' ('%s')", cparams.Dockerfile, fullDockerfilePath)
111+
return command.ErrBadParamValue
112+
}
95113
}
96114

97115
if cparams.Architecture == "" {
98116
cparams.Architecture = GetDefaultBuildArch()
99117
}
100118

101119
if !IsArchValue(cparams.Architecture) {
120+
logger.Errorf("architecture not supported - %s", cparams.Architecture)
102121
return command.ErrBadParamValue
103122
}
104123

‎pkg/app/master/command/imagebuild/flags.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const (
3131
FlagImageArchiveFileUsage = "local file path for the image tar archive file"
3232

3333
FlagDockerfile = "dockerfile"
34-
FlagDockerfileUsage = "local Dockerfile path"
34+
FlagDockerfileUsage = "local Dockerfile path (for buildkit and depot) or a relative to the build context directory (for docker or podman)"
3535

3636
FlagContextDir = "context-dir"
3737
FlagContextDirUsage = "local build context directory"

‎pkg/app/master/command/imagebuild/prompt.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ var CommandFlagSuggestions = &command.FlagSuggestions{
5454
{Text: command.FullFlagName(FlagDockerfile), Description: FlagDockerfileUsage},
5555
{Text: command.FullFlagName(FlagContextDir), Description: FlagContextDirUsage},
5656
{Text: command.FullFlagName(FlagBuildArg), Description: FlagBuildArgUsage},
57+
{Text: command.FullFlagName(FlagLabel), Description: FlagLabelUsage},
5758
{Text: command.FullFlagName(FlagArchitecture), Description: FlagArchitectureUsage},
5859
},
5960
Values: map[string]command.CompleteValue{
6061
command.FullFlagName(FlagEngine): completeBuildEngine,
6162
command.FullFlagName(FlagRuntimeLoad): completeRuntimeLoad,
6263
command.FullFlagName(FlagArchitecture): completeArchitecture,
64+
command.FullFlagName(FlagContextDir): command.CompleteDir,
6365
},
6466
}

‎pkg/crt/docker/dockercrtclient/dockercrtclient.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
6363
options.Labels = labels
6464
}
6565

66+
if options.BuildContext == "" {
67+
options.BuildContext = "."
68+
}
69+
6670
//not using options.CacheTo in this image builder...
6771
buildOptions := docker.BuildImageOptions{
6872
Dockerfile: options.Dockerfile,
@@ -90,21 +94,16 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
9094
} else {
9195
if exists := fsutil.DirExists(options.BuildContext); exists {
9296
buildOptions.ContextDir = options.BuildContext
97+
//Dockerfile path is expected to be relative to build context
98+
fullDockerfileName := filepath.Join(buildOptions.ContextDir, buildOptions.Dockerfile)
99+
if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
100+
return fmt.Errorf("invalid dockerfile reference (%s) - %s", buildOptions.Dockerfile, fullDockerfileName)
101+
}
93102
} else {
94103
return imagebuilder.ErrInvalidContextDir
95104
}
96105
}
97106

98-
if !fsutil.Exists(buildOptions.Dockerfile) || !fsutil.IsRegularFile(buildOptions.Dockerfile) {
99-
//a slightly hacky behavior using the build context directory if the dockerfile flag doesn't include a usable path
100-
fullDockerfileName := filepath.Join(buildOptions.ContextDir, buildOptions.Dockerfile)
101-
if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
102-
return fmt.Errorf("invalid dockerfile reference - %s", fullDockerfileName)
103-
}
104-
105-
buildOptions.Dockerfile = fullDockerfileName
106-
}
107-
108107
if options.OutputStream != nil {
109108
buildOptions.OutputStream = options.OutputStream
110109
} else if ref.showBuildLogs {

‎pkg/crt/podman/podmancrtclient/podmancrtclient.go‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
7777
}
7878
}
7979

80+
if options.BuildContext == "" {
81+
options.BuildContext = "."
82+
}
83+
8084
var contextDir string
8185
if strings.HasPrefix(options.BuildContext, "http://") ||
8286
strings.HasPrefix(options.BuildContext, "https://") {
@@ -86,20 +90,16 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
8690
} else {
8791
if exists := fsutil.DirExists(options.BuildContext); exists {
8892
contextDir = options.BuildContext
93+
//Dockerfile path is expected to be relative to build context
94+
fullDockerfileName := filepath.Join(contextDir, options.Dockerfile)
95+
if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
96+
return fmt.Errorf("invalid dockerfile reference (%s) - %s", options.Dockerfile, fullDockerfileName)
97+
}
8998
} else {
9099
return imagebuilder.ErrInvalidContextDir
91100
}
92101
}
93102

94-
fullDockerfileName := options.Dockerfile
95-
if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
96-
//a slightly hacky behavior using the build context directory if the dockerfile flag doesn't include a usable path
97-
fullDockerfileName = filepath.Join(contextDir, fullDockerfileName)
98-
if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
99-
return fmt.Errorf("invalid dockerfile reference - %s", fullDockerfileName)
100-
}
101-
}
102-
103103
buildOptions := images.BuildOptions{
104104
BuildOptions: buildahDefine.BuildOptions{
105105
Output: options.ImagePath,
@@ -110,15 +110,18 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
110110
RemoveIntermediateCtrs: true,
111111
PullPolicy: buildahDefine.PullIfMissing,
112112
OutputFormat: buildahDefine.Dockerv2ImageManifest, //buildah.OCIv1ImageManifest
113-
CommonBuildOpts: &buildahDefine.CommonBuildOptions{
114-
AddHost: strings.Split(options.ExtraHosts, ","),
113+
CommonBuildOpts: &buildahDefine.CommonBuildOptions{
114+
//AddHost: strings.Split(options.ExtraHosts, ","),
115115
},
116-
117-
//ConfigureNetwork: tbd <- options.NetworkMode
116+
ConfigureNetwork: buildahDefine.NetworkDefault, //tbd <- options.NetworkMode
118117
//CacheFrom: tbd <- options.CacheFrom
119118
//CacheTo: tbd <- options.CacheFrom
120119
},
121-
ContainerFiles: []string{fullDockerfileName},
120+
ContainerFiles: []string{options.Dockerfile},
121+
}
122+
123+
if options.ExtraHosts != "" {
124+
buildOptions.CommonBuildOpts.AddHost = strings.Split(options.ExtraHosts, ",")
122125
}
123126

124127
if len(options.Platforms) > 0 {
@@ -149,8 +152,11 @@ func (ref *Instance) BuildImage(options imagebuilder.DockerfileBuildOptions) err
149152
}
150153
}
151154

152-
for _, nv := range options.BuildArgs {
153-
buildOptions.Args[nv.Name] = nv.Value
155+
if len(options.BuildArgs) > 0 {
156+
buildOptions.Args = map[string]string{}
157+
for _, nv := range options.BuildArgs {
158+
buildOptions.Args[nv.Name] = nv.Value
159+
}
154160
}
155161

156162
if options.OutputStream != nil {

‎pkg/imagebuilder/standardbuilder/engine.go‎

Lines changed: 1 addition & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,8 @@
11
package standardbuilder
22

33
import (
4-
//"bytes"
5-
//"fmt"
6-
//"path/filepath"
7-
//"strings"
8-
9-
//docker "github.com/fsouza/go-dockerclient"
10-
114
"github.com/mintoolkit/mint/pkg/crt"
125
"github.com/mintoolkit/mint/pkg/imagebuilder"
13-
//"github.com/mintoolkit/mint/pkg/util/fsutil"
146
)
157

168
const (
@@ -52,89 +44,9 @@ func (ref *Engine) Name() string {
5244
}
5345

5446
func (ref *Engine) BuildLog() string {
55-
return ref.pclient.BuildOutputLog() //ref.buildLog.String()
47+
return ref.pclient.BuildOutputLog()
5648
}
5749

5850
func (ref *Engine) Build(options imagebuilder.DockerfileBuildOptions) error {
5951
return ref.pclient.BuildImage(options)
60-
/*
61-
if len(options.Labels) > 0 {
62-
//Docker has a limit on how long the labels can be
63-
labels := map[string]string{}
64-
for k, v := range options.Labels {
65-
lineLen := len(k) + len(v) + 7
66-
if lineLen > 65535 {
67-
//TODO: improve JSON data splitting
68-
valueLen := len(v)
69-
parts := valueLen / 50000
70-
parts++
71-
offset := 0
72-
for i := 0; i < parts && offset < valueLen; i++ {
73-
chunkSize := 50000
74-
if (offset + chunkSize) > valueLen {
75-
chunkSize = valueLen - offset
76-
}
77-
value := v[offset:(offset + chunkSize)]
78-
offset += chunkSize
79-
key := fmt.Sprintf("%s.%d", k, i)
80-
labels[key] = value
81-
}
82-
} else {
83-
labels[k] = v
84-
}
85-
}
86-
options.Labels = labels
87-
}
88-
89-
//not using options.CacheTo in this image builder...
90-
buildOptions := docker.BuildImageOptions{
91-
Dockerfile: options.Dockerfile,
92-
Target: options.Target,
93-
Name: options.ImagePath,
94-
95-
NetworkMode: options.NetworkMode,
96-
ExtraHosts: options.ExtraHosts,
97-
CacheFrom: options.CacheFrom,
98-
Labels: options.Labels,
99-
RmTmpContainer: true,
100-
}
101-
102-
if len(options.Platforms) > 0 {
103-
buildOptions.Platform = strings.Join(options.Platforms, ",")
104-
}
105-
106-
for _, nv := range options.BuildArgs {
107-
buildOptions.BuildArgs = append(buildOptions.BuildArgs, docker.BuildArg{Name: nv.Name, Value: nv.Value})
108-
}
109-
110-
if strings.HasPrefix(options.BuildContext, "http://") ||
111-
strings.HasPrefix(options.BuildContext, "https://") {
112-
buildOptions.Remote = options.BuildContext
113-
} else {
114-
if exists := fsutil.DirExists(options.BuildContext); exists {
115-
buildOptions.ContextDir = options.BuildContext
116-
} else {
117-
return imagebuilder.ErrInvalidContextDir
118-
}
119-
}
120-
121-
if !fsutil.Exists(buildOptions.Dockerfile) || !fsutil.IsRegularFile(buildOptions.Dockerfile) {
122-
//a slightly hacky behavior using the build context directory if the dockerfile flag doesn't include a usable path
123-
fullDockerfileName := filepath.Join(buildOptions.ContextDir, buildOptions.Dockerfile)
124-
if !fsutil.Exists(fullDockerfileName) || !fsutil.IsRegularFile(fullDockerfileName) {
125-
return fmt.Errorf("invalid dockerfile reference - %s", fullDockerfileName)
126-
}
127-
128-
buildOptions.Dockerfile = fullDockerfileName
129-
}
130-
131-
if options.OutputStream != nil {
132-
buildOptions.OutputStream = options.OutputStream
133-
} else if ref.showBuildLogs {
134-
ref.buildLog.Reset()
135-
buildOptions.OutputStream = &ref.buildLog
136-
}
137-
138-
return ref.pclient.BuildImage(buildOptions)
139-
*/
14052
}

0 commit comments

Comments
 (0)