From 14215ed5a1900a88a3b17dd7cd566def50bfcbc9 Mon Sep 17 00:00:00 2001 From: Anusha Ragunathan Date: Tue, 12 Jan 2016 14:38:55 -0800 Subject: [PATCH] Make daemonbuilder.Docker leaner. Currently builder.Backend is implemented by daemonbuilder.Docker{} for the daemon. This registration happens in the API/server code. However, this is too implementation specific. Ideally we should be able to specify that docker daemon (or any other) is implementing the Backend and abstract the implementation details. So we should remove package daemonbuilder dependency in build_routes.go With this change, daemonbuilder.Docker is nothing more than the daemon. A follow on change will remove the daemonbuilder package and move relevant methods under daemon, so that API only knows about the backend. Also cleanup code in api/client/build.go. docker cli always performs build context tar download for remoteURLs and sends an empty remoteContext. So remove relevant dead code. Signed-off-by: Anusha Ragunathan --- api/client/build.go | 11 ++--------- api/server/router/build/build_routes.go | 26 ++++++------------------- builder/builder.go | 2 +- builder/dockerfile/builder.go | 3 ++- builder/dockerfile/dispatchers.go | 2 +- daemon/daemonbuilder/builder.go | 25 ++++++++++++++---------- 6 files changed, 27 insertions(+), 42 deletions(-) diff --git a/api/client/build.go b/api/client/build.go index 7fb3391ec8..e6b09845a8 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -77,9 +77,8 @@ func (cli *DockerCli) CmdBuild(args ...string) error { cmd.ParseFlags(args, true) var ( - context io.ReadCloser - isRemote bool - err error + context io.ReadCloser + err error ) _, err = exec.LookPath("git") @@ -215,18 +214,12 @@ func (cli *DockerCli) CmdBuild(args ...string) error { } } - var remoteContext string - if isRemote { - remoteContext = cmd.Arg(0) - } - options := types.ImageBuildOptions{ Context: body, Memory: memory, MemorySwap: memorySwap, Tags: flTags.GetAll(), SuppressOutput: *suppressOutput, - RemoteContext: remoteContext, NoCache: *noCache, Remove: *rm, ForceRemove: *forceRm, diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index 2962d87f35..aa33fc071b 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -16,8 +16,6 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerfile" "github.com/docker/docker/daemon/daemonbuilder" - "github.com/docker/docker/pkg/archive" - "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" @@ -206,31 +204,19 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * buildOptions.Dockerfile = dockerfileName } - uidMaps, gidMaps := br.backend.GetUIDGIDMaps() - defaultArchiver := &archive.Archiver{ - Untar: chrootarchive.Untar, - UIDMaps: uidMaps, - GIDMaps: gidMaps, - } - - docker := &daemonbuilder.Docker{ - Daemon: br.backend, - OutOld: output, - AuthConfigs: authConfigs, - Archiver: defaultArchiver, - } - if buildOptions.SuppressOutput { - docker.OutOld = notVerboseBuffer - } - b, err := dockerfile.NewBuilder( buildOptions, // result of newBuildConfig - docker, + &daemonbuilder.Docker{br.backend}, builder.DockerIgnoreContext{ModifiableContext: context}, nil) if err != nil { return errf(err) } + if buildOptions.SuppressOutput { + b.Output = notVerboseBuffer + } else { + b.Output = output + } b.Stdout = &streamformatter.StdoutFormatter{Writer: output, StreamFormatter: sf} b.Stderr = &streamformatter.StderrFormatter{Writer: output, StreamFormatter: sf} if buildOptions.SuppressOutput { diff --git a/builder/builder.go b/builder/builder.go index 0defa613dc..5ef8692950 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -101,7 +101,7 @@ type Backend interface { // GetImage looks up a Docker image referenced by `name`. GetImage(name string) (Image, error) // Pull tells Docker to pull image referenced by `name`. - Pull(name string) (Image, error) + Pull(name string, authConfigs map[string]types.AuthConfig, output io.Writer) (Image, error) // ContainerAttach attaches to container. ContainerAttach(cID string, stdin io.ReadCloser, stdout, stderr io.Writer, stream bool) error // ContainerCreate creates a new Docker container and returns potential warnings diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 70d3db01df..3d38416a19 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -67,7 +67,8 @@ type Builder struct { allowedBuildArgs map[string]bool // list of build-time args that are allowed for expansion/substitution and passing to commands in 'run'. // TODO: remove once docker.Commit can receive a tag - id string + id string + Output io.Writer } // NewBuilder creates a new Dockerfile builder from an optional dockerfile and a Config. diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index ea674edc5c..7094cfb927 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -212,7 +212,7 @@ func from(b *Builder, args []string, attributes map[string]bool, original string // TODO: shouldn't we error out if error is different from "not found" ? } if image == nil { - image, err = b.docker.Pull(name) + image, err = b.docker.Pull(name, b.options.AuthConfigs, b.Output) if err != nil { return err } diff --git a/daemon/daemonbuilder/builder.go b/daemon/daemonbuilder/builder.go index fba6039091..6e29447b24 100644 --- a/daemon/daemonbuilder/builder.go +++ b/daemon/daemonbuilder/builder.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/daemon" "github.com/docker/docker/image" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/httputils" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/ioutils" @@ -27,16 +28,13 @@ import ( // Docker implements builder.Backend for the docker Daemon object. type Docker struct { *daemon.Daemon - OutOld io.Writer - AuthConfigs map[string]types.AuthConfig - Archiver *archive.Archiver } // ensure Docker implements builder.Backend var _ builder.Backend = Docker{} // Pull tells Docker to pull image referenced by `name`. -func (d Docker) Pull(name string) (builder.Image, error) { +func (d Docker) Pull(name string, authConfigs map[string]types.AuthConfig, output io.Writer) (builder.Image, error) { ref, err := reference.ParseNamed(name) if err != nil { return nil, err @@ -44,7 +42,7 @@ func (d Docker) Pull(name string) (builder.Image, error) { ref = reference.WithDefaultTag(ref) pullRegistryAuth := &types.AuthConfig{} - if len(d.AuthConfigs) > 0 { + if len(authConfigs) > 0 { // The request came with a full auth config file, we prefer to use that repoInfo, err := d.Daemon.RegistryService.ResolveRepository(ref) if err != nil { @@ -52,13 +50,13 @@ func (d Docker) Pull(name string) (builder.Image, error) { } resolvedConfig := registry.ResolveAuthConfig( - d.AuthConfigs, + authConfigs, repoInfo.Index, ) pullRegistryAuth = &resolvedConfig } - if err := d.Daemon.PullImage(ref, nil, pullRegistryAuth, ioutils.NopWriteCloser(d.OutOld)); err != nil { + if err := d.Daemon.PullImage(ref, nil, pullRegistryAuth, ioutils.NopWriteCloser(output)); err != nil { return nil, err } return d.GetImage(name) @@ -140,9 +138,16 @@ func (d Docker) BuilderCopy(cID string, destPath string, src builder.FileInfo, d destExists = false } + uidMaps, gidMaps := d.Daemon.GetUIDGIDMaps() + archiver := &archive.Archiver{ + Untar: chrootarchive.Untar, + UIDMaps: uidMaps, + GIDMaps: gidMaps, + } + if src.IsDir() { // copy as directory - if err := d.Archiver.CopyWithTar(srcPath, destPath); err != nil { + if err := archiver.CopyWithTar(srcPath, destPath); err != nil { return err } return fixPermissions(srcPath, destPath, rootUID, rootGID, destExists) @@ -160,7 +165,7 @@ func (d Docker) BuilderCopy(cID string, destPath string, src builder.FileInfo, d } // try to successfully untar the orig - err := d.Archiver.UntarPath(srcPath, tarDest) + err := archiver.UntarPath(srcPath, tarDest) if err != nil { logrus.Errorf("Couldn't untar to %s: %v", tarDest, err) } @@ -175,7 +180,7 @@ func (d Docker) BuilderCopy(cID string, destPath string, src builder.FileInfo, d if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, rootUID, rootGID); err != nil { return err } - if err := d.Archiver.CopyFileWithTar(srcPath, destPath); err != nil { + if err := archiver.CopyFileWithTar(srcPath, destPath); err != nil { return err }