Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for experimental BuildKit #1111

Merged
merged 20 commits into from
Jun 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
656fe85
build: add experimental buildkit base
tonistiigi Apr 18, 2018
0f97642
build: basic buildkit progress support
tonistiigi Apr 19, 2018
b19294e
system: add buildcache formatting
tonistiigi May 18, 2018
8cf213b
build: use a separate upload request for early progress
tonistiigi May 19, 2018
e0b3921
build: Add support for using context from stdin with buildkit
May 19, 2018
5314a8f
build: Add support for using dockerfile from stdin with buildkit
May 22, 2018
89e1024
build: error out if buildkit is on and stdin is used for both dockerf…
May 22, 2018
82f0e1e
build: fix `-f` handling with buildkit
May 23, 2018
640cbb8
build: fix output handling with buildkit (quiet option, redirects)
May 24, 2018
b2b3f9c
build: setting DOCKER_BUILDKIT environment variable to any non-empty …
Jun 5, 2018
584d59d
formatter: fix TestDiskUsageContextFormatWrite expected output
Jun 6, 2018
ed75f62
build: add experimental --no-console flag to support non-tty human-re…
Jun 9, 2018
15674d9
build: simplify Close logic in WriteTempDockerfile
Jun 9, 2018
5919e8a
build: fix lint issues + refactor
Jun 9, 2018
aef4209
build: skip moby.buildkit.trace Aux message to be future proof
Jun 9, 2018
8945270
vendor: update buildkit and fsutil
tonistiigi Jun 10, 2018
6c60bb4
vendor: update docker/docker to c752b0991e31ba9869ab6a0661af57e9423874fb
Jun 12, 2018
00792d1
build: ensure temporary folder is removed in error case
Jun 13, 2018
5a103e1
build: change --no-console to --console=[true|false|auto]
Jun 13, 2018
b3a5c15
build: address some review nits
Jun 13, 2018
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
38 changes: 35 additions & 3 deletions cli/command/formatter/disk_usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ const (
defaultDiskUsageContainerTableFormat = "table {{.ID}}\t{{.Image}}\t{{.Command}}\t{{.LocalVolumes}}\t{{.Size}}\t{{.RunningFor}} ago\t{{.Status}}\t{{.Names}}"
defaultDiskUsageVolumeTableFormat = "table {{.Name}}\t{{.Links}}\t{{.Size}}"
defaultDiskUsageTableFormat = "table {{.Type}}\t{{.TotalCount}}\t{{.Active}}\t{{.Size}}\t{{.Reclaimable}}"
defaultBuildCacheVerboseFormat = `
ID: {{.ID}}
Description: {{.Description}}
Mutable: {{.Mutable}}
Size: {{.Size}}
CreatedAt: {{.CreatedAt}}
LastUsedAt: {{.LastUsedAt}}
UsageCount: {{.UsageCount}}
`

typeHeader = "TYPE"
totalHeader = "TOTAL"
Expand All @@ -34,6 +43,7 @@ type DiskUsageContext struct {
Images []*types.ImageSummary
Containers []*types.Container
Volumes []*types.Volume
BuildCache []*types.BuildCache
BuilderSize int64
}

Expand Down Expand Up @@ -100,6 +110,7 @@ func (ctx *DiskUsageContext) Write() (err error) {

err = ctx.contextFormat(tmpl, &diskUsageBuilderContext{
builderSize: ctx.BuilderSize,
buildCache: ctx.BuildCache,
})
if err != nil {
return err
Expand Down Expand Up @@ -184,6 +195,13 @@ func (ctx *DiskUsageContext) verboseWrite() error {

// And build cache
fmt.Fprintf(ctx.Output, "\nBuild cache usage: %s\n\n", units.HumanSize(float64(ctx.BuilderSize)))

t := template.Must(template.New("buildcache").Parse(defaultBuildCacheVerboseFormat))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be only visible when the daemon is experimental right ?


for _, v := range ctx.BuildCache {
t.Execute(ctx.Output, *v)
}

return nil
}

Expand Down Expand Up @@ -366,6 +384,7 @@ func (c *diskUsageVolumesContext) Reclaimable() string {
type diskUsageBuilderContext struct {
HeaderContext
builderSize int64
buildCache []*types.BuildCache
}

func (c *diskUsageBuilderContext) MarshalJSON() ([]byte, error) {
Expand All @@ -377,17 +396,30 @@ func (c *diskUsageBuilderContext) Type() string {
}

func (c *diskUsageBuilderContext) TotalCount() string {
return ""
return fmt.Sprintf("%d", len(c.buildCache))
}

func (c *diskUsageBuilderContext) Active() string {
return ""
numActive := 0
for _, bc := range c.buildCache {
if bc.InUse {
numActive++
}
}
return fmt.Sprintf("%d", numActive)
}

func (c *diskUsageBuilderContext) Size() string {
return units.HumanSize(float64(c.builderSize))
}

func (c *diskUsageBuilderContext) Reclaimable() string {
return c.Size()
var inUseBytes int64
for _, bc := range c.buildCache {
if bc.InUse {
inUseBytes += bc.Size
}
}

return units.HumanSize(float64(c.builderSize - inUseBytes))
}
4 changes: 2 additions & 2 deletions cli/command/formatter/disk_usage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestDiskUsageContextFormatWrite(t *testing.T) {
Images 0 0 0B 0B
Containers 0 0 0B 0B
Local Volumes 0 0 0B 0B
Build Cache 0B 0B
Build Cache 0 0 0B 0B
`,
},
{
Expand Down Expand Up @@ -76,7 +76,7 @@ Build cache usage: 0B
Images 0 0 0B 0B
Containers 0 0 0B 0B
Local Volumes 0 0 0B 0B
Build Cache 0B 0B
Build Cache 0 0 0B 0B
`,
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ TYPE ACTIVE
Images 0
Containers 0
Local Volumes 0
Build Cache
Build Cache 0
4 changes: 2 additions & 2 deletions cli/command/formatter/testdata/disk-usage-raw-format.golden
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ size: 0B
reclaimable: 0B

type: Build Cache
total:
active:
total: 0
active: 0
size: 0B
reclaimable: 0B

84 changes: 50 additions & 34 deletions cli/command/image/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import (
"github.com/spf13/cobra"
)

var errStdinConflict = errors.New("invalid argument: can't use stdin for both build context and dockerfile")

type buildOptions struct {
context string
dockerfileName string
Expand All @@ -55,6 +57,7 @@ type buildOptions struct {
isolation string
quiet bool
noCache bool
console opts.NullableBool
rm bool
forceRm bool
pull bool
Expand Down Expand Up @@ -149,6 +152,9 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
flags.SetAnnotation("stream", "experimental", nil)
flags.SetAnnotation("stream", "version", []string{"1.31"})

flags.Var(&options.console, "console", "Show console output (with buildkit only) (true, false, auto)")
flags.SetAnnotation("console", "experimental", nil)
flags.SetAnnotation("console", "version", []string{"1.38"})
return cmd
}

Expand All @@ -170,6 +176,10 @@ func (out *lastProgressOutput) WriteProgress(prog progress.Progress) error {

// nolint: gocyclo
func runBuild(dockerCli command.Cli, options buildOptions) error {
if os.Getenv("DOCKER_BUILDKIT") != "" {
return runBuildBuildKit(dockerCli, options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason not to use CLI flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AkihiroSuda because in the future I'd like to remove it.

}

var (
buildCtx io.ReadCloser
dockerfileCtx io.ReadCloser
Expand All @@ -188,7 +198,7 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {

if options.dockerfileFromStdin() {
if options.contextFromStdin() {
return errors.New("invalid argument: can't use stdin for both build context and dockerfile")
return errStdinConflict
}
dockerfileCtx = dockerCli.In()
}
Expand Down Expand Up @@ -362,37 +372,11 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {

configFile := dockerCli.ConfigFile()
authConfigs, _ := configFile.GetAllCredentials()
buildOptions := types.ImageBuildOptions{
Memory: options.memory.Value(),
MemorySwap: options.memorySwap.Value(),
Tags: options.tags.GetAll(),
SuppressOutput: options.quiet,
NoCache: options.noCache,
Remove: options.rm,
ForceRemove: options.forceRm,
PullParent: options.pull,
Isolation: container.Isolation(options.isolation),
CPUSetCPUs: options.cpuSetCpus,
CPUSetMems: options.cpuSetMems,
CPUShares: options.cpuShares,
CPUQuota: options.cpuQuota,
CPUPeriod: options.cpuPeriod,
CgroupParent: options.cgroupParent,
Dockerfile: relDockerfile,
ShmSize: options.shmSize.Value(),
Ulimits: options.ulimits.GetList(),
BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
AuthConfigs: authConfigs,
Labels: opts.ConvertKVStringsToMap(options.labels.GetAll()),
CacheFrom: options.cacheFrom,
SecurityOpt: options.securityOpt,
NetworkMode: options.networkMode,
Squash: options.squash,
ExtraHosts: options.extraHosts.GetAll(),
Target: options.target,
RemoteContext: remote,
Platform: options.platform,
}
buildOptions := imageBuildOptions(dockerCli, options)
buildOptions.Version = types.BuilderV1
buildOptions.Dockerfile = relDockerfile
buildOptions.AuthConfigs = authConfigs
buildOptions.RemoteContext = remote

if s != nil {
go func() {
Expand All @@ -416,9 +400,9 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
defer response.Body.Close()

imageID := ""
aux := func(m jsonmessage.JSONMessage) {
aux := func(msg jsonmessage.JSONMessage) {
var result types.BuildResult
if err := json.Unmarshal(*m.Aux, &result); err != nil {
if err := json.Unmarshal(*msg.Aux, &result); err != nil {
fmt.Fprintf(dockerCli.Err(), "Failed to parse aux message: %s", err)
} else {
imageID = result.ID
Expand Down Expand Up @@ -602,3 +586,35 @@ func replaceDockerfileForContentTrust(ctx context.Context, inputTarStream io.Rea

return pipeReader
}

func imageBuildOptions(dockerCli command.Cli, options buildOptions) types.ImageBuildOptions {
configFile := dockerCli.ConfigFile()
return types.ImageBuildOptions{
Memory: options.memory.Value(),
MemorySwap: options.memorySwap.Value(),
Tags: options.tags.GetAll(),
SuppressOutput: options.quiet,
NoCache: options.noCache,
Remove: options.rm,
ForceRemove: options.forceRm,
PullParent: options.pull,
Isolation: container.Isolation(options.isolation),
CPUSetCPUs: options.cpuSetCpus,
CPUSetMems: options.cpuSetMems,
CPUShares: options.cpuShares,
CPUQuota: options.cpuQuota,
CPUPeriod: options.cpuPeriod,
CgroupParent: options.cgroupParent,
ShmSize: options.shmSize.Value(),
Ulimits: options.ulimits.GetList(),
BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
Labels: opts.ConvertKVStringsToMap(options.labels.GetAll()),
CacheFrom: options.cacheFrom,
SecurityOpt: options.securityOpt,
NetworkMode: options.networkMode,
Squash: options.squash,
ExtraHosts: options.extraHosts.GetAll(),
Target: options.target,
Platform: options.platform,
}
}
73 changes: 48 additions & 25 deletions cli/command/image/build/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,59 +81,82 @@ func ValidateContextDirectory(srcPath string, excludes []string) error {
})
}

// GetContextFromReader will read the contents of the given reader as either a
// Dockerfile or tar archive. Returns a tar archive used as a context and a
// path to the Dockerfile inside the tar.
func GetContextFromReader(r io.ReadCloser, dockerfileName string) (out io.ReadCloser, relDockerfile string, err error) {
buf := bufio.NewReader(r)
// DetectArchiveReader detects whether the input stream is an archive or a
// Dockerfile and returns a buffered version of input, safe to consume in lieu
// of input. If an archive is detected, isArchive is set to true, and to false
// otherwise, in which case it is safe to assume input represents the contents
// of a Dockerfile.
func DetectArchiveReader(input io.ReadCloser) (rc io.ReadCloser, isArchive bool, err error) {
buf := bufio.NewReader(input)

magic, err := buf.Peek(archiveHeaderSize)
if err != nil && err != io.EOF {
return nil, "", errors.Errorf("failed to peek context header from STDIN: %v", err)
return nil, false, errors.Errorf("failed to peek context header from STDIN: %v", err)
}

if IsArchive(magic) {
return ioutils.NewReadCloserWrapper(buf, func() error { return r.Close() }), dockerfileName, nil
}
return ioutils.NewReadCloserWrapper(buf, func() error { return input.Close() }), IsArchive(magic), nil
}

if dockerfileName == "-" {
return nil, "", errors.New("build context is not an archive")
// WriteTempDockerfile writes a Dockerfile stream to a temporary file with a
// name specified by DefaultDockerfileName and returns the path to the
// temporary directory containing the Dockerfile.
func WriteTempDockerfile(rc io.ReadCloser) (dockerfileDir string, err error) {
// err is a named return value, due to the defer call below.
Copy link
Member

Choose a reason for hiding this comment

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

was thinking naming it retErr (eg), but just a nit; no need to change

dockerfileDir, err = ioutil.TempDir("", "docker-build-tempdockerfile-")
if err != nil {
return "", errors.Errorf("unable to create temporary context directory: %v", err)
}
defer func() {
if err != nil {
os.RemoveAll(dockerfileDir)
}
}()

// Input should be read as a Dockerfile.
tmpDir, err := ioutil.TempDir("", "docker-build-context-")
f, err := os.Create(filepath.Join(dockerfileDir, DefaultDockerfileName))
if err != nil {
return nil, "", errors.Errorf("unable to create temporary context directory: %v", err)
return "", err
}
defer f.Close()
if _, err := io.Copy(f, rc); err != nil {
return "", err
}
return dockerfileDir, rc.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

If rc.Close() returns an error then the callers will leak tmpDir, since they don't clean it up in the error case (and it would be a little odd to expect them to do so).

The dir is also leaked on errors in in the os.Create above too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ijc Addressed.

}

f, err := os.Create(filepath.Join(tmpDir, DefaultDockerfileName))
// GetContextFromReader will read the contents of the given reader as either a
// Dockerfile or tar archive. Returns a tar archive used as a context and a
// path to the Dockerfile inside the tar.
func GetContextFromReader(rc io.ReadCloser, dockerfileName string) (out io.ReadCloser, relDockerfile string, err error) {
rc, isArchive, err := DetectArchiveReader(rc)
if err != nil {
return nil, "", err
}
_, err = io.Copy(f, buf)
if err != nil {
f.Close()
return nil, "", err

if isArchive {
return rc, dockerfileName, nil
}

if err := f.Close(); err != nil {
return nil, "", err
// Input should be read as a Dockerfile.

if dockerfileName == "-" {
return nil, "", errors.New("build context is not an archive")
}
if err := r.Close(); err != nil {

dockerfileDir, err := WriteTempDockerfile(rc)
if err != nil {
return nil, "", err
}

tar, err := archive.Tar(tmpDir, archive.Uncompressed)
tar, err := archive.Tar(dockerfileDir, archive.Uncompressed)
if err != nil {
return nil, "", err
}

return ioutils.NewReadCloserWrapper(tar, func() error {
err := tar.Close()
os.RemoveAll(tmpDir)
os.RemoveAll(dockerfileDir)
return err
}), DefaultDockerfileName, nil

}

// IsArchive checks for the magic bytes of a tar or any supported compression
Expand Down
Loading