From 0296797f0f39477d675128c93c1646b3186937ee Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 13 Apr 2017 14:37:32 -0400 Subject: [PATCH] Extract squash and tagging from the Dockerfile builder. Remove pathCache and replace it with syncmap Cleanup NewBuilder Create an api/server/backend/build Extract BuildTagger Signed-off-by: Daniel Nephin --- api/server/backend/build/backend.go | 70 ++++++++ api/server/backend/build/tag.go | 77 +++++++++ api/server/router/build/backend.go | 15 +- api/server/router/build/build.go | 7 +- api/server/router/build/build_routes.go | 104 +++++++----- api/types/backend/backend.go | 9 - api/types/backend/build.go | 23 +++ builder/builder.go | 16 +- builder/dockerfile/builder.go | 210 ++++++++---------------- builder/dockerfile/dispatchers.go | 2 +- builder/dockerfile/evaluator.go | 2 +- builder/dockerfile/imagecontext.go | 37 +---- builder/dockerfile/internals_test.go | 9 +- builder/remotecontext/detect.go | 11 +- cmd/dockerd/daemon.go | 4 +- 15 files changed, 337 insertions(+), 259 deletions(-) create mode 100644 api/server/backend/build/backend.go create mode 100644 api/server/backend/build/tag.go create mode 100644 api/types/backend/build.go diff --git a/api/server/backend/build/backend.go b/api/server/backend/build/backend.go new file mode 100644 index 0000000000..bf171441ca --- /dev/null +++ b/api/server/backend/build/backend.go @@ -0,0 +1,70 @@ +package build + +import ( + "fmt" + + "github.com/docker/distribution/reference" + "github.com/docker/docker/api/types/backend" + "github.com/docker/docker/builder" + "github.com/docker/docker/builder/dockerfile" + "github.com/docker/docker/image" + "github.com/docker/docker/pkg/stringid" + "github.com/pkg/errors" + "golang.org/x/net/context" +) + +// ImageComponent provides an interface for working with images +type ImageComponent interface { + SquashImage(from string, to string) (string, error) + TagImageWithReference(image.ID, reference.Named) error +} + +// Backend provides build functionality to the API router +type Backend struct { + manager *dockerfile.BuildManager + imageComponent ImageComponent +} + +// NewBackend creates a new build backend from components +func NewBackend(components ImageComponent, builderBackend builder.Backend) *Backend { + manager := dockerfile.NewBuildManager(builderBackend) + return &Backend{imageComponent: components, manager: manager} +} + +// Build builds an image from a Source +func (b *Backend) Build(ctx context.Context, config backend.BuildConfig) (string, error) { + options := config.Options + tagger, err := NewTagger(b.imageComponent, config.ProgressWriter.StdoutFormatter, options.Tags) + if err != nil { + return "", err + } + + build, err := b.manager.Build(ctx, config) + if err != nil { + return "", err + } + + var imageID = build.ImageID + if options.Squash { + if imageID, err = squashBuild(build, b.imageComponent); err != nil { + return "", err + } + } + + stdout := config.ProgressWriter.StdoutFormatter + fmt.Fprintf(stdout, "Successfully built %s\n", stringid.TruncateID(imageID)) + err = tagger.TagImages(image.ID(imageID)) + return imageID, err +} + +func squashBuild(build *builder.Result, imageComponent ImageComponent) (string, error) { + var fromID string + if build.FromImage != nil { + fromID = build.FromImage.ImageID() + } + imageID, err := imageComponent.SquashImage(build.ImageID, fromID) + if err != nil { + return "", errors.Wrap(err, "error squashing image") + } + return imageID, nil +} diff --git a/api/server/backend/build/tag.go b/api/server/backend/build/tag.go new file mode 100644 index 0000000000..5c3918a3e1 --- /dev/null +++ b/api/server/backend/build/tag.go @@ -0,0 +1,77 @@ +package build + +import ( + "fmt" + "io" + + "github.com/docker/distribution/reference" + "github.com/docker/docker/image" + "github.com/pkg/errors" +) + +// Tagger is responsible for tagging an image created by a builder +type Tagger struct { + imageComponent ImageComponent + stdout io.Writer + repoAndTags []reference.Named +} + +// NewTagger returns a new Tagger for tagging the images of a build. +// If any of the names are invalid tags an error is returned. +func NewTagger(backend ImageComponent, stdout io.Writer, names []string) (*Tagger, error) { + reposAndTags, err := sanitizeRepoAndTags(names) + if err != nil { + return nil, err + } + return &Tagger{ + imageComponent: backend, + stdout: stdout, + repoAndTags: reposAndTags, + }, nil +} + +// TagImages creates image tags for the imageID +func (bt *Tagger) TagImages(imageID image.ID) error { + for _, rt := range bt.repoAndTags { + if err := bt.imageComponent.TagImageWithReference(imageID, rt); err != nil { + return err + } + fmt.Fprintf(bt.stdout, "Successfully tagged %s\n", reference.FamiliarString(rt)) + } + return nil +} + +// sanitizeRepoAndTags parses the raw "t" parameter received from the client +// to a slice of repoAndTag. +// It also validates each repoName and tag. +func sanitizeRepoAndTags(names []string) ([]reference.Named, error) { + var ( + repoAndTags []reference.Named + // This map is used for deduplicating the "-t" parameter. + uniqNames = make(map[string]struct{}) + ) + for _, repo := range names { + if repo == "" { + continue + } + + ref, err := reference.ParseNormalizedNamed(repo) + if err != nil { + return nil, err + } + + if _, isCanonical := ref.(reference.Canonical); isCanonical { + return nil, errors.New("build tag cannot contain a digest") + } + + ref = reference.TagNameOnly(ref) + + nameWithTag := ref.String() + + if _, exists := uniqNames[nameWithTag]; !exists { + uniqNames[nameWithTag] = struct{}{} + repoAndTags = append(repoAndTags, ref) + } + } + return repoAndTags, nil +} diff --git a/api/server/router/build/backend.go b/api/server/router/build/backend.go index 5625f97157..835b11abba 100644 --- a/api/server/router/build/backend.go +++ b/api/server/router/build/backend.go @@ -1,20 +1,17 @@ package build import ( - "io" - - "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "golang.org/x/net/context" ) // Backend abstracts an image builder whose only purpose is to build an image referenced by an imageID. type Backend interface { - // BuildFromContext builds a Docker image referenced by an imageID string. - // - // Note: Tagging an image should not be done by a Builder, it should instead be done - // by the caller. - // + // Build a Docker image returning the id of the image // TODO: make this return a reference instead of string - BuildFromContext(ctx context.Context, src io.ReadCloser, buildOptions *types.ImageBuildOptions, pg backend.ProgressWriter) (string, error) + Build(context.Context, backend.BuildConfig) (string, error) +} + +type experimentalProvider interface { + HasExperimental() bool } diff --git a/api/server/router/build/build.go b/api/server/router/build/build.go index 8c9137015a..dcf0c53953 100644 --- a/api/server/router/build/build.go +++ b/api/server/router/build/build.go @@ -5,14 +5,13 @@ import "github.com/docker/docker/api/server/router" // buildRouter is a router to talk with the build controller type buildRouter struct { backend Backend + daemon experimentalProvider routes []router.Route } // NewRouter initializes a new build router -func NewRouter(b Backend) router.Router { - r := &buildRouter{ - backend: b, - } +func NewRouter(b Backend, d experimentalProvider) router.Router { + r := &buildRouter{backend: b, daemon: d} r.initRoutes() return r } diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index bbe312ef53..eea5af4a18 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -13,6 +13,7 @@ import ( "sync" "github.com/Sirupsen/logrus" + apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" @@ -22,6 +23,7 @@ import ( "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" units "github.com/docker/go-units" + "github.com/pkg/errors" "golang.org/x/net/context" ) @@ -87,9 +89,6 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui options.Ulimits = buildUlimits } - var buildArgs = map[string]*string{} - buildArgsJSON := r.FormValue("buildargs") - // Note that there are two ways a --build-arg might appear in the // json of the query param: // "foo":"bar" @@ -102,25 +101,27 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui // the fact they mentioned it, we need to pass that along to the builder // so that it can print a warning about "foo" being unused if there is // no "ARG foo" in the Dockerfile. + buildArgsJSON := r.FormValue("buildargs") if buildArgsJSON != "" { + var buildArgs = map[string]*string{} if err := json.Unmarshal([]byte(buildArgsJSON), &buildArgs); err != nil { return nil, err } options.BuildArgs = buildArgs } - var labels = map[string]string{} labelsJSON := r.FormValue("labels") if labelsJSON != "" { + var labels = map[string]string{} if err := json.Unmarshal([]byte(labelsJSON), &labels); err != nil { return nil, err } options.Labels = labels } - var cacheFrom = []string{} cacheFromJSON := r.FormValue("cachefrom") if cacheFromJSON != "" { + var cacheFrom = []string{} if err := json.Unmarshal([]byte(cacheFromJSON), &cacheFrom); err != nil { return nil, err } @@ -130,33 +131,8 @@ func newImageBuildOptions(ctx context.Context, r *http.Request) (*types.ImageBui return options, nil } -type syncWriter struct { - w io.Writer - mu sync.Mutex -} - -func (s *syncWriter) Write(b []byte) (count int, err error) { - s.mu.Lock() - count, err = s.w.Write(b) - s.mu.Unlock() - return -} - func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - var ( - authConfigs = map[string]types.AuthConfig{} - authConfigsEncoded = r.Header.Get("X-Registry-Config") - notVerboseBuffer = bytes.NewBuffer(nil) - ) - - if authConfigsEncoded != "" { - authConfigsJSON := base64.NewDecoder(base64.URLEncoding, strings.NewReader(authConfigsEncoded)) - if err := json.NewDecoder(authConfigsJSON).Decode(&authConfigs); err != nil { - // for a pull it is not an error if no auth was given - // to increase compatibility with the existing api it is defaulting - // to be empty. - } - } + var notVerboseBuffer = bytes.NewBuffer(nil) w.Header().Set("Content-Type", "application/json") @@ -183,7 +159,12 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * if err != nil { return errf(err) } - buildOptions.AuthConfigs = authConfigs + buildOptions.AuthConfigs = getAuthConfigs(r.Header) + + if buildOptions.Squash && !br.daemon.HasExperimental() { + return apierrors.NewBadRequestError( + errors.New("squash is only supported with experimental mode")) + } // Currently, only used if context is from a remote url. // Look at code in DetectContextFromRemoteURL for more information. @@ -199,18 +180,12 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * if buildOptions.SuppressOutput { out = notVerboseBuffer } - out = &syncWriter{w: out} - stdout := &streamformatter.StdoutFormatter{Writer: out, StreamFormatter: sf} - stderr := &streamformatter.StderrFormatter{Writer: out, StreamFormatter: sf} - pg := backend.ProgressWriter{ - Output: out, - StdoutFormatter: stdout, - StderrFormatter: stderr, - ProgressReaderFunc: createProgressReader, - } - - imgID, err := br.backend.BuildFromContext(ctx, r.Body, buildOptions, pg) + imgID, err := br.backend.Build(ctx, backend.BuildConfig{ + Source: r.Body, + Options: buildOptions, + ProgressWriter: buildProgressWriter(out, sf, createProgressReader), + }) if err != nil { return errf(err) } @@ -219,8 +194,47 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * // should be just the image ID and we'll print that to stdout. if buildOptions.SuppressOutput { stdout := &streamformatter.StdoutFormatter{Writer: output, StreamFormatter: sf} - fmt.Fprintf(stdout, "%s\n", string(imgID)) + fmt.Fprintln(stdout, imgID) } - return nil } + +func getAuthConfigs(header http.Header) map[string]types.AuthConfig { + authConfigs := map[string]types.AuthConfig{} + authConfigsEncoded := header.Get("X-Registry-Config") + + if authConfigsEncoded == "" { + return authConfigs + } + + authConfigsJSON := base64.NewDecoder(base64.URLEncoding, strings.NewReader(authConfigsEncoded)) + // Pulling an image does not error when no auth is provided so to remain + // consistent with the existing api decode errors are ignored + json.NewDecoder(authConfigsJSON).Decode(&authConfigs) + return authConfigs +} + +type syncWriter struct { + w io.Writer + mu sync.Mutex +} + +func (s *syncWriter) Write(b []byte) (count int, err error) { + s.mu.Lock() + count, err = s.w.Write(b) + s.mu.Unlock() + return +} + +func buildProgressWriter(out io.Writer, sf *streamformatter.StreamFormatter, createProgressReader func(io.ReadCloser) io.ReadCloser) backend.ProgressWriter { + out = &syncWriter{w: out} + stdout := &streamformatter.StdoutFormatter{Writer: out, StreamFormatter: sf} + stderr := &streamformatter.StderrFormatter{Writer: out, StreamFormatter: sf} + + return backend.ProgressWriter{ + Output: out, + StdoutFormatter: stdout, + StderrFormatter: stderr, + ProgressReaderFunc: createProgressReader, + } +} diff --git a/api/types/backend/backend.go b/api/types/backend/backend.go index de7e24f208..6dfd15e79a 100644 --- a/api/types/backend/backend.go +++ b/api/types/backend/backend.go @@ -6,7 +6,6 @@ import ( "time" "github.com/docker/docker/api/types" - "github.com/docker/docker/pkg/streamformatter" ) // ContainerAttachConfig holds the streams to use when connecting to a container to view logs. @@ -99,11 +98,3 @@ type ContainerCommitConfig struct { types.ContainerCommitConfig Changes []string } - -// ProgressWriter is a data object to transport progress streams to the client -type ProgressWriter struct { - Output io.Writer - StdoutFormatter *streamformatter.StdoutFormatter - StderrFormatter *streamformatter.StderrFormatter - ProgressReaderFunc func(io.ReadCloser) io.ReadCloser -} diff --git a/api/types/backend/build.go b/api/types/backend/build.go new file mode 100644 index 0000000000..78f955b8fc --- /dev/null +++ b/api/types/backend/build.go @@ -0,0 +1,23 @@ +package backend + +import ( + "io" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/pkg/streamformatter" +) + +// ProgressWriter is a data object to transport progress streams to the client +type ProgressWriter struct { + Output io.Writer + StdoutFormatter *streamformatter.StdoutFormatter + StderrFormatter *streamformatter.StderrFormatter + ProgressReaderFunc func(io.ReadCloser) io.ReadCloser +} + +// BuildConfig is the configuration used by a BuildManager to start a build +type BuildConfig struct { + Source io.ReadCloser + ProgressWriter ProgressWriter + Options *types.ImageBuildOptions +} diff --git a/builder/builder.go b/builder/builder.go index 2e27b4a9b2..785f0eb3c3 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -8,11 +8,9 @@ import ( "io" "time" - "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" - "github.com/docker/docker/image" "golang.org/x/net/context" ) @@ -40,8 +38,6 @@ type Backend interface { // GetImageOnBuild looks up a Docker image referenced by `name`. GetImageOnBuild(name string) (Image, error) - // TagImageWithReference tags an image with newTag - TagImageWithReference(image.ID, reference.Named) error // PullOnBuild tells Docker to pull image referenced by `name`. PullOnBuild(ctx context.Context, name string, authConfigs map[string]types.AuthConfig, output io.Writer) (Image, error) // ContainerAttachRaw attaches to container. @@ -69,12 +65,6 @@ type Backend interface { // TODO: use containerd/fs.changestream instead as a source CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error - // HasExperimental checks if the backend supports experimental features - HasExperimental() bool - - // SquashImage squashes the fs layers from the provided image down to the specified `to` image - SquashImage(from string, to string) (string, error) - // MountImage returns mounted path with rootfs of an image. MountImage(name string) (string, func() error, error) } @@ -85,6 +75,12 @@ type Image interface { RunConfig() *container.Config } +// Result is the output produced by a Builder +type Result struct { + ImageID string + FromImage Image +} + // ImageCacheBuilder represents a generator for stateful image cache. type ImageCacheBuilder interface { // MakeImageCache creates a stateful image cache. diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index a17b047ba8..6ed1fbdd8b 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -5,12 +5,9 @@ import ( "fmt" "io" "io/ioutil" - "os" "strings" "github.com/Sirupsen/logrus" - "github.com/docker/distribution/reference" - apierrors "github.com/docker/docker/api/errors" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" @@ -18,10 +15,10 @@ import ( "github.com/docker/docker/builder/dockerfile/command" "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/builder/remotecontext" - "github.com/docker/docker/image" "github.com/docker/docker/pkg/stringid" "github.com/pkg/errors" "golang.org/x/net/context" + "golang.org/x/sync/syncmap" ) var validCommitCommands = map[string]bool{ @@ -39,6 +36,52 @@ var validCommitCommands = map[string]bool{ var defaultLogConfig = container.LogConfig{Type: "none"} +// BuildManager is shared across all Builder objects +type BuildManager struct { + backend builder.Backend + pathCache pathCache // TODO: make this persistent +} + +// NewBuildManager creates a BuildManager +func NewBuildManager(b builder.Backend) *BuildManager { + return &BuildManager{backend: b, pathCache: &syncmap.Map{}} +} + +// Build starts a new build from a BuildConfig +func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (*builder.Result, error) { + if config.Options.Dockerfile == "" { + config.Options.Dockerfile = builder.DefaultDockerfileName + } + + source, dockerfile, err := remotecontext.Detect(config) + if err != nil { + return nil, err + } + if source != nil { + defer func() { + if err := source.Close(); err != nil { + logrus.Debugf("[BUILDER] failed to remove temporary context: %v", err) + } + }() + } + + builderOptions := builderOptions{ + Options: config.Options, + ProgressWriter: config.ProgressWriter, + Backend: bm.backend, + PathCache: bm.pathCache, + } + return newBuilder(ctx, builderOptions).build(source, dockerfile) +} + +// builderOptions are the dependencies required by the builder +type builderOptions struct { + Options *types.ImageBuildOptions + Backend builder.Backend + ProgressWriter backend.ProgressWriter + PathCache pathCache +} + // Builder is a Dockerfile builder // It implements the builder.Backend interface. type Builder struct { @@ -68,65 +111,25 @@ type Builder struct { from builder.Image } -// BuildManager implements builder.Backend and is shared across all Builder objects. -type BuildManager struct { - backend builder.Backend - pathCache *pathCache // TODO: make this persistent -} - -// NewBuildManager creates a BuildManager. -func NewBuildManager(b builder.Backend) (bm *BuildManager) { - return &BuildManager{backend: b, pathCache: &pathCache{}} -} - -// BuildFromContext builds a new image from a given context. -func (bm *BuildManager) BuildFromContext(ctx context.Context, src io.ReadCloser, buildOptions *types.ImageBuildOptions, pg backend.ProgressWriter) (string, error) { - if buildOptions.Squash && !bm.backend.HasExperimental() { - return "", apierrors.NewBadRequestError(errors.New("squash is only supported with experimental mode")) - } - if buildOptions.Dockerfile == "" { - buildOptions.Dockerfile = builder.DefaultDockerfileName - } - - source, dockerfile, err := remotecontext.Detect(ctx, buildOptions.RemoteContext, buildOptions.Dockerfile, src, pg.ProgressReaderFunc) - if err != nil { - return "", err - } - if source != nil { - defer func() { - if err := source.Close(); err != nil { - logrus.Debugf("[BUILDER] failed to remove temporary context: %v", err) - } - }() - } - b, err := NewBuilder(ctx, buildOptions, bm.backend, source) - if err != nil { - return "", err - } - b.imageContexts.cache = bm.pathCache - return b.build(dockerfile, pg.StdoutFormatter, pg.StderrFormatter, pg.Output) -} - -// NewBuilder creates a new Dockerfile builder from an optional dockerfile and a Config. -// If dockerfile is nil, the Dockerfile specified by Config.DockerfileName, -// will be read from the Context passed to Build(). -func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, source builder.Source) (b *Builder, err error) { +// newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options. +func newBuilder(clientCtx context.Context, options builderOptions) *Builder { + config := options.Options if config == nil { config = new(types.ImageBuildOptions) } - b = &Builder{ + b := &Builder{ clientCtx: clientCtx, options: config, - Stdout: os.Stdout, - Stderr: os.Stderr, - docker: backend, - source: source, + Stdout: options.ProgressWriter.StdoutFormatter, + Stderr: options.ProgressWriter.StderrFormatter, + Output: options.ProgressWriter.Output, + docker: options.Backend, runConfig: new(container.Config), tmpContainers: map[string]struct{}{}, buildArgs: newBuildArgs(config.BuildArgs), } - b.imageContexts = &imageContexts{b: b} - return b, nil + b.imageContexts = &imageContexts{b: b, cache: options.PathCache} + return b } func (b *Builder) resetImageCache() { @@ -137,83 +140,31 @@ func (b *Builder) resetImageCache() { b.cacheBusted = false } -// sanitizeRepoAndTags parses the raw "t" parameter received from the client -// to a slice of repoAndTag. -// It also validates each repoName and tag. -func sanitizeRepoAndTags(names []string) ([]reference.Named, error) { - var ( - repoAndTags []reference.Named - // This map is used for deduplicating the "-t" parameter. - uniqNames = make(map[string]struct{}) - ) - for _, repo := range names { - if repo == "" { - continue - } - - ref, err := reference.ParseNormalizedNamed(repo) - if err != nil { - return nil, err - } - - if _, isCanonical := ref.(reference.Canonical); isCanonical { - return nil, errors.New("build tag cannot contain a digest") - } - - ref = reference.TagNameOnly(ref) - - nameWithTag := ref.String() - - if _, exists := uniqNames[nameWithTag]; !exists { - uniqNames[nameWithTag] = struct{}{} - repoAndTags = append(repoAndTags, ref) - } - } - return repoAndTags, nil -} - -// build runs the Dockerfile builder from a context and a docker object that allows to make calls -// to Docker. -func (b *Builder) build(dockerfile *parser.Result, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) { +// Build runs the Dockerfile builder by parsing the Dockerfile and executing +// the instructions from the file. +func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*builder.Result, error) { defer b.imageContexts.unmount() - b.Stdout = stdout - b.Stderr = stderr - b.Output = out - - repoAndTags, err := sanitizeRepoAndTags(b.options.Tags) - if err != nil { - return "", err - } + // TODO: Remove source field from Builder + b.source = source addNodesForLabelOption(dockerfile.AST, b.options.Labels) if err := checkDispatchDockerfile(dockerfile.AST); err != nil { - return "", err + return nil, err } imageID, err := b.dispatchDockerfileWithCancellation(dockerfile) if err != nil { - return "", err + return nil, err } b.warnOnUnusedBuildArgs() if imageID == "" { - return "", errors.New("No image was generated. Is your Dockerfile empty?") + return nil, errors.New("No image was generated. Is your Dockerfile empty?") } - - if b.options.Squash { - if imageID, err = b.squashBuild(imageID); err != nil { - return "", err - } - } - - fmt.Fprintf(b.Stdout, "Successfully built %s\n", stringid.TruncateID(imageID)) - if err := b.tagImages(imageID, repoAndTags); err != nil { - return "", err - } - return imageID, nil + return &builder.Result{ImageID: imageID, FromImage: b.from}, nil } func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) (string, error) { @@ -258,19 +209,6 @@ func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) return imageID, nil } -func (b *Builder) squashBuild(imageID string) (string, error) { - var fromID string - var err error - if b.from != nil { - fromID = b.from.ImageID() - } - imageID, err = b.docker.SquashImage(imageID, fromID) - if err != nil { - return "", errors.Wrap(err, "error squashing image") - } - return imageID, nil -} - func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) { if len(labels) == 0 { return @@ -289,17 +227,6 @@ func (b *Builder) warnOnUnusedBuildArgs() { } } -func (b *Builder) tagImages(id string, repoAndTags []reference.Named) error { - imageID := image.ID(id) - for _, rt := range repoAndTags { - if err := b.docker.TagImageWithReference(imageID, rt); err != nil { - return err - } - fmt.Fprintf(b.Stdout, "Successfully tagged %s\n", reference.FamiliarString(rt)) - } - return nil -} - // hasFromImage returns true if the builder has processed a `FROM ` line // TODO: move to DispatchState func (b *Builder) hasFromImage() bool { @@ -316,10 +243,7 @@ func (b *Builder) hasFromImage() bool { // // TODO: Remove? func BuildFromConfig(config *container.Config, changes []string) (*container.Config, error) { - b, err := NewBuilder(context.Background(), nil, nil, nil) - if err != nil { - return nil, err - } + b := newBuilder(context.Background(), builderOptions{}) result, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n"))) if err != nil { diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index f624e815cd..7dfecce9f2 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -595,7 +595,7 @@ func healthcheck(req dispatchRequest) error { // to /usr/sbin/nginx. Uses the default shell if not in JSON format. // // Handles command processing similar to CMD and RUN, only req.runConfig.Entrypoint -// is initialized at NewBuilder time instead of through argument parsing. +// is initialized at newBuilder time instead of through argument parsing. // func entrypoint(req dispatchRequest) error { if err := req.flags.Parse(); err != nil { diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index cd4bc4dd92..abad15e096 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -2,7 +2,7 @@ // // It incorporates a dispatch table based on the parser.Node values (see the // parser package for more information) that are yielded from the parser itself. -// Calling NewBuilder with the BuildOpts struct can be used to customize the +// Calling newBuilder with the BuildOpts struct can be used to customize the // experience for execution purposes only. Parsing is controlled in the parser // package, and this division of responsibility should be respected. // diff --git a/builder/dockerfile/imagecontext.go b/builder/dockerfile/imagecontext.go index ab26782d0d..fd141137c6 100644 --- a/builder/dockerfile/imagecontext.go +++ b/builder/dockerfile/imagecontext.go @@ -3,7 +3,6 @@ package dockerfile import ( "strconv" "strings" - "sync" "github.com/Sirupsen/logrus" "github.com/docker/docker/api/types/container" @@ -12,13 +11,18 @@ import ( "github.com/pkg/errors" ) +type pathCache interface { + Load(key interface{}) (value interface{}, ok bool) + Store(key, value interface{}) +} + // imageContexts is a helper for stacking up built image rootfs and reusing // them as contexts type imageContexts struct { b *Builder list []*imageMount byName map[string]*imageMount - cache *pathCache + cache pathCache currentName string } @@ -104,14 +108,14 @@ func (ic *imageContexts) getCache(id, path string) (interface{}, bool) { if id == "" { return nil, false } - return ic.cache.get(id + path) + return ic.cache.Load(id + path) } return nil, false } func (ic *imageContexts) setCache(id, path string, v interface{}) { if ic.cache != nil { - ic.cache.set(id+path, v) + ic.cache.Store(id+path, v) } } @@ -160,28 +164,3 @@ func (im *imageMount) ImageID() string { func (im *imageMount) RunConfig() *container.Config { return im.runConfig } - -type pathCache struct { - mu sync.Mutex - items map[string]interface{} -} - -func (c *pathCache) set(k string, v interface{}) { - c.mu.Lock() - if c.items == nil { - c.items = make(map[string]interface{}) - } - c.items[k] = v - c.mu.Unlock() -} - -func (c *pathCache) get(k string) (interface{}, bool) { - c.mu.Lock() - if c.items == nil { - c.mu.Unlock() - return nil, false - } - v, ok := c.items[k] - c.mu.Unlock() - return v, ok -} diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go index 3119576788..4cba2c9842 100644 --- a/builder/dockerfile/internals_test.go +++ b/builder/dockerfile/internals_test.go @@ -1,10 +1,11 @@ package dockerfile import ( - "context" "fmt" "testing" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder" "github.com/docker/docker/builder/remotecontext" @@ -69,7 +70,11 @@ func readAndCheckDockerfile(t *testing.T, testName, contextDir, dockerfilePath, dockerfilePath = builder.DefaultDockerfileName } - _, _, err = remotecontext.Detect(context.Background(), "", dockerfilePath, tarStream, nil) + config := backend.BuildConfig{ + Options: &types.ImageBuildOptions{Dockerfile: dockerfilePath}, + Source: tarStream, + } + _, _, err = remotecontext.Detect(config) assert.EqualError(t, err, expectedError) } diff --git a/builder/remotecontext/detect.go b/builder/remotecontext/detect.go index ef21de21d3..d398004100 100644 --- a/builder/remotecontext/detect.go +++ b/builder/remotecontext/detect.go @@ -2,7 +2,6 @@ package remotecontext import ( "bufio" - "context" "fmt" "io" "os" @@ -10,6 +9,7 @@ import ( "strings" "github.com/Sirupsen/logrus" + "github.com/docker/docker/api/types/backend" "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/builder/dockerignore" @@ -23,14 +23,17 @@ import ( // Detect returns a context and dockerfile from remote location or local // archive. progressReader is only used if remoteURL is actually a URL // (not empty, and not a Git endpoint). -func Detect(ctx context.Context, remoteURL string, dockerfilePath string, r io.ReadCloser, progressReader func(in io.ReadCloser) io.ReadCloser) (remote builder.Source, dockerfile *parser.Result, err error) { +func Detect(config backend.BuildConfig) (remote builder.Source, dockerfile *parser.Result, err error) { + remoteURL := config.Options.RemoteContext + dockerfilePath := config.Options.Dockerfile + switch { case remoteURL == "": - remote, dockerfile, err = newArchiveRemote(r, dockerfilePath) + remote, dockerfile, err = newArchiveRemote(config.Source, dockerfilePath) case urlutil.IsGitURL(remoteURL): remote, dockerfile, err = newGitRemote(remoteURL, dockerfilePath) case urlutil.IsURL(remoteURL): - remote, dockerfile, err = newURLRemote(remoteURL, dockerfilePath, progressReader) + remote, dockerfile, err = newURLRemote(remoteURL, dockerfilePath, config.ProgressWriter.ProgressReaderFunc) default: err = fmt.Errorf("remoteURL (%s) could not be recognized as URL", remoteURL) } diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 56405541bf..93f1e59f95 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -14,6 +14,7 @@ import ( "github.com/docker/distribution/uuid" "github.com/docker/docker/api" apiserver "github.com/docker/docker/api/server" + buildbackend "github.com/docker/docker/api/server/backend/build" "github.com/docker/docker/api/server/middleware" "github.com/docker/docker/api/server/router" "github.com/docker/docker/api/server/router/build" @@ -25,7 +26,6 @@ import ( swarmrouter "github.com/docker/docker/api/server/router/swarm" systemrouter "github.com/docker/docker/api/server/router/system" "github.com/docker/docker/api/server/router/volume" - "github.com/docker/docker/builder/dockerfile" cliconfig "github.com/docker/docker/cli/config" "github.com/docker/docker/cli/debug" cliflags "github.com/docker/docker/cli/flags" @@ -484,7 +484,7 @@ func initRouter(s *apiserver.Server, d *daemon.Daemon, c *cluster.Cluster) { image.NewRouter(d, decoder), systemrouter.NewRouter(d, c), volume.NewRouter(d), - build.NewRouter(dockerfile.NewBuildManager(d)), + build.NewRouter(buildbackend.NewBackend(d, d), d), swarmrouter.NewRouter(c), pluginrouter.NewRouter(d.PluginManager()), }