diff --git a/builder/dispatchers.go b/builder/dispatchers.go index db7476c5ed..eb9485fa79 100644 --- a/builder/dispatchers.go +++ b/builder/dispatchers.go @@ -12,6 +12,7 @@ import ( "io/ioutil" "path/filepath" "regexp" + "sort" "strings" log "github.com/Sirupsen/logrus" @@ -326,14 +327,21 @@ func expose(b *Builder, args []string, attributes map[string]bool, original stri return err } + // instead of using ports directly, we build a list of ports and sort it so + // the order is consistent. This prevents cache burst where map ordering + // changes between builds + portList := make([]string, len(ports)) + var i int for port := range ports { if _, exists := b.Config.ExposedPorts[port]; !exists { b.Config.ExposedPorts[port] = struct{}{} } + portList[i] = string(port) + i++ } + sort.Strings(portList) b.Config.PortSpecs = nil - - return b.commit("", b.Config.Cmd, fmt.Sprintf("EXPOSE %v", ports)) + return b.commit("", b.Config.Cmd, fmt.Sprintf("EXPOSE %s", strings.Join(portList, " "))) } // USER foo diff --git a/builder/evaluator.go b/builder/evaluator.go index 3d9ebb162c..4ed66c0054 100644 --- a/builder/evaluator.go +++ b/builder/evaluator.go @@ -212,6 +212,21 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { msg += " " + ast.Value } + // count the number of nodes that we are going to traverse first + // so we can pre-create the argument and message array. This speeds up the + // allocation of those list a lot when they have a lot of arguments + cursor := ast + var n int + for cursor.Next != nil { + cursor = cursor.Next + n++ + } + l := len(strs) + strList := make([]string, n+l) + copy(strList, strs) + msgList := make([]string, n) + + var i int for ast.Next != nil { ast = ast.Next var str string @@ -219,16 +234,18 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error { if _, ok := replaceEnvAllowed[cmd]; ok { str = b.replaceEnv(ast.Value) } - strs = append(strs, str) - msg += " " + ast.Value + strList[i+l] = str + msgList[i] = ast.Value + i++ } + msg += " " + strings.Join(msgList, " ") fmt.Fprintln(b.OutStream, msg) // XXX yes, we skip any cmds that are not valid; the parser should have // picked these out already. if f, ok := evaluateTable[cmd]; ok { - return f(b, strs, attrs, original) + return f(b, strList, attrs, original) } fmt.Fprintf(b.ErrStream, "# Skipping unknown instruction %s\n", strings.ToUpper(cmd)) diff --git a/daemon/daemon.go b/daemon/daemon.go index a2e6a79bd6..f6a5f317a0 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1099,9 +1099,9 @@ func (daemon *Daemon) ImageGetCached(imgID string, config *runconfig.Config) (*i // Loop on the children of the given image and check the config var match *image.Image for elem := range imageMap[imgID] { - img, err := daemon.Graph().Get(elem) - if err != nil { - return nil, err + img, ok := images[elem] + if !ok { + return nil, fmt.Errorf("unable to find image %q", elem) } if runconfig.Compare(&img.ContainerConfig, config) { if match == nil || match.Created.Before(img.Created) { diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 49905d204a..f2093a927f 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2,6 +2,7 @@ package main import ( "archive/tar" + "bytes" "encoding/json" "fmt" "io/ioutil" @@ -11,9 +12,11 @@ import ( "path/filepath" "reflect" "regexp" + "strconv" "strings" "syscall" "testing" + "text/template" "time" "github.com/docker/docker/pkg/archive" @@ -1942,6 +1945,85 @@ func TestBuildExpose(t *testing.T) { logDone("build - expose") } +func TestBuildExposeMorePorts(t *testing.T) { + // start building docker file with a large number of ports + portList := make([]string, 50) + line := make([]string, 100) + expectedPorts := make([]int, len(portList)*len(line)) + for i := 0; i < len(portList); i++ { + for j := 0; j < len(line); j++ { + p := i*len(line) + j + 1 + line[j] = strconv.Itoa(p) + expectedPorts[p-1] = p + } + if i == len(portList)-1 { + portList[i] = strings.Join(line, " ") + } else { + portList[i] = strings.Join(line, " ") + ` \` + } + } + + dockerfile := `FROM scratch + EXPOSE {{range .}} {{.}} + {{end}}` + tmpl := template.Must(template.New("dockerfile").Parse(dockerfile)) + buf := bytes.NewBuffer(nil) + tmpl.Execute(buf, portList) + + name := "testbuildexpose" + defer deleteImages(name) + _, err := buildImage(name, buf.String(), true) + if err != nil { + t.Fatal(err) + } + + // check if all the ports are saved inside Config.ExposedPorts + res, err := inspectFieldJSON(name, "Config.ExposedPorts") + if err != nil { + t.Fatal(err) + } + var exposedPorts map[string]interface{} + if err := json.Unmarshal([]byte(res), &exposedPorts); err != nil { + t.Fatal(err) + } + + for _, p := range expectedPorts { + ep := fmt.Sprintf("%d/tcp", p) + if _, ok := exposedPorts[ep]; !ok { + t.Errorf("Port(%s) is not exposed", ep) + } else { + delete(exposedPorts, ep) + } + } + if len(exposedPorts) != 0 { + t.Errorf("Unexpected extra exposed ports %v", exposedPorts) + } + logDone("build - expose large number of ports") +} + +func TestBuildExposeOrder(t *testing.T) { + buildID := func(name, exposed string) string { + _, err := buildImage(name, fmt.Sprintf(`FROM scratch + EXPOSE %s`, exposed), true) + if err != nil { + t.Fatal(err) + } + id, err := inspectField(name, "Id") + if err != nil { + t.Fatal(err) + } + return id + } + + id1 := buildID("testbuildexpose1", "80 2375") + id2 := buildID("testbuildexpose2", "2375 80") + defer deleteImages("testbuildexpose1", "testbuildexpose2") + if id1 != id2 { + t.Errorf("EXPOSE should invalidate the cache only when ports actually changed") + } + logDone("build - expose order") +} + func TestBuildEmptyEntrypointInheritance(t *testing.T) { name := "testbuildentrypointinheritance" name2 := "testbuildentrypointinheritance2"