From 6678a26d1c94e7838c055a3da3b91ae3de8c3e3c Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 15:11:30 -0700 Subject: [PATCH 1/7] gofmt --- api_test.go | 2 +- container.go | 10 +++++----- network.go | 2 +- server.go | 2 +- utils/utils_test.go | 3 +-- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/api_test.go b/api_test.go index 1e8ed20b98..0a627f2e85 100644 --- a/api_test.go +++ b/api_test.go @@ -449,7 +449,7 @@ func TestGetContainersChanges(t *testing.T) { } func TestGetContainersTop(t *testing.T) { - t.Skip("Fixme. Skipping test for now. Reported error when testing using dind: 'api_test.go:527: Expected 2 processes, found 0.'") + t.Skip("Fixme. Skipping test for now. Reported error when testing using dind: 'api_test.go:527: Expected 2 processes, found 0.'") runtime, err := newTestRuntime() if err != nil { t.Fatal(err) diff --git a/container.go b/container.go index 39c19c53d4..5799fa4e12 100644 --- a/container.go +++ b/container.go @@ -11,6 +11,7 @@ import ( "io" "io/ioutil" "log" + "net" "os" "os/exec" "path" @@ -20,7 +21,6 @@ import ( "strings" "syscall" "time" - "net" ) type Container struct { @@ -813,10 +813,10 @@ func (container *Container) allocateNetwork() error { iface = &NetworkInterface{disabled: true} } else { iface = &NetworkInterface{ - IPNet: net.IPNet{IP: net.ParseIP(container.NetworkSettings.IPAddress), Mask: manager.bridgeNetwork.Mask}, + IPNet: net.IPNet{IP: net.ParseIP(container.NetworkSettings.IPAddress), Mask: manager.bridgeNetwork.Mask}, Gateway: manager.bridgeNetwork.IP, manager: manager, - } + } ipNum := ipToInt(iface.IPNet.IP) manager.ipAllocator.inUse[ipNum] = struct{}{} } @@ -827,10 +827,10 @@ func (container *Container) allocateNetwork() error { portSpecs = container.Config.PortSpecs } else { for backend, frontend := range container.NetworkSettings.PortMapping["Tcp"] { - portSpecs = append(portSpecs, fmt.Sprintf("%s:%s/tcp",frontend, backend)) + portSpecs = append(portSpecs, fmt.Sprintf("%s:%s/tcp", frontend, backend)) } for backend, frontend := range container.NetworkSettings.PortMapping["Udp"] { - portSpecs = append(portSpecs, fmt.Sprintf("%s:%s/udp",frontend, backend)) + portSpecs = append(portSpecs, fmt.Sprintf("%s:%s/udp", frontend, backend)) } } diff --git a/network.go b/network.go index c2673bd803..b552919253 100644 --- a/network.go +++ b/network.go @@ -642,7 +642,7 @@ func (manager *NetworkManager) Allocate() (*NetworkInterface, error) { if err != nil { return nil, err } - // avoid duplicate IP + // avoid duplicate IP ipNum := ipToInt(ip) firstIP := manager.ipAllocator.network.IP.To4().Mask(manager.ipAllocator.network.Mask) firstIPNum := ipToInt(firstIP) + 1 diff --git a/server.go b/server.go index d2c4e5fa59..129cace8ab 100644 --- a/server.go +++ b/server.go @@ -679,7 +679,7 @@ func (srv *Server) getImageList(localRepo map[string]string) ([][]*registry.ImgD depGraph.NewNode(img.ID) img.WalkHistory(func(current *Image) error { imgList[current.ID] = ®istry.ImgData{ - ID: current.ID, + ID: current.ID, Tag: tag, } parent, err := current.GetParent() diff --git a/utils/utils_test.go b/utils/utils_test.go index be796b2381..9a55e7f62d 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -366,7 +366,6 @@ func TestParseRelease(t *testing.T) { assertParseRelease(t, "3.8.0-19-generic", &KernelVersionInfo{Kernel: 3, Major: 8, Minor: 0, Flavor: "19-generic"}, 0) } - func TestDependencyGraphCircular(t *testing.T) { g1 := NewDependencyGraph() a := g1.NewNode("a") @@ -421,4 +420,4 @@ func TestDependencyGraph(t *testing.T) { if len(res[2]) != 1 || res[2][0] != "d" { t.Fatalf("Expected [d], found %v instead", res[2]) } -} \ No newline at end of file +} From 672f1e068392e95966cd57b4bc49bd8d1b8859dd Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 15:21:04 -0700 Subject: [PATCH 2/7] failing test case for multiline command in dockerfile --- buildfile_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/buildfile_test.go b/buildfile_test.go index 14986161d8..aa2cfa4fdf 100644 --- a/buildfile_test.go +++ b/buildfile_test.go @@ -45,6 +45,21 @@ run [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ] nil, }, + // Exactly the same as above, except uses a line split with a \ to test + // multiline support. + { + ` +from {IMAGE} +run sh -c 'echo root:testpass \ + > /tmp/passwd' +run mkdir -p /var/run/sshd +run [ "$(cat /tmp/passwd)" = "root:testpass" ] +run [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ] +`, + nil, + nil, + }, + { ` from {IMAGE} From 6921ca4813a583948d3871a53d0349b834f31036 Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 16:42:04 -0700 Subject: [PATCH 3/7] read Dockerfile into memory before parsing, to facilitate regexp preprocessing --- buildfile.go | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/buildfile.go b/buildfile.go index 4c8db2c60e..5472b84982 100644 --- a/buildfile.go +++ b/buildfile.go @@ -1,7 +1,6 @@ package docker import ( - "bufio" "encoding/json" "fmt" "github.com/dotcloud/docker/utils" @@ -459,6 +458,8 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error { return nil } +var multilineRegex = regexp.MustCompile("\\.*\n") + func (b *buildFile) Build(context io.Reader) (string, error) { // FIXME: @creack any reason for using /tmp instead of ""? // FIXME: @creack "name" is a terrible variable name @@ -471,22 +472,26 @@ func (b *buildFile) Build(context io.Reader) (string, error) { } defer os.RemoveAll(name) b.context = name - dockerfile, err := os.Open(path.Join(name, "Dockerfile")) - if err != nil { + filename := path.Join(name, "Dockerfile") + if _, err := os.Stat(filename); os.IsNotExist(err) { return "", fmt.Errorf("Can't build a directory with no Dockerfile") } - // FIXME: "file" is also a terrible variable name ;) - file := bufio.NewReader(dockerfile) + fileBytes, err := ioutil.ReadFile(filename) + if err != nil { + return "", err + } + dockerfile := string(fileBytes) + // dockerfile = multilineRegex.ReplaceAllString(dockerfile, " ") stepN := 0 - for { - line, err := file.ReadString('\n') - if err != nil { - if err == io.EOF && line == "" { - break - } else if err != io.EOF { - return "", err - } - } + for _, line := range strings.Split(dockerfile, "\n") { + /* line, err := dockerfile.ReadString('\n') + if err != nil { + if err == io.EOF && line == "" { + break + } else if err != io.EOF { + return "", err + } + }*/ line = strings.Trim(strings.Replace(line, "\t", " ", -1), " \t\r\n") // Skip comments and empty line if len(line) == 0 || line[0] == '#' { From ebb934c1b044f4bbffe312e9c88c00134c94ef7f Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 17:02:45 -0700 Subject: [PATCH 4/7] line continuation regex --- buildfile.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/buildfile.go b/buildfile.go index 5472b84982..c40ee5f063 100644 --- a/buildfile.go +++ b/buildfile.go @@ -458,7 +458,8 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error { return nil } -var multilineRegex = regexp.MustCompile("\\.*\n") +// Long lines can be split with a backslash +var lineContinuation = regexp.MustCompile(`\s*\\.*\n`) func (b *buildFile) Build(context io.Reader) (string, error) { // FIXME: @creack any reason for using /tmp instead of ""? @@ -481,7 +482,7 @@ func (b *buildFile) Build(context io.Reader) (string, error) { return "", err } dockerfile := string(fileBytes) - // dockerfile = multilineRegex.ReplaceAllString(dockerfile, " ") + dockerfile = lineContinuation.ReplaceAllString(dockerfile, " ") stepN := 0 for _, line := range strings.Split(dockerfile, "\n") { /* line, err := dockerfile.ReadString('\n') From 4f3b8033f287051156e6b81aeff9da3c44f64cba Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 17:17:09 -0700 Subject: [PATCH 5/7] cruft removal --- buildfile.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/buildfile.go b/buildfile.go index c40ee5f063..cbb6ecfcef 100644 --- a/buildfile.go +++ b/buildfile.go @@ -485,14 +485,6 @@ func (b *buildFile) Build(context io.Reader) (string, error) { dockerfile = lineContinuation.ReplaceAllString(dockerfile, " ") stepN := 0 for _, line := range strings.Split(dockerfile, "\n") { - /* line, err := dockerfile.ReadString('\n') - if err != nil { - if err == io.EOF && line == "" { - break - } else if err != io.EOF { - return "", err - } - }*/ line = strings.Trim(strings.Replace(line, "\t", " ", -1), " \t\r\n") // Skip comments and empty line if len(line) == 0 || line[0] == '#' { From c01f6df43e1280c398d921c2d5eff01db8c26f94 Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 18:22:58 -0700 Subject: [PATCH 6/7] Test dockerfile with line containing literal "\n" --- buildfile_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/buildfile_test.go b/buildfile_test.go index aa2cfa4fdf..bdc3680386 100644 --- a/buildfile_test.go +++ b/buildfile_test.go @@ -60,6 +60,19 @@ run [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ] nil, }, + // Line containing literal "\n" + { + ` +from {IMAGE} +run sh -c 'echo root:testpass > /tmp/passwd' +run echo "foo \n bar"; echo "baz" +run mkdir -p /var/run/sshd +run [ "$(cat /tmp/passwd)" = "root:testpass" ] +run [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ] +`, + nil, + nil, + }, { ` from {IMAGE} From 6a4afb7f8eacde9cb81202347258983908a9f7c6 Mon Sep 17 00:00:00 2001 From: Jason McVetta Date: Mon, 9 Sep 2013 18:23:42 -0700 Subject: [PATCH 7/7] stricter regexp for Dockerfile line continuations --- buildfile.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildfile.go b/buildfile.go index cbb6ecfcef..8ff21bf939 100644 --- a/buildfile.go +++ b/buildfile.go @@ -459,7 +459,7 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error { } // Long lines can be split with a backslash -var lineContinuation = regexp.MustCompile(`\s*\\.*\n`) +var lineContinuation = regexp.MustCompile(`\s*\\\s*\n`) func (b *buildFile) Build(context io.Reader) (string, error) { // FIXME: @creack any reason for using /tmp instead of ""?